Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3fd09e7
WIP adding REST support for relative group members.
shaunrd0 Aug 21, 2025
83d2aa5
Core create_group test passes.
shaunrd0 Aug 22, 2025
ac56ddd
Relative group member tests pass.
shaunrd0 Aug 22, 2025
9bfa0a7
Add test case for a server-side TODO.
shaunrd0 Aug 22, 2025
d94c700
Add tests for relative array members.
shaunrd0 Aug 25, 2025
db8e655
Tests passing from core.
shaunrd0 Aug 26, 2025
7fddc58
Test adding the same member with different names.
shaunrd0 Aug 26, 2025
c4b108f
Test for adding the same group member multiple times.
shaunrd0 Aug 27, 2025
e5cd8c6
Fix CI.
shaunrd0 Sep 2, 2025
666f193
Fix failure checking if legacy REST.
shaunrd0 Sep 2, 2025
93914a4
Skip some tests for legacy REST.
shaunrd0 Sep 4, 2025
eeb4a6d
Set group member name during serialization if unset.
shaunrd0 Sep 4, 2025
8c1f2e1
Fix test failures from sync.
shaunrd0 Sep 8, 2025
dee4d35
WIP default storage test.
shaunrd0 Sep 9, 2025
691f20f
Add test for default storage.
shaunrd0 Sep 10, 2025
1d39d0c
Format and update check for skipping test.
shaunrd0 Sep 10, 2025
9b9e8d0
Do not use explicit backend storage URIs.
shaunrd0 Sep 11, 2025
043730c
Add helper to check if rest client is enabled.
shaunrd0 Sep 11, 2025
5055e10
Remove duplicated test.
shaunrd0 Sep 11, 2025
c3b6120
Revert changes group creation precheck.
shaunrd0 Sep 11, 2025
3be3f65
Add virtual to RestClient helper.
shaunrd0 Sep 11, 2025
8fbc929
Skip for non-rest tests.
shaunrd0 Sep 12, 2025
b0b5683
Fix override warning in standalone.
shaunrd0 Sep 12, 2025
9022729
Add test for relative member nested under prefix.
shaunrd0 Sep 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 58 additions & 30 deletions test/src/unit-cppapi-group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1102,38 +1102,66 @@ TEST_CASE(
}

TEST_CASE(
"C++ API: Group with Relative URI members, write/read, rest",
"[cppapi][group][relative][rest]") {
"C++ API: Group add_member with relative URI members, write/read, rest",
"[cppapi][group][add_member][relative][rest]") {
VFSTestSetup vfs_test_setup;
tiledb::Context ctx{vfs_test_setup.ctx()};
auto group_name{vfs_test_setup.array_uri("groups_relative")};
auto subgroup_name = group_name + "/subgroup";

// Create groups
tiledb::create_group(ctx, group_name);
tiledb::create_group(ctx, subgroup_name);

// Open group in write mode
{
auto group = tiledb::Group(ctx, group_name, TILEDB_WRITE);
CHECK_NOTHROW(group.add_member("subgroup", true, "subgroup"));
if (vfs_test_setup.is_rest()) {
CHECK_THROWS_WITH(
group.close(),
Catch::Matchers::ContainsSubstring(
"relative paths are not yet supported for cloud groups"));
} else {
CHECK_NOTHROW(group.close());
}
if (!vfs_test_setup.is_rest() || vfs_test_setup.is_legacy_rest()) {
SKIP("Relative group URIs are not supported in legacy REST servers");
}

if (!vfs_test_setup.is_rest()) {
auto group = tiledb::Group(ctx, group_name, TILEDB_READ);

auto subgroup_member = group.member("subgroup");

CHECK(subgroup_member.type() == tiledb::Object::Type::Group);
CHECK(subgroup_member.name() == "subgroup");
CHECK(group.is_relative("subgroup"));
auto group_uri{vfs_test_setup.default_storage_uri("groups_relative")};
// Create parent group using tiledb URI.
REQUIRE_NOTHROW(tiledb::create_group(ctx, group_uri));

tiledb::Domain domain(ctx);
domain.add_dimension(
tiledb::Dimension::create<int>(ctx, "rows", {{1, 10}}, 10));
tiledb::ArraySchema schema(ctx, TILEDB_SPARSE);
schema.set_domain(domain);
schema.add_attribute(tiledb::Attribute::create<int>(ctx, "a1"));
// This adds the new array as a member
auto array_member_uri = group_uri + "/relative_array";
REQUIRE_NOTHROW(tiledb::Array::create(ctx, array_member_uri, schema));
// This adds the new group as a member
REQUIRE_NOTHROW(tiledb::create_group(ctx, group_uri + "/relative_group"));

// REST creates the group in a groupID prefix, we don't know what this ID is.
tiledb::VFS vfs{ctx};
auto created_group_uri = vfs.ls(vfs_test_setup.default_storage()).back();
// Creates a new group at a relative backend storage location using S3 URI.
REQUIRE_NOTHROW(
tiledb::create_group(ctx, created_group_uri + "/groups/relative_group"));

auto group = tiledb::Group(ctx, group_uri, TILEDB_WRITE);
// Attempts to add the same array as a member with the same name.
CHECK_NOTHROW(group.add_member("relative_array", true, "relative_array"));
CHECK_NOTHROW(
group.add_member("relative_array", true, "relative_array_rename"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question on spec that is not clear to me today: What does it mean to add a relative array? Relative refers to storage location? If yes, do we support group.add_member("prefix/relative_array", true, "relative_array") ? If yes we should ideally test that this works too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a manual test on the review app too, let's just not forget to check it.

CHECK_NOTHROW(group.add_member("relative_group", true, "relative_group"));
// Add the relative member we created on S3 to the parent group after creation
CHECK_NOTHROW(
group.add_member("groups/relative_group", true, "relative_group_nested"));
CHECK_NOTHROW(
group.add_member("relative_group", true, "relative_group_rename"));
REQUIRE_NOTHROW(group.close());

REQUIRE_NOTHROW(group.open(TILEDB_READ));
tiledb::sm::URI member_uri(array_member_uri);
CHECK(group.member_count() == 5);
for (const std::string name :
{"relative_array",
"relative_array_rename",
"relative_group",
"relative_group_nested",
"relative_group_rename"}) {
auto member = group.member(name);
auto object_type = tiledb::Object::Type::Group;
if (name.find("array") != std::string::npos) {
object_type = tiledb::Object::Type::Array;
}
CHECK(member.type() == object_type);
CHECK(member.name() == name);
CHECK(member.uri() == group_uri + "/" + name);
CHECK(group.is_relative(name));
}
}
13 changes: 13 additions & 0 deletions test/support/src/helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1651,6 +1651,19 @@ void schema_equiv(
CHECK(schema1.array_uri().to_string() == schema2.array_uri().to_string());
}

std::string build_tiledb_uri(
const URI& uri, const std::string& path, bool include_storage) {
URI::RESTURIComponents components;
CHECK(uri.get_rest_components(false, &components).ok());
const auto trim = components.server_path.find_first_of('/');
components.server_path.resize(trim == std::string::npos ? 0 : trim);
components.server_path += path.starts_with('/') ? path : "/" + path;
std::string result =
"tiledb://" + components.server_namespace + "/" + components.server_path;

return include_storage ? result + "/" + components.asset_storage : result;
}

template void check_subarray<int8_t>(
tiledb::sm::Subarray& subarray, const SubarrayRanges<int8_t>& ranges);
template void check_subarray<uint8_t>(
Expand Down
16 changes: 16 additions & 0 deletions test/support/src/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,22 @@ void read_sparse_v11(
void schema_equiv(
const sm::ArraySchema& schema1, const sm::ArraySchema& schema2);

/**
* Helper function to build a tiledb URI using the provided asset path,
* following the randomly generated top-level directory used in all 3.0 REST
* tests.
*
* This helper only applies to 3.0 REST, there is no concept of asset paths in
* legacy REST.
*
* @param uri The URI to update.
* @param path The new asset path to use after the randomly generated top-level
* directory.
* @return The tiledb URI built from the provided asset path.
*/
std::string build_tiledb_uri(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's not used anymore and can be removed?

Copy link
Member Author

@shaunrd0 shaunrd0 Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used to create / empty the default storage bucket in REST CI. I could do it inline, it just seemed cleaner to have a helper. Flexible either way though if you'd rather it removed.

const sm::URI& uri, const std::string& path, bool include_storage = false);

} // namespace tiledb::test

#endif
2 changes: 1 addition & 1 deletion test/support/src/vfs_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ LocalFsTest::LocalFsTest(const std::vector<size_t>& test_tree)
}

bool VFSTestSetup::is_legacy_rest() {
return ctx_c->rest_client().rest_legacy();
return is_rest() && ctx_c->rest_client().rest_legacy();
}

} // namespace tiledb::test
43 changes: 38 additions & 5 deletions test/support/src/vfs_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,14 @@ struct VFSTestSetup {
vfs_test_remove_temp_dir(ctx_c, vfs_c, temp_dir);
}
tiledb_vfs_create_dir(ctx_c, vfs_c, temp_dir.c_str());

int is_bucket = 0;
tiledb_vfs_is_bucket(ctx_c, vfs_c, default_storage().c_str(), &is_bucket);
if (!is_bucket) {
tiledb_vfs_create_bucket(ctx_c, vfs_c, default_storage().c_str());
} else {
tiledb_vfs_empty_bucket(ctx_c, vfs_c, default_storage().c_str());
}
};

void update_config(tiledb_config_t* config) {
Expand All @@ -843,13 +851,13 @@ struct VFSTestSetup {
vfs_test_init(fs_vec, &ctx_c, &vfs_c, cfg_c).ok();
}

bool is_rest() {
bool is_rest() const {
return fs_vec[0]->is_rest();
}

bool is_legacy_rest();

bool is_local() {
bool is_local() const {
return fs_vec[0]->is_local();
}

Expand All @@ -863,10 +871,34 @@ struct VFSTestSetup {
// Non-REST case is handled above.
if (is_legacy_rest()) {
return "tiledb://unit/" + temp_dir + array_name;
} else {
return "tiledb://unit-workspace/unit-teamspace/" + random_label() + "/" +
temp_dir + array_name;
}
return "tiledb://unit-workspace/unit-teamspace/" + random_label() + "/" +
temp_dir + array_name;
}

/**
* Returns the bucket used as the default storage location for REST tests.
* The default storage location is configured when the REST user is created.
* This is the bucket name used in REST CI within the TileDB-Internal
* repository.
*
* @return Backend storage location used for default storage.
*/
std::string default_storage() const {
return sm::URI(temp_dir).backend_name() + "://" + "default-bucket";
}

/**
* Generates a tiledb URI for testing using the default storage location.
*/
std::string default_storage_uri(
const std::string& name, bool include_backend = false) {
const std::string label = "_" + random_label();
const std::string backend = include_backend ? default_storage() + "/" : "";
if (is_legacy_rest()) {
return "tiledb://unit/" + backend + name + label;
}
return "tiledb://unit-workspace/unit-teamspace/" + backend + name + label;
}

Context ctx() {
Expand All @@ -876,6 +908,7 @@ struct VFSTestSetup {
~VFSTestSetup() {
vfs_test_remove_temp_dir(ctx_c, vfs_c, temp_dir);
vfs_test_close(fs_vec, ctx_c, vfs_c).ok();
tiledb_vfs_empty_bucket(ctx_c, vfs_c, default_storage().c_str());

tiledb_ctx_free(&ctx_c);
tiledb_vfs_free(&vfs_c);
Expand Down
18 changes: 9 additions & 9 deletions tiledb/sm/group/group_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void GroupDetails::mark_member_for_addition(
std::optional<std::string>& name,
std::optional<ObjectType> type) {
std::lock_guard<std::mutex> lck(mtx_);
ObjectType obj_type;
ObjectType obj_type = ObjectType::INVALID;
if (type.has_value()) {
obj_type = type.value();
} else {
Expand All @@ -97,15 +97,14 @@ void GroupDetails::mark_member_for_addition(
absolute_group_member_uri =
group_uri_.join_path(group_member_uri.to_string());
}
obj_type = object_type(resources, absolute_group_member_uri);
}

if (obj_type == ObjectType::INVALID) {
throw GroupDetailsException(
"Cannot add group member " + group_member_uri.to_string() +
", type is INVALID. The member likely does not exist.");
// 3.0 REST will identify the object type server side.
if (!absolute_group_member_uri.is_tiledb() ||
(resources.rest_client()->rest_enabled() &&
resources.rest_client()->rest_legacy())) {
obj_type = object_type(resources, absolute_group_member_uri);
}
}

auto group_member = tdb::make_shared<GroupMemberV2>(
HERE(), group_member_uri, obj_type, relative, name, false);

Expand Down Expand Up @@ -301,7 +300,8 @@ GroupDetails::member_by_name(const std::string& name) {

auto member = it->second;
std::string uri = member->uri().to_string();
if (member->relative()) {
// Relative tiledb URIs are returned in the expected format from REST.
if (!member->uri().is_tiledb() && member->relative()) {
uri = group_uri_.join_path(member->uri().to_string()).to_string();
}

Expand Down
4 changes: 2 additions & 2 deletions tiledb/sm/object/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ ObjectType object_type(ContextResources& resources, const URI& uri) {
}
}

if (is_array(resources, uri)) {
if (is_array(resources, dir_uri)) {
return ObjectType::ARRAY;
}
if (is_group(resources, uri)) {
if (is_group(resources, dir_uri)) {
return ObjectType::GROUP;
}

Expand Down
10 changes: 10 additions & 0 deletions tiledb/sm/rest/rest_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,16 @@ class RestClient {
return rest_server_;
}

/**
* Provides context to the caller if this RestClient is enabled for remote
* operations without throwing an exception.
*
* @return False for instances of RestClient, true in RestClientRemote.
*/
virtual bool rest_enabled() const {
return false;
}

/// Operation disabled in base class.
inline virtual const std::optional<TileDBVersion>& rest_tiledb_version()
const {
Expand Down
18 changes: 18 additions & 0 deletions tiledb/sm/rest/rest_client_remote.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,15 @@ Status RestClientRemote::post_group_create_to_rest(
RETURN_NOT_OK(serialization::group_create_serialize(
group, serialization_type_, buff, rest_legacy()));

// Credential used for creating a group as a child of an existing REST group.
const auto creation_access_credentials_name{
config_->get<std::string>("rest.creation_access_credentials_name")};
if (creation_access_credentials_name.has_value()) {
add_header(
"X-TILEDB-CLOUD-ACCESS-CREDENTIALS-NAME",
creation_access_credentials_name.value());
}

// Init curl and form the URL
Curl curlc(logger_);
URI::RESTURIComponents rest_uri;
Expand Down Expand Up @@ -1516,6 +1525,15 @@ Status RestClientRemote::patch_group_to_rest(const URI& uri, Group* group) {
RETURN_NOT_OK(
serialization::group_update_serialize(group, serialization_type_, buff));

// Credential name for adding group members that are not registered on REST.
const auto creation_access_credentials_name{
config_->get<std::string>("rest.creation_access_credentials_name")};
if (creation_access_credentials_name.has_value()) {
add_header(
"X-TILEDB-CLOUD-ACCESS-CREDENTIALS-NAME",
creation_access_credentials_name.value());
}

// Init curl and form the URL
Curl curlc(logger_);
URI::RESTURIComponents rest_uri;
Expand Down
10 changes: 10 additions & 0 deletions tiledb/sm/rest/rest_client_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ class RestClientRemote : public RestClient {
*/
static bool use_refactored_query(const Config& config);

/**
* Provides context to the caller if this RestClient is enabled for remote
* operations without throwing an exception.
*
* @return True in RestClientRemote, false for instances of RestClient.
*/
bool rest_enabled() const override {
return true;
}

/**
* @return TileDB core version currently deployed to the REST server.
*/
Expand Down
7 changes: 3 additions & 4 deletions tiledb/sm/serialization/group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,9 @@ Status group_member_to_capnp(

group_member_builder->setRelative(group_member->relative());

auto name = group_member->name();
if (name.has_value()) {
group_member_builder->setName(name.value());
}
// Avoids sending a request to add a member with no name.
group_member_builder->setName(
group_member->name().value_or(group_member->uri().last_path_part()));

return Status::Ok();
}
Expand Down
Loading