Skip to content

Commit

Permalink
Update testing.
Browse files Browse the repository at this point in the history
  • Loading branch information
shaunrd0 committed Jan 27, 2025
1 parent 2ec686c commit f05f1d0
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 12 deletions.
41 changes: 34 additions & 7 deletions test/src/unit-ctx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,26 +116,53 @@ TEST_CASE("REST capabilities endpoint", "[rest][version]") {
auto expected_version =
std::make_from_tuple<tiledb::sm::RestCapabilities::TileDBVersion>(
tiledb::version());
tiledb::common::ThreadPool tp(1);
DYNAMIC_SECTION(
"GET request to retrieve REST tiledb version - "
<< serialization_format) {
vfs_test_setup.update_config(config.ptr().get());
auto ctx = vfs_test_setup.ctx();
auto rest_client = tiledb::sm::RestClientRemote(
tiledb::test::g_helper_stats,
config.ptr()->config(),
tp,
*tiledb::test::g_helper_logger(),
tiledb::test::get_test_memory_tracker());
tiledb::sm::RestCapabilities expected_capabilities(
expected_version,
{expected_version.major_,
expected_version.minor_ - 1,
expected_version.patch_});
auto actual_capabilities =
ctx.ptr()->rest_client().get_capabilities_from_rest();
// Check on construction the capabilities are not initialized.
REQUIRE(!rest_client.rest_capabilities_detected());
auto actual_capabilities = rest_client.get_capabilities_from_rest();
// GET request above initializes RestCapabilities and contents are valid.
REQUIRE(expected_capabilities == actual_capabilities);
REQUIRE(rest_client.rest_capabilities_detected());
}
DYNAMIC_SECTION(
"Initialization of rest_tiledb_version_ on first access - "
<< serialization_format) {
vfs_test_setup.update_config(config.ptr().get());
auto ctx = vfs_test_setup.ctx();
REQUIRE(ctx.ptr()->rest_client().rest_version() == expected_version);
// Construct enabled Stats for this test to verify http request count.
auto stats = tiledb::sm::stats::Stats("capabilities_stats");
auto rest_client = tiledb::sm::RestClientRemote(
stats,
config.ptr()->config(),
tp,
*tiledb::test::g_helper_logger(),
tiledb::test::get_test_memory_tracker());
// Here we don't call `get_capabilities_from_rest`, but instead attempt to
// first access RestCapabilities directly. The RestClient should submit
// the GET request and initialize RestCapabilities, returning the result.
REQUIRE(!rest_client.rest_capabilities_detected());
REQUIRE(rest_client.rest_tiledb_version() == expected_version);
auto match_request_count = Catch::Matchers::ContainsSubstring(
"\"capabilities_stats.RestClient.rest_http_requests\": 1");
REQUIRE_THAT(stats.dump(0, 0), match_request_count);

// After the access above, RestCapabilities has been initialized.
// Subsequent access attempts should not submit any additional requests.
REQUIRE(rest_client.rest_capabilities_detected());
REQUIRE(rest_client.rest_tiledb_version() == expected_version);
REQUIRE_THAT(stats.dump(0, 0), match_request_count);
}
}
}
3 changes: 3 additions & 0 deletions tiledb/sm/rest/curl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ size_t write_header_callback(
auto* const header_buffer = static_cast<char*>(res_data);
auto* const pmHeader = static_cast<HeaderCbData*>(userdata);

// If we have enabled caching of the redirect URI ensure it's not empty.
// If disabled for this request, do not treat an empty asset URI as an error.
if (pmHeader->should_cache_redirect && pmHeader->uri->empty()) {
LOG_ERROR("Rest components as array_ns and array_uri cannot be empty");
return 0;
Expand Down Expand Up @@ -540,6 +542,7 @@ Status Curl::make_curl_request_common(
data->get_offset();
}

stats->add_counter("rest_http_requests", 1);
// <= because the 0ths retry is actually the initial request
for (uint8_t i = 0; i <= retry_count_; i++) {
WriteCbState write_cb_state;
Expand Down
12 changes: 9 additions & 3 deletions tiledb/sm/rest/rest_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class FragmentInfo;
class MemoryTracker;
class Query;
class QueryPlan;
class RestCapabilities;
struct RestCapabilities;

// Forward for friend declaration within `class RestClient`
class RestClientFactory;
Expand Down Expand Up @@ -299,12 +299,18 @@ class RestClient {
}

/// Operation disabled in base class.
inline virtual const TileDBVersion& rest_version() const {
inline virtual const TileDBVersion& rest_tiledb_version() const {
throw RestClientDisabledException();
}

/// Operation disabled in base class.
inline virtual const TileDBVersion& rest_minimum_supported_version() const {
inline virtual const TileDBVersion& rest_minimum_supported_tiledb_version()
const {
throw RestClientDisabledException();
}

/// Operation disabled in base class.
inline virtual bool rest_capabilities_detected() const {
throw RestClientDisabledException();
}

Expand Down
16 changes: 14 additions & 2 deletions tiledb/sm/rest/rest_client_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,29 @@ class RestClientRemote : public RestClient {
/**
* @return TileDB core version currently deployed to the REST server.
*/
inline const TileDBVersion& rest_version() const override {
inline const TileDBVersion& rest_tiledb_version() const override {
return get_capabilities_from_rest().rest_tiledb_version_;
}

/**
* @return Minimum TileDB core version currently supported by the REST server.
*/
inline const TileDBVersion& rest_minimum_supported_version() const override {
inline const TileDBVersion& rest_minimum_supported_tiledb_version()
const override {
return get_capabilities_from_rest().rest_minimum_supported_version_;
}

/**
* Check if REST capabilities are currently known to the RestClient. This
* will not attempt to initialize them if they are currently unknown.
*
* @return True if RestCapabilities member has been initialized by a previous
* REST capabilities endpoint request, else False.
*/
inline bool rest_capabilities_detected() const override {
return rest_capabilities_.detected_;
}

/**
* Check if an array exists by making a REST call. To start with this fetches
* the schema but ignores the body returned if non-error
Expand Down

0 comments on commit f05f1d0

Please sign in to comment.