Relations support for Z.DataObjectUtilities.diff() and applyChanges()

chrome/content/zotero/xpcom/data/dataObjectUtilities.js

@@ -259,7 +259,9 @@ Zotero.DataObjectUtilities = {
 			
 			// All remaining fields don't exist in data1
 			
-			if (data2[field] === false || data2[field] === "") {
+			let val = data2[field];
+			if (val === false || val === ""
+					|| (typeof val == 'object' && Object.keys(val).length == 0)) {
 				continue;
 			}
 			
@@ -351,10 +353,38 @@ Zotero.DataObjectUtilities = {
 	},
 	
 	_relationsDiff: function (data1, data2) {
-		if (!data1.length && !data2.length) {
-			return [];
+		data2 = data2 || {};
+		var changeset = [];
+		for (let pred in data1) {
+			let vals1 = typeof data1[pred] == 'string' ? [data1[pred]] : data1[pred];
+			let vals2 = (!data2[pred] || data2[pred] === '')
+				? []
+				: typeof data2[pred] == 'string' ? [data2[pred]] : data2[pred];
+			
+			var removed = Zotero.Utilities.arrayDiff(vals1, vals2);
+			for (let i = 0; i < removed.length; i++) {
+				changeset.push({
+					field: "relations",
+					op: "property-member-remove",
+					value: {
+						key: pred,
+						value: removed[i]
+					}
+				});
+			}
+			let added = Zotero.Utilities.arrayDiff(vals2, vals1);
+			for (let i = 0; i < added.length; i++) {
+				changeset.push({
+					field: "relations",
+					op: "property-member-add",
+					value: {
+						key: pred,
+						value: added[i]
+					}
+				});
+			}
 		}
-		throw new Error("Unimplemented");
+		return changeset;
 	},
 	
 	
@@ -385,10 +415,6 @@ Zotero.DataObjectUtilities = {
 					throw new Error("Unimplemented");
 					break;
 				
-				case 'relations':
-					throw new Error("Unimplemented");
-					break;
-				
 				case 'tags':
 					let found = false;
 					for (let i = 0; i < json[c.field].length; i++) {
@@ -403,7 +429,7 @@ Zotero.DataObjectUtilities = {
 					break;
 					
 				default:
-					throw new Error("Unexpected field");
+					throw new Error("Unexpected field '" + c.field + "'");
 				}
 			}
 			else if (c.op == 'member-remove') {
@@ -430,12 +456,67 @@ Zotero.DataObjectUtilities = {
 					break;
 					
 				default:
-					throw new Error("Unexpected field");
+					throw new Error("Unexpected field '" + c.field + "'");
+				}
+			}
+			else if (c.op == 'property-member-add') {
+				switch (c.field) {
+				case 'relations':
+					let obj = json[c.field];
+					let prop = c.value.key;
+					let val = c.value.value;
+					if (!obj) {
+						obj = json[c.field] = {};
+					}
+					if (!obj[prop]) {
+						obj[prop] = [];
+					}
+					// Convert string to array
+					if (typeof obj[prop] == 'string') {
+						obj[prop] = [obj[prop]];
+					}
+					if (obj[prop].indexOf(val) == -1) {
+						obj[prop].push(val);
+					}
+					break;
+					
+				default:
+					throw new Error("Unexpected field '" + c.field + "'");
+				}
+			}
+			else if (c.op == 'property-member-remove') {
+				switch (c.field) {
+				case 'relations':
+					let obj = json[c.field];
+					let prop = c.value.key;
+					let val = c.value.value;
+					if (!obj || !obj[prop]) {
+						continue;
+					}
+					if (typeof obj[prop] == 'string') {
+						// If propetty was the specified string, remove property
+						if (obj[prop] === val) {
+							delete obj[prop];
+						}
+						continue;
+					}
+					let pos = obj[prop].indexOf(val);
+					if (pos == -1) {
+						continue;
+					}
+					obj[prop].splice(pos, 1);
+					// If no more members in property array, remove property
+					if (obj[prop].length == 0) {
+						delete obj[prop];
+					}
+					break;
+					
+				default:
+					throw new Error("Unexpected field '" + c.field + "'");
 				}
 			}
-			// TODO: properties
 			else {
-				throw new Error("Unimplemented");
+				throw new Error("Unexpected change operation '" + c.op + "'");
 			}
 		}
 	}

test/tests/dataObjectUtilitiesTest.js

@@ -1,277 +1,544 @@
 "use strict";
 
 describe("Zotero.DataObjectUtilities", function() {
+	// This is mostly covered by syncLocal::_reconcileChanges() tests, but we test some
+	// additional things here
 	describe("#diff()", function () {
-		it("should not show empty items as different", function* () {
-			var id1, id2, json1, json2;
-			yield Zotero.DB.executeTransaction(function* () {
-				var item = new Zotero.Item('book');
-				id1 = yield item.save();
-				item = yield Zotero.Items.getAsync(id1);
-				json1 = yield item.toJSON();
+		//
+		// Fields
+		//
+		describe("fields", function () {
+			it("should not show empty items as different", function* () {
+				var id1, id2, json1, json2;
+				yield Zotero.DB.executeTransaction(function* () {
+					var item = new Zotero.Item('book');
+					id1 = yield item.save();
+					item = yield Zotero.Items.getAsync(id1);
+					json1 = yield item.toJSON();
+					
+					var item = new Zotero.Item('book');
+					id2 = yield item.save();
+					item = yield Zotero.Items.getAsync(id2);
+					json2 = yield item.toJSON();
+				});
 				
-				var item = new Zotero.Item('book');
-				id2 = yield item.save();
-				item = yield Zotero.Items.getAsync(id2);
-				json2 = yield item.toJSON();
-			});
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+				
+				yield Zotero.Items.erase(id1, id2);
+			})
 			
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.lengthOf(changes, 0);
+			it("should not show empty strings as different", function () {
+				var json1 = {
+					title: ""
+				};
+				var json2 = {
+					title: ""
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+			})
 			
-			yield Zotero.Items.erase(id1, id2);
+			it("should not show empty string and undefined as different", function () {
+				var json1 = {
+					title: ""
+				};
+				var json2 = {
+					place: ""
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+			})
 		})
 		
-		it("should not show empty strings as different", function* () {
-			var json1 = {
-				title: ""
-			};
-			var json2 = {
-				title: ""
-			};
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.lengthOf(changes, 0);
+		//
+		// Creators
+		//
+		describe("creators", function () {
+			it("should not show identical creators as different", function () {
+				var json1 = {
+					creators: [
+						{
+							name: "Center for History and New Media",
+							creatorType: "author"
+						}
+					]
+				};
+				var json2 = {
+					creators: [
+						{
+							creatorType: "author",
+							name: "Center for History and New Media"
+						}
+					]
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+			})
+			
+			it("should not show an empty creators array and a missing one as different", function () {
+				var json1 = {
+					creators: []
+				};
+				var json2 = {};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+				
+				var json1 = {};
+				var json2 = {
+					creators: []
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+	
+			})
 		})
 		
-		it("should not show empty string and undefined as different", function* () {
-			var json1 = {
-				title: ""
-			};
-			var json2 = {
-				place: ""
-			};
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.lengthOf(changes, 0);
+		//
+		// Relations
+		//
+		describe("relations", function () {
+			it("should not show an empty relations object and a missing one as different", function () {
+				var json1 = {
+					relations: {}
+				};
+				var json2 = {
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				Zotero.debug(changes);
+				assert.lengthOf(changes, 0);
+				
+				var json1 = {};
+				var json2 = {
+					relations: {}
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				Zotero.debug(changes);
+				assert.lengthOf(changes, 0);
+			})
 		})
 		
-		it("should not show identical creators as different", function* () {
-			var json1 = {
-				creators: [
-					{
-						name: "Center for History and New Media",
-						creatorType: "author"
-					}
-				]
-			};
-			var json2 = {
-				creators: [
-					{
-						creatorType: "author",
-						name: "Center for History and New Media"
-					}
-				]
-			};
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.lengthOf(changes, 0);
-		})
-		
-		it("should not show manual tags with or without 'type' property as different", function* () {
-			var json1 = {
-				tags: [
-					{
-						tag: "Foo"
-					}
-				]
-			};
-			var json2 = {
-				tags: [
-					{
-						tag: "Foo",
-						type: 0
-					}
-				]
-			};
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.lengthOf(changes, 0);
-		})
-		
-		it("should show tags of different types as different", function* () {
-			var json1 = {
-				tags: [
-					{
-						tag: "Foo"
-					}
-				]
-			};
-			var json2 = {
-				tags: [
-					{
-						tag: "Foo",
-						type: 1
-					}
-				]
-			};
-			var changes = Zotero.DataObjectUtilities.diff(json1, json2);
-			assert.sameDeepMembers(
-				changes,
-				[
-					{
-						field: "tags",
-						op: "member-remove",
-						value: {
+		//
+		// Tags
+		//
+		describe("tags", function () {
+			it("should not show manual tags with or without 'type' property as different", function () {
+				var json1 = {
+					tags: [
+						{
 							tag: "Foo"
 						}
+					]
+				};
+				var json2 = {
+					tags: [
+						{
+							tag: "Foo",
+							type: 0
+						}
+					]
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.lengthOf(changes, 0);
+			})
+			
+			it("should show tags of different types as different", function () {
+				var json1 = {
+					tags: [
+						{
+							tag: "Foo"
+						}
+					]
+				};
+				var json2 = {
+					tags: [
+						{
+							tag: "Foo",
+							type: 1
+						}
+					]
+				};
+				var changes = Zotero.DataObjectUtilities.diff(json1, json2);
+				assert.sameDeepMembers(
+					changes,
+					[
+						{
+							field: "tags",
+							op: "member-remove",
+							value: {
+								tag: "Foo"
+							}
+						},
+						{
+							field: "tags",
+							op: "member-add",
+							value: {
+								tag: "Foo",
+								type: 1
+							}
+						}
+					]
+				);
+			})
+		})
+	})
+	
+	
+	describe("#applyChanges()", function () {
+		//
+		// Fields
+		//
+		describe("fields", function () {
+			it("should set added/modified field values", function () {
+				var json = {
+					title: "A"
+				};
+				var changes = [
+					{
+						field: "title",
+						op: "add",
+						value: "B"
 					},
+					{
+						field: "date",
+						op: "modify",
+						value: "2015-05-19"
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.equal(json.title, "B");
+				assert.equal(json.date, "2015-05-19");
+			})
+		})
+		
+		//
+		// Collections
+		//
+		describe("collections", function () {
+			it("should add a collection", function () {
+				var json = {
+					collections: ["AAAAAAAA"]
+				};
+				var changes = [
+					{
+						field: "collections",
+						op: "member-add",
+						value: "BBBBBBBB"
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameMembers(json.collections, ["AAAAAAAA", "BBBBBBBB"]);
+			})
+			
+			it("should not duplicate an existing collection", function () {
+				var json = {
+					collections: ["AAAAAAAA"]
+				};
+				var changes = [
+					{
+						field: "collections",
+						op: "member-add",
+						value: "AAAAAAAA"
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameMembers(json.collections, ["AAAAAAAA"]);
+				assert.lengthOf(json.collections, 1);
+			})
+			
+			it("should remove a collection", function () {
+				var json = {
+					collections: ["AAAAAAAA"]
+				};
+				var changes = [
+					{
+						field: "collections",
+						op: "member-remove",
+						value: "AAAAAAAA"
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.lengthOf(json.collections, 0);
+			})
+		})
+		
+		//
+		// Relations
+		//
+		describe("relations", function () {
+			it("should add a predicate and object to an empty relations object", function () {
+				var json = {
+					relations: {}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-add",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ["A"] }]);
+			})
+			
+			it("should add a predicate and object to a missing relations object", function () {
+				var json = {};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-add",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ["A"] }]);
+			})
+			
+			it("should add an object to an existing predicate string", function () {
+				var json = {
+					relations: {
+						a: 'A1'
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-add",
+						value: {
+							key: "a",
+							value: "A2"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ["A1", "A2"] }]);
+			})
+			
+			it("should add an object to an existing predicate array", function () {
+				var json = {
+					relations: {
+						a: ['A1']
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-add",
+						value: {
+							key: "a",
+							value: "A2"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ['A1', 'A2'] }]);
+			})
+			
+			it("should ignore a removal for an missing relations object", function () {
+				var json = {};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.notProperty(json, 'relations');
+			})
+			
+			it("should ignore a removal for a missing relations predicate", function () {
+				var json = {
+					relations: {}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.lengthOf(Object.keys(json.relations), 0);
+			})
+			
+			it("should ignore a removal for a missing object", function () {
+				var json = {
+					relations: {
+						a: ['A1']
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A2"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ['A1'] }]);
+			})
+			
+			it("should remove a predicate and object string from a relations object", function () {
+				var json = {
+					relations: {
+						a: "A"
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.lengthOf(Object.keys(json.relations), 0);
+			})
+			
+			it("should remove a predicate and object array from a relations object", function () {
+				var json = {
+					relations: {
+						a: ["A"]
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.lengthOf(Object.keys(json.relations), 0);
+			})
+			
+			it("should remove an object from an existing predicate array", function () {
+				var json = {
+					relations: {
+						a: ['A1', 'A2']
+					}
+				};
+				var changes = [
+					{
+						field: "relations",
+						op: "property-member-remove",
+						value: {
+							key: "a",
+							value: "A2"
+						}
+					}
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers([json.relations], [{ a: ["A1"] }]);
+			})
+		})
+		
+		//
+		// Tags
+		//
+		describe("tags", function () {
+			it("should add a tag", function () {
+				var json = {
+					tags: [
+						{
+							tag: "A"
+						}
+					]
+				};
+				var changes = [
 					{
 						field: "tags",
 						op: "member-add",
 						value: {
-							tag: "Foo",
-							type: 1
+							tag: "B"
 						}
 					}
-				]
-			);
-		})
-	})
-	
-	describe("#applyChanges()", function () {
-		it("should set added/modified field values", function* () {
-			var json = {
-				title: "A"
-			};
-			var changes = [
-				{
-					field: "title",
-					op: "add",
-					value: "B"
-				},
-				{
-					field: "date",
-					op: "modify",
-					value: "2015-05-19"
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.equal(json.title, "B");
-			assert.equal(json.date, "2015-05-19");
-		})
-		
-		it("should add a collection", function* () {
-			var json = {
-				collections: ["AAAAAAAA"]
-			};
-			var changes = [
-				{
-					field: "collections",
-					op: "member-add",
-					value: "BBBBBBBB"
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.sameMembers(json.collections, ["AAAAAAAA", "BBBBBBBB"]);
-		})
-		
-		it("should not duplicate an existing collection", function* () {
-			var json = {
-				collections: ["AAAAAAAA"]
-			};
-			var changes = [
-				{
-					field: "collections",
-					op: "member-add",
-					value: "AAAAAAAA"
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.sameMembers(json.collections, ["AAAAAAAA"]);
-			assert.lengthOf(json.collections, 1);
-		})
-		
-		it("should remove a collection", function* () {
-			var json = {
-				collections: ["AAAAAAAA"]
-			};
-			var changes = [
-				{
-					field: "collections",
-					op: "member-remove",
-					value: "AAAAAAAA"
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.lengthOf(json.collections, 0);
-		})
-		
-		it("should add a tag", function* () {
-			var json = {
-				tags: [
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers(
+					json.tags,
+					[
+						{
+							tag: "A"
+						},
+						{
+							tag: "B"
+						}
+					]
+				);
+			})
+			
+			it("should not duplicate an existing tag", function () {
+				var json = {
+					tags: [
+						{
+							tag: "A"
+						}
+					]
+				};
+				var changes = [
 					{
-						tag: "A"
+						field: "tags",
+						op: "member-add",
+						value: {
+							tag: "A"
+						}
 					}
-				]
-			};
-			var changes = [
-				{
-					field: "tags",
-					op: "member-add",
-					value: {
-						tag: "B"
-					}
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.sameDeepMembers(
-				json.tags,
-				[
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.sameDeepMembers(
+					json.tags,
+					[
+						{
+							tag: "A"
+						}
+					]
+				);
+				assert.lengthOf(json.tags, 1);
+			})
+			
+			it("should remove a tag", function () {
+				var json = {
+					tags: [
+						{
+							tag: "A"
+						}
+					]
+				};
+				var changes = [
 					{
-						tag: "A"
-					},
-					{
-						tag: "B"
+						field: "tags",
+						op: "member-remove",
+						value: {
+							tag: "A"
+						}
 					}
-				]
-			);
-		})
-		
-		it("should not duplicate an existing tag", function* () {
-			var json = {
-				tags: [
-					{
-						tag: "A"
-					}
-				]
-			};
-			var changes = [
-				{
-					field: "tags",
-					op: "member-add",
-					value: {
-						tag: "A"
-					}
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.sameDeepMembers(
-				json.tags,
-				[
-					{
-						tag: "A"
-					}
-				]
-			);
-			assert.lengthOf(json.tags, 1);
-		})
-		
-		it("should remove a tag", function* () {
-			var json = {
-				tags: [
-					{
-						tag: "A"
-					}
-				]
-			};
-			var changes = [
-				{
-					field: "tags",
-					op: "member-remove",
-					value: {
-						tag: "A"
-					}
-				}
-			];
-			Zotero.DataObjectUtilities.applyChanges(json, changes);
-			assert.lengthOf(json.tags, 0);
+				];
+				Zotero.DataObjectUtilities.applyChanges(json, changes);
+				assert.lengthOf(json.tags, 0);
+			})
 		})
 	})
 })