diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index ab57d02a2e..005bf8afa0 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -25,6 +25,7 @@ import { import {addCurrentOrgToPath, getGristConfig} from 'app/common/urlUtils'; import { AxiosProgressEvent } from 'axios'; import omitBy from 'lodash/omitBy'; +import {StringUnion} from 'app/common/StringUnion'; export type {FullUser, UserProfile}; @@ -481,6 +482,11 @@ interface SqlResult extends TableRecordValuesWithoutIds { statement: string; } +export const DocAttachmentsLocation = StringUnion( + "none", "internal", "mixed", "external" +); +export type DocAttachmentsLocation = typeof DocAttachmentsLocation.type; + /** * Collect endpoints related to the content of a single document that we've been thinking * of as the (restful) "Doc API". A few endpoints that could be here are not, for historical diff --git a/app/plugin/DocApiTypes-ti.ts b/app/plugin/DocApiTypes-ti.ts index ed2d8a0b99..cdffbc8da8 100644 --- a/app/plugin/DocApiTypes-ti.ts +++ b/app/plugin/DocApiTypes-ti.ts @@ -90,6 +90,10 @@ export const SqlPost = t.iface([], { "timeout": t.opt("number"), }); +export const SetAttachmentStorePost = t.iface([], { + "type": t.union(t.lit("internal"), t.lit("external")), +}); + const exportedTypeSuite: t.ITypeSuite = { NewRecord, NewRecordWithStringId, @@ -108,5 +112,6 @@ const exportedTypeSuite: t.ITypeSuite = { TablesPost, TablesPatch, SqlPost, + SetAttachmentStorePost, }; export default exportedTypeSuite; diff --git a/app/plugin/DocApiTypes.ts b/app/plugin/DocApiTypes.ts index ed99c3b398..996c5c8ff2 100644 --- a/app/plugin/DocApiTypes.ts +++ b/app/plugin/DocApiTypes.ts @@ -120,3 +120,7 @@ export interface SqlPost { // other queued queries on same document, because of // limitations of API node-sqlite3 exposes. } + +export interface SetAttachmentStorePost { + type: "internal" | "external" +} diff --git a/app/server/generateInitialDocSql.ts b/app/server/generateInitialDocSql.ts index 661b4542d3..68b0038ae5 100644 --- a/app/server/generateInitialDocSql.ts +++ b/app/server/generateInitialDocSql.ts @@ -34,11 +34,13 @@ export async function main(baseName: string) { if (await fse.pathExists(fname)) { await fse.remove(fname); } - const docManager = new DocManager(storageManager, pluginManager, null as any, new AttachmentStoreProvider([], ""), { - create, - getAuditLogger() { return createNullAuditLogger(); }, - getTelemetry() { return createDummyTelemetry(); }, - } as any); + const docManager = new DocManager(storageManager, pluginManager, null as any, + new AttachmentStoreProvider([], ""), { + create, + getAuditLogger() { return createNullAuditLogger(); }, + getTelemetry() { return createDummyTelemetry(); }, + } as any + ); const activeDoc = new ActiveDoc(docManager, baseName); const session = makeExceptionalDocSession('nascent'); await activeDoc.createEmptyDocWithDataEngine(session); diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 32b2050eb4..5f439fa2e3 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -286,7 +286,7 @@ export class ActiveDoc extends EventEmitter { constructor( private readonly _docManager: DocManager, private _docName: string, - externalAttachmentStoreProvider?: IAttachmentStoreProvider, + private _attachmentStoreProvider?: IAttachmentStoreProvider, private _options?: ICreateActiveDocOptions ) { super(); @@ -392,11 +392,11 @@ export class ActiveDoc extends EventEmitter { loadTable: this._rawPyCall.bind(this, 'load_table'), }); - // This will throw errors if _options?.doc or externalAttachmentStoreProvider aren't provided, + // This will throw errors if _options?.doc or _attachmentStoreProvider aren't provided, // and ActiveDoc tries to use an external attachment store. this._attachmentFileManager = new AttachmentFileManager( this.docStorage, - externalAttachmentStoreProvider, + _attachmentStoreProvider, _options?.doc, ); @@ -871,8 +871,12 @@ export class ActiveDoc extends EventEmitter { } } ); - const userActions: UserAction[] = await Promise.all( - upload.files.map(file => this._prepAttachment(docSession, file))); + const userActions: UserAction[] = []; + // Process uploads sequentially to reduce risk of race conditions. + // Minimal performance impact due to the main async operation being serialized SQL queries. + for (const file of upload.files) { + userActions.push(await this._prepAttachment(docSession, file)); + } const result = await this._applyUserActionsWithExtendedOptions(docSession, userActions, { attachment: true, }); @@ -945,6 +949,57 @@ export class ActiveDoc extends EventEmitter { return data; } + @ActiveDoc.keepDocOpen + public async startTransferringAllAttachmentsToDefaultStore() { + const attachmentStoreId = (await this._getDocumentSettings()).attachmentStoreId; + // If no attachment store is set on the doc, it should transfer everything to internal storage + await this._attachmentFileManager.startTransferringAllFilesToOtherStore(attachmentStoreId); + } + + /** + * Returns a summary of pending attachment transfers between attachment stores. + */ + public attachmentTransferStatus() { + return this._attachmentFileManager.transferStatus(); + } + + /** + * Returns a summary of where attachments on this doc are stored. + */ + public async attachmentLocationSummary() { + return await this._attachmentFileManager.locationSummary(); + } + + /* + * Wait for all attachment transfers to be finished, keeping the doc open + * for as long as possible. + */ + @ActiveDoc.keepDocOpen + public async allAttachmentTransfersCompleted() { + await this._attachmentFileManager.allTransfersCompleted(); + } + + + public async setAttachmentStore(docSession: OptDocSession, id: string | undefined): Promise { + const docSettings = await this._getDocumentSettings(); + docSettings.attachmentStoreId = id; + await this._updateDocumentSettings(docSession, docSettings); + } + + /** + * Sets the document attachment store using the store's label. + * This avoids needing to know the exact store ID, which can be challenging to calculate in all + * the places we might want to set the store. + */ + public async setAttachmentStoreFromLabel(docSession: OptDocSession, label: string | undefined): Promise { + const id = label === undefined ? undefined : this._attachmentStoreProvider?.getStoreIdFromLabel(label); + return this.setAttachmentStore(docSession, id); + } + + public async getAttachmentStore(): Promise { + return (await this._getDocumentSettings()).attachmentStoreId; + } + /** * Fetches the meta tables to return to the client when first opening a document. */ @@ -2857,15 +2912,24 @@ export class ActiveDoc extends EventEmitter { } private async _getDocumentSettings(): Promise { - const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo'); - const docSettingsString = docInfo?.documentSettings; - const docSettings = docSettingsString ? safeJsonParse(docSettingsString, undefined) : undefined; + const docSettings = this.docData?.docSettings(); if (!docSettings) { throw new Error("No document settings found"); } return docSettings; } + private async _updateDocumentSettings(docSessions: OptDocSession, settings: DocumentSettings): Promise { + const docInfo = this.docData?.docInfo(); + if (!docInfo) { + throw new Error("No document info found"); + } + await this.applyUserActions(docSessions, [ + // Use docInfo.id to avoid hard-coding a reference to a specific row id, in case it changes. + ["UpdateRecord", "_grist_DocInfo", docInfo.id, { documentSettings: JSON.stringify(settings) }] + ]); + } + private async _makeEngine(): Promise { // Figure out what kind of engine we need for this document. let preferredPythonVersion: '2' | '3' = process.env.PYTHON_VERSION === '2' ? '2' : '3'; diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index f406ee12bb..4f0bcf320b 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -1,3 +1,4 @@ +import {DocAttachmentsLocation} from 'app/common/UserAPI'; import { AttachmentStoreDocInfo, DocPoolId, @@ -12,11 +13,6 @@ import {LogMethods} from 'app/server/lib/LogMethods'; import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream'; import {Readable} from 'node:stream'; -export interface IAttachmentFileManager { - addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; - getFileData(fileIdent: string): Promise; -} - export interface AddFileResult { fileIdent: string; isNewFile: boolean; @@ -47,25 +43,20 @@ export class MissingAttachmentError extends Error { } export class AttachmentRetrievalError extends Error { - public readonly storeId: AttachmentStoreId; + public readonly storeId: AttachmentStoreId | null; public readonly fileId: string; - constructor(storeId: AttachmentStoreId, fileId: string, cause?: any) { + constructor(storeId: AttachmentStoreId | null, fileId: string, cause?: any) { const causeError = cause instanceof Error ? cause : undefined; - const causeDescriptor = causeError ? `: ${cause.message}` : ''; - super(`Unable to retrieve '${fileId}' from '${storeId}'${causeDescriptor}`); + const reason = (causeError ? causeError.message : cause) ?? ""; + const storeName = storeId ? `'${storeId}'` : "internal storage"; + super(`Unable to retrieve '${fileId}' from ${storeName} ${reason}`); this.storeId = storeId; this.fileId = fileId; this.cause = causeError; } } - -interface AttachmentFileManagerLogInfo { - fileIdent?: string; - storeId?: string | null; -} - /** * Instantiated on a per-document basis to provide a document with access to its attachments. * Handles attachment uploading / fetching, as well as trying to ensure consistency with the local @@ -73,8 +64,19 @@ interface AttachmentFileManagerLogInfo { * * This class should prevent the document code from having to worry about accessing the underlying * stores. + * + * Before modifying this class, it's suggested to understand document pools (described in + * AttachmentStore.ts), which are used to perform the (eventual) cleanup of the files in external + * stores. + * + * The general design philosophy for this class is: + * - Avoid data loss at all costs (missing files in stores, or missing file table entries) + * - Always be in a valid state if possible (e.g no file entries with missing attachments) + * - Files in stores with no file record pointing to them is acceptable (but not preferable), as + * they'll eventually be cleaned up when the document pool is deleted. + * */ -export class AttachmentFileManager implements IAttachmentFileManager { +export class AttachmentFileManager { // _docPoolId is a critical point for security. Documents with a common pool id can access each others' attachments. private readonly _docPoolId: DocPoolId | null; private readonly _docName: string; @@ -83,6 +85,11 @@ export class AttachmentFileManager implements IAttachmentFileManager { (logInfo: AttachmentFileManagerLogInfo) => this._getLogMeta(logInfo) ); + // Maps file identifiers to their desired store. This may be the same as their current store, + // in which case nothing will happen. Map ensures new requests override older pending transfers. + private _pendingFileTransfers: Map = new Map(); + private _transferJob?: TransferJob; + /** * @param _docStorage - Storage of this manager's document. * @param _storeProvider - Allows instantiating of stores. Should be provided except in test @@ -99,6 +106,9 @@ export class AttachmentFileManager implements IAttachmentFileManager { this._docPoolId = _docInfo ? getDocPoolIdFromDocInfo(_docInfo) : null; } + // This attempts to add the attachment to the given store. + // If the file already exists in another store, it doesn't take any action. + // Therefore, there isn't a guarantee that the file exists in the given store, even if this method doesn't error. public async addFile( storeId: AttachmentStoreId | undefined, fileExtension: string, @@ -108,24 +118,254 @@ export class AttachmentFileManager implements IAttachmentFileManager { return this._addFile(storeId, fileIdent, fileData); } - public async _addFile( - storeId: AttachmentStoreId | undefined, + + public async getFileData(fileIdent: string): Promise { + return (await this._getFile(fileIdent)).data; + } + + public async locationSummary(): Promise { + const files = await this._docStorage.listAllFiles(); + if (files.length == 0) { + return "none"; + } + const hasInternal = files.some(file => !file.storageId); + const hasExternal = files.some(file => file.storageId); + if (hasInternal && hasExternal) { + return "mixed"; + } + if (hasExternal) { + return "external"; + } + return "internal"; + } + + public async startTransferringAllFilesToOtherStore(newStoreId: AttachmentStoreId | undefined): Promise { + // Take a "snapshot" of the files we want to transfer, and schedule those files for transfer. + // It's possibly that other code will modify the file statuses / list during this process. + // As a consequence, after this process completes, some files may still be in their original + // store. Simple approaches to solve this (e.g transferring files until everything in the DB + // shows as being in the new store) risk livelock issues, such as if two transfers somehow end + // up running simultaneously. This "snapshot" approach has few guarantees about final state, + // but is extremely unlikely to result in any severe problems. + const allFiles = await this._docStorage.listAllFiles(); + const filesToTransfer = allFiles.filter(file => (file.storageId ?? undefined) !== newStoreId); + + if (filesToTransfer.length === 0) { + return; + } + + const fileIdents = filesToTransfer.map(file => file.ident); + for (const fileIdent of fileIdents) { + this.startTransferringFileToOtherStore(fileIdent, newStoreId); + } + } + + // File transfers are handled by an async job that goes through all pending files, and one-by-one + // transfers them from their current store to their target store. This ensures that for a given + // doc, we never accidentally start several transfers at once and load many files into memory + // simultaneously (e.g. a badly written script spamming API calls). It allows any new transfers + // to overwrite any scheduled transfers. This provides a well-defined behaviour where the latest + // scheduled transfer happens, instead of the last transfer to finish "winning". + public startTransferringFileToOtherStore(fileIdent: string, newStoreId: AttachmentStoreId | undefined) { + this._pendingFileTransfers.set(fileIdent, newStoreId); + this._runTransferJob(); + } + + // Generally avoid calling this directly, instead use other methods to schedule and run the + // transfer job. If a file with a matching identifier already exists in the new store, no + // transfer will happen, as the default _addFileToX behaviour is to avoid re-uploading files. + public async transferFileToOtherStore(fileIdent: string, newStoreId: AttachmentStoreId | undefined): Promise { + this._log.info({fileIdent, storeId: newStoreId}, `transferring file to new store`); + const fileMetadata = await this._docStorage.getFileInfoNoData(fileIdent); + // This check runs before the file is retrieved as an optimisation to avoid loading files into + // memory unnecessarily. + if (!fileMetadata || (fileMetadata.storageId ?? undefined) === newStoreId) { + return; + } + // It's possible that the record has changed between the original metadata check and here. + // However, the worst case is we transfer a file that's already been transferred, so no need to + // re-check. + // Streaming isn't an option here, as SQLite only supports buffers (meaning we need to keep at + // least 1 full copy of the file in memory during transfers). + const file = await this._getFile(fileIdent); + if (!await validateFileChecksum(fileIdent, file.data)) { + throw new AttachmentRetrievalError(file.storageId, file.ident, "checksum verification failed for retrieved file"); + } + if (!newStoreId) { + await this._storeFileInLocalStorage(fileIdent, file.data); + return; + } + const newStore = await this._getStore(newStoreId); + if (!newStore) { + this._log.warn({ + fileIdent, + storeId: newStoreId + }, `unable to transfer file to unavailable store`); + throw new StoreNotAvailableError(newStoreId); + } + // Store should error if the upload fails in any way. + await this._storeFileInAttachmentStore(newStore, fileIdent, file.data); + + // Don't remove the file from the previous store, in case we need to roll back to an earlier + // snapshot. + // Internal storage is the exception (and is automatically erased), as that's included in + // snapshots. + } + + public async allTransfersCompleted(): Promise { + if (this._transferJob) { + await this._transferJob; + } + } + + public transferStatus() { + return { + pendingTransferCount: this._pendingFileTransfers.size, + isRunning: this._transferJob !== undefined + }; + } + + private _runTransferJob() { + if (this._transferJob) { + return; + } + this._transferJob = this._performPendingTransfers(); + + this._transferJob.catch((err) => this._log.error({}, `Error during transfer: ${err}`)); + + this._transferJob.finally(() => { + this._transferJob = undefined; + }); + } + + private async _performPendingTransfers() { + try { + while (this._pendingFileTransfers.size > 0) { + // Map.entries() will always return the most recent key/value from the map, even after a long async delay + // Meaning we can safely iterate here and know the transfer is up to date. + for (const [fileIdent, targetStoreId] of this._pendingFileTransfers.entries()) { + try { + await this.transferFileToOtherStore(fileIdent, targetStoreId); + } catch (e) { + this._log.warn({fileIdent, storeId: targetStoreId}, `transfer failed: ${e.message}`); + } finally { + // If a transfer request comes in mid-transfer, it will need re-running. + if (this._pendingFileTransfers.get(fileIdent) === targetStoreId) { + this._pendingFileTransfers.delete(fileIdent); + } + } + } + } + } finally { + await this._docStorage.requestVacuum(); + } + } + + private async _addFileToLocalStorage( fileIdent: string, fileData: Buffer ): Promise { - this._log.info({ fileIdent, storeId }, `adding file to ${storeId ? "external" : "document"} storage`); - if (storeId === undefined) { - return this._addFileToLocalStorage(fileIdent, fileData); + this._log.info({ + fileIdent, + }, `adding file to document storage`); + + const fileInfoNoData = await this._docStorage.getFileInfoNoData(fileIdent); + const fileExists = fileInfoNoData !== null; + + if (fileExists) { + const isFileInLocalStorage = fileInfoNoData.storageId === null; + // File is already stored in a different store, the 'add file' operation shouldn't change that. + // One way this could happen, is if the store has changed and no migration has happened. + if (!isFileInLocalStorage) { + return { + fileIdent, + isNewFile: false, + }; + } } - const store = await this._getStore(storeId); - if (!store) { - this._log.info({ fileIdent, storeId }, "tried to fetch attachment from an unavailable store"); - throw new StoreNotAvailableError(storeId); + + // A race condition can occur here, if the file's database record is modified between the + // `getFileInfoNoData` call earlier in this function and now. + // Any changes made after that point will be overwritten below. + // However, the database will always end up referencing a valid file, and the pool-based file + // deletion guarantees any files in external storage will be cleaned up eventually. + + await this._storeFileInLocalStorage(fileIdent, fileData); + + return { + fileIdent, + isNewFile: !fileExists, + }; + } + + private async _addFileToExternalStorage( + destStoreId: AttachmentStoreId, + fileIdent: string, + fileData: Buffer + ): Promise { + this._log.info({ + fileIdent, + storeId: destStoreId + }, `adding file to external storage`); + + const destStore = await this._getStore(destStoreId); + + if (!destStore) { + this._log.warn({ fileIdent, storeId: destStoreId }, "tried to fetch attachment from an unavailable store"); + throw new StoreNotAvailableError(destStoreId); + } + + const fileInfoNoData = await this._docStorage.getFileInfoNoData(fileIdent); + const fileExists = fileInfoNoData !== null; + + if (fileExists) { + const isFileInTargetStore = destStoreId === fileInfoNoData.storageId; + // File is already stored in a different store, the 'add file' operation shouldn't change that. + // One way this could happen, is if the store has changed and no migration has happened. + if (!isFileInTargetStore) { + return { + fileIdent, + isNewFile: false, + }; + } + + // Only exit early in the file exists in the store, otherwise we should allow users to fix + // any missing files by proceeding to the normal upload logic. + const fileAlreadyExistsInStore = await destStore.exists(this._getDocPoolId(), fileIdent); + if (fileAlreadyExistsInStore) { + return { + fileIdent, + isNewFile: false, + }; + } } - return this._addFileToAttachmentStore(store, fileIdent, fileData); + + // A race condition can occur here, if the file's database record is modified between the + // `getFileInfoNoData` call earlier in this function and now. + // Any changes made after that point will be overwritten below. + // However, the database will always end up referencing a valid file, and the pool-based file + // deletion guarantees any files in external storage will be cleaned up eventually. + + await this._storeFileInAttachmentStore(destStore, fileIdent, fileData); + + return { + fileIdent, + isNewFile: !fileExists, + }; } - public async getFileData(fileIdent: string): Promise { + private async _addFile( + destStoreId: AttachmentStoreId | undefined, + fileIdent: string, + fileData: Buffer + ): Promise { + if (destStoreId === undefined) { + return this._addFileToLocalStorage(fileIdent, fileData); + } + return this._addFileToExternalStorage(destStoreId, fileIdent, fileData); + } + + private async _getFile(fileIdent: string): Promise { const fileInfo = await this._docStorage.getFileInfo(fileIdent); if (!fileInfo) { this._log.error({ fileIdent }, "cannot find file metadata in document"); @@ -136,22 +376,21 @@ export class AttachmentFileManager implements IAttachmentFileManager { `fetching attachment from ${fileInfo.storageId ? "external" : "document "} storage` ); if (!fileInfo.storageId) { - return fileInfo.data; + return { + ident: fileIdent, + storageId: null, + data: fileInfo.data, + }; } const store = await this._getStore(fileInfo.storageId); if (!store) { this._log.warn({ fileIdent, storeId: fileInfo.storageId }, `unable to retrieve file, store is unavailable`); throw new StoreNotAvailableError(fileInfo.storageId); } - return this._getFileDataFromAttachmentStore(store, fileIdent); - } - - private async _addFileToLocalStorage(fileIdent: string, fileData: Buffer): Promise { - const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, fileData); - return { - fileIdent, - isNewFile, + ident: fileIdent, + storageId: store.id, + data: await this._getFileDataFromAttachmentStore(store, fileIdent), }; } @@ -174,32 +413,31 @@ export class AttachmentFileManager implements IAttachmentFileManager { return `${checksum}${fileExtension}`; } - private async _addFileToAttachmentStore( - store: IAttachmentStore, fileIdent: string, fileData: Buffer - ): Promise { - const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, undefined, store.id); - - // Verify the file exists in the store. This allows for a second attempt to correct a failed upload. - const existsInRemoteStorage = !isNewFile && await store.exists(this._getDocPoolId(), fileIdent); + // Uploads the file to local storage, overwriting the current DB record for the file. + private async _storeFileInLocalStorage(fileIdent: string, fileData: Buffer): Promise { + // Insert (or overwrite) the entry for this file in the document database. + const isNewFile = await this._docStorage.attachOrUpdateFile(fileIdent, fileData, undefined); - if (!isNewFile && existsInRemoteStorage) { - return { - fileIdent, - isNewFile: false, - }; - } - - // Possible issue if this upload fails - we have the file tracked in the document, but not available in the store. - // TODO - Decide if we keep an entry in SQLite after an upload error or not. Probably not? - await store.upload(this._getDocPoolId(), fileIdent, Readable.from(fileData)); - - // TODO - Confirm in doc storage that it's successfully uploaded? Need to decide how to handle a failed upload. return { fileIdent, isNewFile, }; } + // Uploads the file to an attachment store, overwriting the current DB record for the file if successful. + private async _storeFileInAttachmentStore( + store: IAttachmentStore, fileIdent: string, fileData: Buffer + ): Promise { + // The underlying store should guarantee the file exists if this method doesn't error, + // so no extra validation is needed here. + await store.upload(this._getDocPoolId(), fileIdent, Readable.from(fileData)); + + // Insert (or overwrite) the entry for this file in the document database. + await this._docStorage.attachOrUpdateFile(fileIdent, undefined, store.id); + + return fileIdent; + } + private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { try { const outputStream = new MemoryWritableStream(); @@ -218,3 +456,20 @@ export class AttachmentFileManager implements IAttachmentFileManager { }; } } + +async function validateFileChecksum(fileIdent: string, fileData: Buffer): Promise { + return fileIdent.startsWith(await checksumFileStream(Readable.from(fileData))); +} + +interface AttachmentFileManagerLogInfo { + fileIdent?: string; + storeId?: string | null; +} + +interface AttachmentFileInfo { + ident: string; + storageId: string | null; + data: Buffer; +} + +type TransferJob = Promise diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 7b90625a05..e0c14f5801 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -76,6 +76,9 @@ export interface IAttachmentStore { // implementation and gives them control over local buffering. download(docPoolId: DocPoolId, fileId: FileId, outputStream: stream.Writable): Promise; + // Remove attachment from the store + delete(docPoolId: DocPoolId, fileId: FileId): Promise; + // Remove attachments for all documents in the given document pool. removePool(docPoolId: DocPoolId): Promise; @@ -120,6 +123,10 @@ export class ExternalStorageAttachmentStore implements IAttachmentStore { await this._storage.downloadStream(this._getKey(docPoolId, fileId), outputStream); } + public async delete(docPoolId: string, fileId: string): Promise { + await this._storage.remove(this._getKey(docPoolId, fileId)); + } + public async removePool(docPoolId: string): Promise { // Null assertion is safe because this should be checked before this class is instantiated. await this._storage.removeAllWithPrefix!(this._getPoolPrefix(docPoolId)); @@ -164,6 +171,10 @@ export class FilesystemAttachmentStore implements IAttachmentStore { ); } + public async delete(docPoolId: string, fileId: string): Promise { + await fse.remove(this._createPath(docPoolId, fileId)); + } + public async removePool(docPoolId: DocPoolId): Promise { await fse.remove(this._createPath(docPoolId)); } diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 1ae1d0f678..625d501eba 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,6 +1,10 @@ -import {IAttachmentStore} from 'app/server/lib/AttachmentStore'; +import {appSettings} from 'app/server/lib/AppSettings'; +import {FilesystemAttachmentStore, IAttachmentStore} from 'app/server/lib/AttachmentStore'; import log from 'app/server/lib/log'; import {ICreateAttachmentStoreOptions} from './ICreate'; +import * as fse from 'fs-extra'; +import path from 'path'; +import * as tmp from 'tmp-promise'; export type AttachmentStoreId = string @@ -15,8 +19,10 @@ export type AttachmentStoreId = string * to store/retrieve them as long as that store exists on the document's installation. */ export interface IAttachmentStoreProvider { + getStoreIdFromLabel(label: string): string; + // Returns the store associated with the given id, returning null if no store with that id exists. - getStore(id: AttachmentStoreId): Promise + getStore(id: AttachmentStoreId): Promise; getAllStores(): Promise; @@ -25,56 +31,76 @@ export interface IAttachmentStoreProvider { listAllStoreIds(): AttachmentStoreId[]; } +// All the information needed to instantiate an instance of a particular store type export interface IAttachmentStoreSpecification { name: string, create: (storeId: string) => Promise, } -interface IAttachmentStoreDetails { - id: string; +// All the information needed to create a particular store instance +export interface IAttachmentStoreConfig { + // This is the name for the store, but it also used to construct the store ID. + label: string; spec: IAttachmentStoreSpecification; } +/** + * Provides access to instances of attachment stores. + * + * Stores can be accessed using ID or Label. + * + * Labels are a convenient/user-friendly way to refer to stores. + * Each label is unique within a Grist instance, but other instances may have stores that use + * the same label. + * E.g "my-s3" or "myFolder". + * + * IDs are globally unique - and are created by prepending the label with the installation UUID. + * E.g "22ec6867-67bc-414e-a707-da9204c84cab-my-s3" or "22ec6867-67bc-414e-a707-da9204c84cab-myFolder" + */ export class AttachmentStoreProvider implements IAttachmentStoreProvider { - private _storeDetailsById: { [storeId: string]: IAttachmentStoreDetails } = {}; + private _storeDetailsById: Map = new Map(); constructor( - _backends: IAttachmentStoreSpecification[], - _installationUuid: string + storeConfigs: IAttachmentStoreConfig[], + private _installationUuid: string ) { - // In the current setup, we automatically generate store IDs based on the installation ID. - // The installation ID is guaranteed to be unique, and we only allow one store of each backend type. - // This gives us a way to reproducibly generate a unique ID for the stores. - _backends.forEach((storeSpec) => { - const storeId = `${_installationUuid}-${storeSpec.name}`; - this._storeDetailsById[storeId] = { - id: storeId, - spec: storeSpec, - }; + // It's convenient to have stores with a globally unique ID, so there aren't conflicts as + // documents are moved between installations. This is achieved by prepending the store labels + // with the installation ID. Enforcing that using AttachmentStoreProvider makes it + // much harder to accidentally bypass this constraint, as the provider should be the only way of + // accessing stores. + storeConfigs.forEach((storeConfig) => { + this._storeDetailsById.set(this.getStoreIdFromLabel(storeConfig.label), storeConfig); }); - const storeIds = Object.values(this._storeDetailsById).map(storeDetails => storeDetails.id); + const storeIds = Array.from(this._storeDetailsById.keys()); log.info(`AttachmentStoreProvider initialised with stores: ${storeIds}`); } + public getStoreIdFromLabel(label: string): string { + return `${this._installationUuid}-${label}`; + } + public async getStore(id: AttachmentStoreId): Promise { - const storeDetails = this._storeDetailsById[id]; + const storeDetails = this._storeDetailsById.get(id); if (!storeDetails) { return null; } return storeDetails.spec.create(id); } public async getAllStores(): Promise { return await Promise.all( - Object.values(this._storeDetailsById).map(storeDetails => storeDetails.spec.create(storeDetails.id)) + Array.from(this._storeDetailsById.entries()).map( + ([storeId, storeConfig]) => storeConfig.spec.create(storeId) + ) ); } public async storeExists(id: AttachmentStoreId): Promise { - return id in this._storeDetailsById; + return this._storeDetailsById.has(id); } public listAllStoreIds(): string[] { - return Object.keys(this._storeDetailsById); + return Array.from(this._storeDetailsById.keys()); } } @@ -95,3 +121,46 @@ export async function checkAvailabilityAttachmentStoreOptions(options: ICreateAt unavailable: options.filter((option, index) => !availability[index]), }; } + +// Make a filesystem store that will be cleaned up on process exit. +// This is only used when external attachments are in 'test' mode, which is used for some unit tests. +// TODO: Remove this when setting up a filesystem store is possible using normal configuration options +export async function makeTempFilesystemStoreSpec( + name: string = "filesystem" +) { + const tempFolder = await tmp.dir(); + const tempDir = await fse.mkdtemp(path.join(tempFolder.path, 'filesystem-store-test-')); + return { + rootDirectory: tempDir, + name, + create: async (storeId: string) => (new FilesystemAttachmentStore(storeId, tempDir)) + }; +} + +const settings = appSettings.section("attachmentStores"); +const ATTACHMENT_STORE_MODE = settings.flag("mode").readString({ + envVar: "GRIST_EXTERNAL_ATTACHMENTS_MODE", + defaultValue: "none", +}); + +export function getConfiguredStandardAttachmentStore(): string | undefined { + switch (ATTACHMENT_STORE_MODE) { + case "test": + return 'test-filesystem'; + default: + return undefined; + } +} + +export async function getConfiguredAttachmentStoreConfigs(): Promise { + switch (ATTACHMENT_STORE_MODE) { + // This mode should be removed once stores can be configured fully via env vars. + case "test": + return [{ + label: 'test-filesystem', + spec: await makeTempFilesystemStoreSpec(), + }]; + default: + return []; + } +} diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 28652552d9..1c67f16bc7 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -47,7 +47,10 @@ import {ActiveDoc, colIdToRef as colIdToReference, getRealTableId, tableIdToRef} import {appSettings} from "app/server/lib/AppSettings"; import {sendForCompletion} from 'app/server/lib/Assistance'; import {getDocPoolIdFromDocInfo} from 'app/server/lib/AttachmentStore'; -import {IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; +import { + getConfiguredStandardAttachmentStore, + IAttachmentStoreProvider +} from 'app/server/lib/AttachmentStoreProvider'; import { assertAccess, getAuthorizedUserId, @@ -137,6 +140,7 @@ const { ColumnsPost, ColumnsPatch, ColumnsPut, SqlPost, TablesPost, TablesPatch, + SetAttachmentStorePost, } = t.createCheckers(DocApiTypesTI, GristDataTI); for (const checker of [RecordsPatch, RecordsPost, RecordsPut, ColumnsPost, ColumnsPatch, @@ -517,6 +521,58 @@ export class DocWorkerApi { res.json({records}); })); + // Starts transferring all attachments to the named store, if it exists. + this._app.post('/api/docs/:docId/attachments/transferAll', isOwner, withDoc(async (activeDoc, req, res) => { + await activeDoc.startTransferringAllAttachmentsToDefaultStore(); + const locationSummary = await activeDoc.attachmentLocationSummary(); + + // Respond with the current status to allow for immediate UI updates. + res.json({ + status: activeDoc.attachmentTransferStatus(), + locationSummary, + }); + })); + + // Returns the status of any current / pending attachment transfers + this._app.get('/api/docs/:docId/attachments/transferStatus', isOwner, withDoc(async (activeDoc, req, res) => { + const locationSummary = await activeDoc.attachmentLocationSummary(); + res.json({ + status: activeDoc.attachmentTransferStatus(), + locationSummary, + }); + })); + + this._app.get('/api/docs/:docId/attachments/store', isOwner, + withDoc(async (activeDoc, req, res) => { + const storeId = await activeDoc.getAttachmentStore(); + res.json({ + type: storeId ? 'external' : 'internal', + }); + }) + ); + + this._app.post('/api/docs/:docId/attachments/store', isOwner, validate(SetAttachmentStorePost), + withDoc(async (activeDoc, req, res) => { + const body = req.body as Types.SetAttachmentStorePost; + if (body.type === 'internal') { + await activeDoc.setAttachmentStoreFromLabel(docSessionFromRequest(req), undefined); + } + + if (body.type === 'external') { + const storeLabel = getConfiguredStandardAttachmentStore(); + if (storeLabel === undefined) { + throw new ApiError("server is not configured with an external store", 400); + } + // This store might not exist - that's acceptable, and should be handled elsewhere. + await activeDoc.setAttachmentStoreFromLabel(docSessionFromRequest(req), storeLabel); + } + + res.json({ + store: await activeDoc.getAttachmentStore() + }); + }) + ); + // Returns cleaned metadata for a given attachment ID (i.e. a rowId in _grist_Attachments table). this._app.get('/api/docs/:docId/attachments/:attId', canView, withDoc(async (activeDoc, req, res) => { const attId = integerParam(req.params.attId, 'attId'); @@ -898,7 +954,8 @@ export class DocWorkerApi { ); /** - @deprecated please call to POST /webhooks instead, this endpoint is only for sake of backward compatibility + @deprecated please call to POST /webhooks instead, this endpoint is only for sake of backward + compatibility */ this._app.post('/api/docs/:docId/tables/:tableId/_subscribe', isOwner, validate(WebhookSubscribe), withDocTriggersLock(async (activeDoc, req, res) => { @@ -923,7 +980,8 @@ export class DocWorkerApi { ); /** - @deprecated please call to DEL /webhooks instead, this endpoint is only for sake of backward compatibility + @deprecated please call to DEL /webhooks instead, this endpoint is only for sake of backward + compatibility */ this._app.post('/api/docs/:docId/tables/:tableId/_unsubscribe', canEdit, withDocTriggersLock(removeWebhook) @@ -1887,7 +1945,8 @@ export class DocWorkerApi { } /** - * Creates a middleware that checks the current usage of a limit and rejects the request if it is exceeded. + * Creates a middleware that checks the current usage of a limit and rejects the request if it is + * exceeded. */ private async _checkLimit(limit: LimitType, req: Request, res: Response, next: NextFunction) { await this._dbManager.increaseUsage(getDocScope(req), limit, {dryRun: true, delta: 1}); @@ -1923,8 +1982,8 @@ export class DocWorkerApi { /** * Check if user is an owner of the document. - * If acceptTrunkForSnapshot is set, being an owner of the trunk of the document (if it is a snapshot) - * is sufficient. Uses cachedDoc, which could be stale if access has changed recently. + * If acceptTrunkForSnapshot is set, being an owner of the trunk of the document (if it is a + * snapshot) is sufficient. Uses cachedDoc, which could be stale if access has changed recently. */ private async _isOwner(req: Request, options?: { acceptTrunkForSnapshot?: boolean }) { const scope = getDocScope(req); @@ -2597,12 +2656,15 @@ export function docPeriodicApiUsageKey(docId: string, current: boolean, period: * Maintain up to 5 buckets: current day, next day, current hour, next hour, current minute. * For each API request, check in order: * - if current_day < DAILY_LIMIT, allow; increment all 3 current buckets - * - else if current_hour < DAILY_LIMIT/24, allow; increment next_day, current_hour, and current_minute buckets. - * - else if current_minute < DAILY_LIMIT/24/60, allow; increment next_day, next_hour, and current_minute buckets. + * - else if current_hour < DAILY_LIMIT/24, allow; increment next_day, current_hour, and + * current_minute buckets. + * - else if current_minute < DAILY_LIMIT/24/60, allow; increment next_day, next_hour, and + * current_minute buckets. * - else reject. * I think it has pretty good properties: * - steady low usage may be maintained even if a burst exhausted the daily limit - * - user could get close to twice the daily limit on the first day with steady usage after a burst, + * - user could get close to twice the daily limit on the first day with steady usage after a + * burst, * but would then be limited to steady usage the next day. */ export function getDocApiUsageKeysToIncr( diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 537862dd64..2cf468c846 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -32,7 +32,6 @@ import cloneDeep = require('lodash/cloneDeep'); import groupBy = require('lodash/groupBy'); import { MinDBOptions } from './SqliteCommon'; - // Run with environment variable NODE_DEBUG=db (may include additional comma-separated sections) // for verbose logging. const debuglog = util.debuglog('db'); @@ -776,48 +775,100 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { * would be (very?) inefficient until node-sqlite3 adds support for incremental reading from a * blob: https://github.com/mapbox/node-sqlite3/issues/424. * - * @param {string} fileIdent - The unique identifier of the file in the database. ActiveDoc uses the - * checksum of the file's contents with the original extension. + * @param {string} fileIdent - The unique identifier of the file in the database. * @param {Buffer | undefined} fileData - Contents of the file. * @param {string | undefined} storageId - Identifier of the store that file is stored in. * @returns {Promise[Boolean]} True if the file got attached; false if this ident already exists. */ - public findOrAttachFile( + public attachFileIfNew( fileIdent: string, fileData: Buffer | undefined, storageId?: string, ): Promise { - return this.execTransaction(db => { - // Try to insert a new record with the given ident. It'll fail UNIQUE constraint if exists. - return db.run('INSERT INTO _gristsys_Files (ident) VALUES (?)', fileIdent) - // Only if this succeeded, do the work of reading the file and inserting its data. - .then(() => - db.run('UPDATE _gristsys_Files SET data=?, storageId=? WHERE ident=?', fileData, storageId, fileIdent)) - .then(() => true) - // If UNIQUE constraint failed, this ident must already exists, so return false. - .catch(err => { - if (/^(SQLITE_CONSTRAINT: )?UNIQUE constraint failed/.test(err.message)) { - return false; - } - throw err; - }); + return this.execTransaction(async (db) => { + const isNewFile = await this._addBasicFileRecord(db, fileIdent); + if (isNewFile) { + await this._updateFileRecord(db, fileIdent, fileData, storageId); + } + return isNewFile; + }); + } + + /** + * Attaches a file to the document, updating the file record if it already exists. + * + * TODO: This currently does not make the attachment available to the sandbox code. This is likely + * to be needed in the future, and a suitable API will need to be provided. Note that large blobs + * would be (very?) inefficient until node-sqlite3 adds support for incremental reading from a + * blob: https://github.com/mapbox/node-sqlite3/issues/424. + * + * @param {string} fileIdent - The unique identifier of the file in the database. + * @param {Buffer | undefined} fileData - Contents of the file. + * @param {string | undefined} storageId - Identifier of the store that file is stored in. + * @returns {Promise[Boolean]} True if the file got attached; false if this ident already exists. + */ + public attachOrUpdateFile( + fileIdent: string, + fileData: Buffer | undefined, + storageId?: string, + ): Promise { + return this.execTransaction(async (db) => { + const isNewFile = await this._addBasicFileRecord(db, fileIdent); + await this._updateFileRecord(db, fileIdent, fileData, storageId); + return isNewFile; }); } /** * Reads and returns the data for the given attachment. - * @param {string} fileIdent - The unique identifier of a file, as used by findOrAttachFile. - * @returns {Promise[Buffer]} The data buffer associated with fileIdent. + * @param {string} fileIdent - The unique identifier of a file, as used by attachFileIfNew. + * file identifier. + * @returns {Promise[FileInfo | null]} - File information, or null if no record exists for that file identifier. */ - public getFileInfo(fileIdent: string): Promise { - return this.get('SELECT ident, storageId, data FROM _gristsys_Files WHERE ident=?', fileIdent) - .then(row => row ? ({ + public async getFileInfo(fileIdent: string): Promise { + const row = await this.get(`SELECT ident, storageId, data FROM _gristsys_Files WHERE ident=?`, fileIdent); + if(!row) { + return null; + } + + return { + ident: row.ident as string, + storageId: (row.storageId ?? null) as (string | null), + // Use a zero buffer for now if it doesn't exist. Should be refactored to allow null. + data: row.data ? row.data as Buffer : Buffer.alloc(0), + }; + } + + /** + * Reads and returns the metadata for a file, without retrieving the file's contents. + * @param {string} fileIdent - The unique identifier of a file, as used by attachFileIfNew. + * @returns {Promise[FileInfo | null]} - File information, or null if no record exists for that + * file identifier. + */ + public async getFileInfoNoData(fileIdent: string): Promise { + const row = await this.get(`SELECT ident, storageId FROM _gristsys_Files WHERE ident=?`, fileIdent); + if (!row) { + return null; + } + + return { ident: row.ident as string, storageId: (row.storageId ?? null) as (string | null), - data: row.data as Buffer, - }) : null); + // Use a zero buffer for now if it doesn't exist. Should be refactored to allow null. + data: Buffer.alloc(0), + }; } + public async listAllFiles(): Promise { + const rows = await this.all(`SELECT ident, storageId FROM _gristsys_Files`); + + return rows.map(row => ({ + ident: row.ident as string, + storageId: (row.storageId ?? null) as (string | null), + // Use a zero buffer for now to represent no data. Should be refactored to allow null. + data: Buffer.alloc(0), + })); + } /** * Fetches the given table from the database. See fetchQuery() for return value. @@ -1838,6 +1889,32 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { } return null; } + + private async _addBasicFileRecord(db: SQLiteDB, fileIdent: string): Promise { + // Try to insert a new record with the given ident. It'll fail the UNIQUE constraint if exists. + // Catching the violation is the simplest and most reliable way of doing this. + // If this function runs multiple times in parallel (which can happen), nothing guarantees the + // order that multiple `db.run` or `db.get` statements will run in (not even .execTransaction). + // This means it's not safe to check then insert - we just have to try the insert and see if it + // fails. + try { + await db.run('INSERT INTO _gristsys_Files (ident) VALUES (?)', fileIdent); + } catch(err) { + // If UNIQUE constraint failed, this ident must already exist. + if (/^(SQLITE_CONSTRAINT: )?UNIQUE constraint failed/.test(err.message)) { + return false; + } else { + throw err; + } + } + return true; + } + + private async _updateFileRecord( + db: SQLiteDB, fileIdent: string, fileData?: Buffer, storageId?: string + ): Promise { + await db.run('UPDATE _gristsys_Files SET data=?, storageId=? WHERE ident=?', fileData, storageId, fileIdent); + } } interface RebuildResult { diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 2263cd9ebf..c939563305 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -28,7 +28,10 @@ import {attachAppEndpoint} from 'app/server/lib/AppEndpoint'; import {appSettings} from 'app/server/lib/AppSettings'; import {attachEarlyEndpoints} from 'app/server/lib/attachEarlyEndpoints'; import { - AttachmentStoreProvider, checkAvailabilityAttachmentStoreOptions, IAttachmentStoreProvider + AttachmentStoreProvider, + checkAvailabilityAttachmentStoreOptions, + getConfiguredAttachmentStoreConfigs, + IAttachmentStoreProvider } from 'app/server/lib/AttachmentStoreProvider'; import {addRequestUser, getTransitiveHeaders, getUser, getUserId, isAnonymousUser, isSingleUserMode, redirectToLoginUnconditionally} from 'app/server/lib/Authorizer'; @@ -1396,7 +1399,7 @@ export class FlexServer implements GristServer { }); this._attachmentStoreProvider = this._attachmentStoreProvider || new AttachmentStoreProvider( - storeOptions.available, + await getConfiguredAttachmentStoreConfigs(), (await this.getActivations().current()).id, ); this._docManager = this._docManager || new DocManager(this._storageManager, diff --git a/app/server/utils/gristify.ts b/app/server/utils/gristify.ts index 57a0c68fd1..05bfd52d42 100644 --- a/app/server/utils/gristify.ts +++ b/app/server/utils/gristify.ts @@ -53,7 +53,8 @@ export class Gristifier { // Open the file as an empty Grist document, creating Grist metadata // tables. const docManager = new DocManager( - new TrivialDocStorageManager(), null, null, new AttachmentStoreProvider([], ""), createDummyGristServer() + new TrivialDocStorageManager(), null, null, + new AttachmentStoreProvider([], ""), createDummyGristServer() ); const activeDoc = new ActiveDoc(docManager, this._filename); const docSession = makeExceptionalDocSession('system'); diff --git a/test/server/docTools.ts b/test/server/docTools.ts index 20576f1471..be2f07a140 100644 --- a/test/server/docTools.ts +++ b/test/server/docTools.ts @@ -44,7 +44,9 @@ const noCleanup = Boolean(process.env.NO_CLEANUP); export function createDocTools(options: {persistAcrossCases?: boolean, useFixturePlugins?: boolean, storageManager?: IDocStorageManager, - server?: () => GristServer} = {}) { + server?: () => GristServer, + createAttachmentStoreProvider?: () => Promise + } = {}) { let tmpDir: string; let docManager: DocManager; @@ -52,7 +54,8 @@ export function createDocTools(options: {persistAcrossCases?: boolean, tmpDir = await createTmpDir(); const pluginManager = options.useFixturePlugins ? await createFixturePluginManager() : undefined; docManager = await createDocManager({tmpDir, pluginManager, storageManager: options.storageManager, - server: options.server?.()}); + server: options.server?.(), + createAttachmentStoreProvider: options.createAttachmentStoreProvider}); } async function doAfter() { @@ -137,13 +140,15 @@ export async function createDocManager( options: {tmpDir?: string, pluginManager?: PluginManager, storageManager?: IDocStorageManager, server?: GristServer, - attachmentStoreProvider?: IAttachmentStoreProvider, + createAttachmentStoreProvider?: () => Promise, } = {}): Promise { // Set Grist home to a temporary directory, and wipe it out on exit. const tmpDir = options.tmpDir || await createTmpDir(); const docStorageManager = options.storageManager || await create.createLocalDocStorageManager(tmpDir); const pluginManager = options.pluginManager || await getGlobalPluginManager(); - const attachmentStoreProvider = options.attachmentStoreProvider || new AttachmentStoreProvider([], "TEST_INSTALL"); + const attachmentStoreProvider = options.createAttachmentStoreProvider + ? (await options.createAttachmentStoreProvider()) + : new AttachmentStoreProvider([], "TEST_INSTALL"); const store = getDocWorkerMap(); const internalPermitStore = store.getPermitStore('1'); const externalPermitStore = store.getPermitStore('2'); diff --git a/test/server/lib/ActiveDoc.ts b/test/server/lib/ActiveDoc.ts index 8babb27b96..006882c809 100644 --- a/test/server/lib/ActiveDoc.ts +++ b/test/server/lib/ActiveDoc.ts @@ -10,19 +10,22 @@ import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; import {DummyAuthorizer} from 'app/server/lib/Authorizer'; import {Client} from 'app/server/lib/Client'; import {makeExceptionalDocSession, OptDocSession} from 'app/server/lib/DocSession'; +//import {guessExt} from 'app/server/lib/guessExt'; import log from 'app/server/lib/log'; import {timeoutReached} from 'app/server/lib/serverUtils'; import {Throttle} from 'app/server/lib/Throttle'; +//import {createTmpDir as createTmpUploadDir, FileUploadInfo, globalUploadSet} from 'app/server/lib/uploads'; import {promisify} from 'bluebird'; import {assert} from 'chai'; import * as child_process from 'child_process'; import * as fse from 'fs-extra'; import * as _ from 'lodash'; -import {resolve} from 'path'; +import /*path,*/ {resolve} from 'path'; import * as sinon from 'sinon'; import {createDocTools} from 'test/server/docTools'; import * as testUtils from 'test/server/testUtils'; import {EnvironmentSnapshot} from 'test/server/testUtils'; +import {makeTestingFilesystemStoreConfig} from 'test/server/lib/FilesystemAttachmentStore'; import * as tmp from 'tmp'; const execFileAsync = promisify(child_process.execFile); @@ -31,13 +34,18 @@ const UNSUPPORTED_FORMULA: CellValue = [GristObjCode.Exception, 'Formula not sup tmp.setGracefulCleanup(); -describe('ActiveDoc', function() { +describe('ActiveDoc', async function() { this.timeout(10000); // Turn off logging for this test, and restore afterwards. testUtils.setTmpLogLevel('warn'); - const docTools = createDocTools(); + const createAttachmentStoreProvider = async () => new AttachmentStoreProvider( + [await makeTestingFilesystemStoreConfig("filesystem")], + "TEST-INSTALLATION-UUID" + ); + + const docTools = createDocTools({ createAttachmentStoreProvider }); const fakeSession = makeExceptionalDocSession('system'); @@ -1145,6 +1153,82 @@ describe('ActiveDoc', function() { await activeDoc.shutdown(); } }); + + /* + describe('attachment transfers', async function() { + // Provides the fake userId `null`, so we can access uploaded files with hitting an + // authorization errors. + const fakeTransferSession = docTools.createFakeSession(); + + async function uploadAttachments(doc: ActiveDoc, files: {name: string, contents: string}[]) { + const { tmpDir, cleanupCallback } = await createTmpUploadDir({}); + const uploadedFiles: FileUploadInfo[] = []; + + const uploadIdPromises = files.map(async (file) => { + const filePath = resolve(tmpDir, file.name); + const buffer = Buffer.from(file.contents, 'utf8'); + await fse.writeFile(path.join(tmpDir, file.name), buffer); + uploadedFiles.push({ + absPath: filePath, + origName: file.name, + size: buffer.length, + ext: await guessExt(filePath, file.name, null) + }); + return globalUploadSet.registerUpload(uploadedFiles, tmpDir, cleanupCallback, null); + }); + + const uploadIds = await Promise.all(uploadIdPromises); + + for (const uploadId of uploadIds) { + await doc.addAttachments(fakeTransferSession, uploadId); + } + } + + it('can transfer attachments to a new store, with correct status reporting', async function() { + const docName = 'transfer status'; + const activeDoc1 = await docTools.createDoc(docName); + await activeDoc1.applyUserActions(fakeSession, [ + ['AddTable', 'MyAttachments', [{id: "A", type: allTypes.Attachments, isFormula: false}]], + ]); + + const initialTransferStatus = activeDoc1.attachmentTransferStatus(); + assert.isFalse(initialTransferStatus.isRunning); + assert.equal(initialTransferStatus.pendingTransferCount, 0); + + const initialAttachmentsLocation = await activeDoc1.attachmentLocationSummary(); + assert.equal(initialAttachmentsLocation, "NO FILES"); + + await uploadAttachments(activeDoc1, [{ + name: "A.txt", + contents: "Contents1", + }]); + + const postUploadAttachmentsLocation = await activeDoc1.attachmentLocationSummary(); + assert.equal(postUploadAttachmentsLocation, "INTERNAL"); + + await activeDoc1.setAttachmentStore(fakeSession, attachmentStoreProvider.listAllStoreIds()[0]); + await activeDoc1.startTransferringAllAttachmentsToDefaultStore(); + + // These assertions should always be correct, as we don't await any promises here, so there's + // no time for the async transfers to run. + const transferStartedStatus = activeDoc1.attachmentTransferStatus(); + assert.isTrue(transferStartedStatus.isRunning); + assert.isTrue(transferStartedStatus.pendingTransferCount > 0, "at least one transfer should be pending"); + + // Can't assert location here, as "INTERNAL", "MIXED" and "EXTERNAL" are all valid, depending + // on how the transfer status is going in the background. + + await activeDoc1.allAttachmentTransfersCompleted(); + + const finalTransferStatus = activeDoc1.attachmentTransferStatus(); + assert.isFalse(finalTransferStatus.isRunning); + assert.equal(finalTransferStatus.pendingTransferCount, 0); + + const finalAttachmentsLocation = await activeDoc1.attachmentLocationSummary(); + assert(finalAttachmentsLocation, "INTERNAL"); + }); + }); + */ }); async function dumpTables(path: string): Promise { diff --git a/test/server/lib/AttachmentFileManager.ts b/test/server/lib/AttachmentFileManager.ts index d7be3c5201..67e4d0fa69 100644 --- a/test/server/lib/AttachmentFileManager.ts +++ b/test/server/lib/AttachmentFileManager.ts @@ -1,16 +1,22 @@ import { DocStorage, FileInfo } from "app/server/lib/DocStorage"; import { - AttachmentFileManager, AttachmentRetrievalError, + AttachmentFileManager, + AttachmentRetrievalError, StoreNotAvailableError, StoresNotConfiguredError } from "app/server/lib/AttachmentFileManager"; +import { getDocPoolIdFromDocInfo } from "app/server/lib/AttachmentStore"; import { AttachmentStoreProvider, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; -import { makeTestingFilesystemStoreSpec } from "./FilesystemAttachmentStore"; +import { makeTestingFilesystemStoreConfig } from "test/server/lib/FilesystemAttachmentStore"; import { assert } from "chai"; import * as sinon from "sinon"; +import * as stream from "node:stream"; // Minimum features of doc storage that are needed to make AttachmentFileManager work. -type IMinimalDocStorage = Pick +type IMinimalDocStorage = Pick // Implements the minimal functionality needed for the AttachmentFileManager to work. class DocStorageFake implements IMinimalDocStorage { @@ -19,23 +25,52 @@ class DocStorageFake implements IMinimalDocStorage { constructor(public docName: string) { } + public async requestVacuum(): Promise { + return true; + } + + public async listAllFiles(): Promise { + const fileInfoPromises = Object.keys(this._files).map(key => this.getFileInfo(key)); + const fileInfo = await Promise.all(fileInfoPromises); + + const isFileInfo = (item: FileInfo | null): item is FileInfo => item !== null; + + return fileInfo.filter(isFileInfo); + } + public async getFileInfo(fileIdent: string): Promise { return this._files[fileIdent] ?? null; } - // Return value is true if the file was newly added. - public async findOrAttachFile( + public async getFileInfoNoData(fileIdent: string): Promise { + return this.getFileInfo(fileIdent); + } + + // Needs to match the semantics of DocStorage's implementation. + public async attachFileIfNew( fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined ): Promise { if (fileIdent in this._files) { return false; } + this._setFileRecord(fileIdent, fileData, storageId); + return true; + } + + public async attachOrUpdateFile( + fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined + ): Promise { + const exists = fileIdent in this._files; + this._setFileRecord(fileIdent, fileData, storageId); + return !exists; + } + + private _setFileRecord(fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined) { this._files[fileIdent] = { ident: fileIdent, data: fileData ?? Buffer.alloc(0), storageId: storageId ?? null, }; - return true; } } @@ -48,7 +83,10 @@ function createDocStorageFake(docName: string): DocStorage { async function createFakeAttachmentStoreProvider(): Promise { return new AttachmentStoreProvider( - [await makeTestingFilesystemStoreSpec("filesystem")], + [ + await makeTestingFilesystemStoreConfig("filesystem"), + await makeTestingFilesystemStoreConfig("filesystem-alt"), + ], "TEST-INSTALLATION-UUID" ); } @@ -92,7 +130,7 @@ describe("AttachmentFileManager", function() { ); const fileId = "123456.png"; - await defaultDocStorageFake.findOrAttachFile(fileId, undefined, "SOME-STORE-ID"); + await defaultDocStorageFake.attachFileIfNew(fileId, undefined, "SOME-STORE-ID"); await assert.isRejected(manager.getFileData(fileId), StoreNotAvailableError); }); @@ -126,6 +164,28 @@ describe("AttachmentFileManager", function() { assert.isTrue(await store!.exists(docId, result.fileIdent), "file does not exist in store"); }); + it("shouldn't do anything when trying to add an existing attachment to a new store", async function() { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const allStoreIds = defaultProvider.listAllStoreIds(); + const result1 = await manager.addFile(allStoreIds[0], ".txt", Buffer.from(defaultTestFileContent)); + const store1 = await defaultProvider.getStore(allStoreIds[0]); + assert.isTrue(await store1!.exists(docId, result1.fileIdent), "file does not exist in store"); + + const result2 = await manager.addFile(allStoreIds[1], ".txt", Buffer.from(defaultTestFileContent)); + const store2 = await defaultProvider.getStore(allStoreIds[1]); + // File shouldn't exist in the new store + assert.isFalse(await store2!.exists(docId, result2.fileIdent)); + + const fileInfo = await defaultDocStorageFake.getFileInfo(result2.fileIdent); + assert.equal(fileInfo?.storageId, allStoreIds[0], "file record should not refer to the new store"); + }); + it("should get a file from local storage", async function() { const docId = "12345"; const manager = new AttachmentFileManager( @@ -210,4 +270,116 @@ describe("AttachmentFileManager", function() { assert(fileData); assert.equal(fileData.toString(), defaultTestFileContent); }); + + async function testStoreTransfer(sourceStore?: string, destStore?: string) { + const docInfo = { id: "12345", trunkId: null }; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + docInfo, + ); + + const fileAddResult = await manager.addFile(sourceStore, ".txt", Buffer.from(defaultTestFileContent)); + manager.startTransferringFileToOtherStore(fileAddResult.fileIdent, destStore); + + await manager.allTransfersCompleted(); + + if (!destStore) { + await defaultDocStorageFake.getFileInfo(fileAddResult.fileIdent); + assert.equal( + (await defaultDocStorageFake.getFileInfo(fileAddResult.fileIdent))?.data?.toString(), + defaultTestFileContent, + ); + return; + } + + const store = (await defaultProvider.getStore(destStore))!; + + assert( + await store.exists(getDocPoolIdFromDocInfo(docInfo), fileAddResult.fileIdent), + "file does not exist in new store" + ); + } + + it("can transfer a file from internal to external storage", async function() { + await testStoreTransfer(undefined, defaultProvider.listAllStoreIds()[0]); + }); + + it("can transfer a file from external to internal storage", async function() { + await testStoreTransfer(defaultProvider.listAllStoreIds()[0], undefined); + }); + + it("can transfer a file from external to a different external storage", async function() { + await testStoreTransfer(defaultProvider.listAllStoreIds()[0], defaultProvider.listAllStoreIds()[1]); + }); + + it("throws an error if the downloaded file is corrupted", async function() { + const docInfo = { id: "12345", trunkId: null }; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + docInfo, + ); + + const sourceStoreId = defaultProvider.listAllStoreIds()[0]; + const fileAddResult = await manager.addFile(sourceStoreId, ".txt", Buffer.from(defaultTestFileContent)); + + const sourceStore = await defaultProvider.getStore(defaultProvider.listAllStoreIds()[0]); + const badData = stream.Readable.from(Buffer.from("I am corrupted")); + await sourceStore?.upload(getDocPoolIdFromDocInfo(docInfo), fileAddResult.fileIdent, badData); + + const transferPromise = + manager.transferFileToOtherStore(fileAddResult.fileIdent, defaultProvider.listAllStoreIds()[1]); + await assert.isRejected(transferPromise, AttachmentRetrievalError, "checksum verification failed"); + }); + + it("transfers all files in the background", async function() { + const docInfo = { id: "12345", trunkId: null }; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + docInfo, + ); + + const allStoreIds = defaultProvider.listAllStoreIds(); + const sourceStoreId = allStoreIds[0]; + const fileAddResult1 = await manager.addFile(sourceStoreId, ".txt", Buffer.from("A")); + const fileAddResult2 = await manager.addFile(sourceStoreId, ".txt", Buffer.from("B")); + const fileAddResult3 = await manager.addFile(sourceStoreId, ".txt", Buffer.from("C")); + + await manager.startTransferringAllFilesToOtherStore(allStoreIds[1]); + assert.isTrue(manager.transferStatus().isRunning); + await manager.allTransfersCompleted(); + assert.isFalse(manager.transferStatus().isRunning); + + + const destStore = (await defaultProvider.getStore(allStoreIds[1]))!; + const poolId = getDocPoolIdFromDocInfo(docInfo); + assert.isTrue(await destStore.exists(poolId, fileAddResult1.fileIdent)); + assert.isTrue(await destStore.exists(poolId, fileAddResult2.fileIdent)); + assert.isTrue(await destStore.exists(poolId, fileAddResult3.fileIdent)); + }); + + it("uses the most recent transfer destination", async function() { + const docInfo = { id: "12345", trunkId: null }; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + docInfo, + ); + + const allStoreIds = defaultProvider.listAllStoreIds(); + const fileAddResult1 = await manager.addFile(allStoreIds[0], ".txt", Buffer.from("A")); + + manager.startTransferringFileToOtherStore(fileAddResult1.fileIdent, allStoreIds[1]); + manager.startTransferringFileToOtherStore(fileAddResult1.fileIdent, allStoreIds[0]); + manager.startTransferringFileToOtherStore(fileAddResult1.fileIdent, allStoreIds[1]); + manager.startTransferringFileToOtherStore(fileAddResult1.fileIdent, allStoreIds[0]); + await manager.allTransfersCompleted(); + + const fileInfo = await defaultDocStorageFake.getFileInfo(fileAddResult1.fileIdent); + assert.equal(fileInfo?.storageId, allStoreIds[0], "the file should be in the original store"); + // We can't assert on if the files exists in the store, as it might be transferred from A to B and back to A, + // and so exist in both stores. + }); }); diff --git a/test/server/lib/AttachmentStoreProvider.ts b/test/server/lib/AttachmentStoreProvider.ts index eb14db7647..278b60d66d 100644 --- a/test/server/lib/AttachmentStoreProvider.ts +++ b/test/server/lib/AttachmentStoreProvider.ts @@ -1,37 +1,52 @@ import {assert} from 'chai'; -import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; -import {makeTestingFilesystemStoreSpec} from './FilesystemAttachmentStore'; +import { + AttachmentStoreProvider, +} from 'app/server/lib/AttachmentStoreProvider'; +import { + makeTestingFilesystemStoreConfig, +} from 'test/server/lib/FilesystemAttachmentStore'; const testInstallationUUID = "FAKE-UUID"; -function expectedStoreId(type: string) { - return `${testInstallationUUID}-${type}`; +function expectedStoreId(label: string) { + return `${testInstallationUUID}-${label}`; } describe('AttachmentStoreProvider', () => { - it('constructs stores using the installations UUID and store type', async () => { - const filesystemType1 = await makeTestingFilesystemStoreSpec("filesystem1"); - const filesystemType2 = await makeTestingFilesystemStoreSpec("filesystem2"); + it('constructs stores using the installations UID and store type', async () => { + const storesConfig = [ + await makeTestingFilesystemStoreConfig("filesystem1"), + await makeTestingFilesystemStoreConfig("filesystem2") + ]; - const provider = new AttachmentStoreProvider([filesystemType1, filesystemType2], testInstallationUUID); + const provider = new AttachmentStoreProvider(storesConfig, testInstallationUUID); const allStores = await provider.getAllStores(); const ids = allStores.map(store => store.id); - assert.includeMembers(ids, [expectedStoreId("filesystem1"), expectedStoreId("filesystem2")]); + assert.deepEqual(ids, [expectedStoreId("filesystem1"), expectedStoreId("filesystem2")]); + + const allStoreIds = provider.listAllStoreIds(); + assert.deepEqual(allStoreIds, [expectedStoreId("filesystem1"), expectedStoreId("filesystem2")]); }); it("can retrieve a store if it exists", async () => { - const filesystemSpec = await makeTestingFilesystemStoreSpec("filesystem"); - const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); + const storesConfig = [ + await makeTestingFilesystemStoreConfig("filesystem1"), + ]; + + const provider = new AttachmentStoreProvider(storesConfig, testInstallationUUID); assert.isNull(await provider.getStore("doesn't exist"), "store shouldn't exist"); - assert(await provider.getStore(expectedStoreId("filesystem")), "store not present"); + assert(await provider.getStore(expectedStoreId("filesystem1")), "store not present"); }); it("can check if a store exists", async () => { - const filesystemSpec = await makeTestingFilesystemStoreSpec("filesystem"); - const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); + const storesConfig = [ + await makeTestingFilesystemStoreConfig("filesystem1"), + ]; + + const provider = new AttachmentStoreProvider(storesConfig, testInstallationUUID); - assert(await provider.storeExists(expectedStoreId("filesystem"))); + assert(await provider.storeExists(expectedStoreId("filesystem1"))); }); }); diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index b327cabfb2..3ba9a97d64 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -137,7 +137,8 @@ describe('DocApi', function () { setup('merged', async () => { const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, - GRIST_DATA_DIR: dataDir + GRIST_DATA_DIR: dataDir, + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', }; home = docs = await TestServer.startServer('home,docs', tmpDir, suitename, additionalEnvConfiguration); homeUrl = serverUrl = home.serverUrl; @@ -151,7 +152,8 @@ describe('DocApi', function () { const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, GRIST_DATA_DIR: dataDir, - GRIST_ANON_PLAYGROUND: 'false' + GRIST_ANON_PLAYGROUND: 'false', + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', }; home = docs = await TestServer.startServer('home,docs', tmpDir, suitename, additionalEnvConfiguration); homeUrl = serverUrl = home.serverUrl; @@ -171,7 +173,8 @@ describe('DocApi', function () { setup('separated', async () => { const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, - GRIST_DATA_DIR: dataDir + GRIST_DATA_DIR: dataDir, + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); @@ -199,6 +202,7 @@ describe('DocApi', function () { GRIST_ORG_IN_PATH: 'true', GRIST_SINGLE_PORT: '0', APP_HOME_INTERNAL_URL: withAppHomeInternalUrl ? home.serverUrl : '', + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', }; await home.start(home.serverUrl, additionalEnvConfiguration); @@ -287,7 +291,8 @@ describe('DocApi', function () { setup('docs', async () => { const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, - GRIST_DATA_DIR: dataDir + GRIST_DATA_DIR: dataDir, + GRIST_EXTERNAL_ATTACHMENTS_MODE: 'test', }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); homeUrl = home.serverUrl; @@ -2738,6 +2743,67 @@ function testDocApi(settings: { await checkAttachmentIds([]); }); + describe("external attachment stores", async () => { + let docId = ""; + let docUrl = ""; + + before(async () => { + const wid = await getWorkspaceId(userApi, 'Private'); + docId = await userApi.newDoc({name: 'TestDocExternalAttachments'}, wid); + docUrl = `${serverUrl}/api/docs/${docId}`; + + const formData = new FormData(); + formData.append('upload', 'foobar', "hello.doc"); + formData.append('upload', '123456', "world.jpg"); + formData.append('upload', 'foobar', "hello2.doc"); + const resp = await axios.post(`${docUrl}/attachments`, formData, + defaultsDeep({headers: formData.getHeaders()}, chimpy)); + assert.equal(resp.status, 200); + assert.deepEqual(resp.data, [1, 2, 3]); + }); + + after(async () => { + await userApi?.deleteDoc(docId); + }); + + it("GET /docs/{did}/attachments/transferStatus reports idle transfer status", async function () { + const resp = await axios.get(`${docUrl}/attachments/transferStatus`, chimpy); + assert.deepEqual(resp.data, { + status: { + pendingTransferCount: 0, + isRunning: false, + }, + locationSummary: "internal", + }); + }); + + it("GET /docs/{did}/attachments/store gets the external store", async function () { + const resp = await axios.get(`${docUrl}/attachments/store`, chimpy); + assert.equal(resp.data.type, 'internal'); + }); + + it("POST /docs/{did}/attachments/store sets the external store", async function () { + const postResp = await axios.post(`${docUrl}/attachments/store`, { + type: 'external', + }, chimpy); + assert.equal(postResp.status, 200, JSON.stringify(postResp.data)); + + const getResp = await axios.get(`${docUrl}/attachments/store`, chimpy); + assert.equal(getResp.data.type, 'external'); + }); + + it("POST /docs/{did}/attachments/transferAll transfers all attachments", async function () { + const transferResp = await axios.post(`${docUrl}/attachments/transferAll`, {}, chimpy); + + assert.deepEqual(transferResp.data, { + status: { + pendingTransferCount: 2, + isRunning: true, + }, + locationSummary: "internal", + }); + }); + }); }); it("GET /docs/{did}/download serves document", async function () { diff --git a/test/server/lib/DocStorage.js b/test/server/lib/DocStorage.js index 7f09e5d38c..60670ff1f9 100644 --- a/test/server/lib/DocStorage.js +++ b/test/server/lib/DocStorage.js @@ -385,22 +385,29 @@ describe('DocStorage', function() { }); }); - describe('findOrAttachFile', function() { + describe('attachFileIfNew', function() { var docStorage; it("should create attachment blob", function() { docStorage = new DocStorage(docStorageManager, 'test_Attachments'); const correctFileContents = "Hello, world!" + const replacementFileContents = "Another file" return docStorage.createFile() - .then(() => docStorage.findOrAttachFile( "hello_world.txt", Buffer.from(correctFileContents))) + .then(() => docStorage.attachFileIfNew( "hello_world.txt", Buffer.from(correctFileContents))) .then(result => assert.isTrue(result)) .then(() => docStorage.getFileInfo("hello_world.txt")) .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents)) // If we use the same fileIdent for another file, it should not get attached. - .then(() => docStorage.findOrAttachFile("hello_world.txt", Buffer.from("Another file"))) + .then(() => docStorage.attachFileIfNew("hello_world.txt", Buffer.from(replacementFileContents))) .then(result => assert.isFalse(result)) .then(() => docStorage.getFileInfo("hello_world.txt")) - .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents)); + .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents)) + + // The update parameter should allow the record to be overwritten + .then(() => docStorage.attachOrUpdateFile("hello_world.txt", Buffer.from(replacementFileContents), undefined)) + .then(result => assert.isFalse(result)) + .then(() => docStorage.getFileInfo("hello_world.txt")) + .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), replacementFileContents)); }); }); diff --git a/test/server/lib/FilesystemAttachmentStore.ts b/test/server/lib/FilesystemAttachmentStore.ts index 7f2cd31ea9..8fdda274e6 100644 --- a/test/server/lib/FilesystemAttachmentStore.ts +++ b/test/server/lib/FilesystemAttachmentStore.ts @@ -1,4 +1,5 @@ import {FilesystemAttachmentStore} from 'app/server/lib/AttachmentStore'; +import {IAttachmentStoreConfig} from 'app/server/lib/AttachmentStoreProvider'; import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream'; import {createTmpDir} from 'test/server/docTools'; @@ -31,6 +32,15 @@ export async function makeTestingFilesystemStoreSpec( }; } +export async function makeTestingFilesystemStoreConfig( + name: string = "test-filesystem" +): Promise { + return { + label: name, + spec: await makeTestingFilesystemStoreSpec(name), + }; +} + describe('FilesystemAttachmentStore', () => { it('can upload a file', async () => { const spec = await makeTestingFilesystemStoreSpec(); @@ -61,6 +71,17 @@ describe('FilesystemAttachmentStore', () => { assert.isTrue(await store.exists(testingDocPoolId, testingFileId)); }); + it('can delete a file', async () => { + const spec = await makeTestingFilesystemStoreSpec(); + const store = await spec.create("test-filesystem-store"); + + assert.isFalse(await store.exists(testingDocPoolId, testingFileId)); + await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); + assert.isTrue(await store.exists(testingDocPoolId, testingFileId)); + await store.delete(testingDocPoolId, testingFileId); + assert.isFalse(await store.exists(testingDocPoolId, testingFileId)); + }); + it('can remove an entire pool', async () => { const spec = await makeTestingFilesystemStoreSpec(); const store = await spec.create("test-filesystem-store"); diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index 830e8f50ce..767c682e11 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -315,7 +315,8 @@ class TestStore { return result; }; - const attachmentStoreProvider = this._attachmentStoreProvider ?? new AttachmentStoreProvider([], "TESTINSTALL"); + const attachmentStoreProvider = + this._attachmentStoreProvider ?? new AttachmentStoreProvider([], "TESTINSTALL"); const testStore = this; const gristServer: GristServer = {