diff --git a/app/server/lib/DocSnapshots.ts b/app/server/lib/DocSnapshots.ts index 01b1f5a1..a353502f 100644 --- a/app/server/lib/DocSnapshots.ts +++ b/app/server/lib/DocSnapshots.ts @@ -148,7 +148,29 @@ export class DocSnapshotInventory implements IInventory { * The snapshot supplied will be modified in place to a normalized form. */ public async add(key: string, snapshot: ObjSnapshotWithMetadata, prevSnapshotId: string|null) { + await this.uploadAndAdd(key, async () => { + return { snapshot, prevSnapshotId }; + }); + } + + /** + * Like add(), but takes an "upload" callback that allows + * preparing snapshot and prevSnapshotId atomically with the + * rest of the add operation, and thus serialized with any + * other operations such as versions(). This is important since an + * upload changes the list of versions as far as the external store + * is concerned, which could trigger a "surprise" and a full reload + * of the version list. + */ + public async uploadAndAdd(key: string, + upload: () => Promise<{snapshot?: ObjSnapshotWithMetadata, + prevSnapshotId: string|null}>) { await this._mutex.runExclusive(key, async() => { + const {snapshot, prevSnapshotId} = await upload(); + if (!snapshot) { + // the upload generated no snapshot, so there is nothing to do. + return; + } const snapshots = await this._getSnapshots(key, prevSnapshotId); // Could be already added if reconstruction happened. if (snapshots[0] && snapshots[0].snapshotId === snapshot.snapshotId) { return; } diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts index ac33b4e1..30f64298 100644 --- a/app/server/lib/HostedStorageManager.ts +++ b/app/server/lib/HostedStorageManager.ts @@ -739,23 +739,29 @@ export class HostedStorageManager implements IDocStorageManager { ...label && {label}, t, }; - const prevSnapshotId = this._latestVersions.get(docId) || null; - const newSnapshotId = await this._ext.upload(docId, tmpPath, metadata); - if (newSnapshotId === Unchanged) { - // Nothing uploaded because nothing changed - return; + let changeMade: boolean = false; + await this._inventory.uploadAndAdd(docId, async () => { + const prevSnapshotId = this._latestVersions.get(docId) || null; + const newSnapshotId = await this._ext.upload(docId, tmpPath as string, metadata); + if (newSnapshotId === Unchanged) { + // Nothing uploaded because nothing changed + return { prevSnapshotId }; + } + if (!newSnapshotId) { + // This is unexpected. + throw new Error('No snapshotId allocated after upload'); + } + const snapshot = { + lastModified: t, + snapshotId: newSnapshotId, + metadata + }; + changeMade = true; + return { snapshot, prevSnapshotId }; + }); + if (changeMade) { + await this._onInventoryChange(docId); } - if (!newSnapshotId) { - // This is unexpected. - throw new Error('No snapshotId allocated after upload'); - } - const snapshot = { - lastModified: t, - snapshotId: newSnapshotId, - metadata - }; - await this._inventory.add(docId, snapshot, prevSnapshotId); - await this._onInventoryChange(docId); } finally { // Clean up backup. // NOTE: fse.remove succeeds also when the file does not exist.