Skip to content

Commit

Permalink
NSFS | NC | fix list-objecs-versions issues
Browse files Browse the repository at this point in the history 
Signed-off-by: nadav mizrahi <[email protected]>
(cherry picked from commit b21517a)
  • Loading branch information
@nadavMiz @romayalon
nadavMiz authored and romayalon committed Sep 19, 2024
1 parent 55002e2 commit 449cdeb
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 16 deletions.
39 changes: 30 additions & 9 deletions src/sdk/namespace_fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function _get_version_id_by_stat({ino, mtimeNsBigint}) {
return 'mtime-' + mtimeNsBigint.toString(36) + '-ino-' + ino.toString(36);
}

function _is_version_object_including_null_version(filename) {
function _is_version_or_null_in_file_name(filename) {
const is_version_object = _is_version_object(filename);
if (!is_version_object) {
return _is_version_null_version(filename);
Expand Down Expand Up @@ -626,6 +626,7 @@ class NamespaceFS {
* key: string,
* common_prefix: boolean,
* stat?: nb.NativeFSStats,
* is_latest: boolean,
* }} Result
*/

Expand Down Expand Up @@ -714,15 +715,17 @@ class NamespaceFS {
const isDir = await is_directory_or_symlink_to_directory(ent, fs_context, path.join(dir_path, ent.name));

let r;
if (list_versions && _is_version_object_including_null_version(ent.name)) {
if (list_versions && _is_version_or_null_in_file_name(ent.name)) {
r = {
key: this._get_version_entry_key(dir_key, ent),
common_prefix: isDir,
is_latest: false
};
} else {
r = {
key: this._get_entry_key(dir_key, ent, isDir),
common_prefix: isDir,
is_latest: true
};
}
await insert_entry_to_results_arr(r);
Expand Down Expand Up @@ -837,6 +840,20 @@ class NamespaceFS {
}
};

let previous_key;
/**
* delete markers are always in the .versions folder, so we need to have special case to determine
* if they are delete markers. since the result list is ordered by latest entries first, the first
* entry of every key is the latest
* TODO need different way to check for isLatest in case of unordered list object versions
* @param {Object} obj_info
*/
const set_latest_delete_marker = obj_info => {
if (obj_info.delete_marker && previous_key !== obj_info.key) {
obj_info.is_latest = true;
}
};

const prefix_dir_key = prefix.slice(0, prefix.lastIndexOf('/') + 1);
await process_dir(prefix_dir_key);
await Promise.all(results.map(async r => {
Expand All @@ -858,15 +875,17 @@ class NamespaceFS {
if (r.common_prefix) {
res.common_prefixes.push(r.key);
} else {
obj_info = this._get_object_info(bucket, r.key, r.stat, 'null', false, true);
obj_info = this._get_object_info(bucket, r.key, r.stat, false, r.is_latest);
if (!list_versions && obj_info.delete_marker) {
continue;
}
if (this._is_hidden_version_path(obj_info.key)) {
obj_info.key = path.normalize(obj_info.key.replace(HIDDEN_VERSIONS_PATH + '/', ''));
obj_info.key = _get_filename(obj_info.key);
set_latest_delete_marker(obj_info);
}
res.objects.push(obj_info);
previous_key = obj_info.key;
}
if (res.is_truncated) {
if (list_versions && _is_version_object(r.key)) {
Expand Down Expand Up @@ -910,7 +929,7 @@ class NamespaceFS {
}
}
this._throw_if_delete_marker(stat);
return this._get_object_info(params.bucket, params.key, stat, params.version_id || 'null', isDir);
return this._get_object_info(params.bucket, params.key, stat, isDir);
} catch (err) {
if (this._should_update_issues_report(params, file_path, err)) {
this.run_update_issues_report(object_sdk, err);
Expand Down Expand Up @@ -2272,16 +2291,18 @@ class NamespaceFS {
}

/**
* @param {string} bucket
* @param {string} key
* @param {nb.NativeFSStats} stat
* @param {string} bucket
* @param {string} key
* @param {nb.NativeFSStats} stat
* @param {Boolean} isDir
* @param {boolean} [is_latest=true]
* @returns {nb.ObjectInfo}
*/
_get_object_info(bucket, key, stat, return_version_id, isDir, is_latest = true) {
_get_object_info(bucket, key, stat, isDir, is_latest = true) {
const etag = this._get_etag(stat);
const create_time = stat.mtime.getTime();
const encryption = this._get_encryption_info(stat);
const version_id = return_version_id && this._is_versioning_enabled() && this._get_version_id_by_xattr(stat);
const version_id = (this._is_versioning_enabled() || this._is_versioning_suspended()) && this._get_version_id_by_xattr(stat);
const delete_marker = stat.xattr?.[XATTR_DELETE_MARKER] === 'true';
const dir_content_type = stat.xattr?.[XATTR_DIR_CONTENT] && ((Number(stat.xattr?.[XATTR_DIR_CONTENT]) > 0 && 'application/octet-stream') || 'application/x-directory');
const content_type = stat.xattr?.[XATTR_CONTENT_TYPE] ||
Expand Down
47 changes: 40 additions & 7 deletions src/test/unit_tests/test_bucketspace_versioning.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2846,6 +2846,7 @@ mocha.describe('List-objects', function() {
const version_key_2 = 'search_key_mtime-crkfjum9883k-ino-guu7';
const version_key_3 = 'search_key_mtime-crkfjx1hui2o-ino-guu9';
const version_key_4 = 'search_key_mtime-crkfjx1hui2o-ino-guuh';
const key_version = 'mtime-gffdt785k3k-ino-fgy';

const dir_key = 'dir1/delete_marker_key';
const dir_version_key_1 = 'delete_marker_key_mtime-crkfjknr7xmo-ino-guu4';
Expand Down Expand Up @@ -2910,13 +2911,13 @@ mocha.describe('List-objects', function() {
await s3_client.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Enabled' } });
const bucket_ver = await s3_client.getBucketVersioning({ Bucket: bucket_name });
assert.equal(bucket_ver.Status, 'Enabled');
await create_object(`${full_path}/${key}`, body, 'null');
await create_object(`${full_path_version_dir}/${version_key_1}`, version_body, 'null');
await create_object(`${full_path_version_dir}/${version_key_2}`, version_body, 'null');
await create_object(`${full_path_version_dir}/${version_key_3}`, version_body, 'null');
await create_object(`${full_path}/${key}`, body, key_version);
await create_object(`${full_path_version_dir}/${version_key_1}`, version_body, 'mtime-crh3783sxk3k-ino-guty');
await create_object(`${full_path_version_dir}/${version_key_2}`, version_body, 'mtime-crkfjum9883k-ino-guu7');
await create_object(`${full_path_version_dir}/${version_key_3}`, version_body, 'mtime-crkfjx1hui2o-ino-guu9');
file_pointer = await create_object(`${full_path}/${dir_key}`, version_body, 'null', true);
await create_object(`${dir1_version_dir}/${dir_version_key_1}`, version_body, 'null');
await create_object(`${dir1_version_dir}/${dir_version_key_2}`, version_body, 'null');
await create_object(`${dir1_version_dir}/${dir_version_key_1}`, version_body, 'mtime-crkfjknr7xmo-ino-guu4');
await create_object(`${dir1_version_dir}/${dir_version_key_2}`, version_body, 'mtime-crkfjr98uiv4-ino-guu6');
});

mocha.after(async () => {
Expand Down Expand Up @@ -3048,14 +3049,46 @@ mocha.describe('List-objects', function() {
file_pointer.replacexattr(DEFAULT_FS_CONFIG, xattr_delete_marker);
const res = await s3_client.listObjectVersions({Bucket: bucket_name});

res.Versions.forEach(val => {
res.DeleteMarkers.forEach(val => {
if (val.Key === dir_key) {
assert.equal(val.IsLatest, true);
assert.equal(res.DeleteMarkers[0].IsLatest, true);
assert.equal(res.DeleteMarkers[0].Key, dir_key);
}
});
});

mocha.it('list object versions - should show isLatest flag only for latest objects', async function() {
const res = await s3_client.listObjectVersions({Bucket: bucket_name});
let count = 0;
res.Versions.forEach(val => {
if (val.IsLatest) {
if (val.Key === key) {
assert.equal(val.VersionId, key_version);
}
count += 1;
}
});
res.DeleteMarkers.forEach(val => {
if (val.IsLatest) {
assert.equal(val.VersionId, 'null');
count += 1;
}
});
assert.equal(count, 2);
});

mocha.it('list object versions - should show version-id in suspention mode', async function() {
await s3_client.putBucketVersioning({ Bucket: bucket_name, VersioningConfiguration: { MFADelete: 'Disabled', Status: 'Suspended' } });
const res = await s3_client.listObjectVersions({Bucket: bucket_name});
let count = 0;
res.Versions.forEach(val => {
if (val.VersionId !== 'null') {
count += 1;
}
});
assert.equal(count, 6);
});
});

async function create_object(object_path, data, version_id, return_fd) {
Expand Down

0 comments on commit 449cdeb

Please sign in to comment.