PR remarks

app/server/lib/ExternalStorage.ts

@@ -1,10 +1,11 @@
-import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot';
 import log from 'app/server/lib/log';
 import {createTmpDir} from 'app/server/lib/uploads';
+import {isAffirmative} from 'app/common/gutil';
+import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot';
+
 import {delay} from 'bluebird';
 import * as fse from 'fs-extra';
 import * as path from 'path';
-import {isAffirmative} from 'app/common/gutil';
 
 // A special token representing a deleted document, used in places where a
 // checksum is expected otherwise.
@@ -227,7 +228,10 @@ export class ChecksummedExternalStorage implements ExternalStorage {
           const expectedChecksum = await this._options.sharedHash.load(fromKey);
           // Let null docMD5s pass.  Otherwise we get stuck if redis is cleared.
           // Otherwise, make sure what we've got matches what we expect to get.
-          // S3 is eventually consistent. However, times ago, if you overwrote an object in it,
+          // AWS S3 was eventually consistent, but now has stronger guarantees:
+          // https://aws.amazon.com/blogs/aws/amazon-s3-update-strong-read-after-write-consistency/
+          //
+          // Previous to this change, if you overwrote an object in it,
           // and then read from it, you may have got an old version for some time.
           // We are confident this should not be the case anymore, though this has to be studied carefully.
           // If a snapshotId was specified, we can skip this check.

test/server/lib/HostedStorageManager.ts

@@ -25,7 +25,7 @@ import {createClient, RedisClient} from 'redis';
 import * as sinon from 'sinon';
 import {createInitialDb, removeConnection, setUpDB} from 'test/gen-server/seed';
 import {createTmpDir, getGlobalPluginManager} from 'test/server/docTools';
-import {setTmpLogLevel, useFixtureDoc} from 'test/server/testUtils';
+import {EnvironmentSnapshot, setTmpLogLevel, useFixtureDoc} from 'test/server/testUtils';
 import {waitForIt} from 'test/server/wait';
 import uuidv4 from "uuid/v4";
 
@@ -377,6 +377,7 @@ describe('HostedStorageManager', function() {
     describe(storage, function() {
 
       const sandbox = sinon.createSandbox();
+      let oldEnv: EnvironmentSnapshot;
 
       const workerId = 'dw17';
       let cli: RedisClient;
@@ -387,6 +388,7 @@ describe('HostedStorageManager', function() {
       before(async function() {
         if (!process.env.TEST_REDIS_URL) { this.skip(); return; }
         cli = createClient(process.env.TEST_REDIS_URL);
+        oldEnv = new EnvironmentSnapshot();
         await cli.flushdbAsync();
         workers = new DocWorkerMap([cli]);
         await workers.addWorker({
@@ -450,7 +452,7 @@ describe('HostedStorageManager', function() {
       });
 
       afterEach(async function() {
-        delete process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH;
+        oldEnv.restore();
         sandbox.restore();
         if (store) {
           await store.end();