diff --git a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js index 04a63fce8..1cf0d3128 100644 --- a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js +++ b/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 + "'"); } } } diff --git a/test/tests/dataObjectUtilitiesTest.js b/test/tests/dataObjectUtilitiesTest.js index 61a1d9471..63679d397 100644 --- a/test/tests/dataObjectUtilitiesTest.js +++ b/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); + }) }) }) })