Skip to content

Commit

Permalink
ARSN-392 Fix processVersionSpecificPut
Browse files Browse the repository at this point in the history
- For backward compatibility (if isNull is undefined), add the nullVersionId field to the master update. The nullVersionId is needed for listing, retrieving, and deleting null versions.

- For the new null key implementation (if isNull is defined): add the isNull2 field and set it to true to specify that the new version is null AND has been put with a Cloudserver handling null keys (i.e., supporting S3C-7352).

- Manage scenarios in which a version is marked with the isNull attribute set to true, but without a version ID. This happens after BackbeatClient.putMetadata() is applied to a standalone null master.
  • Loading branch information
nicolas2bert committed Feb 12, 2024
1 parent 1653fd2 commit 5178536
Show file tree
Hide file tree
Showing 4 changed files with 414 additions and 11 deletions.
76 changes: 67 additions & 9 deletions lib/versioning/Version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -13,6 +13,7 @@ const VID_SEP = VersioningConstants.VersionId.Separator;
export class Version {
version: {
isNull?: boolean;
isNull2?: boolean;
isDeleteMarker?: boolean;
versionId?: string;
isPHD?: boolean;
Expand All @@ -22,12 +23,16 @@ export class Version {
* 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;
Expand Down Expand Up @@ -83,6 +88,31 @@ export class Version {
return `{ "isPHD": true, "versionId": "${versionId}" }`;
}

/**
* Appends a key-value pair to a JSON object represented as a string. It adds
* a comma if the object is not empty (i.e., not just '{}'). It assumes the input
* string is formatted as a JSON object.
*
* @param {string} stringifiedObject The JSON object as a string to which the key-value pair will be appended.
* @param {string} key The key to append to the JSON object.
* @param {string} value The value associated with the key to append to the JSON object.
* @returns {string} The updated JSON object as a string with the new key-value pair appended.
* @example
* _jsonAppend('{"existingKey":"existingValue"}', 'newKey', 'newValue');
* // returns '{"existingKey":"existingValue","newKey":"newValue"}'
*/
static _jsonAppend(stringifiedObject: string, key: string, value: string): string {
// stringifiedObject value has the format of '{...}'
let index = stringifiedObject.length - 2;
while (stringifiedObject.charAt(index--) === ' ');
const comma = stringifiedObject.charAt(index + 1) !== '{';
return (
`${stringifiedObject.slice(0, stringifiedObject.length - 1)}` + // eslint-disable-line
(comma ? ',' : '') +
`"${key}":"${value}"}`
);
}

/**
* Put versionId into an object in the (cheap) way of string manipulation,
* instead of the more expensive alternative parsing and stringification.
Expand All @@ -93,14 +123,32 @@ export class Version {
*/
static appendVersionId(value: string, versionId: string): string {
// assuming value has the format of '{...}'
let index = value.length - 2;
while (value.charAt(index--) === ' ');
const comma = value.charAt(index + 1) !== '{';
return (
`${value.slice(0, value.length - 1)}` + // eslint-disable-line
(comma ? ',' : '') +
`"versionId":"${versionId}"}`
);
return Version._jsonAppend(value, 'versionId', versionId);
}

/**
* Updates or appends a `nullVersionId` property to a JSON-formatted string.
* This function first checks if the `nullVersionId` property already exists within the input string.
* If it exists, the function updates the `nullVersionId` with the new value provided.
* If it does not exist, the function appends a `nullVersionId` property with the provided value.
*
* @static
* @param {string} value - The JSON-formatted string that may already contain a `nullVersionId` property.
* @param {string} nullVersionId - The new value for the `nullVersionId` property to be updated or appended.
* @returns {string} The updated JSON-formatted string with the new `nullVersionId` value.
*/
static updateOrAppendNullVersionId(value: string, nullVersionId: string): string {
// Check if "nullVersionId" already exists in the string
const nullVersionIdPattern = /"nullVersionId":"[^"]*"/;
const nullVersionIdExists = nullVersionIdPattern.test(value);

if (nullVersionIdExists) {
// Replace the existing nullVersionId with the new one
return value.replace(nullVersionIdPattern, `"nullVersionId":"${nullVersionId}"`);
} else {
// Append nullVersionId
return Version._jsonAppend(value, 'nullVersionId', nullVersionId);
}
}

/**
Expand Down Expand Up @@ -190,6 +238,16 @@ export class Version {
return this;
}

/**
* Mark that the null version has been put with a Cloudserver handling null keys (i.e. supporting S3C-7352)
*
* @return - the updated version
*/
setNull2Version() {
this.version.isNull2 = true;
return this;
}

/**
* Serialize the version.
*
Expand Down
14 changes: 12 additions & 2 deletions lib/versioning/VersioningRequestProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ 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');
// new behavior when isNull is defined is to only
// update the master key if it is the latest
Expand All @@ -509,7 +510,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) {
Expand Down Expand Up @@ -537,13 +538,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()) {
Expand Down
55 changes: 55 additions & 0 deletions tests/unit/versioning/Version.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
const { Version } = require('../../../lib/versioning/Version');

describe('Version', () => {
describe('appendVersionId', () => {
it('should append versionId to an empty object', () => {
const emptyObject = '{}';
const versionId = '123';
const expected = '{"versionId":"123"}';
const result = Version.appendVersionId(emptyObject, versionId);
expect(result).toEqual(expected);
});

it('should append versionId to an object with existing properties', () => {
const existingObject = '{"key":"value"}';
const versionId = '456';
const expected = '{"key":"value","versionId":"456"}';
const result = Version.appendVersionId(existingObject, versionId);
expect(result).toEqual(expected);
});

it('should append versionId to an object with existing versionId', () => {
const objectWithVersionId = '{"key":"value","versionId":"old"}';
const versionId = 'new';
const expected = '{"key":"value","versionId":"old","versionId":"new"}';
const result = Version.appendVersionId(objectWithVersionId, versionId);
expect(result).toEqual(expected);
});
});

describe('updateOrAppendNullVersionId', () => {
it('should append nullVersionId when it does not exist', () => {
const initialValue = '{"key":"value"}';
const nullVersionId = '12345';
const expectedValue = '{"key":"value","nullVersionId":"12345"}';
const result = Version.updateOrAppendNullVersionId(initialValue, nullVersionId);
expect(result).toEqual(expectedValue);
});

it('should update nullVersionId when it exists', () => {
const initialValue = '{"key":"value","nullVersionId":"initial"}';
const nullVersionId = 'updated12345';
const expectedValue = '{"key":"value","nullVersionId":"updated12345"}';
const result = Version.updateOrAppendNullVersionId(initialValue, nullVersionId);
expect(result).toEqual(expectedValue);
});

it('should handle empty string by appending nullVersionId', () => {
const initialValue = '{}';
const nullVersionId = 'emptyCase12345';
const expectedValue = '{"nullVersionId":"emptyCase12345"}';
const result = Version.updateOrAppendNullVersionId(initialValue, nullVersionId);
expect(result).toEqual(expectedValue);
});
});
});
Loading

0 comments on commit 5178536

Please sign in to comment.