diff --git a/app/gen-server/lib/DocWorkerMap.ts b/app/gen-server/lib/DocWorkerMap.ts index 3a40a947..4fa20a69 100644 --- a/app/gen-server/lib/DocWorkerMap.ts +++ b/app/gen-server/lib/DocWorkerMap.ts @@ -580,8 +580,10 @@ let dummyDocWorkerMap: DummyDocWorkerMap|null = null; export function getDocWorkerMap(): IDocWorkerMap { if (process.env.REDIS_URL) { + log.info("Creating Redis-based DocWorker"); return new DocWorkerMap(); } else { + log.info("Creating local/dummy DocWorker"); dummyDocWorkerMap = dummyDocWorkerMap || new DummyDocWorkerMap(); return dummyDocWorkerMap; } diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index 6a9f7c6e..a11c2f34 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -638,6 +638,7 @@ export class DocManager extends EventEmitter { throw new Error('Grist docs must be uploaded individually'); } const first = uploadInfo.files[0].origName; + log.debug(`DocManager._doImportDoc: Received doc with name ${first}`); const ext = extname(first); const basename = path.basename(first, ext).trim() || "Untitled upload"; let id: string; @@ -662,6 +663,7 @@ export class DocManager extends EventEmitter { } await options.register?.(id, basename); if (ext === '.grist') { + log.debug(`DocManager._doImportDoc: Importing .grist doc`); // If the import is a grist file, copy it to the docs directory. // TODO: We should be skeptical of the upload file to close a possible // security vulnerability. See https://phab.getgrist.com/T457. diff --git a/app/server/lib/HostedMetadataManager.ts b/app/server/lib/HostedMetadataManager.ts index f49a545b..c108c5ef 100644 --- a/app/server/lib/HostedMetadataManager.ts +++ b/app/server/lib/HostedMetadataManager.ts @@ -1,6 +1,9 @@ -import {DocumentMetadata, HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; +import {DocumentMetadata} from 'app/gen-server/lib/homedb/HomeDBManager'; import log from 'app/server/lib/log'; +// Callback that persists the updated metadata to storage for each document. +export type SaveDocsMetadataFunc = (metadata: { [docId: string]: DocumentMetadata }) => Promise; + /** * HostedMetadataManager handles pushing document metadata changes to the Home database when * a doc is updated. Currently updates doc updatedAt time and usage. @@ -29,7 +32,7 @@ export class HostedMetadataManager { * Create an instance of HostedMetadataManager. * The minPushDelay is the default delay in seconds between metadata pushes to the database. */ - constructor(private _dbManager: HomeDBManager, minPushDelay: number = 60) { + constructor(private _saveDocsMetadata: SaveDocsMetadataFunc, minPushDelay: number = 60) { this._minPushDelayMs = minPushDelay * 1000; } @@ -68,7 +71,7 @@ export class HostedMetadataManager { } public setDocsMetadata(docUpdateMap: {[docId: string]: DocumentMetadata}): Promise { - return this._dbManager.setDocsMetadata(docUpdateMap); + return this._saveDocsMetadata(docUpdateMap); } /** diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts index 443ab52b..7aa02696 100644 --- a/app/server/lib/HostedStorageManager.ts +++ b/app/server/lib/HostedStorageManager.ts @@ -8,7 +8,6 @@ import {DocumentUsage} from 'app/common/DocUsage'; import {buildUrlId, parseUrlId} from 'app/common/gristUrls'; import {KeyedOps} from 'app/common/KeyedOps'; import {DocReplacementOptions, NEW_DOCUMENT_CODE} from 'app/common/UserAPI'; -import {HomeDBManager} from 'app/gen-server/lib/homedb/HomeDBManager'; import {checksumFile} from 'app/server/lib/checksumFile'; import {DocSnapshotInventory, DocSnapshotPruner} from 'app/server/lib/DocSnapshots'; import {IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; @@ -19,14 +18,15 @@ import { ExternalStorageCreator, ExternalStorageSettings, Unchanged } from 'app/server/lib/ExternalStorage'; -import {HostedMetadataManager} from 'app/server/lib/HostedMetadataManager'; +import {HostedMetadataManager, SaveDocsMetadataFunc} from 'app/server/lib/HostedMetadataManager'; import {EmptySnapshotProgress, IDocStorageManager, SnapshotProgress} from 'app/server/lib/IDocStorageManager'; import {LogMethods} from "app/server/lib/LogMethods"; import {fromCallback} from 'app/server/lib/serverUtils'; import * as fse from 'fs-extra'; import * as path from 'path'; import uuidv4 from "uuid/v4"; -import { OpenMode, SQLiteDB } from './SQLiteDB'; +import {OpenMode, SQLiteDB} from './SQLiteDB'; +import {Features} from "app/common/Features"; // Check for a valid document id. const docIdRegex = /^[-=_\w~%]+$/; @@ -52,6 +52,13 @@ function checkValidDocId(docId: string): void { } } +export interface HostedStorageCallbacks { + // Saves the given metadata for the specified documents. + setDocsMetadata: SaveDocsMetadataFunc, + // Retrieves account features enabled for the given document. + getDocFeatures: (docId: string) => Promise +} + export interface HostedStorageOptions { secondsBeforePush: number; secondsBeforeFirstRetry: number; @@ -133,7 +140,7 @@ export class HostedStorageManager implements IDocStorageManager { private _docWorkerId: string, private _disableS3: boolean, private _docWorkerMap: IDocWorkerMap, - dbManager: HomeDBManager, + callbacks: HostedStorageCallbacks, createExternalStorage: ExternalStorageCreator, options: HostedStorageOptions = defaultOptions ) { @@ -144,7 +151,7 @@ export class HostedStorageManager implements IDocStorageManager { if (!externalStoreDoc) { this._disableS3 = true; } const secondsBeforePush = options.secondsBeforePush; if (options.pushDocUpdateTimes) { - this._metadataManager = new HostedMetadataManager(dbManager); + this._metadataManager = new HostedMetadataManager(callbacks.setDocsMetadata); } this._uploads = new KeyedOps(key => this._pushToS3(key), { delayBeforeOperationMs: secondsBeforePush * 1000, @@ -178,7 +185,7 @@ export class HostedStorageManager implements IDocStorageManager { return path.join(dir, 'meta.json'); }, async docId => { - const features = await dbManager.getDocFeatures(docId); + const features = await callbacks.getDocFeatures(docId); return features?.snapshotWindow; }, ); @@ -639,7 +646,7 @@ export class HostedStorageManager implements IDocStorageManager { const existsLocally = await fse.pathExists(this.getPath(docName)); if (existsLocally) { - if (!docStatus.docMD5 || docStatus.docMD5 === DELETED_TOKEN) { + if (!docStatus.docMD5 || docStatus.docMD5 === DELETED_TOKEN || docStatus.docMD5 === 'unknown') { // New doc appears to already exist, but may not exist in S3. // Let's check. const head = await this._ext.head(docName); diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts index 285bea29..0678d858 100644 --- a/test/server/lib/HostedStorageManager.ts +++ b/test/server/lib/HostedStorageManager.ts @@ -1,12 +1,13 @@ import {ErrorOrValue, freezeError, mapGetOrSet, MapWithTTL} from 'app/common/AsyncCreate'; import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot'; import {SCHEMA_VERSION} from 'app/common/schema'; -import {DocWorkerMap} from 'app/gen-server/lib/DocWorkerMap'; +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 {create} from 'app/server/lib/create'; import {DocManager} from 'app/server/lib/DocManager'; import {makeExceptionalDocSession} from 'app/server/lib/DocSession'; +import {IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; import { DELETED_TOKEN, ExternalStorage, ExternalStorageCreator, @@ -274,7 +275,7 @@ class TestStore { public constructor( private _localDirectory: string, private _workerId: string, - private _workers: DocWorkerMap, + private _workers: IDocWorkerMap, private _externalStorageCreate: ExternalStorageCreator) { } @@ -962,6 +963,98 @@ describe('HostedStorageManager', function() { }); }); } + + describe('minio-without-redis', async () => { + const workerId = 'dw17'; + let tmpDir: string; + let oldEnv: EnvironmentSnapshot; + let docWorkerMap: IDocWorkerMap; + let externalStorageCreate: ExternalStorageCreator; + let defaultParams: ConstructorParameters; + + before(async function() { + tmpDir = await createTmpDir(); + oldEnv = new EnvironmentSnapshot(); + // Disable Redis + delete process.env.REDIS_URL; + + const storage = create?.getStorageOptions?.('minio'); + const creator = storage?.create; + if (!creator || !storage?.check()) { + return this.skip(); + } + externalStorageCreate = creator; + }); + + after(async () => { + oldEnv.restore(); + }); + + beforeEach(async function() { + // With Redis disabled, this should be the non-redis version of IDocWorkerMap (DummyDocWorkerMap) + docWorkerMap = getDocWorkerMap(); + await docWorkerMap.addWorker({ + id: workerId, + publicUrl: "none", + internalUrl: "none", + }); + await docWorkerMap.setWorkerAvailability(workerId, true); + + defaultParams = [ + tmpDir, + workerId, + false, + docWorkerMap, + { + setDocsMetadata: async (metadata) => {}, + getDocFeatures: async (docId) => undefined, + }, + externalStorageCreate, + ]; + }); + + it("doesn't wipe local docs when they exist on disk but not remote storage", async function() { + const storageManager = new HostedStorageManager(...defaultParams); + + const docId = "NewDoc"; + + const path = storageManager.getPath(docId); + // Simulate an uploaded .grist file. + await fse.writeFile(path, ""); + + await storageManager.prepareLocalDoc(docId); + + assert.isTrue(await fse.pathExists(path)); + }); + + it("fetches remote docs if they don't exist locally", async function() { + const testStore = new TestStore( + tmpDir, + workerId, + docWorkerMap, + externalStorageCreate + ); + + let docName: string = ""; + let docPath: string = ""; + + await testStore.run(async () => { + const newDoc = await testStore.docManager.createNewEmptyDoc(docSession, "NewRemoteDoc"); + docName = newDoc.docName; + docPath = testStore.storageManager.getPath(docName); + }); + + // This should be safe since testStore.run closes everything down. + await fse.remove(docPath); + assert.isFalse(await fse.pathExists(docPath)); + + await testStore.run(async () => { + await testStore.docManager.fetchDoc(docSession, docName); + }); + + assert.isTrue(await fse.pathExists(docPath)); + }); + }); }); // This is a performance test, to check if the backup settings are plausible.