Merge branch 'bugfix/ARSN-398-doNotRefreshGapBuildingIfDisabled' into user/jonathan/S3C-3926

- Fix the situation where gap building is disabled by
  `_saveBuildingGap()` but we attempted to reset the building gap state
  anyway.

- Introduce a new state 'Expired' that can be differentiated from
  'Disabled': it makes `getGapBuildingValidityPeriodMs()` return 0
  instead of 'null' to hint the listing backend that it should trigger
  a new listing.

lib/algos/cache/GapCache.ts

@@ -346,4 +346,18 @@ export default class GapCache implements GapCacheInterface {
     toArray(): GapSetEntry[] {
         return this._exposedGaps.toArray();
     }
+
+    /**
+     * Clear all exposed and staging gaps from the cache.
+     *
+     * Note: retains invalidating updates from removeOverlappingGaps()
+     * for correctness of gaps inserted afterwards.
+     *
+     * @return {undefined}
+     */
+    clear(): void {
+        this._stagingUpdates.newGaps = new GapSet(this.maxGapWeight);
+        this._frozenUpdates.newGaps = new GapSet(this.maxGapWeight);
+        this._exposedGaps = new GapSet(this.maxGapWeight);
+    }
 }

lib/algos/list/delimiterMaster.ts

@@ -73,13 +73,14 @@ type GapCachingInfo = GapCachingInfo_NoGapCache
 
 
 export const enum GapBuildingState {
-    Disabled = 0, // no gap cache or not allowed to build due to exposure delay timeout
+    Disabled = 0, // no gap cache or no gap building needed (e.g. in V1 versioning format)
     NotBuilding = 1, // not currently building a gap (i.e. not listing within a gap)
     Building = 2, // currently building a gap (i.e. listing within a gap)
+    Expired = 3, // not allowed to build due to exposure delay timeout
 };
 
-type GapBuildingInfo_Disabled = {
-    state: GapBuildingState.Disabled;
+type GapBuildingInfo_NothingToBuild = {
+    state: GapBuildingState.Disabled | GapBuildingState.Expired;
 };
 
 type GapBuildingParams = {
@@ -118,7 +119,7 @@ type GapBuildingInfo_Building = {
     gapWeight: number;
 };
 
-type GapBuildingInfo = GapBuildingInfo_Disabled
+type GapBuildingInfo = GapBuildingInfo_NothingToBuild
     | GapBuildingInfo_NotBuilding
     | GapBuildingInfo_Building;
 
@@ -214,6 +215,8 @@ export class DelimiterMaster extends Delimiter {
         switch (this._gapBuilding.state) {
         case GapBuildingState.Disabled:
             return null;
+        case GapBuildingState.Expired:
+            return 0;
         case GapBuildingState.NotBuilding:
             gapBuilding = <GapBuildingInfo_NotBuilding> this._gapBuilding;
             break;
@@ -314,6 +317,7 @@ export class DelimiterMaster extends Delimiter {
     _checkGapOnMasterDeleteMarker(key: string): FilterReturnValue {
         switch (this._gapBuilding.state) {
         case GapBuildingState.Disabled:
+        case GapBuildingState.Expired:
             break;
         case GapBuildingState.NotBuilding:
             this._createBuildingGap(key, 1);
@@ -494,16 +498,23 @@ export class DelimiterMaster extends Delimiter {
         return params;
     }
 
-    _saveBuildingGap(): void {
+    /**
+     * Save the gap being built if allowed (i.e. still within the
+     * allocated exposure time window).
+     *
+     * @return {boolean} - true if the gap was saved, false if we are
+     * outside the allocated exposure time window.
+     */
+    _saveBuildingGap(): boolean {
         const { gapCache, params, gap, gapWeight } =
               <GapBuildingInfo_Building> this._gapBuilding;
         const totalElapsed = Date.now() - params.initTimestamp;
         if (totalElapsed >= gapCache.exposureDelayMs) {
             this._gapBuilding = {
-                state: GapBuildingState.Disabled,
+                state: GapBuildingState.Expired,
             };
             this._refreshedBuildingParams = null;
-            return;
+            return false;
         }
         const { firstKey, lastKey, weight } = gap;
         gapCache.setGap(firstKey, lastKey, weight);
@@ -518,6 +529,7 @@ export class DelimiterMaster extends Delimiter {
             },
             gapWeight,
         };
+        return true;
     }
 
     /**
@@ -569,7 +581,10 @@ export class DelimiterMaster extends Delimiter {
             // only set gaps that are significant enough in weight and
             // with a non-empty extension
             if (gapWeight >= params.minGapWeight && gap.weight > 0) {
-                this._saveBuildingGap();
+                // we're done if we were not allowed to save the gap
+                if (!this._saveBuildingGap()) {
+                    return;
+                }
                 // params may have been refreshed, reload them
                 gapBuilding = <GapBuildingInfo_Building> this._gapBuilding;
                 params = gapBuilding.params;

tests/unit/algos/cache/GapCache.spec.ts

@@ -50,6 +50,52 @@ describe('GapCache', () => {
         });
     });
 
+    describe('clear()', () => {
+        it('should clear all exposed gaps', async () => {
+            gapCache.setGap('bar', 'baz', 10);
+            gapCache.setGap('qux', 'quz', 20);
+            await new Promise(resolve => setTimeout(resolve, 300));
+
+            expect(await gapCache.lookupGap('ape', 'zoo')).toEqual(
+                { firstKey: 'bar', lastKey: 'baz', weight: 10 }
+            );
+            gapCache.clear();
+            expect(await gapCache.lookupGap('ape', 'zoo')).toBeNull();
+        });
+
+        it('should clear all staging gaps', async () => {
+            gapCache.setGap('bar', 'baz', 10);
+            gapCache.setGap('qux', 'quz', 20);
+
+            gapCache.clear();
+            await new Promise(resolve => setTimeout(resolve, 300));
+
+            expect(await gapCache.lookupGap('ape', 'zoo')).toBeNull();
+        });
+
+        it('should keep existing invalidating updates against the next new gaps', async () => {
+            // invalidate future gaps containing 'dog'
+            expect(gapCache.removeOverlappingGaps(['dog'])).toEqual(0);
+
+            // then, clear the cache
+            gapCache.clear();
+
+            // wait for 50ms (half of exposure delay of 100ms) before
+            // setting a new gap overlapping with 'dog'
+            await new Promise(resolve => setTimeout(resolve, 50));
+            gapCache.setGap('cat', 'fox', 10);
+
+            // also set a non-overlapping gap to make sure it is not invalidated
+            gapCache.setGap('goat', 'hog', 20);
+
+            // wait an extra 250ms to ensure all valid gaps have been exposed
+            await new Promise(resolve => setTimeout(resolve, 250));
+            // the next gap is indeed 'goat'... because 'cat'... should have been invalidated
+            expect(await gapCache.lookupGap('bat', 'zoo')).toEqual(
+                { firstKey: 'goat', lastKey: 'hog', weight: 20 });
+        });
+    });
+
     it('should expose gaps after at least exposureDelayMs milliseconds', async () => {
         gapCache.setGap('bar', 'baz', 10);
         expect(await gapCache.lookupGap('ape', 'cat')).toBeNull();

tests/unit/algos/list/delimiterMaster.spec.ts

@@ -1186,6 +1186,11 @@ describe('DelimiterMaster listing algorithm: gap caching and lookup', () => {
         resumeFromState = filterEntries(listing, 'Ddv Ddv Ddv Vvv', 'ass ass ass ass',
                                         resumeFromState);
         expect(gapCache.toArray()).toEqual(gapsArray);
+        // gap building should be in expired state
+        expect(listing._gapBuilding.state).toEqual(GapBuildingState.Expired);
+        // remaining validity period should still be 0 because gap building has expired
+        validityPeriod = listing.getGapBuildingValidityPeriodMs();
+        expect(validityPeriod).toEqual(0);
 
         // we should still be able to skip over the existing cached gaps
         expect(listing._gapCaching.state).toEqual(GapCachingState.GapLookupInProgress);