Fixes

Author: kounelisagis

test/src/unit-cppapi-profile.cc

@@ -35,44 +35,33 @@
 
 #include "test/support/src/temporary_local_directory.h"
 #include "tiledb/sm/cpp_api/profile_experimental.h"
+#include "tiledb/sm/rest/rest_profile.h"
 
 using namespace tiledb;
 using namespace tiledb::test;
 
-const char* name_ = tiledb::sm::RestProfile::DEFAULT_NAME.c_str();
+const std::string name_ = tiledb::sm::RestProfile::DEFAULT_NAME;
 tiledb::sm::TemporaryLocalDirectory tempdir_("unit_cppapi_profile");
 
-/**
- * Notes to reviewer:
- * This test is segfaulting. I suspect it's something to do with the
- * destruction of the underlying object. I had a lot of trouble getting
- * the behavior on L91 of tiledb/sm/cpp_api/profile_experimental.h to work,
- * which may be a conrtibuting factor.
- *
- * Please see my other notes to reviewer on L127 of
- * tiledb/sm/rest/rest_profile.h for important information regarding the
- * get_homedir() method.
- */
-
 TEST_CASE(
     "C++ API: Profile get_name validation", "[cppapi][profile][get_name]") {
-  auto homedir_ = tempdir_.path().c_str();
+  const std::string homedir_ = tempdir_.path();
   SECTION("default, explicitly passed") {
     Profile p(name_, homedir_);
     REQUIRE(p.get_name() == name_);
   }
-  /*SECTION("default, inherited from nullptr") {
-    Profile p(nullptr, homedir_);
+  SECTION("default, inherited from nullptr") {
+    Profile p(std::nullopt, homedir_);
     REQUIRE(p.get_name() == name_);
   }
   SECTION("non-default") {
     const char* name = "non_default";
     Profile p(name, homedir_);
     REQUIRE(p.get_name() == name);
-  }*/
+  }
 }
 
-/*TEST_CASE(
+TEST_CASE(
     "C++ API: Profile get_homedir validation",
     "[cppapi][profile][get_homedir]") {
   auto homedir_ = tempdir_.path().c_str();
@@ -81,7 +70,12 @@ TEST_CASE(
     REQUIRE(p.get_homedir() == homedir_);
   }
   SECTION("inherited from nullptr") {
-    Profile p(name_, nullptr);
-    REQUIRE(p.get_homedir() == homedir_);
+    Profile p(name_, std::nullopt);
+    REQUIRE(p.get_homedir() == tiledb::common::filesystem::home_directory());
   }
-}*/
+  SECTION("non-default") {
+    const char* homedir = "non_default";
+    Profile p(name_, homedir);
+    REQUIRE(p.get_homedir() == "non_default");
+  }
+}

tiledb/api/c_api/profile/profile_api.cc

@@ -43,22 +43,20 @@ capi_return_t tiledb_profile_alloc(
   if (!name) {
     // Passing nullptr resolves to the default case.
     name = tiledb::sm::RestProfile::DEFAULT_NAME.c_str();
-  }
-  if (name[0] == '\0') {
+  } else if (name[0] == '\0') {
     throw CAPIException("[tiledb_profile_alloc] Name cannot be empty.");
   }
 
-  if (homedir) {
-    if (homedir[0] == '\0') {
-      throw CAPIException(
-          "[tiledb_profile_alloc] $HOME directory cannot be empty.");
-    }
-    *profile =
-        tiledb_profile_t::make_handle(std::string(name), std::string(homedir));
-  } else {
-    *profile = tiledb_profile_t::make_handle(std::string(name));
+  if (!homedir) {
+    // Passing nullptr resolves to the default case.
+    homedir = tiledb::common::filesystem::home_directory().c_str();
+  } else if (homedir[0] == '\0') {
+    throw CAPIException(
+        "[tiledb_profile_alloc] $HOME directory cannot be empty.");
   }
 
+  *profile = tiledb_profile_t::make_handle(name, homedir);
+
   return TILEDB_OK;
 }
 

tiledb/api/c_api/profile/test/unit_capi_profile.cc

@@ -86,7 +86,12 @@ TEST_CASE(
     REQUIRE(tiledb_status(rc) == TILEDB_OK);
     // Validate homedir is non-empty. The path may not be resolved when using
     // the default homedir (per the RestProfile::homedir() invariant).
-    // #TODO REQUIRE(homedir[0] != '\0');
+    const char* out_ptr;
+    size_t out_length;
+    rc = tiledb_string_view(homedir, &out_ptr, &out_length);
+    REQUIRE(rc == TILEDB_OK);
+    std::string out_str(out_ptr, out_length);
+    CHECK(out_str == tiledb::common::filesystem::home_directory());
     REQUIRE_NOTHROW(tiledb_profile_free(&profile));
     CHECK(profile == nullptr);
   }

tiledb/api/c_api/string/string_api_internal.h

@@ -64,14 +64,6 @@ struct tiledb_string_handle_t
    * Ordinary constructor.
    * @param s A string
    */
-  explicit tiledb_string_handle_t(const std::string& s)
-      : value_{s} {
-  }
-
-  /**
-   * Ordinary constructor.
-   * @param s A string_view
-   */
   explicit tiledb_string_handle_t(const std::string_view s)
       : value_{s} {
   }

tiledb/sm/cpp_api/profile_experimental.h

@@ -70,13 +70,15 @@ class Profile {
    * preserve local files from in-test changes. Users may pass their own
    * `homedir`, but are encouraged to use `nullptr`, the default case.
    *
-   * @param name The profile name, or `nullptr` for default.
-   * @param homedir The local $HOME directory path, or `nullptr` for default.
+   * @param name The profile name if you want to override the default.
+   * @param homedir The local $HOME directory path if you want to override the
+   * default.
    */
-  explicit Profile(const std::string& name, const std::string& homedir) {
-    const char *n = nullptr, *h = nullptr;
-    name.c_str();
-    homedir.c_str();
+  explicit Profile(
+      std::optional<std::string> name = std::nullopt,
+      std::optional<std::string> homedir = std::nullopt) {
+    const char* n = name.has_value() ? name->c_str() : nullptr;
+    const char* h = homedir.has_value() ? homedir->c_str() : nullptr;
 
     tiledb_profile_t* capi_profile;
     tiledb_error_t* capi_error = nullptr;
@@ -117,7 +119,11 @@ class Profile {
       throw ProfileException(
           "Failed to retrieve profile name due to an internal error.");
 
-    return impl::CAPIString(&name).str();
+    const char* out_ptr;
+    size_t out_length;
+    tiledb_string_view(name, &out_ptr, &out_length);
+    std::string out_str(out_ptr, out_length);
+    return out_str;
   }
 
   /** Retrieves the homedir of the profile. */
@@ -128,7 +134,12 @@ class Profile {
     if (rc != TILEDB_OK)
       throw ProfileException(
           "Failed to retrieve profile homedir due to an internal error.");
-    return impl::CAPIString(&homedir).str();
+
+    const char* out_ptr;
+    size_t out_length;
+    tiledb_string_view(homedir, &out_ptr, &out_length);
+    std::string out_str(out_ptr, out_length);
+    return out_str;
   }
 
  private:

tiledb/sm/rest/test/unit_rest_profile.cc

@@ -102,13 +102,14 @@ struct RestProfileFx {
    * Returns the RestProfile at the given name from the local file,
    * as a json object.
    */
-  json profile_from_file_to_json(std::string filepath, std::string name) {
+  json profile_from_file_to_json(
+      std::string_view filepath, std::string_view name) {
     json data;
     if (std::filesystem::exists(filepath)) {
       std::ifstream file(filepath);
       file >> data;
       file.close();
-      return data[name];
+      return data[std::string(name)];
     } else {
       return data;
     }