Address review comments

TileDB-Inc · Oct 8, 2024 · 7a6d039 · 7a6d039
1 parent 2ee3ec9
commit 7a6d039
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 23 deletions.
diff --git 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
@@ -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<char*>(std::malloc(str.size() + 1));
   if (ret == nullptr) {
@@ -276,14 +282,15 @@ 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,
     const char* name,
     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);
 
@@ -690,7 +697,7 @@ CAPI_INTERFACE(
     const uint8_t relative,
     const char* name,
     tiledb_object_t type) {
-  return api_entry_context<tiledb::api::tiledb_group_add_member_by_type>(
+  return api_entry_context<tiledb::api::tiledb_group_add_member_with_type>(
       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
@@ -375,19 +375,23 @@ 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:**
  *
  * @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,
+ * 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
@@ -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(