diff --git a/chrome/content/zotero/xpcom/storage/storageEngine.js b/chrome/content/zotero/xpcom/storage/storageEngine.js index 9ba53726b6..255d8d6d15 100644 --- a/chrome/content/zotero/xpcom/storage/storageEngine.js +++ b/chrome/content/zotero/xpcom/storage/storageEngine.js @@ -292,10 +292,19 @@ Zotero.Sync.Storage.Engine.prototype.start = Zotero.Promise.coroutine(function* }) -Zotero.Sync.Storage.Engine.prototype.stop = function () { - Zotero.debug("Stopping file sync for " + this.library.name); - for (let type in this.queues) { - this.queues[type].stop(); +/** + * @param {String} [queueToStop] - 'upload' or 'download'; if not specified, stop all queues + */ +Zotero.Sync.Storage.Engine.prototype.stop = function (queueToStop) { + if (queueToStop) { + Zotero.debug(`Stopping file sync ${queueToStop} queue for ` + this.library.name); + this.queues[queueToStop].stop(); + } + else { + Zotero.debug("Stopping file sync for " + this.library.name); + for (let type in this.queues) { + this.queues[type].stop(); + } } } @@ -330,9 +339,10 @@ Zotero.Sync.Storage.Engine.prototype.queueItem = Zotero.Promise.coroutine(functi this.queues[type].add(() => { var request = new Zotero.Sync.Storage.Request({ type, + engine: this, libraryID: this.libraryID, name: item.libraryKey, - onStart: request => this.controller[fn](request), + onStart: request => this.controller[fn](request, this), onStop: () => { this.requestsRemaining--; this.onProgress(this.numRequests - this.requestsRemaining, this.numRequests); diff --git a/chrome/content/zotero/xpcom/storage/storageLocal.js b/chrome/content/zotero/xpcom/storage/storageLocal.js index cf084196cb..4b50b767be 100644 --- a/chrome/content/zotero/xpcom/storage/storageLocal.js +++ b/chrome/content/zotero/xpcom/storage/storageLocal.js @@ -9,6 +9,10 @@ Zotero.Sync.Storage.Local = { SYNC_STATE_FORCE_DOWNLOAD: 4, SYNC_STATE_IN_CONFLICT: 5, + // If last-known remaining storage is below this number of megabytes, skip further upload + // attempts in various situations + STORAGE_REMAINING_MINIMUM: 5, + lastFullFileCheck: {}, uploadCheckFiles: [], storageRemainingForLibrary: new Map(), diff --git a/chrome/content/zotero/xpcom/storage/storageRequest.js b/chrome/content/zotero/xpcom/storage/storageRequest.js index fbbb7ebbaa..573b4ebb91 100644 --- a/chrome/content/zotero/xpcom/storage/storageRequest.js +++ b/chrome/content/zotero/xpcom/storage/storageRequest.js @@ -28,6 +28,7 @@ * Transfer request for storage sync * * @param {Object} options + * @param {Zotero.Sync.Storage.Engine} options.engine * @param {String} options.type * @param {Integer} options.libraryID * @param {String} options.name - Identifier for request (e.g., "[libraryID]/[key]") @@ -36,16 +37,18 @@ * @param {Function|Function[]} [options.onStop] */ Zotero.Sync.Storage.Request = function (options) { + if (!options.engine) throw new Error("engine must be provided"); if (!options.type) throw new Error("type must be provided"); if (!options.libraryID) throw new Error("libraryID must be provided"); if (!options.name) throw new Error("name must be provided"); - ['type', 'libraryID', 'name'].forEach(x => this[x] = options[x]); + ['engine', 'type', 'libraryID', 'name'].forEach(x => this[x] = options[x]); Zotero.debug(`Initializing ${this.type} request ${this.name}`); this.callbacks = ['onStart', 'onProgress', 'onStop']; this.Type = Zotero.Utilities.capitalize(this.type); + this.engine = options.engine; this.channel = null; this.queue = null; this.progress = 0; diff --git a/chrome/content/zotero/xpcom/storage/zfs.js b/chrome/content/zotero/xpcom/storage/zfs.js index 9d9990f9de..0d3e0a9f73 100644 --- a/chrome/content/zotero/xpcom/storage/zfs.js +++ b/chrome/content/zotero/xpcom/storage/zfs.js @@ -284,6 +284,11 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } } if (skip) { + // Stop trying to upload files if there's very little storage remaining + if (remaining < Zotero.Sync.Storage.Local.STORAGE_REMAINING_MINIMUM) { + Zotero.debug(`${remaining} MB remaining in storage -- skipping further uploads`); + request.engine.stop('upload'); + } throw yield this._getQuotaError(item); } } @@ -294,7 +299,22 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { return new Zotero.Sync.Storage.Result; } } - return this._processUploadFile(request); + + try { + return yield this._processUploadFile(request); + } + catch (e) { + // Stop trying to upload files if we hit a quota error and there's very little space + // remaining. If there's more space, we keep going, because it might just be a big file. + if (e.error == Zotero.Error.ERROR_ZFS_OVER_QUOTA) { + let remaining = Zotero.Sync.Storage.Local.storageRemainingForLibrary.get(item.libraryID); + if (remaining < Zotero.Sync.Storage.Local.STORAGE_REMAINING_MINIMUM) { + Zotero.debug(`${remaining} MB remaining in storage -- skipping further uploads`); + request.engine.stop('upload'); + } + } + throw e; + } }), @@ -607,7 +627,7 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = { } // Store the remaining space so that we can skip files bigger than that until the next - // manual sync + // manual sync. Values are in megabytes. let usage = req.getResponseHeader('Zotero-Storage-Usage'); let quota = req.getResponseHeader('Zotero-Storage-Quota'); Zotero.Sync.Storage.Local.storageRemainingForLibrary.set(item.libraryID, quota - usage); diff --git a/test/tests/zfsTest.js b/test/tests/zfsTest.js index e64597701c..afdfd2a2a6 100644 --- a/test/tests/zfsTest.js +++ b/test/tests/zfsTest.js @@ -74,9 +74,10 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var setup = Zotero.Promise.coroutine(function* (options = {}) { Components.utils.import("resource://zotero/concurrentCaller.js"); + var stopOnError = options.stopOnError !== undefined ? options.stopOnError : true; var caller = new ConcurrentCaller(1); caller.setLogger(msg => Zotero.debug(msg)); - caller.stopOnError = true; + caller.stopOnError = stopOnError; Components.utils.import("resource://zotero/config.js"); var client = new Zotero.Sync.APIClient({ @@ -93,7 +94,8 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { apiClient: client, maxS3ConsecutiveFailures: 2 }), - stopOnError: true + background: options.background, + stopOnError }); return { engine, client, caller }; @@ -802,6 +804,75 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var result = yield engine.start(); assert.equal(called, 4); }); + + + it("should stop uploading files on quota error", async function () { + var { engine, client, caller } = await setup({ stopOnError: false }); + + var numItems = 4; + var items = []; + for (let i = 0; i < numItems; i++) { + let item = await importFileAttachment('test.png'); + item.version = 5; + item.synced = true; + await item.saveTx(); + items.push(item); + } + + var requests = 0; + server.respond(function (req) { + if (req.method == "POST" + && req.url.startsWith(`${baseURL}users/1/items/`) + && req.url.endsWith('/file') + && req.requestBody.indexOf('upload=') == -1 + && req.requestHeaders["If-None-Match"] == "*") { + requests++; + req.respond( + 413, + { + "Content-Type": "application/json", + "Last-Modified-Version": 10, + "Zotero-Storage-Usage": "300", + "Zotero-Storage-Quota": "300" + }, + "File would exceed quota (299.7 + 0.5 > 300)" + ); + } + }) + + await engine.start(); + assert.equal(requests, Zotero.Prefs.get('sync.storage.maxUploads')); + + Zotero.Sync.Storage.Local.storageRemainingForLibrary.delete(items[0].libraryID); + }); + + + // If there was a quota error in a previous run and remaining storage was determined to be + // very low, stop further file uploads for a background sync even when we bail without an + // HTTP request. A manual sync clears the remaining-storage value. + it("should stop uploading files for background sync if no storage remaining after previous quota error", async function () { + var { engine, client, caller } = await setup({ background: true, stopOnError: false }); + + var numItems = 4; + var items = []; + for (let i = 0; i < numItems; i++) { + let item = await importFileAttachment('test.png'); + item.version = 5; + item.synced = true; + await item.saveTx(); + items.push(item); + } + + Zotero.Sync.Storage.Local.storageRemainingForLibrary.set(items[0].libraryID, 0); + + var spy = sinon.spy(engine.controller, 'uploadFile'); + await engine.start() + + assert.equal(spy.callCount, Zotero.Prefs.get('sync.storage.maxUploads')); + spy.restore(); + + Zotero.Sync.Storage.Local.storageRemainingForLibrary.delete(items[0].libraryID); + }); }) @@ -830,7 +901,7 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { var request = { name: item.libraryKey }; - var stub = sinon.stub(zfs, '_processUploadFile'); + var stub = sinon.stub(zfs, '_processUploadFile').returns(Zotero.Promise.resolve()); await zfs.uploadFile(request); var zipFile = OS.Path.join(Zotero.getTempDirectory().path, item.key + '.zip'); @@ -1057,7 +1128,7 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { "Zotero-Storage-Usage": "300", "Zotero-Storage-Quota": "300" }, - "File would exceed quota (299.7 + 0.5 > 300)" + "File would exceed quota (299.7 + 0.5 > 300)" ); } }) @@ -1073,7 +1144,8 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () { // Try again var e = yield getPromiseError(zfs.uploadFile({ - name: item.libraryKey + name: item.libraryKey, + engine })); assert.ok(e); assert.equal(e.errorType, 'warning');