Fix incorrect purging of files on WebDAV

chrome/content/zotero/xpcom/storage/webdav.js

@@ -940,6 +940,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 	 */
 	purgeOrphanedStorageFiles: Zotero.Promise.coroutine(function* () {
 		const libraryID = Zotero.Libraries.userLibraryID;
+		const library = Zotero.Libraries.get(libraryID);
 		const daysBeforeSyncTime = 7;
 		
 		// If recently purged, skip
@@ -966,7 +967,7 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 			+ "<getlastmodified/>"
 			+ "</prop></propfind>";
 		
-		var lastSyncDate = Zotero.Libraries.userLibrary.lastSync;
+		var lastSyncDate = library.lastSync;
 		if (!lastSyncDate) {
 			Zotero.debug(`No last sync date for library ${libraryID} -- not purging orphaned files`);
 			return false;
@@ -990,6 +991,9 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 			return Zotero.Utilities.xpath(this, path, { D: 'DAV:' });
 		};
 		
+		var syncQueueKeys = new Set(
+			yield Zotero.Sync.Data.Local.getObjectsFromSyncQueue('item', libraryID)
+		);
 		var deleteFiles = [];
 		var trailingSlash = !!path.match(/\/$/);
 		for (let response of responseNode.xpath("D:response")) {
@@ -1045,20 +1049,24 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 				continue;
 			}
 			
-			var isLastSyncFile = file !== 'lastsync.txt' || file != 'lastsync';
+			var isLastSyncFile = file == 'lastsync.txt' || file == 'lastsync';
-			
-			if (!file.match(/\.zip$/) && !file.match(/\.prop$/) && !isLastSyncFile) {
-				Zotero.debug("Skipping file " + file);
-				continue;
-			}
-			
 			if (!isLastSyncFile) {
-				var key = file.replace(/\.(zip|prop)$/, '');
+				if (!file.match(/\.zip$/) && !file.match(/\.prop$/)) {
-				var item = yield Zotero.Items.getByLibraryAndKeyAsync(libraryID, key);
+					Zotero.debug("Skipping file " + file);
+					continue;
+				}
+				
+				let key = file.replace(/\.(zip|prop)$/, '');
+				let item = yield Zotero.Items.getByLibraryAndKeyAsync(libraryID, key);
 				if (item) {
 					Zotero.debug("Skipping existing file " + file);
 					continue;
 				}
+				
+				if (syncQueueKeys.has(key)) {
+					Zotero.debug(`Skipping file for item ${key} in sync queue`);
+					continue;
+				}
 			}
 			
 			Zotero.debug("Checking orphaned file " + file);
@@ -1082,6 +1090,8 @@ Zotero.Sync.Storage.Mode.WebDAV.prototype = {
 		var results = yield this._deleteStorageFiles(deleteFiles);
 		Zotero.Prefs.set("lastWebDAVOrphanPurge", Math.round(new Date().getTime() / 1000));
 		Zotero.debug(results);
+		
+		return results;
 	}),
 	
 	

test/tests/webdavTest.js

@@ -697,6 +697,13 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 			library.updateLastSyncTime();
 			yield library.saveTx();
 			
+			// Create one item
+			var item1 = yield createDataObject('item');
+			var item1Key = item1.key;
+			// Add another item to sync queue
+			var item2Key = Zotero.DataObjectUtilities.generateKey();
+			yield Zotero.Sync.Data.Local.addObjectsToSyncQueue('item', library.id, [item2Key]);
+			
 			const daysBeforeSyncTime = 7;
 			
 			var beforeTime = new Date(Date.now() - (daysBeforeSyncTime * 86400 * 1000 + 1)).toUTCString();
@@ -738,6 +745,7 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 								+ '<D:status>HTTP/1.1 200 OK</D:status>'
 							+ '</D:propstat>'
 						+ '</D:response>'
+						
 						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
 							+ `<D:href>${davBasePath}zotero/AAAAAAAA.zip</D:href>`
 							+ '<D:propstat>'
@@ -756,6 +764,7 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 								+ '<D:status>HTTP/1.1 200 OK</D:status>'
 							+ '</D:propstat>'
 						+ '</D:response>'
+						
 						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
 							+ `<D:href>${davBasePath}zotero/BBBBBBBB.zip</D:href>`
 							+ '<D:propstat>'
@@ -774,6 +783,46 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 								+ '<D:status>HTTP/1.1 200 OK</D:status>'
 							+ '</D:propstat>'
 						+ '</D:response>'
+						
+						// Item that exists
+						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
+							+ `<D:href>${davBasePath}zotero/${item1Key}.zip</D:href>`
+							+ '<D:propstat>'
+								+ '<D:prop>'
+								+ `<lp1:getlastmodified>${beforeTime}</lp1:getlastmodified>`
+								+ '</D:prop>'
+								+ '<D:status>HTTP/1.1 200 OK</D:status>'
+							+ '</D:propstat>'
+						+ '</D:response>'
+						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
+							+ `<D:href>${davBasePath}zotero/${item1Key}.prop</D:href>`
+							+ '<D:propstat>'
+								+ '<D:prop>'
+								+ `<lp1:getlastmodified>${beforeTime}</lp1:getlastmodified>`
+								+ '</D:prop>'
+								+ '<D:status>HTTP/1.1 200 OK</D:status>'
+							+ '</D:propstat>'
+						+ '</D:response>'
+						
+						// Item in sync queue
+						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
+							+ `<D:href>${davBasePath}zotero/${item2Key}.zip</D:href>`
+							+ '<D:propstat>'
+								+ '<D:prop>'
+								+ `<lp1:getlastmodified>${beforeTime}</lp1:getlastmodified>`
+								+ '</D:prop>'
+								+ '<D:status>HTTP/1.1 200 OK</D:status>'
+							+ '</D:propstat>'
+						+ '</D:response>'
+						+ '<D:response xmlns:lp1="DAV:" xmlns:lp2="http://apache.org/dav/props/">'
+							+ `<D:href>${davBasePath}zotero/${item2Key}.prop</D:href>`
+							+ '<D:propstat>'
+								+ '<D:prop>'
+								+ `<lp1:getlastmodified>${beforeTime}</lp1:getlastmodified>`
+								+ '</D:prop>'
+								+ '<D:status>HTTP/1.1 200 OK</D:status>'
+							+ '</D:propstat>'
+						+ '</D:response>'
 					+ '</D:multistatus>'
 			});
 			setResponse({
@@ -797,8 +846,12 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () {
 				status: 204
 			});
 			
-			yield controller.purgeOrphanedStorageFiles();
+			var results = yield controller.purgeOrphanedStorageFiles();
 			assertRequestCount(5);
+			
+			assert.sameMembers(results.deleted, ['lastsync.txt', 'lastsync', 'AAAAAAAA.prop', 'AAAAAAAA.zip']);
+			assert.lengthOf(results.missing, 0);
+			assert.lengthOf(results.error, 0);
 		})
 		
 		it("shouldn't purge if purged recently", function* () {