diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index 049a7e467..147e9d73a 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/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} data2 - JSON of second object - * @param {Array} diff - Empty array to put diff data in - * @param {Boolean} [includeMatches=false] - Include all fields, even those - * that aren't different + * @param {Object} data1 - API JSON of first object + * @param {Object} data2 - API JSON of second object + * @param {Array} [ignoreFields] - Fields to ignore */ -Zotero.DataObjects.prototype.diff = function (data1, data2, diff, includeMatches) { - diff.push({}, {}); +Zotero.DataObjects.prototype.diff = function (data1, data2, ignoreFields) { + 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; } - var changed = data1[field] !== data2[field]; + 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; - if (includeMatches || changed) { + default: + var changed = data1[field] !== data2[field]; + } + + 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)) { - 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]; } - 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; } diff --git a/test/tests/dataObjectsTest.js b/test/tests/dataObjectsTest.js new file mode 100644 index 000000000..73dd3ff77 --- /dev/null +++ b/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); + }) + }) +})