diff --git a/lib/versioning/Version.ts b/lib/versioning/Version.ts index ff2b1113d..3c3976444 100644 --- a/lib/versioning/Version.ts +++ b/lib/versioning/Version.ts @@ -3,7 +3,7 @@ import { VersioningConstants } from './constants'; const VID_SEP = VersioningConstants.VersionId.Separator; /** * Class for manipulating an object version. - * The format of a version: { isNull, isDeleteMarker, versionId, otherInfo } + * The format of a version: { isNull, isNull2, isDeleteMarker, versionId, otherInfo } * * @note Some of these functions are optimized based on string search * prior to a full JSON parse/stringify. (Vinh: 18K op/s are achieved @@ -13,24 +13,31 @@ const VID_SEP = VersioningConstants.VersionId.Separator; export class Version { version: { isNull?: boolean; + isNull2?: boolean; isDeleteMarker?: boolean; versionId?: string; isPHD?: boolean; + nullVersionId?: string; }; /** * Create a new version instantiation from its data object. * @param version - the data object to instantiate * @param version.isNull - is a null version + * @param version.isNull2 - Whether new version is null or not AND has + * been put with a Cloudserver handling null keys (i.e. supporting + * S3C-7352) * @param version.isDeleteMarker - is a delete marker * @param version.versionId - the version id * @constructor */ constructor(version?: { isNull?: boolean; + isNull2?: boolean; isDeleteMarker?: boolean; versionId?: string; isPHD?: boolean; + nullVersionId?: string; }) { this.version = version || {}; } @@ -121,6 +128,19 @@ export class Version { return this.version.isNull ?? false; } + /** + * Check if a version is a null version and has + * been put with a Cloudserver handling null keys (i.e. supporting + * S3C-7352). + * + * @return - stating if the value is a null version and has + * been put with a Cloudserver handling null keys (i.e. supporting + * S3C-7352). + */ + isNull2Version(): boolean { + return this.version.isNull2 ?? false; + } + /** * Check if a stringified object is a delete marker. * @@ -170,6 +190,15 @@ 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. * @@ -190,6 +219,19 @@ export class Version { return this; } + /** + * Mark that the null version has been put with a Cloudserver handling null keys (i.e. supporting S3C-7352) + * + * If `isNull2` is set, `isNull` is also set to maintain consistency. + * Explicitly setting both avoids misunderstandings and mistakes in future updates or fixes. + * @return - the updated version + */ + setNull2Version() { + this.version.isNull2 = true; + this.version.isNull = true; + return this; + } + /** * Serialize the version. * diff --git a/lib/versioning/VersioningRequestProcessor.ts b/lib/versioning/VersioningRequestProcessor.ts index 914da35f2..c1d79e014 100644 --- a/lib/versioning/VersioningRequestProcessor.ts +++ b/lib/versioning/VersioningRequestProcessor.ts @@ -496,7 +496,26 @@ export default class VersioningRequestProcessor { const versionIdFromMaster = masterVersion.getVersionId(); if (versionIdFromMaster === undefined || versionIdFromMaster >= versionId) { + let value = request.value; logger.debug('version to put is not older than master'); + // Delete the deprecated, null key for backward compatibility + // to avoid storing both deprecated and new null keys. + // If master null version was put with an older Cloudserver (or in compat mode), + // there is a possibility that it also has a null versioned key + // associated, so we need to delete it as we write the null key. + // Deprecated null key gets deleted when the new CloudServer: + // - updates metadata of a null master (options.isNull=true) + // - puts metadata on top of a master null key (options.isNull=false) + 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. + if (masterNullVersionId) { + const oldNullVersionKey = formatVersionKey(key, masterNullVersionId); + ops.push({ key: oldNullVersionKey, type: 'del' }); + } + } // new behavior when isNull is defined is to only // update the master key if it is the latest // version, old behavior needs to copy master to @@ -509,7 +528,7 @@ export default class VersioningRequestProcessor { request.options.isNull === undefined) { // master key is strictly older than the put version let masterVersionId; - if (masterVersion.isNullVersion()) { + if (masterVersion.isNullVersion() && versionIdFromMaster) { logger.debug('master key is a null version'); masterVersionId = versionIdFromMaster; } else if (versionIdFromMaster === undefined) { @@ -537,13 +556,22 @@ export default class VersioningRequestProcessor { '' : masterVersionId; const masterVersionKey = formatVersionKey(key, masterKeyVersionId); masterVersion.setNullVersion(); + // isNull === false means Cloudserver supports null keys, + // so create a null key with the isNull2 flag + if (request.options.isNull === false) { + masterVersion.setNull2Version(); + // else isNull === undefined means Cloudserver does not support null keys, + // hence set/update the new master nullVersionId for backward compatibility + } else { + value = Version.updateOrAppendNullVersionId(request.value, masterVersionId); + } ops.push({ key: masterVersionKey, value: masterVersion.toString() }); } } else { logger.debug('version to put is the master'); } - ops.push({ key, value: request.value }); + ops.push({ key, value: value }); } else { logger.debug('version to put is older than master'); if (request.options.isNull === true && !masterVersion.isNullVersion()) { diff --git a/tests/unit/versioning/VersioningRequestProcessor.spec.js b/tests/unit/versioning/VersioningRequestProcessor.spec.js index a801b4ebb..6b720b28c 100644 --- a/tests/unit/versioning/VersioningRequestProcessor.spec.js +++ b/tests/unit/versioning/VersioningRequestProcessor.spec.js @@ -254,6 +254,443 @@ describe('test VRP', () => { }], done); }); + + it('should be able to put Metadata on top of a standalone null version', done => { + const versionId = '00000000000000999999PARIS '; + + async.waterfall([next => { + // simulate the creation of a standalone null version. + const request = { + db: 'foo', + key: 'bar', + value: '{"qux":"quz"}', + options: {}, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + // simulate a BackbeatClient.putMetadata + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + options: { + versioning: true, + versionId, + // isNull === false means Cloudserver supports the new "null key" logic. + isNull: false, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + wgm.list({}, logger, next); + }, + (res, next) => { + const expectedListing = [ + // master version should have the provided version id + { + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + }, + // The null version will get the highest version number. + // It should have "isNull" and "isNul2" set to true, + // showing it's a null version made by Cloudserver that works with null keys. + { + key: `bar${VID_SEP}`, + value: '{"qux":"quz","versionId":"99999999999999999999PARIS ","isNull":true,"isNull2":true}', + }, + // the new version + { + key: `bar${VID_SEP}${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, + }; + assert.deepStrictEqual(JSON.parse(res), expectedGet); + next(); + }], + done); + }); + + it('should be able to put Metadata on top of a standalone null version in backward compatibility mode', done => { + const versionId = '00000000000000999999PARIS '; + + async.waterfall([next => { + // simulate the creation of a standalone null version. + const request = { + db: 'foo', + key: 'bar', + value: '{"qux":"quz"}', + options: {}, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + // simulate a BackbeatClient.putMetadata + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + options: { + versioning: true, + versionId, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + wgm.list({}, logger, next); + }, + (res, next) => { + const expectedListing = [ + // master version should have the provided version id and a reference of the null version id. + { + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}","nullVersionId":"99999999999999999999PARIS "}`, + }, + // the "internal" master version should have the provided version id. + { + key: `bar${VID_SEP}${versionId}`, + value: `{"qux":"quz2","versionId":"${versionId}"}`, + }, + // should create a version that represents the old null master with the infinite version id and + // the isNull property set to true. + { + key: `bar${VID_SEP}99999999999999999999PARIS `, + value: '{"qux":"quz","versionId":"99999999999999999999PARIS ","isNull":true}', + }, + ]; + assert.deepStrictEqual(res, expectedListing); + const request = { + db: 'foo', + key: 'bar', + }; + vrp.get(request, logger, next); + }, + (res, next) => { + const expectedGet = { + qux: 'quz2', + versionId, + nullVersionId: '99999999999999999999PARIS ', + }; + assert.deepStrictEqual(JSON.parse(res), expectedGet); + next(); + }], + done); + }); + + it('should be able to put Metadata on top of a null suspended version', 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 + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + options: { + versioning: true, + versionId, + // isNull === false means Cloudserver supports the new "null key" logic. + isNull: false, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + wgm.list({}, logger, next); + }, + (res, next) => { + const expectedListing = [ + // master version should have the provided version id + { + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + }, + // The null version will get the highest version number. + // It should have "isNull" and "isNul2" set to true, + // showing it's a null version made by Cloudserver that works with null keys. + { + key: `bar${VID_SEP}`, + value: `{"qux":"quz","isNull":true,"versionId":"${nullVersionId}","isNull2":true}`, + }, + // the new version + { + key: `bar${VID_SEP}${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, + }; + assert.deepStrictEqual(JSON.parse(res), expectedGet); + next(); + }], + done); + }); + + it('should be able to put Metadata on top of a null suspended version in backward compatibility mode', 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 + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}"}`, + options: { + versioning: true, + versionId, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + wgm.list({}, logger, next); + }, + (res, next) => { + const expectedListing = [ + // master version should have the provided version id and a reference of the null version id. + { + key: 'bar', + value: `{"qux":"quz2","versionId":"${versionId}","nullVersionId":"${nullVersionId}"}`, + }, + // the "internal" master version should have the provided version id. + { + key: `bar${VID_SEP}${versionId}`, + value: `{"qux":"quz2","versionId":"${versionId}"}`, + }, + // should create a version that represents the old null master with the infinite version id and + // the isNull property set to true. + { + key: `bar${VID_SEP}${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', + versionId, + nullVersionId, + }; + 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; + + 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; + // update metadata of the same null version with compat mode (options.isNull not defined) + // to generate a deprecated null key. + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","isNull":true,"versionId":"${nullVersionId}"}`, + options: { + versionId: nullVersionId, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + // put metadata with the new keys implementation (options.isNull defined) + // on top of the null master with a deprecated null key. + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz3","versionId":"${versionId}"}`, + options: { + versionId, + isNull: false, + }, + }; + vrp.put(request, logger, next); + }, + (res, next) => { + wgm.list({}, logger, next); + }, + (res, next) => { + const expectedListing = [ + // master version should have the provided version id. + { + key: 'bar', + value: `{"qux":"quz3","versionId":"${versionId}"}`, + }, + // the null key + { + key: `bar${VID_SEP}`, + value: `{"qux":"quz2","isNull":true,"versionId":"${nullVersionId}",` + + `"nullVersionId":"${nullVersionId}","isNull2":true}`, + }, + // version key + { + key: `bar${VID_SEP}${versionId}`, + value: `{"qux":"quz3","versionId":"${versionId}"}`, + }, + ]; + assert.deepStrictEqual(res, expectedListing); + const request = { + db: 'foo', + key: 'bar', + }; + vrp.get(request, logger, next); + }, + (res, next) => { + const expectedGet = { + qux: 'quz3', + versionId, + }; + assert.deepStrictEqual(JSON.parse(res), expectedGet); + next(); + }], + done); + }); + + it('should delete the deprecated null key after updating metadata of an old null master', 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; + // update metadata of the same null version with compat mode (options.isNull not defined) + // to generate a deprecated null key. + const request = { + db: 'foo', + key: 'bar', + value: `{"qux":"quz2","isNull":true,"versionId":"${nullVersionId}"}`, + options: { + versionId: nullVersionId, + }, + }; + 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 = [ + // the internal null version should be deleted. + { + key: 'bar', + value: `{"qux":"quz3","isNull2":true,"isNull":true,"versionId":"${nullVersionId}"}`, + }, + ]; + assert.deepStrictEqual(res, expectedListing); + const request = { + db: 'foo', + key: 'bar', + }; + vrp.get(request, logger, next); + }, + (res, next) => { + const expectedGet = { + qux: 'quz3', + isNull2: true, + isNull: true, + versionId: nullVersionId, + }; + assert.deepStrictEqual(JSON.parse(res), expectedGet); + next(); + }], + done); + }); });