From b9176676746e247a2d87a3acfe14a9fd325f593d Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Thu, 12 Jun 2025 18:04:50 -0700 Subject: [PATCH 1/2] server connector endpoint to cancel a session - added /connector/cancelSession endpoint to connector server that the connector can call to cancel or undo the saving process. - if there is not session, it will be created, so we can record that the session with that ID is canceled for further endpoint calls. - when the session is cancelled, subsequent calls to /saveAttachment, /saveSnapshot, /saveSingleFile, /saveItems, /updateSession are rejected with 409 error. - after the session is cancelled, wait a second, and try to remove any existing session items again. This accounts for edge cases like if an items is saved at the same time as the session is cancelled. Additionally, fixed session not getting removed or garbage-collected --- .../zotero/xpcom/server/saveSession.js | 18 ++- .../zotero/xpcom/server/server_connector.js | 62 +++++++- test/tests/server_connectorTest.js | 149 ++++++++++++++++++ 3 files changed, 224 insertions(+), 5 deletions(-) diff --git a/chrome/content/zotero/xpcom/server/saveSession.js b/chrome/content/zotero/xpcom/server/saveSession.js index 54aff5e402..c5eadf87e7 100644 --- a/chrome/content/zotero/xpcom/server/saveSession.js +++ b/chrome/content/zotero/xpcom/server/saveSession.js @@ -49,9 +49,9 @@ Zotero.Server.Connector.SessionManager = { var ttl = this._sessions.size >= 10 ? 60 : 600; var deleteBefore = new Date() - ttl * 1000; - for (let session of this._sessions) { + for (let [_, session] of this._sessions.entries()) { if (session.created < deleteBefore) { - this._session.delete(session.id); + this._sessions.delete(session.id); } } } @@ -63,6 +63,7 @@ Zotero.Server.Connector.SaveSession = class { constructor(id, action, requestData) { this.id = id; this.created = new Date(); + this.cancelled = false; this._action = action; this._requestData = requestData; this._items = {}; @@ -112,6 +113,8 @@ Zotero.Server.Connector.SaveSession = class { } async saveSnapshot(target) { + // This should not be called if session is cancelled but just in case + if (this.cancelled) return; var { library, collection } = Zotero.Server.Connector.resolveTarget(target); var libraryID = library.libraryID; var data = this._requestData.data; @@ -160,7 +163,14 @@ Zotero.Server.Connector.SaveSession = class { } remove() { - delete Zotero.Server.Connector.SessionManager._sessions[this.id]; + Zotero.Server.Connector.SessionManager._sessions.delete(this.id); + } + + async cancel() { + this.cancelled = true; + for (let item of Object.values(this._items)) { + await item.eraseTx(); + } } /** @@ -202,6 +212,8 @@ Zotero.Server.Connector.SaveSession = class { if (Object.values(items).length == 0) { return; } + // This should not be called if session is cancelled but just in case + if (this.cancelled) return; var { library, collection } = Zotero.Server.Connector.resolveTarget(this._currentTargetID); var libraryID = library.libraryID; diff --git a/chrome/content/zotero/xpcom/server/server_connector.js b/chrome/content/zotero/xpcom/server/server_connector.js index 50e53214ef..4872756ec3 100644 --- a/chrome/content/zotero/xpcom/server/server_connector.js +++ b/chrome/content/zotero/xpcom/server/server_connector.js @@ -329,8 +329,9 @@ Zotero.Server.Connector.SaveItems.prototype = { ); } catch (e) { + let session = Zotero.Server.Connector.SessionManager.get(data.sessionID); Zotero.debug(e); - return [409, "application/json", JSON.stringify({ error: "SESSION_EXISTS" })]; + return [409, "application/json", JSON.stringify({ error: session?.cancelled ? "SESSION_CANCELLED" : "SESSION_EXISTS" })]; } await session.update(targetID); @@ -508,6 +509,10 @@ Zotero.Server.Connector.SaveAttachment.prototype = { return [200, 'text/plain', 'Library files are not editable.']; } + if (session.cancelled) { + Zotero.debug("Session is cancelled: " + session.sessionID, 1); + return [409, "application/json", JSON.stringify({ error: "SESSION_CANCELLED" })]; + } // Save attachment based on provided parent id from stream let parentItem = session.getItemByConnectorKey(metadata.parentItemID); await Zotero.Attachments.importFromNetworkStream({ @@ -563,6 +568,10 @@ Zotero.Server.Connector.SaveSingleFile.prototype = { Zotero.debug("Can't find session " + data.sessionID, 1); return [400, "application/json", JSON.stringify({ error: "SESSION_NOT_FOUND" })]; } + if (session.cancelled) { + Zotero.debug("Session is cancelled: " + data.sessionID, 1); + return [409, "application/json", JSON.stringify({ error: "SESSION_CANCELLED" })]; + } let { library } = Zotero.Server.Connector.getSaveTarget(); if (!library.filesEditable) { @@ -635,8 +644,9 @@ Zotero.Server.Connector.SaveSnapshot.prototype = { ); } catch (e) { + let session = Zotero.Server.Connector.SessionManager.get(data.sessionID); Zotero.debug(e); - return [409, "application/json", JSON.stringify({ error: "SESSION_EXISTS" })]; + return [409, "application/json", JSON.stringify({ error: session?.cancelled ? "SESSION_CANCELLED" : "SESSION_EXISTS" })]; } await session.update(collection ? collection.treeViewID : library.treeViewID); @@ -756,6 +766,10 @@ Zotero.Server.Connector.UpdateSession.prototype = { Zotero.debug("Can't find session " + data.sessionID, 1); return [400, "application/json", JSON.stringify({ error: "SESSION_NOT_FOUND" })]; } + if (session.cancelled) { + Zotero.debug("Session is cancelled: " + session.sessionID, 1); + return [409, "application/json", JSON.stringify({ error: "SESSION_CANCELLED" })]; + } // Parse treeViewID var [type, id] = [data.target[0], parseInt(data.target.substr(1))]; @@ -780,6 +794,49 @@ Zotero.Server.Connector.UpdateSession.prototype = { } }; +/** + * + * + * Accepts: + * sessionID - A session ID + * + * Returns: + * 200 response if the session is cancelled + * 400 if the session ID is not provided + */ +Zotero.Server.Connector.CancelSession = function() {}; +Zotero.Server.Endpoints["/connector/cancelSession"] = Zotero.Server.Connector.CancelSession; +Zotero.Server.Connector.CancelSession.prototype = { + supportedMethods: ["POST"], + supportedDataTypes: ["application/json"], + permitBookmarklet: true, + + init: async function (requestData) { + var data = requestData.data; + + if (!data.sessionID) { + return [400, "application/json", JSON.stringify({ error: "SESSION_ID_NOT_PROVIDED" })]; + } + + var session = Zotero.Server.Connector.SessionManager.get(data.sessionID); + // If the session with the specified ID does not exist, create it here so + // we can record it as cancelled. + if (!session) { + session = Zotero.Server.Connector.SessionManager.create(data.sessionID); + } + // After a session is cancelled, all items added are removed and + // subsequent child attachments/notes should no longer be added. + // To handle any possible race conditions (e.g. if an item is being added at the same time as + // the session is being cancelled), wait a second and try to clear any remaining items again. + await session.cancel(); + setTimeout(() => { + session.cancel(); + }, 1000); + + return [200, "application/json", JSON.stringify({})]; + } +}; + Zotero.Server.Connector.DelaySync = function () {}; Zotero.Server.Endpoints["/connector/delaySync"] = Zotero.Server.Connector.DelaySync; @@ -1116,6 +1173,7 @@ Zotero.Server.Connector.Ping.prototype = { supportsTagsAutocomplete: true, googleDocsAddNoteEnabled: true, canUserAddNote: true, + supportsSaveCancelling: true, googleDocsCitationExplorerEnabled: false, translatorsHash, sortedTranslatorHash diff --git a/test/tests/server_connectorTest.js b/test/tests/server_connectorTest.js index 8e91c81bd7..ff6d219be0 100644 --- a/test/tests/server_connectorTest.js +++ b/test/tests/server_connectorTest.js @@ -1928,4 +1928,153 @@ describe("Connector Server", function () { assert.equal(JSON.parse(req.response).status, 200); }); }); + + describe('/connector/cancelSession', function () { + var pdfArrayBuffer; + var pdfPath = OS.Path.join(getTestDataDirectory().path, 'test.pdf'); + before(async () => { + await selectLibrary(win, Zotero.Libraries.userLibraryID); + pdfArrayBuffer = (await OS.File.read(pdfPath)).buffer; + }); + + it('should delete saved items if called after everything is saved', async function () { + var collection = await createDataObject('collection'); + let sessionID = Zotero.Utilities.randomString(); + let bookItemID = Zotero.Utilities.randomString(); + await select(win, collection); + + let body = { + sessionID, + items: [ + { + id: bookItemID, + itemType: "book", + title: "Book Title", + } + ] + }; + let itemAddPromise = waitForItemEvent('add'); + // Save the item as if by the connector + await Zotero.HTTP.request( + 'POST', + connectorServerPath + "/connector/saveItems", + { + headers: { + "Content-Type": "application/json" + }, + body: JSON.stringify(body) + } + ); + let itemIDs = await itemAddPromise; + + // Add the attachment to the item + let attachmentAddPromise = waitForItemEvent('add'); + let attachmentReq = await httpRequest( + 'POST', + connectorServerPath + "/connector/saveAttachment", + { + headers: { + "Content-Type": "application/pdf", + "X-Metadata": JSON.stringify({ + sessionID, + title: "Book Attachment", + parentItemID: bookItemID, + url: `${testServerPath}/attachment1.pdf`, + }) + }, + body: pdfArrayBuffer + } + ); + assert.equal(attachmentReq.status, 201); + let attachmentIds = await attachmentAddPromise; + assert.lengthOf(attachmentIds, 1); + let attachment1 = Zotero.Items.get(attachmentIds[0]); + let bookItem = Zotero.Items.get(itemIDs[0]); + assert.equal(attachment1.parentItemID, bookItem.id); + + // Cancel the session + let cancelReq = await Zotero.HTTP.request( + 'POST', + connectorServerPath + '/connector/cancelSession', + { + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ sessionID }) + } + ); + assert.equal(cancelReq.status, 200); + + // The item and attachment should be deleted + let deletedItem = await Zotero.Items.getAsync(attachment1.parentItemID); + let deletedAttachment = await Zotero.Items.getAsync(attachment1.id); + assert.isFalse(deletedItem); + assert.isFalse(deletedAttachment); + }); + + it('should not save attachment items if session is cancelled half-way through', async function () { + var collection = await createDataObject('collection'); + let sessionID = Zotero.Utilities.randomString(); + let bookItemID = Zotero.Utilities.randomString(); + await select(win, collection); + + let body = { + sessionID, + items: [ + { + id: bookItemID, + itemType: "book", + title: "Book Title", + } + ] + }; + + let itemAddPromise = waitForItemEvent('add'); + // Save the item as if by the connector + await Zotero.HTTP.request( + 'POST', + connectorServerPath + "/connector/saveItems", + { + headers: { + "Content-Type": "application/json" + }, + body: JSON.stringify(body) + } + ); + let itemIDs = await itemAddPromise; + + // Cancel the session + let cancelReq = await Zotero.HTTP.request( + 'POST', + connectorServerPath + '/connector/cancelSession', + { + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ sessionID }) + } + ); + assert.equal(cancelReq.status, 200); + + // Try to add the attachment to the item + let attachmentReq = await getPromiseError(httpRequest( + 'POST', + connectorServerPath + "/connector/saveAttachment", + { + headers: { + "Content-Type": "application/pdf", + "X-Metadata": JSON.stringify({ + sessionID, + title: "Book Attachment", + parentItemID: bookItemID, + url: `${testServerPath}/attachment1.pdf`, + }) + }, + body: pdfArrayBuffer + } + )); + // It should be rejected + assert.equal(attachmentReq.status, 409); + + // The parent item should also be removed + let bookItem = await Zotero.Items.getAsync(itemIDs[0]); + assert.isFalse(bookItem); + }); + }); }); From 8dd7c899a271c681bcc641fecdaa0c695916dbd2 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Tue, 15 Jul 2025 15:30:39 -0700 Subject: [PATCH 2/2] indicate which group is user library --- chrome/content/zotero/xpcom/server/server_connector.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/server/server_connector.js b/chrome/content/zotero/xpcom/server/server_connector.js index 4872756ec3..c75acca038 100644 --- a/chrome/content/zotero/xpcom/server/server_connector.js +++ b/chrome/content/zotero/xpcom/server/server_connector.js @@ -1027,7 +1027,8 @@ Zotero.Server.Connector.GetSelectedCollection.prototype = { id: library.treeViewID, name: library.name, filesEditable: library.filesEditable, - level: 0 + level: 0, + isUserLibrary: Zotero.Libraries.userLibraryID == library.libraryID, }, ...Zotero.Collections.getByLibrary(library.libraryID, true).map(c => ({ id: c.treeViewID,