Skip to content

Commit 2dfca6a

Browse files
authored
Update group URI to Logical URI coming from the Server (#5702)
In the Server it is allowed to open a group by providing its URI in AST_ID format. This causes problems for relative URIs, because Core has some existing logic to concatenate the group name with its relative members URI, in order to present nice and absolute member URIs to the users instead of relative ones. However, concatenating AST_IDs with member paths results in invalid asset URIs, as you can't append a path to an ID. To avoid this, we could either resort to just always return the member URI as is, without concatenating it: this would make sense but would break existing user scripts and expectations across backends. Or to ask the Server to update the group URI to a logical one in Array open so that concatenation always produces valid path URIs. This PR is aiming to do the second. part of CLOUD-2737 --- TYPE: BUG DESC: Update group URI to Logical URI coming from the Server
1 parent 49c1000 commit 2dfca6a

File tree

8 files changed

+212
-116
lines changed

8 files changed

+212
-116
lines changed

tiledb/sm/group/group.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,12 @@ const shared_ptr<GroupDetails> Group::group_details() const {
317317
return group_details_;
318318
}
319319

320+
void Group::set_uri(const URI& uri) {
321+
std::lock_guard<std::mutex> lck(mtx_);
322+
group_uri_ = uri;
323+
group_details_->set_group_uri(uri);
324+
}
325+
320326
QueryType Group::get_query_type() const {
321327
// Error if the group is not open
322328
if (!is_open_) {

tiledb/sm/group/group.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,13 @@ class Group {
423423
*/
424424
const shared_ptr<GroupDetails> group_details() const;
425425

426+
/**
427+
* Set the group URI
428+
*
429+
* @param uri
430+
*/
431+
void set_uri(const URI& uri);
432+
426433
protected:
427434
/* ********************************* */
428435
/* PROTECTED ATTRIBUTES */

tiledb/sm/group/group_details.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "tiledb/sm/enums/datatype.h"
3737
#include "tiledb/sm/enums/encryption_type.h"
3838
#include "tiledb/sm/enums/query_type.h"
39+
#include "tiledb/sm/filesystem/uri.h"
3940
#include "tiledb/sm/global_state/unit_test_config.h"
4041
#include "tiledb/sm/group/group_details_v1.h"
4142
#include "tiledb/sm/group/group_details_v2.h"
@@ -260,6 +261,11 @@ const URI& GroupDetails::group_uri() const {
260261
return group_uri_;
261262
}
262263

264+
void GroupDetails::set_group_uri(const URI& uri) {
265+
std::lock_guard<std::mutex> lck(mtx_);
266+
group_uri_ = uri;
267+
}
268+
263269
uint64_t GroupDetails::member_count() const {
264270
std::lock_guard<std::mutex> lck(mtx_);
265271

tiledb/sm/group/group_details.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,9 @@ class GroupDetails {
184184
/** Returns the group URI. */
185185
const URI& group_uri() const;
186186

187+
/** Sets the group URI. */
188+
void set_group_uri(const URI& uri);
189+
187190
/**
188191
* Get count of members
189192
*

tiledb/sm/serialization/group.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,11 @@ Status group_details_from_capnp(
184184
group->set_metadata_loaded(true);
185185
}
186186

187+
if (group_details_reader.hasLogicalURI()) {
188+
const char* logical_uri = group_details_reader.getLogicalURI().cStr();
189+
group->set_uri(URI(logical_uri));
190+
}
191+
187192
group->group_details()->set_modified();
188193

189194
return Status::Ok();

tiledb/sm/serialization/tiledb-rest.capnp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,8 @@ struct Group {
996996

997997
metadata @1 :ArrayMetadata;
998998
# metadata attached to group
999+
1000+
logicalURI @2 :Text;
9991001
}
10001002

10011003
config @0 :Config;

tiledb/sm/serialization/tiledb-rest.capnp.c++

Lines changed: 131 additions & 115 deletions
Large diffs are not rendered by default.

tiledb/sm/serialization/tiledb-rest.capnp.h

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ struct Group::GroupDetails {
13871387
class Pipeline;
13881388

13891389
struct _capnpPrivate {
1390-
CAPNP_DECLARE_STRUCT_HEADER(a2ea10c715b475c1, 0, 2)
1390+
CAPNP_DECLARE_STRUCT_HEADER(a2ea10c715b475c1, 0, 3)
13911391
#if !CAPNP_LITE
13921392
static constexpr ::capnp::_::RawBrandedSchema const* brand() {
13931393
return &schema->defaultBrand;
@@ -12205,6 +12205,9 @@ class Group::GroupDetails::Reader {
1220512205
inline ::tiledb::sm::serialization::capnp::ArrayMetadata::Reader getMetadata()
1220612206
const;
1220712207

12208+
inline bool hasLogicalURI() const;
12209+
inline ::capnp::Text::Reader getLogicalURI() const;
12210+
1220812211
private:
1220912212
::capnp::_::StructReader _reader;
1221012213
template <typename, ::capnp::Kind>
@@ -12277,6 +12280,13 @@ class Group::GroupDetails::Builder {
1227712280
inline ::capnp::Orphan<::tiledb::sm::serialization::capnp::ArrayMetadata>
1227812281
disownMetadata();
1227912282

12283+
inline bool hasLogicalURI();
12284+
inline ::capnp::Text::Builder getLogicalURI();
12285+
inline void setLogicalURI(::capnp::Text::Reader value);
12286+
inline ::capnp::Text::Builder initLogicalURI(unsigned int size);
12287+
inline void adoptLogicalURI(::capnp::Orphan<::capnp::Text>&& value);
12288+
inline ::capnp::Orphan<::capnp::Text> disownLogicalURI();
12289+
1228012290
private:
1228112291
::capnp::_::StructBuilder _builder;
1228212292
template <typename, ::capnp::Kind>
@@ -29915,6 +29925,47 @@ Group::GroupDetails::Builder::disownMetadata() {
2991529925
_builder.getPointerField(::capnp::bounded<1>() * ::capnp::POINTERS));
2991629926
}
2991729927

29928+
inline bool Group::GroupDetails::Reader::hasLogicalURI() const {
29929+
return !_reader.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS)
29930+
.isNull();
29931+
}
29932+
inline bool Group::GroupDetails::Builder::hasLogicalURI() {
29933+
return !_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS)
29934+
.isNull();
29935+
}
29936+
inline ::capnp::Text::Reader Group::GroupDetails::Reader::getLogicalURI()
29937+
const {
29938+
return ::capnp::_::PointerHelpers<::capnp::Text>::get(
29939+
_reader.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS));
29940+
}
29941+
inline ::capnp::Text::Builder Group::GroupDetails::Builder::getLogicalURI() {
29942+
return ::capnp::_::PointerHelpers<::capnp::Text>::get(
29943+
_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS));
29944+
}
29945+
inline void Group::GroupDetails::Builder::setLogicalURI(
29946+
::capnp::Text::Reader value) {
29947+
::capnp::_::PointerHelpers<::capnp::Text>::set(
29948+
_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS),
29949+
value);
29950+
}
29951+
inline ::capnp::Text::Builder Group::GroupDetails::Builder::initLogicalURI(
29952+
unsigned int size) {
29953+
return ::capnp::_::PointerHelpers<::capnp::Text>::init(
29954+
_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS),
29955+
size);
29956+
}
29957+
inline void Group::GroupDetails::Builder::adoptLogicalURI(
29958+
::capnp::Orphan<::capnp::Text>&& value) {
29959+
::capnp::_::PointerHelpers<::capnp::Text>::adopt(
29960+
_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS),
29961+
kj::mv(value));
29962+
}
29963+
inline ::capnp::Orphan<::capnp::Text>
29964+
Group::GroupDetails::Builder::disownLogicalURI() {
29965+
return ::capnp::_::PointerHelpers<::capnp::Text>::disown(
29966+
_builder.getPointerField(::capnp::bounded<2>() * ::capnp::POINTERS));
29967+
}
29968+
2991829969
inline bool GroupUpdate::Reader::hasConfig() const {
2991929970
return !_reader.getPointerField(::capnp::bounded<0>() * ::capnp::POINTERS)
2992029971
.isNull();

0 commit comments

Comments
 (0)