Prompt to reset local group files on 403 for file attachment upload

And reset modified file attachments when resetting files

chrome/content/zotero/xpcom/sync/syncEngine.js

@@ -1983,6 +1983,21 @@ Zotero.Sync.Data.Engine.prototype._checkObjectUploadError = Zotero.Promise.corou
 			}
 		}
 	}
+	else if (code == 403) {
+		// Prompt to reset local group files on 403 for file attachment upload
+		if (objectType == 'item') {
+			let item = Zotero.Items.getByLibraryAndKey(this.libraryID, key);
+			if (this.library.libraryType == 'group' && item.isFileAttachment()) {
+				let index = Zotero.Sync.Storage.Utilities.showFileWriteAccessLostPrompt(
+					null, this.library
+				);
+				if (index === 0) {
+					yield Zotero.Sync.Data.Local.resetUnsyncedLibraryFiles(this.libraryID);
+				}
+				return false;
+			}
+		}
+	}
 	// This shouldn't happen, because the upload request includes a library version and should
 	// prevent an outdated upload before the object version is checked. If it does, we need to
 	// do a full sync. This error is checked in handleUploadError().

chrome/content/zotero/xpcom/sync/syncLocal.js

@@ -239,6 +239,8 @@ Zotero.Sync.Data.Local = {
 	
 	
 	_libraryHasUnsyncedFiles: Zotero.Promise.coroutine(function* (libraryID) {
+		// TODO: Check for modified file attachment items, which also can't be uploaded
+		// (and which are corrected by resetUnsyncedLibraryFiles())
 		yield Zotero.Sync.Storage.Local.checkForUpdatedFiles(libraryID);
 		return !!(yield Zotero.Sync.Storage.Local.getFilesToUpload(libraryID)).length;
 	}),
@@ -253,42 +255,9 @@ Zotero.Sync.Data.Local = {
 		}
 		
 		for (let objectType of Zotero.DataObjectUtilities.getTypesForLibrary(libraryID)) {
-			let objectTypePlural = Zotero.DataObjectUtilities.getObjectTypePlural(objectType);
-			let objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
-			
 			// New/modified objects
-			let ids = yield Zotero.Sync.Data.Local.getUnsynced(objectType, libraryID);
+			let ids = yield this.getUnsynced(objectType, libraryID);
-			let keys = ids.map(id => objectsClass.getLibraryAndKeyFromID(id).key);
+			yield this._resetObjects(libraryID, objectType, ids);
-			let cacheVersions = yield this.getLatestCacheObjectVersions(objectType, libraryID, keys);
-			let toDelete = [];
-			for (let key of keys) {
-				let obj = objectsClass.getByLibraryAndKey(libraryID, key);
-				
-				// If object is in cache, overwrite with pristine data
-				if (cacheVersions[key]) {
-					let json = yield this.getCacheObject(objectType, libraryID, key, cacheVersions[key]);
-					yield Zotero.DB.executeTransaction(function* () {
-						yield this._saveObjectFromJSON(obj, json, {});
-					}.bind(this));
-				}
-				// Otherwise, erase
-				else {
-					toDelete.push(objectsClass.getIDFromLibraryAndKey(libraryID, key));
-				}
-			}
-			if (toDelete.length) {
-				yield objectsClass.erase(
-					toDelete,
-					{
-						skipEditCheck: true,
-						skipDeleteLog: true
-					}
-				);
-			}
-			
-			// Deleted objects
-			keys = yield Zotero.Sync.Data.Local.getDeleted(objectType, libraryID);
-			yield this.removeObjectsFromDeleteLog(objectType, libraryID, keys);
 		}
 		
 		// Mark library for full sync
@@ -305,13 +274,62 @@ Zotero.Sync.Data.Local = {
 	 *
 	 * _libraryHasUnsyncedFiles(), which checks for updated files, must be called first.
 	 */
-	resetUnsyncedLibraryFiles: Zotero.Promise.coroutine(function* (libraryID) {
+	resetUnsyncedLibraryFiles: async function (libraryID) {
-		var itemIDs = yield Zotero.Sync.Storage.Local.getFilesToUpload(libraryID);
+		// Reset unsynced file attachments
+		var itemIDs = await Zotero.Sync.Data.Local.getUnsynced('item', libraryID);
+		var toReset = [];
 		for (let itemID of itemIDs) {
 			let item = Zotero.Items.get(itemID);
-			yield item.deleteAttachmentFile();
+			if (item.isFileAttachment()) {
+				toReset.push(item.id);
 			}
-	}),
+		}
+		await this._resetObjects(libraryID, 'item', toReset);
+		
+		// Delete unsynced files
+		var itemIDs = await Zotero.Sync.Storage.Local.getFilesToUpload(libraryID);
+		for (let itemID of itemIDs) {
+			let item = Zotero.Items.get(itemID);
+			await item.deleteAttachmentFile();
+		}
+	},
+	
+	
+	_resetObjects: async function (libraryID, objectType, ids) {
+		var objectsClass = Zotero.DataObjectUtilities.getObjectsClassForObjectType(objectType);
+		
+		var keys = ids.map(id => objectsClass.getLibraryAndKeyFromID(id).key);
+		var cacheVersions = await this.getLatestCacheObjectVersions(objectType, libraryID, keys);
+		var toDelete = [];
+		for (let key of keys) {
+			let obj = objectsClass.getByLibraryAndKey(libraryID, key);
+			
+			// If object is in cache, overwrite with pristine data
+			if (cacheVersions[key]) {
+				let json = await this.getCacheObject(objectType, libraryID, key, cacheVersions[key]);
+				await Zotero.DB.executeTransaction(async function () {
+					await this._saveObjectFromJSON(obj, json, {});
+				}.bind(this));
+			}
+			// Otherwise, erase
+			else {
+				toDelete.push(objectsClass.getIDFromLibraryAndKey(libraryID, key));
+			}
+		}
+		if (toDelete.length) {
+			await objectsClass.erase(
+				toDelete,
+				{
+					skipEditCheck: true,
+					skipDeleteLog: true
+				}
+			);
+		}
+		
+		// Deleted objects
+		keys = await Zotero.Sync.Data.Local.getDeleted(objectType, libraryID);
+		await this.removeObjectsFromDeleteLog(objectType, libraryID, keys);
+	},
 	
 	
 	getSkippedLibraries: function () {

test/tests/syncEngineTest.js

@@ -3024,6 +3024,67 @@ describe("Zotero.Sync.Data.Engine", function () {
 			assert.equal(called, 1);
 			assert.equal(spy.callCount, 2);
 		});
+		
+		
+		it("should show file-write-access-lost dialog on 403 for attachment upload in group", async function () {
+			var group = await createGroup();
+			var libraryID = group.libraryID;
+			var libraryVersion = 5;
+			group.libraryVersion = libraryVersion;
+			await group.saveTx();
+			
+			({ engine, client, caller } = await setup({
+				libraryID,
+				stopOnError: false
+			}));
+			
+			var item1 = await createDataObject('item', { libraryID });
+			var item2 = await importFileAttachment(
+				'test.png',
+				{
+					libraryID,
+					parentID: item1.id,
+					version: 5
+				}
+			);
+			
+			var called = 0;
+			server.respond(function (req) {
+				let requestJSON = JSON.parse(req.requestBody);
+				if (called == 0) {
+					req.respond(
+						200,
+						{
+							"Last-Modified-Version": ++libraryVersion
+						},
+						JSON.stringify({
+							successful: {
+								0: item1.toResponseJSON({ version: libraryVersion })
+							},
+							unchanged: {},
+							failed: {
+								1: {
+									code: 403,
+									message: "File editing access denied"
+								}
+							}
+						})
+					);
+				}
+				called++;
+			});
+			
+			var promise = waitForDialog();
+			var spy = sinon.spy(engine, "onError");
+			var result = await engine._startUpload();
+			assert.isTrue(promise.isResolved());
+			assert.equal(result, engine.UPLOAD_RESULT_SUCCESS);
+			assert.equal(called, 1);
+			assert.equal(spy.callCount, 1);
+			
+			assert.ok(Zotero.Items.get(item1.id));
+			assert.isFalse(Zotero.Items.get(item2.id));
+		});
 	});
 	
 	

test/tests/syncLocalTest.js

@@ -313,27 +313,88 @@ describe("Zotero.Sync.Data.Local", function() {
 				});
 				var libraryID = group.libraryID;
 				
+				// File attachment that's totally in sync -- leave alone
 				var attachment1 = yield importFileAttachment('test.png', { libraryID });
 				attachment1.attachmentSyncState = "in_sync";
-				attachment1.attachmentSyncedModificationTime = 1234567890000;
+				attachment1.attachmentSyncedModificationTime = yield attachment1.attachmentModificationTime;
-				attachment1.attachmentSyncedHash = "8caf2ee22919d6725eb0648b98ef6bad";
+				attachment1.attachmentSyncedHash = yield attachment1.attachmentHash;
-				var attachment2 = yield importFileAttachment('test.pdf', { libraryID });
+				attachment1.synced = true;
+				yield attachment1.saveTx({
+					skipSyncedUpdate: true
+				});
+				
+				// File attachment that's in sync with changed file -- delete file and mark for download
+				var attachment2 = yield importFileAttachment('test.png', { libraryID });
+				attachment2.synced = true;
+				yield attachment2.saveTx({
+					skipSyncedUpdate: true
+				});
+				
+				// File attachment that's unsynced -- delete item and file
+				var attachment3 = yield importFileAttachment('test.pdf', { libraryID });
 				
 				// Has to be called before resetUnsyncedLibraryFiles()
 				assert.isTrue(yield Zotero.Sync.Data.Local._libraryHasUnsyncedFiles(libraryID));
 				
 				yield Zotero.Sync.Data.Local.resetUnsyncedLibraryFiles(libraryID);
 				
-				assert.isFalse(yield attachment1.fileExists());
+				assert.isTrue(yield attachment1.fileExists());
 				assert.isFalse(yield attachment2.fileExists());
+				assert.isFalse(yield attachment3.fileExists());
 				assert.equal(
-					attachment1.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD
+					attachment1.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_IN_SYNC
 				);
 				assert.equal(
 					attachment2.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD
 				);
+				assert.isFalse(Zotero.Items.get(attachment3.id));
 			});
 		});
+		
+		it("should revert modified file attachment item", async function () {
+			var group = await createGroup({
+				version: 1,
+				libraryVersion: 2
+			});
+			var libraryID = group.libraryID;
+			
+			// File attachment that's changed but file is in sync -- reset item, keep file
+			var attachment = await importFileAttachment('test.png', { libraryID });
+			var originalTitle = attachment.getField('title');
+			attachment.attachmentSyncedModificationTime = await attachment.attachmentModificationTime;
+			attachment.attachmentSyncedHash = await attachment.attachmentHash;
+			attachment.attachmentSyncState = "in_sync";
+			attachment.synced = true;
+			attachment.version = 2;
+			await attachment.saveTx({
+				skipSyncedUpdate: true
+			});
+			// Save original in cache
+			await Zotero.Sync.Data.Local.saveCacheObject(
+				'item',
+				libraryID,
+				Object.assign(
+					attachment.toJSON(),
+					// TEMP: md5 and mtime aren't currently included in JSON, and without it the
+					// file gets marked for download when the item gets reset from the cache
+					{
+						md5: attachment.attachmentHash,
+						mtime: attachment.attachmentSyncedModificationTime
+					}
+				)
+			);
+			// Modify title
+			attachment.setField('title', "New Title");
+			await attachment.saveTx();
+			
+			await Zotero.Sync.Data.Local.resetUnsyncedLibraryFiles(libraryID);
+			
+			assert.isTrue(await attachment.fileExists());
+			assert.equal(attachment.getField('title'), originalTitle);
+			assert.equal(
+				attachment.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_IN_SYNC
+			);
+		});
 	});