From 24cc59cc7efe3ff5c201bf174614b3821e7b1c8b Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Wed, 10 Mar 2021 09:36:56 -0500
Subject: [PATCH] Compress single-file HTML attachments and other text files

The test for ZIP uploads was having multiple files, but now snapshots
are all single files.
---
 chrome/content/zotero/xpcom/storage/zfs.js | 22 ++++++++-----
 test/tests/zfsTest.js                      | 38 ++++++++++++++++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js
index 794b5cbad6..9d9990f9de 100644
--- a/chrome/content/zotero/xpcom/storage/zfs.js
+++ b/chrome/content/zotero/xpcom/storage/zfs.js
@@ -257,15 +257,15 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 	
 	uploadFile: Zotero.Promise.coroutine(function* (request) {
 		var item = Zotero.Sync.Storage.Utilities.getItemFromRequest(request);
-		var multipleFiles = yield Zotero.Attachments.hasMultipleFiles(item);
+		var isZipUpload = yield this._isZipUpload(item);
 		
-		// If we got a quota error for this library, skip upload for all multi-file attachments
+		// If we got a quota error for this library, skip upload for all zipped attachments
 		// and for single-file attachments that are bigger than the remaining space. This is cleared
 		// in storageEngine for manual syncs.
 		var remaining = Zotero.Sync.Storage.Local.storageRemainingForLibrary.get(item.libraryID);
 		if (remaining !== undefined) {
 			let skip = false;
-			if (multipleFiles) {
+			if (isZipUpload) {
 				Zotero.debug("Skipping multi-file upload after quota error");
 				skip = true;
 			}
@@ -288,7 +288,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 			}
 		}
 		
-		if (multipleFiles) {
+		if (isZipUpload) {
 			let created = yield Zotero.Sync.Storage.Utilities.createUploadFile(request);
 			if (!created) {
 				return new Zotero.Sync.Storage.Result;
@@ -346,7 +346,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 		
 		var path = item.getFilePath();
 		var filename = OS.Path.basename(path);
-		var zip = yield Zotero.Attachments.hasMultipleFiles(item);
+		var zip = yield this._isZipUpload(item);
 		if (zip) {
 			var uploadPath = OS.Path.join(Zotero.getTempDirectory().path, item.key + '.zip');
 		}
@@ -818,7 +818,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 		});
 		
 		try {
-			if (yield Zotero.Attachments.hasMultipleFiles(item)) {
+			if (yield this._isZipUpload(item)) {
 				var file = Zotero.getTempDirectory();
 				file.append(item.key + '.zip');
 				yield OS.File.remove(file.path);
@@ -837,7 +837,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 		Zotero.debug("Upload of attachment " + item.key + " cancelled with status code " + status);
 		
 		try {
-			if (yield Zotero.Attachments.hasMultipleFiles(item)) {
+			if (yield this._isZipUpload(item)) {
 				var file = Zotero.getTempDirectory();
 				file.append(item.key + '.zip');
 				file.remove(false);
@@ -850,7 +850,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 	
 	
 	_getUploadFile: Zotero.Promise.coroutine(function* (item) {
-		if (yield Zotero.Attachments.hasMultipleFiles(item)) {
+		if (yield this._isZipUpload(item)) {
 			var file = Zotero.getTempDirectory();
 			var filename = item.key + '.zip';
 			file.append(filename);
@@ -1026,6 +1026,12 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 	}),
 	
 	
+	_isZipUpload: async function (item) {
+		return (item.isImportedAttachment() && item.attachmentContentType.startsWith('text/'))
+			|| Zotero.Attachments.hasMultipleFiles(item);
+	},
+	
+	
 	_getQuotaError: async function (item) {
 		var text, buttonText = null, buttonCallback;
 		var libraryType = item.library.libraryType;
diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js
index 0a3bdb16f9..e64597701c 100644
--- a/test/tests/zfsTest.js
+++ b/test/tests/zfsTest.js
@@ -805,6 +805,44 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () {
 	})
 	
 	
+	describe("#uploadFile()", function () {
+		it("should compress single-file HTML snapshots", async function () {
+			var { engine, client, caller } = await setup();
+			var zfs = new Zotero.Sync.Storage.Mode.ZFS({
+				apiClient: client
+			})
+			
+			var dir = await getTempDirectory();
+			var file = OS.Path.join(getTestDataDirectory().path, 'snapshot', 'index.html');
+			var file2 = OS.Path.join(dir, 'index.html');
+			await OS.File.copy(file, file2);
+			file = file2;
+			
+			var parentItem = await createDataObject('item');
+			var item = await Zotero.Attachments.importSnapshotFromFile({
+				file,
+				url: 'http://example.com/',
+				parentItemID: parentItem.id,
+				title: 'Test',
+				contentType: 'text/html',
+				charset: 'utf-8'
+			});
+			
+			var request = { name: item.libraryKey };
+			
+			var stub = sinon.stub(zfs, '_processUploadFile');
+			await zfs.uploadFile(request);
+			
+			var zipFile = OS.Path.join(Zotero.getTempDirectory().path, item.key + '.zip');
+			// _getUploadFile() should return the ZIP file
+			assert.equal((await zfs._getUploadFile(item)).path, zipFile);
+			assert.isTrue(await OS.File.exists(zipFile));
+			
+			stub.restore();
+		});
+	});
+	
+	
 	describe("#_processUploadFile()", function () {
 		it("should handle 404 from upload authorization request", function* () {
 			var { engine, client, caller } = yield setup();