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

@@ -4110,17 +4110,23 @@ Zotero.Item.prototype.toJSON = function (options = {}) {
 		if (this.isAttachment()) {
 			let linkMode = this.attachmentLinkMode;
 			obj.linkMode = Zotero.Attachments.linkModeToName(linkMode);
+			
+			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;

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
 		});

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());