Fixes an import error when using S3/Minio with no redis (#1224)

## Context

Error is caused due to these steps:
- File is uploaded to Home server and attempts to import
- Import ends up in `claimDocument` in `HostedStorageManager`
- Tries to read doc metadata from DocWorkerMap, gets 'unknown' as md5 hash
- Thinks local doc is out of date and erases it.
- Downloads a non-existent file from S3, so import fails as it has no data.

## Proposed solution

This fixes it by checking for DummyDocWorker's special 'unknown' MD5, forcing an S3 check.

## Related issues

https://community.getgrist.com/t/no-metadata-for-imported-grist-document/6029/32

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;
   }

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.

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<any>;
+
 /**
  * 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<any> {
-    return this._dbManager.setDocsMetadata(docUpdateMap);
+    return this._saveDocsMetadata(docUpdateMap);
   }
 
   /**

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<Features | undefined>
+}
+
 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);

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<typeof HostedStorageManager>;
+
+    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.