From be4a555346798271a6d1efc6719999d2a28ac22f Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Fri, 27 Sep 2024 15:22:59 +0300 Subject: [PATCH 1/9] Add new C API --- tiledb/api/c_api/group/group_api.cc | 37 ++++++++++++++++++++- tiledb/api/c_api/group/group_api_external.h | 33 ++++++++++++++++++ tiledb/sm/enums/object_type.h | 2 +- tiledb/sm/group/group.cc | 5 +-- tiledb/sm/group/group.h | 4 ++- tiledb/sm/group/group_details.cc | 25 +++++++++----- tiledb/sm/group/group_details.h | 4 ++- 7 files changed, 95 insertions(+), 15 deletions(-) diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index 8696fd91cc6..c2359f73848 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -270,7 +270,30 @@ capi_return_t tiledb_group_add_member( name_optional = name; } - group->group().mark_member_for_addition(uri, relative, name_optional); + group->group().mark_member_for_addition( + uri, relative, name_optional, std::nullopt); + + return TILEDB_OK; +} + +capi_return_t tiledb_group_add_member_by_type( + tiledb_group_handle_t* group, + const char* group_uri, + const uint8_t relative, + const char* name, + tiledb_object_t type) { + ensure_group_is_valid(group); + ensure_group_uri_argument_is_valid(group_uri); + + auto uri = tiledb::sm::URI(group_uri, !relative); + + std::optional name_optional = std::nullopt; + if (name != nullptr) { + name_optional = name; + } + + group->group().mark_member_for_addition( + uri, relative, name_optional, static_cast(type)); return TILEDB_OK; } @@ -659,6 +682,18 @@ CAPI_INTERFACE( ctx, group, uri, relative, name); } +CAPI_INTERFACE( + group_add_member_by_type, + tiledb_ctx_t* ctx, + tiledb_group_t* group, + const char* uri, + const uint8_t relative, + const char* name, + tiledb_object_t type) { + return api_entry_context( + ctx, group, uri, relative, name, type); +} + CAPI_INTERFACE( group_remove_member, tiledb_ctx_t* ctx, diff --git a/tiledb/api/c_api/group/group_api_external.h b/tiledb/api/c_api/group/group_api_external.h index c8bf99ba472..74d32b225a0 100644 --- a/tiledb/api/c_api/group/group_api_external.h +++ b/tiledb/api/c_api/group/group_api_external.h @@ -373,6 +373,39 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( const uint8_t relative, const char* name) TILEDB_NOEXCEPT; +/** + * Add a member with a known type to a group + * The caller should make sure the correct type is provided for the member being + * added as this API will not check it for correctness in favor of efficiency + * and results can be undefined otherwise. + * + * **Example:** + * + * @code{.c} + * tiledb_group_t* group; + * tiledb_group_alloc(ctx, "s3://tiledb_bucket/my_group", &group); + * tiledb_group_open(ctx, group, TILEDB_WRITE); + * tiledb_group_add_member(ctx, group, "s3://tiledb_bucket/my_array", + * TILEDB_ARRAY); tiledb_group_add_member(ctx, group, + * "s3://tiledb_bucket/my_group_2", TILEDB_GROUP); + * @endcode + * + * @param ctx The TileDB context. + * @param group An group opened in WRITE mode. + * @param uri URI of member to add + * @param relative is the URI relative to the group + * @param name optional name group member can be given to be looked up by. + * Can be set to NULL. + * @param type the type of the member getting added if known in advance. + * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. + */ +TILEDB_EXPORT capi_return_t tiledb_group_add_member_by_type( + tiledb_group_handle_t* group, + const char* group_uri, + const uint8_t relative, + const char* name, + tiledb_object_t type) TILEDB_NOEXCEPT; + /** * Remove a member from a group * diff --git a/tiledb/sm/enums/object_type.h b/tiledb/sm/enums/object_type.h index 6b4b02cd0dc..cb9ab4497b2 100644 --- a/tiledb/sm/enums/object_type.h +++ b/tiledb/sm/enums/object_type.h @@ -28,7 +28,7 @@ * @section DESCRIPTION * * This file defines the tiledb ObjectType enum that maps to the - * tiledb_object_type_t C-api enum + * tiledb_object_t C-api enum */ #ifndef TILEDB_OBJECT_TYPE_H diff --git a/tiledb/sm/group/group.cc b/tiledb/sm/group/group.cc index aa8eed1fbf4..af32b6ecfab 100644 --- a/tiledb/sm/group/group.cc +++ b/tiledb/sm/group/group.cc @@ -643,7 +643,8 @@ void Group::delete_member(const shared_ptr group_member) { void Group::mark_member_for_addition( const URI& group_member_uri, const bool& relative, - std::optional& name) { + std::optional& name, + std::optional type) { std::lock_guard lck(mtx_); // Check if group is open if (!is_open_) { @@ -666,7 +667,7 @@ void Group::mark_member_for_addition( "mode"); } group_details_->mark_member_for_addition( - resources_, group_member_uri, relative, name); + resources_, group_member_uri, relative, name, type); } void Group::mark_member_for_removal(const std::string& name) { diff --git a/tiledb/sm/group/group.h b/tiledb/sm/group/group.h index b2ebb2f022b..3c33a025e03 100644 --- a/tiledb/sm/group/group.h +++ b/tiledb/sm/group/group.h @@ -299,11 +299,13 @@ class Group { * @param group_member_uri group member uri * @param relative is this URI relative * @param name optional name for member + * @param type optional type for member if known in advance */ void mark_member_for_addition( const URI& group_member_uri, const bool& relative, - std::optional& name); + std::optional& name, + std::optional type); /** * Remove a member from a group, this will be flushed to disk on close diff --git a/tiledb/sm/group/group_details.cc b/tiledb/sm/group/group_details.cc index cff44effcc5..3d4e5fce276 100644 --- a/tiledb/sm/group/group_details.cc +++ b/tiledb/sm/group/group_details.cc @@ -85,22 +85,29 @@ void GroupDetails::mark_member_for_addition( ContextResources& resources, const URI& group_member_uri, const bool& relative, - std::optional& name) { + std::optional& name, + std::optional type) { std::lock_guard lck(mtx_); - URI absolute_group_member_uri = group_member_uri; - if (relative) { - absolute_group_member_uri = - group_uri_.join_path(group_member_uri.to_string()); + ObjectType obj_type; + if (type.has_value()) { + obj_type = type.value(); + } else { + URI absolute_group_member_uri = group_member_uri; + if (relative) { + absolute_group_member_uri = + group_uri_.join_path(group_member_uri.to_string()); + } + obj_type = object_type(resources, absolute_group_member_uri); } - ObjectType type = object_type(resources, absolute_group_member_uri); - if (type == ObjectType::INVALID) { + + if (obj_type == ObjectType::INVALID) { throw GroupDetailsException( - "Cannot add group member " + absolute_group_member_uri.to_string() + + "Cannot add group member " + group_member_uri.to_string() + ", type is INVALID. The member likely does not exist."); } auto group_member = tdb::make_shared( - HERE(), group_member_uri, type, relative, name, false); + HERE(), group_member_uri, obj_type, relative, name, false); if (!member_keys_to_add_.insert(group_member->key()).second) { throw GroupDetailsException( diff --git a/tiledb/sm/group/group_details.h b/tiledb/sm/group/group_details.h index 71c91cbdb40..cc73d151fdc 100644 --- a/tiledb/sm/group/group_details.h +++ b/tiledb/sm/group/group_details.h @@ -67,12 +67,14 @@ class GroupDetails { * @param group_member_uri group member uri * @param relative is this URI relative * @param name optional name for member + * @param type optional type for member if known in advance */ void mark_member_for_addition( ContextResources& resources, const URI& group_member_uri, const bool& relative, - std::optional& name); + std::optional& name, + std::optional type); /** * Remove a member from a group, this will be flushed to disk on close From 6118738fab6a477ca887ee5d7be69f159dbac03f Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Fri, 27 Sep 2024 16:19:40 +0300 Subject: [PATCH 2/9] UTs and fixes --- test/src/unit-capi-group.cc | 140 ++++++++++++++------ tiledb/api/c_api/group/group_api_external.h | 1 + 2 files changed, 104 insertions(+), 37 deletions(-) diff --git a/test/src/unit-capi-group.cc b/test/src/unit-capi-group.cc index eeacbb13638..2aa3f164cf1 100644 --- a/test/src/unit-capi-group.cc +++ b/test/src/unit-capi-group.cc @@ -592,18 +592,34 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group1, array1_uri.c_str(), false, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group1, array2_uri.c_str(), false, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group2, array3_uri.c_str(), false, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group1, group2_uri.c_str(), false, nullptr); - REQUIRE(rc == TILEDB_OK); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + rc = tiledb_group_add_member_by_type( + ctx_, group1, array1_uri.c_str(), false, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, array2_uri.c_str(), false, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group2, array3_uri.c_str(), false, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, group2_uri.c_str(), false, nullptr, TILEDB_GROUP); + REQUIRE(rc == TILEDB_OK); + } else { + rc = tiledb_group_add_member( + ctx_, group1, array1_uri.c_str(), false, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group1, array2_uri.c_str(), false, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group2, array3_uri.c_str(), false, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group1, group2_uri.c_str(), false, nullptr); + REQUIRE(rc == TILEDB_OK); + } // Close group from write mode rc = tiledb_group_close(ctx_, group1); @@ -720,8 +736,16 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group1, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member(ctx_, group1, array1_uri.c_str(), false, "one"); - REQUIRE(rc == TILEDB_OK); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + rc = tiledb_group_add_member_by_type( + ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + } else { + rc = + tiledb_group_add_member(ctx_, group1, array1_uri.c_str(), false, "one"); + REQUIRE(rc == TILEDB_OK); + } // Close group from write mode rc = tiledb_group_close(ctx_, group1); @@ -755,8 +779,15 @@ TEST_CASE_METHOD( rc = tiledb_group_remove_member(ctx_, group1, "one"); REQUIRE(rc == TILEDB_OK); // Add one back with different URI - rc = tiledb_group_add_member(ctx_, group1, array2_uri.c_str(), false, "one"); - REQUIRE(rc == TILEDB_OK); + if (add_by_type) { + rc = tiledb_group_add_member_by_type( + ctx_, group1, array2_uri.c_str(), false, "one", TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + } else { + rc = + tiledb_group_add_member(ctx_, group1, array2_uri.c_str(), false, "one"); + REQUIRE(rc == TILEDB_OK); + } // Close group rc = tiledb_group_close(ctx_, group1); @@ -827,15 +858,34 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member(ctx_, group1, array1_uri.c_str(), false, "one"); - REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member(ctx_, group1, array2_uri.c_str(), false, "two"); - REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group2, array3_uri.c_str(), false, "three"); - REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member(ctx_, group1, group2_uri.c_str(), false, "four"); - REQUIRE(rc == TILEDB_OK); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + rc = tiledb_group_add_member_by_type( + ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, array2_uri.c_str(), false, "two", TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group2, array3_uri.c_str(), false, "three", TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, group2_uri.c_str(), false, "four", TILEDB_GROUP); + REQUIRE(rc == TILEDB_OK); + } else { + rc = + tiledb_group_add_member(ctx_, group1, array1_uri.c_str(), false, "one"); + REQUIRE(rc == TILEDB_OK); + rc = + tiledb_group_add_member(ctx_, group1, array2_uri.c_str(), false, "two"); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group2, array3_uri.c_str(), false, "three"); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group1, group2_uri.c_str(), false, "four"); + REQUIRE(rc == TILEDB_OK); + } // Close group from write mode rc = tiledb_group_close(ctx_, group1); @@ -1186,18 +1236,34 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member( - ctx_, group1, array1_relative_uri.c_str(), true, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member( - ctx_, group1, array2_relative_uri.c_str(), true, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member( - ctx_, group2, array3_relative_uri.c_str(), true, nullptr); - REQUIRE(rc == TILEDB_OK); - rc = - tiledb_group_add_member(ctx_, group1, group2_uri.c_str(), false, nullptr); - REQUIRE(rc == TILEDB_OK); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + rc = tiledb_group_add_member_by_type( + ctx_, group1, array1_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, array2_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group2, array3_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member_by_type( + ctx_, group1, group2_uri.c_str(), false, nullptr, TILEDB_GROUP); + REQUIRE(rc == TILEDB_OK); + } else { + rc = tiledb_group_add_member( + ctx_, group1, array1_relative_uri.c_str(), true, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group1, array2_relative_uri.c_str(), true, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group2, array3_relative_uri.c_str(), true, nullptr); + REQUIRE(rc == TILEDB_OK); + rc = tiledb_group_add_member( + ctx_, group1, group2_uri.c_str(), false, nullptr); + REQUIRE(rc == TILEDB_OK); + } // Close group from write mode rc = tiledb_group_close(ctx_, group1); diff --git a/tiledb/api/c_api/group/group_api_external.h b/tiledb/api/c_api/group/group_api_external.h index 74d32b225a0..045f9520f9c 100644 --- a/tiledb/api/c_api/group/group_api_external.h +++ b/tiledb/api/c_api/group/group_api_external.h @@ -400,6 +400,7 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */ TILEDB_EXPORT capi_return_t tiledb_group_add_member_by_type( + tiledb_ctx_t* ctx, tiledb_group_handle_t* group, const char* group_uri, const uint8_t relative, From 170441ad434cad96eeab82cb88f87fc75cdc1dbf Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Mon, 30 Sep 2024 09:29:12 +0000 Subject: [PATCH 3/9] Fix declaration and add UTs to rest --- test/src/unit-cppapi-group.cc | 349 ++++++++------------ tiledb/api/c_api/group/group_api_external.h | 4 +- 2 files changed, 134 insertions(+), 219 deletions(-) diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 524dfdcd3f0..8d3d9c3bc08 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -57,44 +57,21 @@ struct GroupCPPFx { const std::string ARRAY = "array/"; // TileDB context - tiledb::Context ctx_; + tiledb::test::VFSTestSetup vfs_test_setup_; tiledb_ctx_t* ctx_c_; - tiledb_vfs_t* vfs_; - - // Vector of supported filesystems - const std::vector> fs_vec_; - - /** - * If true, array schema is serialized before submission, to test the - * serialization paths. - */ - bool serialize_ = false; + tiledb::Context ctx_; // Functions GroupCPPFx(); - ~GroupCPPFx(); void create_array(const std::string& path) const; - void create_temp_dir(const std::string& path) const; - void remove_temp_dir(const std::string& path) const; std::vector read_group(const tiledb::Group& group) const; void set_group_timestamp( tiledb::Group* group, const uint64_t& timestamp) const; }; GroupCPPFx::GroupCPPFx() - : fs_vec_(vfs_test_get_fs_vec()) { - // Initialize vfs test - ctx_c_ = nullptr; - vfs_ = nullptr; - REQUIRE(vfs_test_init(fs_vec_, &ctx_c_, &vfs_).ok()); - ctx_ = tiledb::Context(ctx_c_, false); -} - -GroupCPPFx::~GroupCPPFx() { - // Close vfs test - REQUIRE(vfs_test_close(fs_vec_, ctx_c_, vfs_).ok()); - tiledb_vfs_free(&vfs_); - tiledb_ctx_free(&ctx_c_); + : ctx_c_(vfs_test_setup_.ctx_c) + , ctx_(vfs_test_setup_.ctx()) { } void GroupCPPFx::set_group_timestamp( @@ -115,18 +92,6 @@ std::vector GroupCPPFx::read_group( return ret; } -void GroupCPPFx::create_temp_dir(const std::string& path) const { - remove_temp_dir(path); - REQUIRE(tiledb_vfs_create_dir(ctx_c_, vfs_, path.c_str()) == TILEDB_OK); -} - -void GroupCPPFx::remove_temp_dir(const std::string& path) const { - int is_dir = 0; - REQUIRE(tiledb_vfs_is_dir(ctx_c_, vfs_, path.c_str(), &is_dir) == TILEDB_OK); - if (is_dir) - REQUIRE(tiledb_vfs_remove_dir(ctx_c_, vfs_, path.c_str()) == TILEDB_OK); -} - void GroupCPPFx::create_array(const std::string& path) const { tiledb_attribute_t* a1; tiledb_attribute_alloc(ctx_c_, "a1", TILEDB_FLOAT32, &a1); @@ -155,9 +120,7 @@ void GroupCPPFx::create_array(const std::string& path) const { REQUIRE(tiledb_array_schema_check(ctx_c_, array_schema) == TILEDB_OK); // Create array - REQUIRE( - tiledb_array_create_serialization_wrapper( - ctx_c_, path, array_schema, serialize_) == TILEDB_OK); + REQUIRE(tiledb_array_create(ctx_c_, path.c_str(), array_schema) == TILEDB_OK); // Free objects tiledb_attribute_free(&a1); @@ -169,12 +132,8 @@ void GroupCPPFx::create_array(const std::string& path) const { TEST_CASE_METHOD( GroupCPPFx, "C++ API: Test creating group with config", - "[cppapi][group][config]") { - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); - - std::string group1_uri = temp_dir + "group1"; + "[cppapi][group][config][rest]") { + std::string group1_uri = vfs_test_setup_.array_uri("group1"); tiledb::Group::create(ctx_, group1_uri); const std::string& test_key = "foo"; @@ -190,12 +149,11 @@ TEST_CASE_METHOD( } TEST_CASE_METHOD( - GroupCPPFx, "C++ API: Test group metadata", "[cppapi][group][metadata]") { - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); + GroupCPPFx, + "C++ API: Test group metadata", + "[cppapi][group][metadata][rest]") { + std::string group1_uri = vfs_test_setup_.array_uri("group1"); - std::string group1_uri = temp_dir + "group1"; tiledb::Group::create(ctx_, group1_uri); tiledb::Group group(ctx_, group1_uri, TILEDB_WRITE); group.close(); @@ -220,27 +178,25 @@ TEST_CASE_METHOD( // Write a correct item group.put_metadata("key", TILEDB_INT32, 1, &v); - // Consolidate and vacuum metadata with default config - group.consolidate_metadata(ctx_, group1_uri); - group.vacuum_metadata(ctx_, group1_uri); + // For some reason we don't yet allow group metadata consolidation so + // disabling for the time being from REST testing + if (!vfs_test_setup_.is_rest()) { + // Consolidate and vacuum metadata with default config + group.consolidate_metadata(ctx_, group1_uri); + group.vacuum_metadata(ctx_, group1_uri); + } // Close group group.close(); - - // Clean up - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( GroupCPPFx, "C++ API: Group Metadata, write/read", - "[cppapi][group][metadata][read]") { + "[cppapi][group][metadata][read][rest]") { // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); - std::string group1_uri = temp_dir + "group1"; tiledb::Group::create(ctx_, group1_uri); // Open group in write mode tiledb::Group group(ctx_, std::string(group1_uri), TILEDB_WRITE); @@ -323,58 +279,48 @@ TEST_CASE_METHOD( // Close group group.close(); - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( - GroupCPPFx, "C++ API: Group, set name", "[cppapi][group][read]") { - // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); + GroupCPPFx, "C++ API: Group, set name", "[cppapi][group][read][rest]") { + std::string array1_uri = vfs_test_setup_.array_uri("array1"); + std::string array2_uri = vfs_test_setup_.array_uri("array2"); + std::string array3_uri = vfs_test_setup_.array_uri("array3"); - const tiledb::sm::URI array1_uri(temp_dir + "array1"); - const tiledb::sm::URI array2_uri(temp_dir + "array2"); - const tiledb::sm::URI array3_uri(temp_dir + "array3"); - create_array(array1_uri.to_string()); - create_array(array2_uri.to_string()); - create_array(array3_uri.to_string()); + create_array(array1_uri); + create_array(array2_uri); + create_array(array3_uri); - tiledb::sm::URI group1_uri(temp_dir + "group1"); - tiledb::Group::create(ctx_, group1_uri.to_string()); - - tiledb::sm::URI group2_uri(temp_dir + "group2"); - tiledb::Group::create(ctx_, group2_uri.to_string()); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); + std::string group2_uri = vfs_test_setup_.array_uri("group2"); + tiledb::Group::create(ctx_, group1_uri); + tiledb::Group::create(ctx_, group2_uri); // Set expected std::vector group1_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array1_uri.to_string(), "array1"), - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), "array2"), - tiledb::Object( - tiledb::Object::Type::Group, group2_uri.to_string(), "group2"), + tiledb::Object(tiledb::Object::Type::Array, array1_uri, "array1"), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, "array2"), + tiledb::Object(tiledb::Object::Type::Group, group2_uri, "group2"), }; std::vector group2_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array3_uri.to_string(), "array3"), + tiledb::Object(tiledb::Object::Type::Array, array3_uri, "array3"), }; - tiledb::Group group1(ctx_, group1_uri.to_string(), TILEDB_WRITE); + tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - tiledb::Group group2(ctx_, group2_uri.to_string(), TILEDB_WRITE); + tiledb::Group group2(ctx_, group2_uri, TILEDB_WRITE); group2.close(); set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_uri.to_string(), false, "array1"); - group1.add_member(array2_uri.to_string(), false, "array2"); - group1.add_member(group2_uri.to_string(), false, "group2"); + group1.add_member(array1_uri, false, "array1"); + group1.add_member(array2_uri, false, "array2"); + group1.add_member(group2_uri, false, "group2"); - group2.add_member(array3_uri.to_string(), false, "array3"); + group2.add_member(array3_uri, false, "array3"); // Close group from write mode group1.close(); @@ -441,58 +387,49 @@ TEST_CASE_METHOD( // Close group group1.close(); group2.close(); - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( - GroupCPPFx, "C++ API: Group, write/read", "[cppapi][group][read]") { + GroupCPPFx, "C++ API: Group, write/read", "[cppapi][group][read][rest]") { // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); + std::string array1_uri = vfs_test_setup_.array_uri("array1"); + std::string array2_uri = vfs_test_setup_.array_uri("array2"); + std::string array3_uri = vfs_test_setup_.array_uri("array3"); - const tiledb::sm::URI array1_uri(temp_dir + "array1"); - const tiledb::sm::URI array2_uri(temp_dir + "array2"); - const tiledb::sm::URI array3_uri(temp_dir + "array3"); - create_array(array1_uri.to_string()); - create_array(array2_uri.to_string()); - create_array(array3_uri.to_string()); + create_array(array1_uri); + create_array(array2_uri); + create_array(array3_uri); - tiledb::sm::URI group1_uri(temp_dir + "group1"); - tiledb::Group::create(ctx_, group1_uri.to_string()); - - tiledb::sm::URI group2_uri(temp_dir + "group2"); - tiledb::Group::create(ctx_, group2_uri.to_string()); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); + std::string group2_uri = vfs_test_setup_.array_uri("group2"); + tiledb::Group::create(ctx_, group1_uri); + tiledb::Group::create(ctx_, group2_uri); // Set expected std::vector group1_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array1_uri.to_string(), std::nullopt), - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), std::nullopt), - tiledb::Object( - tiledb::Object::Type::Group, group2_uri.to_string(), std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array1_uri, std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, std::nullopt), + tiledb::Object(tiledb::Object::Type::Group, group2_uri, std::nullopt), }; std::vector group2_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array3_uri.to_string(), std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array3_uri, std::nullopt), }; - tiledb::Group group1(ctx_, group1_uri.to_string(), TILEDB_WRITE); + tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - tiledb::Group group2(ctx_, group2_uri.to_string(), TILEDB_WRITE); + tiledb::Group group2(ctx_, group2_uri, TILEDB_WRITE); group2.close(); set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_uri.to_string(), false); - group1.add_member(array2_uri.to_string(), false); - group1.add_member(group2_uri.to_string(), false); + group1.add_member(array1_uri, false); + group1.add_member(array2_uri, false); + group1.add_member(group2_uri, false); - group2.add_member(array3_uri.to_string(), false); + group2.add_member(array3_uri, false); // Close group from write mode group1.close(); @@ -520,11 +457,11 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 2); group2.open(TILEDB_WRITE); - group1.remove_member(group2_uri.to_string()); + group1.remove_member(group2_uri); // Group is the latest element group1_expected.resize(group1_expected.size() - 1); - group2.remove_member(array3_uri.to_string()); + group2.remove_member(array3_uri); // There should be nothing left in group2 group2_expected.clear(); @@ -554,70 +491,63 @@ TEST_CASE_METHOD( // Close group group1.close(); group2.close(); - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( GroupCPPFx, "C++ API: Group, write/read, relative", - "[cppapi][group][read]") { + "[cppapi][group][read][non-rest]") { // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); - - tiledb::sm::URI group1_uri(temp_dir + "group1"); - tiledb::Group::create(ctx_, group1_uri.to_string()); - - tiledb::sm::URI group2_uri(temp_dir + "group2"); - tiledb::Group::create(ctx_, group2_uri.to_string()); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); + std::string group2_uri = vfs_test_setup_.array_uri("group2"); + tiledb::Group::create(ctx_, group1_uri); + tiledb::Group::create(ctx_, group2_uri); REQUIRE( tiledb_vfs_create_dir( - ctx_.ptr().get(), vfs_, (temp_dir + "group1/arrays").c_str()) == - TILEDB_OK); + ctx_.ptr().get(), + vfs_test_setup_.vfs_c, + (group1_uri + "/arrays").c_str()) == TILEDB_OK); REQUIRE( tiledb_vfs_create_dir( - ctx_.ptr().get(), vfs_, (temp_dir + "group2/arrays").c_str()) == - TILEDB_OK); + ctx_.ptr().get(), + vfs_test_setup_.vfs_c, + (group2_uri + "/arrays").c_str()) == TILEDB_OK); const std::string array1_relative_uri("arrays/array1"); - const tiledb::sm::URI array1_uri(temp_dir + "group1/arrays/array1"); + std::string array1_uri = vfs_test_setup_.array_uri("group1/arrays/array1"); const std::string array2_relative_uri("arrays/array2"); - const tiledb::sm::URI array2_uri(temp_dir + "group1/arrays/array2"); + std::string array2_uri = vfs_test_setup_.array_uri("group1/arrays/array2"); const std::string array3_relative_uri("arrays/array3"); - const tiledb::sm::URI array3_uri(temp_dir + "group2/arrays/array3"); - create_array(array1_uri.to_string()); - create_array(array2_uri.to_string()); - create_array(array3_uri.to_string()); + std::string array3_uri = vfs_test_setup_.array_uri("group2/arrays/array3"); + + create_array(array1_uri); + create_array(array2_uri); + create_array(array3_uri); // Set expected std::vector group1_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array1_uri.to_string(), std::nullopt), - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), std::nullopt), - tiledb::Object( - tiledb::Object::Type::Group, group2_uri.to_string(), std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array1_uri, std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, std::nullopt), + tiledb::Object(tiledb::Object::Type::Group, group2_uri, std::nullopt), }; std::vector group2_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array3_uri.to_string(), std::nullopt), + tiledb::Object(tiledb::Object::Type::Array, array3_uri, std::nullopt), }; - tiledb::Group group1(ctx_, group1_uri.to_string(), TILEDB_WRITE); + tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - tiledb::Group group2(ctx_, group2_uri.to_string(), TILEDB_WRITE); + tiledb::Group group2(ctx_, group2_uri, TILEDB_WRITE); group2.close(); set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); group1.add_member(array1_relative_uri, true); group1.add_member(array2_relative_uri, true); - group1.add_member(group2_uri.to_string(), false); + group1.add_member(group2_uri, false); group2.add_member(array3_relative_uri, true); @@ -647,7 +577,7 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 2); group2.open(TILEDB_WRITE); - group1.remove_member(group2_uri.to_string()); + group1.remove_member(group2_uri); // Group is the latest element group1_expected.resize(group1_expected.size() - 1); @@ -676,72 +606,65 @@ TEST_CASE_METHOD( // Close group group1.close(); group2.close(); - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( GroupCPPFx, "C++ API: Group, write/read, relative named", - "[cppapi][group][read]") { + "[cppapi][group][read][non-rest]") { bool remove_by_name = GENERATE(true, false); // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); - - tiledb::sm::URI group1_uri(temp_dir + "group1"); - tiledb::Group::create(ctx_, group1_uri.to_string()); - - tiledb::sm::URI group2_uri(temp_dir + "group2"); - tiledb::Group::create(ctx_, group2_uri.to_string()); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); + std::string group2_uri = vfs_test_setup_.array_uri("group2"); + tiledb::Group::create(ctx_, group1_uri); + tiledb::Group::create(ctx_, group2_uri); REQUIRE( tiledb_vfs_create_dir( - ctx_.ptr().get(), vfs_, (temp_dir + "group1/arrays").c_str()) == - TILEDB_OK); + ctx_.ptr().get(), + vfs_test_setup_.vfs_c, + (group1_uri + "/arrays").c_str()) == TILEDB_OK); REQUIRE( tiledb_vfs_create_dir( - ctx_.ptr().get(), vfs_, (temp_dir + "group2/arrays").c_str()) == - TILEDB_OK); + ctx_.ptr().get(), + vfs_test_setup_.vfs_c, + (group2_uri + "/arrays").c_str()) == TILEDB_OK); const std::string array1_relative_uri("arrays/array1"); - const tiledb::sm::URI array1_uri(temp_dir + "group1/arrays/array1"); + std::string array1_uri = vfs_test_setup_.array_uri("group1/arrays/array1"); const std::string array2_relative_uri("arrays/array2"); - const tiledb::sm::URI array2_uri(temp_dir + "group1/arrays/array2"); + std::string array2_uri = vfs_test_setup_.array_uri("group1/arrays/array2"); const std::string array3_relative_uri("arrays/array3"); - const tiledb::sm::URI array3_uri(temp_dir + "group2/arrays/array3"); - create_array(array1_uri.to_string()); - create_array(array2_uri.to_string()); - create_array(array3_uri.to_string()); + std::string array3_uri = vfs_test_setup_.array_uri("group2/arrays/array3"); + + create_array(array1_uri); + create_array(array2_uri); + create_array(array3_uri); // Set expected std::vector group1_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array1_uri.to_string(), "one"), - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), "two"), - tiledb::Object( - tiledb::Object::Type::Group, group2_uri.to_string(), "three"), + tiledb::Object(tiledb::Object::Type::Array, array1_uri, "one"), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, "two"), + tiledb::Object(tiledb::Object::Type::Group, group2_uri, "three"), }; std::vector group2_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array3_uri.to_string(), "four"), + tiledb::Object(tiledb::Object::Type::Array, array3_uri, "four"), }; - tiledb::Group group1(ctx_, group1_uri.to_string(), TILEDB_WRITE); + tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - tiledb::Group group2(ctx_, group2_uri.to_string(), TILEDB_WRITE); + tiledb::Group group2(ctx_, group2_uri, TILEDB_WRITE); group2.close(); set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); group1.add_member(array1_relative_uri, true, "one"); group1.add_member(array2_relative_uri, true, "two"); - group1.add_member(group2_uri.to_string(), false, "three"); + group1.add_member(group2_uri, false, "three"); group2.add_member(array3_relative_uri, true, "four"); @@ -786,7 +709,7 @@ TEST_CASE_METHOD( if (remove_by_name) { group1.remove_member("three"); } else { - group1.remove_member(group2_uri.to_string()); + group1.remove_member(group2_uri); } // Group is the latest element @@ -822,49 +745,42 @@ TEST_CASE_METHOD( // Close group group1.close(); group2.close(); - remove_temp_dir(temp_dir); } TEST_CASE_METHOD( GroupCPPFx, "C++ API: Group, delete by URI, duplicates", - "[cppapi][group][delete]") { + "[cppapi][group][delete][non-rest]") { bool nameless_uri = GENERATE(true, false); // Create and open group in write mode - // TODO: refactor for each supported FS. - std::string temp_dir = fs_vec_[0]->temp_dir(); - create_temp_dir(temp_dir); - - tiledb::sm::URI group1_uri(temp_dir + "group1"); - tiledb::Group::create(ctx_, group1_uri.to_string()); + std::string group1_uri = vfs_test_setup_.array_uri("group1"); + tiledb::Group::create(ctx_, group1_uri); REQUIRE( tiledb_vfs_create_dir( - ctx_.ptr().get(), vfs_, (temp_dir + "group1/arrays").c_str()) == - TILEDB_OK); + ctx_.ptr().get(), + vfs_test_setup_.vfs_c, + (group1_uri + "/arrays").c_str()) == TILEDB_OK); const std::string array1_relative_uri("arrays/array1"); - const tiledb::sm::URI array1_uri(temp_dir + "group1/arrays/array1"); + std::string array1_uri = vfs_test_setup_.array_uri("group1/arrays/array1"); const std::string array2_relative_uri("arrays/array2"); - const tiledb::sm::URI array2_uri(temp_dir + "group1/arrays/array2"); - create_array(array1_uri.to_string()); - create_array(array2_uri.to_string()); + std::string array2_uri = vfs_test_setup_.array_uri("group1/arrays/array2"); + + create_array(array1_uri); + create_array(array2_uri); // Set expected std::vector group1_expected = { - tiledb::Object( - tiledb::Object::Type::Array, array1_uri.to_string(), "one"), - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), "two"), + tiledb::Object(tiledb::Object::Type::Array, array1_uri, "one"), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, "two"), nameless_uri ? - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), nullopt) : - tiledb::Object( - tiledb::Object::Type::Array, array2_uri.to_string(), "three"), + tiledb::Object(tiledb::Object::Type::Array, array2_uri, nullopt) : + tiledb::Object(tiledb::Object::Type::Array, array2_uri, "three"), }; - tiledb::Group group1(ctx_, group1_uri.to_string(), TILEDB_WRITE); + tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); @@ -930,7 +846,6 @@ TEST_CASE_METHOD( // Close group group1.close(); - remove_temp_dir(temp_dir); } /** Test Exception For Assertability */ diff --git a/tiledb/api/c_api/group/group_api_external.h b/tiledb/api/c_api/group/group_api_external.h index 045f9520f9c..0fe844fe5b7 100644 --- a/tiledb/api/c_api/group/group_api_external.h +++ b/tiledb/api/c_api/group/group_api_external.h @@ -401,8 +401,8 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( */ TILEDB_EXPORT capi_return_t tiledb_group_add_member_by_type( tiledb_ctx_t* ctx, - tiledb_group_handle_t* group, - const char* group_uri, + tiledb_group_t* group, + const char* uri, const uint8_t relative, const char* name, tiledb_object_t type) TILEDB_NOEXCEPT; From 2ee3ec991b1ca8f88c11ca5afb3a5ca97a92f087 Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Mon, 7 Oct 2024 17:04:07 +0300 Subject: [PATCH 4/9] Add cpp API --- test/src/unit-cppapi-group.cc | 91 +++++++++++++++------ tiledb/api/c_api/group/group_api_external.h | 2 +- tiledb/sm/cpp_api/group.h | 15 +++- 3 files changed, 78 insertions(+), 30 deletions(-) diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 8d3d9c3bc08..6c3bfdede40 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -316,11 +316,18 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_uri, false, "array1"); - group1.add_member(array2_uri, false, "array2"); - group1.add_member(group2_uri, false, "group2"); - - group2.add_member(array3_uri, false, "array3"); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + group1.add_member(array1_uri, false, "array1", TILEDB_ARRAY); + group1.add_member(array2_uri, false, "array2", TILEDB_ARRAY); + group1.add_member(group2_uri, false, "group2", TILEDB_GROUP); + group2.add_member(array3_uri, false, "array3", TILEDB_ARRAY); + } else { + group1.add_member(array1_uri, false, "array1"); + group1.add_member(array2_uri, false, "array2"); + group1.add_member(group2_uri, false, "group2"); + group2.add_member(array3_uri, false, "array3"); + } // Close group from write mode group1.close(); @@ -425,11 +432,18 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_uri, false); - group1.add_member(array2_uri, false); - group1.add_member(group2_uri, false); - - group2.add_member(array3_uri, false); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + group1.add_member(array1_uri, false, std::nullopt, TILEDB_ARRAY); + group1.add_member(array2_uri, false, std::nullopt, TILEDB_ARRAY); + group1.add_member(group2_uri, false, std::nullopt, TILEDB_GROUP); + group2.add_member(array3_uri, false, std::nullopt, TILEDB_ARRAY); + } else { + group1.add_member(array1_uri, false); + group1.add_member(array2_uri, false); + group1.add_member(group2_uri, false); + group2.add_member(array3_uri, false); + } // Close group from write mode group1.close(); @@ -545,11 +559,18 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_relative_uri, true); - group1.add_member(array2_relative_uri, true); - group1.add_member(group2_uri, false); - - group2.add_member(array3_relative_uri, true); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + group1.add_member(array1_relative_uri, true, std::nullopt, TILEDB_ARRAY); + group1.add_member(array2_relative_uri, true, std::nullopt, TILEDB_ARRAY); + group1.add_member(group2_uri, false, std::nullopt, TILEDB_GROUP); + group2.add_member(array3_relative_uri, true, std::nullopt, TILEDB_ARRAY); + } else { + group1.add_member(array1_relative_uri, true); + group1.add_member(array2_relative_uri, true); + group1.add_member(group2_uri, false); + group2.add_member(array3_relative_uri, true); + } // Close group from write mode group1.close(); @@ -662,11 +683,18 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - group1.add_member(array1_relative_uri, true, "one"); - group1.add_member(array2_relative_uri, true, "two"); - group1.add_member(group2_uri, false, "three"); - - group2.add_member(array3_relative_uri, true, "four"); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + group1.add_member(array1_relative_uri, true, "one", TILEDB_ARRAY); + group1.add_member(array2_relative_uri, true, "two", TILEDB_ARRAY); + group1.add_member(group2_uri, false, "three", TILEDB_GROUP); + group2.add_member(array3_relative_uri, true, "four", TILEDB_ARRAY); + } else { + group1.add_member(array1_relative_uri, true, "one"); + group1.add_member(array2_relative_uri, true, "two"); + group1.add_member(group2_uri, false, "three"); + group2.add_member(array3_relative_uri, true, "four"); + } // Close group from write mode group1.close(); @@ -785,12 +813,23 @@ TEST_CASE_METHOD( set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - group1.add_member(array1_relative_uri, true, "one"); - group1.add_member(array2_relative_uri, true, "two"); - group1.add_member( - array2_relative_uri, - true, - nameless_uri ? nullopt : std::make_optional("three")); + bool add_by_type = GENERATE(true, false); + if (add_by_type) { + group1.add_member(array1_relative_uri, true, "one", TILEDB_ARRAY); + group1.add_member(array2_relative_uri, true, "two", TILEDB_ARRAY); + group1.add_member( + array2_relative_uri, + true, + nameless_uri ? nullopt : std::make_optional("three"), + TILEDB_ARRAY); + } else { + group1.add_member(array1_relative_uri, true, "one"); + group1.add_member(array2_relative_uri, true, "two"); + group1.add_member( + array2_relative_uri, + true, + nameless_uri ? nullopt : std::make_optional("three")); + } // Close group from write mode group1.close(); diff --git a/tiledb/api/c_api/group/group_api_external.h b/tiledb/api/c_api/group/group_api_external.h index 0fe844fe5b7..99faac2f862 100644 --- a/tiledb/api/c_api/group/group_api_external.h +++ b/tiledb/api/c_api/group/group_api_external.h @@ -396,7 +396,7 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( * @param relative is the URI relative to the group * @param name optional name group member can be given to be looked up by. * Can be set to NULL. - * @param type the type of the member getting added if known in advance. + * @param type the type of the member getting added if known in advance. * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */ TILEDB_EXPORT capi_return_t tiledb_group_add_member_by_type( diff --git a/tiledb/sm/cpp_api/group.h b/tiledb/sm/cpp_api/group.h index 0d1e544e402..bcd3ba1d76e 100644 --- a/tiledb/sm/cpp_api/group.h +++ b/tiledb/sm/cpp_api/group.h @@ -383,19 +383,28 @@ class Group { * * @param uri of member to add * @param relative is the URI relative to the group location + * @param name optional name group member can be given to be looked up by + * @param type the type of the member getting added if known in advance */ void add_member( const std::string& uri, const bool& relative, - std::optional name = std::nullopt) { + std::optional name = std::nullopt, + std::optional type = std::nullopt) { auto& ctx = ctx_.get(); tiledb_ctx_t* c_ctx = ctx.ptr().get(); const char* name_cstr = nullptr; if (name.has_value()) { name_cstr = name->c_str(); } - ctx.handle_error(tiledb_group_add_member( - c_ctx, group_.get(), uri.c_str(), relative, name_cstr)); + + if (type.has_value()) { + ctx.handle_error(tiledb_group_add_member_by_type( + c_ctx, group_.get(), uri.c_str(), relative, name_cstr, type.value())); + } else { + ctx.handle_error(tiledb_group_add_member( + c_ctx, group_.get(), uri.c_str(), relative, name_cstr)); + } } /** From 02e3d3fbe04195cb7404f8ef574e28bbd68000bd Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Tue, 8 Oct 2024 16:00:06 +0300 Subject: [PATCH 5/9] Address review comments --- test/src/unit-capi-group.cc | 28 ++++++++++----------- tiledb/api/c_api/group/group_api.cc | 13 +++++++--- tiledb/api/c_api/group/group_api_external.h | 16 +++++++----- tiledb/sm/cpp_api/group.h | 2 +- 4 files changed, 35 insertions(+), 24 deletions(-) diff --git a/test/src/unit-capi-group.cc b/test/src/unit-capi-group.cc index 2aa3f164cf1..074c159f198 100644 --- a/test/src/unit-capi-group.cc +++ b/test/src/unit-capi-group.cc @@ -594,16 +594,16 @@ TEST_CASE_METHOD( bool add_by_type = GENERATE(true, false); if (add_by_type) { - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array2_uri.c_str(), false, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group2, array3_uri.c_str(), false, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, group2_uri.c_str(), false, nullptr, TILEDB_GROUP); REQUIRE(rc == TILEDB_OK); } else { @@ -738,7 +738,7 @@ TEST_CASE_METHOD( bool add_by_type = GENERATE(true, false); if (add_by_type) { - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); } else { @@ -780,7 +780,7 @@ TEST_CASE_METHOD( REQUIRE(rc == TILEDB_OK); // Add one back with different URI if (add_by_type) { - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array2_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); } else { @@ -860,16 +860,16 @@ TEST_CASE_METHOD( bool add_by_type = GENERATE(true, false); if (add_by_type) { - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array2_uri.c_str(), false, "two", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group2, array3_uri.c_str(), false, "three", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, group2_uri.c_str(), false, "four", TILEDB_GROUP); REQUIRE(rc == TILEDB_OK); } else { @@ -1238,16 +1238,16 @@ TEST_CASE_METHOD( bool add_by_type = GENERATE(true, false); if (add_by_type) { - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array1_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, array2_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group2, array3_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); - rc = tiledb_group_add_member_by_type( + rc = tiledb_group_add_member_with_type( ctx_, group1, group2_uri.c_str(), false, nullptr, TILEDB_GROUP); REQUIRE(rc == TILEDB_OK); } else { diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index c2359f73848..b8b55b67bd8 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -66,6 +66,12 @@ inline void ensure_name_argument_is_valid(const char* name) { } } +inline void ensure_object_type_argument_is_valid(const tiledb_object_t type) { + if (type != TILEDB_GROUP && type != TILEDB_ARRAY) { + throw CAPIStatusException("Invalid object `type`"); + } +} + inline char* copy_string(const std::string& str) { auto ret = static_cast(std::malloc(str.size() + 1)); if (ret == nullptr) { @@ -276,7 +282,7 @@ capi_return_t tiledb_group_add_member( return TILEDB_OK; } -capi_return_t tiledb_group_add_member_by_type( +capi_return_t tiledb_group_add_member_with_type( tiledb_group_handle_t* group, const char* group_uri, const uint8_t relative, @@ -284,6 +290,7 @@ capi_return_t tiledb_group_add_member_by_type( tiledb_object_t type) { ensure_group_is_valid(group); ensure_group_uri_argument_is_valid(group_uri); + ensure_object_type_argument_is_valid(type); auto uri = tiledb::sm::URI(group_uri, !relative); @@ -683,14 +690,14 @@ CAPI_INTERFACE( } CAPI_INTERFACE( - group_add_member_by_type, + group_add_member_with_type, tiledb_ctx_t* ctx, tiledb_group_t* group, const char* uri, const uint8_t relative, const char* name, tiledb_object_t type) { - return api_entry_context( + return api_entry_context( ctx, group, uri, relative, name, type); } diff --git a/tiledb/api/c_api/group/group_api_external.h b/tiledb/api/c_api/group/group_api_external.h index 99faac2f862..9bd03659fff 100644 --- a/tiledb/api/c_api/group/group_api_external.h +++ b/tiledb/api/c_api/group/group_api_external.h @@ -375,9 +375,10 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( /** * Add a member with a known type to a group - * The caller should make sure the correct type is provided for the member being - * added as this API will not check it for correctness in favor of efficiency - * and results can be undefined otherwise. + * The caller should make sure that the member exists and the correct type is + * provided for the member being added as this API will not check it for + * existence or correctness in favor of efficiency and results can be undefined + * otherwise. * * **Example:** * @@ -385,9 +386,12 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( * tiledb_group_t* group; * tiledb_group_alloc(ctx, "s3://tiledb_bucket/my_group", &group); * tiledb_group_open(ctx, group, TILEDB_WRITE); - * tiledb_group_add_member(ctx, group, "s3://tiledb_bucket/my_array", - * TILEDB_ARRAY); tiledb_group_add_member(ctx, group, + * tiledb_group_add_member_with_type(ctx, group, "s3://tiledb_bucket/my_array", + * TILEDB_ARRAY); + * tiledb_group_add_member_with_type(ctx, group, * "s3://tiledb_bucket/my_group_2", TILEDB_GROUP); + * + * tiledb_group_close(ctx, group); * @endcode * * @param ctx The TileDB context. @@ -399,7 +403,7 @@ TILEDB_EXPORT capi_return_t tiledb_group_add_member( * @param type the type of the member getting added if known in advance. * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */ -TILEDB_EXPORT capi_return_t tiledb_group_add_member_by_type( +TILEDB_EXPORT capi_return_t tiledb_group_add_member_with_type( tiledb_ctx_t* ctx, tiledb_group_t* group, const char* uri, diff --git a/tiledb/sm/cpp_api/group.h b/tiledb/sm/cpp_api/group.h index bcd3ba1d76e..fc8021f45d7 100644 --- a/tiledb/sm/cpp_api/group.h +++ b/tiledb/sm/cpp_api/group.h @@ -399,7 +399,7 @@ class Group { } if (type.has_value()) { - ctx.handle_error(tiledb_group_add_member_by_type( + ctx.handle_error(tiledb_group_add_member_with_type( c_ctx, group_.get(), uri.c_str(), relative, name_cstr, type.value())); } else { ctx.handle_error(tiledb_group_add_member( From 1e7e70e2e5f232882159353f088efedf5331b36f Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Wed, 9 Oct 2024 13:13:15 +0300 Subject: [PATCH 6/9] Fix group cpp tests --- test/src/unit-capi-group.cc | 18 ++--- test/src/unit-cppapi-group.cc | 122 ++++++++++++++++++++++++--------- test/support/src/vfs_helpers.h | 62 +++++++++++++++++ 3 files changed, 162 insertions(+), 40 deletions(-) diff --git a/test/src/unit-capi-group.cc b/test/src/unit-capi-group.cc index 074c159f198..920362990ef 100644 --- a/test/src/unit-capi-group.cc +++ b/test/src/unit-capi-group.cc @@ -592,8 +592,8 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); @@ -736,8 +736,8 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group1, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); @@ -779,7 +779,7 @@ TEST_CASE_METHOD( rc = tiledb_group_remove_member(ctx_, group1, "one"); REQUIRE(rc == TILEDB_OK); // Add one back with different URI - if (add_by_type) { + if (add_with_type) { rc = tiledb_group_add_member_with_type( ctx_, group1, array2_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); @@ -858,8 +858,8 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { rc = tiledb_group_add_member_with_type( ctx_, group1, array1_uri.c_str(), false, "one", TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); @@ -1236,8 +1236,8 @@ TEST_CASE_METHOD( rc = tiledb_group_open(ctx_, group2, TILEDB_WRITE); REQUIRE(rc == TILEDB_OK); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { rc = tiledb_group_add_member_with_type( ctx_, group1, array1_relative_uri.c_str(), true, nullptr, TILEDB_ARRAY); REQUIRE(rc == TILEDB_OK); diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 6c3bfdede40..1ac33db656f 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -298,13 +298,26 @@ TEST_CASE_METHOD( // Set expected std::vector group1_expected = { - tiledb::Object(tiledb::Object::Type::Array, array1_uri, "array1"), - tiledb::Object(tiledb::Object::Type::Array, array2_uri, "array2"), - tiledb::Object(tiledb::Object::Type::Group, group2_uri, "group2"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array1_uri), + "array1"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + "array2"), + tiledb::Object( + tiledb::Object::Type::Group, + vfs_test_setup_.with_prefix(group2_uri), + "group2"), }; std::vector group2_expected = { - tiledb::Object(tiledb::Object::Type::Array, array3_uri, "array3"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array3_uri), + "array3"), }; + std::cerr << group1_expected[0].uri() << std::endl; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); @@ -316,8 +329,8 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { group1.add_member(array1_uri, false, "array1", TILEDB_ARRAY); group1.add_member(array2_uri, false, "array2", TILEDB_ARRAY); group1.add_member(group2_uri, false, "group2", TILEDB_GROUP); @@ -414,12 +427,24 @@ TEST_CASE_METHOD( // Set expected std::vector group1_expected = { - tiledb::Object(tiledb::Object::Type::Array, array1_uri, std::nullopt), - tiledb::Object(tiledb::Object::Type::Array, array2_uri, std::nullopt), - tiledb::Object(tiledb::Object::Type::Group, group2_uri, std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array1_uri), + std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + std::nullopt), + tiledb::Object( + tiledb::Object::Type::Group, + vfs_test_setup_.with_prefix(group2_uri), + std::nullopt), }; std::vector group2_expected = { - tiledb::Object(tiledb::Object::Type::Array, array3_uri, std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array3_uri), + std::nullopt), }; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); @@ -432,8 +457,8 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { group1.add_member(array1_uri, false, std::nullopt, TILEDB_ARRAY); group1.add_member(array2_uri, false, std::nullopt, TILEDB_ARRAY); group1.add_member(group2_uri, false, std::nullopt, TILEDB_GROUP); @@ -541,12 +566,24 @@ TEST_CASE_METHOD( // Set expected std::vector group1_expected = { - tiledb::Object(tiledb::Object::Type::Array, array1_uri, std::nullopt), - tiledb::Object(tiledb::Object::Type::Array, array2_uri, std::nullopt), - tiledb::Object(tiledb::Object::Type::Group, group2_uri, std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array1_uri), + std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + std::nullopt), + tiledb::Object( + tiledb::Object::Type::Group, + vfs_test_setup_.with_prefix(group2_uri), + std::nullopt), }; std::vector group2_expected = { - tiledb::Object(tiledb::Object::Type::Array, array3_uri, std::nullopt), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array3_uri), + std::nullopt), }; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); @@ -559,8 +596,8 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { group1.add_member(array1_relative_uri, true, std::nullopt, TILEDB_ARRAY); group1.add_member(array2_relative_uri, true, std::nullopt, TILEDB_ARRAY); group1.add_member(group2_uri, false, std::nullopt, TILEDB_GROUP); @@ -665,12 +702,24 @@ TEST_CASE_METHOD( // Set expected std::vector group1_expected = { - tiledb::Object(tiledb::Object::Type::Array, array1_uri, "one"), - tiledb::Object(tiledb::Object::Type::Array, array2_uri, "two"), - tiledb::Object(tiledb::Object::Type::Group, group2_uri, "three"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array1_uri), + "one"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + "two"), + tiledb::Object( + tiledb::Object::Type::Group, + vfs_test_setup_.with_prefix(group2_uri), + "three"), }; std::vector group2_expected = { - tiledb::Object(tiledb::Object::Type::Array, array3_uri, "four"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array3_uri), + "four"), }; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); @@ -683,8 +732,8 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_WRITE); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { group1.add_member(array1_relative_uri, true, "one", TILEDB_ARRAY); group1.add_member(array2_relative_uri, true, "two", TILEDB_ARRAY); group1.add_member(group2_uri, false, "three", TILEDB_GROUP); @@ -801,11 +850,22 @@ TEST_CASE_METHOD( // Set expected std::vector group1_expected = { - tiledb::Object(tiledb::Object::Type::Array, array1_uri, "one"), - tiledb::Object(tiledb::Object::Type::Array, array2_uri, "two"), - nameless_uri ? - tiledb::Object(tiledb::Object::Type::Array, array2_uri, nullopt) : - tiledb::Object(tiledb::Object::Type::Array, array2_uri, "three"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array1_uri), + "one"), + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + "two"), + nameless_uri ? tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + nullopt) : + tiledb::Object( + tiledb::Object::Type::Array, + vfs_test_setup_.with_prefix(array2_uri), + "three"), }; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); @@ -813,8 +873,8 @@ TEST_CASE_METHOD( set_group_timestamp(&group1, 1); group1.open(TILEDB_WRITE); - bool add_by_type = GENERATE(true, false); - if (add_by_type) { + bool add_with_type = GENERATE(true, false); + if (add_with_type) { group1.add_member(array1_relative_uri, true, "one", TILEDB_ARRAY); group1.add_member(array2_relative_uri, true, "two", TILEDB_ARRAY); group1.add_member( diff --git a/test/support/src/vfs_helpers.h b/test/support/src/vfs_helpers.h index 9230dade980..8d45ebe9cbb 100644 --- a/test/support/src/vfs_helpers.h +++ b/test/support/src/vfs_helpers.h @@ -166,6 +166,14 @@ class SupportedFs { * @return true if local, false if not */ virtual bool is_local() = 0; + + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + virtual std::string file_prefix() = 0; }; /** @@ -238,6 +246,16 @@ class SupportedFsS3 : public SupportedFs { return false; } + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + inline std::string file_prefix() { + return ""; + } + private: /* ********************************* */ /* ATTRIBUTES */ @@ -324,6 +342,16 @@ class SupportedFsHDFS : public SupportedFs { return false; } + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + inline std::string file_prefix() { + return ""; + } + private: /* ********************************* */ /* ATTRIBUTES */ @@ -404,6 +432,16 @@ class SupportedFsAzure : public SupportedFs { return false; } + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + inline std::string file_prefix() { + return ""; + } + private: /* ********************************* */ /* ATTRIBUTES */ @@ -489,6 +527,16 @@ class SupportedFsGCS : public SupportedFs { return false; } + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + inline std::string file_prefix() { + return ""; + } + private: /* ********************************* */ /* ATTRIBUTES */ @@ -677,6 +725,16 @@ class SupportedFsMem : public SupportedFs { return true; } + /** + * Get the name of the filesystem's file prefix. Applies only to local + * filesystems + * + * @return string prefix name + */ + inline std::string file_prefix() { + return ""; + } + private: /* ********************************* */ /* ATTRIBUTES */ @@ -934,6 +992,10 @@ struct VFSTestSetup { return uri; } + std::string with_prefix(const std::string& array_name) { + return fs_vec[0]->file_prefix() + array_name; + } + Context ctx() { return Context(ctx_c, false); } From 53e5d390e04d728e5ebacb1b962dcbf1546c2511 Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Wed, 16 Oct 2024 15:17:05 +0300 Subject: [PATCH 7/9] Fix failing tests and file bug --- test/src/unit-cppapi-group.cc | 126 ++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 30 deletions(-) diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 1ac33db656f..33eccbbc203 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -65,6 +65,10 @@ struct GroupCPPFx { GroupCPPFx(); void create_array(const std::string& path) const; std::vector read_group(const tiledb::Group& group) const; + std::vector>> + read_group_details(const tiledb::Group& group) const; + // compare type and name of objects, but not the URIs that do not match on + // remote arrays void set_group_timestamp( tiledb::Group* group, const uint64_t& timestamp) const; }; @@ -92,6 +96,17 @@ std::vector GroupCPPFx::read_group( return ret; } +std::vector>> +GroupCPPFx::read_group_details(const tiledb::Group& group) const { + std::vector>> ret; + uint64_t count = group.member_count(); + for (uint64_t i = 0; i < count; i++) { + tiledb::Object obj = group.member(i); + ret.emplace_back(obj.type(), obj.name()); + } + return ret; +} + void GroupCPPFx::create_array(const std::string& path) const { tiledb_attribute_t* a1; tiledb_attribute_alloc(ctx_c_, "a1", TILEDB_FLOAT32, &a1); @@ -311,13 +326,19 @@ TEST_CASE_METHOD( vfs_test_setup_.with_prefix(group2_uri), "group2"), }; + std::vector>> + group1_exp_det = { + {tiledb::Object::Type::Array, "array1"}, + {tiledb::Object::Type::Array, "array2"}, + {tiledb::Object::Type::Group, "group2"}}; std::vector group2_expected = { tiledb::Object( tiledb::Object::Type::Array, vfs_test_setup_.with_prefix(array3_uri), "array3"), }; - std::cerr << group1_expected[0].uri() << std::endl; + std::vector>> + group2_exp_det = {{tiledb::Object::Type::Array, "array3"}}; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); @@ -352,13 +373,23 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 1); group2.open(TILEDB_READ); - std::vector group1_received = read_group(group1); - REQUIRE_THAT( - group1_received, Catch::Matchers::UnorderedEquals(group1_expected)); - - std::vector group2_received = read_group(group2); - REQUIRE_THAT( - group2_received, Catch::Matchers::UnorderedEquals(group2_expected)); + // group URIs returned on remote array open are actually trasformed by the + // REST server if the form `tiledb://UUID` so they don't match the initial + // `tiledb://{namespace}/fs://temp_dir/name` format, so let's only compare the + // other fields of the objects. + if (vfs_test_setup_.is_rest()) { + REQUIRE_THAT( + read_group_details(group1), + Catch::Matchers::UnorderedEquals(group1_exp_det)); + REQUIRE_THAT( + read_group_details(group2), + Catch::Matchers::UnorderedEquals(group2_exp_det)); + } else { + REQUIRE_THAT( + read_group(group1), Catch::Matchers::UnorderedEquals(group1_expected)); + REQUIRE_THAT( + read_group(group2), Catch::Matchers::UnorderedEquals(group2_expected)); + } // Close group group1.close(); @@ -373,10 +404,12 @@ TEST_CASE_METHOD( group1.remove_member("group2"); // Group is the latest element group1_expected.resize(group1_expected.size() - 1); + group1_exp_det.resize(group1_exp_det.size() - 1); group2.remove_member("array3"); // There should be nothing left in group2 group2_expected.clear(); + group2_exp_det.clear(); // Close group group1.close(); @@ -388,14 +421,18 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 2); group2.open(TILEDB_READ); - group1_received = read_group(group1); - REQUIRE_THAT( - group1_received, Catch::Matchers::UnorderedEquals(group1_expected)); - - const auto& obj = group1.member(group1_expected[0].name().value()); - REQUIRE(obj == group1_expected[0]); + if (vfs_test_setup_.is_rest()) { + REQUIRE_THAT( + read_group_details(group1), + Catch::Matchers::UnorderedEquals(group1_exp_det)); + } else { + REQUIRE_THAT( + read_group(group1), Catch::Matchers::UnorderedEquals(group1_expected)); + const auto& obj = group1.member(group1_expected[0].name().value()); + REQUIRE(obj == group1_expected[0]); + } - group2_received = read_group(group2); + auto group2_received = read_group(group2); REQUIRE_THAT( group2_received, Catch::Matchers::UnorderedEquals(group2_expected)); @@ -410,7 +447,9 @@ TEST_CASE_METHOD( } TEST_CASE_METHOD( - GroupCPPFx, "C++ API: Group, write/read", "[cppapi][group][read][rest]") { + GroupCPPFx, + "C++ API: Group, write/read", + "[cppapi][group][read][rest-fails][sc-57858]") { // Create and open group in write mode std::string array1_uri = vfs_test_setup_.array_uri("array1"); std::string array2_uri = vfs_test_setup_.array_uri("array2"); @@ -440,12 +479,19 @@ TEST_CASE_METHOD( vfs_test_setup_.with_prefix(group2_uri), std::nullopt), }; + std::vector>> + group1_exp_det = { + {tiledb::Object::Type::Array, std::nullopt}, + {tiledb::Object::Type::Array, std::nullopt}, + {tiledb::Object::Type::Group, std::nullopt}}; std::vector group2_expected = { tiledb::Object( tiledb::Object::Type::Array, vfs_test_setup_.with_prefix(array3_uri), std::nullopt), }; + std::vector>> + group2_exp_det = {{tiledb::Object::Type::Array, std::nullopt}}; tiledb::Group group1(ctx_, group1_uri, TILEDB_WRITE); group1.close(); @@ -478,13 +524,25 @@ TEST_CASE_METHOD( group1.open(TILEDB_READ); group2.open(TILEDB_READ); - std::vector group1_received = read_group(group1); - REQUIRE_THAT( - group1_received, Catch::Matchers::UnorderedEquals(group1_expected)); - - std::vector group2_received = read_group(group2); - REQUIRE_THAT( - group2_received, Catch::Matchers::UnorderedEquals(group2_expected)); + // group URIs returned on remote array open are actually trasformed by the + // REST server if the form `tiledb://UUID` so they don't match the initial + // `tiledb://{namespace}/fs://temp_dir/name` format, so let's only compare the + // other fields of the objects. + if (vfs_test_setup_.is_rest()) { + // Those checks fail for REST because of sc-57858: names are not empty as + // they are supposed to but are equal to the REST Uri of the asset + REQUIRE_THAT( + read_group_details(group1), + Catch::Matchers::UnorderedEquals(group1_exp_det)); + REQUIRE_THAT( + read_group_details(group2), + Catch::Matchers::UnorderedEquals(group2_exp_det)); + } else { + REQUIRE_THAT( + read_group(group1), Catch::Matchers::UnorderedEquals(group1_expected)); + REQUIRE_THAT( + read_group(group2), Catch::Matchers::UnorderedEquals(group2_expected)); + } // Close group group1.close(); @@ -499,10 +557,12 @@ TEST_CASE_METHOD( group1.remove_member(group2_uri); // Group is the latest element group1_expected.resize(group1_expected.size() - 1); + group1_exp_det.resize(group1_exp_det.size() - 1); group2.remove_member(array3_uri); // There should be nothing left in group2 group2_expected.clear(); + group2_exp_det.clear(); // Close group group1.close(); @@ -514,13 +574,19 @@ TEST_CASE_METHOD( set_group_timestamp(&group2, 2); group2.open(TILEDB_READ); - group1_received = read_group(group1); - REQUIRE_THAT( - group1_received, Catch::Matchers::UnorderedEquals(group1_expected)); - - group2_received = read_group(group2); - REQUIRE_THAT( - group2_received, Catch::Matchers::UnorderedEquals(group2_expected)); + if (vfs_test_setup_.is_rest()) { + REQUIRE_THAT( + read_group_details(group1), + Catch::Matchers::UnorderedEquals(group1_exp_det)); + REQUIRE_THAT( + read_group_details(group2), + Catch::Matchers::UnorderedEquals(group2_exp_det)); + } else { + REQUIRE_THAT( + read_group(group1), Catch::Matchers::UnorderedEquals(group1_expected)); + REQUIRE_THAT( + read_group(group2), Catch::Matchers::UnorderedEquals(group2_expected)); + } // Check that out of bounds indexing throws REQUIRE_THROWS(group1.member(10)); From 3186b6ec104b11b251f6516d838f118fee3905a7 Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Wed, 16 Oct 2024 18:12:58 +0300 Subject: [PATCH 8/9] Disable another rest test because it's not passing --- test/src/unit-cppapi-group.cc | 47 ++++++++++++++++------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 33eccbbc203..9f1c1bf5c3f 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -297,10 +297,13 @@ TEST_CASE_METHOD( } TEST_CASE_METHOD( - GroupCPPFx, "C++ API: Group, set name", "[cppapi][group][read][rest]") { + GroupCPPFx, + "C++ API: Group, set name", + "[cppapi][group][read][rest-fails][sc-57867]") { std::string array1_uri = vfs_test_setup_.array_uri("array1"); std::string array2_uri = vfs_test_setup_.array_uri("array2"); std::string array3_uri = vfs_test_setup_.array_uri("array3"); + std::cerr << array1_uri << std::endl; create_array(array1_uri); create_array(array2_uri); @@ -310,21 +313,22 @@ TEST_CASE_METHOD( std::string group2_uri = vfs_test_setup_.array_uri("group2"); tiledb::Group::create(ctx_, group1_uri); tiledb::Group::create(ctx_, group2_uri); + std::cerr << group1_uri << std::endl; // Set expected + auto uri_format = [&](std::string uri) { + return vfs_test_setup_.is_rest() ? vfs_test_setup_.with_prefix(uri) : uri; + }; + auto expected_uri = vfs_test_setup_.is_rest() ? + vfs_test_setup_.with_prefix(array1_uri) : + array1_uri; std::vector group1_expected = { tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array1_uri), - "array1"), + tiledb::Object::Type::Array, uri_format(array1_uri), "array1"), tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), - "array2"), + tiledb::Object::Type::Array, uri_format(array2_uri), "array2"), tiledb::Object( - tiledb::Object::Type::Group, - vfs_test_setup_.with_prefix(group2_uri), - "group2"), + tiledb::Object::Type::Group, uri_format(group2_uri), "group2"), }; std::vector>> group1_exp_det = { @@ -333,9 +337,7 @@ TEST_CASE_METHOD( {tiledb::Object::Type::Group, "group2"}}; std::vector group2_expected = { tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array3_uri), - "array3"), + tiledb::Object::Type::Array, uri_format(array3_uri), "array3"), }; std::vector>> group2_exp_det = {{tiledb::Object::Type::Array, "array3"}}; @@ -465,19 +467,16 @@ TEST_CASE_METHOD( tiledb::Group::create(ctx_, group2_uri); // Set expected + auto uri_format = [&](std::string uri) { + return vfs_test_setup_.is_rest() ? vfs_test_setup_.with_prefix(uri) : uri; + }; std::vector group1_expected = { tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array1_uri), - std::nullopt), + tiledb::Object::Type::Array, uri_format(array1_uri), std::nullopt), tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), - std::nullopt), + tiledb::Object::Type::Array, uri_format(array2_uri), std::nullopt), tiledb::Object( - tiledb::Object::Type::Group, - vfs_test_setup_.with_prefix(group2_uri), - std::nullopt), + tiledb::Object::Type::Group, uri_format(group2_uri), std::nullopt), }; std::vector>> group1_exp_det = { @@ -486,9 +485,7 @@ TEST_CASE_METHOD( {tiledb::Object::Type::Group, std::nullopt}}; std::vector group2_expected = { tiledb::Object( - tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array3_uri), - std::nullopt), + tiledb::Object::Type::Array, uri_format(array3_uri), std::nullopt), }; std::vector>> group2_exp_det = {{tiledb::Object::Type::Array, std::nullopt}}; From 22b550fef011b800051c5154f3671940a3281af5 Mon Sep 17 00:00:00 2001 From: Ypatia Tsavliri Date: Thu, 17 Oct 2024 10:21:34 +0000 Subject: [PATCH 9/9] Fix URIs in tests --- test/src/unit-cppapi-group.cc | 69 ++++++++++++++++++---------------- test/support/src/vfs_helpers.h | 62 ------------------------------ 2 files changed, 36 insertions(+), 95 deletions(-) diff --git a/test/src/unit-cppapi-group.cc b/test/src/unit-cppapi-group.cc index 9f1c1bf5c3f..8e852d02da5 100644 --- a/test/src/unit-cppapi-group.cc +++ b/test/src/unit-cppapi-group.cc @@ -303,7 +303,6 @@ TEST_CASE_METHOD( std::string array1_uri = vfs_test_setup_.array_uri("array1"); std::string array2_uri = vfs_test_setup_.array_uri("array2"); std::string array3_uri = vfs_test_setup_.array_uri("array3"); - std::cerr << array1_uri << std::endl; create_array(array1_uri); create_array(array2_uri); @@ -313,22 +312,20 @@ TEST_CASE_METHOD( std::string group2_uri = vfs_test_setup_.array_uri("group2"); tiledb::Group::create(ctx_, group1_uri); tiledb::Group::create(ctx_, group2_uri); - std::cerr << group1_uri << std::endl; - // Set expected - auto uri_format = [&](std::string uri) { - return vfs_test_setup_.is_rest() ? vfs_test_setup_.with_prefix(uri) : uri; - }; - auto expected_uri = vfs_test_setup_.is_rest() ? - vfs_test_setup_.with_prefix(array1_uri) : - array1_uri; std::vector group1_expected = { tiledb::Object( - tiledb::Object::Type::Array, uri_format(array1_uri), "array1"), + tiledb::Object::Type::Array, + tiledb::sm::URI(array1_uri).to_string(), + "array1"), tiledb::Object( - tiledb::Object::Type::Array, uri_format(array2_uri), "array2"), + tiledb::Object::Type::Array, + tiledb::sm::URI(array2_uri).to_string(), + "array2"), tiledb::Object( - tiledb::Object::Type::Group, uri_format(group2_uri), "group2"), + tiledb::Object::Type::Group, + tiledb::sm::URI(group2_uri).to_string(), + "group2"), }; std::vector>> group1_exp_det = { @@ -337,7 +334,9 @@ TEST_CASE_METHOD( {tiledb::Object::Type::Group, "group2"}}; std::vector group2_expected = { tiledb::Object( - tiledb::Object::Type::Array, uri_format(array3_uri), "array3"), + tiledb::Object::Type::Array, + tiledb::sm::URI(array3_uri).to_string(), + "array3"), }; std::vector>> group2_exp_det = {{tiledb::Object::Type::Array, "array3"}}; @@ -466,17 +465,19 @@ TEST_CASE_METHOD( tiledb::Group::create(ctx_, group1_uri); tiledb::Group::create(ctx_, group2_uri); - // Set expected - auto uri_format = [&](std::string uri) { - return vfs_test_setup_.is_rest() ? vfs_test_setup_.with_prefix(uri) : uri; - }; std::vector group1_expected = { tiledb::Object( - tiledb::Object::Type::Array, uri_format(array1_uri), std::nullopt), + tiledb::Object::Type::Array, + tiledb::sm::URI(array1_uri).to_string(), + std::nullopt), tiledb::Object( - tiledb::Object::Type::Array, uri_format(array2_uri), std::nullopt), + tiledb::Object::Type::Array, + tiledb::sm::URI(array2_uri).to_string(), + std::nullopt), tiledb::Object( - tiledb::Object::Type::Group, uri_format(group2_uri), std::nullopt), + tiledb::Object::Type::Group, + tiledb::sm::URI(group2_uri).to_string(), + std::nullopt), }; std::vector>> group1_exp_det = { @@ -485,7 +486,9 @@ TEST_CASE_METHOD( {tiledb::Object::Type::Group, std::nullopt}}; std::vector group2_expected = { tiledb::Object( - tiledb::Object::Type::Array, uri_format(array3_uri), std::nullopt), + tiledb::Object::Type::Array, + tiledb::sm::URI(array3_uri).to_string(), + std::nullopt), }; std::vector>> group2_exp_det = {{tiledb::Object::Type::Array, std::nullopt}}; @@ -631,21 +634,21 @@ TEST_CASE_METHOD( std::vector group1_expected = { tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array1_uri), + tiledb::sm::URI(array1_uri).to_string(), std::nullopt), tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), + tiledb::sm::URI(array2_uri).to_string(), std::nullopt), tiledb::Object( tiledb::Object::Type::Group, - vfs_test_setup_.with_prefix(group2_uri), + tiledb::sm::URI(group2_uri).to_string(), std::nullopt), }; std::vector group2_expected = { tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array3_uri), + tiledb::sm::URI(array3_uri).to_string(), std::nullopt), }; @@ -767,21 +770,21 @@ TEST_CASE_METHOD( std::vector group1_expected = { tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array1_uri), + tiledb::sm::URI(array1_uri).to_string(), "one"), tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), + tiledb::sm::URI(array2_uri).to_string(), "two"), tiledb::Object( tiledb::Object::Type::Group, - vfs_test_setup_.with_prefix(group2_uri), + tiledb::sm::URI(group2_uri).to_string(), "three"), }; std::vector group2_expected = { tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array3_uri), + tiledb::sm::URI(array3_uri).to_string(), "four"), }; @@ -915,19 +918,19 @@ TEST_CASE_METHOD( std::vector group1_expected = { tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array1_uri), + tiledb::sm::URI(array1_uri).to_string(), "one"), tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), + tiledb::sm::URI(array2_uri).to_string(), "two"), nameless_uri ? tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), + tiledb::sm::URI(array2_uri).to_string(), nullopt) : tiledb::Object( tiledb::Object::Type::Array, - vfs_test_setup_.with_prefix(array2_uri), + tiledb::sm::URI(array2_uri).to_string(), "three"), }; diff --git a/test/support/src/vfs_helpers.h b/test/support/src/vfs_helpers.h index 8d45ebe9cbb..9230dade980 100644 --- a/test/support/src/vfs_helpers.h +++ b/test/support/src/vfs_helpers.h @@ -166,14 +166,6 @@ class SupportedFs { * @return true if local, false if not */ virtual bool is_local() = 0; - - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - virtual std::string file_prefix() = 0; }; /** @@ -246,16 +238,6 @@ class SupportedFsS3 : public SupportedFs { return false; } - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - inline std::string file_prefix() { - return ""; - } - private: /* ********************************* */ /* ATTRIBUTES */ @@ -342,16 +324,6 @@ class SupportedFsHDFS : public SupportedFs { return false; } - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - inline std::string file_prefix() { - return ""; - } - private: /* ********************************* */ /* ATTRIBUTES */ @@ -432,16 +404,6 @@ class SupportedFsAzure : public SupportedFs { return false; } - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - inline std::string file_prefix() { - return ""; - } - private: /* ********************************* */ /* ATTRIBUTES */ @@ -527,16 +489,6 @@ class SupportedFsGCS : public SupportedFs { return false; } - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - inline std::string file_prefix() { - return ""; - } - private: /* ********************************* */ /* ATTRIBUTES */ @@ -725,16 +677,6 @@ class SupportedFsMem : public SupportedFs { return true; } - /** - * Get the name of the filesystem's file prefix. Applies only to local - * filesystems - * - * @return string prefix name - */ - inline std::string file_prefix() { - return ""; - } - private: /* ********************************* */ /* ATTRIBUTES */ @@ -992,10 +934,6 @@ struct VFSTestSetup { return uri; } - std::string with_prefix(const std::string& array_name) { - return fs_vec[0]->file_prefix() + array_name; - } - Context ctx() { return Context(ctx_c, false); }