impr: ARSN-389 change contract of skipping() API

Instead of returning a "prefix" for the listing task to skip over,
directly return the key on which to skip and continue the listing.

It is both more natural as well as needed to implement skipping over
cached "gaps" of deleted objects.

Note that it could even be more powerful to return the type of query
param to apply for the next listing ('gt' or 'gte'), but it would be
more complex to implement with little practical benefit, so instead we
add a null byte at the end of the returned key to skip to, whenever we
want a 'gt' behavior from the returned 'gte' key.

lib/algos/list/Extension.js

@@ -101,12 +101,15 @@ class Extension {
     }
 
     /**
-     * Provides the insight into why filter is skipping an entry. This could be
-     * because it is skipping a range of delimited keys or a range of specific
-     * version when doing master version listing.
+     * Provides the next key at which the listing task is allowed to skip to.
+     * This could allow to skip over:
+     * - a key prefix ending with the delimiter
+     * - all remaining versions of an object when doing a current
+     *   versions listing in v0 format
+     * - a cached "gap" of deleted objects when doing a current
+     *   versions listing in v0 format
      *
-     * @return {string} - the insight: a common prefix or a master key,
-     *                                 or SKIP_NONE if there is no insight
+     * @return {string} - the next key at which the listing task is allowed to skip to
      */
     skipping() {
         return SKIP_NONE;

lib/algos/list/MPU.js

@@ -1,7 +1,7 @@
 'use strict'; // eslint-disable-line strict
 
 const { inc, checkLimit, listingParamsMasterKeysV0ToV1,
-    FILTER_END, FILTER_ACCEPT } = require('./tools');
+    FILTER_END, FILTER_ACCEPT, SKIP_NONE } = require('./tools');
 const DEFAULT_MAX_KEYS = 1000;
 const VSConst = require('../../versioning/constants').VersioningConstants;
 const { DbPrefixes, BucketVersioningKeyFormat } = VSConst;
@@ -163,7 +163,7 @@ class MultipartUploads {
     }
 
     skipping() {
-        return '';
+        return SKIP_NONE;
     }
 
     /**

lib/algos/list/delimiter.ts

@@ -305,7 +305,7 @@ export class Delimiter extends Extension {
         switch (this.state.id) {
         case DelimiterFilterStateId.SkippingPrefix:
             const { prefix } = <DelimiterFilterState_SkippingPrefix> this.state;
-            return prefix;
+            return inc(prefix);
 
         default:
             return SKIP_NONE;

lib/algos/list/delimiterMaster.ts

@@ -9,7 +9,7 @@ import {
 const Version = require('../../versioning/Version').Version;
 const VSConst = require('../../versioning/constants').VersioningConstants;
 const { BucketVersioningKeyFormat } = VSConst;
-const { FILTER_ACCEPT, FILTER_SKIP, FILTER_END } = require('./tools');
+const { FILTER_ACCEPT, FILTER_SKIP, FILTER_END, inc } = require('./tools');
 
 const VID_SEP = VSConst.VersionId.Separator;
 const { DbPrefixes } = VSConst;
@@ -181,7 +181,7 @@ export class DelimiterMaster extends Delimiter {
         switch (this.state.id) {
         case DelimiterMasterFilterStateId.SkippingVersionsV0:
             const { masterKey } = <DelimiterMasterFilterState_SkippingVersionsV0> this.state;
-            return masterKey + VID_SEP;
+            return masterKey + inc(VID_SEP);
 
         default:
             return super.skippingBase();

lib/algos/list/delimiterVersions.ts

@@ -452,11 +452,14 @@ export class DelimiterVersions extends Extension {
         switch (this.state.id) {
         case DelimiterVersionsFilterStateId.SkippingPrefix:
             const { prefix } = <DelimiterVersionsFilterState_SkippingPrefix> this.state;
-            return prefix;
+            return inc(prefix);
 
         case DelimiterVersionsFilterStateId.SkippingVersions:
             const { gt } = <DelimiterVersionsFilterState_SkippingVersions> this.state;
-            return gt;
+            // the contract of skipping() is to return the first key
+            // that can be skipped to, so adding a null byte to skip
+            // over the existing versioned key set in 'gt'
+            return `${gt}\0`;
 
         default:
             return SKIP_NONE;

lib/algos/list/skip.js

@@ -52,21 +52,21 @@ class Skip {
         assert(this.skipRangeCb);
 
         const filteringResult = this.extension.filter(entry);
-        const skippingRange = this.extension.skipping();
+        const skipTo = this.extension.skipping();
 
         if (filteringResult === FILTER_END) {
             this.listingEndCb();
         } else if (filteringResult === FILTER_SKIP
-                   && skippingRange !== SKIP_NONE) {
+                   && skipTo !== SKIP_NONE) {
             if (++this.streakLength >= MAX_STREAK_LENGTH) {
                 let newRange;
-                if (Array.isArray(skippingRange)) {
+                if (Array.isArray(skipTo)) {
                     newRange = [];
-                    for (let i = 0; i < skippingRange.length; ++i) {
-                        newRange.push(this._inc(skippingRange[i]));
+                    for (let i = 0; i < skipTo.length; ++i) {
+                        newRange.push(skipTo[i]);
                     }
                 } else {
-                    newRange = this._inc(skippingRange);
+                    newRange = skipTo;
                 }
                 /* Avoid to loop on the same range again and again. */
                 if (newRange === this.gteParams) {
@@ -79,16 +79,6 @@ class Skip {
             this.streakLength = 0;
         }
     }
-
-    _inc(str) {
-        if (!str) {
-            return str;
-        }
-        const lastCharValue = str.charCodeAt(str.length - 1);
-        const lastCharNewValue = String.fromCharCode(lastCharValue + 1);
-
-        return `${str.slice(0, str.length - 1)}${lastCharNewValue}`;
-    }
 }
 
 

tests/unit/algos/list/delimiter.spec.js

@@ -727,7 +727,7 @@ function getTestListing(mdParams, data, vFormat) {
                 });
             }
             assert.strictEqual(delimiter.skipping(),
-                `${vFormat === 'v1' ? DbPrefixes.Master : ''}foo/`);
+                `${vFormat === 'v1' ? DbPrefixes.Master : ''}foo0`);
         });
 
         tests.forEach(test => {