Skip to content

Commit b21517a

Browse files
nadavMiznadavMiz
committed
NSFS | NC | fix list-objecs-versions issues
Signed-off-by: nadav mizrahi <nadav.mizrahi16@gmail.com>
1 parent 8f62e5b commit b21517a

File tree

2 files changed

+70
-16
lines changed

2 files changed

+70
-16
lines changed

‎src/sdk/namespace_fs.js

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ function _get_version_id_by_stat({ino, mtimeNsBigint}) {
106106
return 'mtime-' + mtimeNsBigint.toString(36) + '-ino-' + ino.toString(36);
107107
}
108108

109-
function _is_version_object_including_null_version(filename) {
109+
function _is_version_or_null_in_file_name(filename) {
110110
const is_version_object = _is_version_object(filename);
111111
if (!is_version_object) {
112112
return _is_version_null_version(filename);
@@ -637,6 +637,7 @@ class NamespaceFS {
637637
* key: string,
638638
* common_prefix: boolean,
639639
* stat?: nb.NativeFSStats,
640+
* is_latest: boolean,
640641
* }} Result
641642
*/
642643

@@ -725,15 +726,17 @@ class NamespaceFS {
725726
const isDir = await is_directory_or_symlink_to_directory(ent, fs_context, path.join(dir_path, ent.name));
726727

727728
let r;
728-
if (list_versions && _is_version_object_including_null_version(ent.name)) {
729+
if (list_versions && _is_version_or_null_in_file_name(ent.name)) {
729730
r = {
730731
key: this._get_version_entry_key(dir_key, ent),
731732
common_prefix: isDir,
733+
is_latest: false
732734
};
733735
} else {
734736
r = {
735737
key: this._get_entry_key(dir_key, ent, isDir),
736738
common_prefix: isDir,
739+
is_latest: true
737740
};
738741
}
739742
await insert_entry_to_results_arr(r);
@@ -848,6 +851,20 @@ class NamespaceFS {
848851
}
849852
};
850853

854+
let previous_key;
855+
/**
856+
* delete markers are always in the .versions folder, so we need to have special case to determine
857+
* if they are delete markers. since the result list is ordered by latest entries first, the first
858+
* entry of every key is the latest
859+
* TODO need different way to check for isLatest in case of unordered list object versions
860+
* @param {Object} obj_info
861+
*/
862+
const set_latest_delete_marker = obj_info => {
863+
if (obj_info.delete_marker && previous_key !== obj_info.key) {
864+
obj_info.is_latest = true;
865+
}
866+
};
867+
851868
const prefix_dir_key = prefix.slice(0, prefix.lastIndexOf('/') + 1);
852869
await process_dir(prefix_dir_key);
853870
await Promise.all(results.map(async r => {
@@ -869,15 +886,17 @@ class NamespaceFS {
869886
if (r.common_prefix) {
870887
res.common_prefixes.push(r.key);
871888
} else {
872-
obj_info = this._get_object_info(bucket, r.key, r.stat, 'null', false, true);
889+
obj_info = this._get_object_info(bucket, r.key, r.stat, false, r.is_latest);
873890
if (!list_versions && obj_info.delete_marker) {
874891
continue;
875892
}
876893
if (this._is_hidden_version_path(obj_info.key)) {
877894
obj_info.key = path.normalize(obj_info.key.replace(HIDDEN_VERSIONS_PATH + '/', ''));
878895
obj_info.key = _get_filename(obj_info.key);
896+
set_latest_delete_marker(obj_info);
879897
}
880898
res.objects.push(obj_info);
899+
previous_key = obj_info.key;
881900
}
882901
if (res.is_truncated) {
883902
if (list_versions && _is_version_object(r.key)) {
@@ -921,7 +940,7 @@ class NamespaceFS {
921940
}
922941
}
923942
this._throw_if_delete_marker(stat);
924-
return this._get_object_info(params.bucket, params.key, stat, params.version_id || 'null', isDir);
943+
return this._get_object_info(params.bucket, params.key, stat, isDir);
925944
} catch (err) {
926945
if (this._should_update_issues_report(params, file_path, err)) {
927946
this.run_update_issues_report(object_sdk, err);
@@ -2293,16 +2312,18 @@ class NamespaceFS {
22932312
}
22942313

22952314
/**
2296-
* @param {string} bucket
2297-
* @param {string} key
2298-
* @param {nb.NativeFSStats} stat
2315+
* @param {string} bucket
2316+
* @param {string} key
2317+
* @param {nb.NativeFSStats} stat
2318+
* @param {Boolean} isDir
2319+
* @param {boolean} [is_latest=true]
22992320
* @returns {nb.ObjectInfo}
23002321
*/
2301-
_get_object_info(bucket, key, stat, return_version_id, isDir, is_latest = true) {
2322+
_get_object_info(bucket, key, stat, isDir, is_latest = true) {
23022323
const etag = this._get_etag(stat);
23032324
const create_time = stat.mtime.getTime();
23042325
const encryption = this._get_encryption_info(stat);
2305-
const version_id = return_version_id && this._is_versioning_enabled() && this._get_version_id_by_xattr(stat);
2326+
const version_id = (this._is_versioning_enabled() || this._is_versioning_suspended()) && this._get_version_id_by_xattr(stat);
23062327
const delete_marker = stat.xattr?.[XATTR_DELETE_MARKER] === 'true';
23072328
const dir_content_type = stat.xattr?.[XATTR_DIR_CONTENT] && ((Number(stat.xattr?.[XATTR_DIR_CONTENT]) > 0 && 'application/octet-stream') || 'application/x-directory');
23082329
const content_type = stat.xattr?.[XATTR_CONTENT_TYPE] ||

‎src/test/unit_tests/test_bucketspace_versioning.js

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2881,6 +2881,7 @@ mocha.describe('List-objects', function() {
28812881
const version_key_2 = 'search_key_mtime-crkfjum9883k-ino-guu7';
28822882
const version_key_3 = 'search_key_mtime-crkfjx1hui2o-ino-guu9';
28832883
const version_key_4 = 'search_key_mtime-crkfjx1hui2o-ino-guuh';
2884+
const key_version = 'mtime-gffdt785k3k-ino-fgy';
28842885

28852886
const dir_key = 'dir1/delete_marker_key';
28862887
const dir_version_key_1 = 'delete_marker_key_mtime-crkfjknr7xmo-ino-guu4';
@@ -2950,13 +2951,13 @@ mocha.describe('List-objects', function() {
29502951
await s3_client.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
29512952
const bucket_ver = await s3_client.getBucketVersioning({ Bucket: bucket_name });
29522953
assert.equal(bucket_ver.Status, 'Enabled');
2953-
await create_object(`${full_path}/${key}`, body, 'null');
2954-
await create_object(`${full_path_version_dir}/${version_key_1}`, version_body, 'null');
2955-
await create_object(`${full_path_version_dir}/${version_key_2}`, version_body, 'null');
2956-
await create_object(`${full_path_version_dir}/${version_key_3}`, version_body, 'null');
2954+
await create_object(`${full_path}/${key}`, body, key_version);
2955+
await create_object(`${full_path_version_dir}/${version_key_1}`, version_body, 'mtime-crh3783sxk3k-ino-guty');
2956+
await create_object(`${full_path_version_dir}/${version_key_2}`, version_body, 'mtime-crkfjum9883k-ino-guu7');
2957+
await create_object(`${full_path_version_dir}/${version_key_3}`, version_body, 'mtime-crkfjx1hui2o-ino-guu9');
29572958
file_pointer = await create_object(`${full_path}/${dir_key}`, version_body, 'null', true);
2958-
await create_object(`${dir1_version_dir}/${dir_version_key_1}`, version_body, 'null');
2959-
await create_object(`${dir1_version_dir}/${dir_version_key_2}`, version_body, 'null');
2959+
await create_object(`${dir1_version_dir}/${dir_version_key_1}`, version_body, 'mtime-crkfjknr7xmo-ino-guu4');
2960+
await create_object(`${dir1_version_dir}/${dir_version_key_2}`, version_body, 'mtime-crkfjr98uiv4-ino-guu6');
29602961
});
29612962

29622963
mocha.after(async () => {
@@ -3088,7 +3089,7 @@ mocha.describe('List-objects', function() {
30883089
file_pointer.replacexattr(DEFAULT_FS_CONFIG, xattr_delete_marker);
30893090
const res = await s3_client.listObjectVersions({Bucket: bucket_name});
30903091

3091-
res.Versions.forEach(val => {
3092+
res.DeleteMarkers.forEach(val => {
30923093
if (val.Key === dir_key) {
30933094
assert.equal(val.IsLatest, true);
30943095
assert.equal(res.DeleteMarkers[0].IsLatest, true);
@@ -3103,6 +3104,38 @@ mocha.describe('List-objects', function() {
31033104
assert.notEqual(obj.Prefix, ".versions/");
31043105
});
31053106
});
3107+
3108+
mocha.it('list object versions - should show isLatest flag only for latest objects', async function() {
3109+
const res = await s3_client.listObjectVersions({Bucket: bucket_name});
3110+
let count = 0;
3111+
res.Versions.forEach(val => {
3112+
if (val.IsLatest) {
3113+
if (val.Key === key) {
3114+
assert.equal(val.VersionId, key_version);
3115+
}
3116+
count += 1;
3117+
}
3118+
});
3119+
res.DeleteMarkers.forEach(val => {
3120+
if (val.IsLatest) {
3121+
assert.equal(val.VersionId, 'null');
3122+
count += 1;
3123+
}
3124+
});
3125+
assert.equal(count, 2);
3126+
});
3127+
3128+
mocha.it('list object versions - should show version-id in suspention mode', async function() {
3129+
await s3_client.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Suspended' } });
3130+
const res = await s3_client.listObjectVersions({Bucket: bucket_name});
3131+
let count = 0;
3132+
res.Versions.forEach(val => {
3133+
if (val.VersionId !== 'null') {
3134+
count += 1;
3135+
}
3136+
});
3137+
assert.equal(count, 6);
3138+
});
31063139
});
31073140

31083141
async function create_object(object_path, data, version_id, return_fd) {

0 commit comments

Comments
 (0)