From 2473375e4c8d732dd2fbae6580167a6dac0d75ea Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sun, 27 Oct 2024 17:13:25 +0000 Subject: [PATCH 01/60] Enables checksumFile to handle streams --- app/server/lib/checksumFile.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/server/lib/checksumFile.ts b/app/server/lib/checksumFile.ts index dfd6241af1..84af3247ed 100644 --- a/app/server/lib/checksumFile.ts +++ b/app/server/lib/checksumFile.ts @@ -1,4 +1,5 @@ import {createHash} from 'crypto'; +import {Readable} from 'node:stream'; import * as fs from 'fs'; /** @@ -6,8 +7,12 @@ import * as fs from 'fs'; * supported by crypto.createHash(). */ export async function checksumFile(filePath: string, algorithm: string = 'sha1'): Promise { - const shaSum = createHash(algorithm); const stream = fs.createReadStream(filePath); + return checksumFileStream(stream, algorithm); +} + +export async function checksumFileStream(stream: Readable, algorithm: string = 'sha1'): Promise { + const shaSum = createHash(algorithm); try { stream.on('data', (data) => shaSum.update(data)); await new Promise((resolve, reject) => { From ff2fe69fe9836b0433537b5c761550cfcfae4296 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sun, 27 Oct 2024 17:14:35 +0000 Subject: [PATCH 02/60] Routes attachment handling through AttachmentFileManager --- app/server/lib/ActiveDoc.ts | 24 ++++++++++++------- app/server/lib/AttachmentFileManager.ts | 31 +++++++++++++++++++++++++ app/server/lib/DocStorage.ts | 8 +++---- 3 files changed, 49 insertions(+), 14 deletions(-) create mode 100644 app/server/lib/AttachmentFileManager.ts diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index ed512b03ff..3782162335 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -95,7 +95,6 @@ import {AssistanceSchemaPromptV1Context} from 'app/server/lib/Assistance'; import {AssistanceContext} from 'app/common/AssistancePrompts'; import {AuditEventProperties} from 'app/server/lib/AuditLogger'; import {Authorizer, RequestWithLogin} from 'app/server/lib/Authorizer'; -import {checksumFile} from 'app/server/lib/checksumFile'; import {Client} from 'app/server/lib/Client'; import {getMetaTables} from 'app/server/lib/DocApi'; import {DEFAULT_CACHE_TTL, DocManager} from 'app/server/lib/DocManager'; @@ -137,6 +136,7 @@ import tmp from 'tmp'; import {ActionHistory} from './ActionHistory'; import {ActionHistoryImpl} from './ActionHistoryImpl'; import {ActiveDocImport, FileImportOptions} from './ActiveDocImport'; +import {AttachmentFileManager} from "./AttachmentFileManager"; import {DocClients} from './DocClients'; import {DocPluginManager} from './DocPluginManager'; import {DocSession, makeExceptionalDocSession, OptDocSession} from './DocSession'; @@ -154,6 +154,7 @@ import remove = require('lodash/remove'); import sum = require('lodash/sum'); import without = require('lodash/without'); import zipObject = require('lodash/zipObject'); +import { readFile } from "fs-extra"; bluebird.promisifyAll(tmp); @@ -268,6 +269,7 @@ export class ActiveDoc extends EventEmitter { private _onlyAllowMetaDataActionsOnDb: boolean = false; // Cache of which columns are attachment columns. private _attachmentColumns?: AttachmentColumns; + private _attachmentFileManager: AttachmentFileManager; // Client watching for 'product changed' event published by Billing to update usage private _redisSubscriber?: RedisClient; @@ -283,7 +285,11 @@ export class ActiveDoc extends EventEmitter { private _doShutdown?: Promise; private _intervals: Interval[] = []; - constructor(docManager: DocManager, docName: string, private _options?: ICreateActiveDocOptions) { + constructor( + docManager: DocManager, + docName: string, + private _options?: ICreateActiveDocOptions, + ) { super(); const {forkId, snapshotId} = parseUrlId(docName); this._isSnapshot = Boolean(snapshotId); @@ -378,6 +384,8 @@ export class ActiveDoc extends EventEmitter { loadTable: this._rawPyCall.bind(this, 'load_table'), }); + this._attachmentFileManager = new AttachmentFileManager(this.docStorage); + // Our DataEngine is a separate sandboxed process (one sandbox per open document, // corresponding to one process for pynbox, more for gvisor). // The data engine runs user-defined python code including formula calculations. @@ -912,7 +920,7 @@ export class ActiveDoc extends EventEmitter { } } } - const data = await this.docStorage.getFileData(fileIdent); + const data = await this._attachmentFileManager.getFileData(fileIdent); if (!data) { throw new ApiError("Invalid attachment identifier", 404); } this._log.info(docSession, "getAttachment: %s -> %s bytes", fileIdent, data.length); return data; @@ -2347,13 +2355,11 @@ export class ActiveDoc extends EventEmitter { dimensions.height = 0; dimensions.width = 0; } - const checksum = await checksumFile(fileData.absPath); - const fileIdent = checksum + fileData.ext; - const ret: boolean = await this.docStorage.findOrAttachFile(fileData.absPath, fileIdent); - this._log.info(docSession, "addAttachment: file %s (image %sx%s) %s", fileIdent, - dimensions.width, dimensions.height, ret ? "attached" : "already exists"); + const addFileResult = await this._attachmentFileManager.addFile(fileData.ext, await readFile(fileData.absPath)); + this._log.info(docSession, "addAttachment: file %s (image %sx%s) %s", addFileResult.fileIdent, + dimensions.width, dimensions.height, addFileResult.alreadyExisted ? "attached" : "already exists"); return ['AddRecord', '_grist_Attachments', null, { - fileIdent, + fileIdent: addFileResult.fileIdent, fileName: fileData.origName, // We used to set fileType, but it's not easily available for native types. Since it's // also entirely unused, we just skip it until it becomes relevant. diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts new file mode 100644 index 0000000000..1dbab33235 --- /dev/null +++ b/app/server/lib/AttachmentFileManager.ts @@ -0,0 +1,31 @@ +import { DocStorage } from "app/server/lib/DocStorage"; +import { checksumFileStream } from "app/server/lib/checksumFile"; +import { Readable } from "node:stream"; + +export interface IAttachmentFileManager { + addFile(fileExtension: string, fileData: Buffer): Promise; +} + +export interface AddFileResult { + fileIdent: string; + alreadyExisted: boolean; +} + +export class AttachmentFileManager implements IAttachmentFileManager { + constructor(private _docStorage: DocStorage) {} + + public async addFile(fileExtension: string, fileData: Buffer): Promise { + const checksum = await checksumFileStream(Readable.from(fileData)); + const fileIdent = `${checksum}${fileExtension}`; + const existed = await this._docStorage.findOrAttachFile(fileIdent, fileData); + + return { + fileIdent, + alreadyExisted: existed, + }; + } + + public async getFileData(fileIdent: string): Promise { + return await this._docStorage.getFileData(fileIdent); + } +} diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 584f6cbedb..1666066eae 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -21,7 +21,6 @@ import {IDocStorageManager} from 'app/server/lib/IDocStorageManager'; import log from 'app/server/lib/log'; import assert from 'assert'; import * as bluebird from 'bluebird'; -import * as fse from 'fs-extra'; import * as _ from 'underscore'; import * as util from 'util'; import uuidv4 from "uuid/v4"; @@ -773,14 +772,13 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { * checksum of the file's contents with the original extension. * @returns {Promise[Boolean]} True if the file got attached; false if this ident already exists. */ - public findOrAttachFile(sourcePath: string, fileIdent: string): Promise { + public findOrAttachFile(fileIdent: string, fileData: Buffer | undefined, ): 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(() => fse.readFile(sourcePath)) - .then(data => - db.run('UPDATE _gristsys_Files SET data=? WHERE ident=?', data, fileIdent)) + .then(() => + db.run('UPDATE _gristsys_Files SET data=? WHERE ident=?', fileData, fileIdent)) .then(() => true) // If UNIQUE constraint failed, this ident must already exists, so return false. .catch(err => { From fc35744df0308cff982292c564cb048e065a098b Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 4 Nov 2024 14:51:13 +0000 Subject: [PATCH 03/60] Switches ActiveDoc to using AttachmentFileManager for attachments --- app/server/lib/ActiveDoc.ts | 13 ++- app/server/lib/AttachmentFileManager.ts | 120 ++++++++++++++++++++-- app/server/lib/AttachmentStore.ts | 32 ++++++ app/server/lib/AttachmentStoreProvider.ts | 25 +++++ app/server/lib/DocStorage.ts | 26 ++++- 5 files changed, 203 insertions(+), 13 deletions(-) create mode 100644 app/server/lib/AttachmentStore.ts create mode 100644 app/server/lib/AttachmentStoreProvider.ts diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 3782162335..931b2335c1 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -142,6 +142,7 @@ import {DocPluginManager} from './DocPluginManager'; import {DocSession, makeExceptionalDocSession, OptDocSession} from './DocSession'; import {createAttachmentsIndex, DocStorage, REMOVE_UNUSED_ATTACHMENTS_DELAY} from './DocStorage'; import {expandQuery, getFormulaErrorForExpandQuery} from './ExpandedQuery'; +import {IAttachmentStoreProvider} from "./AttachmentStoreProvider"; import {GranularAccess, GranularAccessForBundle} from './GranularAccess'; import {OnDemandActions} from './OnDemandActions'; import {getPubSubPrefix} from './serverUtils'; @@ -289,6 +290,7 @@ export class ActiveDoc extends EventEmitter { docManager: DocManager, docName: string, private _options?: ICreateActiveDocOptions, + externalAttachmentStoreProvider?: IAttachmentStoreProvider ) { super(); const {forkId, snapshotId} = parseUrlId(docName); @@ -384,7 +386,13 @@ export class ActiveDoc extends EventEmitter { loadTable: this._rawPyCall.bind(this, 'load_table'), }); - this._attachmentFileManager = new AttachmentFileManager(this.docStorage); + // This will throw errors if _doc or externalAttachmentStoreProvider aren't provided, and ActiveDoc tries to use + // an external attachment store. + this._attachmentFileManager = new AttachmentFileManager( + this.docStorage, + externalAttachmentStoreProvider, + this._doc + ); // Our DataEngine is a separate sandboxed process (one sandbox per open document, // corresponding to one process for pynbox, more for gvisor). @@ -2355,7 +2363,8 @@ export class ActiveDoc extends EventEmitter { dimensions.height = 0; dimensions.width = 0; } - const addFileResult = await this._attachmentFileManager.addFile(fileData.ext, await readFile(fileData.absPath)); + const addFileResult = await this._attachmentFileManager + .addFile(undefined, fileData.ext, await readFile(fileData.absPath)); this._log.info(docSession, "addAttachment: file %s (image %sx%s) %s", addFileResult.fileIdent, dimensions.width, dimensions.height, addFileResult.alreadyExisted ? "attached" : "already exists"); return ['AddRecord', '_grist_Attachments', null, { diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 1dbab33235..a4b7c0072b 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -1,9 +1,17 @@ import { DocStorage } from "app/server/lib/DocStorage"; import { checksumFileStream } from "app/server/lib/checksumFile"; import { Readable } from "node:stream"; +import { + DocPoolId, IAttachmentStore +} from "app/server/lib/AttachmentStore"; +import { + AttachmentStoreId, + IAttachmentStoreProvider +} from "app/server/lib/AttachmentStoreProvider"; export interface IAttachmentFileManager { - addFile(fileExtension: string, fileData: Buffer): Promise; + addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; + getFileData(fileIdent: string): Promise; } export interface AddFileResult { @@ -11,12 +19,78 @@ export interface AddFileResult { alreadyExisted: boolean; } +export class StoresNotConfiguredError extends Error { + constructor() { + super('Attempted to access a file store, but AttachmentFileManager was initialised without store access'); + } +} + +export class StoreNotAvailableError extends Error { + public readonly storeId: AttachmentStoreId; + + constructor(storeId: AttachmentStoreId) { + super(`Store '${storeId}' is not a valid and available store`); + this.storeId = storeId; + } +} + +// Minimum required info from a document +// Compatible with Document entity for ease of use +interface DocInfo { + id: string; + trunkId: string | null | undefined; +} + +/** + * Provides management of a specific document's attachments. + */ export class AttachmentFileManager implements IAttachmentFileManager { - constructor(private _docStorage: DocStorage) {} + private readonly _docPoolId: DocPoolId | null; + + /** + * @param _docStorage - Storage of this manager's document. + * @param _storeProvider - Allows instantiating of stores. Should be provided except in test scenarios. + * @param _docInfo - The document this manager is for. Should be provided except in test scenarios. + */ + constructor( + private _docStorage: DocStorage, + // TODO - Pull these into a "StoreAccess" module-private type + private _storeProvider: IAttachmentStoreProvider | undefined, + _docInfo: DocInfo | undefined, + ) { + this._docPoolId = _docInfo?.trunkId ?? _docInfo?.id ?? null; + } + + public async addFile( + storeId: AttachmentStoreId | undefined, + fileExtension: string, + fileData: Buffer + ): Promise { + const fileIdent = await this._getFileIdentifier(fileExtension, Readable.from(fileData)); + if (storeId === undefined) { + return this._addFileToLocalStorage(fileIdent, fileData); + } + const store = await this._getStore(storeId); + if (!store) { + throw new StoreNotAvailableError(storeId); + } + return this._addFileToAttachmentStore(store, fileIdent, fileData); + } - public async addFile(fileExtension: string, fileData: Buffer): Promise { - const checksum = await checksumFileStream(Readable.from(fileData)); - const fileIdent = `${checksum}${fileExtension}`; + public async getFileData(fileIdent: string): Promise { + const fileInfo = await this._docStorage.getFileInfo(fileIdent); + if (!fileInfo) { return null; } + if (!fileInfo.storageId) { + return fileInfo.data; + } + const store = await this._getStore(fileInfo.storageId); + if (!store) { + throw new StoreNotAvailableError(fileInfo.storageId); + } + return this._getFileDataFromAttachmentStore(store, fileIdent); + } + + private async _addFileToLocalStorage(fileIdent: string, fileData: Buffer): Promise { const existed = await this._docStorage.findOrAttachFile(fileIdent, fileData); return { @@ -25,7 +99,39 @@ export class AttachmentFileManager implements IAttachmentFileManager { }; } - public async getFileData(fileIdent: string): Promise { - return await this._docStorage.getFileData(fileIdent); + private async _getStore(storeId: AttachmentStoreId): Promise { + if (!this._storeProvider) { + throw new StoresNotConfiguredError(); + } + return this._storeProvider.getStore(storeId); + } + + private _getDocPoolId(): string { + if (!this._docPoolId) { + throw new StoresNotConfiguredError(); + } + return this._docPoolId; + } + + private async _getFileIdentifier(fileExtension: string, fileData: Readable): Promise { + const checksum = await checksumFileStream(fileData); + return `${checksum}${fileExtension}`; + } + + private async _addFileToAttachmentStore(store: IAttachmentStore, fileIdent: string, fileData: Buffer): Promise { + // TODO - Register it in doc storage? + // Upload to the remote storage + // TODO Error handling? + await store.upload(this._getDocPoolId(), fileIdent, fileData); + // TODO - Confirm in doc storage that it's successfully uploaded? Need to decide how to handle a failed upload. + return { + fileIdent, + // TODO - Actually check this value. Do we really even need this? + alreadyExisted: false, + }; + } + + private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { + return await store.download(this._getDocPoolId(), fileIdent); } } diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts new file mode 100644 index 0000000000..fa1cf49cab --- /dev/null +++ b/app/server/lib/AttachmentStore.ts @@ -0,0 +1,32 @@ +export type DocPoolId = string; +type AttachmentId = string; + + +/** + * Provides access to external storage, specifically for storing attachments. + * + * Attachments are stored in a "Document Pool", which is used to manage the attachments' lifecycle. + * Typically a document's trunk id should be used for this if available, or the document's id if it isn't. + * + * This is a general-purpose interface that should abstract over many different storage providers, + * so shouldn't have methods which rely on one the features of one specific provider. + * + * This is distinct from `ExternalStorage`, in that it's more generic and doesn't support versioning. + */ +export interface IAttachmentStore { + // Check if attachment exists in the store. + exists(docPoolId: DocPoolId, attachmentId: AttachmentId): Promise; + + // Upload attachment to the store. + upload(docPoolId: DocPoolId, attachmentId: AttachmentId, fileData: Buffer): Promise; + + // Download attachment to an in-memory buffer + download(docPoolId: DocPoolId, attachmentId: AttachmentId): Promise; + + // Remove attachments for all documents in the given document pool. + removePool(docPoolId: DocPoolId): Promise; + + // Close the storage object. + close(): Promise; +} + diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts new file mode 100644 index 0000000000..8e02978fea --- /dev/null +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -0,0 +1,25 @@ +import { IAttachmentStore } from "./AttachmentStore"; + +export type AttachmentStoreId = string + +export interface IAttachmentStoreProvider { + // Returns the store associated with the given id + getStore(id: AttachmentStoreId): Promise + + storeExists(id: AttachmentStoreId): Promise; +} + +/* +export class AttachmentStoreProvider implements IAttachmentStoreProvider { + constructor() { + } + + public getStore(id: string): Promise { + + } + + public storeExists(id: string): Promise { + + } +} +*/ diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 1666066eae..57b79f1694 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -72,6 +72,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { await db.exec(`CREATE TABLE _gristsys_Files ( id INTEGER PRIMARY KEY, ident TEXT UNIQUE, + storageId TEXT, data BLOB )`); await db.exec(`CREATE TABLE _gristsys_Action ( @@ -393,7 +394,13 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { } await createAttachmentsIndex(db); }, - + async function(db: SQLiteDB): Promise { + // Storage version 9. + // Migration to add `storage` column to _gristsys_files, which can optionally refer to an external storage + // where the file is stored. + // Default should be NULL. + await db.exec(`ALTER TABLE _gristsys_files ADD COLUMN storageId TEXT`); + }, ] }; @@ -795,9 +802,13 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { * @param {String} fileIdent: The unique identifier of a file, as used by findOrAttachFile. * @returns {Promise[Buffer]} The data buffer associated with fileIdent. */ - public getFileData(fileIdent: string): Promise { - return this.get('SELECT data FROM _gristsys_Files WHERE ident=?', fileIdent) - .then(row => row && row.data); + public getFileInfo(fileIdent: string): Promise { + return this.get('SELECT ident, storageId, data FROM _gristsys_Files WHERE ident=?', fileIdent) + .then(row => row ? ({ + ident: row.ident as string, + storageId: row.storageId as string, + data: row.data as Buffer, + }) : null); } @@ -1849,3 +1860,10 @@ export async function createAttachmentsIndex(db: ISQLiteDB) { function fixDefault(def: string) { return (def === '""') ? "''" : def; } + +// Information on an attached file from _gristsys_files +export interface FileInfo { + ident: string; + storageId: string; + data: Buffer; +} From ef13def9ca0fca0e0b17e4da86acd3274a4fef39 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 4 Nov 2024 15:50:34 +0000 Subject: [PATCH 04/60] Fixes a casing error in DocStorage --- app/server/lib/DocStorage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 57b79f1694..cd7350899c 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -396,10 +396,10 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { }, async function(db: SQLiteDB): Promise { // Storage version 9. - // Migration to add `storage` column to _gristsys_files, which can optionally refer to an external storage + // Migration to add `storage` column to _gristsys_Files, which can optionally refer to an external storage // where the file is stored. // Default should be NULL. - await db.exec(`ALTER TABLE _gristsys_files ADD COLUMN storageId TEXT`); + await db.exec(`ALTER TABLE _gristsys_Files ADD COLUMN storageId TEXT`); }, ] }; From d19ff546e877da21c752a3fb9ef077d4ee9b9c82 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 4 Nov 2024 17:41:30 +0000 Subject: [PATCH 05/60] Adds working filesystem store (in use by default) --- app/server/lib/ActiveDoc.ts | 4 +-- app/server/lib/AttachmentFileManager.ts | 27 ++++++++++++----- app/server/lib/AttachmentStore.ts | 37 +++++++++++++++++++++++ app/server/lib/AttachmentStoreProvider.ts | 12 +++----- app/server/lib/DocManager.ts | 4 ++- app/server/lib/DocStorage.ts | 15 ++++++--- 6 files changed, 76 insertions(+), 23 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 931b2335c1..c2258575f1 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -2364,9 +2364,9 @@ export class ActiveDoc extends EventEmitter { dimensions.width = 0; } const addFileResult = await this._attachmentFileManager - .addFile(undefined, fileData.ext, await readFile(fileData.absPath)); + .addFile("TEST-FILESYSTEM-STORE", fileData.ext, await readFile(fileData.absPath)); this._log.info(docSession, "addAttachment: file %s (image %sx%s) %s", addFileResult.fileIdent, - dimensions.width, dimensions.height, addFileResult.alreadyExisted ? "attached" : "already exists"); + dimensions.width, dimensions.height, addFileResult.isNewFile ? "attached" : "already exists"); return ['AddRecord', '_grist_Attachments', null, { fileIdent: addFileResult.fileIdent, fileName: fileData.origName, diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index a4b7c0072b..42ffd52d3d 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -16,7 +16,7 @@ export interface IAttachmentFileManager { export interface AddFileResult { fileIdent: string; - alreadyExisted: boolean; + isNewFile: boolean; } export class StoresNotConfiguredError extends Error { @@ -91,11 +91,11 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _addFileToLocalStorage(fileIdent: string, fileData: Buffer): Promise { - const existed = await this._docStorage.findOrAttachFile(fileIdent, fileData); + const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, fileData); return { fileIdent, - alreadyExisted: existed, + isNewFile, }; } @@ -119,15 +119,26 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _addFileToAttachmentStore(store: IAttachmentStore, fileIdent: string, fileData: Buffer): Promise { - // TODO - Register it in doc storage? - // Upload to the remote storage - // TODO Error handling? + const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, fileData, 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); + + 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, fileData); + // TODO - Confirm in doc storage that it's successfully uploaded? Need to decide how to handle a failed upload. return { fileIdent, - // TODO - Actually check this value. Do we really even need this? - alreadyExisted: false, + isNewFile, }; } diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index fa1cf49cab..97152fccd5 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -1,3 +1,6 @@ +import * as path from "path"; +import * as fse from "fs-extra"; + export type DocPoolId = string; type AttachmentId = string; @@ -14,6 +17,8 @@ type AttachmentId = string; * This is distinct from `ExternalStorage`, in that it's more generic and doesn't support versioning. */ export interface IAttachmentStore { + readonly id: string; + // Check if attachment exists in the store. exists(docPoolId: DocPoolId, attachmentId: AttachmentId): Promise; @@ -30,3 +35,35 @@ export interface IAttachmentStore { close(): Promise; } +export class FilesystemAttachmentStore implements IAttachmentStore { + constructor(public readonly id: string, private _rootFolderPath: string) { + } + + public async exists(docPoolId: string, attachmentId: string): Promise { + return fse.pathExists(this._createPath(docPoolId, attachmentId)) + .catch(() => false); + } + + public async upload(docPoolId: string, attachmentId: string, fileData: Buffer): Promise { + const filePath = this._createPath(docPoolId, attachmentId); + await fse.ensureDir(path.dirname(filePath)); + await fse.writeFile(filePath, fileData); + } + + public async download(docPoolId: string, attachmentId: string): Promise { + return fse.readFile(this._createPath(docPoolId, attachmentId)); + } + + public async removePool(docPoolId: string): Promise { + await fse.remove(this._createPath(docPoolId)); + } + + public async close(): Promise { + // Not needed here, no resources held. + } + + private _createPath(docPoolId: string, attachmentId: string = ""): string { + return path.join(this._rootFolderPath, docPoolId, attachmentId); + } + +} diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 8e02978fea..680d9cff24 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,4 +1,4 @@ -import { IAttachmentStore } from "./AttachmentStore"; +import { FilesystemAttachmentStore, IAttachmentStore } from "./AttachmentStore"; export type AttachmentStoreId = string @@ -9,17 +9,15 @@ export interface IAttachmentStoreProvider { storeExists(id: AttachmentStoreId): Promise; } -/* export class AttachmentStoreProvider implements IAttachmentStoreProvider { constructor() { } - public getStore(id: string): Promise { - + public async getStore(id: string): Promise { + return new FilesystemAttachmentStore(id, "/home/spoffy/Downloads/Grist/TEST ATTACHMENTS"); } - public storeExists(id: string): Promise { - + public async storeExists(id: string): Promise { + return true; } } -*/ diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index a11c2f340b..290ed6073f 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -32,6 +32,7 @@ import {PluginManager} from './PluginManager'; import {getFileUploadInfo, globalUploadSet, makeAccessId, UploadInfo} from './uploads'; import merge = require('lodash/merge'); import noop = require('lodash/noop'); +import { AttachmentStoreProvider } from "./AttachmentStoreProvider"; // A TTL in milliseconds to use for material that can easily be recomputed / refetched // but is a bit of a burden under heavy traffic. @@ -615,7 +616,8 @@ export class DocManager extends EventEmitter { const doc = await this._getDoc(docSession, docName); // Get URL for document for use with SELF_HYPERLINK(). const docUrls = doc && await this._getDocUrls(doc); - const activeDoc = new ActiveDoc(this, docName, {...docUrls, safeMode, doc}); + // TODO - Use an actual attachment store provider, this is placeholder + const activeDoc = new ActiveDoc(this, docName, {...docUrls, safeMode, doc}, new AttachmentStoreProvider()); // Restore the timing mode of the document. activeDoc.isTimingOn = this._inTimingOn.get(docName) || false; return activeDoc; diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index cd7350899c..33822b6f31 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -774,18 +774,23 @@ 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} sourcePath: The path of the file containing the attachment data. - * @param {String} fileIdent: The unique identifier of the file in the database. ActiveDoc uses the + * @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 {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(fileIdent: string, fileData: Buffer | undefined, ): Promise { + public findOrAttachFile( + 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=? WHERE ident=?', fileData, fileIdent)) + 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 => { @@ -799,7 +804,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { /** * Reads and returns the data for the given attachment. - * @param {String} fileIdent: The unique identifier of a file, as used by findOrAttachFile. + * @param {string} fileIdent - The unique identifier of a file, as used by findOrAttachFile. * @returns {Promise[Buffer]} The data buffer associated with fileIdent. */ public getFileInfo(fileIdent: string): Promise { From 8295f5e14e5fad7703b3941269c980f34331608b Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 4 Nov 2024 17:58:58 +0000 Subject: [PATCH 06/60] Avoids storing external attachments in the sqlite file --- app/server/lib/AttachmentFileManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 42ffd52d3d..c5b8423a49 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -119,7 +119,7 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _addFileToAttachmentStore(store: IAttachmentStore, fileIdent: string, fileData: Buffer): Promise { - const isNewFile = await this._docStorage.findOrAttachFile(fileIdent, fileData, store.id); + 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); From 8f095736801431b5227851aaf3a4d07a5ed0f4d4 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 4 Nov 2024 18:23:45 +0000 Subject: [PATCH 07/60] Renames some parameters in AttachmentStore --- app/server/lib/AttachmentStore.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 97152fccd5..6db222edd2 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -2,7 +2,7 @@ import * as path from "path"; import * as fse from "fs-extra"; export type DocPoolId = string; -type AttachmentId = string; +type FileId = string; /** @@ -20,13 +20,13 @@ export interface IAttachmentStore { readonly id: string; // Check if attachment exists in the store. - exists(docPoolId: DocPoolId, attachmentId: AttachmentId): Promise; + exists(docPoolId: DocPoolId, fileId: FileId): Promise; // Upload attachment to the store. - upload(docPoolId: DocPoolId, attachmentId: AttachmentId, fileData: Buffer): Promise; + upload(docPoolId: DocPoolId, fileId: FileId, fileData: Buffer): Promise; // Download attachment to an in-memory buffer - download(docPoolId: DocPoolId, attachmentId: AttachmentId): Promise; + download(docPoolId: DocPoolId, fileId: FileId): Promise; // Remove attachments for all documents in the given document pool. removePool(docPoolId: DocPoolId): Promise; @@ -39,22 +39,22 @@ export class FilesystemAttachmentStore implements IAttachmentStore { constructor(public readonly id: string, private _rootFolderPath: string) { } - public async exists(docPoolId: string, attachmentId: string): Promise { - return fse.pathExists(this._createPath(docPoolId, attachmentId)) + public async exists(docPoolId: DocPoolId, fileId: FileId): Promise { + return fse.pathExists(this._createPath(docPoolId, fileId)) .catch(() => false); } - public async upload(docPoolId: string, attachmentId: string, fileData: Buffer): Promise { - const filePath = this._createPath(docPoolId, attachmentId); + public async upload(docPoolId: DocPoolId, fileId: FileId, fileData: Buffer): Promise { + const filePath = this._createPath(docPoolId, fileId); await fse.ensureDir(path.dirname(filePath)); await fse.writeFile(filePath, fileData); } - public async download(docPoolId: string, attachmentId: string): Promise { - return fse.readFile(this._createPath(docPoolId, attachmentId)); + public async download(docPoolId: DocPoolId, fileId: FileId): Promise { + return fse.readFile(this._createPath(docPoolId, fileId)); } - public async removePool(docPoolId: string): Promise { + public async removePool(docPoolId: DocPoolId): Promise { await fse.remove(this._createPath(docPoolId)); } @@ -62,8 +62,8 @@ export class FilesystemAttachmentStore implements IAttachmentStore { // Not needed here, no resources held. } - private _createPath(docPoolId: string, attachmentId: string = ""): string { - return path.join(this._rootFolderPath, docPoolId, attachmentId); + private _createPath(docPoolId: DocPoolId, fileId: FileId = ""): string { + return path.join(this._rootFolderPath, docPoolId, fileId); } } From 72f2e5c39d219c6f400d8abdea6eff3ae80b5bc5 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 12 Nov 2024 21:53:14 +0000 Subject: [PATCH 08/60] Adds "removeAllWithPrefix?" to ExternalStorage and MinIOExternalStorage --- app/server/lib/ExternalStorage.ts | 3 ++ app/server/lib/MinIOExternalStorage.ts | 63 +++++++++++++++++--------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/app/server/lib/ExternalStorage.ts b/app/server/lib/ExternalStorage.ts index 74394ba535..2caeff49ea 100644 --- a/app/server/lib/ExternalStorage.ts +++ b/app/server/lib/ExternalStorage.ts @@ -39,6 +39,9 @@ export interface ExternalStorage { // newest should be given first. remove(key: string, snapshotIds?: string[]): Promise; + // Removes all keys which start with the given prefix + removeAllWithPrefix?(prefix: string): Promise; + // List content versions that exist for the given key. More recent versions should // come earlier in the result list. versions(key: string): Promise; diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index f5b5788616..a123219d99 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -122,12 +122,21 @@ export class MinIOExternalStorage implements ExternalStorage { public async remove(key: string, snapshotIds?: string[]) { if (snapshotIds) { - await this._deleteBatch(key, snapshotIds); + await this._deleteVersions(key, snapshotIds); } else { await this._deleteAllVersions(key); } } + public async removeAllWithPrefix(prefix: string) { + const objects = await this._listObjects(this.bucket, prefix, false, { IncludeVersion: true }); + const objectsToDelete = objects.filter(o => o.name !== undefined).map(o => ({ + name: o.name!, + versionId: (o as any).versionId as (string | undefined), + })); + await this._deleteObjects(objectsToDelete); + } + public async hasVersioning(): Promise { const versioning = await this._s3.getBucketVersioning(this.bucket); // getBucketVersioning() may return an empty string when versioning has never been enabled. @@ -136,18 +145,7 @@ export class MinIOExternalStorage implements ExternalStorage { } public async versions(key: string, options?: { includeDeleteMarkers?: boolean }) { - const results: minio.BucketItem[] = []; - await new Promise((resolve, reject) => { - const stream = this._s3.listObjects(this.bucket, key, false, {IncludeVersion: true}); - stream - .on('error', reject) - .on('end', () => { - resolve(results); - }) - .on('data', data => { - results.push(data); - }); - }); + const results = await this._listObjects(this.bucket, key, false, {IncludeVersion: true}); return results .filter(v => v.name === key && v.lastModified && (v as any).versionId && @@ -182,21 +180,44 @@ export class MinIOExternalStorage implements ExternalStorage { // Delete all versions of an object. public async _deleteAllVersions(key: string) { const vs = await this.versions(key, {includeDeleteMarkers: true}); - await this._deleteBatch(key, vs.map(v => v.snapshotId)); + await this._deleteVersions(key, vs.map(v => v.snapshotId)); } // Delete a batch of versions for an object. - private async _deleteBatch(key: string, versions: Array) { + private async _deleteVersions(key: string, versions: Array) { + return this._deleteObjects( + versions.filter(v => v).map(versionId => ({ + name: key, + versionId, + })) + ); + } + + // Delete an arbitrary number of objects, batched appropriately. + private async _deleteObjects(objects: { name: string, versionId?: string }[]): Promise { // Max number of keys per request for AWS S3 is 1000, see: // https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html // Stick to this maximum in case we are using this client to talk to AWS. const N = this._batchSize || 1000; - for (let i = 0; i < versions.length; i += N) { - const iVersions = versions.slice(i, i + N).filter(v => v) as string[]; - if (iVersions.length === 0) { continue; } - await this._s3.removeObjects(this.bucket, iVersions.map(versionId => { - return { name: key, versionId }; - })); + for (let i = 0; i < objects.length; i += N) { + const batch = objects.slice(i, i + N); + if (batch.length === 0) { continue; } + await this._s3.removeObjects(this.bucket, batch); } } + + private async _listObjects(...args: Parameters): Promise { + return new Promise((resolve, reject) => { + const stream = this._s3.listObjects(...args); + const results: minio.BucketItem[] = [] + stream + .on('error', reject) + .on('end', () => { + resolve(results); + }) + .on('data', data => { + results.push(data); + }); + }) + } } From 5e26ec82d4b878a03e1c0229f75ef0e41345eeb2 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 12 Nov 2024 23:16:10 +0000 Subject: [PATCH 09/60] Adds a streaming API to MinIOExternalStorage --- app/server/lib/MinIOExternalStorage.ts | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index a123219d99..1437bb3782 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -4,6 +4,7 @@ import {ExternalStorage} from 'app/server/lib/ExternalStorage'; import {IncomingMessage} from 'http'; import * as fse from 'fs-extra'; import * as minio from 'minio'; +import * as stream from 'node:stream'; // The minio-js v8.0.0 typings are sometimes incorrect. Here are some workarounds. interface MinIOClient extends @@ -86,18 +87,21 @@ export class MinIOExternalStorage implements ExternalStorage { } } - public async upload(key: string, fname: string, metadata?: ObjMetadata) { - const stream = fse.createReadStream(fname); + public async uploadStream(key: string, inStream: stream.Readable, metadata?: ObjMetadata) { const result = await this._s3.putObject( - this.bucket, key, stream, undefined, + this.bucket, key, inStream, undefined, metadata ? {Metadata: toExternalMetadata(metadata)} : undefined ); // Empirically VersionId is available in result for buckets with versioning enabled. return result.versionId || null; } - public async download(key: string, fname: string, snapshotId?: string) { - const stream = fse.createWriteStream(fname); + public async upload(key: string, fname: string, metadata?: ObjMetadata) { + const filestream = fse.createReadStream(fname); + return this.uploadStream(key, filestream, metadata); + } + + public async downloadStream(key: string, outStream: stream.Writable, snapshotId?: string ) { const request = await this._s3.getObject( this.bucket, key, snapshotId ? {versionId: snapshotId} : {} @@ -114,12 +118,17 @@ export class MinIOExternalStorage implements ExternalStorage { return new Promise((resolve, reject) => { request .on('error', reject) // handle errors on the read stream - .pipe(stream) + .pipe(outStream) .on('error', reject) // handle errors on the write stream .on('finish', () => resolve(downloadedSnapshotId)); }); } + public async download(key: string, fname: string, snapshotId?: string) { + const fileStream = fse.createWriteStream(fname); + return this.downloadStream(key, fileStream, snapshotId); + } + public async remove(key: string, snapshotIds?: string[]) { if (snapshotIds) { await this._deleteVersions(key, snapshotIds); From 20a1a7992e29b3194daf97bd34814791841697ac Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 15 Nov 2024 16:57:55 +0000 Subject: [PATCH 10/60] Adds prototype MinIO attachment store --- app/server/lib/AttachmentStore.ts | 59 +++++++++++++++++++++++++++++-- app/server/lib/ExternalStorage.ts | 21 +++++++++++ app/server/lib/ICreate.ts | 21 ++++++++--- app/server/lib/coreCreator.ts | 20 +++++++++++ 4 files changed, 115 insertions(+), 6 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 6db222edd2..6770a274b6 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -1,10 +1,12 @@ import * as path from "path"; import * as fse from "fs-extra"; +import { MinIOExternalStorage } from "./MinIOExternalStorage"; +import { joinKeySegments } from "./ExternalStorage"; +import * as stream from "node:stream"; export type DocPoolId = string; type FileId = string; - /** * Provides access to external storage, specifically for storing attachments. * @@ -35,6 +37,60 @@ export interface IAttachmentStore { close(): Promise; } +// TODO - Can make this generic if *Stream methods become part of an interface. +export class MinIOAttachmentStore implements IAttachmentStore { + constructor( + public id: string, + private _storage: MinIOExternalStorage, + private _prefixParts: string[] + ) { + + } + + public exists(docPoolId: string, fileId: string): Promise { + return this._storage.exists(this._getKey(docPoolId, fileId)); + } + + public async upload(docPoolId: string, fileId: string, fileData: Buffer): Promise { + await this._storage.uploadStream(this._getKey(docPoolId, fileId), stream.Readable.from(fileData)); + } + + public async download(docPoolId: string, fileId: string): Promise { + // Need to read a stream into a buffer - this is a bit dirty, but does the job. + const chunks: Buffer[] = []; + let buffer: Buffer | undefined = undefined; + const readStream = new stream.PassThrough(); + const readComplete = new Promise((resolve, reject) => { + readStream.on("data", (data) => chunks.push(data)); + readStream.on("end", () => { buffer = Buffer.concat(chunks); resolve(); }); + readStream.on("error", (err) => { reject(err); }); + }); + await this._storage.downloadStream(this._getKey(docPoolId, fileId), readStream); + // Shouldn't need to wait for PassThrough stream to finish, but doing it in case it somehow finishes later than + // downloadStream resolves. + await readComplete; + + // If an error happens, it's thrown by the above `await`. This should safely be a buffer. + return buffer!; + } + + public async removePool(docPoolId: string): Promise { + await this._storage.removeAllWithPrefix(this._getPoolPrefix(docPoolId)); + } + + public async close(): Promise { + await this._storage.close(); + } + + private _getPoolPrefix(docPoolId: string): string { + return joinKeySegments([...this._prefixParts, docPoolId]); + } + + private _getKey(docPoolId: string, fileId: string): string { + return joinKeySegments([this._getPoolPrefix(docPoolId), fileId]); + } +} + export class FilesystemAttachmentStore implements IAttachmentStore { constructor(public readonly id: string, private _rootFolderPath: string) { } @@ -65,5 +121,4 @@ export class FilesystemAttachmentStore implements IAttachmentStore { private _createPath(docPoolId: DocPoolId, fileId: FileId = ""): string { return path.join(this._rootFolderPath, docPoolId, fileId); } - } diff --git a/app/server/lib/ExternalStorage.ts b/app/server/lib/ExternalStorage.ts index 2caeff49ea..d0840ad96a 100644 --- a/app/server/lib/ExternalStorage.ts +++ b/app/server/lib/ExternalStorage.ts @@ -389,6 +389,27 @@ export interface ExternalStorageSettings { export type ExternalStorageCreator = (purpose: ExternalStorageSettings["purpose"], extraPrefix: string) => ExternalStorage | undefined; +function stripTrailingSlash(text: string): string { + return text.endsWith("/") ? text.slice(0, -1) : text; +} + +function stripLeadingSlash(text: string): string { + return text[0] === "/" ? text.slice(1) : text; +} + +export function joinKeySegments(keySegments: string[]): string { + if (keySegments.length < 1) { + return ""; + } + const firstPart = keySegments[0]; + const remainingParts = keySegments.slice(1); + const strippedParts = [ + stripTrailingSlash(firstPart), + ...remainingParts.map(stripTrailingSlash).map(stripLeadingSlash) + ]; + return strippedParts.join("/"); +} + /** * The storage mapping we use for our SaaS. A reasonable default, but relies * on appropriate lifecycle rules being set up in the bucket. diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 77d2a8a225..35420c8361 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -15,10 +15,11 @@ import {createSandbox, SpawnFn} from 'app/server/lib/NSandbox'; import {SqliteVariant} from 'app/server/lib/SqliteCommon'; import {ITelemetry} from 'app/server/lib/Telemetry'; import {IDocStorageManager} from './IDocStorageManager'; -import { Comm } from "./Comm"; -import { IDocWorkerMap } from "./DocWorkerMap"; -import { HostedStorageManager, HostedStorageOptions } from "./HostedStorageManager"; -import { DocStorageManager } from "./DocStorageManager"; +import {Comm} from "./Comm"; +import {IDocWorkerMap} from "./DocWorkerMap"; +import {HostedStorageManager, HostedStorageOptions} from "./HostedStorageManager"; +import {DocStorageManager} from "./DocStorageManager"; +import {IAttachmentStore} from "./AttachmentStore"; // In the past, the session secret was used as an additional // protection passed on to expressjs-session for security when @@ -89,6 +90,7 @@ export interface ICreate { // static page. getExtraHeadHtml?(): string; getStorageOptions?(name: string): ICreateStorageOptions|undefined; + getAttachmentStoreBackends(): ICreateAttachmentStoreOptions[]; getSqliteVariant?(): SqliteVariant; getSandboxVariants?(): Record; @@ -127,6 +129,13 @@ export interface ICreateTelemetryOptions { create(dbManager: HomeDBManager, gristConfig: GristServer): ITelemetry|undefined; } +export interface ICreateAttachmentStoreOptions { + name: string; + check(): boolean; + checkBackend?(): Promise; + create(): IAttachmentStore|undefined; +} + /** * This function returns a `create` object that defines various core * aspects of a Grist installation, such as what kind of billing or @@ -153,6 +162,7 @@ export function makeSimpleCreator(opts: { getLoginSystem?: () => Promise, createHostedDocStorageManager?: HostedDocStorageManagerCreator, createLocalDocStorageManager?: LocalDocStorageManagerCreator, + attachmentStoreBackends?: ICreateAttachmentStoreOptions[], }): ICreate { const {deploymentType, sessionSecret, storage, notifier, billing, auditLogger, telemetry} = opts; return { @@ -223,6 +233,9 @@ export function makeSimpleCreator(opts: { getStorageOptions(name: string) { return storage?.find(s => s.name === name); }, + getAttachmentStoreBackends(): ICreateAttachmentStoreOptions[] { + return opts.attachmentStoreBackends ?? []; + }, getSqliteVariant: opts.getSqliteVariant, getSandboxVariants: opts.getSandboxVariants, createInstallAdmin: opts.createInstallAdmin || (async (dbManager) => new SimpleInstallAdmin(dbManager)), diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index c3d777c3fc..f2673eacd6 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -3,6 +3,8 @@ import { checkMinIOBucket, checkMinIOExternalStorage, configureMinIOExternalStorage } from 'app/server/lib/configureMinIOExternalStorage'; import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; +import { MinIOAttachmentStore } from "./AttachmentStore"; +import { MinIOExternalStorage } from "./MinIOExternalStorage"; export const makeCoreCreator = () => makeSimpleCreator({ deploymentType: 'core', @@ -21,6 +23,24 @@ export const makeCoreCreator = () => makeSimpleCreator({ create: configureGristAuditLogger, }, ], + attachmentStoreBackends: [ + { + name: 'minio', + check: () => checkMinIOExternalStorage() !== undefined, + checkBackend: () => checkMinIOBucket(), + create: () => { + const options = checkMinIOExternalStorage(); + if (!options) { + return undefined; + } + return new MinIOAttachmentStore( + "minio", + new MinIOExternalStorage(options.bucket, options), + [options?.prefix || "", "attachments"] + ); + } + } + ], telemetry: { create: (dbManager, gristServer) => new Telemetry(dbManager, gristServer), } From 482b5c76c1b26670d86a5408ac12aac7387396ba Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 15 Nov 2024 21:38:38 +0000 Subject: [PATCH 11/60] Makes AttachmentStoreProvider function correctly --- app/server/lib/ActiveDoc.ts | 2 +- app/server/lib/AttachmentStoreProvider.ts | 43 +++++++++++++++++++---- app/server/lib/DocManager.ts | 6 +++- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index c2258575f1..fb21f205ce 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -289,8 +289,8 @@ export class ActiveDoc extends EventEmitter { constructor( docManager: DocManager, docName: string, + externalAttachmentStoreProvider?: IAttachmentStoreProvider, private _options?: ICreateActiveDocOptions, - externalAttachmentStoreProvider?: IAttachmentStoreProvider ) { super(); const {forkId, snapshotId} = parseUrlId(docName); diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 680d9cff24..3abccacbff 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,4 +1,4 @@ -import { FilesystemAttachmentStore, IAttachmentStore } from "./AttachmentStore"; +import { IAttachmentStore } from "./AttachmentStore"; export type AttachmentStoreId = string @@ -9,15 +9,46 @@ export interface IAttachmentStoreProvider { storeExists(id: AttachmentStoreId): Promise; } +export interface IAttachmentStoreBackendFactory { + name: string, + create: () => IAttachmentStore | undefined, +} + +interface IAttachmentStoreDetails { + id: string; + factory: IAttachmentStoreBackendFactory; +} + export class AttachmentStoreProvider implements IAttachmentStoreProvider { - constructor() { + private _storeDetailsById: { [storeId: string]: IAttachmentStoreDetails } = {}; + + constructor( + _backendFactories: IAttachmentStoreBackendFactory[], + _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. + _backendFactories.forEach((factory) => { + const storeId = `${_installationUuid}-${factory.name}`; + this._storeDetailsById[storeId] = { + id: storeId, + factory, + }; + }); + } + + public async getStore(id: AttachmentStoreId): Promise { + const storeDetails = this._storeDetailsById[id]; + if (!storeDetails) { return null; } + return storeDetails.factory.create() || null; } - public async getStore(id: string): Promise { - return new FilesystemAttachmentStore(id, "/home/spoffy/Downloads/Grist/TEST ATTACHMENTS"); + public async storeExists(id: AttachmentStoreId): Promise { + return id in this._storeDetailsById; } - public async storeExists(id: string): Promise { - return true; + public listStoreIds(): string[] { + return Object.keys(this._storeDetailsById); } } diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index 290ed6073f..fd35a00a80 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -617,7 +617,11 @@ export class DocManager extends EventEmitter { // Get URL for document for use with SELF_HYPERLINK(). const docUrls = doc && await this._getDocUrls(doc); // TODO - Use an actual attachment store provider, this is placeholder - const activeDoc = new ActiveDoc(this, docName, {...docUrls, safeMode, doc}, new AttachmentStoreProvider()); + const attachmentStoreProvider = new AttachmentStoreProvider( + this.gristServer.create.getAttachmentStoreBackends(), + (await this.gristServer.getActivations().current()).id, + ); + const activeDoc = new ActiveDoc(this, docName, attachmentStoreProvider, {...docUrls, safeMode, doc}); // Restore the timing mode of the document. activeDoc.isTimingOn = this._inTimingOn.get(docName) || false; return activeDoc; From 59d1633fbdc21aea0389c4e7ece63dfd0c9c7765 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sat, 16 Nov 2024 00:16:09 +0000 Subject: [PATCH 12/60] Enables setting document attachment store --- app/common/DocumentSettings.ts | 1 + app/server/lib/ActiveDoc.ts | 24 +++++++++++++++-------- app/server/lib/AttachmentStoreProvider.ts | 4 ++-- app/server/lib/DocStorage.ts | 1 + app/server/lib/ICreate.ts | 2 +- app/server/lib/coreCreator.ts | 11 +++++++---- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/app/common/DocumentSettings.ts b/app/common/DocumentSettings.ts index 54239cd678..9d4d5d2a88 100644 --- a/app/common/DocumentSettings.ts +++ b/app/common/DocumentSettings.ts @@ -2,6 +2,7 @@ export interface DocumentSettings { locale: string; currency?: string; engine?: EngineCode; + idOfDefaultAttachmentStore?: string; } /** diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index fb21f205ce..9fe8bcb1e0 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -2363,10 +2363,14 @@ export class ActiveDoc extends EventEmitter { dimensions.height = 0; dimensions.width = 0; } + const attachmentStoreId = (await this._getDocumentSettings())?.idOfDefaultAttachmentStore; const addFileResult = await this._attachmentFileManager - .addFile("TEST-FILESYSTEM-STORE", fileData.ext, await readFile(fileData.absPath)); - this._log.info(docSession, "addAttachment: file %s (image %sx%s) %s", addFileResult.fileIdent, - dimensions.width, dimensions.height, addFileResult.isNewFile ? "attached" : "already exists"); + .addFile(attachmentStoreId, fileData.ext, await readFile(fileData.absPath)); + this._log.info( + docSession, "addAttachment: store: '%s', file: '%s' (image %sx%s) %s", + attachmentStoreId ?? 'local document', addFileResult.fileIdent, dimensions.width, dimensions.height, + addFileResult.isNewFile ? "attached" : "already exists" + ); return ['AddRecord', '_grist_Attachments', null, { fileIdent: addFileResult.fileIdent, fileName: fileData.origName, @@ -2837,17 +2841,21 @@ export class ActiveDoc extends EventEmitter { return this._dataEngine; } + private async _getDocumentSettings(): Promise { + const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo'); + const docSettingsString = docInfo?.documentSettings; + return docSettingsString ? safeJsonParse(docSettingsString, undefined) : undefined; + } + private async _makeEngine(): Promise { // Figure out what kind of engine we need for this document. let preferredPythonVersion: '2' | '3' = process.env.PYTHON_VERSION === '3' ? '3' : '2'; // Careful, migrations may not have run on this document and it may not have a // documentSettings column. Failures are treated as lack of an engine preference. - const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo').catch(e => undefined); - const docSettingsString = docInfo?.documentSettings; - if (docSettingsString) { - const docSettings: DocumentSettings|undefined = safeJsonParse(docSettingsString, undefined); - const engine = docSettings?.engine; + const docSettings = await this._getDocumentSettings().catch(e => undefined); + if (docSettings) { + const engine = docSettings.engine; if (engine) { if (engine === 'python2') { preferredPythonVersion = '2'; diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 3abccacbff..c1940a6685 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -11,7 +11,7 @@ export interface IAttachmentStoreProvider { export interface IAttachmentStoreBackendFactory { name: string, - create: () => IAttachmentStore | undefined, + create: (storeId: string) => IAttachmentStore | undefined, } interface IAttachmentStoreDetails { @@ -41,7 +41,7 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { public async getStore(id: AttachmentStoreId): Promise { const storeDetails = this._storeDetailsById[id]; if (!storeDetails) { return null; } - return storeDetails.factory.create() || null; + return storeDetails.factory.create(id) || null; } public async storeExists(id: AttachmentStoreId): Promise { diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 33822b6f31..99fa4f271b 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -1417,6 +1417,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { /** * Delete attachments from _gristsys_Files that have no matching metadata row in _grist_Attachments. + * This leaves any attachment files in any remote attachment stores, which will be cleaned up separately. */ public async removeUnusedAttachments() { const result = await this._getDB().run(` diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 35420c8361..2d7dafe955 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -133,7 +133,7 @@ export interface ICreateAttachmentStoreOptions { name: string; check(): boolean; checkBackend?(): Promise; - create(): IAttachmentStore|undefined; + create(storeId: string): IAttachmentStore|undefined; } /** diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index f2673eacd6..cb56fddae2 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -1,6 +1,9 @@ import { checkGristAuditLogger, configureGristAuditLogger } from 'app/server/lib/configureGristAuditLogger'; -import { checkMinIOBucket, checkMinIOExternalStorage, - configureMinIOExternalStorage } from 'app/server/lib/configureMinIOExternalStorage'; +import { + checkMinIOBucket, + checkMinIOExternalStorage, + configureMinIOExternalStorage +} from 'app/server/lib/configureMinIOExternalStorage'; import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; import { MinIOAttachmentStore } from "./AttachmentStore"; @@ -28,13 +31,13 @@ export const makeCoreCreator = () => makeSimpleCreator({ name: 'minio', check: () => checkMinIOExternalStorage() !== undefined, checkBackend: () => checkMinIOBucket(), - create: () => { + create: (storeId: string) => { const options = checkMinIOExternalStorage(); if (!options) { return undefined; } return new MinIOAttachmentStore( - "minio", + storeId, new MinIOExternalStorage(options.bucket, options), [options?.prefix || "", "attachments"] ); From c1058da03c116921e33f314eb2f757ee083204bf Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 18 Nov 2024 23:08:43 +0000 Subject: [PATCH 13/60] Makes store creation error if not setup Avoids returning `undefined`, which simplifies a lot of the parent logic. --- app/server/lib/AttachmentStore.ts | 7 +++++++ app/server/lib/AttachmentStoreProvider.ts | 12 +++++++++--- app/server/lib/ICreate.ts | 3 ++- app/server/lib/coreCreator.ts | 4 ++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 6770a274b6..019c440047 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -37,6 +37,13 @@ export interface IAttachmentStore { close(): Promise; } +export class AttachmentStoreCreationError extends Error { + constructor(storeBackend: string, storeId: string, context?: string) { + const formattedContext = context ? `: ${context}` : ""; + super(`Unable to create ${storeBackend} store '${storeId}'` + formattedContext); + } +} + // TODO - Can make this generic if *Stream methods become part of an interface. export class MinIOAttachmentStore implements IAttachmentStore { constructor( diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index c1940a6685..2f6c0b091c 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -3,15 +3,17 @@ import { IAttachmentStore } from "./AttachmentStore"; export type AttachmentStoreId = string export interface IAttachmentStoreProvider { - // Returns the store associated with the given id + // Returns the store associated with the given id, returning null if no store with that id exists. getStore(id: AttachmentStoreId): Promise + getAllStores(): Promise; + storeExists(id: AttachmentStoreId): Promise; } export interface IAttachmentStoreBackendFactory { name: string, - create: (storeId: string) => IAttachmentStore | undefined, + create: (storeId: string) => IAttachmentStore, } interface IAttachmentStoreDetails { @@ -41,7 +43,11 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { public async getStore(id: AttachmentStoreId): Promise { const storeDetails = this._storeDetailsById[id]; if (!storeDetails) { return null; } - return storeDetails.factory.create(id) || null; + return storeDetails.factory.create(id); + } + + public async getAllStores(): Promise { + return Object.values(this._storeDetailsById).map(storeDetails => storeDetails.factory.create(storeDetails.id)); } public async storeExists(id: AttachmentStoreId): Promise { diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 2d7dafe955..34ad822b2a 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -133,7 +133,8 @@ export interface ICreateAttachmentStoreOptions { name: string; check(): boolean; checkBackend?(): Promise; - create(storeId: string): IAttachmentStore|undefined; + // Avoid undefined returns - `check()` should be called to ensure creation is valid. + create(storeId: string): IAttachmentStore; } /** diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index cb56fddae2..73b7c86581 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -6,7 +6,7 @@ import { } from 'app/server/lib/configureMinIOExternalStorage'; import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; -import { MinIOAttachmentStore } from "./AttachmentStore"; +import { AttachmentStoreCreationError, MinIOAttachmentStore } from "./AttachmentStore"; import { MinIOExternalStorage } from "./MinIOExternalStorage"; export const makeCoreCreator = () => makeSimpleCreator({ @@ -34,7 +34,7 @@ export const makeCoreCreator = () => makeSimpleCreator({ create: (storeId: string) => { const options = checkMinIOExternalStorage(); if (!options) { - return undefined; + throw new AttachmentStoreCreationError('minio', storeId, 'MinIO storage not configured'); } return new MinIOAttachmentStore( storeId, From 74102d038153e430c0ff21da7522f5edabbe1d00 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 19 Nov 2024 21:46:46 +0000 Subject: [PATCH 14/60] Refactors DocInfo to be reusable Helps document and prevent programmer errors by adding a utility function to turn it into a docId. --- app/server/lib/AttachmentFileManager.ts | 14 ++++---------- app/server/lib/AttachmentStore.ts | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index c5b8423a49..3b35f94907 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -2,7 +2,8 @@ import { DocStorage } from "app/server/lib/DocStorage"; import { checksumFileStream } from "app/server/lib/checksumFile"; import { Readable } from "node:stream"; import { - DocPoolId, IAttachmentStore + AttachmentStoreDocInfo, + DocPoolId, getDocPoolIdFromDocInfo, IAttachmentStore } from "app/server/lib/AttachmentStore"; import { AttachmentStoreId, @@ -34,13 +35,6 @@ export class StoreNotAvailableError extends Error { } } -// Minimum required info from a document -// Compatible with Document entity for ease of use -interface DocInfo { - id: string; - trunkId: string | null | undefined; -} - /** * Provides management of a specific document's attachments. */ @@ -56,9 +50,9 @@ export class AttachmentFileManager implements IAttachmentFileManager { private _docStorage: DocStorage, // TODO - Pull these into a "StoreAccess" module-private type private _storeProvider: IAttachmentStoreProvider | undefined, - _docInfo: DocInfo | undefined, + _docInfo: AttachmentStoreDocInfo | undefined, ) { - this._docPoolId = _docInfo?.trunkId ?? _docInfo?.id ?? null; + this._docPoolId = _docInfo ? getDocPoolIdFromDocInfo(_docInfo) : null; } public async addFile( diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 019c440047..f11af30020 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -7,6 +7,26 @@ import * as stream from "node:stream"; export type DocPoolId = string; type FileId = string; + +// Minimum required info from a document +// Compatible with Document entity for ease of use +export interface AttachmentStoreDocInfo { + id: string; + trunkId: string | null | undefined; +} + + +/** + * Gets the correct pool id for a given document, given the document's id and trunk id. + * Avoids other areas of the codebase having to understand how documents are mapped to pools. + * @param {string} docId - Document's ID + * @param {string | null} trunkId - Document's Trunk ID (if it's a fork) + * @returns {string} - ID of the pool the attachments will be stored in. + */ +export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string { + return docInfo.trunkId ?? docInfo.id; +} + /** * Provides access to external storage, specifically for storing attachments. * From 66401335edcf27851bf08425a6c0984d6939d165 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 19 Nov 2024 22:53:08 +0000 Subject: [PATCH 15/60] Fixes pool deletion not working for MinIO --- app/server/lib/MinIOExternalStorage.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index 1437bb3782..34ef9be359 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -138,7 +138,7 @@ export class MinIOExternalStorage implements ExternalStorage { } public async removeAllWithPrefix(prefix: string) { - const objects = await this._listObjects(this.bucket, prefix, false, { IncludeVersion: true }); + const objects = await this._listObjects(this.bucket, prefix, true, { IncludeVersion: true }); const objectsToDelete = objects.filter(o => o.name !== undefined).map(o => ({ name: o.name!, versionId: (o as any).versionId as (string | undefined), @@ -218,7 +218,7 @@ export class MinIOExternalStorage implements ExternalStorage { private async _listObjects(...args: Parameters): Promise { return new Promise((resolve, reject) => { const stream = this._s3.listObjects(...args); - const results: minio.BucketItem[] = [] + const results: minio.BucketItem[] = []; stream .on('error', reject) .on('end', () => { @@ -227,6 +227,6 @@ export class MinIOExternalStorage implements ExternalStorage { .on('data', data => { results.push(data); }); - }) + }); } } From 85ba845784fb5551e4c0910fdebafe5de98af381 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 19 Nov 2024 22:53:39 +0000 Subject: [PATCH 16/60] Adds lifecycle management to external attachments --- app/server/generateInitialDocSql.ts | 10 ++++++---- app/server/lib/DocApi.ts | 18 +++++++++++++++--- app/server/lib/DocManager.ts | 12 ++++-------- app/server/lib/FlexServer.ts | 17 ++++++++++++++--- app/server/utils/gristify.ts | 3 ++- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/app/server/generateInitialDocSql.ts b/app/server/generateInitialDocSql.ts index 75fead2c66..0c5ae42f3d 100644 --- a/app/server/generateInitialDocSql.ts +++ b/app/server/generateInitialDocSql.ts @@ -1,4 +1,5 @@ import { ActiveDoc } from 'app/server/lib/ActiveDoc'; +import { AttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; import { create } from 'app/server/lib/create'; import { DocManager } from 'app/server/lib/DocManager'; import { makeExceptionalDocSession } from 'app/server/lib/DocSession'; @@ -33,10 +34,11 @@ export async function main(baseName: string) { await fse.remove(fname); } const docManager = new DocManager(storageManager, pluginManager, null as any, { - create, - getAuditLogger() { return createDummyAuditLogger(); }, - getTelemetry() { return createDummyTelemetry(); }, - } as any); + create, + getAuditLogger() { return createDummyAuditLogger(); }, + getTelemetry() { return createDummyTelemetry(); }, + } as any, + new AttachmentStoreProvider([], "")); const activeDoc = new ActiveDoc(docManager, baseName); const session = makeExceptionalDocSession('nascent'); await activeDoc.createEmptyDocWithDataEngine(session); diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index be2fce479e..18e1734a2f 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -104,6 +104,8 @@ import * as t from "ts-interface-checker"; import {Checker} from "ts-interface-checker"; import uuidv4 from "uuid/v4"; import { Document } from "app/gen-server/entity/Document"; +import { IAttachmentStoreProvider } from "./AttachmentStoreProvider"; +import { getDocPoolIdFromDocInfo } from "./AttachmentStore"; // Cap on the number of requests that can be outstanding on a single document via the // rest doc api. When this limit is exceeded, incoming requests receive an immediate @@ -178,7 +180,8 @@ export class DocWorkerApi { constructor(private _app: Application, private _docWorker: DocWorker, private _docWorkerMap: IDocWorkerMap, private _docManager: DocManager, - private _dbManager: HomeDBManager, private _grist: GristServer) {} + private _dbManager: HomeDBManager, private _attachmentStoreProvider: IAttachmentStoreProvider, + private _grist: GristServer) {} /** * Adds endpoints for the doc api. @@ -2032,6 +2035,15 @@ export class DocWorkerApi { ...forks.map((fork) => buildUrlId({forkId: fork.id, forkUserId: fork.createdBy!, trunkId: docId})), ]; + if (!forkId) { + // Delete all remote document attachments first, so we can re-attempt deletion if an error is thrown. + const attachmentStores = await this._attachmentStoreProvider.getAllStores(); + log.debug(`Deleting all attachments for ${docId} from ${attachmentStores.length} stores`); + const poolDeletions = attachmentStores.map( + store => store.removePool(getDocPoolIdFromDocInfo({ id: docId, trunkId: null })) + ); + await Promise.all(poolDeletions); + } await Promise.all(docsToDelete.map(docName => this._docManager.deleteDoc(null, docName, true))); // Permanently delete from database. result = await this._dbManager.deleteDocument(scope); @@ -2331,9 +2343,9 @@ export class DocWorkerApi { export function addDocApiRoutes( app: Application, docWorker: DocWorker, docWorkerMap: IDocWorkerMap, docManager: DocManager, dbManager: HomeDBManager, - grist: GristServer + attachmentStoreProvider: IAttachmentStoreProvider, grist: GristServer ) { - const api = new DocWorkerApi(app, docWorker, docWorkerMap, docManager, dbManager, grist); + const api = new DocWorkerApi(app, docWorker, docWorkerMap, docManager, dbManager, attachmentStoreProvider, grist); api.addEndpoints(); } diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index fd35a00a80..f276af125a 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -18,6 +18,7 @@ import {NEW_DOCUMENT_CODE} from 'app/common/UserAPI'; import {HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; import {assertAccess, Authorizer, DocAuthorizer, DummyAuthorizer, isSingleUserMode, RequestWithLogin} from 'app/server/lib/Authorizer'; +import {IAttachmentStoreProvider} from "app/server/lib/AttachmentStoreProvider"; import {Client} from 'app/server/lib/Client'; import {makeExceptionalDocSession, makeOptDocSession, OptDocSession} from 'app/server/lib/DocSession'; import * as docUtils from 'app/server/lib/docUtils'; @@ -32,7 +33,6 @@ import {PluginManager} from './PluginManager'; import {getFileUploadInfo, globalUploadSet, makeAccessId, UploadInfo} from './uploads'; import merge = require('lodash/merge'); import noop = require('lodash/noop'); -import { AttachmentStoreProvider } from "./AttachmentStoreProvider"; // A TTL in milliseconds to use for material that can easily be recomputed / refetched // but is a bit of a burden under heavy traffic. @@ -63,7 +63,8 @@ export class DocManager extends EventEmitter { public readonly storageManager: IDocStorageManager, public readonly pluginManager: PluginManager|null, private _homeDbManager: HomeDBManager|null, - public gristServer: GristServer + public gristServer: GristServer, + private _attachmentStoreProvider: IAttachmentStoreProvider, ) { super(); } @@ -616,12 +617,7 @@ export class DocManager extends EventEmitter { const doc = await this._getDoc(docSession, docName); // Get URL for document for use with SELF_HYPERLINK(). const docUrls = doc && await this._getDocUrls(doc); - // TODO - Use an actual attachment store provider, this is placeholder - const attachmentStoreProvider = new AttachmentStoreProvider( - this.gristServer.create.getAttachmentStoreBackends(), - (await this.gristServer.getActivations().current()).id, - ); - const activeDoc = new ActiveDoc(this, docName, attachmentStoreProvider, {...docUrls, safeMode, doc}); + const activeDoc = new ActiveDoc(this, docName, this._attachmentStoreProvider, {...docUrls, safeMode, doc}); // Restore the timing mode of the document. activeDoc.isTimingOn = this._inTimingOn.get(docName) || false; return activeDoc; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index adef4754ce..7b89ec11e8 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -86,6 +86,7 @@ import * as path from 'path'; import * as serveStatic from "serve-static"; import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; import {IGristCoreConfig} from "app/server/lib/configCore"; +import { AttachmentStoreProvider, IAttachmentStoreProvider } from "./AttachmentStoreProvider"; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: @@ -141,6 +142,7 @@ export class FlexServer implements GristServer { private _billing: IBilling; private _installAdmin: InstallAdmin; private _instanceRoot: string; + private _attachmentStoreProvider: IAttachmentStoreProvider; private _docManager: DocManager; private _docWorker: DocWorker; private _hosts: Hosts; @@ -1361,8 +1363,16 @@ export class FlexServer implements GristServer { } const pluginManager = await this._addPluginManager(); - this._docManager = this._docManager || new DocManager(this._storageManager, pluginManager, - this._dbManager, this); + // TODO - Validity checks on the backends. + this._attachmentStoreProvider = this._attachmentStoreProvider || new AttachmentStoreProvider( + this.create.getAttachmentStoreBackends(), + (await this.getActivations().current()).id, + ); + this._docManager = this._docManager || new DocManager(this._storageManager, + pluginManager, + this._dbManager, + this, + this._attachmentStoreProvider); const docManager = this._docManager; shutdown.addCleanupHandler(null, this._shutdown.bind(this), 25000, 'FlexServer._shutdown'); @@ -1392,7 +1402,8 @@ export class FlexServer implements GristServer { this._addSupportPaths(docAccessMiddleware); if (!isSingleUserMode()) { - addDocApiRoutes(this.app, docWorker, this._docWorkerMap, docManager, this._dbManager, this); + addDocApiRoutes(this.app, docWorker, this._docWorkerMap, docManager, this._dbManager, + this._attachmentStoreProvider, this); } } diff --git a/app/server/utils/gristify.ts b/app/server/utils/gristify.ts index 5a57a5b147..8cf76d6891 100644 --- a/app/server/utils/gristify.ts +++ b/app/server/utils/gristify.ts @@ -5,6 +5,7 @@ import { makeExceptionalDocSession, OptDocSession } from 'app/server/lib/DocSess import { createDummyGristServer } from 'app/server/lib/GristServer'; import { TrivialDocStorageManager } from 'app/server/lib/IDocStorageManager'; import { DBMetadata, quoteIdent, SQLiteDB } from 'app/server/lib/SQLiteDB'; +import { AttachmentStoreProvider } from "../lib/AttachmentStoreProvider"; /** * A utility class for modifying a SQLite file to be viewed/edited with Grist. @@ -52,7 +53,7 @@ export class Gristifier { // Open the file as an empty Grist document, creating Grist metadata // tables. const docManager = new DocManager( - new TrivialDocStorageManager(), null, null, createDummyGristServer() + new TrivialDocStorageManager(), null, null, createDummyGristServer(), new AttachmentStoreProvider([], "") ); const activeDoc = new ActiveDoc(docManager, this._filename); const docSession = makeExceptionalDocSession('system'); From 3902f8260caab933798269c0b21eb9bf1669775f Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 22 Nov 2024 15:49:53 +0000 Subject: [PATCH 17/60] Adds attachment upload/download logging --- app/server/lib/AttachmentFileManager.ts | 21 +++++++++++++++------ app/server/lib/AttachmentStoreProvider.ts | 6 +++++- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 3b35f94907..edd995dae5 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -3,12 +3,12 @@ import { checksumFileStream } from "app/server/lib/checksumFile"; import { Readable } from "node:stream"; import { AttachmentStoreDocInfo, - DocPoolId, getDocPoolIdFromDocInfo, IAttachmentStore + DocPoolId, + getDocPoolIdFromDocInfo, + IAttachmentStore } from "app/server/lib/AttachmentStore"; -import { - AttachmentStoreId, - IAttachmentStoreProvider -} from "app/server/lib/AttachmentStoreProvider"; +import { AttachmentStoreId, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; +import log from 'app/server/lib/log'; export interface IAttachmentFileManager { addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; @@ -62,25 +62,34 @@ export class AttachmentFileManager implements IAttachmentFileManager { ): Promise { const fileIdent = await this._getFileIdentifier(fileExtension, Readable.from(fileData)); if (storeId === undefined) { + log.info(`AttachmentFileManager adding ${fileIdent} to document storage`); return this._addFileToLocalStorage(fileIdent, fileData); } const store = await this._getStore(storeId); if (!store) { throw new StoreNotAvailableError(storeId); } + log.info(`AttachmentFileManager adding ${fileIdent} to attachment store '${store?.id}', pool ${this._docPoolId}`); return this._addFileToAttachmentStore(store, fileIdent, fileData); } public async getFileData(fileIdent: string): Promise { const fileInfo = await this._docStorage.getFileInfo(fileIdent); - if (!fileInfo) { return null; } + if (!fileInfo) { + log.info(`AttachmentFileManager file ${fileIdent} does not exist`); + return null; + } if (!fileInfo.storageId) { + log.info(`AttachmentFileManager retrieving ${fileIdent} from document storage`); return fileInfo.data; } const store = await this._getStore(fileInfo.storageId); if (!store) { throw new StoreNotAvailableError(fileInfo.storageId); } + log.info( + `AttachmentFileManager retrieving ${fileIdent} from attachment store '${store?.id}', pool ${this._docPoolId}` + ); return this._getFileDataFromAttachmentStore(store, fileIdent); } diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 2f6c0b091c..8866a6ecfd 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,4 +1,5 @@ -import { IAttachmentStore } from "./AttachmentStore"; +import { IAttachmentStore } from "app/server/lib/AttachmentStore"; +import log from 'app/server/lib/log'; export type AttachmentStoreId = string @@ -38,6 +39,9 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { factory, }; }); + + const storeIds = Object.values(this._storeDetailsById).map(storeDetails => storeDetails.id); + log.info(`AttachmentStoreProvider initialised with stores: ${storeIds}`); } public async getStore(id: AttachmentStoreId): Promise { From 4085c06c17b07244d7d0d4e67c9875acece4be47 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 22 Nov 2024 16:23:20 +0000 Subject: [PATCH 18/60] Fixes test build errors --- app/server/generateInitialDocSql.ts | 6 +++--- app/server/lib/DocManager.ts | 2 +- app/server/lib/FlexServer.ts | 4 ++-- app/server/utils/gristify.ts | 2 +- test/server/docTools.ts | 8 ++++++-- test/server/lib/HostedStorageManager.ts | 10 +++++++--- 6 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/server/generateInitialDocSql.ts b/app/server/generateInitialDocSql.ts index 0c5ae42f3d..7dbbf2fa3f 100644 --- a/app/server/generateInitialDocSql.ts +++ b/app/server/generateInitialDocSql.ts @@ -33,12 +33,12 @@ export async function main(baseName: string) { if (await fse.pathExists(fname)) { await fse.remove(fname); } - const docManager = new DocManager(storageManager, pluginManager, null as any, { + const docManager = new DocManager(storageManager, pluginManager, null as any, new AttachmentStoreProvider([], ""), { create, getAuditLogger() { return createDummyAuditLogger(); }, getTelemetry() { return createDummyTelemetry(); }, - } as any, - new AttachmentStoreProvider([], "")); + } as any + ); const activeDoc = new ActiveDoc(docManager, baseName); const session = makeExceptionalDocSession('nascent'); await activeDoc.createEmptyDocWithDataEngine(session); diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index f276af125a..18e784db65 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -63,8 +63,8 @@ export class DocManager extends EventEmitter { public readonly storageManager: IDocStorageManager, public readonly pluginManager: PluginManager|null, private _homeDbManager: HomeDBManager|null, - public gristServer: GristServer, private _attachmentStoreProvider: IAttachmentStoreProvider, + public gristServer: GristServer, ) { super(); } diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 7b89ec11e8..e135f59d36 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -1371,8 +1371,8 @@ export class FlexServer implements GristServer { this._docManager = this._docManager || new DocManager(this._storageManager, pluginManager, this._dbManager, - this, - this._attachmentStoreProvider); + this._attachmentStoreProvider, + this); const docManager = this._docManager; shutdown.addCleanupHandler(null, this._shutdown.bind(this), 25000, 'FlexServer._shutdown'); diff --git a/app/server/utils/gristify.ts b/app/server/utils/gristify.ts index 8cf76d6891..80f8fb7bfc 100644 --- a/app/server/utils/gristify.ts +++ b/app/server/utils/gristify.ts @@ -53,7 +53,7 @@ export class Gristifier { // Open the file as an empty Grist document, creating Grist metadata // tables. const docManager = new DocManager( - new TrivialDocStorageManager(), null, null, createDummyGristServer(), new AttachmentStoreProvider([], "") + 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 72043ff8d4..3eb69537f6 100644 --- a/test/server/docTools.ts +++ b/test/server/docTools.ts @@ -17,6 +17,7 @@ import {tmpdir} from 'os'; import * as path from 'path'; import * as tmp from 'tmp'; import {create} from "app/server/lib/create"; +import { AttachmentStoreProvider, IAttachmentStoreProvider } from "../../app/server/lib/AttachmentStoreProvider"; tmp.setGracefulCleanup(); @@ -135,15 +136,18 @@ export function createDocTools(options: {persistAcrossCases?: boolean, export async function createDocManager( options: {tmpDir?: string, pluginManager?: PluginManager, storageManager?: IDocStorageManager, - server?: GristServer} = {}): Promise { + server?: GristServer, + attachmentStoreProvider?: IAttachmentStoreProvider, + } = {}): 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 store = getDocWorkerMap(); const internalPermitStore = store.getPermitStore('1'); const externalPermitStore = store.getPermitStore('2'); - return new DocManager(docStorageManager, pluginManager, null, options.server || { + return new DocManager(docStorageManager, pluginManager, null, attachmentStoreProvider, options.server || { ...createDummyGristServer(), getPermitStore() { return internalPermitStore; }, getExternalPermitStore() { return externalPermitStore; }, diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index 0678d85873..f477b9f1f4 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -34,6 +34,7 @@ import {createTmpDir, getGlobalPluginManager} from 'test/server/docTools'; import {EnvironmentSnapshot, setTmpLogLevel, useFixtureDoc} from 'test/server/testUtils'; import {waitForIt} from 'test/server/wait'; import uuidv4 from "uuid/v4"; +import { AttachmentStoreProvider, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; bluebird.promisifyAll(RedisClient.prototype); @@ -276,7 +277,8 @@ class TestStore { private _localDirectory: string, private _workerId: string, private _workers: IDocWorkerMap, - private _externalStorageCreate: ExternalStorageCreator) { + private _externalStorageCreate: ExternalStorageCreator, + private _attachmentStoreProvider?: IAttachmentStoreProvider) { } public async run(fn: () => Promise): Promise { @@ -310,6 +312,8 @@ class TestStore { return result; }; + const attachmentStoreProvider = this._attachmentStoreProvider ?? new AttachmentStoreProvider([], "TESTINSTALL"); + const storageManager = new HostedStorageManager(this._localDirectory, this._workerId, false, @@ -318,8 +322,8 @@ class TestStore { externalStorageCreator, options); this.storageManager = storageManager; - this.docManager = new DocManager(storageManager, await getGlobalPluginManager(), - dbManager, { + this.docManager = new DocManager(storageManager, await getGlobalPluginManager(), dbManager, attachmentStoreProvider, + { ...createDummyGristServer(), getStorageManager() { return storageManager; }, }); From 65bc7a9a6c8f70e376bb868a329f839f5c659bf5 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 22 Nov 2024 19:09:44 +0000 Subject: [PATCH 19/60] Misc. cleanup for external attachments. --- app/server/lib/ActiveDoc.ts | 2 +- app/server/lib/AttachmentFileManager.ts | 10 ++++++---- app/server/lib/AttachmentStore.ts | 4 ++++ app/server/lib/DocApi.ts | 3 ++- app/server/lib/coreCreator.ts | 1 + 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 9fe8bcb1e0..e312c3c04b 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -137,12 +137,12 @@ import {ActionHistory} from './ActionHistory'; import {ActionHistoryImpl} from './ActionHistoryImpl'; import {ActiveDocImport, FileImportOptions} from './ActiveDocImport'; import {AttachmentFileManager} from "./AttachmentFileManager"; +import {IAttachmentStoreProvider} from "./AttachmentStoreProvider"; import {DocClients} from './DocClients'; import {DocPluginManager} from './DocPluginManager'; import {DocSession, makeExceptionalDocSession, OptDocSession} from './DocSession'; import {createAttachmentsIndex, DocStorage, REMOVE_UNUSED_ATTACHMENTS_DELAY} from './DocStorage'; import {expandQuery, getFormulaErrorForExpandQuery} from './ExpandedQuery'; -import {IAttachmentStoreProvider} from "./AttachmentStoreProvider"; import {GranularAccess, GranularAccessForBundle} from './GranularAccess'; import {OnDemandActions} from './OnDemandActions'; import {getPubSubPrefix} from './serverUtils'; diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index edd995dae5..199f7e0642 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -39,7 +39,9 @@ export class StoreNotAvailableError extends Error { * Provides management of a specific document's attachments. */ export class AttachmentFileManager implements IAttachmentFileManager { + // _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; /** * @param _docStorage - Storage of this manager's document. @@ -48,10 +50,10 @@ export class AttachmentFileManager implements IAttachmentFileManager { */ constructor( private _docStorage: DocStorage, - // TODO - Pull these into a "StoreAccess" module-private type private _storeProvider: IAttachmentStoreProvider | undefined, _docInfo: AttachmentStoreDocInfo | undefined, ) { + this._docName = _docStorage.docName; this._docPoolId = _docInfo ? getDocPoolIdFromDocInfo(_docInfo) : null; } @@ -62,7 +64,7 @@ export class AttachmentFileManager implements IAttachmentFileManager { ): Promise { const fileIdent = await this._getFileIdentifier(fileExtension, Readable.from(fileData)); if (storeId === undefined) { - log.info(`AttachmentFileManager adding ${fileIdent} to document storage`); + log.info(`AttachmentFileManager adding ${fileIdent} to local document storage in '${this._docName}'`); return this._addFileToLocalStorage(fileIdent, fileData); } const store = await this._getStore(storeId); @@ -76,11 +78,11 @@ export class AttachmentFileManager implements IAttachmentFileManager { public async getFileData(fileIdent: string): Promise { const fileInfo = await this._docStorage.getFileInfo(fileIdent); if (!fileInfo) { - log.info(`AttachmentFileManager file ${fileIdent} does not exist`); + log.info(`AttachmentFileManager file ${fileIdent} does not exist in doc '${this._docName}'`); return null; } if (!fileInfo.storageId) { - log.info(`AttachmentFileManager retrieving ${fileIdent} from document storage`); + log.info(`AttachmentFileManager retrieving ${fileIdent} from document storage in ${this._docName}`); return fileInfo.data; } const store = await this._getStore(fileInfo.storageId); diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index f11af30020..35a0268058 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -19,6 +19,9 @@ export interface AttachmentStoreDocInfo { /** * Gets the correct pool id for a given document, given the document's id and trunk id. * Avoids other areas of the codebase having to understand how documents are mapped to pools. + * This is a key security point - documents which share a pool can access each others' attachments. + * Therefore documents with different security domains (e.g from different teams) need to be sure not to share + * a pool. * @param {string} docId - Document's ID * @param {string | null} trunkId - Document's Trunk ID (if it's a fork) * @returns {string} - ID of the pool the attachments will be stored in. @@ -65,6 +68,7 @@ export class AttachmentStoreCreationError extends Error { } // TODO - Can make this generic if *Stream methods become part of an interface. +// TODO - Put this into another file. export class MinIOAttachmentStore implements IAttachmentStore { constructor( public id: string, diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 18e1734a2f..7db2dd97fb 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -2036,7 +2036,8 @@ export class DocWorkerApi { buildUrlId({forkId: fork.id, forkUserId: fork.createdBy!, trunkId: docId})), ]; if (!forkId) { - // Delete all remote document attachments first, so we can re-attempt deletion if an error is thrown. + // Delete all remote document attachments before the doc itself. + // This way we can re-attempt deletion if an error is thrown. const attachmentStores = await this._attachmentStoreProvider.getAllStores(); log.debug(`Deleting all attachments for ${docId} from ${attachmentStores.length} stores`); const poolDeletions = attachmentStores.map( diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index 73b7c86581..f89e8bfaa9 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -31,6 +31,7 @@ export const makeCoreCreator = () => makeSimpleCreator({ name: 'minio', check: () => checkMinIOExternalStorage() !== undefined, checkBackend: () => checkMinIOBucket(), + // TODO - Move this to another file create: (storeId: string) => { const options = checkMinIOExternalStorage(); if (!options) { From ce5282fce3c8be22ba320ee0454c836095ca3d44 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 25 Nov 2024 22:40:52 +0000 Subject: [PATCH 20/60] Fixes import ordering issues --- app/server/lib/ActiveDoc.ts | 2 +- app/server/lib/AttachmentFileManager.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index e312c3c04b..112c2c02f3 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -126,6 +126,7 @@ import assert from 'assert'; import {Mutex} from 'async-mutex'; import * as bluebird from 'bluebird'; import {EventEmitter} from 'events'; +import { readFile } from 'fs-extra'; import {IMessage, MsgType} from 'grain-rpc'; import imageSize from 'image-size'; import * as moment from 'moment-timezone'; @@ -155,7 +156,6 @@ import remove = require('lodash/remove'); import sum = require('lodash/sum'); import without = require('lodash/without'); import zipObject = require('lodash/zipObject'); -import { readFile } from "fs-extra"; bluebird.promisifyAll(tmp); diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 199f7e0642..d35f4ec5c4 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -1,6 +1,3 @@ -import { DocStorage } from "app/server/lib/DocStorage"; -import { checksumFileStream } from "app/server/lib/checksumFile"; -import { Readable } from "node:stream"; import { AttachmentStoreDocInfo, DocPoolId, @@ -8,7 +5,10 @@ import { IAttachmentStore } from "app/server/lib/AttachmentStore"; import { AttachmentStoreId, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; -import log from 'app/server/lib/log'; +import { checksumFileStream } from "app/server/lib/checksumFile"; +import { DocStorage } from "app/server/lib/DocStorage"; +import log from "app/server/lib/log"; +import { Readable } from "node:stream"; export interface IAttachmentFileManager { addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; @@ -22,7 +22,7 @@ export interface AddFileResult { export class StoresNotConfiguredError extends Error { constructor() { - super('Attempted to access a file store, but AttachmentFileManager was initialised without store access'); + super('Attempted to access a file store, but AttachmentFileManager was initialized without store access'); } } From e53f6ebaab65e93f89196e73819421f79c3bfc68 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 25 Nov 2024 22:55:58 +0000 Subject: [PATCH 21/60] Improves documentation of AttachmentFileManager --- app/server/lib/AttachmentFileManager.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index d35f4ec5c4..5d78090324 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -36,7 +36,11 @@ export class StoreNotAvailableError extends Error { } /** - * Provides management of a specific document's attachments. + * Instantiated on a per-document to basis to provide a document with access to its attachments. + * Handles attachment uploading / fetching, as well as trying to ensure consistency with the local document database, + * which tracks attachments and where they're stored. + * + * This class should prevent the document code from having to worry about accessing the underlying stores. */ export class AttachmentFileManager implements IAttachmentFileManager { // _docPoolId is a critical point for security. Documents with a common pool id can access each others' attachments. From 819e0d694254ff6ffd1e9ac5e3484b99b71a39f6 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 00:31:11 +0000 Subject: [PATCH 22/60] Switches AttachmentFileManager to structured logging --- app/server/lib/AttachmentFileManager.ts | 39 ++++++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 5d78090324..c9177b97e1 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -8,6 +8,7 @@ import { AttachmentStoreId, IAttachmentStoreProvider } from "app/server/lib/Atta import { checksumFileStream } from "app/server/lib/checksumFile"; import { DocStorage } from "app/server/lib/DocStorage"; import log from "app/server/lib/log"; +import { LogMethods } from "app/server/lib/LogMethods"; import { Readable } from "node:stream"; export interface IAttachmentFileManager { @@ -35,6 +36,12 @@ export class StoreNotAvailableError extends Error { } } + +interface AttachmentFileManagerLogInfo { + fileIdent?: string; + storeId?: string; +} + /** * Instantiated on a per-document to basis to provide a document with access to its attachments. * Handles attachment uploading / fetching, as well as trying to ensure consistency with the local document database, @@ -46,6 +53,10 @@ export class AttachmentFileManager implements IAttachmentFileManager { // _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; + private _log = new LogMethods( + "AttachmentFileManager ", + (logInfo: AttachmentFileManagerLogInfo) => this._getLogMeta(logInfo) + ); /** * @param _docStorage - Storage of this manager's document. @@ -67,35 +78,37 @@ export class AttachmentFileManager implements IAttachmentFileManager { fileData: Buffer ): Promise { const fileIdent = await this._getFileIdentifier(fileExtension, Readable.from(fileData)); + this._log.info({ fileIdent, storeId }, `adding file to ${storeId ? "external" : "document"} storage`); if (storeId === undefined) { - log.info(`AttachmentFileManager adding ${fileIdent} to local document storage in '${this._docName}'`); return this._addFileToLocalStorage(fileIdent, fileData); } 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); } - log.info(`AttachmentFileManager adding ${fileIdent} to attachment store '${store?.id}', pool ${this._docPoolId}`); return this._addFileToAttachmentStore(store, fileIdent, fileData); } public async getFileData(fileIdent: string): Promise { + this._log.debug({ fileIdent }, "retrieving file data"); const fileInfo = await this._docStorage.getFileInfo(fileIdent); if (!fileInfo) { - log.info(`AttachmentFileManager file ${fileIdent} does not exist in doc '${this._docName}'`); + this._log.warn({ fileIdent }, "cannot find file metadata in document"); return null; } + this._log.debug( + { fileIdent, storeId: fileInfo.storageId }, + `fetching attachment from ${fileInfo.storageId ? "external" : "document "} storage` + ); if (!fileInfo.storageId) { - log.info(`AttachmentFileManager retrieving ${fileIdent} from document storage in ${this._docName}`); return 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); } - log.info( - `AttachmentFileManager retrieving ${fileIdent} from attachment store '${store?.id}', pool ${this._docPoolId}` - ); return this._getFileDataFromAttachmentStore(store, fileIdent); } @@ -127,7 +140,9 @@ export class AttachmentFileManager implements IAttachmentFileManager { return `${checksum}${fileExtension}`; } - private async _addFileToAttachmentStore(store: IAttachmentStore, fileIdent: string, fileData: Buffer): Promise { + 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. @@ -154,4 +169,12 @@ export class AttachmentFileManager implements IAttachmentFileManager { private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { return await store.download(this._getDocPoolId(), fileIdent); } + + private _getLogMeta(logInfo?: AttachmentFileManagerLogInfo): log.ILogMeta { + return { + docName: this._docName, + docPoolId: this._docPoolId, + ...logInfo, + }; + } } From 52b7e24703afae1502e9b0e0f6b7036ee0f61e29 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 00:33:58 +0000 Subject: [PATCH 23/60] Tidies up imports --- app/server/lib/AttachmentStore.ts | 6 +++--- app/server/lib/FlexServer.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 35a0268058..1e39fac85f 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -1,8 +1,8 @@ -import * as path from "path"; +import { joinKeySegments } from "app/server/lib/ExternalStorage"; +import { MinIOExternalStorage } from "app/server/lib/MinIOExternalStorage"; import * as fse from "fs-extra"; -import { MinIOExternalStorage } from "./MinIOExternalStorage"; -import { joinKeySegments } from "./ExternalStorage"; import * as stream from "node:stream"; +import * as path from "path"; export type DocPoolId = string; type FileId = string; diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index e135f59d36..81240cbacf 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -26,12 +26,15 @@ import {createSandbox} from 'app/server/lib/ActiveDoc'; import {attachAppEndpoint} from 'app/server/lib/AppEndpoint'; import {appSettings} from 'app/server/lib/AppSettings'; import {attachEarlyEndpoints} from 'app/server/lib/attachEarlyEndpoints'; +import {AttachmentStoreProvider, IAttachmentStoreProvider} from "app/server/lib/AttachmentStoreProvider"; import {IAuditLogger} from 'app/server/lib/AuditLogger'; import {addRequestUser, getTransitiveHeaders, getUser, getUserId, isAnonymousUser, isSingleUserMode, redirectToLoginUnconditionally} from 'app/server/lib/Authorizer'; import {redirectToLogin, RequestWithLogin, signInStatusMiddleware} from 'app/server/lib/Authorizer'; import {forceSessionChange} from 'app/server/lib/BrowserSession'; import {Comm} from 'app/server/lib/Comm'; +import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; +import {IGristCoreConfig} from "app/server/lib/configCore"; import {create} from 'app/server/lib/create'; import {addDiscourseConnectEndpoints} from 'app/server/lib/DiscourseConnect'; import {addDocApiRoutes} from 'app/server/lib/DocApi'; @@ -84,9 +87,6 @@ import {AddressInfo} from 'net'; import fetch from 'node-fetch'; import * as path from 'path'; import * as serveStatic from "serve-static"; -import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; -import {IGristCoreConfig} from "app/server/lib/configCore"; -import { AttachmentStoreProvider, IAttachmentStoreProvider } from "./AttachmentStoreProvider"; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: From 79396dcf94b83b69171a8c795685f63525974dd4 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 00:55:53 +0000 Subject: [PATCH 24/60] Adds doc pool description to AttachmentStore --- app/server/lib/AttachmentStore.ts | 33 ++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 1e39fac85f..5d4bb0303b 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -12,18 +12,30 @@ type FileId = string; // Compatible with Document entity for ease of use export interface AttachmentStoreDocInfo { id: string; + // This should NOT be an optional: the programmer should make an explicit choice for this value to prevent + // accidental omission breaking attachments. trunkId: string | null | undefined; } - /** * Gets the correct pool id for a given document, given the document's id and trunk id. + * + * Attachments are stored in a "Document Pool", which is used to manage the attachments' lifecycle. + * Document pools are shared between snapshots and forks, but not between documents. This provides quick forking + * and snapshotting (not having to copy attachments), while avoiding more complex systems like reference tracking. + * + * Generally, the pool id of a document should be its trunk id if available (because it's a fork), + * or the document's id (if it isn't a fork). + * + * This means that attachments need copying to a new pool when a document is copied. * Avoids other areas of the codebase having to understand how documents are mapped to pools. - * This is a key security point - documents which share a pool can access each others' attachments. - * Therefore documents with different security domains (e.g from different teams) need to be sure not to share - * a pool. - * @param {string} docId - Document's ID - * @param {string | null} trunkId - Document's Trunk ID (if it's a fork) + * + * This is a key security measure, as only a specific document and its forks can access its attachments. This helps + * prevent malicious documents being uploaded, which might attempt to access another user's attachments. + * + * Therefore, it is CRITICAL that documents with different security domains (e.g from different teams) do not share a + * document pool. + * @param {AttachmentStoreDocInfo} docInfo - Document details needed to calculate the document pool. * @returns {string} - ID of the pool the attachments will be stored in. */ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string { @@ -33,13 +45,16 @@ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string /** * Provides access to external storage, specifically for storing attachments. * - * Attachments are stored in a "Document Pool", which is used to manage the attachments' lifecycle. - * Typically a document's trunk id should be used for this if available, or the document's id if it isn't. * * This is a general-purpose interface that should abstract over many different storage providers, * so shouldn't have methods which rely on one the features of one specific provider. * - * This is distinct from `ExternalStorage`, in that it's more generic and doesn't support versioning. + * `IAttachmentStore` is distinct from `ExternalStorage` as it's specific to attachments, and can therefore not concern + * itself with some features ExternalStorage has (e.g versioning). This means it can present a more straightforward + * interface for components which need to access attachment files. + * + * A document pool needs specifying for all store operations, which should be calculated with `getDocPoolIdFromDocInfo` + * See {@link getDocPoolIdFromDocInfo} for more details. */ export interface IAttachmentStore { readonly id: string; From d527e591a950ee70c007798355e114dace96579e Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 01:07:44 +0000 Subject: [PATCH 25/60] Improves naming of attachment store backend options --- app/server/lib/AttachmentStoreProvider.ts | 16 ++++++++-------- app/server/lib/FlexServer.ts | 2 +- app/server/lib/ICreate.ts | 8 ++++---- app/server/lib/coreCreator.ts | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 8866a6ecfd..d961e8e6f6 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -12,31 +12,31 @@ export interface IAttachmentStoreProvider { storeExists(id: AttachmentStoreId): Promise; } -export interface IAttachmentStoreBackendFactory { +export interface IAttachmentStoreSpecification { name: string, create: (storeId: string) => IAttachmentStore, } interface IAttachmentStoreDetails { id: string; - factory: IAttachmentStoreBackendFactory; + spec: IAttachmentStoreSpecification; } export class AttachmentStoreProvider implements IAttachmentStoreProvider { private _storeDetailsById: { [storeId: string]: IAttachmentStoreDetails } = {}; constructor( - _backendFactories: IAttachmentStoreBackendFactory[], + _backends: IAttachmentStoreSpecification[], _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. - _backendFactories.forEach((factory) => { - const storeId = `${_installationUuid}-${factory.name}`; + _backends.forEach((storeSpec) => { + const storeId = `${_installationUuid}-${storeSpec.name}`; this._storeDetailsById[storeId] = { id: storeId, - factory, + spec: storeSpec, }; }); @@ -47,11 +47,11 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { public async getStore(id: AttachmentStoreId): Promise { const storeDetails = this._storeDetailsById[id]; if (!storeDetails) { return null; } - return storeDetails.factory.create(id); + return storeDetails.spec.create(id); } public async getAllStores(): Promise { - return Object.values(this._storeDetailsById).map(storeDetails => storeDetails.factory.create(storeDetails.id)); + return Object.values(this._storeDetailsById).map(storeDetails => storeDetails.spec.create(storeDetails.id)); } public async storeExists(id: AttachmentStoreId): Promise { diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 81240cbacf..8c16a65226 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -1365,7 +1365,7 @@ export class FlexServer implements GristServer { const pluginManager = await this._addPluginManager(); // TODO - Validity checks on the backends. this._attachmentStoreProvider = this._attachmentStoreProvider || new AttachmentStoreProvider( - this.create.getAttachmentStoreBackends(), + this.create.getAttachmentStoreOptions(), (await this.getActivations().current()).id, ); this._docManager = this._docManager || new DocManager(this._storageManager, diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index 34ad822b2a..b180b542bb 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -90,7 +90,7 @@ export interface ICreate { // static page. getExtraHeadHtml?(): string; getStorageOptions?(name: string): ICreateStorageOptions|undefined; - getAttachmentStoreBackends(): ICreateAttachmentStoreOptions[]; + getAttachmentStoreOptions(): ICreateAttachmentStoreOptions[]; getSqliteVariant?(): SqliteVariant; getSandboxVariants?(): Record; @@ -163,7 +163,7 @@ export function makeSimpleCreator(opts: { getLoginSystem?: () => Promise, createHostedDocStorageManager?: HostedDocStorageManagerCreator, createLocalDocStorageManager?: LocalDocStorageManagerCreator, - attachmentStoreBackends?: ICreateAttachmentStoreOptions[], + attachmentStoreOptions?: ICreateAttachmentStoreOptions[], }): ICreate { const {deploymentType, sessionSecret, storage, notifier, billing, auditLogger, telemetry} = opts; return { @@ -234,8 +234,8 @@ export function makeSimpleCreator(opts: { getStorageOptions(name: string) { return storage?.find(s => s.name === name); }, - getAttachmentStoreBackends(): ICreateAttachmentStoreOptions[] { - return opts.attachmentStoreBackends ?? []; + getAttachmentStoreOptions(): ICreateAttachmentStoreOptions[] { + return opts.attachmentStoreOptions ?? []; }, getSqliteVariant: opts.getSqliteVariant, getSandboxVariants: opts.getSandboxVariants, diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index f89e8bfaa9..332e7376ac 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -26,7 +26,7 @@ export const makeCoreCreator = () => makeSimpleCreator({ create: configureGristAuditLogger, }, ], - attachmentStoreBackends: [ + attachmentStoreOptions: [ { name: 'minio', check: () => checkMinIOExternalStorage() !== undefined, From c3c82bd663f0135a7d60ddd0ced1223c720a0c88 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 01:23:57 +0000 Subject: [PATCH 26/60] Adds explanation on store concepts --- app/server/lib/AttachmentStore.ts | 7 +++++-- app/server/lib/AttachmentStoreProvider.ts | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 5d4bb0303b..15fbfa2dc6 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -43,8 +43,8 @@ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string } /** - * Provides access to external storage, specifically for storing attachments. - * + * Provides access to external storage, specifically for storing attachments. Each store represents a specific + * location to store attachments, e.g. "/srv/grist/attachments" on the filesystem. * * This is a general-purpose interface that should abstract over many different storage providers, * so shouldn't have methods which rely on one the features of one specific provider. @@ -57,6 +57,9 @@ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string * See {@link getDocPoolIdFromDocInfo} for more details. */ export interface IAttachmentStore { + // Universally unique id, such that no two Grist installations should have the same store ids, if they're for + // different stores. + // This allows for explicit detection of unavailable stores. readonly id: string; // Check if attachment exists in the store. diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index d961e8e6f6..764457b013 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -3,6 +3,16 @@ import log from 'app/server/lib/log'; export type AttachmentStoreId = string +/** + * Creates an {@link IAttachmentStore} from a given store id, if the Grist installation is configured with that store's + * unique id. + * + * Each store represents a specific location to store attachments at, for example a "/attachments" bucket on MinIO, + * or "/srv/grist/attachments" on the filesystem. + * + * Attachments in Grist Documents are accompanied by the id of the store they're in, allowing Grist to store/retrieve + * them as long as that store exists on the document's installation. + */ export interface IAttachmentStoreProvider { // Returns the store associated with the given id, returning null if no store with that id exists. getStore(id: AttachmentStoreId): Promise From dc19e544c7babefc271f1d791850c6f0cddcb9f8 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 26 Nov 2024 01:37:26 +0000 Subject: [PATCH 27/60] Changes MinIOAttachmentStore to ExternalStorageAttachmentStore --- app/server/lib/AttachmentStore.ts | 23 +++++++++++++++-------- app/server/lib/ExternalStorage.ts | 6 ++++++ app/server/lib/MinIOExternalStorage.ts | 4 ++-- app/server/lib/coreCreator.ts | 4 ++-- test/server/lib/HostedStorageManager.ts | 18 +++++++++++++----- 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 15fbfa2dc6..e01bf52172 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -1,5 +1,4 @@ -import { joinKeySegments } from "app/server/lib/ExternalStorage"; -import { MinIOExternalStorage } from "app/server/lib/MinIOExternalStorage"; +import { joinKeySegments, StreamingExternalStorage } from "app/server/lib/ExternalStorage"; import * as fse from "fs-extra"; import * as stream from "node:stream"; import * as path from "path"; @@ -78,6 +77,13 @@ export interface IAttachmentStore { close(): Promise; } +export class InvalidAttachmentExternalStorageError extends Error { + constructor(storeId: string, context?: string) { + const formattedContext = context ? `: ${context}` : ""; + super(`External Storage for store '${storeId}' is invalid` + formattedContext); + } +} + export class AttachmentStoreCreationError extends Error { constructor(storeBackend: string, storeId: string, context?: string) { const formattedContext = context ? `: ${context}` : ""; @@ -85,15 +91,15 @@ export class AttachmentStoreCreationError extends Error { } } -// TODO - Can make this generic if *Stream methods become part of an interface. -// TODO - Put this into another file. -export class MinIOAttachmentStore implements IAttachmentStore { +export class ExternalStorageAttachmentStore implements IAttachmentStore { constructor( public id: string, - private _storage: MinIOExternalStorage, + private _storage: StreamingExternalStorage, private _prefixParts: string[] ) { - + if (_storage.removeAllWithPrefix == undefined) { + throw new InvalidAttachmentExternalStorageError("ExternalStorage does not support removeAllWithPrefix"); + } } public exists(docPoolId: string, fileId: string): Promise { @@ -124,7 +130,8 @@ export class MinIOAttachmentStore implements IAttachmentStore { } public async removePool(docPoolId: string): Promise { - await this._storage.removeAllWithPrefix(this._getPoolPrefix(docPoolId)); + // Null assertion is safe because this should be checked before this class is instantiated. + await this._storage.removeAllWithPrefix!(this._getPoolPrefix(docPoolId)); } public async close(): Promise { diff --git a/app/server/lib/ExternalStorage.ts b/app/server/lib/ExternalStorage.ts index d0840ad96a..fb1238f6be 100644 --- a/app/server/lib/ExternalStorage.ts +++ b/app/server/lib/ExternalStorage.ts @@ -5,6 +5,7 @@ import {createTmpDir} from 'app/server/lib/uploads'; import {delay} from 'bluebird'; import * as fse from 'fs-extra'; import * as path from 'path'; +import stream from "node:stream"; // A special token representing a deleted document, used in places where a // checksum is expected otherwise. @@ -58,6 +59,11 @@ export interface ExternalStorage { close(): Promise; } +export interface StreamingExternalStorage extends ExternalStorage { + uploadStream(key: string, inStream: stream.Readable, metadata?: ObjMetadata): Promise; + downloadStream(key: string, outStream: stream.Writable, snapshotId?: string ): Promise; +} + /** * Convenience wrapper to transform keys for an external store. * E.g. this could convert "" to "v1/.grist" diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index 34ef9be359..ad3eb77537 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -1,6 +1,6 @@ import {ApiError} from 'app/common/ApiError'; import {ObjMetadata, ObjSnapshotWithMetadata, toExternalMetadata, toGristMetadata} from 'app/common/DocSnapshot'; -import {ExternalStorage} from 'app/server/lib/ExternalStorage'; +import {StreamingExternalStorage} from 'app/server/lib/ExternalStorage'; import {IncomingMessage} from 'http'; import * as fse from 'fs-extra'; import * as minio from 'minio'; @@ -44,7 +44,7 @@ type RemoveObjectsResponse = null | undefined | { * An external store implemented using the MinIO client, which * will work with MinIO and other S3-compatible storage. */ -export class MinIOExternalStorage implements ExternalStorage { +export class MinIOExternalStorage implements StreamingExternalStorage { // Specify bucket to use, and optionally the max number of keys to request // in any call to listObjectVersions (used for testing) constructor( diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index 332e7376ac..564995b68b 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -6,7 +6,7 @@ import { } from 'app/server/lib/configureMinIOExternalStorage'; import { makeSimpleCreator } from 'app/server/lib/ICreate'; import { Telemetry } from 'app/server/lib/Telemetry'; -import { AttachmentStoreCreationError, MinIOAttachmentStore } from "./AttachmentStore"; +import { AttachmentStoreCreationError, ExternalStorageAttachmentStore } from "./AttachmentStore"; import { MinIOExternalStorage } from "./MinIOExternalStorage"; export const makeCoreCreator = () => makeSimpleCreator({ @@ -37,7 +37,7 @@ export const makeCoreCreator = () => makeSimpleCreator({ if (!options) { throw new AttachmentStoreCreationError('minio', storeId, 'MinIO storage not configured'); } - return new MinIOAttachmentStore( + return new ExternalStorageAttachmentStore( storeId, new MinIOExternalStorage(options.bucket, options), [options?.prefix || "", "attachments"] diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index f477b9f1f4..4578b49d06 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -17,7 +17,7 @@ import { import {createDummyGristServer} from 'app/server/lib/GristServer'; import { BackupEvent, - backupSqliteDatabase, + backupSqliteDatabase, HostedStorageCallbacks, HostedStorageManager, HostedStorageOptions } from 'app/server/lib/HostedStorageManager'; @@ -1004,15 +1004,23 @@ describe('HostedStorageManager', function() { }); await docWorkerMap.setWorkerAvailability(workerId, true); + class Callbacks implements HostedStorageCallbacks { + private _internalSet(metadata: any) {} + public async setDocsMetadata(metadata: any) { + // Access `this` to ensure the callback is being bound correctly when passed to HostedMetadataManager. + this._internalSet(metadata); + } + public async getDocFeatures(docId: any) { + return undefined; + } + } + defaultParams = [ tmpDir, workerId, false, docWorkerMap, - { - setDocsMetadata: async (metadata) => {}, - getDocFeatures: async (docId) => undefined, - }, + new Callbacks(), externalStorageCreate, ]; }); From 4d7dc11529bc651460834d229831195a4f9c4952 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 27 Nov 2024 03:17:05 +0000 Subject: [PATCH 28/60] Makes attachment store API stream based Simplifies the code, and prevents us locking ourselves into Buffers for attachment store implementations, which would cause trouble when we inevitably move to streams later. --- app/server/lib/AttachmentFileManager.ts | 17 ++++++++++-- app/server/lib/AttachmentStore.ts | 37 ++++++++----------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index c9177b97e1..f322708cd5 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -10,6 +10,7 @@ import { DocStorage } from "app/server/lib/DocStorage"; import log from "app/server/lib/log"; import { LogMethods } from "app/server/lib/LogMethods"; import { Readable } from "node:stream"; +import { Writable } from "stream"; export interface IAttachmentFileManager { addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; @@ -157,7 +158,7 @@ export class AttachmentFileManager implements IAttachmentFileManager { // 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, fileData); + 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 { @@ -167,7 +168,19 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { - return await store.download(this._getDocPoolId(), fileIdent); + // Sub-optimal implementation, as we end up with *at least* two copies in memory one in `buffers`, and one + // produced by `Buffer.concat` at the end. + const buffers: Buffer[] = []; + const writableStream = new Writable({ + write(chunk, encoding, callback) { + buffers.push(chunk); + callback(); + } + }); + + await store.download(this._getDocPoolId(), fileIdent, writableStream); + + return Buffer.concat(buffers); } private _getLogMeta(logInfo?: AttachmentFileManagerLogInfo): log.ILogMeta { diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index e01bf52172..eb69ac4ffa 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -65,10 +65,12 @@ export interface IAttachmentStore { exists(docPoolId: DocPoolId, fileId: FileId): Promise; // Upload attachment to the store. - upload(docPoolId: DocPoolId, fileId: FileId, fileData: Buffer): Promise; + upload(docPoolId: DocPoolId, fileId: FileId, fileData: stream.Readable): Promise; - // Download attachment to an in-memory buffer - download(docPoolId: DocPoolId, fileId: FileId): Promise; + // Download attachment to an in-memory buffer. + // It's preferable to accept an output stream as a parameter, as it simplifies attachment store implementation + // and gives them control over local buffering. + download(docPoolId: DocPoolId, fileId: FileId, outputStream: stream.Writable): Promise; // Remove attachments for all documents in the given document pool. removePool(docPoolId: DocPoolId): Promise; @@ -106,27 +108,12 @@ export class ExternalStorageAttachmentStore implements IAttachmentStore { return this._storage.exists(this._getKey(docPoolId, fileId)); } - public async upload(docPoolId: string, fileId: string, fileData: Buffer): Promise { - await this._storage.uploadStream(this._getKey(docPoolId, fileId), stream.Readable.from(fileData)); + public async upload(docPoolId: string, fileId: string, fileData: stream.Readable): Promise { + await this._storage.uploadStream(this._getKey(docPoolId, fileId), fileData); } - public async download(docPoolId: string, fileId: string): Promise { - // Need to read a stream into a buffer - this is a bit dirty, but does the job. - const chunks: Buffer[] = []; - let buffer: Buffer | undefined = undefined; - const readStream = new stream.PassThrough(); - const readComplete = new Promise((resolve, reject) => { - readStream.on("data", (data) => chunks.push(data)); - readStream.on("end", () => { buffer = Buffer.concat(chunks); resolve(); }); - readStream.on("error", (err) => { reject(err); }); - }); - await this._storage.downloadStream(this._getKey(docPoolId, fileId), readStream); - // Shouldn't need to wait for PassThrough stream to finish, but doing it in case it somehow finishes later than - // downloadStream resolves. - await readComplete; - - // If an error happens, it's thrown by the above `await`. This should safely be a buffer. - return buffer!; + public async download(docPoolId: string, fileId: string, outputStream: stream.Writable): Promise { + await this._storage.downloadStream(this._getKey(docPoolId, fileId), outputStream); } public async removePool(docPoolId: string): Promise { @@ -156,14 +143,14 @@ export class FilesystemAttachmentStore implements IAttachmentStore { .catch(() => false); } - public async upload(docPoolId: DocPoolId, fileId: FileId, fileData: Buffer): Promise { + public async upload(docPoolId: DocPoolId, fileId: FileId, fileData: stream.Readable): Promise { const filePath = this._createPath(docPoolId, fileId); await fse.ensureDir(path.dirname(filePath)); await fse.writeFile(filePath, fileData); } - public async download(docPoolId: DocPoolId, fileId: FileId): Promise { - return fse.readFile(this._createPath(docPoolId, fileId)); + public async download(docPoolId: DocPoolId, fileId: FileId, output: stream.Writable): Promise { + fse.createReadStream(this._createPath(docPoolId, fileId)).pipe(output); } public async removePool(docPoolId: DocPoolId): Promise { From 340c5b3bf88e4255ec294a90a72644ae228263c7 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sun, 1 Dec 2024 22:39:04 +0000 Subject: [PATCH 29/60] Adds tests for FilesystemAttachmentStore.ts --- app/server/lib/AttachmentFileManager.ts | 18 ++---- app/server/lib/AttachmentStore.ts | 11 +++- app/server/utils/MemoryWritableStream.ts | 21 +++++++ test/server/lib/FilesystemAttachmentStore.ts | 66 ++++++++++++++++++++ 4 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 app/server/utils/MemoryWritableStream.ts create mode 100644 test/server/lib/FilesystemAttachmentStore.ts diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index f322708cd5..0ed701a79f 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -9,8 +9,8 @@ import { checksumFileStream } from "app/server/lib/checksumFile"; import { DocStorage } from "app/server/lib/DocStorage"; import log from "app/server/lib/log"; import { LogMethods } from "app/server/lib/LogMethods"; +import { MemoryWritableStream } from "app/server/utils/MemoryWritableStream"; import { Readable } from "node:stream"; -import { Writable } from "stream"; export interface IAttachmentFileManager { addFile(storeId: AttachmentStoreId, fileExtension: string, fileData: Buffer): Promise; @@ -168,19 +168,9 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { - // Sub-optimal implementation, as we end up with *at least* two copies in memory one in `buffers`, and one - // produced by `Buffer.concat` at the end. - const buffers: Buffer[] = []; - const writableStream = new Writable({ - write(chunk, encoding, callback) { - buffers.push(chunk); - callback(); - } - }); - - await store.download(this._getDocPoolId(), fileIdent, writableStream); - - return Buffer.concat(buffers); + const outputStream = new MemoryWritableStream(); + await store.download(this._getDocPoolId(), fileIdent, outputStream); + return outputStream.getBuffer(); } private _getLogMeta(logInfo?: AttachmentFileManagerLogInfo): log.ILogMeta { diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index eb69ac4ffa..ce02a9a5df 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -146,11 +146,18 @@ export class FilesystemAttachmentStore implements IAttachmentStore { public async upload(docPoolId: DocPoolId, fileId: FileId, fileData: stream.Readable): Promise { const filePath = this._createPath(docPoolId, fileId); await fse.ensureDir(path.dirname(filePath)); - await fse.writeFile(filePath, fileData); + const writeStream = fse.createWriteStream(filePath); + await stream.promises.pipeline( + fileData, + writeStream, + ); } public async download(docPoolId: DocPoolId, fileId: FileId, output: stream.Writable): Promise { - fse.createReadStream(this._createPath(docPoolId, fileId)).pipe(output); + await stream.promises.pipeline( + fse.createReadStream(this._createPath(docPoolId, fileId)), + output, + ); } public async removePool(docPoolId: DocPoolId): Promise { diff --git a/app/server/utils/MemoryWritableStream.ts b/app/server/utils/MemoryWritableStream.ts new file mode 100644 index 0000000000..ecd90e2120 --- /dev/null +++ b/app/server/utils/MemoryWritableStream.ts @@ -0,0 +1,21 @@ +import { Writable } from "stream"; + +// Creates a writable stream that can be retrieved as a buffer. +// Sub-optimal implementation, as we end up with *at least* two copies in memory one in `buffers`, and one +// produced by `Buffer.concat` at the end. +export class MemoryWritableStream extends Writable { + private _buffers: Buffer[] = []; + + public getBuffer(): Buffer { + return Buffer.concat(this._buffers); + } + + public _write(chunk: any, encoding: BufferEncoding, callback: (error?: (Error | null)) => void) { + if (typeof(chunk) == "string") { + this._buffers.push(Buffer.from(chunk, encoding)); + } else { + this._buffers.push(chunk); + } + callback(); + } +} diff --git a/test/server/lib/FilesystemAttachmentStore.ts b/test/server/lib/FilesystemAttachmentStore.ts new file mode 100644 index 0000000000..74490d0de9 --- /dev/null +++ b/test/server/lib/FilesystemAttachmentStore.ts @@ -0,0 +1,66 @@ +import { FilesystemAttachmentStore, IAttachmentStore } from "app/server/lib/AttachmentStore"; +import { MemoryWritableStream } from "app/server/utils/MemoryWritableStream"; +import { assert } from "chai"; +import { createTmpDir } from "../docTools"; +import { mkdtemp, pathExists } from 'fs-extra'; +import * as stream from 'node:stream'; +import * as path from 'path'; + +const testingDocPoolId = "1234-5678"; +const testingFileId = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.grist"; +const testingFileContents = "Grist is the best tool ever."; + +function getTestingFileAsBuffer(contents: string = testingFileContents) { + return Buffer.from(contents, 'utf8'); +} + +function getTestingFileAsReadableStream(contents?: string): stream.Readable { + return stream.Readable.from(getTestingFileAsBuffer(contents)); +} + +async function makeTestingFilesystemStore(): Promise<{ store: IAttachmentStore, dir: string }> { + const tempFolder = await createTmpDir(); + const tempDir = await mkdtemp(path.join(tempFolder, 'filesystem-store-test-')); + return { + store: new FilesystemAttachmentStore("test-filesystem-store", tempDir), + dir: tempDir, + }; +} + +describe('FilesystemAttachmentStore', () => { + it('can upload a file', async () => { + const { store, dir } = await makeTestingFilesystemStore(); + await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); + + const exists = await pathExists(path.join(dir, testingDocPoolId, testingFileId)); + assert.isTrue(exists, "uploaded file does not exist on the filesystem"); + }); + + it('can download a file', async () => { + const { store } = await makeTestingFilesystemStore(); + await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); + + const outputBuffer = new MemoryWritableStream(); + await store.download(testingDocPoolId, testingFileId, outputBuffer); + + assert.equal(outputBuffer.getBuffer().toString(), testingFileContents, "file contents do not match"); + }); + + it('can check if a file exists', async () => { + const { store } = await makeTestingFilesystemStore(); + + assert.isFalse(await store.exists(testingDocPoolId, testingFileId)); + await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); + assert.isTrue(await store.exists(testingDocPoolId, testingFileId)); + }); + + it('can remove an entire pool', async () => { + const { store } = await makeTestingFilesystemStore(); + await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); + assert.isTrue(await store.exists(testingDocPoolId, testingFileId)); + + await store.removePool(testingDocPoolId); + + assert.isFalse(await store.exists(testingDocPoolId, testingFileId)); + }); +}); From 6af18181c6542b4f07a89a5ab63a53a0122539d7 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 2 Dec 2024 10:35:23 +0000 Subject: [PATCH 30/60] Makes attachment store creation async, removes check methods --- app/server/lib/AttachmentStoreProvider.ts | 6 ++++-- app/server/lib/ICreate.ts | 5 +---- app/server/lib/coreCreator.ts | 4 +--- test/server/lib/FilesystemAttachmentStore.ts | 6 ++++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 764457b013..6a1d091795 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -24,7 +24,7 @@ export interface IAttachmentStoreProvider { export interface IAttachmentStoreSpecification { name: string, - create: (storeId: string) => IAttachmentStore, + create: (storeId: string) => Promise, } interface IAttachmentStoreDetails { @@ -61,7 +61,9 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { } public async getAllStores(): Promise { - return Object.values(this._storeDetailsById).map(storeDetails => storeDetails.spec.create(storeDetails.id)); + return await Promise.all( + Object.values(this._storeDetailsById).map(storeDetails => storeDetails.spec.create(storeDetails.id)) + ); } public async storeExists(id: AttachmentStoreId): Promise { diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index b180b542bb..b9103cdc67 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -131,10 +131,7 @@ export interface ICreateTelemetryOptions { export interface ICreateAttachmentStoreOptions { name: string; - check(): boolean; - checkBackend?(): Promise; - // Avoid undefined returns - `check()` should be called to ensure creation is valid. - create(storeId: string): IAttachmentStore; + create(storeId: string): Promise; } /** diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index 564995b68b..b8f963affa 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -29,10 +29,8 @@ export const makeCoreCreator = () => makeSimpleCreator({ attachmentStoreOptions: [ { name: 'minio', - check: () => checkMinIOExternalStorage() !== undefined, - checkBackend: () => checkMinIOBucket(), // TODO - Move this to another file - create: (storeId: string) => { + create: async (storeId: string) => { const options = checkMinIOExternalStorage(); if (!options) { throw new AttachmentStoreCreationError('minio', storeId, 'MinIO storage not configured'); diff --git a/test/server/lib/FilesystemAttachmentStore.ts b/test/server/lib/FilesystemAttachmentStore.ts index 74490d0de9..1c0cce6807 100644 --- a/test/server/lib/FilesystemAttachmentStore.ts +++ b/test/server/lib/FilesystemAttachmentStore.ts @@ -18,11 +18,13 @@ function getTestingFileAsReadableStream(contents?: string): stream.Readable { return stream.Readable.from(getTestingFileAsBuffer(contents)); } -async function makeTestingFilesystemStore(): Promise<{ store: IAttachmentStore, dir: string }> { +export async function makeTestingFilesystemStore( + storeId: string = "test-filesystem-store" +): Promise<{ store: IAttachmentStore, dir: string }> { const tempFolder = await createTmpDir(); const tempDir = await mkdtemp(path.join(tempFolder, 'filesystem-store-test-')); return { - store: new FilesystemAttachmentStore("test-filesystem-store", tempDir), + store: new FilesystemAttachmentStore(storeId, tempDir), dir: tempDir, }; } From 8ca7196604a997af03b95cedcccac439c8358e11 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 2 Dec 2024 11:35:22 +0000 Subject: [PATCH 31/60] Adds tests for AttachmentStoreProvider --- test/server/lib/AttachmentStoreProvider.ts | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 test/server/lib/AttachmentStoreProvider.ts diff --git a/test/server/lib/AttachmentStoreProvider.ts b/test/server/lib/AttachmentStoreProvider.ts new file mode 100644 index 0000000000..83eaa5821a --- /dev/null +++ b/test/server/lib/AttachmentStoreProvider.ts @@ -0,0 +1,44 @@ +import { assert } from 'chai'; +import { AttachmentStoreProvider, IAttachmentStoreSpecification } from "app/server/lib/AttachmentStoreProvider"; +import { makeTestingFilesystemStore } from "./FilesystemAttachmentStore"; + +function makeFilesystemStoreSpec(typeId: string): IAttachmentStoreSpecification { + return { + name: typeId, + create: async (storeId) => (await makeTestingFilesystemStore(storeId)).store, + }; +} + +const testInstallationUUID = "FAKE-UUID"; +function expectedStoreId(type: string) { + return `${testInstallationUUID}-${type}`; +} + +describe('AttachmentStoreProvider', () => { + it('constructs stores using the installations UUID and store type', async () => { + const filesystemType1 = makeFilesystemStoreSpec("filesystem1"); + const filesystemType2 = makeFilesystemStoreSpec("filesystem2"); + + const provider = new AttachmentStoreProvider([filesystemType1, filesystemType2], testInstallationUUID); + const allStores = await provider.getAllStores(); + const ids = allStores.map(store => store.id); + + assert.includeMembers(ids, [expectedStoreId("filesystem1"), expectedStoreId("filesystem2")]); + }); + + it("can retrieve a store if it exists", async () => { + const filesystemSpec = makeFilesystemStoreSpec("filesystem"); + const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); + + assert.isNull(await provider.getStore("doesn't exist"), "store shouldn't exist"); + + assert(await provider.getStore(expectedStoreId("filesystem")), "store not present"); + }); + + it("can check if a store exists", async () => { + const filesystemSpec = makeFilesystemStoreSpec("filesystem"); + const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); + + assert(await provider.storeExists(expectedStoreId("filesystem"))); + }); +}); From 2d59deffb3d7bea9916e993599a9ee62311a5bcf Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 2 Dec 2024 12:17:03 +0000 Subject: [PATCH 32/60] Fixes type for FileInfo.storageId --- app/server/lib/DocStorage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 99fa4f271b..07a9a6ee52 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -811,7 +811,7 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { return this.get('SELECT ident, storageId, data FROM _gristsys_Files WHERE ident=?', fileIdent) .then(row => row ? ({ ident: row.ident as string, - storageId: row.storageId as string, + storageId: (row.storageId ?? null) as (string | null), data: row.data as Buffer, }) : null); } @@ -1870,6 +1870,6 @@ function fixDefault(def: string) { // Information on an attached file from _gristsys_files export interface FileInfo { ident: string; - storageId: string; + storageId: string | null; data: Buffer; } From fc9db0c3c96415e9b4ae94449503a48f8a1574af Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 2 Dec 2024 12:17:18 +0000 Subject: [PATCH 33/60] Adds listAllStoreIds to IAttachmentStoreProvider --- app/server/lib/AttachmentStoreProvider.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 6a1d091795..0a0449510e 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -20,6 +20,8 @@ export interface IAttachmentStoreProvider { getAllStores(): Promise; storeExists(id: AttachmentStoreId): Promise; + + listAllStoreIds(): AttachmentStoreId[]; } export interface IAttachmentStoreSpecification { @@ -70,7 +72,7 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { return id in this._storeDetailsById; } - public listStoreIds(): string[] { + public listAllStoreIds(): string[] { return Object.keys(this._storeDetailsById); } } From 72eecf36f0633ae4f0b91366a5dac86f375339ba Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 2 Dec 2024 12:17:35 +0000 Subject: [PATCH 34/60] Fixes typing on AttachmentFileManagerLogInfo --- app/server/lib/AttachmentFileManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 0ed701a79f..03c130cb8a 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -40,7 +40,7 @@ export class StoreNotAvailableError extends Error { interface AttachmentFileManagerLogInfo { fileIdent?: string; - storeId?: string; + storeId?: string | null; } /** From cdcfb0dea68564ee90e88a68bdf7edd4b64ab6f4 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 3 Dec 2024 00:29:32 +0000 Subject: [PATCH 35/60] Make makeTestingFilesystemStoreSpec reuse the directory --- test/server/lib/AttachmentStoreProvider.ts | 19 +++++--------- test/server/lib/FilesystemAttachmentStore.ts | 27 ++++++++++++-------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/test/server/lib/AttachmentStoreProvider.ts b/test/server/lib/AttachmentStoreProvider.ts index 83eaa5821a..8aad3f1fd6 100644 --- a/test/server/lib/AttachmentStoreProvider.ts +++ b/test/server/lib/AttachmentStoreProvider.ts @@ -1,13 +1,6 @@ import { assert } from 'chai'; -import { AttachmentStoreProvider, IAttachmentStoreSpecification } from "app/server/lib/AttachmentStoreProvider"; -import { makeTestingFilesystemStore } from "./FilesystemAttachmentStore"; - -function makeFilesystemStoreSpec(typeId: string): IAttachmentStoreSpecification { - return { - name: typeId, - create: async (storeId) => (await makeTestingFilesystemStore(storeId)).store, - }; -} +import { AttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; +import { makeTestingFilesystemStoreSpec } from "./FilesystemAttachmentStore"; const testInstallationUUID = "FAKE-UUID"; function expectedStoreId(type: string) { @@ -16,8 +9,8 @@ function expectedStoreId(type: string) { describe('AttachmentStoreProvider', () => { it('constructs stores using the installations UUID and store type', async () => { - const filesystemType1 = makeFilesystemStoreSpec("filesystem1"); - const filesystemType2 = makeFilesystemStoreSpec("filesystem2"); + const filesystemType1 = await makeTestingFilesystemStoreSpec("filesystem1"); + const filesystemType2 = await makeTestingFilesystemStoreSpec("filesystem2"); const provider = new AttachmentStoreProvider([filesystemType1, filesystemType2], testInstallationUUID); const allStores = await provider.getAllStores(); @@ -27,7 +20,7 @@ describe('AttachmentStoreProvider', () => { }); it("can retrieve a store if it exists", async () => { - const filesystemSpec = makeFilesystemStoreSpec("filesystem"); + const filesystemSpec = await makeTestingFilesystemStoreSpec("filesystem"); const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); assert.isNull(await provider.getStore("doesn't exist"), "store shouldn't exist"); @@ -36,7 +29,7 @@ describe('AttachmentStoreProvider', () => { }); it("can check if a store exists", async () => { - const filesystemSpec = makeFilesystemStoreSpec("filesystem"); + const filesystemSpec = await makeTestingFilesystemStoreSpec("filesystem"); const provider = new AttachmentStoreProvider([filesystemSpec], testInstallationUUID); assert(await provider.storeExists(expectedStoreId("filesystem"))); diff --git a/test/server/lib/FilesystemAttachmentStore.ts b/test/server/lib/FilesystemAttachmentStore.ts index 1c0cce6807..248d530ad1 100644 --- a/test/server/lib/FilesystemAttachmentStore.ts +++ b/test/server/lib/FilesystemAttachmentStore.ts @@ -1,4 +1,4 @@ -import { FilesystemAttachmentStore, IAttachmentStore } from "app/server/lib/AttachmentStore"; +import { FilesystemAttachmentStore } from "app/server/lib/AttachmentStore"; import { MemoryWritableStream } from "app/server/utils/MemoryWritableStream"; import { assert } from "chai"; import { createTmpDir } from "../docTools"; @@ -18,28 +18,31 @@ function getTestingFileAsReadableStream(contents?: string): stream.Readable { return stream.Readable.from(getTestingFileAsBuffer(contents)); } -export async function makeTestingFilesystemStore( - storeId: string = "test-filesystem-store" -): Promise<{ store: IAttachmentStore, dir: string }> { +export async function makeTestingFilesystemStoreSpec( + name: string = "filesystem" +) { const tempFolder = await createTmpDir(); const tempDir = await mkdtemp(path.join(tempFolder, 'filesystem-store-test-')); return { - store: new FilesystemAttachmentStore(storeId, tempDir), - dir: tempDir, + rootDirectory: tempDir, + name, + create: async (storeId: string) => (new FilesystemAttachmentStore(storeId, tempDir)) }; } describe('FilesystemAttachmentStore', () => { it('can upload a file', async () => { - const { store, dir } = await makeTestingFilesystemStore(); + const spec = await makeTestingFilesystemStoreSpec(); + const store = await spec.create("test-filesystem-store"); await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); - const exists = await pathExists(path.join(dir, testingDocPoolId, testingFileId)); + const exists = await pathExists(path.join(spec.rootDirectory, testingDocPoolId, testingFileId)); assert.isTrue(exists, "uploaded file does not exist on the filesystem"); }); it('can download a file', async () => { - const { store } = await makeTestingFilesystemStore(); + const spec = await makeTestingFilesystemStoreSpec(); + const store = await spec.create("test-filesystem-store"); await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); const outputBuffer = new MemoryWritableStream(); @@ -49,7 +52,8 @@ describe('FilesystemAttachmentStore', () => { }); it('can check if a file exists', async () => { - const { store } = await makeTestingFilesystemStore(); + 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()); @@ -57,7 +61,8 @@ describe('FilesystemAttachmentStore', () => { }); it('can remove an entire pool', async () => { - const { store } = await makeTestingFilesystemStore(); + const spec = await makeTestingFilesystemStoreSpec(); + const store = await spec.create("test-filesystem-store"); await store.upload(testingDocPoolId, testingFileId, getTestingFileAsReadableStream()); assert.isTrue(await store.exists(testingDocPoolId, testingFileId)); From 0bddba203bd92b98171ab2031a1fa5eae28576e8 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 3 Dec 2024 01:20:05 +0000 Subject: [PATCH 36/60] Adds AttachmentFileManager unit tests --- app/server/lib/AttachmentFileManager.ts | 27 ++- test/server/lib/AttachmentFileManager.ts | 213 +++++++++++++++++++++++ 2 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 test/server/lib/AttachmentFileManager.ts diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 03c130cb8a..8021677784 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -37,6 +37,20 @@ export class StoreNotAvailableError extends Error { } } +export class AttachmentRetrievalError extends Error { + public readonly storeId: AttachmentStoreId; + public readonly fileId: string; + + constructor(storeId: AttachmentStoreId, fileId: string, cause?: any) { + const causeError = cause instanceof Error ? cause : undefined; + const causeDescriptor = causeError ? `: ${cause.message}` : ''; + super(`Unable to retrieve '${fileId}' from '${storeId}'${causeDescriptor}`); + this.storeId = storeId; + this.fileId = fileId; + this.cause = causeError; + } +} + interface AttachmentFileManagerLogInfo { fileIdent?: string; @@ -91,6 +105,9 @@ export class AttachmentFileManager implements IAttachmentFileManager { return this._addFileToAttachmentStore(store, fileIdent, fileData); } + // TODO Given we throw if the underlying store can't retrieve the file, maybe this should throw if we're missing + // the fileInfo from doc storage too, instead of returning null? + // That, or we return a result type which might have an error in. public async getFileData(fileIdent: string): Promise { this._log.debug({ fileIdent }, "retrieving file data"); const fileInfo = await this._docStorage.getFileInfo(fileIdent); @@ -168,9 +185,13 @@ export class AttachmentFileManager implements IAttachmentFileManager { } private async _getFileDataFromAttachmentStore(store: IAttachmentStore, fileIdent: string): Promise { - const outputStream = new MemoryWritableStream(); - await store.download(this._getDocPoolId(), fileIdent, outputStream); - return outputStream.getBuffer(); + try { + const outputStream = new MemoryWritableStream(); + await store.download(this._getDocPoolId(), fileIdent, outputStream); + return outputStream.getBuffer(); + } catch(e) { + throw new AttachmentRetrievalError(store.id, fileIdent, e); + } } private _getLogMeta(logInfo?: AttachmentFileManagerLogInfo): log.ILogMeta { diff --git a/test/server/lib/AttachmentFileManager.ts b/test/server/lib/AttachmentFileManager.ts new file mode 100644 index 0000000000..d65b1047a0 --- /dev/null +++ b/test/server/lib/AttachmentFileManager.ts @@ -0,0 +1,213 @@ +import { DocStorage, FileInfo } from "app/server/lib/DocStorage"; +import { + AttachmentFileManager, AttachmentRetrievalError, + StoreNotAvailableError, + StoresNotConfiguredError +} from "app/server/lib/AttachmentFileManager"; +import { AttachmentStoreProvider, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; +import { makeTestingFilesystemStoreSpec } from "./FilesystemAttachmentStore"; +import { assert } from "chai"; +import * as sinon from "sinon"; + +// Minimum features of doc storage that are needed to make AttachmentFileManager work. +type IMinimalDocStorage = Pick + +// Implements the minimal functionality needed for the AttachmentFileManager to work. +class DocStorageFake implements IMinimalDocStorage { + private _files: { [key: string]: FileInfo } = {}; + + constructor(public docName: string) { + } + + public async getFileInfo(fileIdent: string): Promise { + return this._files[fileIdent] ?? null; + } + + // Return value is true if the file was newly added. + public async findOrAttachFile( + fileIdent: string, fileData: Buffer | undefined, storageId?: string | undefined + ): Promise { + if (fileIdent in this._files) { + return false; + } + this._files[fileIdent] = { + ident: fileIdent, + data: fileData ?? Buffer.alloc(0), + storageId: storageId ?? null, + }; + return true; + } +} + +const defaultTestDocName = "1234"; +const defaultTestFileContent = "Some content"; + +function createDocStorageFake(docName: string): DocStorage { + return new DocStorageFake(docName) as unknown as DocStorage; +} + +async function createFakeAttachmentStoreProvider(): Promise { + return new AttachmentStoreProvider( + [await makeTestingFilesystemStoreSpec("filesystem")], + "TEST-INSTALLATION-UUID" + ); +} + +describe("AttachmentFileManager", function() { + let defaultProvider: IAttachmentStoreProvider; + let defaultDocStorageFake: DocStorage; + + beforeEach(async function() { + defaultProvider = await createFakeAttachmentStoreProvider(); + defaultDocStorageFake = createDocStorageFake(defaultTestDocName); + }); + + it("should throw if uses an external store when no document pool id is available", async function () { + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + undefined, + ); + + const storeId = defaultProvider.listAllStoreIds()[0]; + + await assert.isRejected(manager.addFile(storeId, ".txt", Buffer.alloc(0)), StoresNotConfiguredError); + }); + + it("should throw if it tries to add a file to an unavailable store", async function () { + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: "Unimportant", trunkId: null }, + ); + + await assert.isRejected(manager.addFile("BAD STORE ID", ".txt", Buffer.alloc(0)), StoreNotAvailableError); + }); + + it("should throw if it tries to get a file from an unavailable store", async function() { + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: "Unimportant", trunkId: null }, + ); + + const fileId = "123456.png"; + await defaultDocStorageFake.findOrAttachFile(fileId, undefined, "SOME-STORE-ID"); + + await assert.isRejected(manager.getFileData(fileId), StoreNotAvailableError); + }); + + it("should add a file to local document storage if no store id is provided", async function() { + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: "Unimportant", trunkId: null }, + ); + + const result = await manager.addFile(undefined, ".txt", Buffer.from(defaultTestFileContent)); + + // Checking the file is present in the document storage. + const fileInfo = await defaultDocStorageFake.getFileInfo(result.fileIdent); + assert.equal(fileInfo?.data.toString(), "Some content"); + }); + + it("should add a file to an available attachment store", async function() { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const storeId = defaultProvider.listAllStoreIds()[0]; + const result = await manager.addFile(storeId, ".txt", Buffer.from(defaultTestFileContent)); + + const store = await defaultProvider.getStore(storeId); + assert.isTrue(await store!.exists(docId, result.fileIdent), "file does not exist in store"); + }); + + it("should get a file from local storage", async function() { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const result = await manager.addFile(undefined, ".txt", Buffer.from(defaultTestFileContent)); + const fileData = await manager.getFileData(result.fileIdent); + + assert.equal(fileData?.toString(), defaultTestFileContent, "downloaded file contents do not match original file") + }); + + it("should get a file from an attachment store", async function() { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const storeIds = defaultProvider.listAllStoreIds(); + const result = await manager.addFile(storeIds[0], ".txt", Buffer.from(defaultTestFileContent)); + const fileData = await manager.getFileData(result.fileIdent); + const fileInfo = await defaultDocStorageFake.getFileInfo(result.fileIdent); + assert.equal(fileInfo?.storageId, storeIds[0]); + // Ideally this should be null, but the current fake returns an empty buffer. + assert.equal(fileInfo?.data.length, 0); + assert.equal(fileData?.toString(), defaultTestFileContent, "downloaded file contents do not match original file"); + }); + + it("should detect existing files and not upload them", async function () { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const storeId = defaultProvider.listAllStoreIds()[0]; + const addFile = () => manager.addFile(storeId, ".txt", Buffer.from(defaultTestFileContent)); + + const addFileResult1 = await addFile(); + assert.isTrue(addFileResult1.isNewFile); + + // Makes the store's upload method throw an error, so we can detect if it gets called. + const originalGetStore = defaultProvider.getStore.bind(defaultProvider); + sinon.replace(defaultProvider, 'getStore', sinon.fake( + async function(...args: Parameters) { + const store = (await originalGetStore(...args))!; + sinon.replace(store, 'upload', () => { throw new Error("Upload should never be called") }); + return store; + } + )); + + const addFileResult2 = await addFile(); + assert.isFalse(addFileResult2.isNewFile); + }); + + it("should check if an existing file is in the attachment store, and re-upload them if not", async function() { + const docId = "12345"; + const manager = new AttachmentFileManager( + defaultDocStorageFake, + defaultProvider, + { id: docId, trunkId: null }, + ); + + const storeId = defaultProvider.listAllStoreIds()[0]; + const store = (await defaultProvider.getStore(storeId))!; + const addFile = () => manager.addFile(storeId, ".txt", Buffer.from(defaultTestFileContent)); + + const addFileResult = await addFile(); + // This might be overkill, but it works. + await store.removePool(docId); + + await assert.isRejected(manager.getFileData(addFileResult.fileIdent), AttachmentRetrievalError); + + await addFile(); + + const fileData = await manager.getFileData(addFileResult.fileIdent); + assert(fileData); + assert.equal(fileData!.toString(), defaultTestFileContent); + }); +}); From b9d2f8c7f5c2563cf6bb9cb2232a999c3472c686 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 3 Dec 2024 18:40:53 +0000 Subject: [PATCH 37/60] Adds attachment store backend availability checks --- app/server/lib/AttachmentStoreProvider.ts | 19 +++++++++++++++++++ app/server/lib/FlexServer.ts | 16 ++++++++++++---- app/server/lib/ICreate.ts | 1 + app/server/lib/coreCreator.ts | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index 0a0449510e..dcb0fc32d9 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,5 +1,6 @@ import { IAttachmentStore } from "app/server/lib/AttachmentStore"; import log from 'app/server/lib/log'; +import { ICreateAttachmentStoreOptions } from "./ICreate"; export type AttachmentStoreId = string @@ -76,3 +77,21 @@ export class AttachmentStoreProvider implements IAttachmentStoreProvider { return Object.keys(this._storeDetailsById); } } + +async function checkAvailabilityAttachmentStoreOption(option: ICreateAttachmentStoreOptions) { + try { + return await option.isAvailable(); + } catch (error) { + log.error(`Error checking availability of store option '${option}'`, error); + return false; + } +} + +export async function checkAvailabilityAttachmentStoreOptions(options: ICreateAttachmentStoreOptions[]) { + const availability = await Promise.all(options.map(checkAvailabilityAttachmentStoreOption)); + + return { + available: options.filter((option, index) => availability[index]), + unavailable: options.filter((option, index) => !availability[index]), + }; +} diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 8c16a65226..65e33b52d9 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -26,7 +26,9 @@ import {createSandbox} from 'app/server/lib/ActiveDoc'; import {attachAppEndpoint} from 'app/server/lib/AppEndpoint'; import {appSettings} from 'app/server/lib/AppSettings'; import {attachEarlyEndpoints} from 'app/server/lib/attachEarlyEndpoints'; -import {AttachmentStoreProvider, IAttachmentStoreProvider} from "app/server/lib/AttachmentStoreProvider"; +import { + AttachmentStoreProvider, checkAvailabilityAttachmentStoreOptions, IAttachmentStoreProvider +} from "app/server/lib/AttachmentStoreProvider"; import {IAuditLogger} from 'app/server/lib/AuditLogger'; import {addRequestUser, getTransitiveHeaders, getUser, getUserId, isAnonymousUser, isSingleUserMode, redirectToLoginUnconditionally} from 'app/server/lib/Authorizer'; @@ -1363,10 +1365,16 @@ export class FlexServer implements GristServer { } const pluginManager = await this._addPluginManager(); - // TODO - Validity checks on the backends. + + const storeOptions = await checkAvailabilityAttachmentStoreOptions(this.create.getAttachmentStoreOptions()); + log.info("Attachment store backend availability", { + available: storeOptions.available.map(option => option.name), + unavailable: storeOptions.unavailable.map(option => option.name), + }); + this._attachmentStoreProvider = this._attachmentStoreProvider || new AttachmentStoreProvider( - this.create.getAttachmentStoreOptions(), - (await this.getActivations().current()).id, + storeOptions.available, + (await this.getActivations().current()).id, ); this._docManager = this._docManager || new DocManager(this._storageManager, pluginManager, diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index b9103cdc67..c730180fb3 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -131,6 +131,7 @@ export interface ICreateTelemetryOptions { export interface ICreateAttachmentStoreOptions { name: string; + isAvailable(): Promise; create(storeId: string): Promise; } diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index b8f963affa..cd2c823d29 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -29,6 +29,7 @@ export const makeCoreCreator = () => makeSimpleCreator({ attachmentStoreOptions: [ { name: 'minio', + isAvailable: async () => checkMinIOExternalStorage() !== undefined, // TODO - Move this to another file create: async (storeId: string) => { const options = checkMinIOExternalStorage(); From f712ef8d78799fd29b2d5ba920686b9afcf2667f Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 4 Dec 2024 00:05:25 +0000 Subject: [PATCH 38/60] Cleans up linter issues --- app/server/lib/MinIOExternalStorage.ts | 4 ++-- test/server/lib/AttachmentFileManager.ts | 4 ++-- test/server/lib/HostedStorageManager.ts | 18 +++++------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index ad3eb77537..a9f6b91513 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -217,9 +217,9 @@ export class MinIOExternalStorage implements StreamingExternalStorage { private async _listObjects(...args: Parameters): Promise { return new Promise((resolve, reject) => { - const stream = this._s3.listObjects(...args); + const bucketItemStream = this._s3.listObjects(...args); const results: minio.BucketItem[] = []; - stream + bucketItemStream .on('error', reject) .on('end', () => { resolve(results); diff --git a/test/server/lib/AttachmentFileManager.ts b/test/server/lib/AttachmentFileManager.ts index d65b1047a0..a8c58da172 100644 --- a/test/server/lib/AttachmentFileManager.ts +++ b/test/server/lib/AttachmentFileManager.ts @@ -137,7 +137,7 @@ describe("AttachmentFileManager", function() { const result = await manager.addFile(undefined, ".txt", Buffer.from(defaultTestFileContent)); const fileData = await manager.getFileData(result.fileIdent); - assert.equal(fileData?.toString(), defaultTestFileContent, "downloaded file contents do not match original file") + assert.equal(fileData?.toString(), defaultTestFileContent, "downloaded file contents do not match original file"); }); it("should get a file from an attachment store", async function() { @@ -177,7 +177,7 @@ describe("AttachmentFileManager", function() { sinon.replace(defaultProvider, 'getStore', sinon.fake( async function(...args: Parameters) { const store = (await originalGetStore(...args))!; - sinon.replace(store, 'upload', () => { throw new Error("Upload should never be called") }); + sinon.replace(store, 'upload', () => { throw new Error("Upload should never be called"); }); return store; } )); diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index 18e131a553..1eaa0b71e2 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -18,7 +18,7 @@ import { import {createDummyGristServer} from 'app/server/lib/GristServer'; import { BackupEvent, - backupSqliteDatabase, HostedStorageCallbacks, + backupSqliteDatabase, HostedStorageManager, HostedStorageOptions } from 'app/server/lib/HostedStorageManager'; @@ -1004,23 +1004,15 @@ describe('HostedStorageManager', function() { }); await docWorkerMap.setWorkerAvailability(workerId, true); - class Callbacks implements HostedStorageCallbacks { - private _internalSet(metadata: any) {} - public async setDocsMetadata(metadata: any) { - // Access `this` to ensure the callback is being bound correctly when passed to HostedMetadataManager. - this._internalSet(metadata); - } - public async getDocFeatures(docId: any) { - return undefined; - } - } - defaultParams = [ tmpDir, workerId, false, docWorkerMap, - new Callbacks(), + { + setDocsMetadata: async (metadata) => {}, + getDocFeatures: async (docId) => undefined, + }, externalStorageCreate, ]; }); From 364ac083661cc8fa8d31d7e229c86d3c9e30bcb1 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 4 Dec 2024 00:08:57 +0000 Subject: [PATCH 39/60] Fixes findOrAttachFile test --- test/server/lib/DocStorage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/server/lib/DocStorage.js b/test/server/lib/DocStorage.js index 78e9fa825e..0ad4ef5fd8 100644 --- a/test/server/lib/DocStorage.js +++ b/test/server/lib/DocStorage.js @@ -393,14 +393,14 @@ describe('DocStorage', function() { .then(() => testUtils.writeTmpFile("Hello, world!")) .then(tmpPath => docStorage.findOrAttachFile(tmpPath, "hello_world.txt")) .then(result => assert.isTrue(result)) - .then(() => docStorage.getFileData("hello_world.txt")) + .then(() => docStorage.getFileInfo("hello_world.txt")) .then(data => assert.equal(data.toString('utf8'), "Hello, world!")) // If we use the same fileIdent for another file, it should not get attached. .then(() => testUtils.writeTmpFile("Another file")) .then(tmpPath => docStorage.findOrAttachFile(tmpPath, "hello_world.txt")) .then(result => assert.isFalse(result)) - .then(() => docStorage.getFileData("hello_world.txt")) + .then(() => docStorage.getFileInfo("hello_world.txt")) .then(data => assert.equal(data.toString('utf8'), "Hello, world!")); }); }); From ae78bba9f833818e091b93f04ed7b074de23fe82 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 4 Dec 2024 00:15:23 +0000 Subject: [PATCH 40/60] Removes unnecessary log line --- app/server/lib/AttachmentFileManager.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 8021677784..3b884cb7d8 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -109,7 +109,6 @@ export class AttachmentFileManager implements IAttachmentFileManager { // the fileInfo from doc storage too, instead of returning null? // That, or we return a result type which might have an error in. public async getFileData(fileIdent: string): Promise { - this._log.debug({ fileIdent }, "retrieving file data"); const fileInfo = await this._docStorage.getFileInfo(fileIdent); if (!fileInfo) { this._log.warn({ fileIdent }, "cannot find file metadata in document"); From 4730d993a52403794184277af3ab92373b422680 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 4 Dec 2024 00:25:28 +0000 Subject: [PATCH 41/60] Makes _getDocumentSettings error if not found. --- app/server/lib/ActiveDoc.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index a6ef9ef71d..b9b6212d68 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -2356,7 +2356,7 @@ export class ActiveDoc extends EventEmitter { dimensions.height = 0; dimensions.width = 0; } - const attachmentStoreId = (await this._getDocumentSettings())?.idOfDefaultAttachmentStore; + const attachmentStoreId = (await this._getDocumentSettings()).idOfDefaultAttachmentStore; const addFileResult = await this._attachmentFileManager .addFile(attachmentStoreId, fileData.ext, await readFile(fileData.absPath)); this._log.info( @@ -2837,10 +2837,14 @@ export class ActiveDoc extends EventEmitter { return this._dataEngine; } - private async _getDocumentSettings(): Promise { + private async _getDocumentSettings(): Promise { const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo'); const docSettingsString = docInfo?.documentSettings; - return docSettingsString ? safeJsonParse(docSettingsString, undefined) : undefined; + const docSettings = docSettingsString ? safeJsonParse(docSettingsString, undefined) : undefined; + if (docSettings) { + throw new Error("No document settings found"); + } + return docSettings; } private async _makeEngine(): Promise { From c39806f1432af44dfceaa2ad1cf89263dd19cbcc Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 4 Dec 2024 00:33:41 +0000 Subject: [PATCH 42/60] Undoes erroneous storage schema change --- app/server/lib/DocStorage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index b4106a4fbd..88f8086eee 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -72,7 +72,6 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { await db.exec(`CREATE TABLE _gristsys_Files ( id INTEGER PRIMARY KEY, ident TEXT UNIQUE, - storageId TEXT, data BLOB )`); await db.exec(`CREATE TABLE _gristsys_Action ( From 58a101d7d4394707398f5c08c08fc24e7f06fc47 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 6 Dec 2024 21:14:19 +0000 Subject: [PATCH 43/60] Fix getDocumentSettings bad error condition --- app/server/lib/ActiveDoc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index b9b6212d68..191ef5bbee 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -2841,7 +2841,7 @@ export class ActiveDoc extends EventEmitter { const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo'); const docSettingsString = docInfo?.documentSettings; const docSettings = docSettingsString ? safeJsonParse(docSettingsString, undefined) : undefined; - if (docSettings) { + if (!docSettings) { throw new Error("No document settings found"); } return docSettings; From f93948f7f61c330783cb4a7acd65c06d81fa13aa Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sat, 7 Dec 2024 00:14:26 +0000 Subject: [PATCH 44/60] Updates migration tests with additional fixtures --- test/fixtures/docs/BlobMigrationV9.grist | Bin 0 -> 151552 bytes test/fixtures/docs/DefaultValuesV9.grist | Bin 0 -> 151552 bytes test/server/lib/DocStorageMigrations.ts | 14 +++++++------- 3 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/docs/BlobMigrationV9.grist create mode 100644 test/fixtures/docs/DefaultValuesV9.grist diff --git a/test/fixtures/docs/BlobMigrationV9.grist b/test/fixtures/docs/BlobMigrationV9.grist new file mode 100644 index 0000000000000000000000000000000000000000..0878744bf157b5d6226b8c0c655f3c856d8d18d9 GIT binary patch literal 151552 zcmeI53w#qr{=jGRXwpZTLMfph$wm@qJJNV_&f!b|IBXMrs*ciai3GU-+oGxo!@VM z`jd$PKsV99WIHF(up^SC8F8Fm#J&zKkym2l*3w!_k#>8H zO)lPeHoD2_^H#YSGhftttIf-}!%00(x3|D)!`yhUYqL6LRk_VoT(0rc4d`%ILOdWe zh}=lK8<@a_cvKjB9b@U1KPf4y=G1tJV(3t+rAPOo6jZF_;fqeBBIBU>Gs1h}pjFL@ zGKnHJm1<^!JvGvG`LsJwl)005USBy`BxmQ~`oo_A_<|MA(CkZNa&vQ4&5AgQV)*dx zUKzZM-P7q8dnya(87qzC8}`7)@H-dAZB_GtSczivXsX%DM(%XQ#&9hKm%W%vARsQB zcLDi-+P{(*i9(~HTDml1`gyo6IcS?H*y*l;vAlkE)%iDlwb1yov?mN{)}?a3BCTra z8!b`fTBKApIz8TyiqAG|F5Lcvgp)SI=OLK+Yp2CZ z+i-gn5{tFg=}DJ9fR%Z?PB-1qy#ZCV*rXDLN<}SB42EN{6W7|SNZ>4*7eqi9Dfz5r z{tDuBTN{e3HikFnFbWG}v)b{!jGH&XP9Lrxvwco)Xu=3Z#$%>&uYmgwzPN0xa6->4 ztIO)JdNFH{i#GFGcbJ_PtD}L>b|dX6W@y|AhBixT)uK!Zdv~xTHW;^&)N_x??6mpp z?RN)V!lbFGug-&?jM3MGn>BdJAG_z|*&x|573GPp?0Xf{vEefH37 zB#nr*TQ5h7H0c0CwAtiaaGTB-?!==Ccem0nQGqG5EHJKp@d|gAzW7@E;sN00{hH39PB2#E98epT^h}sgYQ<7g& zsx8G4O9~97e!m~bI&|pJh;3Vcd_?#76HjdT%f>Z#tr2ZH95|4-I+<+Vb$Izute#%G>5&=b!DHYqrk3 z#!^3S+BMZvr%jw(qnj}CmD>xR+Wqtcn;#tA*0%oMHoMz%Wkcg98}m&tbzuBx6~GO3ELHQY3N{Oxm!UVi!I9gpwa_}2&2ci+9@_B)o{a%=PA zC6!ZZCsb7%CX}vUQ1tjx{X>iNA8yFM(q4vFSao!tS(r! zW^K;=`JQ=Ad8Qey_VL%v9IsxxHf`0a`Hkau-&A-{+lFydYByf5-+N2pGdCB$cTfI1 zcjv#preJN!?sN>gzqhiy3Z09;L-W&_kKfz%@TQw*H;h+2yO7$azVek3d#@8LeR^g! zLNN+Mq0Tt(os&mOZ#q$tu;hc1r{5oK|JS?YW)89R*r?FGyK8@mt4Biq(oqYxPVMoO z;+LuWh{J?A^_jEO}A+y|=Bp ztG4`l$H5VAEPPGY_{djfU%gr>e&?Y(%9c%fq@eX|tp2l^Pyh7R@=@>2pINZt`!^2! zG<0d@S^Zmu3(w!a^U?DO4=nJVf9c(4uV0@2GZXXU`RNbcoW&qU3K5yxJmfjn8I&`EwZI$&-FunF8@whd+T-i zI|e_lti9=(!KU7+?rF|+nPT4Fl=aumd*p>1ubFKx%sTn^6Fc7hasS`0-*)xt^rj^! z>%kOL?}p>28}>E!esasco^2&bSfy(5ca`x1MX*Biy$8xdG#E z@X7annO!&X{hIS=tbWVYs}ub4rQ6NP)IV*7kLC@g=G|O8xHS6s;ALMdi{3l6u;#J# zD6w#!qL*LubZr|Gv)==>{ zc&BQz|NT=3^On7!PVm2;yYDUW&YEe_8_M6D9F-JRAl+X2ukwC^SKfT$&hv-nWzKkZ z$kW=QqsQ*O-`~7EU%R8%@mGe4ysHJZ=`qv?GgJ2tbw7Xlx>KTg=~qzlE2*r+=YJVD zS#X7W4E69TzoM$8Wq*;@lCNrewpQwDPHtJUd$E6IZp*ToaeFgLD^@N!MBV%12;KO` z{ZX0CO)a-Qqe@nPJ0f96>)c)bxifBTyZog}YTWkXio?5lQVz@Fw#~PmrP8()9sF*S zSNPyMU(3$3Cw2(dZ+$!ElhTI=eJ}5G?YX~aZ&-fjvwJkxzkPV+E>rf8$2?mS-|rEB zI_~)Ke<1ejm$x_nWp(QY_3{r*Cl}`Q8}@p0n$uER^qE@5+)^kVoj$nu&V*h|G-9!S z;i=QJK41T4?4E5uU-kT#%@0JiExTG+@zO)orW0=><5x4$*+V;N+xyox?Romy!4E~< zvR&niRJiTH<-8BMLEXR;#WSq<<(u=Ml5?P@oe>7bNW2@sIVV8cU$T> zgsvI;hA}-aE)TtmzF#iv1fERrq}9vC+Ad|CxzLeq4C`eag6v5&NGwvicj- zf)%LEc<3V$I<$Pmx)=Qk$hX@p-s>&++u!o`Q5D||9ggOk(@#F3jMg5Qy9G7c4P3;?IyLU`Ao?0j1*j&B@1?yGz7E(e!KlpR9Ri zRZ|N!?(JQ79?859lYJxqm|nG~;**b)dluv$Q{AwZPWVtf`ni5RWXn=UEx)tPusr6* zT|Ig>mfp4V{ioiW-0!v5pZ5Lp$d6yX_fthf#%sF|wTTbTTC?dvPt=n`qlVSpmv;m) zbtv}3f}b$=a8UJb}EW>`^Sw$b8@I&NvQ9O=-hfVGX`x( zBemscsHKOK(3u;1qp8=TYb&UnAyLKOP|_=?8F%;IJIqBj7RZhd8Hr|2LnX`5zBdGY z>;utV72>0^6$0h(93-tuM&oy&Ph99s!2r}$>vuiB&_BW4+kbRLFCe_`mDQd0!VllO} zaAe#*bn@Sezm7`OG#0f>gA_D-6AT^P+1)SWiTAco z>t7}s_r(=xQ|hwI>-T>f{e)u3&ug&w`OO8po*L4ptbYHXYIOLqmWnNh>mE(++p40x zD>K_Fo*aI3`Cq@zOG4kx$zRd?y%Rh7PQ7)d;FBB2TsB~5(}9nldAXSSc22LVhqk(p z$v%EzBRYFavubhnkFr$B4m7`K)l(;*?7wG&I6?RBlyx6@clI12KKSsM)597b6V-hB zhTxO<)0_7B#^&88PPqJ+nj)9@=n9lD{qV69SKd3wfQp|HOnFXme2>U~($u!^2Y>$k z<6{rT(D!XR&}YU+wF2$7uQzWWx8e2FD3|*6h9jt;&M&z9(Hh~F{%;7D7A$}Kz)#Cw zeOrXSdTvoqkN@&CZH)Pg3`w5^p*L;y3;o+B;>PWXA56DBg)7U6Cw_UZv~t`vPxQBE_RRAi`L?R$ogrnzv!w|F zwCjnw?P%rS3(7uU`F)=^t{k@&haJ1~`xD5Pdc&uANAA&1{_^9U`4ZIn$edGpbacU| z36{Lq|EV2pKHUH0v#W;BL|;F@_UFeBJjt9NfR0^Zd*q7-*|2YG7c9H~^ycqlO5Yp) zMegdS{_mX^BsC`njy=5N&YBa~%v|?Ipu@rq(c{_PLJJC z`{7GHI`(14O3MEDO5AOHk_01yBIKmZ5;0U!VbfB+Bx0zlwTLSVFziN25- zLNfo47yZKl1b_e#00KY&2mk>f00e*l5C8%|00>;d1YrDs31==O4F~`MAOHk_01yBI zKmZ5;0U!VbfIxr%`~Uyt|3df=4j=#ofB+Bx0zd!=00AHX1b_e#00KbZ(jp+CqL54` zqvFZX}) zXYfVwFO!XIL!7b@>(?rXDaCMEv{}KL%Hm8lxSc*1zE&CDvbsGUXw4wjb|zN=cAyScFN|N}TB-TT0Wi;nu z24|(B(dl5iTVR7#?58D>y?)X{sf@dR8W*zzrjeQF4Pr%3^PTfBE!t*naMq90%aBwBdmX#p5!WnQ)N=wU^bO%?kaZL;iGL;c=OZ=9l!AA znw`PAI5uKZ1{YDFy^M=Ek!)dS%Ex0l@-N80s<~FLxiMuStDq!8Z7i3f00e*l5C8%|00;nqOPBzR z|1aUpg`@!iAOHk_01yBIKmZ5;0U!VbfB+B(5Pf00e*l z5C8&~J^>j2U;23rDFgyQ00;m9AOHk_01yBIKmZ5;0U!Y5e<%Y$00;m9AOHk_01yBI zKmZ5;0U!VbE`0)I{4bOrM)I%aAL1`KfB+Bx0zd!=00AHX1b_e#00KY&2mpaU2?3c< zgm*9yY6BYx$OPn6pbdB*M~U!G0qq+EaO3~INdCF}P5It{3VZ8 z`w^b~e^Gw_A1Z0!3=jYUKmZ5;0U!VbfB+Bx0zd!=0D=E40s}-n5k)1-sAw5EdPt>G znMmkn%uct(GIja@L3go0&>`+e^+eIpU1N6$$@pI$|0f00e*l5C8%|00{hH2=wYD#S7gjflMmWW^i65 zQ&g|S&d_u&v=3U7*pm&Vh_0Fo?QQ7UgAJ6Dz%(u}si(X%Y!nxkm>AO@S)jd)3yU)* zMt26LaDlP4iPFwMB^MYoEm6`L*q;lG&Pj~w4D7=NO0VrDW&?4G1lnF)palKULnOs3 z_$lnBHh~Ki_Yigj1y=owdI&m#I3*dZkg9U#3^4gjY;a4zpU6hH|5RyuMOdQE8ZNpj2aU?_- z{@PXxzOvG&uQ4jC%MFvN^&^yIK?du{2&KpCbkhxt!NO{&T8rfpMOGHIG$Y8N5MDeC z6S}y@%P0~Z_F9!peFz+T1pCz=e2rm44=Tl)YCqj_iC%f+ZZAzM(gnN zE5;p8>TP7~yjGnqQj|O)23b$cjkMcqY;y6&v(ZgXpSQ}znE9gCTWwy(9Zu?Ty1fNX z8|KD)U7OW0tIBPz;z~I`-GB~fCBy?lgUF4vyMYN@h)0F7*D;oE1v@20)tnkHQ4AeQ zwe;v-l!A(oJbclKRAd}9e@1vO9JH!AQ6^EOrc%va8VMs^mruI`)tx(u=k=A7MRIly zZg2QA0HdjJhGt(Hlbf5XYF5N?Z9}M61}|d|v<)VFZT3_a&NEgT$v5nQjp27LjN7W_ z0kIOr=+RWOm5tnq$i{FjE0?|4w?IH#IPU`T|FnN4F%pGFL$!2i#PsuUU2@PiQ?S!r z17msp?5gu``bwJII@%M4H0x41Uy)X|^o^D%^75!f<9YpVw;RZW4qw~DV#9>-3~c56Q{`om97Isur76qEM-* z#fiai40hsLdld8W{h4x_LzHmlu=i(NO*iO}i8^<%cr z=?(1}2t~$Yrg5);`wqUiY^-oX&n&CU>acn-YmbXI^ICV9offO3fzNg$?I~tx+zEy@ zOKR1kObL5;up~AZw~^FykIC$``RtD1#-Ke+%8R~Bv`85c=H7`=;zVIuP%Otj@pMgC z5U!h;Yqj9cZesY}-0CTEy6rw29fFQ{Q_m}5OR$ZuV{9Q&Sb@*swKBN>HCT$=PJ0=T z8nNv0+3mEusmkedn~8RaC8A=ndR+K!f=8%o9vj8>Z!H?0Yzu0uJ(36+6)A^Uv;3f4 zZY%Me^b6e-x}}UQ5}|2RHBS(;mAs`o*!@DM+2E*m2FhFDEQ&V)0b!&RSg^Iwn{zgQ zTuk?hzzT3B}+WD9jC8Bt_#k(kkJo|OCSq1i|p5o@?ecC;M*Po;lDJPROJbx6qANi$m*~#^E|U{xaTv`KChEKH+2?i&Y!f9 z03ja6vkwZ${67jjMTju82N4he0zd!=00AHX1b_e#00KY&2mk>f@Fyl9i*cYtWI)m5 zq%D${qn3*gigpQa7JNi~L|ugp@!RFwW#0JmxV5p{V|&Fc=Se7z&pt?K_Vi_E&=w5{ zdP-!{6_i!_I%d~9P6zNkBkGVwene#Be@(dE8F~0a zn{D`EE`FrWBXcosyVXOUE{0lFtD1)-OB5LyRI@!a)q!iZPmB-{?Lqy+roLSx5`plK z{K7paR5dT`!%ii1%bSW0UV%ru*v-zce&PJ=REgv$c{CSpvZlf9^tr+bNkqM!wuU}= zXS3Mq@OWv5884y;$tqUL`)F0wJXFCx1nxSAiqkrQOsKm(jaFAsJ_QB#4M!kIClUhg zA#jMJq`a9Mt+vnvs8ubh-V#M}GSxbg%}OJg>q+N6le>Dt1xC8Gop$(WTUGaILw3R& zyO6IhtTGiiEum9ZEJuSqbh?UoUD4=tgsPKpotvTgOc5EI(;)=PtINu;h<~1seVkm( z*l@Ko6>{?|JkeDxy^Y&5_32705PDI#I2)#LWi}Fmh@`8WcE*(N z;$9p1qh|L>G)tuXlEkZ35SS)NgN#i~hU7>J+7sW&uk})K|^FzYe;#3e4NE8c0 z!&EIfi4sLx8r9m5^}W4G@_K1=qaClS>ZqfgA-%h%se6=2>BB32a5I7phj_xeOoZVw zo5+k|*1ix~m32N}HNlf}4NNg(ZD)zGvmezin-fnJhc+7ypUtLf&gsd{ z(XT8tU<(+Yr6U%wz!#ZrMn)?oxC`z-aW?hgUWNxp*=4-Gz3i0Y9lh0wD z?PE-{m?rL0UB#CLoZ;4h;$^CxK6O5oT>|kVd;#++x@AiAE*cs|RUdZ^H}ON$9TYb5m1;BCDEf z264^8)SwoPq_c>WfsVxp!67MO(%2s3H>6NmP^>Q~W!LxSDb=bGK_0QI7PthpuP9DU z4OVc@b>OMxj@#2v)ecjt=Bo!t6ywKJE!=HhXJ&9~5=evXbT{R@@ojEnM4>WFf_d!x zhO`H#g|viR0s_mJJG@saxmkMLXtXcC?vSab42)9P$N&1IG>?O(pebq|@nAgkAlHNio^f=P4H@tTKrn!&<4r=^Ll zB%O#S^p8@|S6(3Euy6;#{6Eh#4G01PKmZ5;0U!VbfB+Bx0zd!=00AIyi4h>6h)bamh4Ovp zo*wt}Hbln%()eRY{CAOHk_01yBIKmZ5;0U!VbfWV)F zK#Ej?Bodi4MXT)U@Kg$)`^ZN{e+kx+%dt-2Mf^%-kaSQKmMUb?DcZmr`Ss-g?gc3f z5fg<(c}n}+`Ky>Yj3aH7I@ro!5mrh}!d3$B*{ib7^SXUZkew@pSVfYkNYS$Y#{r>s zd-?SIM#g4y2I&qJu)5e~oB(pPo5e5C4pyUBwdj=gSN*q}E^-E2BjbNbe1C*T|4#X+ z_%Gu($JgUiIDh~U00KY&2mk>f00e*l5C8%|;D3lfu_PKv^pYXb5^4MJSXWl2tkU6a z*Q!#L1v+E##4Z8PrK!`@>a?`1?3_$p#p8{BOiUMrEonGQfuZyS zyv*)V-tCtiNIEPc#F8IDt#e#sGw3e zL0@hRXC*sJSFu}89LW=5E2D{(==NcAl~JiHFW2M$evkbyRvY!*oXE=2Rct@6#nG=4 z;^-NYAyOPYFjAi|MPFH^oM0#{#2eP}q=LZ6%+gg}_BAn}!;|?0gcKvir{t(A)>j$| zb>(;yJADN&OV&iDu5#!!Kh`g!u!#w*iT0s=g|dWPx_U|(Mye|)Hs~koSsr{j!&=M0 z)->em?D$`HKaxKvuZsUU{(<;OvUB(p4j=#ofB+Bx0zd!=00AHX1b_e#_&o?1B{2vO z_)^HxJ`xWaWe3grUdT^lRFKucsJz3fChbB@1?d{aK~@6;^$x4qSwWf?^3x~^vKp91 z@35Mcb0MaJbdACwtAX)6Yn2f00e*l5C8%|00;m9AOHk_z<-Yb8UKsqk0ALe`BC`` z_y-(700;m9AOHk_01yBIKmZ5;0U!VbfWYreAQ8VxK-NztQ%trpbMeOI?B?2u_~ij? zpx$D|8%J`1J@H!tWXV`lXR+fg(^}-`*hI2ZB^O4<|CIbJ!hdi80U!VbfB+Bx z0zd!=00AHX1b_e#00NgB0TCraG8s4i{|3pwx#SWE2?7E@00;m9AOHk_01yBIKmZ5; z0U+?dOdysl14xXPQd85WU&XHf7hQ*BH^-cd`VRkq0|)>CAn;!y(CQUQQL~Jqmd;=@ z)VcLcT25xBIkV23Q%}=sI;XBKEj=?WSFK62m@OG~Y9=EKzfLuy9_yI1GKenO@YI}{ zmZK4h7WiVY)3n)ZbvlY^PoqzZPcPQ2FS^5KxzBDD;WHokFw-{kRQ!!t1;#Olu{m9g zhYKPKR!6;as(}0;S_{}n7RF;PXMf=(SzaFz_@eNK&FYwCu&|0g2|jmv8yRNUMduF#qO5tE|!x($Xo0#)@(?zBP>~jzellC{_@+4U3PkUy@t*eI5hLuiXEwq zI*TPx=6d2Y6T8u|N$2&_=0-cN2%hoeQ5n*%LCD_WbDp4?NyK;2U`InRG$A;r0T_T0ci!IsT=!TY_S#a zHUbxGHKY+B&Fb&g2pqTlxcj&EJ=qou-k>~7W68)cXIpaX>vPPwTQTQqGBnxh`rIsY zdag#3siEuYm^zJ_&dSWH%QiDv8CWOB!epiYS|dO@y?mc-R+#>W?HIatLfjkCa5UVF zusz1V+Y^yZ<0AD$7v-9|)kTpKiK3TIyI6+&1@dAuhO`yj4Hmf>Vz<#FIDd~`300J> z{0#-N{MQN-X+^sfq`o*H(5aAB_8yamVQ;NW^;X7av65FBF7S!*=UleorZ9!yR;Vt} z@B(~^NJZ^G;<%dJGK=CrMe?WQKKW?;0}dbn1b_e#00KY&2mk>f00e*l5C8&yW&(N< zo@ct)*&q>Xf00e*l z5C8%|00;m9AOHmZ_yl?gMQA8xbK(^lCZEHDSHFqm0udS<2x0?b@qheJraX;!*{B6? zt4PNGN09t0`EmRO2M_=PKmZ5;0U!VbfB+Bx0zd!=00AKIry&q6OpfLT_zA+Uj{}79 z!r+Gi?EF8x|Nl>uvEV2W00KY&2mk>f00e*l5C8%|00;nq3nU;y?eG6bf00e*l5C8&~ECI6qAIATeZ1zIJfB+Bx0zd!=00AHX1b_e#00KY& z2(Sdm_@5&0|ATk`L5TnYKmZ5;0U!VbfB+Bx0zd!=00AKIXCa^#GSr1^bu0LPFVkcN literal 0 HcmV?d00001 diff --git a/test/fixtures/docs/DefaultValuesV9.grist b/test/fixtures/docs/DefaultValuesV9.grist new file mode 100644 index 0000000000000000000000000000000000000000..5a20dc73784f80c872145e5c789529b3d08d6645 GIT binary patch literal 151552 zcmeI5Uu+x6eaCkxN+czbEB}#sKF8-ZFNVyseJ5F#Wu4>3ltf94Wm%S}ySPTcu9hQd zLsyA~}1wCSO5LD84?r9~f#Ai*I;(B$3<1VM{yfV>3lOHmX>do7YQ zMS;%DF3II`NyoY1?xOoHY>7L+-^~1epWp1v&hBWqetTJSh`eQ56~&P!g%Lp%h3m2` z2*TU+{~7wfaTMu?Xyb(b7U}=E?>y||+ro?Yf484XBGQkU{Ik;kN10Kzkbrn>jLOSI*DAElSfN6*MooULW;(&VtY~7ZN|mTPN@!l5tDDfi zr&TpWb2LS_tBM+0x}ln7&DajJYe%t{h*Bn&9gvP^AKgyI(o<97TX{e3B13PNR8y~2 z43BGDS}GKMn@X<$R27RDPN5t$hf;Dyt7g#D2{JP?L*>+_j(j{a&R#Xhg{SXo%znYmz@mE{mM)*9?urJ`7S z>t@YTnRcKgrc&1Is;=yX3-4>TMuQY0%+^0Q97~UliTl$bF?FveH&V$SQS22yUTk<2 zs+PvwdPO6<9SR+8b1Q<5XX`hIcn`3@;WvNYR13zI=`}|0Y&g^bPuR?A)+}q~W+85Jp)42kUo6Tix9Im8k6}%8;uRq5*C1BGupPxvNrP`o(bLQBpO3}Ti;Lo;R|78PRMvSd(Sgvk_GT?w%1*02 zgb3qZ91T%@qR(FVS#(xv6=G?syvZ$)3-e6}4LVx7?r*FVZg1opnVL<TzBFF>B^!gXhxYluwT>Zs9aKNj>*qUJpq*_ zdWsW|uJ^~%SFeh1rF%u9OBn+{5B;a%-hT0VI12Of*-R5*CL6%Iq}V%Q5usRfoV!f> zEcG58g_;@Nk=GZGwkXn!w39KnWLJ%Sa&|P^i%zo;*CN_d8bJ*fLI*W|8OU37aFi|6 zba+t~A+&q)O0%2AZJOmhx4-bWjmBqclWNjF&ERFYReM>pHwOmN%GIZA9bMZ+>`^Kj zOHWUWZ(a>;5hdFp)nI$ziTyoW16O%+TWIp|ZjU;~uJKl9lL@~OY8AmQ!oVQq@2uRr zCws_}^25_wzk9zHE1lrekA``0n57i={7I5FeB9fbjbq*8987 zJ=!x2PZNP=3lA`C3xU>w*e|ZxVe^L;yfU3C1WIqiZ_;tn>WGlP23<4m^;GD|wth;C zrO%xc>$Lz|Hd1<%u*Se@N4k*Zx=#UCE3@mE-+Fk1ND~OZQDIZHMoRWtWFUR}+>@_v z*I-|7&l2do`p7f%$Dw4BO|eJV4;&x>0w4eaAOHd&00JNY0w4eaAOHdfmB1i#4DF+2nT5C8!X009sH0T2KI5C8!X00B1t7l%I(=sz4F00JNY0w4eaAOHd&00JOz zzzDoGSf|^T*OOx8_XZ~>E?rVC&uorQT)uQ=X7kF$8D;X~)pS*nK%I1~naaH|o;f>d8>`$oOE%DfvleO#g^uv~0OEk-* z8%b(!(6bubdRzVXhxMXKH^pW; zuh)*ypL|omC8Ae}RhoBqSFX`yVVvh6#(kojRbunv?BUiu zpIcrY=btpXPqAwv6TZ&V*p_}Pp44_YwVGiQ=YmGJg{=)xJ$K7K?o};D&%I6l+yv%= zLpK%N7FL}@f3(cntrSZ*8 zGCe~>d+FkM`SPTysMD7(Ph6Uu8oxAgab{{uRj0Qmwk};!$;C{+aG z<#G@33_@NLUU=^=ce|n(HAP>i+qv_=xX5Zo*>sEVquM;6Youb{?;g!p)-q>&oHbri`ldkt;Q#>;009sH0T2KI5C8!X009sH0TB445m<=UlaUYm zoJn|tz5nl%{!WlSkbXpe;Q#>;009sH0T2KI5C8!X009sH0TB3v6FA%#70yI-ldcje zv85G;BfK#Zi3+E^G&f~fj0(piHvJNIQWoA|@BjOx_XX)c>HPowPq>3H4+KB}1V8`; zKmY_l00ck)1V8`;K)^>J(RV!I&hSV2&WQde0)45z(_R9591!7u|1bSap#N}y00@8p z2!H?xfB*=900@8p2!H?x95@0|Au1%3Z2m8>trKv700@8p2!H?xfB*=900@8p2!H?x z99RPG`~RN^(oYVoi*O7CKmY_l00ck)1V8`;KmY_l00cg*1Y+UcKD+p0E=2OVLn$HQ%bpC;>7H92VU{+;pWW?^Yc`~zLC8tSz3Cz zQhC|7X}V_5)#z8IS6Dp_`Ie&Fq$Oh22SkdlZ5tK3Eg7vwCtFVD;li;L_ixnokJ>(% zXJzJpJ;|LqUY8n9CFbXLOid*|57w)iVGxVj(^2yIjLNm`%*%H(ixwdU)mkIv%$FX{ zUmKbW6_HA#&c^XNN#?TR$hxy+~@Tyypy*}(#-7CJ`f*BI|hx5ncfi@MNn>9^+ z?s$x+pJ=wV*z8J;Umvr_1_D_Ia?JDGL_bSEJw~&f=GT6IMKNlMzHVC1&E}_7({3fd z5M|f9G&=2(jq=KUNauEldXHUr59glg>2s2R+bAZ z#r$G^O2@rtM_p+2vca@_c?Fx3OH5$Hr)Sl`h-}q>t0&n$0Y$ zkbsD1DQk*O)L<@G8^MWc}l2&7MAr-jWD{g&W`@eC zO&$3-d}puP*_>zwcSYZ=s-u}kuuz^+v)Mwtnyv(DWY%njN_aM1*(7=(3m4Q3MchqF8(DX3bKWcAzAtQr7IMuIz;i?`yV3gA^jn);~8KOOK6-`_myY zbvqk3Qpp}s>=iyo~0XOrcbY;}iJlvij@+QvHk*WXH_n&FUbV#!rrvCMl1t=#AvykxIu zkXLRM<&}-)<@3Hmn7y(u$33#=c02)Po`qKH>=2piG;6r8=rs}?cf4*+_VisH#kNgV zqibEu@~-CWbd;2>uO?#Yi3xH4Mt~~oYFjO((V?=G!7fA_IiAQt|VWa!L5c)27?zE?tLPFlFH_l z&8wj;qGUUy8f*_dvA<_);3`jU3r!y0?NP_rHQwrrGT}Euts=tj6^n&oDeq z1ez^8IMDm}us;mxA!hTbfVd5Bnb>MCxk<-Wt0O}GHlS<9y`Bm^+0qH`pR#F8y%u2m zNtwoEuSEvZx6eKK>UItG_4YiWkIv`gY)+5&|G(TP8z>C~KmY_l00ck)1V8`;KmY_l z00cgX1d{Q0g(2aFkotq96Z`kr%D`U_JlF5^WyQax-w<#knH|m!)dyD-nW4R2*7HXR z^()Wt=TZC5`<|JU*7B>%CHCOneKy!SKhgF)s__D~sov||m1>rQ5105`D6{5NYYy9( zq4U|1oFSHFTA{*heJ1U`N;&B_{S~@91${%r-kN&G`%<0_HQ%Su%Yhy*Rl}c6${B80 z$Q%3Ye)(7|oy&=j*V}BTuOID_P1hw*^GjPA(aQ}VIwi-$ed$!}<O$%liGBp3|$sL+f>NYbveTx4X<13x_fbiw>(eBEhT`FwLm5>Di+-- zsjCX9Y?5-vBj~ZQY<=u#EPdvTxPPd#DfyaEx_i^<)nvMMAWsiETR(DyucN5@Uoo(? zF|&&0btmr0u~1uGSubPn(`zL>02ED^B{11~`Y>N()L~V@A(ci~P3z{Tqungy(q6Vv z3v?20To<#m^^=EU>6cy->l)A8LZlP7SMaSp;R#2(S?6W=W!v|K5&jNuzpWE%uDkOa z-JWSK9J}xYsoBxCocw)SyjB{ zE)l&-tkS%@EGwYJ%FOP+_*^V~>Xi8ST#FXlEsb+wA5MqxV(GnF&p`%OKXZ~_lI>ka zZf^M|8$T?2N2@lvwDv@~9i-RSwCnVO>fS#DV#p$`?r3^&W!QMO{_r#W{^wEKEi$cc z)zS=AtJ2N=S|+s8PIa@grP9mD@HhGHdfa?P(SnNxd1GfUmqWV@-i*y!wVP0DE>H?! zVT1PbCt~TzNwHqx4ZG#*8nO8bMxXo5q~}7t-+xw$eFPm>j~)oCpSz9hA8z%tS)+r) zYcFsD;RLXO86WaGQx|OBo(3R$PRiXP#wyz-Mr5kistQp$@ z5x*dI6nlv%^zJ=)J3OAn`~P0gH6Q~45C8!X009sH0T2KI5C8!X0D*%+0Q3KY5l46d z0w4eaAOHd&00JNY0w4eaAOHfr2(bBoMEXFGenx-b009sH0T2KI5C8!X009sH0T2KI z5I7(N1|m^G6eE$S7!?u;cmDr7D#F_Gj)JRVO*ETYm)xyvUbR3;>bt4G zNPUf};s5~<009sH0T2KI5C8!X009sHfnSus=;)vz#zZkmr?q|8=~uh3W8~M8M`^Ms z##wSCIWl_0uQldsNxzjm?9&>SMi2S5&bV5s#pH-jYbZ4;`L&*RwUXD8DWBG0@@R5U zNO*Q#f7VqRzH)TfmwL=i9ojoOy(Y0T2KI5C8!X009sH z0T2KI5C8!XI5-4`#2DQk;7Bt5`MVQ*|36XsnLz*H009sH0T2KI5C8!X009sH0T2KI z5IArIqGD8_UnM~Oib{VeNdG4NK>7~-fCB_T00ck)1V8`;KmY_l00ck)1VG?pCU7Jg z6Vl1#iR89r)~c^)WS3a>kv@N7zN|T>Wgm|4#N)1Ju3XU!`!Id+Pjh=|w{{Jh8Grx( zF^3e@g8&GC00@8p2!H?xfB*=900@A{IKM}pK-%=FDF zR{t6)-~G-H-njeSk#G0;6w?u}6oHl!@d?g~Lqg(+m`X>ZiOhm&9x5(cgcxoaUyu1T z-TD6mLHbAOZ=`pm2cMPzz)uhW0T2KI5C8!X009sH0T2KI5cntvJk0}u+}cw=02p&0 z03_T80Jm-(>Nfu$WX_I!6dt1v5C8!X009sH0T2KI5C8!X009sHfscy-d;gz^{hyF} zFZsP>b~rm!A6!jj;&0L<93TJ!AOHd&00IY*K)wHTES;Ya>!&rNOdgcBEzNdHImc1d zoeD7=yR@b0q)?_GJ7s6)*7CVxUM{T6=f5a-%3FEmTPq>*v(2)z^=IW+`o@%4&-v`I z_w3TDUfb4;dBssmHA8!?MoRa{o?BJB_<7Hmjg`Xfjdq*b6@BH%rs0{@u5z}@Yb}(| z%e1!a{<%}J^vRRr>ZHhi%qr!^?~GS-F*m!Mmov>|`K(r^Y>N5C{F=PFR=Am4 zyCdJo-#IU5DvD84^mWs6GV<*5tyy_KzmVHlF3J;RW@d&eEF#kT_1$=0eiu z18F>SK9IPwvAj(4FN7yA29jA-ylpHIy-KXoyk=K*WiOz`%FOP+_*^V~>Xi8ST#MGC zX}Y-BhtnavSbFAD3aozSWGp>BE!H&-tZ^O9E#D-SO}atXj#jlp6XkZ0p*p>wRuzk0 z*MS(aNUJ-VPQ%gGGRL#^ho6b1r>4Y5Cj)k_TAHD1RYmui*-AUr&B~TaFC(43DhZgv z+jGq(R=%QWdN<*UQVGq-H%{v&q?z5fF}N_pcm}r6(rD$0yoi;~U31 zMz+s{``*gZ%l+oa6|jUm!Gzr!7?TydG-)PV*M2kDY8f@HqHL2TqHXVlU9VjFs#bP( z0_L!+j#eQXRozs|B-p5Ve~tJ52l2)ZK7jxTfB*=900@8p2!H?xfB*=9Ks$l|2Wa!x Ar~m)} literal 0 HcmV?d00001 diff --git a/test/server/lib/DocStorageMigrations.ts b/test/server/lib/DocStorageMigrations.ts index aca14982cf..68f887ca23 100644 --- a/test/server/lib/DocStorageMigrations.ts +++ b/test/server/lib/DocStorageMigrations.ts @@ -39,11 +39,11 @@ describe('DocStorageMigrations', function() { } it('should migrate from v1 correctly', function() { - return testMigration('BlobMigrationV1.grist', 'BlobMigrationV8.grist'); + return testMigration('BlobMigrationV1.grist', 'BlobMigrationV9.grist'); }); it('should migrate from v2 correctly', function() { - return testMigration('BlobMigrationV2.grist', 'BlobMigrationV8.grist'); + return testMigration('BlobMigrationV2.grist', 'BlobMigrationV9.grist'); }); it('should migrate from v3 correctly', async function() { @@ -69,11 +69,11 @@ describe('DocStorageMigrations', function() { // Also do the test to check out the full document against a saved copy. To know if the copy // makes sense, run in test/fixtures/docs: // diff -u <(sqlite3 BlobMigrationV3.grist .dump) <(sqlite3 BlobMigrationV4.grist .dump) - await testMigration('BlobMigrationV3.grist', 'BlobMigrationV8.grist'); + await testMigration('BlobMigrationV3.grist', 'BlobMigrationV9.grist'); }); it('should migrate from v4 correctly', function() { - return testMigration('BlobMigrationV4.grist', 'BlobMigrationV8.grist'); + return testMigration('BlobMigrationV4.grist', 'BlobMigrationV9.grist'); }); it('should migrate from v5 correctly', async function() { @@ -83,7 +83,7 @@ describe('DocStorageMigrations', function() { // // Verify correctness of these fixture files with: // diff -u <(sqlite3 DefaultValuesV5.grist .dump) <(sqlite3 DefaultValuesV7.grist .dump) - return testMigration('DefaultValuesV5.grist', 'DefaultValuesV8.grist'); + return testMigration('DefaultValuesV5.grist', 'DefaultValuesV9.grist'); }); it('should migrate from v6 correctly', async function() { @@ -93,7 +93,7 @@ describe('DocStorageMigrations', function() { // Verify correctness of updated fixture files with, for instance: // cd test/fixtures/docs ; \ // diff -u <(sqlite3 DefaultValuesV6.grist .dump) <(sqlite3 DefaultValuesV7.grist .dump) - await testMigration('BlobMigrationV6.grist', 'BlobMigrationV8.grist'); - await testMigration('DefaultValuesV6.grist', 'DefaultValuesV8.grist'); + await testMigration('BlobMigrationV6.grist', 'BlobMigrationV9.grist'); + await testMigration('DefaultValuesV6.grist', 'DefaultValuesV9.grist'); }); }); From a4768a55b0c1a8c2ec992eb98c26e571f535121a Mon Sep 17 00:00:00 2001 From: Spoffy Date: Sat, 7 Dec 2024 01:44:15 +0000 Subject: [PATCH 45/60] Fixes an error with missing storageId in doc schema --- app/server/lib/DocStorage.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts index 88f8086eee..bca89e7f73 100644 --- a/app/server/lib/DocStorage.ts +++ b/app/server/lib/DocStorage.ts @@ -72,7 +72,8 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage { await db.exec(`CREATE TABLE _gristsys_Files ( id INTEGER PRIMARY KEY, ident TEXT UNIQUE, - data BLOB + data BLOB, + storageId TEXT )`); await db.exec(`CREATE TABLE _gristsys_Action ( id INTEGER PRIMARY KEY, From c41b4db3233f6cb84a22144ccbe4145df38973c5 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 9 Dec 2024 19:47:11 +0000 Subject: [PATCH 46/60] Attempt to change BlobMigrationV8 to V9 using upgradeDocument --- test/fixtures/docs/BlobMigrationV9.grist | Bin 151552 -> 184320 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/fixtures/docs/BlobMigrationV9.grist b/test/fixtures/docs/BlobMigrationV9.grist index 0878744bf157b5d6226b8c0c655f3c856d8d18d9..883c921618d7cb6a35e37bebf190ede2a541797d 100644 GIT binary patch delta 6741 zcmd5Adu$W;`F{5K9=7i;!7)xk(io>Cm?Xr`J21d`7?L)GkT`viWvQ``eN~=}c*21<HvKHb-7$ zttRavU+(*TkKcFS`*)L%v6GLPrq`@HMi4|5{4&=icpx(X{73L(XJ$&Oju9IJgaII( z@Hxhp3114Io4z)A*oT-x>MP1+m?l3b@6o@A_6uLk%v_y)0G;RLk8{vW>YT8}2poC& za9hQn&7nNxTZHh|Y_98dVRHpy2#(`WmAj&4RE$aie|5JwK6G~^x_`nEiUj;2(Xq)P zhO7Gf91n8BRYEVEBZPCpuZ6$O=GvbXx^qB40n0E5`TToBqMKq@=@?=S!Yh4&kw_#I z(-|1pnlHd>3Q;ZzJPL&Z*63*?XX+*LjL)a&u~N$TyeVWtAU`b%G_-D#8QOZy?c7 zxPxmAii|Z{89J3B?So+I>92?M)#kG&X;k9j&-2gmKjt6e@8|dP-{p7mjr@APfH!h~ z=icXj&t2r^xKrFTH_450_i#O&hjVfT9Bclk`Capy=2y&T!6aSgBwuGD5JC(Ge}vI$ z<7>5W-fF%^3tOyKH^6j;KI51I-lZj5mCsje;am&9Q3JCiX{}PlB$a(xk6H0AVc;vZ zbb38sp@nrieuD;PjAq`cg;^87UJEmF~^0^WXm}}u~Q9!&u$514hSA1Otl{E)f zbe)OP>$x==P^aSxuY(4onJdtMtclClfDD^w12C>JJAU_SRjyy1Xsj_IB^~mi{iE-38dFfBX(f9PbkYaVZk+756#po!vg~ z4sY*`WJ)Hr`J=&`Qaeo@#Cx{-dUR}CoW#W}^$wetI?rAZj)ZAiK_ugu_1W`m3Slkp zm0}XmwnaiqD&P+V?i>-r%i-I_v6#c|^WNplYTw=8)rH?P5fYa`1viVaKvWvT2AIcO zUXEGCd&OvM3Dss_)n78;UXx7qn-cW~*&%N(Pr_tO?b?8k*8c(3jbJGpzIUUKp zK&+LgH=$y7>OKo?bGy;exswgZje0=kI2gEH42f|OBD}rD=k;~&^v?JGJ}D%6?~5;R zg7dI0hqkq}prb<88ijMi&4@yKKsK<&#pnVqz}z%kmk2i}j2^eoPNE&U7d^;ew+OX_ z@SgCRa7s8XB!nTMOQ?me@G6gfAYTn5dlEdjlNJQV;VDv`X}1sHX(oYhJ@9L#Z5-?H z;Hd;IuQWUow=pz9(;UNk?8;*HmP-Qygt6C!2BZ5Dy4@#Lx8eUJlT z_ePg}H3=w-6&P~T80o%vG$EGgVV$oXZk3?P^Lnz0}|Ae6V9D?uRbA(?A$As+y&0he!Y~si#=+a#I%yCqJ2Ib#9f-2=} z4}(WMr^phgb>_`-6x z>WqZ4Zc*0vWD)9ybpthn9Ver(Q_Y~RufyBh*$!p5r`OwKZ)t03_wKxV2f_kiOAEVm zQH48BTmg3Xp!};Ovg6*oUeyqJO@W)dx|Lw1vu2 zG-kZ4*v9RyZ)kK64BmEn4kR@UQad4gdhPbMj^55bU#GpVtpgf{$B<>;l8J^adqe%e zVCyGafxUjvAisPN<%`W^mcBaU^elaS6VPA$3h3(w$v&UGrMue;{RVdP_4~Y=^;y>1 zOiU=OiWgz4j@0YGT05whTaw7~(`n?%!NWWfzQhiw622r4LX?Ebxr3L`gC-lSRf0AI z3^h*@!b|*B`)X<$8^s2kHK#UW@9_nc>$XU&sP4Act#$tAo`2u}JjO(wF zuknw}%&oV(>cgWr?IKshVtZMog61} z+;?DXsctC6dPRTm05swyhXKDbD=aMYP!~tV(MYylCe<-RX`&(jUJ+XR4TWDph+#Yn zEHRyo`{RKTF{moj1us8OLb|y}7VkS7%AJSqD5Y)d)}g70)`C6$VKHMq<+)t39f_a8 zaK*`cVI*4e^~Zp!CjzhE-M}I8C@PAo~ zoO3_igg(;A-CNOs9N&WeB%j%acAYx26+T0Om6bAdxER!>%2c}_`kva5red0RIMi9L z966_UYZiz4@PQzc04h+SWPm{daJqg!B!&0mm|p0qB`P{TjxE_qg3FvxkaUofzj(Joq=ExqoWWku#E1ki?lyUy6ki0Z_&R5Bj!bh zX9&YHFh4wMNS^42?{P3fpl2N4c8VYk6A3(~sC~7=9~(({09;n=N~V(pn-$uf7?ljN z>oT&J;**2AAAH)9#w5u6m^u;TV{Bon{~4|4u}X<>OpI4a;aFUryo@RfUH|U> z&87pb2~3JJA%W*FWh{};Uq);2B-CQYu@Z|%qGE7>k{`c<+->dwJmO1uYe|Q1dVIq| zBrHI}91{M7C}Fw@<#f}o@ND;Ohs)=4$xmHDt|5iy=3*w4S8s`m{-L%=P#jR$DA-wA z_NH#4QmUh!LC zGeP-XsYa^!9V-@Z0r86SI>Zpv#-Vm8HWu<9P_|3hK6enUDN0xYyXZM0Q6yI$L?uYoF}Y$L)c}hm#uTjvaP%nQ(WOL_7RD$o zjE$#-akypRI)dm@LNBv`VH|~880+X#Lp>afKrBCd1(k35=CQ0ESY3~xwfNL(0#iJ9 z1(j`Hu_oU3d{b*Ob?Q2HV?VzLWKZV3vIug1BsVC!`M7 zeD1h-hv1!pKlT>}DY%KIX`pLdb>Ve_+xQz^=i;|$c}0mdf;=+ud@`}mr3n&g26=dL zDwRs61r@lcR7A_}9Z(@pPek16kLnXD6#U^0Pad*EI8Pci0@R44lu<*L^Vw{)@j-KT zIz7#8A5JCBKC|LEestoc{7SY%9Bj}?;YPMS<8k3K!C&zkT;_fZ^UHU~U8t|WOCiJ? zR`lY!4&Bg(2MCfaLbmXV9{In7$YWT_-uIl#3T_@bT}Vvs?i9FiC4HTOB`JMoI^ZQ&N&g2QA`U&U1|qaXFYr>?32&$--y zQO7%!d(Wqv%KNgohkg2#u*IGo7cLZ!cfmza+U}#l#zr_3Wd~#MbpB6KVIOuvYrf$2 zv+W*eVB4LLDPj+N56VKXL4%eBXNq6+fd$2u18^EV3sE0+tg6UY#doMpzew)Sr=Y1= zdPewNVExYtKe5eGVYH5KSY=YLHq&5#KP>v$FD+2du8s+my*0+M`Z3}0%W;sxhJKUa z4P3)Fa1syU9{sLzurPAp@^~>Z{g0ogu|ouF+OEqj)mM@4Qh%R zZ?H8&j!Yk)$~b_CWCI48@jwL_PRu4|td5w|Wv|E~qn^9?s?hZ5%*j&;?&GhG^ni;q z)CijFQVJRZjv-owvLc&oBL&7jrvha~F%RFa_}0FbM^Y&VcKHN=ubvZB Date: Tue, 10 Dec 2024 02:41:19 +0000 Subject: [PATCH 47/60] Updates BlobMigrationV9 and associated tooling --- test/fixtures/docs/BlobMigrationV9.grist | Bin 184320 -> 151552 bytes test/upgradeDocumentImpl.ts | 63 ++++++++++++++++++----- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/test/fixtures/docs/BlobMigrationV9.grist b/test/fixtures/docs/BlobMigrationV9.grist index 883c921618d7cb6a35e37bebf190ede2a541797d..3582467b17e479569290bc3961ce142dce1ba200 100644 GIT binary patch delta 1257 zcmZXUU1%It6vywGotZncvomKmv+HJ))ZI=97}JEE=A*Hx?FMZzFa1jVB8C{_(uA&= z(%qKAA~s10s1*|yx%d`}NohgaWFbp^uuuhk2-Kh!Y9EYfK~Yf%*gz=p?#?Dz^ui3o z{r}GYp7ZB27iYDLv+8thV2TjZz@M%4>CB$i&=hGICKBHQ+$ah`-0 zT=dHc@e#pC`9Shf@}B--s9reX!}HoL_p~qdLxzG>(K>?)PmAG zJmNU1w4ebORadl&)jvEOi~oqPdzQG|#j*b`T$k^JySrgB z$! zxS(f@qrQ)|1uf{kJkxDf@q$wIc)Es$o0?#97rXJ2@H=~TP`Fw+)dtr|Yq7rBK3t9D~?9b(9i{s91?QH``dOANbd~ z;_G-+=4x11twgn}9v(e4lBoddr6ORWl?b{(Z}NO{%x;NSy6hG?WQMqlzYL9D$ecZw zD?Ou}V;}uQt~y_M%XPz-`=(fb2kRLfHy- zthSQcp;}AO{4^xyZwh{|m)nTOZv;Op_*Q`#GqW#9hsALn*y?wXD75?qd4E(yK_t5& dLfmp|i<1#ZQFe{{mf%3YP!? delta 6656 zcmd5AX>b$Qd7re>?n>HS8(EeOb}ZyzA#7vG@(BgxLtqozIJU^;DvppAdy6c&I^qzw z2n@`SPQx*R+4fH+X9<%|GR7qy(&Pte8`%xiB$-%42DKyu}DIrqhV{V6tBrdg(UFE6f#(^r3|cbfG9FNm!ZZh^@it+5hDWe zX@jG`gd-4Tt_;wunTCh-)`Ki~y2nP^lF49bbWDgQ6PdqHR%g)U?M~xmVx`OwV}^hM z0sVfNeqf|jk<_XnGQr8(%$dm=9R=vyX0A+DCyN>G1A_k(|8xE+eu@wBTlm#H$$bEd zyw0V|3`GQ@OIgH#x!?g^Sb`-JVss=TgjX>|1TzntMV8SEv1+nd8?^!pE>#OA<3cDF z4=dIQSLjy|UXXqRiHgDsEN1N%4jefGEPMRt}w%^qj>v*YZ2Y(MK|oop$~nEqjU*Yu|8Wz$(O zNslSbH5dtm5Y56LVX#;^j~Xs2=IYh3*-6wFwIyT0O!=Dy|*aa7`Un| zSsEXkrf6=Bnom!$w;J+hp)6LmQUw+jvvw6|wiK-fFuMtrHwkM~5v?d;D^y^inY~2@ z@%}8WC&{An>oW9tbFgLCnea%@kPy%qpqN8{!tW zSVcwAtVIRtsoB>#)QHw-Yy?MqLhvsB0=LriJL5&-6D`SHp#7hvPTi91AL zVo(Sr#aMJe*lo9U_xXL>d;>R<$(hs#O zVSbFTPWOum5z1UgEVAf!!AR)NQ6ah%zEc=a*lm8_UH-hTeLH)4@TpP}VG&e#qmT&2 z#c_PxIm`|RW)nYwq$44O)0s}=Bs0b8D67+n4rXZ6E1COP@*Coq@L((| zEF?G_3#D+C9TbvDD0g$!b2w&xWm=~_>p}$^sdotjks*kT$;JGhDL0&>s`R|JMK?l@ zaMS!W`yK8M(__Z-#xlc6!$~EfP@NSj+aZr6dC;BwVLLCI{GpK~ZYP;(R5WuGIW)pe zY_qg!5~IPmutT=oeC?f6HO16*9S#@H^NX~u0Ueut-;K_o*%unnn}{@=L>p(1H=$=W zBzulnEgj#0%9*JL%#_vbMu%rlH6b_Z2bGgx)=nWJBn61#&Njc#-`(q*>+{`WMDRV3 zoM(FHK}`W=ZEHh^`Mfb^?=3(kbPtFIuA~s3#{`(0#^@q3=7j0twwV~(rg`3rbT%{Z zA^7+BSNYTYF+Rl)^F6!=3jZq{`nL3T6xq_?zg~*xX}h;faiz^Rgy2^Szxv_7owBlw z-HQhgczoi>Xwphk1VynlJ%`Ei5_EZh$XiYp+vey)K38GnS|%2V70u*(LPe4MH*l%|iNQWTKhIEred zYY!tmHq&81*fn;$Jtx)ea=TpkzriyUP#I05R{`=*;GO_(?R;*)+(lKI0fU;o0N@A9 z-K;SX^14}GzbB7SHf$KG4|JW1!%p=9&0v>rpt}<)YyW_+-`3XA*6HiLdmF+6U`sR8 zyP(2dC$9p#J0SfcjcmC6u2nQdK56=4R8$mbzx+5pNhg&Pl*JgbnC1W_)$K!N;wF8b zqB-Yf**0!>V^gzxD6sv^EJ$h!=zAe~25h#Du7U1Bf46P0qYHY1SC?nsl#7NudsE|3 zp#9^`z}^_pNiXh0B|c8qkJ- zx#bChe}Q`fQml^qGmxC%Zu5}`Sk%ockI)iwdt41VS&H<|CFGr5|0$X&sL=5ml30!8 zv3@uK6>+qPJ4k>S}b@EElKYk+_&h4*TQc$OyEdIpEsN1*TZ9;4(qlkzS}(<1|kC z_l_^t0L8?B5Df2y-n(eG56Z*9{2C8caZDJC<*Q{{nIq)B842zZpqt-N^<{(*#Vu%& z@na&G42=q5MVTHr`Wy+_ejZ!6>8x`&58P2nS=X#VQ!%yu`hz1v&Uo@~2UyNR;-6r* z^7Q>M@+^A#aiHpt!Rh;U_Cj8F`m^i3R5DU3Lj)aBfJM{?Ft-~Ha zq_m?sE}jGazj=Jm*YmjZz|J-D#55(S-L+M2!a0X!At!d3UIFq>yV;x?S|X|!Z}sD1 zG`e7fI;WbdlodLa}HPLUvAa$?2I&2yr=7=VY#v-b6f!huf*zIj%Cpz?3`u zBEH&=6YQ(_O7GC)I!_$5tW?TkP#Schzu-GVFcjGui;txu!Tul&=wPln8UxL^5FZm0 z2^j4Yeqn)G5J>RVV9D&p27zxPadB5FnX92a`5GYG?1mzk%LpeICVncHB9Gc&Hz`Gd z|79t1&VGLb`bZ=7ZAL>&g#A`TjPAj)I^R!=)N_3QTC|UZm$;lro89O% z$HJMk{D%bpIe(4+5WdV^;Y%s44n8B$TCHBABN#+$^avIhT65M#+MXa?wpsEw=%2g+ z^CI2TgzjmW9Ujr8Pws@zZ7@Edr|n;Nf*^I1DLkSmU9~Hi7)^NrTvF_ECX*DC7uuH^ z6Lpg7GO|_TTZ6J6e7O?GMM(REG7aN9MQG-Qga># z#V;&G!~#UjA>uEDBBq;^Z#JDW&sP6dc>I2s^yF3K8kT8pE@o0bdRtrw4tK=D!jR0S zhn>YGU&Q}By7!O5bzWh)da{cU5+YC;hdaf@cqF)2-Y#X6UQVM*P0Avr)2Kv~GE1A& zsJJ9m2u$TbK!Gc Yko3sg$ZEz(E|-c9_?ifVLsD@42h6w5Hvj+t diff --git a/test/upgradeDocumentImpl.ts b/test/upgradeDocumentImpl.ts index 499c28a0e2..f335f98487 100644 --- a/test/upgradeDocumentImpl.ts +++ b/test/upgradeDocumentImpl.ts @@ -4,15 +4,59 @@ * Usage: * test/upgradeDocument */ -import {copyFile} from 'app/server/lib/docUtils'; -import {createDocTools} from 'test/server/docTools'; +import { DocStorage } from "app/server/lib/DocStorage"; +import { DocStorageManager } from "app/server/lib/DocStorageManager"; +import { copyFile } from 'app/server/lib/docUtils'; +import { createDocTools } from 'test/server/docTools'; import log from 'app/server/lib/log'; import * as fs from "fs"; +import * as fse from "fs-extra"; +import * as path from "path"; +import * as tmp from "tmp"; + +export async function upgradeDocuments(docPaths: string[]): Promise { + const docTools = createDocTools(); + await docTools.before(); + try { + for (const docPath of docPaths) { + console.log(`Upgrading ${docPath}`); + const activeDoc = await docTools.loadLocalDoc(docPath); + await activeDoc.waitForInitialization(); + await activeDoc.shutdown(); + await copyFile(docTools.getStorageManager().getPath(activeDoc.docName), docPath); + } + } finally { + await docTools.after(); + } +} + +export async function upgradeDocumentsDocStorageOnly(paths: string[]): Promise { + let tmpDir = await tmp.dirAsync({ prefix: 'grist_migrate_', unsafeCleanup: true }); + tmpDir = await fse.realpath(tmpDir); + const docStorageManager = new DocStorageManager(tmpDir); + + for (const docPath of paths) { + console.log(`Upgrading '${docPath}' (DocStorage migrations only)`); + const docName = path.basename(docPath); + const tempPath = docStorageManager.getPath(docName); + fs.copyFileSync(docPath, tempPath); + + const docStorage = new DocStorage(docStorageManager, docName); + await docStorage.openFile(); + await docStorage.shutdown(); + + fs.copyFileSync(tempPath, docPath); + } +} export async function main() { - const docPaths = process.argv.slice(2); + const params = process.argv.slice(2); + const onlyRunDocStorageMigrations = params.map((text) => text.toLowerCase()).includes("--doc-storage-only"); + const docPaths = params.filter((text) => text.trim()[0] != "-"); if (docPaths.length === 0) { console.log(`Usage:\n test/upgradeDocument path/to/doc.grist ...\n`); + console.log(`Parameters: `); + console.log(` --doc-storage-only - Only runs DocStorage migrations`); throw new Error("Document argument required"); } for (const docPath of docPaths) { @@ -26,18 +70,13 @@ export async function main() { const prevLogLevel = log.transports.file.level; log.transports.file.level = 'warn'; - const docTools = createDocTools(); - await docTools.before(); try { - for (const docPath of docPaths) { - console.log(`Upgrading ${docPath}`); - const activeDoc = await docTools.loadLocalDoc(docPath); - await activeDoc.waitForInitialization(); - await activeDoc.shutdown(); - await copyFile(docTools.getStorageManager().getPath(activeDoc.docName), docPath); + if (onlyRunDocStorageMigrations) { + await upgradeDocumentsDocStorageOnly(docPaths); + } else { + await upgradeDocuments(docPaths); } } finally { - await docTools.after(); log.transports.file.level = prevLogLevel; } } From 3008fdd228b7b19c96d3dbf5e2765332b137b637 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 10 Dec 2024 12:01:38 +0000 Subject: [PATCH 48/60] Fixes failing DocStorage tests --- test/fixtures/docs/Hello.grist | Bin 62464 -> 62464 bytes test/server/lib/DocStorage.js | 11 +++++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/test/fixtures/docs/Hello.grist b/test/fixtures/docs/Hello.grist index ca9c56299e143cab226493ab55a821d258435330..5fa4942b4c31da18721413b3abad9f2cefda7dc8 100644 GIT binary patch delta 601 zcmZp8!QAkId4ja63NJ5Z8zhOF_A9Fsx*f1KK$U zsB^7Pv@{n}E8|**wG43#VG}31@i2yiwD^j#i6>eEt<0J)^%`7?W zEQ~grc?txW7>zc|7VTuzwB`b;x8ej6mLS_K*nxyO8;~#qg`O!Wa7~zjgz@J8#Y_q^ z%n=OC46+PN5saIFlqe8_0mPw4r&u$CG_07M(y(Nb4$ESe89Y4949^*u4H)+Wsrx{8 zUt$HTo!q_Z#$>M*x{TVB3rxf|_pSKL$jG~xk%NbySpyiOZ48X>8I>8@m@9x<%9tCa zR3@jbwr4b$yliz8bEBv7w;ILUc8^kmt`n+ z(FHLzGg+Oj-=3bjTfKVCjJO&8jKy|qTVOwK$&}cpwvH>5QuqFM3IB7UPYW6Pnkc|G zwiN@vNuINmaHQ5lF<#2gNOp3!U};6MDnwEWP#z*%J?Hh|%ZZ2>$DGW;7xOa0k8aKZ zkKZ^V{pmdl`Z-!_lW>faceWsZ#xm=jZZ&i&V(4VpP+!PUG5B$BaF7#d=s5qb%am;@ z6mJdWs7bDH^iccWU@!)EO-f}|1iTUz`2;7lF461NZa7M{ zz5|Reeu45XrNdNc4%36R4!W>fLoc48#~XvtP46~F{g( testUtils.writeTmpFile("Hello, world!")) - .then(tmpPath => docStorage.findOrAttachFile(tmpPath, "hello_world.txt")) + .then(() => docStorage.findOrAttachFile( "hello_world.txt", Buffer.from(correctFileContents))) .then(result => assert.isTrue(result)) .then(() => docStorage.getFileInfo("hello_world.txt")) - .then(data => assert.equal(data.toString('utf8'), "Hello, world!")) + .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents)) // If we use the same fileIdent for another file, it should not get attached. - .then(() => testUtils.writeTmpFile("Another file")) - .then(tmpPath => docStorage.findOrAttachFile(tmpPath, "hello_world.txt")) + .then(() => docStorage.findOrAttachFile("hello_world.txt", Buffer.from("Another file"))) .then(result => assert.isFalse(result)) .then(() => docStorage.getFileInfo("hello_world.txt")) - .then(data => assert.equal(data.toString('utf8'), "Hello, world!")); + .then(fileInfo => assert.equal(fileInfo.data.toString('utf8'), correctFileContents)); }); }); From 4a1c1129a2156142570e2be2b40bd49874774fe8 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 10 Dec 2024 14:33:20 +0000 Subject: [PATCH 49/60] Renames "idOfDefaultAttachmentStore" to "attachmentStoreId" --- app/common/DocumentSettings.ts | 2 +- app/server/lib/ActiveDoc.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/common/DocumentSettings.ts b/app/common/DocumentSettings.ts index 9d4d5d2a88..cc680fc0df 100644 --- a/app/common/DocumentSettings.ts +++ b/app/common/DocumentSettings.ts @@ -2,7 +2,7 @@ export interface DocumentSettings { locale: string; currency?: string; engine?: EngineCode; - idOfDefaultAttachmentStore?: string; + attachmentStoreId?: string; } /** diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 191ef5bbee..cfb6317bf6 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -648,6 +648,7 @@ export class ActiveDoc extends EventEmitter { useExisting?: boolean, // If set, document can be created as an overlay on // an existing sqlite file. }): Promise { + console.log(`OPENOPENOPENOPENOPENOPENOPENOPEN ${this.docName} ANYTHING ELSE`); const startTime = Date.now(); this._log.debug(docSession, "loadDoc"); try { @@ -2356,7 +2357,7 @@ export class ActiveDoc extends EventEmitter { dimensions.height = 0; dimensions.width = 0; } - const attachmentStoreId = (await this._getDocumentSettings()).idOfDefaultAttachmentStore; + const attachmentStoreId = (await this._getDocumentSettings()).attachmentStoreId; const addFileResult = await this._attachmentFileManager .addFile(attachmentStoreId, fileData.ext, await readFile(fileData.absPath)); this._log.info( From adac94b0f32313406e03f907b29538810297ddf9 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 10 Dec 2024 23:17:17 +0000 Subject: [PATCH 50/60] Makes getFileData throw an error if file is missing --- app/server/lib/AttachmentFileManager.ts | 26 +++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 3b884cb7d8..1ce7145c31 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -37,6 +37,15 @@ export class StoreNotAvailableError extends Error { } } +export class MissingAttachmentError extends Error { + public readonly fileIdent: string; + + constructor(fileIdent: string) { + super(`Attachment file '${fileIdent}' could not be found in this document`); + this.fileIdent = fileIdent; + } +} + export class AttachmentRetrievalError extends Error { public readonly storeId: AttachmentStoreId; public readonly fileId: string; @@ -93,6 +102,14 @@ export class AttachmentFileManager implements IAttachmentFileManager { fileData: Buffer ): Promise { const fileIdent = await this._getFileIdentifier(fileExtension, Readable.from(fileData)); + return this._addFile(storeId, fileIdent, fileData); + } + + public async _addFile( + storeId: AttachmentStoreId | undefined, + 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); @@ -105,14 +122,11 @@ export class AttachmentFileManager implements IAttachmentFileManager { return this._addFileToAttachmentStore(store, fileIdent, fileData); } - // TODO Given we throw if the underlying store can't retrieve the file, maybe this should throw if we're missing - // the fileInfo from doc storage too, instead of returning null? - // That, or we return a result type which might have an error in. - public async getFileData(fileIdent: string): Promise { + public async getFileData(fileIdent: string): Promise { const fileInfo = await this._docStorage.getFileInfo(fileIdent); if (!fileInfo) { - this._log.warn({ fileIdent }, "cannot find file metadata in document"); - return null; + this._log.error({ fileIdent }, "cannot find file metadata in document"); + throw new MissingAttachmentError(fileIdent); } this._log.debug( { fileIdent, storeId: fileInfo.storageId }, From 3b0f603d5ddaca8838e573e51b3a347bad11a290 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 30 Dec 2024 23:37:31 +0000 Subject: [PATCH 51/60] Updates comments to address feedback --- app/server/lib/ActiveDoc.ts | 4 ++-- app/server/lib/AttachmentFileManager.ts | 2 +- app/server/lib/AttachmentStore.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index cfb6317bf6..5d396d0708 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -392,8 +392,8 @@ export class ActiveDoc extends EventEmitter { loadTable: this._rawPyCall.bind(this, 'load_table'), }); - // This will throw errors if _doc or externalAttachmentStoreProvider aren't provided, and ActiveDoc tries to use - // an external attachment store. + // This will throw errors if _options?.doc or externalAttachmentStoreProvider aren't provided, + // and ActiveDoc tries to use an external attachment store. this._attachmentFileManager = new AttachmentFileManager( this.docStorage, externalAttachmentStoreProvider, diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index 1ce7145c31..bf00bfa44f 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -67,7 +67,7 @@ interface AttachmentFileManagerLogInfo { } /** - * Instantiated on a per-document to basis to provide a document with access to its attachments. + * 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 document database, * which tracks attachments and where they're stored. * diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index ce02a9a5df..89589403b6 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -7,7 +7,7 @@ export type DocPoolId = string; type FileId = string; -// Minimum required info from a document +// Minimum document info needed to know which document pool to use. // Compatible with Document entity for ease of use export interface AttachmentStoreDocInfo { id: string; From 2ba58483ca7badc28d6d4eae3b6121081c3c70bd Mon Sep 17 00:00:00 2001 From: Spoffy Date: Thu, 2 Jan 2025 19:38:42 +0000 Subject: [PATCH 52/60] Removes leftover debugging log --- app/server/lib/ActiveDoc.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 5d396d0708..fa292ae954 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -648,7 +648,6 @@ export class ActiveDoc extends EventEmitter { useExisting?: boolean, // If set, document can be created as an overlay on // an existing sqlite file. }): Promise { - console.log(`OPENOPENOPENOPENOPENOPENOPENOPEN ${this.docName} ANYTHING ELSE`); const startTime = Date.now(); this._log.debug(docSession, "loadDoc"); try { From 635a92c6702fc4ab635a7f5712c264ad3cf7fe37 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Thu, 2 Jan 2025 21:01:18 +0000 Subject: [PATCH 53/60] Updates AttachmentStore comments --- app/server/lib/AttachmentStore.ts | 46 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 89589403b6..f382b26d5d 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -11,8 +11,9 @@ type FileId = string; // Compatible with Document entity for ease of use export interface AttachmentStoreDocInfo { id: string; - // This should NOT be an optional: the programmer should make an explicit choice for this value to prevent - // accidental omission breaking attachments. + // We explicitly make this a union type instead of making the attribute optional because the + // programmer must make a conscious choice to mark it as null or undefined, not merely omit it. + // Omission could easily result in invalid behaviour. trunkId: string | null | undefined; } @@ -20,8 +21,9 @@ export interface AttachmentStoreDocInfo { * Gets the correct pool id for a given document, given the document's id and trunk id. * * Attachments are stored in a "Document Pool", which is used to manage the attachments' lifecycle. - * Document pools are shared between snapshots and forks, but not between documents. This provides quick forking - * and snapshotting (not having to copy attachments), while avoiding more complex systems like reference tracking. + * Document pools are shared between snapshots and forks, but not between documents. This provides + * quick forking and snapshotting (not having to copy attachments), while avoiding more complex + * systems like reference tracking. * * Generally, the pool id of a document should be its trunk id if available (because it's a fork), * or the document's id (if it isn't a fork). @@ -29,12 +31,14 @@ export interface AttachmentStoreDocInfo { * This means that attachments need copying to a new pool when a document is copied. * Avoids other areas of the codebase having to understand how documents are mapped to pools. * - * This is a key security measure, as only a specific document and its forks can access its attachments. This helps - * prevent malicious documents being uploaded, which might attempt to access another user's attachments. + * This is a key security measure, as only a specific document and its forks can access its + * attachments. This helps prevent malicious documents being uploaded, which might attempt to + * access another user's attachments. * - * Therefore, it is CRITICAL that documents with different security domains (e.g from different teams) do not share a - * document pool. - * @param {AttachmentStoreDocInfo} docInfo - Document details needed to calculate the document pool. + * Therefore, it is CRITICAL that documents with different security domains (e.g from different + * teams) do not share a document pool. + * @param {AttachmentStoreDocInfo} docInfo - Document details needed to calculate the document + * pool. * @returns {string} - ID of the pool the attachments will be stored in. */ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string { @@ -42,23 +46,23 @@ export function getDocPoolIdFromDocInfo(docInfo: AttachmentStoreDocInfo): string } /** - * Provides access to external storage, specifically for storing attachments. Each store represents a specific - * location to store attachments, e.g. "/srv/grist/attachments" on the filesystem. + * Provides access to external storage, specifically for storing attachments. Each store represents + * a specific location to store attachments, e.g. "/srv/grist/attachments" on the filesystem. * * This is a general-purpose interface that should abstract over many different storage providers, * so shouldn't have methods which rely on one the features of one specific provider. * - * `IAttachmentStore` is distinct from `ExternalStorage` as it's specific to attachments, and can therefore not concern - * itself with some features ExternalStorage has (e.g versioning). This means it can present a more straightforward - * interface for components which need to access attachment files. + * `IAttachmentStore` is distinct from `ExternalStorage` as it's specific to attachments, and can + * therefore not concern itself with some features ExternalStorage has (e.g versioning). This means + * it can present a more straightforward interface for components which need to access attachment + * files. * - * A document pool needs specifying for all store operations, which should be calculated with `getDocPoolIdFromDocInfo` - * See {@link getDocPoolIdFromDocInfo} for more details. + * A document pool needs specifying for all store operations, which should be calculated with + * `getDocPoolIdFromDocInfo` See {@link getDocPoolIdFromDocInfo} for more details. */ export interface IAttachmentStore { - // Universally unique id, such that no two Grist installations should have the same store ids, if they're for - // different stores. - // This allows for explicit detection of unavailable stores. + // Universally unique id, such that no two Grist installations should have the same store ids, if + // they're for different stores. This allows for explicit detection of unavailable stores. readonly id: string; // Check if attachment exists in the store. @@ -68,8 +72,8 @@ export interface IAttachmentStore { upload(docPoolId: DocPoolId, fileId: FileId, fileData: stream.Readable): Promise; // Download attachment to an in-memory buffer. - // It's preferable to accept an output stream as a parameter, as it simplifies attachment store implementation - // and gives them control over local buffering. + // It's preferable to accept an output stream as a parameter, as it simplifies attachment store + // implementation and gives them control over local buffering. download(docPoolId: DocPoolId, fileId: FileId, outputStream: stream.Writable): Promise; // Remove attachments for all documents in the given document pool. From dc55180d6fef45065203997d39d969cc4a2c9924 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Thu, 2 Jan 2025 21:01:32 +0000 Subject: [PATCH 54/60] Makes removeAllWithPrefix check clearer --- app/server/lib/AttachmentStore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index f382b26d5d..15f34b4aec 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -103,7 +103,7 @@ export class ExternalStorageAttachmentStore implements IAttachmentStore { private _storage: StreamingExternalStorage, private _prefixParts: string[] ) { - if (_storage.removeAllWithPrefix == undefined) { + if (!_storage.removeAllWithPrefix) { throw new InvalidAttachmentExternalStorageError("ExternalStorage does not support removeAllWithPrefix"); } } From ae87fa12eec8d17a77cf903d04079c35024470b7 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Thu, 2 Jan 2025 21:01:38 +0000 Subject: [PATCH 55/60] Simplifies MinIOExternalStorage "_listObjects" method --- app/server/lib/MinIOExternalStorage.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/app/server/lib/MinIOExternalStorage.ts b/app/server/lib/MinIOExternalStorage.ts index a9f6b91513..bf06736813 100644 --- a/app/server/lib/MinIOExternalStorage.ts +++ b/app/server/lib/MinIOExternalStorage.ts @@ -216,17 +216,11 @@ export class MinIOExternalStorage implements StreamingExternalStorage { } private async _listObjects(...args: Parameters): Promise { - return new Promise((resolve, reject) => { - const bucketItemStream = this._s3.listObjects(...args); - const results: minio.BucketItem[] = []; - bucketItemStream - .on('error', reject) - .on('end', () => { - resolve(results); - }) - .on('data', data => { - results.push(data); - }); - }); + const bucketItemStream = this._s3.listObjects(...args); + const results: minio.BucketItem[] = []; + for await (const data of bucketItemStream) { + results.push(data); + } + return results; } } From b46b320c927e884225e1f1fac29f3af04feb35b7 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Thu, 2 Jan 2025 21:02:09 +0000 Subject: [PATCH 56/60] Fixes test import --- test/server/lib/ActiveDoc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/lib/ActiveDoc.ts b/test/server/lib/ActiveDoc.ts index a02eadfa83..43789383d3 100644 --- a/test/server/lib/ActiveDoc.ts +++ b/test/server/lib/ActiveDoc.ts @@ -6,6 +6,7 @@ import * as gristTypes from 'app/common/gristTypes'; import { GristObjCode } from 'app/plugin/GristData'; import { TableData } from 'app/common/TableData'; import { ActiveDoc } from 'app/server/lib/ActiveDoc'; +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'; @@ -23,7 +24,6 @@ import { createDocTools } from 'test/server/docTools'; import * as testUtils from 'test/server/testUtils'; import { EnvironmentSnapshot } from 'test/server/testUtils'; import * as tmp from 'tmp'; -import { AttachmentStoreProvider } from "../../../app/server/lib/AttachmentStoreProvider"; const execFileAsync = promisify(child_process.execFile); From e44aadf62412076b4abd632aba102ba29225f164 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 3 Jan 2025 20:50:39 +0000 Subject: [PATCH 57/60] Removes TODO from coreCreator.ts --- app/server/lib/coreCreator.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index e7d02530f0..19c41c336f 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -22,7 +22,6 @@ export const makeCoreCreator = () => makeSimpleCreator({ { name: 'minio', isAvailable: async () => checkMinIOExternalStorage() !== undefined, - // TODO - Move this to another file create: async (storeId: string) => { const options = checkMinIOExternalStorage(); if (!options) { From bc2a4401fead3d5ac0bb13a84db9478551934153 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 6 Jan 2025 23:50:49 +0000 Subject: [PATCH 58/60] Fixes linter issue --- test/server/lib/AttachmentFileManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/lib/AttachmentFileManager.ts b/test/server/lib/AttachmentFileManager.ts index a8c58da172..d7be3c5201 100644 --- a/test/server/lib/AttachmentFileManager.ts +++ b/test/server/lib/AttachmentFileManager.ts @@ -208,6 +208,6 @@ describe("AttachmentFileManager", function() { const fileData = await manager.getFileData(addFileResult.fileIdent); assert(fileData); - assert.equal(fileData!.toString(), defaultTestFileContent); + assert.equal(fileData.toString(), defaultTestFileContent); }); }); From 3c2c828656451ae97e668a61b987b9faea28e279 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 8 Jan 2025 02:30:09 +0000 Subject: [PATCH 59/60] Tidies imports and adds comments --- app/common/DocumentSettings.ts | 7 ++++ app/server/generateInitialDocSql.ts | 2 +- app/server/lib/ActiveDoc.ts | 10 +++--- app/server/lib/ExternalStorage.ts | 2 +- app/server/lib/FlexServer.ts | 16 ++++----- app/server/lib/ICreate.ts | 12 +++---- test/server/docTools.ts | 4 +-- test/server/lib/ActiveDoc.ts | 36 ++++++++++---------- test/server/lib/FilesystemAttachmentStore.ts | 11 +++--- test/server/lib/HostedStorageManager.ts | 5 ++- test/upgradeDocumentImpl.ts | 8 ++--- 11 files changed, 62 insertions(+), 51 deletions(-) diff --git a/app/common/DocumentSettings.ts b/app/common/DocumentSettings.ts index cc680fc0df..ab7e40c126 100644 --- a/app/common/DocumentSettings.ts +++ b/app/common/DocumentSettings.ts @@ -2,6 +2,13 @@ export interface DocumentSettings { locale: string; currency?: string; engine?: EngineCode; + // Grist attachments can be stored within the document (embedded in the SQLite file), or held + // externally. The attachmentStoreId expresses a preference for which store should be used for + // attachments in this doc. This store will be used for new attachments when they're added, and a + // process can be triggered to start transferring all attachments that aren't already in this + // store over to it. A full id is stored, rather than something more convenient like a boolean or + // the string "external", after thinking carefully about how downloads/uploads and transferring + // files to other installations could work. attachmentStoreId?: string; } diff --git a/app/server/generateInitialDocSql.ts b/app/server/generateInitialDocSql.ts index 542d705751..661b4542d3 100644 --- a/app/server/generateInitialDocSql.ts +++ b/app/server/generateInitialDocSql.ts @@ -1,5 +1,5 @@ import { ActiveDoc } from 'app/server/lib/ActiveDoc'; -import { AttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; +import { AttachmentStoreProvider } from 'app/server/lib/AttachmentStoreProvider'; import { create } from 'app/server/lib/create'; import { DocManager } from 'app/server/lib/DocManager'; import { makeExceptionalDocSession } from 'app/server/lib/DocSession'; diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index fa292ae954..cd527eb266 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -13,7 +13,7 @@ import { UserActionBundle } from 'app/common/ActionBundle'; import {ActionGroup, MinimalActionGroup} from 'app/common/ActionGroup'; -import {ActionSummary} from "app/common/ActionSummary"; +import {ActionSummary} from 'app/common/ActionSummary'; import { AclResources, AclTableDescription, @@ -104,7 +104,7 @@ import {makeForkIds} from 'app/server/lib/idUtils'; import {GRIST_DOC_SQL, GRIST_DOC_WITH_TABLE1_SQL} from 'app/server/lib/initialDocSql'; import {ISandbox} from 'app/server/lib/ISandbox'; import log from 'app/server/lib/log'; -import {LogMethods} from "app/server/lib/LogMethods"; +import {LogMethods} from 'app/server/lib/LogMethods'; import {ISandboxOptions} from 'app/server/lib/NSandbox'; import {NullSandbox, UnavailableSandboxMethodError} from 'app/server/lib/NullSandbox'; import {DocRequests} from 'app/server/lib/Requests'; @@ -120,7 +120,7 @@ import { } from 'app/server/lib/sessionUtils'; import {shortDesc} from 'app/server/lib/shortDesc'; import {TableMetadataLoader} from 'app/server/lib/TableMetadataLoader'; -import {DocTriggers} from "app/server/lib/Triggers"; +import {DocTriggers} from 'app/server/lib/Triggers'; import {fetchURL, FileUploadInfo, globalUploadSet, UploadInfo} from 'app/server/lib/uploads'; import assert from 'assert'; import {Mutex} from 'async-mutex'; @@ -137,8 +137,8 @@ import tmp from 'tmp'; import {ActionHistory} from './ActionHistory'; import {ActionHistoryImpl} from './ActionHistoryImpl'; import {ActiveDocImport, FileImportOptions} from './ActiveDocImport'; -import {AttachmentFileManager} from "./AttachmentFileManager"; -import {IAttachmentStoreProvider} from "./AttachmentStoreProvider"; +import {AttachmentFileManager} from './AttachmentFileManager'; +import {IAttachmentStoreProvider} from './AttachmentStoreProvider'; import {DocClients} from './DocClients'; import {DocPluginManager} from './DocPluginManager'; import {DocSession, makeExceptionalDocSession, OptDocSession} from './DocSession'; diff --git a/app/server/lib/ExternalStorage.ts b/app/server/lib/ExternalStorage.ts index fb1238f6be..7004e2c642 100644 --- a/app/server/lib/ExternalStorage.ts +++ b/app/server/lib/ExternalStorage.ts @@ -5,7 +5,7 @@ import {createTmpDir} from 'app/server/lib/uploads'; import {delay} from 'bluebird'; import * as fse from 'fs-extra'; import * as path from 'path'; -import stream from "node:stream"; +import stream from 'node:stream'; // A special token representing a deleted document, used in places where a // checksum is expected otherwise. diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 3334b7e217..b3ef8e689e 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -11,8 +11,8 @@ import {SandboxInfo} from 'app/common/SandboxInfo'; import {tbind} from 'app/common/tbind'; import * as version from 'app/common/version'; import {ApiServer, getOrgFromRequest} from 'app/gen-server/ApiServer'; -import {Document} from "app/gen-server/entity/Document"; -import {Organization} from "app/gen-server/entity/Organization"; +import {Document} from 'app/gen-server/entity/Document'; +import {Organization} from 'app/gen-server/entity/Organization'; import {User} from 'app/gen-server/entity/User'; import {Workspace} from 'app/gen-server/entity/Workspace'; import {Activations} from 'app/gen-server/lib/Activations'; @@ -29,14 +29,14 @@ import {appSettings} from 'app/server/lib/AppSettings'; import {attachEarlyEndpoints} from 'app/server/lib/attachEarlyEndpoints'; import { AttachmentStoreProvider, checkAvailabilityAttachmentStoreOptions, IAttachmentStoreProvider -} from "app/server/lib/AttachmentStoreProvider"; +} from 'app/server/lib/AttachmentStoreProvider'; import {addRequestUser, getTransitiveHeaders, getUser, getUserId, isAnonymousUser, isSingleUserMode, redirectToLoginUnconditionally} from 'app/server/lib/Authorizer'; import {redirectToLogin, RequestWithLogin, signInStatusMiddleware} from 'app/server/lib/Authorizer'; import {forceSessionChange} from 'app/server/lib/BrowserSession'; import {Comm} from 'app/server/lib/Comm'; -import {ConfigBackendAPI} from "app/server/lib/ConfigBackendAPI"; -import {IGristCoreConfig} from "app/server/lib/configCore"; +import {ConfigBackendAPI} from 'app/server/lib/ConfigBackendAPI'; +import {IGristCoreConfig} from 'app/server/lib/configCore'; import {create} from 'app/server/lib/create'; import {addDiscourseConnectEndpoints} from 'app/server/lib/DiscourseConnect'; import {addDocApiRoutes} from 'app/server/lib/DocApi'; @@ -45,7 +45,7 @@ import {DocWorker} from 'app/server/lib/DocWorker'; import {DocWorkerInfo, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; import {expressWrap, jsonErrorHandler, secureJsonErrorHandler} from 'app/server/lib/expressWrap'; import {Hosts, RequestWithOrg} from 'app/server/lib/extractOrg'; -import {addGoogleAuthEndpoint} from "app/server/lib/GoogleAuth"; +import {addGoogleAuthEndpoint} from 'app/server/lib/GoogleAuth'; import {GristBullMQJobs, GristJobs} from 'app/server/lib/GristJobs'; import {DocTemplate, GristLoginMiddleware, GristLoginSystem, GristServer, RequestWithGrist} from 'app/server/lib/GristServer'; @@ -84,14 +84,14 @@ import * as fse from 'fs-extra'; import * as http from 'http'; import * as https from 'https'; import {i18n} from 'i18next'; -import i18Middleware from "i18next-http-middleware"; +import i18Middleware from 'i18next-http-middleware'; import mapValues = require('lodash/mapValues'); import pick = require('lodash/pick'); import morganLogger from 'morgan'; import {AddressInfo} from 'net'; import fetch from 'node-fetch'; import * as path from 'path'; -import * as serveStatic from "serve-static"; +import * as serveStatic from 'serve-static'; // Health checks are a little noisy in the logs, so we don't show them all. // We show the first N health checks: diff --git a/app/server/lib/ICreate.ts b/app/server/lib/ICreate.ts index ea9343f959..38e3dcb3d0 100644 --- a/app/server/lib/ICreate.ts +++ b/app/server/lib/ICreate.ts @@ -3,10 +3,16 @@ import {getCoreLoginSystem} from 'app/server/lib/coreLogins'; import {getThemeBackgroundSnippet} from 'app/common/Themes'; import {Document} from 'app/gen-server/entity/Document'; import {HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; +import {IAttachmentStore} from 'app/server/lib/AttachmentStore'; +import {Comm} from 'app/server/lib/Comm'; +import {DocStorageManager} from 'app/server/lib/DocStorageManager'; +import {IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; import {ExternalStorage, ExternalStorageCreator} from 'app/server/lib/ExternalStorage'; import {createDummyTelemetry, GristLoginSystem, GristServer} from 'app/server/lib/GristServer'; +import {HostedStorageManager, HostedStorageOptions} from 'app/server/lib/HostedStorageManager'; import {createNullAuditLogger, IAuditLogger} from 'app/server/lib/IAuditLogger'; import {IBilling} from 'app/server/lib/IBilling'; +import {IDocStorageManager} from 'app/server/lib/IDocStorageManager'; import {EmptyNotifier, INotifier} from 'app/server/lib/INotifier'; import {InstallAdmin, SimpleInstallAdmin} from 'app/server/lib/InstallAdmin'; import {ISandbox, ISandboxCreationOptions} from 'app/server/lib/ISandbox'; @@ -14,12 +20,6 @@ import {IShell} from 'app/server/lib/IShell'; import {createSandbox, SpawnFn} from 'app/server/lib/NSandbox'; import {SqliteVariant} from 'app/server/lib/SqliteCommon'; import {ITelemetry} from 'app/server/lib/Telemetry'; -import {IDocStorageManager} from './IDocStorageManager'; -import {Comm} from "./Comm"; -import {IDocWorkerMap} from "./DocWorkerMap"; -import {HostedStorageManager, HostedStorageOptions} from "./HostedStorageManager"; -import {DocStorageManager} from "./DocStorageManager"; -import {IAttachmentStore} from "./AttachmentStore"; // In the past, the session secret was used as an additional // protection passed on to expressjs-session for security when diff --git a/test/server/docTools.ts b/test/server/docTools.ts index 3eb69537f6..7a1cb29ac7 100644 --- a/test/server/docTools.ts +++ b/test/server/docTools.ts @@ -1,7 +1,9 @@ import {Role} from 'app/common/roles'; import {getDocWorkerMap} from 'app/gen-server/lib/DocWorkerMap'; import {ActiveDoc} from 'app/server/lib/ActiveDoc'; +import {AttachmentStoreProvider, IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; import {DummyAuthorizer} from 'app/server/lib/Authorizer'; +import {create} from "app/server/lib/create"; import {DocManager} from 'app/server/lib/DocManager'; import {DocSession, makeExceptionalDocSession} from 'app/server/lib/DocSession'; import {createDummyGristServer, GristServer} from 'app/server/lib/GristServer'; @@ -16,8 +18,6 @@ import * as fse from 'fs-extra'; import {tmpdir} from 'os'; import * as path from 'path'; import * as tmp from 'tmp'; -import {create} from "app/server/lib/create"; -import { AttachmentStoreProvider, IAttachmentStoreProvider } from "../../app/server/lib/AttachmentStoreProvider"; tmp.setGracefulCleanup(); diff --git a/test/server/lib/ActiveDoc.ts b/test/server/lib/ActiveDoc.ts index 43789383d3..8babb27b96 100644 --- a/test/server/lib/ActiveDoc.ts +++ b/test/server/lib/ActiveDoc.ts @@ -1,28 +1,28 @@ -import { getEnvContent } from 'app/common/ActionBundle'; -import { ServerQuery } from 'app/common/ActiveDocAPI'; -import { delay } from 'app/common/delay'; -import { BulkColValues, CellValue, fromTableDataAction } from 'app/common/DocActions'; +import {getEnvContent} from 'app/common/ActionBundle'; +import {ServerQuery} from 'app/common/ActiveDocAPI'; +import {delay} from 'app/common/delay'; +import {BulkColValues, CellValue, fromTableDataAction} from 'app/common/DocActions'; import * as gristTypes from 'app/common/gristTypes'; -import { GristObjCode } from 'app/plugin/GristData'; -import { TableData } from 'app/common/TableData'; -import { ActiveDoc } from 'app/server/lib/ActiveDoc'; -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 {GristObjCode} from 'app/plugin/GristData'; +import {TableData} from 'app/common/TableData'; +import {ActiveDoc} from 'app/server/lib/ActiveDoc'; +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 log from 'app/server/lib/log'; -import { timeoutReached } from 'app/server/lib/serverUtils'; -import { Throttle } from 'app/server/lib/Throttle'; -import { promisify } from 'bluebird'; -import { assert } from 'chai'; +import {timeoutReached} from 'app/server/lib/serverUtils'; +import {Throttle} from 'app/server/lib/Throttle'; +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 {resolve} from 'path'; import * as sinon from 'sinon'; -import { createDocTools } from 'test/server/docTools'; +import {createDocTools} from 'test/server/docTools'; import * as testUtils from 'test/server/testUtils'; -import { EnvironmentSnapshot } from 'test/server/testUtils'; +import {EnvironmentSnapshot} from 'test/server/testUtils'; import * as tmp from 'tmp'; const execFileAsync = promisify(child_process.execFile); diff --git a/test/server/lib/FilesystemAttachmentStore.ts b/test/server/lib/FilesystemAttachmentStore.ts index 248d530ad1..7f2cd31ea9 100644 --- a/test/server/lib/FilesystemAttachmentStore.ts +++ b/test/server/lib/FilesystemAttachmentStore.ts @@ -1,8 +1,9 @@ -import { FilesystemAttachmentStore } from "app/server/lib/AttachmentStore"; -import { MemoryWritableStream } from "app/server/utils/MemoryWritableStream"; -import { assert } from "chai"; -import { createTmpDir } from "../docTools"; -import { mkdtemp, pathExists } from 'fs-extra'; +import {FilesystemAttachmentStore} from 'app/server/lib/AttachmentStore'; +import {MemoryWritableStream} from 'app/server/utils/MemoryWritableStream'; +import {createTmpDir} from 'test/server/docTools'; + +import {assert} from 'chai'; +import {mkdtemp, pathExists} from 'fs-extra'; import * as stream from 'node:stream'; import * as path from 'path'; diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index 1eaa0b71e2..44359259cb 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -4,7 +4,10 @@ import {SCHEMA_VERSION} from 'app/common/schema'; import {DocWorkerMap, getDocWorkerMap} from 'app/gen-server/lib/DocWorkerMap'; import {HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; import {ActiveDoc} from 'app/server/lib/ActiveDoc'; -import { AttachmentStoreProvider, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; +import { + AttachmentStoreProvider, + IAttachmentStoreProvider +} from 'app/server/lib/AttachmentStoreProvider'; import {create} from 'app/server/lib/create'; import {DocManager} from 'app/server/lib/DocManager'; import {makeExceptionalDocSession} from 'app/server/lib/DocSession'; diff --git a/test/upgradeDocumentImpl.ts b/test/upgradeDocumentImpl.ts index f335f98487..f4cb037e4d 100644 --- a/test/upgradeDocumentImpl.ts +++ b/test/upgradeDocumentImpl.ts @@ -4,10 +4,10 @@ * Usage: * test/upgradeDocument */ -import { DocStorage } from "app/server/lib/DocStorage"; -import { DocStorageManager } from "app/server/lib/DocStorageManager"; -import { copyFile } from 'app/server/lib/docUtils'; -import { createDocTools } from 'test/server/docTools'; +import {DocStorage} from 'app/server/lib/DocStorage'; +import {DocStorageManager} from 'app/server/lib/DocStorageManager'; +import {copyFile} from 'app/server/lib/docUtils'; +import {createDocTools} from 'test/server/docTools'; import log from 'app/server/lib/log'; import * as fs from "fs"; import * as fse from "fs-extra"; From e1678ddb0332910784e243b25e8edf3ad65b84f8 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 8 Jan 2025 02:43:12 +0000 Subject: [PATCH 60/60] More import fixes --- app/server/lib/AttachmentFileManager.ts | 29 ++++++++++++---------- app/server/lib/AttachmentStore.ts | 8 +++--- app/server/lib/AttachmentStoreProvider.ts | 16 ++++++------ app/server/lib/DocApi.ts | 4 +-- app/server/lib/DocManager.ts | 2 +- app/server/lib/coreCreator.ts | 11 +++++--- app/server/utils/MemoryWritableStream.ts | 8 +++--- app/server/utils/gristify.ts | 16 ++++++------ test/server/docTools.ts | 2 +- test/server/lib/AttachmentStoreProvider.ts | 6 ++--- 10 files changed, 54 insertions(+), 48 deletions(-) diff --git a/app/server/lib/AttachmentFileManager.ts b/app/server/lib/AttachmentFileManager.ts index bf00bfa44f..f406ee12bb 100644 --- a/app/server/lib/AttachmentFileManager.ts +++ b/app/server/lib/AttachmentFileManager.ts @@ -3,14 +3,14 @@ import { DocPoolId, getDocPoolIdFromDocInfo, IAttachmentStore -} from "app/server/lib/AttachmentStore"; -import { AttachmentStoreId, IAttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; -import { checksumFileStream } from "app/server/lib/checksumFile"; -import { DocStorage } from "app/server/lib/DocStorage"; -import log from "app/server/lib/log"; -import { LogMethods } from "app/server/lib/LogMethods"; -import { MemoryWritableStream } from "app/server/utils/MemoryWritableStream"; -import { Readable } from "node:stream"; +} from 'app/server/lib/AttachmentStore'; +import {AttachmentStoreId, IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; +import {checksumFileStream} from 'app/server/lib/checksumFile'; +import {DocStorage} from 'app/server/lib/DocStorage'; +import log from 'app/server/lib/log'; +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; @@ -68,10 +68,11 @@ interface AttachmentFileManagerLogInfo { /** * 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 document database, - * which tracks attachments and where they're stored. + * Handles attachment uploading / fetching, as well as trying to ensure consistency with the local + * document database, which tracks attachments and where they're stored. * - * This class should prevent the document code from having to worry about accessing the underlying stores. + * This class should prevent the document code from having to worry about accessing the underlying + * stores. */ export class AttachmentFileManager implements IAttachmentFileManager { // _docPoolId is a critical point for security. Documents with a common pool id can access each others' attachments. @@ -84,8 +85,10 @@ export class AttachmentFileManager implements IAttachmentFileManager { /** * @param _docStorage - Storage of this manager's document. - * @param _storeProvider - Allows instantiating of stores. Should be provided except in test scenarios. - * @param _docInfo - The document this manager is for. Should be provided except in test scenarios. + * @param _storeProvider - Allows instantiating of stores. Should be provided except in test + * scenarios. + * @param _docInfo - The document this manager is for. Should be provided except in test + * scenarios. */ constructor( private _docStorage: DocStorage, diff --git a/app/server/lib/AttachmentStore.ts b/app/server/lib/AttachmentStore.ts index 15f34b4aec..7b90625a05 100644 --- a/app/server/lib/AttachmentStore.ts +++ b/app/server/lib/AttachmentStore.ts @@ -1,7 +1,7 @@ -import { joinKeySegments, StreamingExternalStorage } from "app/server/lib/ExternalStorage"; -import * as fse from "fs-extra"; -import * as stream from "node:stream"; -import * as path from "path"; +import {joinKeySegments, StreamingExternalStorage} from 'app/server/lib/ExternalStorage'; +import * as fse from 'fs-extra'; +import * as stream from 'node:stream'; +import * as path from 'path'; export type DocPoolId = string; type FileId = string; diff --git a/app/server/lib/AttachmentStoreProvider.ts b/app/server/lib/AttachmentStoreProvider.ts index dcb0fc32d9..1ae1d0f678 100644 --- a/app/server/lib/AttachmentStoreProvider.ts +++ b/app/server/lib/AttachmentStoreProvider.ts @@ -1,18 +1,18 @@ -import { IAttachmentStore } from "app/server/lib/AttachmentStore"; +import {IAttachmentStore} from 'app/server/lib/AttachmentStore'; import log from 'app/server/lib/log'; -import { ICreateAttachmentStoreOptions } from "./ICreate"; +import {ICreateAttachmentStoreOptions} from './ICreate'; export type AttachmentStoreId = string /** - * Creates an {@link IAttachmentStore} from a given store id, if the Grist installation is configured with that store's - * unique id. + * Creates an {@link IAttachmentStore} from a given store id, if the Grist installation is + * configured with that store's unique id. * - * Each store represents a specific location to store attachments at, for example a "/attachments" bucket on MinIO, - * or "/srv/grist/attachments" on the filesystem. + * Each store represents a specific location to store attachments at, for example a "/attachments" + * bucket on MinIO, or "/srv/grist/attachments" on the filesystem. * - * Attachments in Grist Documents are accompanied by the id of the store they're in, allowing Grist to store/retrieve - * them as long as that store exists on the document's installation. + * Attachments in Grist Documents are accompanied by the id of the store they're in, allowing Grist + * to store/retrieve them as long as that store exists on the document's installation. */ export interface IAttachmentStoreProvider { // Returns the store associated with the given id, returning null if no store with that id exists. diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 40535a89b4..28652552d9 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -46,8 +46,8 @@ import { import {ActiveDoc, colIdToRef as colIdToReference, getRealTableId, tableIdToRef} from "app/server/lib/ActiveDoc"; 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 {getDocPoolIdFromDocInfo} from 'app/server/lib/AttachmentStore'; +import {IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; import { assertAccess, getAuthorizedUserId, diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index ead19bb3b3..45d4dbd901 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -17,7 +17,7 @@ import {NEW_DOCUMENT_CODE} from 'app/common/UserAPI'; import {HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; import {assertAccess, Authorizer, DocAuthorizer, DummyAuthorizer, isSingleUserMode, RequestWithLogin} from 'app/server/lib/Authorizer'; -import {IAttachmentStoreProvider} from "app/server/lib/AttachmentStoreProvider"; +import {IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; import {Client} from 'app/server/lib/Client'; import {makeExceptionalDocSession, makeOptDocSession, OptDocSession} from 'app/server/lib/DocSession'; import * as docUtils from 'app/server/lib/docUtils'; diff --git a/app/server/lib/coreCreator.ts b/app/server/lib/coreCreator.ts index 19c41c336f..5a0da0039f 100644 --- a/app/server/lib/coreCreator.ts +++ b/app/server/lib/coreCreator.ts @@ -1,12 +1,15 @@ -import { AttachmentStoreCreationError, ExternalStorageAttachmentStore } from "app/server/lib/AttachmentStore"; +import { + AttachmentStoreCreationError, + ExternalStorageAttachmentStore +} from 'app/server/lib/AttachmentStore'; import { checkMinIOBucket, checkMinIOExternalStorage, configureMinIOExternalStorage } from 'app/server/lib/configureMinIOExternalStorage'; -import { makeSimpleCreator } from 'app/server/lib/ICreate'; -import { MinIOExternalStorage } from "app/server/lib/MinIOExternalStorage"; -import { Telemetry } from 'app/server/lib/Telemetry'; +import {makeSimpleCreator} from 'app/server/lib/ICreate'; +import {MinIOExternalStorage} from 'app/server/lib/MinIOExternalStorage'; +import {Telemetry} from 'app/server/lib/Telemetry'; export const makeCoreCreator = () => makeSimpleCreator({ deploymentType: 'core', diff --git a/app/server/utils/MemoryWritableStream.ts b/app/server/utils/MemoryWritableStream.ts index ecd90e2120..fd0f81f78c 100644 --- a/app/server/utils/MemoryWritableStream.ts +++ b/app/server/utils/MemoryWritableStream.ts @@ -1,8 +1,8 @@ -import { Writable } from "stream"; +import {Writable} from 'stream'; // Creates a writable stream that can be retrieved as a buffer. -// Sub-optimal implementation, as we end up with *at least* two copies in memory one in `buffers`, and one -// produced by `Buffer.concat` at the end. +// Sub-optimal implementation, as we end up with *at least* two copies in memory one in `buffers`, +// and one produced by `Buffer.concat` at the end. export class MemoryWritableStream extends Writable { private _buffers: Buffer[] = []; @@ -11,7 +11,7 @@ export class MemoryWritableStream extends Writable { } public _write(chunk: any, encoding: BufferEncoding, callback: (error?: (Error | null)) => void) { - if (typeof(chunk) == "string") { + if (typeof (chunk) == "string") { this._buffers.push(Buffer.from(chunk, encoding)); } else { this._buffers.push(chunk); diff --git a/app/server/utils/gristify.ts b/app/server/utils/gristify.ts index 80f8fb7bfc..57a0c68fd1 100644 --- a/app/server/utils/gristify.ts +++ b/app/server/utils/gristify.ts @@ -1,11 +1,11 @@ -import { ColInfoWithId } from 'app/common/DocActions'; -import { ActiveDoc } from 'app/server/lib/ActiveDoc'; -import { DocManager } from 'app/server/lib/DocManager'; -import { makeExceptionalDocSession, OptDocSession } from 'app/server/lib/DocSession'; -import { createDummyGristServer } from 'app/server/lib/GristServer'; -import { TrivialDocStorageManager } from 'app/server/lib/IDocStorageManager'; -import { DBMetadata, quoteIdent, SQLiteDB } from 'app/server/lib/SQLiteDB'; -import { AttachmentStoreProvider } from "../lib/AttachmentStoreProvider"; +import {ColInfoWithId} from 'app/common/DocActions'; +import {ActiveDoc} from 'app/server/lib/ActiveDoc'; +import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; +import {DocManager} from 'app/server/lib/DocManager'; +import {makeExceptionalDocSession, OptDocSession} from 'app/server/lib/DocSession'; +import {createDummyGristServer} from 'app/server/lib/GristServer'; +import {TrivialDocStorageManager} from 'app/server/lib/IDocStorageManager'; +import {DBMetadata, quoteIdent, SQLiteDB} from 'app/server/lib/SQLiteDB'; /** * A utility class for modifying a SQLite file to be viewed/edited with Grist. diff --git a/test/server/docTools.ts b/test/server/docTools.ts index 7a1cb29ac7..20576f1471 100644 --- a/test/server/docTools.ts +++ b/test/server/docTools.ts @@ -3,7 +3,7 @@ import {getDocWorkerMap} from 'app/gen-server/lib/DocWorkerMap'; import {ActiveDoc} from 'app/server/lib/ActiveDoc'; import {AttachmentStoreProvider, IAttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; import {DummyAuthorizer} from 'app/server/lib/Authorizer'; -import {create} from "app/server/lib/create"; +import {create} from 'app/server/lib/create'; import {DocManager} from 'app/server/lib/DocManager'; import {DocSession, makeExceptionalDocSession} from 'app/server/lib/DocSession'; import {createDummyGristServer, GristServer} from 'app/server/lib/GristServer'; diff --git a/test/server/lib/AttachmentStoreProvider.ts b/test/server/lib/AttachmentStoreProvider.ts index 8aad3f1fd6..eb14db7647 100644 --- a/test/server/lib/AttachmentStoreProvider.ts +++ b/test/server/lib/AttachmentStoreProvider.ts @@ -1,6 +1,6 @@ -import { assert } from 'chai'; -import { AttachmentStoreProvider } from "app/server/lib/AttachmentStoreProvider"; -import { makeTestingFilesystemStoreSpec } from "./FilesystemAttachmentStore"; +import {assert} from 'chai'; +import {AttachmentStoreProvider} from 'app/server/lib/AttachmentStoreProvider'; +import {makeTestingFilesystemStoreSpec} from './FilesystemAttachmentStore'; const testInstallationUUID = "FAKE-UUID"; function expectedStoreId(type: string) {