From 59c0c7fcad87358217eeada7fea83328a4cd9fb4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 12 May 2016 16:14:27 -0400 Subject: [PATCH] Don't include file properties for ZFS-synced libraries They're disallowed by the API in group libraries and not necessary in personal libraries, because they're set atomically with the file upload. --- chrome/content/zotero/xpcom/data/item.js | 14 ++- .../content/zotero/xpcom/sync/syncEngine.js | 20 +++- test/tests/syncEngineTest.js | 107 ++++++++++++++++++ 3 files changed, 135 insertions(+), 6 deletions(-) diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 9164ebbb2c..f882458bc8 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -4110,17 +4110,23 @@ Zotero.Item.prototype.toJSON = function (options = {}) { if (this.isAttachment()) { let linkMode = this.attachmentLinkMode; obj.linkMode = Zotero.Attachments.linkModeToName(linkMode); - obj.contentType = this.attachmentContentType; - obj.charset = this.attachmentCharset; + + let imported = this.isImportedAttachment(); + let skipFileProps = options.skipImportedFileProperties; + + if (!imported || !skipFileProps) { + obj.contentType = this.attachmentContentType; + obj.charset = this.attachmentCharset; + } if (linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) { obj.path = this.attachmentPath; } - else if (linkMode != Zotero.Attachments.LINK_MODE_LINKED_URL) { + else if (linkMode != Zotero.Attachments.LINK_MODE_LINKED_URL && !skipFileProps) { obj.filename = this.attachmentFilename; } - if (this.isFileAttachment()) { + if (imported && !skipFileProps) { if (options.syncedStorageProperties) { obj.mtime = this.attachmentSyncedModificationTime; obj.md5 = this.attachmentSyncedHash; diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js index f9550ba959..88a5c335b3 100644 --- a/chrome/content/zotero/xpcom/sync/syncEngine.js +++ b/chrome/content/zotero/xpcom/sync/syncEngine.js @@ -874,7 +874,18 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func continue; } if (!o.json) { - o.json = yield this._getJSONForObject(objectType, o.id); + o.json = yield this._getJSONForObject( + objectType, + o.id, + { + // Only include file properties ('contentType', etc.) for WebDAV files + skipImportedFileProperties: + objectType == 'item' + ? Zotero.Sync.Storage.Local.getModeForLibrary(this.library.libraryID) + != 'webdav' + : undefined + } + ); } batch.push(o.json); } @@ -1041,11 +1052,13 @@ Zotero.Sync.Data.Engine.prototype._uploadDeletions = Zotero.Promise.coroutine(fu }); -Zotero.Sync.Data.Engine.prototype._getJSONForObject = function (objectType, id) { +Zotero.Sync.Data.Engine.prototype._getJSONForObject = function (objectType, id, options = {}) { return Zotero.DB.executeTransaction(function* () { var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType); var obj = objectsClass.get(id); var cacheObj = false; + // If the object has been synced before, get the pristine version from the cache so we can + // use PATCH mode and include only fields that have changed if (obj.version) { cacheObj = yield Zotero.Sync.Data.Local.getCacheObject( objectType, obj.libraryID, obj.key, obj.version @@ -1060,6 +1073,9 @@ Zotero.Sync.Data.Engine.prototype._getJSONForObject = function (objectType, id) includeKey: true, includeVersion: true, // DEBUG: remove? includeDate: true, + // Whether to skip 'contentType', 'filename', 'mtime', and 'md5' + skipImportedFileProperties: options.skipImportedFileProperties, + // Use last-synced mtime/md5 instead of current values from the file itself syncedStorageProperties: true, patchBase: cacheObj ? cacheObj.data : false }); diff --git a/test/tests/syncEngineTest.js b/test/tests/syncEngineTest.js index 9d6ee64f4e..1cfb98bbb5 100644 --- a/test/tests/syncEngineTest.js +++ b/test/tests/syncEngineTest.js @@ -787,6 +787,113 @@ describe("Zotero.Sync.Data.Engine", function () { }); + it("shouldn't include file properties for attachments in ZFS libraries", function* () { + ({ engine, client, caller } = yield setup()); + + var libraryID = Zotero.Libraries.userLibraryID; + var lastLibraryVersion = 2; + yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion); + + var item = new Zotero.Item('attachment'); + item.attachmentLinkMode = 'imported_file'; + item.attachmentFilename = 'test.txt'; + item.attachmentContentType = 'text/plain'; + item.attachmentCharset = 'utf-8'; + yield item.saveTx(); + + var itemResponseJSON = item.toResponseJSON(); + itemResponseJSON.version = itemResponseJSON.data.version = lastLibraryVersion; + + server.respond(function (req) { + if (req.method == "POST") { + if (req.url == baseURL + "users/1/items") { + let json = JSON.parse(req.requestBody); + assert.lengthOf(json, 1); + let itemJSON = json[0]; + assert.equal(itemJSON.key, item.key); + assert.equal(itemJSON.version, 0); + assert.notProperty(itemJSON, "contentType"); + assert.notProperty(itemJSON, "charset"); + assert.notProperty(itemJSON, "filename"); + assert.notProperty(itemJSON, "mtime"); + assert.notProperty(itemJSON, "md5"); + req.respond( + 200, + { + "Content-Type": "application/json", + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": itemResponseJSON + }, + unchanged: {}, + failed: {} + }) + ); + return; + } + } + }) + + yield engine.start(); + }); + + + it("should include file properties for attachments in WebDAV libraries", function* () { + ({ engine, client, caller } = yield setup()); + + var libraryID = Zotero.Libraries.userLibraryID; + var lastLibraryVersion = 2; + yield Zotero.Libraries.setVersion(libraryID, lastLibraryVersion); + Zotero.Sync.Storage.Local.setModeForLibrary(libraryID, 'webdav'); + + var item = new Zotero.Item('attachment'); + item.attachmentLinkMode = 'imported_file'; + item.attachmentFilename = 'test.txt'; + item.attachmentContentType = 'text/plain'; + item.attachmentCharset = 'utf-8'; + yield item.saveTx(); + + var itemResponseJSON = item.toResponseJSON(); + itemResponseJSON.version = itemResponseJSON.data.version = lastLibraryVersion; + + server.respond(function (req) { + if (req.method == "POST") { + if (req.url == baseURL + "users/1/items") { + let json = JSON.parse(req.requestBody); + assert.lengthOf(json, 1); + let itemJSON = json[0]; + assert.equal(itemJSON.key, item.key); + assert.equal(itemJSON.version, 0); + assert.propertyVal(itemJSON, "contentType", item.attachmentContentType); + assert.propertyVal(itemJSON, "charset", item.attachmentCharset); + assert.propertyVal(itemJSON, "filename", item.attachmentFilename); + assert.propertyVal(itemJSON, "mtime", null); + assert.propertyVal(itemJSON, "md5", null); + req.respond( + 200, + { + "Content-Type": "application/json", + "Last-Modified-Version": lastLibraryVersion + }, + JSON.stringify({ + successful: { + "0": itemResponseJSON + }, + unchanged: {}, + failed: {} + }) + ); + return; + } + } + }) + + yield engine.start(); + }); + + it("should upload synced storage properties", function* () { ({ engine, client, caller } = yield setup());