ARSN-403 add tests

lib/versioning/Version.ts

@@ -235,15 +235,6 @@ export class Version {
         return this;
     }
 
-    /**
-     * Get the nullVersionId of the version.
-     *
-     * @return - the nullVersionId
-     */
-    getNullVersionId(): string | undefined {
-        return this.version.nullVersionId;
-    }
-
     /**
      * Mark a version as a delete marker.
      *

lib/versioning/VersioningRequestProcessor.ts

@@ -509,8 +509,8 @@ export default class VersioningRequestProcessor {
                     if (request.options.isNull !== undefined && // new null key behavior when isNull is defined.
                         masterVersion.isNullVersion() && // master is null
                         !masterVersion.isNull2Version()) { // master does not support the new null key behavior yet.
-                        const masterNullVersionId = masterVersion.getNullVersionId();
-                        // The deprecated null key is referenced in the "nullVersionId" property of the master key.
+                        const masterNullVersionId = masterVersion.getVersionId();
+                        // The deprecated null key is referenced in the "versionId" property of the master key.
                         if (masterNullVersionId) {
                             const oldNullVersionKey = formatVersionKey(key, masterNullVersionId);
                             ops.push({ key: oldNullVersionKey, type: 'del' });
@@ -561,8 +561,10 @@ export default class VersioningRequestProcessor {
                             if (request.options.isNull === false) {
                                 masterVersion.setNull2Version();
                             // else isNull === undefined means Cloudserver does not support null keys,
+                            // and versionIdFromMaster !== versionId means that a version is PUT on top of a null version
                             // hence set/update the new master nullVersionId for backward compatibility
-                            } else {
+                            } else if (versionIdFromMaster !== versionId) {
+                                // => set the nullVersionId to the master version if put version on top of null version.
                                 value = Version.updateOrAppendNullVersionId(request.value, masterVersionId);
                             }
                             ops.push({ key: masterVersionKey,

tests/unit/versioning/VersioningRequestProcessor.spec.js

@@ -535,6 +535,69 @@ describe('test VRP', () => {
         done);
     });
 
+    it('should be able to update a null suspended version in backward compatibility mode', done => {
+        let nullVersionId;
+
+        async.waterfall([next => {
+            // simulate the creation of a null suspended version.
+            const request = {
+                db: 'foo',
+                key: 'bar',
+                value: '{"qux":"quz","isNull":true}',
+                options: {
+                    versionId: '',
+                },
+            };
+            vrp.put(request, logger, next);
+        },
+        (res, next) => {
+            nullVersionId = JSON.parse(res).versionId;
+            // simulate update null version with BackbeatClient.putMetadata
+            const request = {
+                db: 'foo',
+                key: 'bar',
+                value: '{"qux":"quz2","isNull":true}',
+                options: {
+                    versioning: true,
+                    versionId: nullVersionId,
+                },
+            };
+            vrp.put(request, logger, next);
+        },
+        (res, next) => {
+            wgm.list({}, logger, next);
+        },
+        (res, next) => {
+            const expectedListing = [
+                // NOTE: should not set nullVersionId to the master version if updating a null version.
+                {
+                    key: 'bar',
+                    value: '{"qux":"quz2","isNull":true}',
+                },
+                {
+                    key: `bar\x00${nullVersionId}`,
+                    value: `{"qux":"quz","isNull":true,"versionId":"${nullVersionId}"}`,
+                },
+            ];
+            assert.deepStrictEqual(res, expectedListing);
+
+            const request = {
+                db: 'foo',
+                key: 'bar',
+            };
+            vrp.get(request, logger, next);
+        },
+        (res, next) => {
+            const expectedGet = {
+                qux: 'quz2',
+                isNull: true,
+            };
+            assert.deepStrictEqual(JSON.parse(res), expectedGet);
+            next();
+        }],
+        done);
+    });
+
     it('should delete the deprecated null key after put Metadata on top of an old null master', done => {
         const versionId = '00000000000000999999PARIS  ';
         let nullVersionId;
@@ -592,8 +655,7 @@ describe('test VRP', () => {
                 // the null key
                 {
                     key: `bar${VID_SEP}`,
-                    value: `{"qux":"quz2","isNull":true,"versionId":"${nullVersionId}",` +
-                    `"nullVersionId":"${nullVersionId}","isNull2":true}`,
+                    value: `{"qux":"quz2","isNull":true,"versionId":"${nullVersionId}","isNull2":true}`,
                 },
                 // version key
                 {
@@ -691,6 +753,88 @@ describe('test VRP', () => {
         }],
         done);
     });
+
+    it('should delete the deprecated null key after updating a non-latest null key', done => {
+        const versionId = '00000000000000999999PARIS  ';
+        let nullVersionId;
+
+        async.waterfall([next => {
+            // simulate the creation of a null suspended version.
+            const request = {
+                db: 'foo',
+                key: 'bar',
+                value: '{"qux":"quz","isNull":true}',
+                options: {
+                    versionId: '',
+                },
+            };
+            vrp.put(request, logger, next);
+        },
+        (res, next) => {
+            nullVersionId = JSON.parse(res).versionId;
+            // simulate a BackbeatClient.putMetadata
+            // null key is not the latest = master is not null.
+            const request = {
+                db: 'foo',
+                key: 'bar',
+                value: `{"qux":"quz2","versionId":"${versionId}"}`,
+                options: {
+                    versioning: true,
+                    versionId,
+                },
+            };
+            vrp.put(request, logger, next);
+        },
+        (res, next) => {
+            // update the null version metadata with the new keys implementation (options.isNull defined)
+            const request = {
+                db: 'foo',
+                key: 'bar',
+                value: `{"qux":"quz3","isNull2":true,"isNull":true,"versionId":"${nullVersionId}"}`,
+                options: {
+                    versionId: nullVersionId,
+                    isNull: true,
+                },
+            };
+            vrp.put(request, logger, next);
+        },
+        (res, next) => {
+            wgm.list({}, logger, next);
+        },
+        (res, next) => {
+            const expectedListing = [
+                {
+                    key: 'bar',
+                    value: `{"qux":"quz2","versionId":"${versionId}","nullVersionId":"${nullVersionId}"}`,
+                },
+                {
+                    key: 'bar\x00',
+                    value: `{"qux":"quz3","isNull2":true,"isNull":true,"versionId":"${nullVersionId}"}`,
+                },
+                {
+                    key: `bar\x00${versionId}`,
+                    value: `{"qux":"quz2","versionId":"${versionId}"}`,
+                },
+            ];
+            assert.deepStrictEqual(res, expectedListing);
+
+            const request = {
+                db: 'foo',
+                key: 'bar',
+            };
+            vrp.get(request, logger, next);
+        },
+        (res, next) => {
+            const expectedGet = {
+                qux: 'quz2',
+                versionId,
+                nullVersionId,
+            };
+            assert.deepStrictEqual(JSON.parse(res), expectedGet);
+            next();
+        }],
+        done);
+    });
 });