diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 719aba990..8c4e11718 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -678,14 +678,43 @@ Zotero.Collection.prototype.fromJSON = Zotero.Promise.coroutine(function* (json) }); -Zotero.Collection.prototype.toJSON = Zotero.Promise.coroutine(function* (options) { - var obj = {}; +Zotero.Collection.prototype.toResponseJSON = Zotero.Promise.coroutine(function* (options = {}) { + var json = yield this.constructor._super.prototype.toResponseJSON.apply(this, options); + + // TODO: library block? + + // creatorSummary + var firstCreator = this.getField('firstCreator'); + if (firstCreator) { + json.meta.creatorSummary = firstCreator; + } + // parsedDate + var parsedDate = Zotero.Date.multipartToSQL(this.getField('date', true, true)); + if (parsedDate) { + // 0000? + json.meta.parsedDate = parsedDate; + } + // numChildren + if (this.isRegularItem()) { + json.meta.numChildren = this.numChildren(); + } + return json; +}) + + +Zotero.Collection.prototype.toJSON = Zotero.Promise.coroutine(function* (options = {}) { + var env = this._preToJSON(options); + var mode = env.mode; + + var obj = env.obj = {}; obj.key = this.key; obj.version = this.version; + obj.name = this.name; obj.parentCollection = this.parentKey ? this.parentKey : false; obj.relations = {}; // TEMP - return obj; + + return this._postToJSON(env); }); diff --git a/chrome/content/zotero/xpcom/data/dataObject.js b/chrome/content/zotero/xpcom/data/dataObject.js index b5d3c97d3..e186ec1de 100644 --- a/chrome/content/zotero/xpcom/data/dataObject.js +++ b/chrome/content/zotero/xpcom/data/dataObject.js @@ -1190,6 +1190,47 @@ Zotero.DataObject.prototype._finalizeErase = function (env) { } } + +Zotero.DataObject.prototype.toResponseJSON = Zotero.Promise.coroutine(function* (options) { + // TODO: library block? + + return { + key: this.key, + version: this.version, + meta: {}, + data: yield this.toJSON(options) + }; +}); + + +Zotero.DataObject.prototype._preToJSON = function (options) { + var env = { options }; + env.mode = options.mode || 'new'; + if (env.mode == 'patch') { + if (!options.patchBase) { + throw new Error("Cannot use patch mode if patchBase not provided"); + } + } + else if (options.patchBase) { + if (options.mode) { + Zotero.debug("Zotero.Item.toJSON: ignoring provided patchBase in " + env.mode + " mode", 2); + } + // If patchBase provided and no explicit mode, use 'patch' + else { + env.mode = 'patch'; + } + } + return env; +} + +Zotero.DataObject.prototype._postToJSON = function (env) { + if (env.mode == 'patch') { + env.obj = Zotero.DataObjectUtilities.patch(env.options.patchBase, env.obj); + } + return env.obj; +} + + /** * Generates data object key * @return {String} key diff --git a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js index 15af32b1d..51a88426c 100644 --- a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js +++ b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js @@ -101,6 +101,42 @@ Zotero.DataObjectUtilities = { return Zotero[className] }, + + patch: function (base, obj) { + var target = {}; + Object.assign(target, obj); + + for (let i in base) { + switch (i) { + case 'key': + case 'version': + case 'dateModified': + continue; + } + + // If field from base exists in the new version, delete it if it's the same + if (i in target) { + if (!this._fieldChanged(i, base[i], target[i])) { + delete target[i]; + } + } + // If field from base doesn't exist in new version, clear it + else { + switch (i) { + case 'deleted': + target[i] = false; + break; + + default: + target[i] = ''; + } + } + } + + return target; + }, + + /** * Determine whether two API JSON objects are equivalent * @@ -129,24 +165,9 @@ Zotero.DataObjectUtilities = { continue; } - let changed; - - switch (field) { - case 'creators': - case 'collections': - case 'tags': - case 'relations': - changed = this["_" + field + "Changed"](val1, val2); - if (changed) { - return true; - } - break; - - default: - changed = val1 !== val2; - if (changed) { - return true; - } + let changed = this._fieldChanged(field, val1, val2); + if (changed) { + return true; } skipFields[field] = true; @@ -170,6 +191,20 @@ Zotero.DataObjectUtilities = { return false; }, + _fieldChanged: function (fieldName, field1, field2) { + switch (fieldName) { + case 'collections': + case 'conditions': + case 'creators': + case 'tags': + case 'relations': + return this["_" + fieldName + "Changed"](field1, field2); + + default: + return field1 !== field2; + } + }, + _creatorsChanged: function (data1, data2) { if (!data2 || data1.length != data2.length) return true; for (let i = 0; i < data1.length; i++) { diff --git a/chrome/content/zotero/xpcom/data/dataObjects.js b/chrome/content/zotero/xpcom/data/dataObjects.js index c6f423b22..52d196d55 100644 --- a/chrome/content/zotero/xpcom/data/dataObjects.js +++ b/chrome/content/zotero/xpcom/data/dataObjects.js @@ -534,7 +534,6 @@ Zotero.DataObjects.prototype._load = Zotero.Promise.coroutine(function* (library return loaded; } - // getPrimaryDataSQL() should use "O" for the primary table alias var sql = this.primaryDataSQL; var params = []; if (libraryID !== false) { @@ -551,7 +550,7 @@ Zotero.DataObjects.prototype._load = Zotero.Promise.coroutine(function* (library params, { onRow: function (row) { - var id = row.getResultByIndex(this._ZDO_id); + var id = row.getResultByName(this._ZDO_id); var columns = Object.keys(this._primaryDataSQLParts); var rowObj = {}; for (let i=0; i { + if (o.key === undefined) { + throw new Error("Missing 'key' property in JSON"); + } + if (o.version === undefined) { + throw new Error("Missing 'version' property in JSON"); + } + // If direct data object passed, wrap in fake response object + return o.data === undefined ? { + key: o.key, + version: o.version, + data: o + } : o; + }); + Zotero.debug("Saving to sync cache:"); Zotero.debug(jsonArray); @@ -174,12 +195,6 @@ Zotero.Sync.Data.Local = { var params = []; for (let i = 0; i < chunk.length; i++) { let o = chunk[i]; - if (o.key === undefined) { - throw new Error("Missing 'key' property in JSON"); - } - if (o.version === undefined) { - throw new Error("Missing 'version' property in JSON"); - } params.push(libraryID, o.key, syncObjectTypeID, o.version, JSON.stringify(o)); } return Zotero.DB.queryAsync( diff --git a/test/content/support.js b/test/content/support.js index 4401b55ac..9f1955dc2 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -260,12 +260,24 @@ var createGroup = Zotero.Promise.coroutine(function* (props) { // // Data objects // -function createUnsavedDataObject(objectType, params) { +/** + * @param {String} objectType - 'collection', 'item', 'search' + * @param {Object} [params] + * @param {Integer} [params.libraryID] + * @param {String} [params.itemType] - Item type + * @param {String} [params.title] - Item title + * @param {Boolean} [params.setTitle] - Assign a random item title + * @param {String} [params.name] - Collection/search name + * @param {Integer} [params.parentID] + * @param {String} [params.parentKey] + * @param {Boolean} [params.synced] + * @param {Integer} [params.version] + */ +function createUnsavedDataObject(objectType, params = {}) { if (!objectType) { throw new Error("Object type not provided"); } - params = params || {}; if (objectType == 'item') { var param = params.itemType || 'book'; } @@ -275,14 +287,14 @@ function createUnsavedDataObject(objectType, params) { } switch (objectType) { case 'item': - if (params.title) { - obj.setField('title', params.title); + if (params.title !== undefined || params.setTitle) { + obj.setField('title', params.title !== undefined ? params.title : Zotero.Utilities.randomString()); } break; case 'collection': case 'search': - obj.name = params.name !== undefined ? params.name : "Test"; + obj.name = params.name !== undefined ? params.name : Zotero.Utilities.randomString(); break; } var allowedParams = ['parentID', 'parentKey', 'synced', 'version']; @@ -294,12 +306,32 @@ function createUnsavedDataObject(objectType, params) { return obj; } -var createDataObject = Zotero.Promise.coroutine(function* (objectType, params, saveOptions) { +var createDataObject = Zotero.Promise.coroutine(function* (objectType, params = {}, saveOptions) { var obj = createUnsavedDataObject(objectType, params); yield obj.saveTx(saveOptions); return obj; }); +function getNameProperty(objectType) { + return objectType == 'item' ? 'title' : 'name'; +} + +var modifyDataObject = Zotero.Promise.coroutine(function* (obj, params = {}) { + switch (obj.objectType) { + case 'item': + yield obj.loadItemData(); + obj.setField( + 'title', + params.title !== undefined ? params.title : Zotero.Utilities.randomString() + ); + break; + + default: + obj.name = params.name !== undefined ? params.name : Zotero.Utilities.randomString(); + } + return obj.saveTx(); +}); + /** * Return a promise for the error thrown by a promise, or false if none */ diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index ec55a7977..4cd180d77 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -709,56 +709,96 @@ describe("Zotero.Item", function () { }) describe("#toJSON()", function () { - it("should output only fields with values in default mode", function* () { - var itemType = "book"; - var title = "Test"; - - var item = new Zotero.Item(itemType); - item.setField("title", title); - var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); - var json = yield item.toJSON(); - - assert.equal(json.itemType, itemType); - assert.equal(json.title, title); - assert.isUndefined(json.date); - assert.isUndefined(json.numPages); - }) - - it("should output all fields in 'full' mode", function* () { - var itemType = "book"; - var title = "Test"; - - var item = new Zotero.Item(itemType); - item.setField("title", title); - var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); - var json = yield item.toJSON({ mode: 'full' }); - assert.equal(json.title, title); - assert.equal(json.date, ""); - assert.equal(json.numPages, ""); - }) - - it("should output only fields that differ in 'patch' mode", function* () { - var itemType = "book"; - var title = "Test"; - var date = "2015-05-12"; - - var item = new Zotero.Item(itemType); - item.setField("title", title); - var id = yield item.saveTx(); - item = yield Zotero.Items.getAsync(id); - var patchBase = yield item.toJSON(); - - item.setField("date", date); - yield item.saveTx(); - var json = yield item.toJSON({ - patchBase: patchBase + describe("default mode", function () { + it("should output only fields with values", function* () { + var itemType = "book"; + var title = "Test"; + + var item = new Zotero.Item(itemType); + item.setField("title", title); + var id = yield item.saveTx(); + item = yield Zotero.Items.getAsync(id); + var json = yield item.toJSON(); + + assert.equal(json.itemType, itemType); + assert.equal(json.title, title); + assert.isUndefined(json.date); + assert.isUndefined(json.numPages); + }) + }) + + describe("'full' mode", function () { + it("should output all fields", function* () { + var itemType = "book"; + var title = "Test"; + + var item = new Zotero.Item(itemType); + item.setField("title", title); + var id = yield item.saveTx(); + item = yield Zotero.Items.getAsync(id); + var json = yield item.toJSON({ mode: 'full' }); + assert.equal(json.title, title); + assert.equal(json.date, ""); + assert.equal(json.numPages, ""); + }) + }) + + describe("'patch' mode", function () { + it("should output only fields that differ", function* () { + var itemType = "book"; + var title = "Test"; + var date = "2015-05-12"; + + var item = new Zotero.Item(itemType); + item.setField("title", title); + var id = yield item.saveTx(); + item = yield Zotero.Items.getAsync(id); + var patchBase = yield item.toJSON(); + + item.setField("date", date); + yield item.saveTx(); + var json = yield item.toJSON({ + patchBase: patchBase + }) + assert.isUndefined(json.itemType); + assert.isUndefined(json.title); + assert.equal(json.date, date); + assert.isUndefined(json.numPages); + assert.isUndefined(json.deleted); + assert.isUndefined(json.creators); + assert.isUndefined(json.relations); + assert.isUndefined(json.tags); + }) + + it("should include changed 'deleted' field", function* () { + // True to false + var item = new Zotero.Item('book'); + item.deleted = true; + var id = yield item.saveTx(); + item = yield Zotero.Items.getAsync(id); + var patchBase = yield item.toJSON(); + + item.deleted = false; + var json = yield item.toJSON({ + patchBase: patchBase + }) + assert.isUndefined(json.title); + assert.isFalse(json.deleted); + + // False to true + var item = new Zotero.Item('book'); + item.deleted = false; + var id = yield item.saveTx(); + item = yield Zotero.Items.getAsync(id); + var patchBase = yield item.toJSON(); + + item.deleted = true; + var json = yield item.toJSON({ + patchBase: patchBase + }) + assert.isUndefined(json.title); + assert.isTrue(json.deleted); }) - assert.isUndefined(json.itemType); - assert.isUndefined(json.title); - assert.equal(json.date, date); - assert.isUndefined(json.numPages); }) }) diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 1a954f566..6e5a04d35 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -122,39 +122,9 @@ describe("Zotero.Sync.Data.Engine", function () { }) describe("Syncing", function () { - it("should perform a sync for a new library", function* () { + it("should download items into a new library", function* () { ({ engine, client, caller } = yield setup()); - server.respond(function (req) { - if (req.method == "POST" && req.url == baseURL + "users/1/items") { - let ifUnmodifiedSince = req.requestHeaders["If-Unmodified-Since-Version"]; - if (ifUnmodifiedSince == 0) { - req.respond(412, {}, "Library has been modified since specified version"); - return; - } - - if (ifUnmodifiedSince == 3) { - let json = JSON.parse(req.requestBody); - req.respond( - 200, - { - "Content-Type": "application/json", - "Last-Modified-Version": 3 - }, - JSON.stringify({ - success: { - "0": json[0].key, - "1": json[1].key - }, - unchanged: {}, - failed: {} - }) - ); - return; - } - } - }) - var headers = { "Last-Modified-Version": 3 }; @@ -280,6 +250,251 @@ describe("Zotero.Sync.Data.Engine", function () { assert.isTrue(obj.synced); }) + it("should upload new full items and subsequent patches", function* () { + ({ engine, client, caller } = yield setup()); + + var libraryID = Zotero.Libraries.userLibraryID; + var lastLibraryVersion = 5; + yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion); + + var types = Zotero.DataObjectUtilities.getTypes(); + var objects = {}; + var objectResponseJSON = {}; + var objectVersions = {}; + for (let type of types) { + objects[type] = [yield createDataObject(type, { setTitle: true })]; + objectVersions[type] = {}; + objectResponseJSON[type] = yield Zotero.Promise.all(objects[type].map(o => o.toResponseJSON())); + } + + server.respond(function (req) { + if (req.method == "POST") { + assert.equal( + req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion + ); + + for (let type of types) { + let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type); + if (req.url == baseURL + "users/1/" + typePlural) { + let json = JSON.parse(req.requestBody); + assert.lengthOf(json, 1); + assert.equal(json[0].key, objects[type][0].key); + assert.equal(json[0].version, 0); + if (type == 'item') { + assert.equal(json[0].title, objects[type][0].getField('title')); + } + else { + assert.equal(json[0].name, objects[type][0].name); + } + let objectJSON = objectResponseJSON[type][0]; + objectJSON.version = ++lastLibraryVersion; + objectJSON.data.version = lastLibraryVersion; + req.respond( + 200, + { + "Content-Type": "application/json", + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": objectJSON + }, + unchanged: {}, + failed: {} + }) + ); + objectVersions[type][objects[type][0].key] = lastLibraryVersion; + return; + } + } + } + }) + + yield engine.start(); + + assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion); + for (let type of types) { + // Make sure objects were set to the correct version and marked as synced + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + let key = objects[type][0].key; + let version = objects[type][0].version; + assert.equal(version, objectVersions[type][key]); + // Make sure uploaded objects were added to cache + let cached = yield Zotero.Sync.Data.Local.getCacheObject(type, libraryID, key, version); + assert.typeOf(cached, 'object'); + assert.equal(cached.key, key); + assert.equal(cached.version, version); + + yield modifyDataObject(objects[type][0]); + } + + ({ engine, client, caller } = yield setup()); + + server.respond(function (req) { + if (req.method == "POST") { + assert.equal( + req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion + ); + + for (let type of types) { + let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type); + if (req.url == baseURL + "users/1/" + typePlural) { + let json = JSON.parse(req.requestBody); + assert.lengthOf(json, 1); + let j = json[0]; + let o = objects[type][0]; + assert.equal(j.key, o.key); + assert.equal(j.version, objectVersions[type][o.key]); + if (type == 'item') { + assert.equal(j.title, o.getField('title')); + } + else { + assert.equal(j.name, o.name); + } + + // Verify PATCH semantics instead of POST (i.e., only changed fields) + let changedFieldsExpected = ['key', 'version']; + if (type == 'item') { + changedFieldsExpected.push('title', 'dateModified'); + } + else { + changedFieldsExpected.push('name'); + } + let changedFields = Object.keys(j); + assert.lengthOf( + changedFields, changedFieldsExpected.length, "same " + type + " length" + ); + assert.sameMembers( + changedFields, changedFieldsExpected, "same " + type + " members" + ); + let objectJSON = objectResponseJSON[type][0]; + objectJSON.version = ++lastLibraryVersion; + objectJSON.data.version = lastLibraryVersion; + req.respond( + 200, + { + "Content-Type": "application/json", + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": objectJSON + }, + unchanged: {}, + failed: {} + }) + ); + objectVersions[type][o.key] = lastLibraryVersion; + return; + } + } + } + }) + + yield engine.start(); + + assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion); + for (let type of types) { + // Make sure objects were set to the correct version and marked as synced + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + let o = objects[type][0]; + let key = o.key; + let version = o.version; + assert.equal(version, objectVersions[type][key]); + // Make sure uploaded objects were added to cache + let cached = yield Zotero.Sync.Data.Local.getCacheObject(type, libraryID, key, version); + assert.typeOf(cached, 'object'); + assert.equal(cached.key, key); + assert.equal(cached.version, version); + + switch (type) { + case 'collection': + assert.isFalse(cached.data.parentCollection); + break; + + case 'item': + assert.equal(cached.data.dateAdded, Zotero.Date.sqlToISO8601(o.dateAdded)); + break; + + case 'search': + assert.typeOf(cached.data.conditions, 'object'); + break; + } + } + }) + + it("should update local objects with remotely saved version after uploading if necessary", function* () { + ({ engine, client, caller } = yield setup()); + + var libraryID = Zotero.Libraries.userLibraryID; + var lastLibraryVersion = 5; + yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion); + + var types = Zotero.DataObjectUtilities.getTypes(); + var objects = {}; + var objectResponseJSON = {}; + var objectNames = {}; + for (let type of types) { + objects[type] = [yield createDataObject(type, { setTitle: true })]; + objectNames[type] = {}; + objectResponseJSON[type] = yield Zotero.Promise.all(objects[type].map(o => o.toResponseJSON())); + } + + server.respond(function (req) { + if (req.method == "POST") { + assert.equal( + req.requestHeaders["If-Unmodified-Since-Version"], lastLibraryVersion + ); + + for (let type of types) { + let typePlural = Zotero.DataObjectUtilities.getObjectTypePlural(type); + if (req.url == baseURL + "users/1/" + typePlural) { + let key = objects[type][0].key; + let objectJSON = objectResponseJSON[type][0]; + objectJSON.version = ++lastLibraryVersion; + objectJSON.data.version = lastLibraryVersion; + let prop = type == 'item' ? 'title' : 'name'; + objectNames[type][key] = objectJSON.data[prop] = Zotero.Utilities.randomString(); + req.respond( + 200, + { + "Content-Type": "application/json", + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": objectJSON + }, + unchanged: {}, + failed: {} + }) + ); + return; + } + } + } + }) + + yield engine.start(); + + assert.equal(Zotero.Libraries.getVersion(libraryID), lastLibraryVersion); + for (let type of types) { + // Make sure local objects were updated with new metadata and marked as synced + assert.lengthOf((yield Zotero.Sync.Data.Local.getUnsynced(libraryID, type)), 0); + let o = objects[type][0]; + let key = o.key; + let version = o.version; + let name = objectNames[type][key]; + if (type == 'item') { + yield o.loadItemData(); + assert.equal(name, o.getField('title')); + } + else { + assert.equal(name, o.name); + } + } + }) + it("should make only one request if in sync", function* () { yield Zotero.Libraries.setVersion(Zotero.Libraries.userLibraryID, 5); ({ engine, client, caller } = yield setup());