Much better data object change detection

Replace Z.DataObjects::diff() with Z.DataObjectUtilities.diff(). Instead
of just returning two objects with the differing fields, the new diff()
generates a changeset with operations to apply with applyChanges(),
including at the array member level for collections and tags. This,
combined with cached pristine copies of objects, will allow for vastly
better conflict resolution, with automatic merging of non-conflicting
changes.

Creators currently don't show granular changes, and ordering might make
it too tough to do so. Relations diffing isn't yet implemented.
This commit is contained in:
Dan Stillman 2015-05-19 01:05:05 -04:00
parent daad18c1b5
commit abaa4da5ab
6 changed files with 670 additions and 253 deletions

View File

@ -140,6 +140,16 @@ Zotero.Creators = new function() {
});
this.equals = function (data1, data2) {
data1 = this.cleanData(data1);
data2 = this.cleanData(data2);
return data1.lastName === data2.lastName
&& data1.firstName === data2.firstName
&& data1.fieldMode === data2.fieldMode
&& data1.creatorTypeID === data2.creatorTypeID;
},
this.cleanData = function (data) {
// Validate data
if (data.name === undefined && data.lastName === undefined) {

View File

@ -66,5 +66,377 @@ Zotero.DataObjectUtilities = {
var objectTypePlural = this.getObjectTypePlural(objectType);
var className = objectTypePlural[0].toUpperCase() + objectTypePlural.substr(1);
return Zotero[className]
},
/**
* Determine whether two API JSON objects are equivalent
*
* @param {Object} data1 - API JSON of first object
* @param {Object} data2 - API JSON of second object
* @param {Array} [ignoreFields] - Fields to ignore
* @param {Boolean} - True if objects are the same, false if not
*/
equals: function (data1, data2, ignoreFields) {
var skipFields = {};
for (let field of ['key', 'version'].concat(ignoreFields || [])) {
skipFields[field] = true;
}
for (let field in data1) {
if (skipFields[field]) {
continue;
}
let val1 = data1[field];
let val2 = data2[field];
let val1HasValue = val1 || val1 === 0;
let val2HasValue = val2 || val2 === 0;
if (!val1HasValue && !val2HasValue) {
continue;
}
let changed;
switch (field) {
case 'creators':
case 'collections':
case 'tags':
case 'relations':
changed = this["_" + field + "Equals"](val1, val2);
if (changed) {
return true;
}
break;
default:
changed = val1 !== val2;
if (changed) {
return true;
}
}
skipFields[field] = true;
}
for (let field in data2) {
// Skip ignored fields and fields we've already compared
if (skipFields[field]) {
continue;
}
// All remaining fields don't exist in data1
if (data2[field] === false) {
continue;
}
return true;
}
return false;
},
_creatorsEquals: 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;
},
_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) {
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 false;
for (let i in pred1) {
if (!Zotero.Utilities.arrayEquals(pred1[i], pred2[i])) {
return false;
}
}
return true;
},
/**
* Compare two API JSON objects and generate a changeset
*
* @param {Object} data1
* @param {Object} data2
* @param {String[]} [ignoreFields] - Fields to ignore
*/
diff: function (data1, data2, ignoreFields) {
var changeset = [];
var skipFields = {};
for (let field of ['key', 'version'].concat(ignoreFields || [])) {
skipFields[field] = true;
}
for (let field in data1) {
if (skipFields[field]) {
continue;
}
let val1 = data1[field];
let val2 = data2[field];
let val1HasValue = (val1 && val1 !== "") || val1 === 0;
let val2HasValue = (val2 && val2 !== "") || val2 === 0;
if (!val1HasValue && !val2HasValue) {
continue;
}
switch (field) {
case 'creators':
case 'collections':
case 'relations':
case 'tags':
let changes = this["_" + field + "Diff"](val1, val2);
if (changes.length) {
changeset = changeset.concat(changes);
}
break;
default:
var changed = val1 !== val2;
if (changed) {
if (val1HasValue && !val2HasValue) {
changeset.push({
field: field,
op: 'delete'
});
}
else if (!val1HasValue && val2HasValue) {
changeset.push({
field: field,
op: 'add',
value: val2
});
}
else {
changeset.push({
field: field,
op: 'modify',
value: val2
});
}
}
}
skipFields[field] = true;
}
for (let field in data2) {
// Skip ignored fields and fields we've already compared
if (skipFields[field]) {
continue;
}
// All remaining fields don't exist in data1
if (data2[field] === false || data2[field] === "") {
continue;
}
changeset.push({
field: field,
op: "add",
value: data2[field]
});
}
return changeset;
},
/**
* For creators, just determine if changed, since ordering makes a full diff too complicated
*/
_creatorsDiff: function (data1, data2) {
if (!data2 || !data2.length) {
if (!data1.length) {
return [];
}
return [{
field: "creators",
op: "delete"
}];
}
if (!this._creatorsEquals(data1, data2)) {
return [{
field: "creators",
op: "modify",
value: data2
}];
}
return [];
},
_collectionsDiff: function (data1, data2) {
data2 = data2 || [];
var changeset = [];
var removed = Zotero.Utilities.arrayDiff(data1, data2);
for (let i = 0; i < removed.length; i++) {
changeset.push({
field: "collections",
op: "member-remove",
value: removed[i]
});
}
let added = Zotero.Utilities.arrayDiff(data2, data1);
for (let i = 0; i < added.length; i++) {
changeset.push({
field: "collections",
op: "member-add",
value: added[i]
});
}
return changeset;
},
_tagsDiff: function (data1, data2) {
data2 = data2 || [];
var changeset = [];
outer:
for (let i = 0; i < data1.length; i++) {
for (let j = 0; j < data2.length; j++) {
if (Zotero.Tags.equals(data1[i], data2[j])) {
continue outer;
}
}
changeset.push({
field: "tags",
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.Tags.equals(data2[i], data1[j])) {
continue outer;
}
}
changeset.push({
field: "tags",
op: "member-add",
value: data2[i]
});
}
return changeset;
},
_relationsDiff: function (data1, data2) {
if (!data1.length && !data2.length) {
return [];
}
throw new Error("Unimplemented");
},
/**
* Apply a set of changes generated by Zotero.DataObjectUtilities.diff() to an API JSON object
*
* @param {Object} json - API JSON object to modify
* @param {Object[]} changeset - Change instructions, as generated by .diff()
*/
applyChanges: function (json, changeset) {
for (let i = 0; i < changeset.length; i++) {
let c = changeset[i];
if (c.op == 'delete') {
delete json[c.field];
}
else if (c.op == 'add' || c.op == 'modify') {
json[c.field] = c.value;
}
else if (c.op == 'member-add') {
switch (c.field) {
case 'collections':
if (json[c.field].indexOf(c.value) == -1) {
json[c.field].push(c.value);
}
break;
case 'creators':
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++) {
if (Zotero.Tags.equals(json[c.field][i], c.value)) {
found = true;
break;
}
}
if (!found) {
json[c.field].push(c.value);
}
break;
default:
throw new Error("Unexpected field");
}
}
else if (c.op == 'member-remove') {
switch (c.field) {
case 'collections':
let pos = json[c.field].indexOf(c.value);
if (pos == -1) {
continue;
}
json[c.field].splice(pos, 1);
break;
case 'creators':
throw new Error("Unimplemented");
break;
case 'tags':
for (let i = 0; i < json[c.field].length; i++) {
if (Zotero.Tags.equals(json[c.field][i], c.value)) {
json[c.field].splice(i, 1);
break;
}
}
break;
default:
throw new Error("Unexpected field");
}
}
// TODO: properties
else {
throw new Error("Unimplemented");
}
}
}
};

View File

@ -424,149 +424,6 @@ Zotero.DataObjects.prototype.unload = function () {
}
/**
* @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, ignoreFields) {
var diff = [{}, {}];
var numDiffs = 0;
var skipFields = {};
for (let field of ['key', 'version'].concat(ignoreFields || [])) {
skipFields[field] = true;
}
for (var field in data1) {
if (skipFields[field]) {
continue;
}
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 (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;
}
for (var field in data2) {
// Skip ignored fields and fields we've already compared
if (skipFields[field]) {
continue;
}
if (!data2[field] && data2[field] !== 0 && !data1[field] && data1[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 (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 ? 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) {
// Child items shouldn't have collections properties, but just in case one does
if (!data2) return false;
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++) {
let t1 = data1[i];
let t2 = data2[i];
if (t1.tag !== t2.tag || (t1.type || 0) !== (t2.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;
}
Zotero.DataObjects.prototype.isEditable = function (obj) {
var libraryID = obj.libraryID;
if (!libraryID) {

View File

@ -818,6 +818,17 @@ Zotero.Tags = new function() {
}
/**
* Compare two API JSON tag objects
*/
this.equals = function (data1, data2) {
data1 = this.cleanData(data1);
data2 = this.cleanData(data2);
return data1.tag === data2.tag
&& ((!data1.type && !data2.type) || data1.type === data2.type);
},
this.cleanData = function (data) {
// Validate data
if (data.tag === undefined) {

View File

@ -0,0 +1,277 @@
"use strict";
describe("Zotero.DataObjectUtilities", function() {
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 changes = Zotero.DataObjectUtilities.diff(json1, json2);
assert.lengthOf(changes, 0);
yield Zotero.Items.erase(id1, id2);
})
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);
})
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: {
tag: "Foo"
}
},
{
field: "tags",
op: "member-add",
value: {
tag: "Foo",
type: 1
}
}
]
);
})
})
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: [
{
tag: "A"
}
]
};
var changes = [
{
field: "tags",
op: "member-add",
value: {
tag: "B"
}
}
];
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 = [
{
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);
})
})
})

View File

@ -1,110 +0,0 @@
"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);
})
it("should show tags of different types as different", function* () {
var json1 = {
tags: [
{
tag: "Foo"
}
]
};
var json2 = {
tags: [
{
tag: "Foo",
type: 1
}
]
};
var diff = Zotero.Items.diff(json1, json2);
assert.isFalse(diff);
})
it("should not show manual tags as different without 'type' property", function* () {
var json1 = {
tags: [
{
tag: "Foo"
}
]
};
var json2 = {
tags: [
{
tag: "Foo",
type: 0
}
]
};
var diff = Zotero.Items.diff(json1, json2);
assert.isFalse(diff);
})
})
})