From 79ed1477f60a1824c7c87a720bf13b2f903047a7 Mon Sep 17 00:00:00 2001 From: Romy <35330373+romayalon@users.noreply.github.com> Date: Tue, 18 Feb 2025 18:20:57 +0200 Subject: [PATCH] NSFS | Fix delete last object deletes directory object if its size is 0 Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com> --- src/sdk/namespace_fs.js | 44 +++++++++++++++++++----- src/test/unit_tests/test_namespace_fs.js | 30 ++++++++++++++++ 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/sdk/namespace_fs.js b/src/sdk/namespace_fs.js index 1ed3ddbecf..9f812d19ee 100644 --- a/src/sdk/namespace_fs.js +++ b/src/sdk/namespace_fs.js @@ -2618,12 +2618,32 @@ class NamespaceFS { } } + /** + * _delete_path_dirs deletes all the paths in the hierarchy that are empty after a successful delete + * if the original file_path to be deleted is a regular object which means file_path is not a directory and it's not a directory object path - + * before deletion of the parent directory - + * if the parent directory is a directory object (has CONTENT_DIR xattr) - stop the deletion loop + * else - delete the directory - if dir is not empty it will stop at the first non empty dir + * NOTE - the directory object check is needed because when object size is zero we won't create a .folder file and the dir will be empty + * therefore the deletion will succeed although we shouldn't delete the directory object + * @param {String} file_path + * @param {nb.NativeFSContext} fs_context + */ async _delete_path_dirs(file_path, fs_context) { try { - let dir = path.dirname(file_path); - while (dir !== this.bucket_path) { - await nb_native().fs.rmdir(fs_context, dir); - dir = path.dirname(dir); + let dir_path = path.dirname(file_path); + const deleted_file_is_dir = file_path.endsWith('/'); + const deleted_file_is_dir_object = file_path.endsWith(config.NSFS_FOLDER_OBJECT_NAME); + let should_check_dir_path_is_content_dir = !deleted_file_is_dir && !deleted_file_is_dir_object; + while (dir_path !== this.bucket_path) { + if (should_check_dir_path_is_content_dir) { + const dir_stat = await nb_native().fs.stat(fs_context, dir_path); + const file_is_disabled_dir_content = dir_stat.xattr && dir_stat.xattr[XATTR_DIR_CONTENT] !== undefined; + if (file_is_disabled_dir_content) break; + } + await nb_native().fs.rmdir(fs_context, dir_path); + dir_path = path.dirname(dir_path); + should_check_dir_path_is_content_dir = true; } } catch (err) { if (err.code !== 'ENOTEMPTY' && @@ -3068,10 +3088,18 @@ class NamespaceFS { return res; } - // delete version_id - - // 1. get version info, if it's empty - return - // 2. unlink key - // 3. if version is latest version - promote second latest -> latest + /** + * delete version_id does the following - + * 1. get version info, if it's empty - return + * 2. unlink key + * 3. if version is latest version - promote second latest -> latest + * 4. if it's the latest version - delete the directory hirerachy of the key if it's empty + * if it's a past version - delete .versions/ and the directory hirerachy if it's empty + * @param {nb.NativeFSContext} fs_context + * @param {String} file_path + * @param {Object} params + * @returns {Promise<{deleted_delete_marker?: string, version_id?: string}>} + */ async _delete_version_id(fs_context, file_path, params) { // TODO optimization - GPFS link overrides, no need to unlink before promoting, but if there is nothing to promote we should unlink const del_obj_version_info = await this._delete_single_object_versioned(fs_context, params.key, params.version_id); diff --git a/src/test/unit_tests/test_namespace_fs.js b/src/test/unit_tests/test_namespace_fs.js index 9f9efb00cf..906b1596fe 100644 --- a/src/test/unit_tests/test_namespace_fs.js +++ b/src/test/unit_tests/test_namespace_fs.js @@ -1201,6 +1201,36 @@ mocha.describe('namespace_fs folders tests', function() { }); + mocha.it('delete inner object in directory object size 0 - no .folder file but directory still exists', async function() { + const inner_key = '/inner_obj'; + const key = upload_key_3 + inner_key; + const source = buffer_utils.buffer_to_read_stream(data); + await upload_object(ns_tmp, upload_bkt, key, dummy_object_sdk, source); + const p1 = path.join(ns_tmp_bucket_path, upload_key_3); + const p2 = path.join(ns_tmp_bucket_path, key); + await fs_utils.file_must_not_exist(path.join(p1, config.NSFS_FOLDER_OBJECT_NAME)); + const full_xattr1 = await get_xattr(p1); + assert.deepEqual(full_xattr1, { ...user_md_and_dir_content_xattr, [XATTR_DIR_CONTENT]: obj_sizes_map[upload_key_3] }); + await ns_tmp.delete_object({ bucket: upload_bkt, key: key }, dummy_object_sdk); + await fs_utils.file_must_exist(p1); + await fs_utils.file_must_not_exist(p2); + }); + + mocha.it('delete inner directory object size > 0 in directory object size 0 - no .folder file but directory still exists', async function() { + const inner_dir_obj_key = '/inner_dir_obj_key'; + const key = upload_key_3 + inner_dir_obj_key; + const source = buffer_utils.buffer_to_read_stream(data); + await upload_object(ns_tmp, upload_bkt, key, dummy_object_sdk, source); + const p1 = path.join(ns_tmp_bucket_path, upload_key_3); + const p2 = path.join(ns_tmp_bucket_path, key); + await fs_utils.file_must_not_exist(path.join(p1, config.NSFS_FOLDER_OBJECT_NAME)); + const full_xattr1 = await get_xattr(p1); + assert.deepEqual(full_xattr1, { ...user_md_and_dir_content_xattr, [XATTR_DIR_CONTENT]: obj_sizes_map[upload_key_3] }); + await ns_tmp.delete_object({ bucket: upload_bkt, key: key }, dummy_object_sdk); + await fs_utils.file_must_exist(p1); + await fs_utils.file_must_not_exist(p2); + }); + mocha.it('delete object content 0 - no .folder file', async function() { const p1 = path.join(ns_tmp_bucket_path, upload_key_3); const full_xattr1 = await get_xattr(p1);