Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARSN-377 correctly handle null keys with common prefix #2192

Merged
merged 4 commits into from
Dec 20, 2023
Merged

ARSN-377 correctly handle null keys with common prefix #2192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
206f14b
ARSN-377 improve versioned listing test
jonathan-gramain Dec 12, 2023
1bc3360
ARSN-377 correctly handle null keys with common prefix
jonathan-gramain Dec 12, 2023
063a2fb
ARSN-377 fix DelimiterNonCurrent and add a unit test
jonathan-gramain Dec 14, 2023
d08a267
ARSN-377 bump arsenal version
jonathan-gramain Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/algos/list/delimiterNonCurrent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ class DelimiterNonCurrent extends DelimiterVersions {
// Overwrite keyHandler_SkippingVersions to include the last version from the previous listing.
// The creation (last-modified) date of this version will be the stale date for the following version.
// eslint-disable-next-line camelcase
keyHandler_SkippingVersions(key, value) {
const { key: nonversionedKey, versionId } = this.parseKey(key);
if (nonversionedKey === this.keyMarker) {
keyHandler_SkippingVersions(key, versionId, value) {
if (key === this.keyMarker) {
// since the nonversioned key equals the marker, there is
// necessarily a versionId in this key
const _versionId = versionId;
Expand All @@ -66,7 +65,7 @@ class DelimiterNonCurrent extends DelimiterVersions {
this.setState({
id: 1 /* NotSkipping */,
});
return this.handleKey(key, value);
return this.handleKey(key, versionId, value);
}

filter(obj) {
Expand Down
171 changes: 65 additions & 106 deletions lib/algos/list/delimiterVersions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ export interface FilterReturnValue {
export const enum DelimiterVersionsFilterStateId {
NotSkipping = 1,
SkippingPrefix = 2,
WaitForNullKey = 3,
SkippingVersions = 4,
SkippingVersions = 3,
};

export interface DelimiterVersionsFilterState_NotSkipping extends FilterState {
Expand All @@ -38,16 +37,12 @@ export interface DelimiterVersionsFilterState_SkippingPrefix extends FilterState
prefix: string;
};

export interface DelimiterVersionsFilterState_WaitForNullKey extends FilterState {
id: DelimiterVersionsFilterStateId.WaitForNullKey,
};

export interface DelimiterVersionsFilterState_SkippingVersions extends FilterState {
id: DelimiterVersionsFilterStateId.SkippingVersions,
gt: string;
};

type KeyHandler = (key: string, value: string) => FilterReturnValue;
type KeyHandler = (key: string, versionId: string | undefined, value: string) => FilterReturnValue;

type ResultObject = {
CommonPrefixes: string[],
Expand Down Expand Up @@ -126,17 +121,14 @@ export class DelimiterVersions extends Delimiter {
DelimiterVersionsFilterStateId.SkippingPrefix,
this.keyHandler_SkippingPrefix.bind(this));

this.setKeyHandler(
DelimiterVersionsFilterStateId.WaitForNullKey,
this.keyHandler_WaitForNullKey.bind(this));

this.setKeyHandler(
DelimiterVersionsFilterStateId.SkippingVersions,
this.keyHandler_SkippingVersions.bind(this));

if (this.versionIdMarker) {
this.state = <DelimiterVersionsFilterState_WaitForNullKey> {
id: DelimiterVersionsFilterStateId.WaitForNullKey,
this.state = <DelimiterVersionsFilterState_SkippingVersions> {
id: DelimiterVersionsFilterStateId.SkippingVersions,
gt: `${this.keyMarker}${VID_SEP}${this.versionIdMarker}`,
};
} else {
this.state = <DelimiterVersionsFilterState_NotSkipping> {
Expand Down Expand Up @@ -236,6 +228,30 @@ export class DelimiterVersions extends Delimiter {
return { key: nonversionedKey, versionId };
}

/**
* Include a key in the listing output, in the Versions or CommonPrefix result
*
* @param {string} key - key (without version ID)
* @param {string} versionId - version ID
* @param {string} value - metadata value
* @return {undefined}
*/
addKey(key: string, versionId: string, value: string) {
// add the subprefix to the common prefixes if the key has the delimiter
const commonPrefix = this.getCommonPrefix(key);
if (commonPrefix) {
this.addCommonPrefix(commonPrefix);
// transition into SkippingPrefix state to skip all following keys
// while they start with the same prefix
this.setState(<DelimiterVersionsFilterState_SkippingPrefix> {
id: DelimiterVersionsFilterStateId.SkippingPrefix,
prefix: commonPrefix,
});
} else {
this.addContents(key, versionId, value);
}
}

/**
* Add a (key, versionId, value) tuple to the listing.
* Set the NextMarker to the current key
Expand Down Expand Up @@ -293,21 +309,6 @@ export class DelimiterVersions extends Delimiter {
this.nullKey = { key, versionId, value };
}

/**
* Add the cached null key to the results. This is called when
* reaching the correct position for the null key in the output.
*
* @return {undefined}
*/
addCurrentNullKey(): void {
this.addContents(
this.nullKey.key,
this.nullKey.versionId,
this.nullKey.value,
);
this.nullKey = null;
}

getObjectKeyV0(obj: { key: string }): string {
return obj.key;
}
Expand All @@ -331,7 +332,24 @@ export class DelimiterVersions extends Delimiter {
const key = this.getObjectKey(obj);
const value = obj.value;

return this.handleKey(key, value);
const { key: nonversionedKey, versionId: keyVersionId } = this.parseKey(key);
if (this.nullKey) {
if (this.nullKey.key !== nonversionedKey
|| this.nullKey.versionId < <string> keyVersionId) {
this.handleKey(
this.nullKey.key, this.nullKey.versionId, this.nullKey.value);
this.nullKey = null;
}
}
if (keyVersionId === '') {
// null key
this.cacheNullKey(nonversionedKey, Version.from(value).getVersionId(), value);
if (this.state.id === DelimiterVersionsFilterStateId.SkippingVersions) {
return FILTER_SKIP;
}
return FILTER_ACCEPT;
}
return this.handleKey(nonversionedKey, keyVersionId, value);
}

setState(state: FilterState): void {
Expand All @@ -342,11 +360,11 @@ export class DelimiterVersions extends Delimiter {
this.keyHandlers[stateId] = keyHandler;
}

handleKey(key: string, value: string): FilterReturnValue {
return this.keyHandlers[this.state.id](key, value);
handleKey(key: string, versionId: string | undefined, value: string): FilterReturnValue {
return this.keyHandlers[this.state.id](key, versionId, value);
}

keyHandler_NotSkippingV0(key: string, value: string): FilterReturnValue {
keyHandler_NotSkippingV0(key: string, versionId: string | undefined, value: string): FilterReturnValue {
if (key.startsWith(DbPrefixes.Replay)) {
// skip internal replay prefix entirely
this.setState(<DelimiterVersionsFilterState_SkippingPrefix> {
Expand All @@ -358,103 +376,44 @@ export class DelimiterVersions extends Delimiter {
if (Version.isPHD(value)) {
return FILTER_ACCEPT;
}
return this.filter_onNewKey(key, value);
return this.filter_onNewKey(key, versionId, value);
}

keyHandler_NotSkippingV1(key: string, value: string): FilterReturnValue {
return this.filter_onNewKey(key, value);
keyHandler_NotSkippingV1(key: string, versionId: string | undefined, value: string): FilterReturnValue {
return this.filter_onNewKey(key, versionId, value);
}

filter_onNewKey(key: string, value: string): FilterReturnValue {
filter_onNewKey(key: string, versionId: string | undefined, value: string): FilterReturnValue {
if (this._reachedMaxKeys()) {
return FILTER_END;
}
const { key: nonversionedKey, versionId: keyVersionId } = this.parseKey(key);
if (this.nullKey &&
(this.nullKey.key !== nonversionedKey
|| this.nullKey.versionId < <string> keyVersionId)) {
this.addCurrentNullKey();
if (this._reachedMaxKeys()) {
// IsTruncated: true is set, which is wanted because
// there is at least one more key to output: the one
// being processed here
return FILTER_END;
}
}
let versionId: string;
if (keyVersionId === undefined) {
if (versionId === undefined) {
this.masterKey = key;
this.masterVersionId = Version.from(value).getVersionId() || 'null';
versionId = this.masterVersionId;
this.addKey(this.masterKey, this.masterVersionId, value);
} else {
if (keyVersionId === '') {
// null key
this.cacheNullKey(nonversionedKey, Version.from(value).getVersionId(), value);
return FILTER_ACCEPT;
}
if (this.masterKey === nonversionedKey && this.masterVersionId === keyVersionId) {
if (this.masterKey === key && this.masterVersionId === versionId) {
// do not add a version key if it is the master version
return FILTER_ACCEPT;
}
versionId = keyVersionId;
}
// add the subprefix to the common prefixes if the key has the delimiter
const commonPrefix = this.getCommonPrefix(nonversionedKey);
if (commonPrefix) {
this.addCommonPrefix(commonPrefix);
// transition into SkippingPrefix state to skip all following keys
// while they start with the same prefix
this.setState(<DelimiterVersionsFilterState_SkippingPrefix> {
id: DelimiterVersionsFilterStateId.SkippingPrefix,
prefix: commonPrefix,
});
return FILTER_ACCEPT;
this.addKey(key, versionId, value);
}
this.addContents(nonversionedKey, versionId, value);
return FILTER_ACCEPT;
}

keyHandler_SkippingPrefix(key: string, value: string): FilterReturnValue {
keyHandler_SkippingPrefix(key: string, versionId: string | undefined, value: string): FilterReturnValue {
const { prefix } = <DelimiterVersionsFilterState_SkippingPrefix> this.state;
if (key.startsWith(prefix)) {
return FILTER_SKIP;
}
this.setState(<DelimiterVersionsFilterState_NotSkipping> {
id: DelimiterVersionsFilterStateId.NotSkipping,
});
return this.handleKey(key, value);
}

keyHandler_WaitForNullKey(key: string, value: string): FilterReturnValue {
const { key: nonversionedKey, versionId } = this.parseKey(key);
if (nonversionedKey !== this.keyMarker) {
this.setState(<DelimiterVersionsFilterState_NotSkipping> {
id: DelimiterVersionsFilterStateId.NotSkipping,
});
return this.handleKey(key, value);
}
// we may now skip versions until VersionIdMarker
this.setState(<DelimiterVersionsFilterState_SkippingVersions> {
id: DelimiterVersionsFilterStateId.SkippingVersions,
gt: `${this.keyMarker}${VID_SEP}${this.versionIdMarker}`,
});

if (versionId === '') {
// only cache the null key if its version is older than
// the current version ID marker, otherwise it has already
// been output in a previous listing output
const nullVersionId = Version.from(value).getVersionId();
if (nullVersionId > this.versionIdMarker) {
this.cacheNullKey(nonversionedKey, nullVersionId, value);
}
return FILTER_SKIP;
}
return this.handleKey(key, value);
return this.handleKey(key, versionId, value);
}

keyHandler_SkippingVersions(key: string, value: string): FilterReturnValue {
const { key: nonversionedKey, versionId } = this.parseKey(key);
if (nonversionedKey === this.keyMarker) {
keyHandler_SkippingVersions(key: string, versionId: string | undefined, value: string): FilterReturnValue {
if (key === this.keyMarker) {
// since the nonversioned key equals the marker, there is
// necessarily a versionId in this key
const _versionId = <string> versionId;
Expand All @@ -470,7 +429,7 @@ export class DelimiterVersions extends Delimiter {
this.setState(<DelimiterVersionsFilterState_NotSkipping> {
id: DelimiterVersionsFilterStateId.NotSkipping,
});
return this.handleKey(key, value);
return this.handleKey(key, versionId, value);
}

skippingBase() {
Expand Down Expand Up @@ -528,8 +487,8 @@ export class DelimiterVersions extends Delimiter {
// does not fit, so we know the result is now truncated
// because there remains the null key to be output.
//
if (this.nullKey && !this._reachedMaxKeys()) {
this.addCurrentNullKey();
if (this.nullKey) {
this.handleKey(this.nullKey.key, this.nullKey.versionId, this.nullKey.value);
}
const result: ResultObject = {
CommonPrefixes: this.CommonPrefixes,
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=16"
},
"version": "7.70.15",
"version": "7.70.16",
"description": "Common utilities for the S3 project components",
"main": "build/index.js",
"repository": {
Expand Down
65 changes: 62 additions & 3 deletions tests/unit/algos/list/delimiterNonCurrent.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ function getListingKey(key, vFormat) {

let expectedParams;
if (v === 'v0') {
expectedParams = { gte: `premark${VID_SEP}`, lt: 'prf' };
expectedParams = { gte: `${keyMarker}${VID_SEP}`, lt: 'prf' };
} else {
expectedParams = [
{
gte: `${DbPrefixes.Master}premark${VID_SEP}`,
gte: `${DbPrefixes.Master}${keyMarker}${VID_SEP}`,
lt: `${DbPrefixes.Master}prf`,
},
{
gte: `${DbPrefixes.Version}premark${VID_SEP}`,
gte: `${DbPrefixes.Version}${keyMarker}${VID_SEP}`,
lt: `${DbPrefixes.Version}prf`,
},
];
Expand Down Expand Up @@ -389,5 +389,64 @@ function getListingKey(key, vFormat) {

assert.deepStrictEqual(delimiter.result(), expectedResult);
});

it('should return noncurrent versions starting from a marker', () => {
const delimiter = new DelimiterNonCurrent({
keyMarker: 'key',
versionIdMarker: 'version1',
}, fakeLogger, v);

const masterKey = 'key';

// filter first version
const versionId1 = 'version1';
const versionKey1 = `${masterKey}${VID_SEP}${versionId1}`;
const date1 = '1970-01-01T00:00:00.002Z';
const value1 = `{"versionId":"${versionId1}", "last-modified": "${date1}"}`;

assert.strictEqual(delimiter.filter({
key: getListingKey(versionKey1, v),
value: value1,
}), FILTER_ACCEPT);

// filter second version
const versionId2 = 'version2';
const versionKey2 = `${masterKey}${VID_SEP}${versionId2}`;
const date2 = '1970-01-01T00:00:00.001Z';
const value2 = `{"versionId":"${versionId2}", "last-modified": "${date2}"}`;

assert.strictEqual(delimiter.filter({
key: getListingKey(versionKey2, v),
value: value2,
}), FILTER_ACCEPT);

// filter third version
const versionId3 = 'version3';
const versionKey3 = `${masterKey}${VID_SEP}${versionId3}`;
const date3 = '1970-01-01T00:00:00.000Z';
const value3 = `{"versionId":"${versionId3}", "last-modified": "${date3}"}`;

assert.strictEqual(delimiter.filter({
key: getListingKey(versionKey3, v),
value: value3,
}), FILTER_ACCEPT);


const expectedResult = {
Contents: [
{
key: masterKey,
value: `{"versionId":"${versionId2}","last-modified":"${date2}","staleDate":"${date1}"}`,
},
{
key: masterKey,
value: `{"versionId":"${versionId3}","last-modified":"${date3}","staleDate":"${date2}"}`,
},
],
IsTruncated: false,
};

assert.deepStrictEqual(delimiter.result(), expectedResult);
});
});
});
Loading
Loading