Skip to content

Commit d3d2cc8

Browse files
authored
Fix deserialization of relative group member URI (#5654)
When testing relative members with the Server, we noticed the URIs returned from certain group member getter APIs were incorrect. This is because we used a constructor for `URI` that implicitly converts paths to absolute paths. This PR is fixing this issue by checking if the URI is relative and skips absolute path conversion in that case. It also adds a regression test. --- TYPE: BUG DESC: Fix deserialization of relative group member URI
1 parent 500e708 commit d3d2cc8

File tree

4 files changed

+110
-2
lines changed

4 files changed

+110
-2
lines changed

tiledb/sm/serialization/group.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ group_member_from_capnp(capnp::GroupMember::Reader* group_member_reader) {
127127
name = group_member_reader->getName().cStr();
128128
}
129129

130-
tdb_shared_ptr<GroupMember> group_member =
131-
tdb::make_shared<GroupMemberV1>(HERE(), URI(uri), type, relative, name);
130+
tdb_shared_ptr<GroupMember> group_member = tdb::make_shared<GroupMemberV1>(
131+
HERE(), URI(uri, !relative), type, relative, name);
132132

133133
return {Status::Ok(), group_member};
134134
}

tiledb/sm/serialization/group.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@
3636
#include <unordered_map>
3737

3838
#include "tiledb/common/status.h"
39+
#include "tiledb/sm/group/group_member.h"
40+
41+
#ifdef TILEDB_SERIALIZATION
42+
#include "tiledb/sm/serialization/capnp_utils.h"
43+
#endif
3944

4045
using namespace tiledb::common;
4146

@@ -160,6 +165,30 @@ Status group_metadata_serialize(
160165
SerializationBuffer& serialized_buffer,
161166
bool load);
162167

168+
#ifdef TILEDB_SERIALIZATION
169+
170+
/**
171+
* Convert Cap'n Proto message to GroupMember object
172+
*
173+
* @param group_member_reader cap'n proto class.
174+
* @return Status and GroupMember object
175+
*/
176+
std::tuple<Status, std::optional<tdb_shared_ptr<GroupMember>>>
177+
group_member_from_capnp(capnp::GroupMember::Reader* group_member_reader);
178+
179+
/**
180+
* Convert GroupMember object to Cap'n Proto message.
181+
*
182+
* @param group_member GroupMember to serialize info from
183+
* @param group_member_builder cap'n proto class.
184+
* @return Status
185+
*/
186+
Status group_member_to_capnp(
187+
const tdb_shared_ptr<GroupMember>& group_member,
188+
capnp::GroupMember::Builder* group_member_builder);
189+
190+
#endif // TILEDB_SERIALIZATION
191+
163192
} // namespace serialization
164193
} // namespace sm
165194
} // namespace tiledb

tiledb/sm/serialization/test/CMakeLists.txt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,11 @@ commence(unit_test capnp_nonempty_domain)
5050
# Enable serialization
5151
target_compile_definitions(unit_capnp_nonempty_domain PRIVATE -DTILEDB_SERIALIZATION)
5252
conclude(unit_test)
53+
54+
commence(unit_test capnp_group)
55+
this_target_sources(main.cc unit_capnp_group.cc)
56+
this_target_link_libraries(TILEDB_CORE_OBJECTS TILEDB_CORE_OBJECTS_ILIB tiledb_test_support_lib tiledb_core_objects_oxidize)
57+
58+
# Enable serialization
59+
target_compile_definitions(unit_capnp_group PRIVATE -DTILEDB_SERIALIZATION)
60+
conclude(unit_test)
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* @file tiledb/sm/serialization/test/unit_capnp_group.cc
3+
*
4+
* @section LICENSE
5+
*
6+
* The MIT License
7+
*
8+
* @copyright Copyright (c) 2025 TileDB, Inc.
9+
*
10+
* Permission is hereby granted, free of charge, to any person obtaining a copy
11+
* of this software and associated documentation files (the "Software"), to deal
12+
* in the Software without restriction, including without limitation the rights
13+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
14+
* copies of the Software, and to permit persons to whom the Software is
15+
* furnished to do so, subject to the following conditions:
16+
*
17+
* The above copyright notice and this permission notice shall be included in
18+
* all copies or substantial portions of the Software.
19+
*
20+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
21+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
22+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
23+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
24+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
25+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
26+
* THE SOFTWARE.
27+
*
28+
* @section DESCRIPTION
29+
*
30+
* This file contains serialization tests for groups
31+
*/
32+
33+
#include <capnp/message.h>
34+
35+
#include <test/support/tdb_catch.h>
36+
#include "test/support/src/mem_helpers.h"
37+
38+
#include "tiledb/sm/serialization/group.h"
39+
40+
using namespace tiledb::common;
41+
using namespace tiledb::sm;
42+
43+
TEST_CASE(
44+
"Check group member serialization correctly handles relative uris",
45+
"[group][serialization][relative_uri][regression]") {
46+
auto group_member = tdb::make_shared<GroupMember>(
47+
HERE(),
48+
URI("relative_member", false),
49+
ObjectType::ARRAY,
50+
2,
51+
true,
52+
std::nullopt,
53+
false);
54+
55+
// Serialize
56+
::capnp::MallocMessageBuilder message;
57+
tiledb::sm::serialization::capnp::GroupMember::Builder builder =
58+
message.initRoot<tiledb::sm::serialization::capnp::GroupMember>();
59+
60+
auto rc =
61+
tiledb::sm::serialization::group_member_to_capnp(group_member, &builder);
62+
REQUIRE(rc.ok());
63+
64+
tiledb::sm::serialization::capnp::GroupMember::Reader reader =
65+
(tiledb::sm::serialization::capnp::GroupMember::Reader)builder;
66+
auto&& [status, clone] =
67+
tiledb::sm::serialization::group_member_from_capnp(&reader);
68+
REQUIRE(status.ok());
69+
70+
REQUIRE(clone.value()->uri().to_string() == "relative_member");
71+
}

0 commit comments

Comments
 (0)