wip

This reverts commit f62c3d22ed.

lib/algos/list/delimiter.js

@@ -2,9 +2,10 @@
 
 const Extension = require('./Extension').default;
 const { inc, listingParamsMasterKeysV0ToV1,
-    FILTER_END, FILTER_ACCEPT, FILTER_SKIP } = require('./tools');
+    FILTER_END, FILTER_ACCEPT, FILTER_SKIP, SKIP_NONE } = require('./tools');
 const VSConst = require('../../versioning/constants').VersioningConstants;
 const { DbPrefixes, BucketVersioningKeyFormat } = VSConst;
+const VID_SEP = VSConst.VersionId.Separator;
 
 /**
  * Find the common prefix in the path
@@ -64,6 +65,7 @@ class Delimiter extends Extension {
         this.maxKeys = parameters.maxKeys || 1000;
         this.startAfter = parameters.startAfter;
         this.continuationToken = parameters.continuationToken;
+        this.prefixToSkip = SKIP_NONE;
         this.alphabeticalOrder =
           typeof parameters.alphabeticalOrder !== 'undefined' ?
               parameters.alphabeticalOrder : true;
@@ -189,6 +191,8 @@ class Delimiter extends Extension {
                 && key <= this[this.nextContinueMarker])) {
             return FILTER_SKIP;
         }
+        // reset `prefixToSkip`, it may be updated if `addCommonPrefix()` is called
+        this.prefixToSkip = SKIP_NONE;
         if (this.delimiter) {
             const baseIndex = this.prefix ? this.prefix.length : 0;
             const delimiterIndex = key.indexOf(this.delimiter, baseIndex);
@@ -208,13 +212,14 @@ class Delimiter extends Extension {
      */
     addCommonPrefix(key, index) {
         const commonPrefix = getCommonPrefix(key, this.delimiter, index);
+        this[this.nextContinueMarker] = key;
+        this.prefixToSkip = commonPrefix;
         if (this.CommonPrefixes.indexOf(commonPrefix) === -1
                 && this[this.nextContinueMarker] !== commonPrefix) {
             if (this._reachedMaxKeys()) {
                 return FILTER_END;
             }
             this.CommonPrefixes.push(commonPrefix);
-            this[this.nextContinueMarker] = commonPrefix;
             ++this.keys;
             return FILTER_ACCEPT;
         }
@@ -229,7 +234,7 @@ class Delimiter extends Extension {
      *                    that it's enough and should move on
      */
     skippingV0() {
-        return this[this.nextContinueMarker];
+        return this.prefixToSkip;
     }
 
     /**
@@ -240,7 +245,7 @@ class Delimiter extends Extension {
      *                    that it's enough and should move on
      */
     skippingV1() {
-        return DbPrefixes.Master + this[this.nextContinueMarker];
+        return this.prefixToSkip ? DbPrefixes.Master + this.prefixToSkip : SKIP_NONE;
     }
 
     /**
@@ -271,4 +276,4 @@ class Delimiter extends Extension {
     }
 }
 
-module.exports = { Delimiter, getCommonPrefix };
+module.exports = { Delimiter };

lib/algos/list/delimiterMaster.js

@@ -41,7 +41,6 @@ class DelimiterMaster extends Delimiter {
             },
             [BucketVersioningKeyFormat.v1]: {
                 filter: this.filterV1,
-                skipping: this.skippingV1,
             },
         }[this.vFormat]);
     }
@@ -62,10 +61,6 @@ class DelimiterMaster extends Delimiter {
         let key = obj.key;
         const value = obj.value;
 
-        if (key === this.prefix) {
-            return this.addContents(key, value);
-        }
-
         if (key.startsWith(DbPrefixes.Replay)) {
             this.inReplayPrefix = true;
             return FILTER_SKIP;
@@ -94,17 +89,17 @@ class DelimiterMaster extends Delimiter {
              *   in the constructor and comparing to NextMarker is the only
              *   way to know  we should not accept this version. This test is
              *   not redundant with the one at the beginning of this function,
-             *   we are comparing here the key without the version suffix,
-             * - key startsWith the previous NextMarker happens because we set
-             *   NextMarker to the common prefix instead of the whole key
-             *   value. (TODO: remove this test once ZENKO-1048 is fixed)
+             *   we are comparing here the key without the version suffix.
              *   */
-            if (key === this.prvKey || key === this[this.nextContinueMarker] ||
-                (this.delimiter && key.startsWith(this[this.nextContinueMarker]))) {
+            if (key === this.prvKey || key === this[this.nextContinueMarker]) {
                 /* master version already filtered */
                 return FILTER_SKIP;
             }
         }
+        // If addCommonPrefix() gets called in this function, it will
+        // set `prefixToSkip` to the current prefix, otherwise in the
+        // general case we may skip all further versions of that key.
+        this.prefixToSkip = key + VID_SEP;
         if (Version.isPHD(value)) {
             /* master version is a PHD version, we want to wait for the next
              * one:
@@ -165,34 +160,11 @@ class DelimiterMaster extends Delimiter {
         return super.filter(obj);
     }
 
-    skippingBase() {
-        if (this[this.nextContinueMarker]) {
-            // next marker or next continuation token:
-            // - foo/ : skipping foo/
-            // - foo  : skipping foo.
-            const index = this[this.nextContinueMarker].
-                lastIndexOf(this.delimiter);
-            if (index === this[this.nextContinueMarker].length - 1) {
-                return this[this.nextContinueMarker];
-            }
-            return this[this.nextContinueMarker] + VID_SEP;
-        }
-        return SKIP_NONE;
-    }
-
     skippingV0() {
         if (this.inReplayPrefix) {
             return DbPrefixes.Replay;
         }
-        return this.skippingBase();
-    }
-
-    skippingV1() {
-        const skipTo = this.skippingBase();
-        if (skipTo === SKIP_NONE) {
-            return SKIP_NONE;
-        }
-        return DbPrefixes.Master + skipTo;
+        return super.skippingV0();
     }
 }
 

lib/algos/list/delimiterVersions.js

@@ -238,13 +238,7 @@ class DelimiterVersions extends Delimiter {
         if (this.inReplayPrefix) {
             return DbPrefixes.Replay;
         }
-        if (this.NextMarker) {
-            const index = this.NextMarker.lastIndexOf(this.delimiter);
-            if (index === this.NextMarker.length - 1) {
-                return this.NextMarker;
-            }
-        }
-        return SKIP_NONE;
+        return this.prefixToSkip;
     }
 
     skippingV1() {

package.json

@@ -62,7 +62,6 @@
     "@types/jest": "^27.4.1",
     "@types/node": "^17.0.21",
     "@types/xml2js": "^0.4.11",
-    "chance": "^1.1.8",
     "eslint": "^8.12.0",
     "eslint-config-airbnb": "6.2.0",
     "eslint-config-scality": "scality/Guidelines#7.10.2",

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

@@ -1,9 +1,7 @@
 'use strict'; // eslint-disable-line strict
 
 const assert = require('assert');
-const chance = require('chance').Chance(); // eslint-disable-line
 
-const { getCommonPrefix } = require('../../../../lib/algos/list/delimiter');
 const DelimiterMaster =
     require('../../../../lib/algos/list/delimiterMaster').DelimiterMaster;
 const {
@@ -18,6 +16,8 @@ const Version = require('../../../../lib/versioning/Version').Version;
 const { generateVersionId } = require('../../../../lib/versioning/VersionID');
 const { DbPrefixes } = VSConst;
 const zpad = require('../../helpers').zpad;
+
+
 const VID_SEP = VSConst.VersionId.Separator;
 const EmptyResult = {
     CommonPrefixes: [],
@@ -46,111 +46,6 @@ function getListingKey(key, vFormat) {
     return assert.fail(`bad vFormat ${vFormat}`);
 }
 
-const MAX_STREAK_LENGTH = 100;
-
-function createStreakState(prefix, vFormat) {
-    return {
-        vFormat,
-        prefix,
-        prefixes: new Set(),
-        params: {},
-        streakLength: 0,
-        gteparams: null,
-    };
-}
-
-/**
- * Simplified version of logic found in Metadata's RepdServer. When MAX_STREAK_LENGTH (typically 100)
- * is reached, we attempt to skip to the next listing range as a performance optimization.
- * @param {Integer} filteringResult - 0, 1, -1 indicating if we can skip to the next character range.
- * @param {String} skippingRange - lower bound that the listing can begin from.
- * @returns {undefined}
- */
-/* eslint-disable no-param-reassign */
-function handleStreak(filteringResult, skippingRange, state) {
-    if (filteringResult < 0) {
-        // readStream would be destroyed. In this mock, we just continue.
-    } else if (filteringResult === 0 && skippingRange) {
-        // check if MAX_STREAK_LENGTH consecutive keys have been
-        // skipped
-        if (++state.streakLength === MAX_STREAK_LENGTH) {
-            if (Array.isArray(skippingRange)) {
-                // With synchronized listing, the listing
-                // algo backend skipping() function must
-                // return as many skip keys as listing
-                // param sets.
-                for (let i = 0; i < skippingRange.length; ++i) {
-                    state.params[i].gte = inc(skippingRange[i]);
-                }
-            } else {
-                state.params.gte = inc(skippingRange);
-            }
-            if (state.gteparams && state.gteparams === state.params.gte) {
-                state.streakLength = 1;
-            } else {
-                // stop listing this key range
-                state.gteparams = state.params.gte;
-            }
-        }
-    } else {
-        state.streakLength = 1;
-    }
-}/* eslint-enable */
-
-/**
- * Generate a random number of versioned keys.
- * @param {String} masterKey - base key used to derive version keys
- * @param {Integer} numKeys - how many versioned keys to generate
- * @param {Object} state - test case state
- * @yields {Object} - { key, isDeleteMarker }
- * @returns {undefined}
- */
-function *generateVersionedKeys(masterKey, numKeys, state) {
-    for (let i = 0; i < numKeys; i++) {
-        yield {
-            key: `${masterKey}${VID_SEP}${zpad(i)}`,
-            isDeleteMarker: state.vFormat === 'v0' && chance.bool(),
-            canSkip: true,
-        };
-    }
-}
-
-/**
- * Generate raw listing keys in alphabetical order.
- * @param {Integer} count - how many keys to generate.
- * @param {Object} state - state for test run.
- * @yields {Object} - { key, isDeleteMarker }
- */
-
-function *generateKeys(count, state) {
-    let idx = 0;
-    let masterKey = '_Common-Prefix/';
-    yield { key: masterKey, isDeleteMarker: false, canSkip: false };
-
-    idx++;
-    while (idx < count) {
-        masterKey = `_Common-Prefix/${zpad(idx)}`;
-        const masterKeyWithSubprefix = `_Common-Prefix/${zpad(idx)}/${zpad(idx)}`;
-
-        const hasSubprefix = chance.bool();
-        const isDeleteMarker = state.vFormat === 'v0' && chance.bool();
-        const baseKey = hasSubprefix ? masterKeyWithSubprefix : masterKey;
-
-        let canSkip = isDeleteMarker;
-        if (hasSubprefix || state.prefix[state.prefix.length - 1] !== '/') {
-            canSkip = canSkip || state.handleSubprefixKey(baseKey);
-        }
-
-        yield { key: baseKey, isDeleteMarker, canSkip };
-        idx++;
-
-        const versioned = chance.integer({ min: 0, max: 200 });
-        const allowedVersionedKeys = Math.min(count - idx, versioned);
-        yield* generateVersionedKeys(baseKey, allowedVersionedKeys, state);
-        idx += allowedVersionedKeys;
-    }
-}
-
 ['v0', 'v1'].forEach(vFormat => {
     describe(`Delimiter All masters listing algorithm vFormat=${vFormat}`, () => {
         it('should return SKIP_NONE for DelimiterMaster when both NextMarker ' +
@@ -164,71 +59,7 @@ function *generateKeys(count, state) {
             assert.strictEqual(delimiter.skipping(), SKIP_NONE);
         });
 
-        it('should list all subprefixes when handleStreak is applied as in RepdServer', () => {
-            ['_Common-Prefix', '_Common-Prefix/'].forEach(prefix => {
-                const state = createStreakState(prefix, vFormat);
-                state.handleSubprefixKey = baseKey => {
-                    const isLastDelim = state.prefix[state.prefix.length - 1] === '/';
-                    const lastIdx = isLastDelim ? baseKey.lastIndexOf('/') : state.prefix.length;
-                    const prefix = isLastDelim ? getCommonPrefix(baseKey, '/', lastIdx) : baseKey.slice(0, lastIdx);
-                    if (state.prefixes.has(prefix) || (!isLastDelim && baseKey.length > lastIdx)) {
-                        return true;
-                    }
-
-                    state.prefixes.add(prefix);
-                    return false;
-                };
-
-                const maxKeys = 1000000;
-                const delimiter = new DelimiterMaster({
-                    maxKeys,
-                    prefix,
-                    delimiter: '/',
-                    startAfter: '',
-                    continuationToken: '',
-                    v2: true,
-                    fetchOwner: false }, fakeLogger, vFormat);
-
-                [...generateKeys(maxKeys, state)].forEach(ob => {
-                    const { key, isDeleteMarker, canSkip } = ob;
-                    // simulate not doing a raw listing when key < params.gte
-                    // this represents a key not reaching the read stream from leveldb
-                    if (state.params.gte && key < state.params.gte) {
-                        return;
-                    }
-
-                    if (!key.includes(VID_SEP)) { // master key
-                        const version = new Version({ isDeleteMarker });
-                        const obj = {
-                            key: getListingKey(key, vFormat),
-                            value: version.toString(),
-                        };
-
-                        const res = delimiter.filter(obj);
-                        const skippingRange = delimiter.skipping();
-                        handleStreak(res, skippingRange, state);
-
-                        const expected = canSkip ? FILTER_SKIP : FILTER_ACCEPT;
-                        assert.strictEqual(res, expected);
-                    } else { // versioned key
-                        if (vFormat === 'v0') {
-                            const vid = key.split(VID_SEP).slice(-1)[0];
-                            const version = new Version({ versionId: vid, isDeleteMarker });
-                            const obj2 = {
-                                key: getListingKey(key, vFormat),
-                                value: version.toString(),
-                            };
-                            const res = delimiter.filter(obj2);
-                            const skippingRange = delimiter.skipping();
-                            handleStreak(res, skippingRange, state);
-                            assert.strictEqual(res, FILTER_SKIP);
-                        }
-                    }
-                });
-            });
-        });
-
-        it('should return <key><VersionIdSeparator> for DelimiterMaster when ' +
+        it('should return good skipping value for DelimiterMaster when ' +
         'NextMarker is set and there is a delimiter', () => {
             const key = 'key';
             const delimiter = new DelimiterMaster({ delimiter: '/', marker: key },
@@ -239,13 +70,16 @@ function *generateKeys(count, state) {
             delimiter.filter({ key: listingKey, value: '' });
             assert.strictEqual(delimiter.NextMarker, key);
 
-            /* With a delimiter skipping should return previous key + VID_SEP
-             * (except when a delimiter is set and the NextMarker ends with the
-             * delimiter) . */
-            assert.strictEqual(delimiter.skipping(), listingKey + VID_SEP);
+            if (vFormat === 'v0') {
+                // With a delimiter skipping should return previous key + VID_SEP in v0
+                assert.strictEqual(delimiter.skipping(), listingKey + VID_SEP);
+            } else {
+                // in v1 there are no versions to skip
+                assert.strictEqual(delimiter.skipping(), SKIP_NONE);
+            }
         });
 
-        it('should return <key><VersionIdSeparator> for DelimiterMaster when ' +
+        it('should return good skipping value for DelimiterMaster when ' +
         'NextContinuationToken is set and there is a delimiter', () => {
             const key = 'key';
             const delimiter = new DelimiterMaster(
@@ -257,11 +91,17 @@ function *generateKeys(count, state) {
             delimiter.filter({ key: listingKey, value: '' });
             assert.strictEqual(delimiter.NextContinuationToken, key);
 
-            assert.strictEqual(delimiter.skipping(), listingKey + VID_SEP);
+            if (vFormat === 'v0') {
+                // With a delimiter skipping should return previous key + VID_SEP in v0
+                assert.strictEqual(delimiter.skipping(), listingKey + VID_SEP);
+            } else {
+                // in v1 there are no versions to skip
+                assert.strictEqual(delimiter.skipping(), SKIP_NONE);
+            }
         });
 
-        it('should return NextMarker for DelimiterMaster when NextMarker is set' +
-        ', there is a delimiter and the key ends with the delimiter', () => {
+        it('should return SKIP_NONE for DelimiterMaster when NextMarker is set' +
+        ', there is a delimiter and the filtered key ends with the delimiter', () => {
             const delimiterChar = '/';
             const keyWithEndingDelimiter = `key${delimiterChar}`;
             const delimiter = new DelimiterMaster({
@@ -269,13 +109,10 @@ function *generateKeys(count, state) {
                 marker: keyWithEndingDelimiter,
             }, fakeLogger, vFormat);
 
-            /* When a delimiter is set and the NextMarker ends with the
-             * delimiter it should return the next marker value. */
+            /* When a NextMarker is set to the key, we should get
+             * SKIP_NONE because no key has been listed yet. */
             assert.strictEqual(delimiter.NextMarker, keyWithEndingDelimiter);
-            const skipKey = vFormat === 'v1' ?
-                `${DbPrefixes.Master}${keyWithEndingDelimiter}` :
-                keyWithEndingDelimiter;
-            assert.strictEqual(delimiter.skipping(), skipKey);
+            assert.strictEqual(delimiter.skipping(), SKIP_NONE);
         });
 
         it('should skip entries not starting with prefix', () => {
@@ -609,22 +446,6 @@ function *generateKeys(count, state) {
                 });
             });
 
-            it('should skip a versioned entry when there is a delimiter and the key ' +
-            'starts with the NextMarker value', () => {
-                const delimiterChar = '/';
-                const commonPrefix = `commonPrefix${delimiterChar}`;
-                const key = `${commonPrefix}key${VID_SEP}version`;
-                const value = 'value';
-
-                const delimiter = new DelimiterMaster({ delimiter: delimiterChar },
-                    fakeLogger, vFormat);
-                /* TODO: should be set to a whole key instead of just a common prefix
-                 * once ZENKO-1048 is fixed. */
-                delimiter.NextMarker = commonPrefix;
-
-                assert.strictEqual(delimiter.filter({ key, value }), FILTER_SKIP);
-            });
-
             it('should return good skipping value for DelimiterMaster on replay keys', () => {
                 const delimiter = new DelimiterMaster(
                     { delimiter: '/', v2: true },
@@ -657,6 +478,81 @@ function *generateKeys(count, state) {
                 // ...it should return to skipping by prefix as usual
                 assert.strictEqual(delimiter.skipping(), `${inc(DbPrefixes.Replay)}foo/`);
             });
+
+            it('should not skip over whole prefix when a key equals the prefix', () => {
+                const FILTER_MAP = {
+                    '0': 'FILTER_SKIP',
+                    '1': 'FILTER_ACCEPT',
+                    '-1': 'FILTER_END',
+                };
+                const delimiter = new DelimiterMaster({
+                    prefix: 'prefix/',
+                    delimiter: '/',
+                }, fakeLogger, vFormat);
+                for (const testEntry of [
+                    {
+                        key: 'prefix/',
+                        expectedRes: FILTER_ACCEPT,
+                        expectedSkipping: `prefix/${VID_SEP}`,
+                    },
+                    {
+                        key: `prefix/${VID_SEP}v1`,
+                        value: '{}',
+                        expectedRes: FILTER_SKIP, // versions get skipped after master
+                        expectedSkipping: `prefix/${VID_SEP}`,
+                    },
+                    {
+                        key: 'prefix/deleted',
+                        isDeleteMarker: true,
+                        expectedRes: FILTER_SKIP, // delete markers get skipped
+                        expectedSkipping: `prefix/deleted${VID_SEP}`,
+                    },
+                    {
+                        key: `prefix/deleted${VID_SEP}v1`,
+                        isDeleteMarker: true,
+                        expectedRes: FILTER_SKIP,
+                        expectedSkipping: `prefix/deleted${VID_SEP}`,
+                    },
+                    {
+                        key: `prefix/deleted${VID_SEP}v2`,
+                        expectedRes: FILTER_SKIP,
+                        expectedSkipping: `prefix/deleted${VID_SEP}`,
+                    },
+                    {
+                        key: 'prefix/subprefix1/key-1',
+                        expectedRes: FILTER_ACCEPT,
+                        expectedSkipping: 'prefix/subprefix1/',
+                    },
+                    {
+                        key: `prefix/subprefix1/key-1${VID_SEP}v1`,
+                        expectedRes: FILTER_SKIP,
+                        expectedSkipping: 'prefix/subprefix1/',
+                    },
+                    {
+                        key: 'prefix/subprefix1/key-2',
+                        expectedRes: FILTER_SKIP,
+                        expectedSkipping: 'prefix/subprefix1/',
+                    },
+                    {
+                        key: `prefix/subprefix1/key-2${VID_SEP}v1`,
+                        expectedRes: FILTER_SKIP,
+                        expectedSkipping: 'prefix/subprefix1/',
+                    },
+                ]) {
+                    const entry = {
+                        key: testEntry.key,
+                    };
+                    if (testEntry.isDeleteMarker) {
+                        entry.value = '{"isDeleteMarker":true}';
+                    } else {
+                        entry.value = '{}';
+                    }
+                    const res = delimiter.filter(entry);
+                    const skipping = delimiter.skipping();
+                    assert.strictEqual(res, testEntry.expectedRes);
+                    assert.strictEqual(skipping, testEntry.expectedSkipping);
+                }
+            });
         }
     });
 });

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

@@ -406,7 +406,7 @@ const tests = [
         }],
     }, {
         Versions: [],
-        CommonPrefixes: ['notes/summer/'],
+        CommonPrefixes: ['notes/spring/'],
         Delimiter: '/',
         IsTruncated: true,
         NextKeyMarker: 'notes/summer/',

tests/unit/network/http/server.spec.js

@@ -166,7 +166,7 @@ describe('network.Server: ', () => {
                 if (err) {
                     return ws.onStop(() => {
                         clearTimeout(requestTimeout);
-                        if (err.code === 'EPROTO' || err.code === 'ECONNRESET') {
+                        if (err.code === 'EPROTO') {
                             return done();
                         }
                         return done(err);

yarn.lock

@@ -2144,11 +2144,6 @@ chalk@^4.0.0:
     ansi-styles "^4.1.0"
     supports-color "^7.1.0"
 
-chance@^1.1.8:
-  version "1.1.8"
-  resolved "https://registry.yarnpkg.com/chance/-/chance-1.1.8.tgz#5d6c2b78c9170bf6eb9df7acdda04363085be909"
-  integrity sha512-v7fi5Hj2VbR6dJEGRWLmJBA83LJMS47pkAbmROFxHWd9qmE1esHRZW8Clf1Fhzr3rjxnNZVCjOEv/ivFxeIMtg==
-
 char-regex@^1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/char-regex/-/char-regex-1.0.2.tgz#d744358226217f981ed58f479b1d6bcc29545dcf"