Update Zotero.DataObjects.prototype.diff() to handle API JSON

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

@@ -425,68 +425,141 @@ Zotero.DataObjects.prototype.unload = function () {
 
 
 /**
- * @param {Object} data1 - JSON of first object
+ * @param {Object} data1 - API JSON of first object
- * @param {Object} data2 - JSON of second object
+ * @param {Object} data2 - API JSON of second object
- * @param {Array} diff - Empty array to put diff data in
+ * @param {Array} [ignoreFields] - Fields to ignore
- * @param {Boolean} [includeMatches=false] - Include all fields, even those
- *                                           that aren't different
  */
-Zotero.DataObjects.prototype.diff = function (data1, data2, diff, includeMatches) {
+Zotero.DataObjects.prototype.diff = function (data1, data2, ignoreFields) {
-	diff.push({}, {});
+	var diff = [{}, {}];
 	var numDiffs = 0;
 	
-	var skipFields = ['collectionKey', 'itemKey', 'searchKey'];
+	var skipFields = {};
+	for (let field of ['key', 'version'].concat(ignoreFields || [])) {
+		skipFields[field] = true;
+	}
 	
 	for (var field in data1) {
-		if (skipFields.indexOf(field) != -1) {
+		if (skipFields[field]) {
 			continue;
 		}
 		
-		if (data1[field] === false && (data2[field] === false || data2[field] === undefined)) {
+		if (!data1[field] && data1[field] !== 0 && !data2[field] && data2[field] !== 0) {
 			continue;
 		}
 		
+		switch (field) {
+		case 'creators':
+		case 'collections':
+		case 'tags':
+		case 'relations':
+			var changed = this["_diff" + field[0].toUpperCase() + field.substr(1)](
+				data1[field], data2[field]
+			);
+			break;
+		
+		default:
 			var changed = data1[field] !== data2[field];
+		}
 		
-		if (includeMatches || changed) {
+		if (changed) {
 			diff[0][field] = data1[field] !== false ? data1[field] : '';
 			diff[1][field] = (data2[field] !== false && data2[field] !== undefined)
 				? data2[field] : '';
 		}
 		
 		if (changed) {
+			//Zotero.debug("Field " + field + " has changed");
 			numDiffs++;
 		}
+		
+		skipFields[field] = true;
 	}
 	
-	// DEBUG: some of this is probably redundant
 	for (var field in data2) {
-		if (skipFields.indexOf(field) != -1) {
+		// Skip ignored fields and fields we've already compared
+		if (skipFields[field]) {
 			continue;
 		}
 		
-		if (diff[0][field] !== undefined) {
+		if (!data2[field] && data2[field] !== 0 && !data1[field] && data1[field] !== 0) {
 			continue;
 		}
 		
-		if (data2[field] === false && (data1[field] === false || data1[field] === undefined)) {
+		switch (field) {
-			continue;
+		case 'creators':
-		}
+		case 'collections':
+		case 'tags':
+		case 'relations':
+			var changed = this["_diff" + field[0].toUpperCase() + field.substr(1)](
+				data1[field], data2[field]
+			);
+			break;
 		
+		default:
 			var changed = data1[field] !== data2[field];
+		}
 		
-		if (includeMatches || changed) {
+		if (changed) {
 			diff[0][field] = (data1[field] !== false && data1[field] !== undefined)
 				? data1[field] : '';
 			diff[1][field] = data2[field] !== false ? data2[field] : '';
 		}
 		
 		if (changed) {
+			//Zotero.debug("Field " + field + " has changed");
 			numDiffs++;
 		}
 	}
 	
-	return numDiffs;
+	return numDiffs ? diff : false;
+}
+
+Zotero.DataObjects.prototype._diffCreators = function (data1, data2) {
+	if (data1.length != data2.length) return false;
+	for (let i = 0; i < data1.length; i++) {
+		let c1 = Zotero.Creators.cleanData(data1[i]);
+		let c2 = Zotero.Creators.cleanData(data2[i]);
+		if (c1.lastName !== c2.lastName
+				|| c1.firstName !== c2.firstName
+				|| c1.fieldMode !== c2.fieldMode
+				|| c1.creatorTypeID !== c2.creatorTypeID) {
+			return true;
+		}
+	}
+	return false;
+}
+
+Zotero.DataObjects.prototype._diffCollections = function (data1, data2) {
+	if (data1.length != data2.length) return false;
+	let c1 = data1.concat();
+	let c2 = data2.concat();
+	c1.sort();
+	c2.sort();
+	return !Zotero.Utilities.arrayEquals(c1, c2);
+}
+
+Zotero.DataObjects.prototype._diffTags = function (data1, data2) {
+	if (data1.length != data2.length) return false;
+	for (let i = 0; i < data1.length; i++) {
+		if (c1.tag !== c2.tag || (c1.type || 0) !== (c2.type || 0)) {
+			return false;
+		}
+	}
+	return false;
+}
+
+Zotero.DataObjects.prototype._diffRelations = function (data1, data2) {
+	var pred1 = Object.keys(data1);
+	pred1.sort();
+	var pred2 = Object.keys(data2);
+	pred2.sort();
+	if (!Zotero.Utilities.arrayEquals(pred1, pred2)) return false;
+	for (let i in pred1) {
+		if (!Zotero.Utilities.arrayEquals(pred1[i], pred2[i])) {
+			return true;
+		}
+	}
+	return false;
 }
 
 

test/tests/dataObjectsTest.js

@@ -0,0 +1,70 @@
+"use strict";
+
+describe("Zotero.DataObjects", function() {
+	var types = ['collection', 'item', 'search'];
+	
+	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();
+				
+				var item = new Zotero.Item('book');
+				id2 = yield item.save();
+				item = yield Zotero.Items.getAsync(id2);
+				json2 = yield item.toJSON();
+			});
+			
+			var diff = Zotero.Items.diff(json1, json2);
+			assert.isFalse(diff);
+			
+			yield Zotero.Items.erase(id1, id2);
+		})
+		
+		it("should not show empty strings as different", function* () {
+			var json1 = {
+				title: ""
+			};
+			var json2 = {
+				title: ""
+			};
+			var diff = Zotero.Items.diff(json1, json2);
+			assert.isFalse(diff);
+		})
+		
+		it("should not show empty string and undefined as different", function* () {
+			var json1 = {
+				title: ""
+			};
+			var json2 = {
+				place: ""
+			};
+			var diff = Zotero.Items.diff(json1, json2);
+			assert.isFalse(diff);
+		})
+		
+		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 diff = Zotero.Items.diff(json1, json2);
+			assert.isFalse(diff);
+		})
+	})
+})