From cd4d084dd961b742842c69e57f6c0acff797e062 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 20 Jul 2015 02:10:23 -0400 Subject: [PATCH] Support for automatically merging collections and searches --- .../zotero/xpcom/data/dataObjectUtilities.js | 120 +++-- chrome/content/zotero/xpcom/search.js | 10 + test/tests/dataObjectUtilitiesTest.js | 461 ++++++++++++------ 3 files changed, 395 insertions(+), 196 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js index 22ecb1ed2..43598b857 100644 --- a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js +++ b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js @@ -120,7 +120,7 @@ Zotero.DataObjectUtilities = { case 'collections': case 'tags': case 'relations': - changed = this["_" + field + "Equals"](val1, val2); + changed = this["_" + field + "Changed"](val1, val2); if (changed) { return true; } @@ -154,36 +154,17 @@ Zotero.DataObjectUtilities = { return false; }, - _creatorsEquals: function (data1, data2) { + _creatorsChanged: function (data1, data2) { if (!data2 || data1.length != data2.length) return true; for (let i = 0; i < data1.length; i++) { if (!Zotero.Creators.equals(data1[i], data2[i])) { - return false; + return true; } } - return true; + return false; }, - _collectionsEquals: function (data1, data2) { - if (!data2 || data1.length != data2.length) return true; - let c1 = data1.concat(); - let c2 = data2.concat(); - c1.sort(); - c2.sort(); - return Zotero.Utilities.arrayEquals(c1, c2); - }, - - _tagsEquals: function (data1, data2) { - if (!data2 || data1.length != data2.length) return true; - for (let i = 0; i < data1.length; i++) { - if (!Zotero.Tags.equals(data1[i], data2[i])) { - return false; - } - } - return true; - }, - - _relationsEquals: function (data1, data2) { + _conditionsChanged: function (data1, data2) { if (!data2) return true; var pred1 = Object.keys(data1); pred1.sort(); @@ -192,10 +173,44 @@ Zotero.DataObjectUtilities = { if (!Zotero.Utilities.arrayEquals(pred1, pred2)) return false; for (let i in pred1) { if (!Zotero.Utilities.arrayEquals(pred1[i], pred2[i])) { - return false; + return true; } } - return true; + return false; + }, + + _collectionsChanged: function (data1, data2) { + if (!data2 || data1.length != data2.length) return true; + let c1 = data1.concat(); + let c2 = data2.concat(); + c1.sort(); + c2.sort(); + return !Zotero.Utilities.arrayEquals(c1, c2); + }, + + _tagsChanged: function (data1, data2) { + if (!data2 || data1.length != data2.length) return true; + for (let i = 0; i < data1.length; i++) { + if (!Zotero.Tags.equals(data1[i], data2[i])) { + return true; + } + } + return false; + }, + + _relationsChanged: function (data1, data2) { + if (!data2) return true; + var pred1 = Object.keys(data1); + pred1.sort(); + var pred2 = Object.keys(data2); + pred2.sort(); + if (!Zotero.Utilities.arrayEquals(pred1, pred2)) return true; + for (let i in pred1) { + if (!Zotero.Utilities.arrayEquals(pred1[i], pred2[i])) { + return true; + } + } + return false; }, @@ -231,6 +246,7 @@ Zotero.DataObjectUtilities = { switch (field) { case 'creators': case 'collections': + case 'conditions': case 'relations': case 'tags': let changes = this["_" + field + "Diff"](val1, val2); @@ -277,7 +293,7 @@ Zotero.DataObjectUtilities = { // All remaining fields don't exist in data1 let val = data2[field]; - if (val === false || val === "" + if (val === false || val === "" || val === null || (typeof val == 'object' && Object.keys(val).length == 0)) { continue; } @@ -305,7 +321,7 @@ Zotero.DataObjectUtilities = { op: "delete" }]; } - if (!this._creatorsEquals(data1, data2)) { + if (this._creatorsChanged(data1, data2)) { return [{ field: "creators", op: "modify", @@ -315,8 +331,7 @@ Zotero.DataObjectUtilities = { return []; }, - _collectionsDiff: function (data1, data2) { - data2 = data2 || []; + _collectionsDiff: function (data1, data2 = []) { var changeset = []; var removed = Zotero.Utilities.arrayDiff(data1, data2); for (let i = 0; i < removed.length; i++) { @@ -337,8 +352,38 @@ Zotero.DataObjectUtilities = { return changeset; }, - _tagsDiff: function (data1, data2) { - data2 = data2 || []; + _conditionsDiff: function (data1, data2 = {}) { + var changeset = []; + outer: + for (let i = 0; i < data1.length; i++) { + for (let j = 0; j < data2.length; j++) { + if (Zotero.SearchConditions.equals(data1[i], data2[j])) { + continue outer; + } + } + changeset.push({ + field: "conditions", + op: "member-remove", + value: data1[i] + }); + } + outer: + for (let i = 0; i < data2.length; i++) { + for (let j = 0; j < data1.length; j++) { + if (Zotero.SearchConditions.equals(data2[i], data1[j])) { + continue outer; + } + } + changeset.push({ + field: "conditions", + op: "member-add", + value: data2[i] + }); + } + return changeset; + }, + + _tagsDiff: function (data1, data2 = []) { var changeset = []; outer: for (let i = 0; i < data1.length; i++) { @@ -369,8 +414,7 @@ Zotero.DataObjectUtilities = { return changeset; }, - _relationsDiff: function (data1, data2) { - data2 = data2 || {}; + _relationsDiff: function (data1, data2 = {}) { var changeset = []; for (let pred in data1) { let vals1 = typeof data1[pred] == 'string' ? [data1[pred]] : data1[pred]; @@ -448,10 +492,12 @@ Zotero.DataObjectUtilities = { throw new Error("Unimplemented"); break; + case 'conditions': case 'tags': let found = false; + let f = c.field == 'conditions' ? Zotero.SearchConditions : Zotero.Tags; for (let i = 0; i < json[c.field].length; i++) { - if (Zotero.Tags.equals(json[c.field][i], c.value)) { + if (f.equals(json[c.field][i], c.value)) { found = true; break; } @@ -479,9 +525,11 @@ Zotero.DataObjectUtilities = { throw new Error("Unimplemented"); break; + case 'conditions': case 'tags': + let f = c.field == 'conditions' ? Zotero.SearchConditions : Zotero.Tags; for (let i = 0; i < json[c.field].length; i++) { - if (Zotero.Tags.equals(json[c.field][i], c.value)) { + if (f.equals(json[c.field][i], c.value)) { json[c.field].splice(i, 1); break; } diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js index f6385ea8d..1b99c9046 100644 --- a/chrome/content/zotero/xpcom/search.js +++ b/chrome/content/zotero/xpcom/search.js @@ -2286,6 +2286,16 @@ Zotero.SearchConditions = new function(){ } + /** + * Compare two API JSON condition objects + */ + this.equals = function (data1, data2) { + return data1.condition === data2.condition + && data1.operator === data2.operator + && data1.value === data2.value; + } + + /* * Parses a search into words and "double-quoted phrases" * diff --git a/test/tests/dataObjectUtilitiesTest.js b/test/tests/dataObjectUtilitiesTest.js index 787e367db..414484cd1 100644 --- a/test/tests/dataObjectUtilitiesTest.js +++ b/test/tests/dataObjectUtilitiesTest.js @@ -1,182 +1,239 @@ "use strict"; describe("Zotero.DataObjectUtilities", function() { - // This is mostly covered by syncLocal::_reconcileChanges() tests, but we test some - // additional things here describe("#diff()", function () { - // - // 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(); - json1 = yield item.toJSON(); + // This is mostly covered by syncLocal::_reconcileChanges() tests, but we test some + // additional things here + describe("items", function () { + // + // 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(); + json1 = yield item.toJSON(); + + var item = new Zotero.Item('book'); + id2 = yield item.save(); + json2 = yield item.toJSON(); + }); - var item = new Zotero.Item('book'); - id2 = yield item.save(); - 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); - }) - - 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); - }) - }) + // + // 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); - // - // 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); - - }) - }) - - // - // 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); - }) - }) - - // - // 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); + // + // 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 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" } - }, - { - field: "tags", - op: "member-add", - value: { + ] + }; + 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 + } + } + ] + ); + }) + }) + }) + + // + // Searches + // + // + // Search conditions + // + describe("searches", function () { + describe("conditions", function () { + it("should not show an empty conditions object and a missing one as different", function () { + var json1 = { + conditions: {} + }; + var json2 = { + }; + var changes = Zotero.DataObjectUtilities.diff(json1, json2); + Zotero.debug(changes); + assert.lengthOf(changes, 0); + + var json1 = {}; + var json2 = { + conditions: {} + }; + var changes = Zotero.DataObjectUtilities.diff(json1, json2); + Zotero.debug(changes); + assert.lengthOf(changes, 0); + }) + + /*it("should not show an empty conditions object and a missing one as different", function () { + var json1 = { + conditions: [] + }; + var json2 = { + conditions: [ + { + condition: 'title', + operator: 'contains', + value: 'test' + } + ] + }; + var changes = Zotero.DataObjectUtilities.diff(json1, json2); + Zotero.debug(changes); + assert.lengthOf(changes, 0); + + var json1 = {}; + var json2 = { + conditions: {} + }; + var changes = Zotero.DataObjectUtilities.diff(json1, json2); + Zotero.debug(changes); + assert.lengthOf(changes, 0); + })*/ }) }) }) @@ -538,5 +595,89 @@ describe("Zotero.DataObjectUtilities", function() { assert.lengthOf(json.tags, 0); }) }) + + + // + // Search conditions + // + describe("conditions", function () { + it("should add a condition", function () { + var json = { + conditions: [ + { + condition: "title", + op: "contains", + value: "A" + } + ] + }; + var changes = [ + { + field: "conditions", + op: "member-add", + value: { + condition: "title", + op: "contains", + value: "B" + } + } + ]; + Zotero.DataObjectUtilities.applyChanges(json, changes); + assert.sameDeepMembers( + json.conditions, + [ + { + condition: "title", + op: "contains", + value: "A" + }, + { + condition: "title", + op: "contains", + value: "B" + } + ] + ); + }) + + it("should remove a condition", function () { + var json = { + conditions: [ + { + condition: "title", + op: "contains", + value: "A" + }, + { + condition: "title", + op: "contains", + value: "B" + } + ] + }; + var changes = [ + { + field: "conditions", + op: "member-remove", + value: { + condition: "title", + op: "contains", + value: "B" + } + } + ]; + Zotero.DataObjectUtilities.applyChanges(json, changes); + assert.sameDeepMembers( + json.conditions, + [ + { + condition: "title", + op: "contains", + value: "A" + } + ] + ); + }) + }) }) })