[QUEUED scylladb next-5.4] Merge 'alternator: fix REST API access to an Alternator LSI' from Nadav Har'El

[Commit Bot]

From: Botond Dénes <bde...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>
Branch: next-5.4

Merge 'alternator: fix REST API access to an Alternator LSI' from Nadav Har'El

The name of the Scylla table backing an Alternator LSI looks like `basename:!lsiname`. Some REST API clients (including Scylla Manager) when they send a "!" character in the REST API request path may decide to "URL encode" it - convert it to `%21`.

Because of a Seastar bug (https://github.com/scylladb/seastar/issues/725) Scylla's REST API server forgets to do the URL decoding on the path part of the request, which leads to the REST API request failing to address the LSI table.

The first patch in this PR fixes the bug by using a new Seastar API introduced in https://github.com/scylladb/seastar/pull/2125 that does the URL decoding as appropriate. The second patch in the PR is a new test for this bug, which fails without the fix, and passes afterwards.

Fixes #5883.

Closes scylladb/scylladb#18286

* github.com:scylladb/scylladb:
test/alternator: test addressing LSI using REST API
REST API: stop using deprecated, buggy, path parameter

(cherry picked from commit 0438febdc9374f0256f6b0fce33324282eb73cd9)

Signed-off-by: Nadav Har'El <n...@scylladb.com>

---
diff --git a/api/api.cc b/api/api.cc
--- a/api/api.cc
+++ b/api/api.cc
@@ -314,7 +314,7 @@ void req_params::process(const request& req) {
continue;
}
try {
- ent.value = req.param[name];
+ ent.value = req.get_path_param(name);
} catch (std::out_of_range&) {
throw httpd::bad_param_exception(fmt::format("Mandatory parameter '{}' was not provided", name));
}
diff --git a/api/collectd.cc b/api/collectd.cc
--- a/api/collectd.cc
+++ b/api/collectd.cc
@@ -54,7 +54,7 @@ static const char* str_to_regex(const sstring& v) {
void set_collectd(http_context& ctx, routes& r) {
cd::get_collectd.set(r, [](std::unique_ptr<request> req) {

- auto id = ::make_shared<scollectd::type_instance_id>(req->param["pluginid"],
+ auto id = ::make_shared<scollectd::type_instance_id>(req->get_path_param("pluginid"),
req->get_query_param("instance"), req->get_query_param("type"),
req->get_query_param("type_instance"));

@@ -91,7 +91,7 @@ void set_collectd(http_context& ctx, routes& r) {
});

cd::enable_collectd.set(r, [](std::unique_ptr<request> req) -> future<json::json_return_type> {
- std::regex plugin(req->param["pluginid"].c_str());
+ std::regex plugin(req->get_path_param("pluginid").c_str());
std::regex instance(str_to_regex(req->get_query_param("instance")));
std::regex type(str_to_regex(req->get_query_param("type")));
std::regex type_instance(str_to_regex(req->get_query_param("type_instance")));
diff --git a/api/column_family.cc b/api/column_family.cc
--- a/api/column_family.cc
+++ b/api/column_family.cc
@@ -333,7 +333,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_memtable_columns_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t{0}, [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t{0}, [](replica::column_family& cf) {
return boost::accumulate(cf.active_memtables() | boost::adaptors::transformed(std::mem_fn(&replica::memtable::partition_count)), uint64_t(0));
}, std::plus<>());
});
@@ -353,7 +353,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_memtable_off_heap_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
return boost::accumulate(cf.active_memtables() | boost::adaptors::transformed([] (replica::memtable* active_memtable) {
return active_memtable->region().occupancy().total_space();
}), uint64_t(0));
@@ -369,7 +369,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_memtable_live_data_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
return boost::accumulate(cf.active_memtables() | boost::adaptors::transformed([] (replica::memtable* active_memtable) {
return active_memtable->region().occupancy().used_space();
}), uint64_t(0));
@@ -394,7 +394,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

cf::get_cf_all_memtables_off_heap_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
warn(unimplemented::cause::INDEXES);
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
return cf.occupancy().total_space();
}, std::plus<int64_t>());
});
@@ -410,7 +410,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

cf::get_cf_all_memtables_live_data_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
warn(unimplemented::cause::INDEXES);
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
return cf.occupancy().used_space();
}, std::plus<int64_t>());
});
@@ -425,7 +425,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_memtable_switch_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats(ctx,req->param["name"] ,&replica::column_family_stats::memtable_switch_count);
+ return get_cf_stats(ctx,req->get_path_param("name") ,&replica::column_family_stats::memtable_switch_count);
});

cf::get_all_memtable_switch_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
@@ -434,7 +434,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

// FIXME: this refers to partitions, not rows.
cf::get_estimated_row_size_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], utils::estimated_histogram(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), utils::estimated_histogram(0), [](replica::column_family& cf) {
utils::estimated_histogram res(0);
for (auto sstables = cf.get_sstables(); auto& i : *sstables) {
res.merge(i->get_stats_metadata().estimated_partition_size);
@@ -446,7 +446,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

// FIXME: this refers to partitions, not rows.
cf::get_estimated_row_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
uint64_t res = 0;
for (auto sstables = cf.get_sstables(); auto& i : *sstables) {
res += i->get_stats_metadata().estimated_partition_size.count();
@@ -457,7 +457,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_estimated_column_count_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], utils::estimated_histogram(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), utils::estimated_histogram(0), [](replica::column_family& cf) {
utils::estimated_histogram res(0);
for (auto sstables = cf.get_sstables(); auto& i : *sstables) {
res.merge(i->get_stats_metadata().estimated_cells_count);
@@ -474,43 +474,43 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_pending_flushes.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats(ctx,req->param["name"] ,&replica::column_family_stats::pending_flushes);
+ return get_cf_stats(ctx,req->get_path_param("name") ,&replica::column_family_stats::pending_flushes);
});

cf::get_all_pending_flushes.set(r, [&ctx] (std::unique_ptr<http::request> req) {
return get_cf_stats(ctx, &replica::column_family_stats::pending_flushes);
});

cf::get_read.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats_count(ctx,req->param["name"] ,&replica::column_family_stats::reads);
+ return get_cf_stats_count(ctx,req->get_path_param("name") ,&replica::column_family_stats::reads);
});

cf::get_all_read.set(r, [&ctx] (std::unique_ptr<http::request> req) {
return get_cf_stats_count(ctx, &replica::column_family_stats::reads);
});

cf::get_write.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats_count(ctx, req->param["name"] ,&replica::column_family_stats::writes);
+ return get_cf_stats_count(ctx, req->get_path_param("name") ,&replica::column_family_stats::writes);
});

cf::get_all_write.set(r, [&ctx] (std::unique_ptr<http::request> req) {
return get_cf_stats_count(ctx, &replica::column_family_stats::writes);
});

cf::get_read_latency_histogram_depricated.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_histogram(ctx, req->param["name"], &replica::column_family_stats::reads);
+ return get_cf_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::reads);
});

cf::get_read_latency_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_rate_and_histogram(ctx, req->param["name"], &replica::column_family_stats::reads);
+ return get_cf_rate_and_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::reads);
});

cf::get_read_latency.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats_sum(ctx,req->param["name"] ,&replica::column_family_stats::reads);
+ return get_cf_stats_sum(ctx,req->get_path_param("name") ,&replica::column_family_stats::reads);
});

cf::get_write_latency.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats_sum(ctx, req->param["name"] ,&replica::column_family_stats::writes);
+ return get_cf_stats_sum(ctx, req->get_path_param("name") ,&replica::column_family_stats::writes);
});

cf::get_all_read_latency_histogram_depricated.set(r, [&ctx] (std::unique_ptr<http::request> req) {
@@ -522,11 +522,11 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_write_latency_histogram_depricated.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_histogram(ctx, req->param["name"], &replica::column_family_stats::writes);
+ return get_cf_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::writes);
});

cf::get_write_latency_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_rate_and_histogram(ctx, req->param["name"], &replica::column_family_stats::writes);
+ return get_cf_rate_and_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::writes);
});

cf::get_all_write_latency_histogram_depricated.set(r, [&ctx] (std::unique_ptr<http::request> req) {
@@ -538,7 +538,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_pending_compactions.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), [](replica::column_family& cf) {
return cf.estimate_pending_compactions();
}, std::plus<int64_t>());
});
@@ -550,27 +550,27 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_live_ss_table_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_stats(ctx, req->param["name"], &replica::column_family_stats::live_sstable_count);
+ return get_cf_stats(ctx, req->get_path_param("name"), &replica::column_family_stats::live_sstable_count);
});

cf::get_all_live_ss_table_count.set(r, [&ctx] (std::unique_ptr<http::request> req) {
return get_cf_stats(ctx, &replica::column_family_stats::live_sstable_count);
});

cf::get_unleveled_sstables.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_unleveled_sstables(ctx, req->param["name"]);
+ return get_cf_unleveled_sstables(ctx, req->get_path_param("name"));
});

cf::get_live_disk_space_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return sum_sstable(ctx, req->param["name"], false);
+ return sum_sstable(ctx, req->get_path_param("name"), false);
});

cf::get_all_live_disk_space_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
return sum_sstable(ctx, false);
});

cf::get_total_disk_space_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return sum_sstable(ctx, req->param["name"], true);
+ return sum_sstable(ctx, req->get_path_param("name"), true);
});

cf::get_all_total_disk_space_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
@@ -579,7 +579,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

// FIXME: this refers to partitions, not rows.
cf::get_min_row_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], INT64_MAX, min_partition_size, min_int64);
+ return map_reduce_cf(ctx, req->get_path_param("name"), INT64_MAX, min_partition_size, min_int64);
});

// FIXME: this refers to partitions, not rows.
@@ -589,7 +589,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

// FIXME: this refers to partitions, not rows.
cf::get_max_row_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], int64_t(0), max_partition_size, max_int64);
+ return map_reduce_cf(ctx, req->get_path_param("name"), int64_t(0), max_partition_size, max_int64);
});

// FIXME: this refers to partitions, not rows.
@@ -600,7 +600,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
// FIXME: this refers to partitions, not rows.
cf::get_mean_row_size.set(r, [&ctx] (std::unique_ptr<http::request> req) {
// Cassandra 3.x mean values are truncated as integrals.
- return map_reduce_cf(ctx, req->param["name"], integral_ratio_holder(), mean_partition_size, std::plus<integral_ratio_holder>());
+ return map_reduce_cf(ctx, req->get_path_param("name"), integral_ratio_holder(), mean_partition_size, std::plus<integral_ratio_holder>());
});

// FIXME: this refers to partitions, not rows.
@@ -610,7 +610,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_bloom_filter_false_positives.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t(0), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t(0), [] (replica::column_family& cf) {
auto sstables = cf.get_sstables();
return std::accumulate(sstables->begin(), sstables->end(), uint64_t(0), [](uint64_t s, auto& sst) {
return s + sst->filter_get_false_positive();
@@ -628,7 +628,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_recent_bloom_filter_false_positives.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t(0), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t(0), [] (replica::column_family& cf) {
auto sstables = cf.get_sstables();
return std::accumulate(sstables->begin(), sstables->end(), uint64_t(0), [](uint64_t s, auto& sst) {
return s + sst->filter_get_recent_false_positive();
@@ -646,7 +646,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_bloom_filter_false_ratio.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], ratio_holder(), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), ratio_holder(), [] (replica::column_family& cf) {
return boost::accumulate(*cf.get_sstables() | boost::adaptors::transformed(filter_false_positive_as_ratio_holder), ratio_holder());
}, std::plus<>());
});
@@ -658,7 +658,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_recent_bloom_filter_false_ratio.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], ratio_holder(), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), ratio_holder(), [] (replica::column_family& cf) {
return boost::accumulate(*cf.get_sstables() | boost::adaptors::transformed(filter_recent_false_positive_as_ratio_holder), ratio_holder());
}, std::plus<>());
});
@@ -670,7 +670,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_bloom_filter_disk_space_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t(0), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t(0), [] (replica::column_family& cf) {
auto sstables = cf.get_sstables();
return std::accumulate(sstables->begin(), sstables->end(), uint64_t(0), [](uint64_t s, auto& sst) {
return s + sst->filter_size();
@@ -688,7 +688,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_bloom_filter_off_heap_memory_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t(0), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t(0), [] (replica::column_family& cf) {
auto sstables = cf.get_sstables();
return std::accumulate(sstables->begin(), sstables->end(), uint64_t(0), [](uint64_t s, auto& sst) {
return s + sst->filter_memory_size();
@@ -706,7 +706,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_index_summary_off_heap_memory_used.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], uint64_t(0), [] (replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), uint64_t(0), [] (replica::column_family& cf) {
auto sstables = cf.get_sstables();
return std::accumulate(sstables->begin(), sstables->end(), uint64_t(0), [](uint64_t s, auto& sst) {
return s + sst->get_summary().memory_footprint();
@@ -729,7 +729,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
// We are missing the off heap memory calculation
// Return 0 is the wrong value. It's a work around
// until the memory calculation will be available
- //auto id = get_uuid(req->param["name"], ctx.db.local());
+ //auto id = get_uuid(req->get_path_param("name"), ctx.db.local());
return make_ready_future<json::json_return_type>(0);
});

@@ -742,7 +742,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
cf::get_speculative_retries.set(r, [] (std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- //auto id = get_uuid(req->param["name"], ctx.db.local());
+ //auto id = get_uuid(req->get_path_param("name"), ctx.db.local());
return make_ready_future<json::json_return_type>(0);
});

@@ -755,7 +755,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
cf::get_key_cache_hit_rate.set(r, [] (std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- //auto id = get_uuid(req->param["name"], ctx.db.local());
+ //auto id = get_uuid(req->get_path_param("name"), ctx.db.local());
return make_ready_future<json::json_return_type>(0);
});

@@ -780,7 +780,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
cf::get_row_cache_hit_out_of_range.set(r, [] (std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- //auto id = get_uuid(req->param["name"], ctx.db.local());
+ //auto id = get_uuid(req->get_path_param("name"), ctx.db.local());
return make_ready_future<json::json_return_type>(0);
});

@@ -791,7 +791,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_row_cache_hit.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf_raw(ctx, req->param["name"], utils::rate_moving_average(), [](const replica::column_family& cf) {
+ return map_reduce_cf_raw(ctx, req->get_path_param("name"), utils::rate_moving_average(), [](const replica::column_family& cf) {
return cf.get_row_cache().stats().hits.rate();
}, std::plus<utils::rate_moving_average>()).then([](const utils::rate_moving_average& m) {
return make_ready_future<json::json_return_type>(meter_to_json(m));
@@ -807,7 +807,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_row_cache_miss.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf_raw(ctx, req->param["name"], utils::rate_moving_average(), [](const replica::column_family& cf) {
+ return map_reduce_cf_raw(ctx, req->get_path_param("name"), utils::rate_moving_average(), [](const replica::column_family& cf) {
return cf.get_row_cache().stats().misses.rate();
}, std::plus<utils::rate_moving_average>()).then([](const utils::rate_moving_average& m) {
return make_ready_future<json::json_return_type>(meter_to_json(m));
@@ -824,57 +824,57 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_cas_prepare.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf_time_histogram(ctx, req->param["name"], [](const replica::column_family& cf) {
+ return map_reduce_cf_time_histogram(ctx, req->get_path_param("name"), [](const replica::column_family& cf) {
return cf.get_stats().cas_prepare.histogram();
});
});

cf::get_cas_propose.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf_time_histogram(ctx, req->param["name"], [](const replica::column_family& cf) {
+ return map_reduce_cf_time_histogram(ctx, req->get_path_param("name"), [](const replica::column_family& cf) {
return cf.get_stats().cas_accept.histogram();
});
});

cf::get_cas_commit.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf_time_histogram(ctx, req->param["name"], [](const replica::column_family& cf) {
+ return map_reduce_cf_time_histogram(ctx, req->get_path_param("name"), [](const replica::column_family& cf) {
return cf.get_stats().cas_learn.histogram();
});
});

cf::get_sstables_per_read_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return map_reduce_cf(ctx, req->param["name"], utils::estimated_histogram(0), [](replica::column_family& cf) {
+ return map_reduce_cf(ctx, req->get_path_param("name"), utils::estimated_histogram(0), [](replica::column_family& cf) {
return cf.get_stats().estimated_sstable_per_read;
},
utils::estimated_histogram_merge, utils_json::estimated_histogram());
});

cf::get_tombstone_scanned_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_histogram(ctx, req->param["name"], &replica::column_family_stats::tombstone_scanned);
+ return get_cf_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::tombstone_scanned);
});

cf::get_live_scanned_histogram.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- return get_cf_histogram(ctx, req->param["name"], &replica::column_family_stats::live_scanned);
+ return get_cf_histogram(ctx, req->get_path_param("name"), &replica::column_family_stats::live_scanned);
});

cf::get_col_update_time_delta_histogram.set(r, [] (std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- //auto id = get_uuid(req->param["name"], ctx.db.local());
+ //auto id = get_uuid(req->get_path_param("name"), ctx.db.local());
std::vector<double> res;
return make_ready_future<json::json_return_type>(res);
});

cf::get_auto_compaction.set(r, [&ctx] (const_req req) {
- auto uuid = get_uuid(req.param["name"], ctx.db.local());
+ auto uuid = get_uuid(req.get_path_param("name"), ctx.db.local());
replica::column_family& cf = ctx.db.local().find_column_family(uuid);
return !cf.is_auto_compaction_disabled_by_user();
});

cf::enable_auto_compaction.set(r, [&ctx](std::unique_ptr<http::request> req) {
- apilog.info("column_family/enable_auto_compaction: name={}", req->param["name"]);
+ apilog.info("column_family/enable_auto_compaction: name={}", req->get_path_param("name"));
return ctx.db.invoke_on(0, [&ctx, req = std::move(req)] (replica::database& db) {
auto g = replica::database::autocompaction_toggle_guard(db);
- return foreach_column_family(ctx, req->param["name"], [](replica::column_family &cf) {
+ return foreach_column_family(ctx, req->get_path_param("name"), [](replica::column_family &cf) {
cf.enable_auto_compaction();
}).then([g = std::move(g)] {
return make_ready_future<json::json_return_type>(json_void());
@@ -883,10 +883,10 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::disable_auto_compaction.set(r, [&ctx](std::unique_ptr<http::request> req) {
- apilog.info("column_family/disable_auto_compaction: name={}", req->param["name"]);
+ apilog.info("column_family/disable_auto_compaction: name={}", req->get_path_param("name"));
return ctx.db.invoke_on(0, [&ctx, req = std::move(req)] (replica::database& db) {
auto g = replica::database::autocompaction_toggle_guard(db);
- return foreach_column_family(ctx, req->param["name"], [](replica::column_family &cf) {
+ return foreach_column_family(ctx, req->get_path_param("name"), [](replica::column_family &cf) {
return cf.disable_auto_compaction();
}).then([g = std::move(g)] {
return make_ready_future<json::json_return_type>(json_void());
@@ -895,31 +895,31 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_tombstone_gc.set(r, [&ctx] (const_req req) {
- auto uuid = get_uuid(req.param["name"], ctx.db.local());
+ auto uuid = get_uuid(req.get_path_param("name"), ctx.db.local());
replica::table& t = ctx.db.local().find_column_family(uuid);
return t.tombstone_gc_enabled();
});

cf::enable_tombstone_gc.set(r, [&ctx](std::unique_ptr<http::request> req) {
- apilog.info("column_family/enable_tombstone_gc: name={}", req->param["name"]);
- return foreach_column_family(ctx, req->param["name"], [](replica::table& t) {
+ apilog.info("column_family/enable_tombstone_gc: name={}", req->get_path_param("name"));
+ return foreach_column_family(ctx, req->get_path_param("name"), [](replica::table& t) {
t.set_tombstone_gc_enabled(true);
}).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
});

cf::disable_tombstone_gc.set(r, [&ctx](std::unique_ptr<http::request> req) {
- apilog.info("column_family/disable_tombstone_gc: name={}", req->param["name"]);
- return foreach_column_family(ctx, req->param["name"], [](replica::table& t) {
+ apilog.info("column_family/disable_tombstone_gc: name={}", req->get_path_param("name"));
+ return foreach_column_family(ctx, req->get_path_param("name"), [](replica::table& t) {
t.set_tombstone_gc_enabled(false);
}).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
});

cf::get_built_indexes.set(r, [&ctx, &sys_ks](std::unique_ptr<http::request> req) {
- auto ks_cf = parse_fully_qualified_cf_name(req->param["name"]);
+ auto ks_cf = parse_fully_qualified_cf_name(req->get_path_param("name"));
auto&& ks = std::get<0>(ks_cf);
auto&& cf_name = std::get<1>(ks_cf);
return sys_ks.local().load_view_build_progress().then([ks, cf_name, &ctx](const std::vector<db::system_keyspace_view_build_progress>& vb) mutable {
@@ -957,7 +957,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_compression_ratio.set(r, [&ctx](std::unique_ptr<http::request> req) {
- auto uuid = get_uuid(req->param["name"], ctx.db.local());
+ auto uuid = get_uuid(req->get_path_param("name"), ctx.db.local());

return ctx.db.map_reduce(sum_ratio<double>(), [uuid](replica::database& db) {
replica::column_family& cf = db.find_column_family(uuid);
@@ -968,29 +968,29 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_read_latency_estimated_histogram.set(r, [&ctx](std::unique_ptr<http::request> req) {
- return map_reduce_cf_time_histogram(ctx, req->param["name"], [](const replica::column_family& cf) {
+ return map_reduce_cf_time_histogram(ctx, req->get_path_param("name"), [](const replica::column_family& cf) {
return cf.get_stats().reads.histogram();
});
});

cf::get_write_latency_estimated_histogram.set(r, [&ctx](std::unique_ptr<http::request> req) {
- return map_reduce_cf_time_histogram(ctx, req->param["name"], [](const replica::column_family& cf) {
+ return map_reduce_cf_time_histogram(ctx, req->get_path_param("name"), [](const replica::column_family& cf) {
return cf.get_stats().writes.histogram();
});
});

cf::set_compaction_strategy_class.set(r, [&ctx](std::unique_ptr<http::request> req) {
sstring strategy = req->get_query_param("class_name");
- apilog.info("column_family/set_compaction_strategy_class: name={} strategy={}", req->param["name"], strategy);
- return foreach_column_family(ctx, req->param["name"], [strategy](replica::column_family& cf) {
+ apilog.info("column_family/set_compaction_strategy_class: name={} strategy={}", req->get_path_param("name"), strategy);
+ return foreach_column_family(ctx, req->get_path_param("name"), [strategy](replica::column_family& cf) {
cf.set_compaction_strategy(sstables::compaction_strategy::type(strategy));
}).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
});

cf::get_compaction_strategy_class.set(r, [&ctx](const_req req) {
- return ctx.db.local().find_column_family(get_uuid(req.param["name"], ctx.db.local())).get_compaction_strategy().name();
+ return ctx.db.local().find_column_family(get_uuid(req.get_path_param("name"), ctx.db.local())).get_compaction_strategy().name();
});

cf::set_compression_parameters.set(r, [](std::unique_ptr<http::request> req) {
@@ -1006,7 +1006,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
});

cf::get_sstable_count_per_level.set(r, [&ctx](std::unique_ptr<http::request> req) {
- return map_reduce_cf_raw(ctx, req->param["name"], std::vector<uint64_t>(), [](const replica::column_family& cf) {
+ return map_reduce_cf_raw(ctx, req->get_path_param("name"), std::vector<uint64_t>(), [](const replica::column_family& cf) {
return cf.sstable_count_per_level();
}, concat_sstable_count_per_level).then([](const std::vector<uint64_t>& res) {
return make_ready_future<json::json_return_type>(res);
@@ -1015,7 +1015,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace

cf::get_sstables_for_key.set(r, [&ctx](std::unique_ptr<http::request> req) {
auto key = req->get_query_param("key");
- auto uuid = get_uuid(req->param["name"], ctx.db.local());
+ auto uuid = get_uuid(req->get_path_param("name"), ctx.db.local());

return ctx.db.map_reduce0([key, uuid] (replica::database& db) -> future<std::unordered_set<sstring>> {
auto sstables = co_await db.find_column_family(uuid).get_sstables_by_partition_key(key);
@@ -1031,7 +1031,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace


cf::toppartitions.set(r, [&ctx] (std::unique_ptr<http::request> req) {
- auto name = req->param["name"];
+ auto name = req->get_path_param("name");
auto [ks, cf] = parse_fully_qualified_cf_name(name);

api::req_param<std::chrono::milliseconds, unsigned> duration{*req, "duration", 1000ms};
@@ -1058,7 +1058,7 @@ void set_column_family(http_context& ctx, routes& r, sharded<db::system_keyspace
}
auto [ks, cf] = parse_fully_qualified_cf_name(*params.get("name"));
auto flush = params.get_as<bool>("flush_memtables").value_or(true);
- apilog.info("column_family/force_major_compaction: name={} flush={}", req->param["name"], flush);
+ apilog.info("column_family/force_major_compaction: name={} flush={}", req->get_path_param("name"), flush);

auto keyspace = validate_keyspace(ctx, ks);
std::vector<table_info> table_infos = {table_info{
diff --git a/api/compaction_manager.cc b/api/compaction_manager.cc
--- a/api/compaction_manager.cc
+++ b/api/compaction_manager.cc
@@ -109,7 +109,7 @@ void set_compaction_manager(http_context& ctx, routes& r) {
});

cm::stop_keyspace_compaction.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto ks_name = validate_keyspace(ctx, req->param);
+ auto ks_name = validate_keyspace(ctx, req);
auto table_names = parse_tables(ks_name, ctx, req->query_parameters, "tables");
if (table_names.empty()) {
table_names = map_keys(ctx.db.local().find_keyspace(ks_name).metadata().get()->cf_meta_data());
diff --git a/api/config.cc b/api/config.cc
--- a/api/config.cc
+++ b/api/config.cc
@@ -91,7 +91,7 @@ void set_config(std::shared_ptr < api_registry_builder20 > rb, http_context& ctx
});

cs::find_config_id.set(r, [&cfg] (const_req r) {
- auto id = r.param["id"];
+ auto id = r.get_path_param("id");
for (auto&& cfg_ref : cfg.values()) {
auto&& cfg = cfg_ref.get();
if (id == cfg.name()) {
diff --git a/api/error_injection.cc b/api/error_injection.cc
--- a/api/error_injection.cc
+++ b/api/error_injection.cc
@@ -24,7 +24,7 @@ namespace hf = httpd::error_injection_json;
void set_error_injection(http_context& ctx, routes& r) {

hf::enable_injection.set(r, [](std::unique_ptr<request> req) {
- sstring injection = req->param["injection"];
+ sstring injection = req->get_path_param("injection");
bool one_shot = req->get_query_param("one_shot") == "True";
auto params = req->content;

@@ -56,7 +56,7 @@ void set_error_injection(http_context& ctx, routes& r) {
});

hf::disable_injection.set(r, [](std::unique_ptr<request> req) {
- sstring injection = req->param["injection"];
+ sstring injection = req->get_path_param("injection");

auto& errinj = utils::get_local_injector();
return errinj.disable_on_all(injection).then([] {
@@ -72,7 +72,7 @@ void set_error_injection(http_context& ctx, routes& r) {
});

hf::message_injection.set(r, [](std::unique_ptr<request> req) {
- sstring injection = req->param["injection"];
+ sstring injection = req->get_path_param("injection");
auto& errinj = utils::get_local_injector();
return errinj.receive_message_on_all(injection).then([] {
return make_ready_future<json::json_return_type>(json::json_void());
diff --git a/api/failure_detector.cc b/api/failure_detector.cc
--- a/api/failure_detector.cc
+++ b/api/failure_detector.cc
@@ -80,9 +80,9 @@ void set_failure_detector(http_context& ctx, routes& r, gms::gossiper& g) {

fd::get_endpoint_state.set(r, [&g] (std::unique_ptr<request> req) {
return g.container().invoke_on(0, [req = std::move(req)] (gms::gossiper& g) {
- auto state = g.get_endpoint_state_ptr(gms::inet_address(req->param["addr"]));
+ auto state = g.get_endpoint_state_ptr(gms::inet_address(req->get_path_param("addr")));
if (!state) {
- return make_ready_future<json::json_return_type>(format("unknown endpoint {}", req->param["addr"]));
+ return make_ready_future<json::json_return_type>(format("unknown endpoint {}", req->get_path_param("addr")));
}
std::stringstream ss;
g.append_endpoint_state(ss, *state);
diff --git a/api/gossiper.cc b/api/gossiper.cc
--- a/api/gossiper.cc
+++ b/api/gossiper.cc
@@ -31,39 +31,39 @@ void set_gossiper(http_context& ctx, routes& r, gms::gossiper& g) {
});

httpd::gossiper_json::get_endpoint_downtime.set(r, [&g] (std::unique_ptr<request> req) -> future<json::json_return_type> {
- gms::inet_address ep(req->param["addr"]);
+ gms::inet_address ep(req->get_path_param("addr"));
// synchronize unreachable_members on all shards
co_await g.get_unreachable_members_synchronized();
co_return g.get_endpoint_downtime(ep);
});

httpd::gossiper_json::get_current_generation_number.set(r, [&g] (std::unique_ptr<http::request> req) {
- gms::inet_address ep(req->param["addr"]);
+ gms::inet_address ep(req->get_path_param("addr"));
return g.get_current_generation_number(ep).then([] (gms::generation_type res) {
return make_ready_future<json::json_return_type>(res.value());
});
});

httpd::gossiper_json::get_current_heart_beat_version.set(r, [&g] (std::unique_ptr<http::request> req) {
- gms::inet_address ep(req->param["addr"]);
+ gms::inet_address ep(req->get_path_param("addr"));
return g.get_current_heart_beat_version(ep).then([] (gms::version_type res) {
return make_ready_future<json::json_return_type>(res.value());
});
});

httpd::gossiper_json::assassinate_endpoint.set(r, [&g](std::unique_ptr<http::request> req) {
if (req->get_query_param("unsafe") != "True") {
- return g.assassinate_endpoint(req->param["addr"]).then([] {
+ return g.assassinate_endpoint(req->get_path_param("addr")).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
}
- return g.unsafe_assassinate_endpoint(req->param["addr"]).then([] {
+ return g.unsafe_assassinate_endpoint(req->get_path_param("addr")).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
});

httpd::gossiper_json::force_remove_endpoint.set(r, [&g](std::unique_ptr<http::request> req) {
- gms::inet_address ep(req->param["addr"]);
+ gms::inet_address ep(req->get_path_param("addr"));
return g.force_remove_endpoint(ep, gms::null_permit_id).then([] {
return make_ready_future<json::json_return_type>(json_void());
});
diff --git a/api/raft.cc b/api/raft.cc
--- a/api/raft.cc
+++ b/api/raft.cc
@@ -24,7 +24,7 @@ using namespace json;

void set_raft(http_context&, httpd::routes& r, sharded<service::raft_group_registry>& raft_gr) {
r::trigger_snapshot.set(r, [&raft_gr] (std::unique_ptr<http::request> req) -> future<json_return_type> {
- raft::group_id gid{utils::UUID{req->param["group_id"]}};
+ raft::group_id gid{utils::UUID{req->get_path_param("group_id")}};
auto timeout_dur = std::invoke([timeout_str = req->get_query_param("timeout")] {
if (timeout_str.empty()) {
return std::chrono::seconds{60};
diff --git a/api/storage_service.cc b/api/storage_service.cc
--- a/api/storage_service.cc
+++ b/api/storage_service.cc
@@ -58,15 +58,19 @@ namespace ss = httpd::storage_service_json;
namespace sp = httpd::storage_proxy_json;
using namespace json;

-sstring validate_keyspace(http_context& ctx, sstring ks_name) {
+sstring validate_keyspace(const http_context& ctx, sstring ks_name) {
if (ctx.db.local().has_keyspace(ks_name)) {
return ks_name;
}
throw bad_param_exception(replica::no_such_keyspace(ks_name).what());
}

-sstring validate_keyspace(http_context& ctx, const parameters& param) {
- return validate_keyspace(ctx, param["keyspace"]);
+sstring validate_keyspace(const http_context& ctx, const std::unique_ptr<http::request>& req) {
+ return validate_keyspace(ctx, req->get_path_param("keyspace"));
+}
+
+sstring validate_keyspace(const http_context& ctx, const http::request& req) {
+ return validate_keyspace(ctx, req.get_path_param("keyspace"));
}

locator::host_id validate_host_id(const sstring& param) {
@@ -171,7 +175,7 @@ using ks_cf_func = std::function<future<json::json_return_type>(http_context&, s

static auto wrap_ks_cf(http_context &ctx, ks_cf_func f) {
return [&ctx, f = std::move(f)](std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto table_infos = parse_table_infos(keyspace, ctx, req->query_parameters, "cf");
return f(ctx, std::move(req), std::move(keyspace), std::move(table_infos));
};
@@ -338,7 +342,7 @@ void set_repair(http_context& ctx, routes& r, sharded<repair_service>& repair) {
// returns immediately, not waiting for the repair to finish. The user
// then has other mechanisms to track the ongoing repair's progress,
// or stop it.
- return repair_start(repair, validate_keyspace(ctx, req->param),
+ return repair_start(repair, validate_keyspace(ctx, req),
options_map).then([] (int i) {
return make_ready_future<json::json_return_type>(i);
});
@@ -421,7 +425,7 @@ void unset_repair(http_context& ctx, routes& r) {

void set_sstables_loader(http_context& ctx, routes& r, sharded<sstables_loader>& sst_loader) {
ss::load_new_ss_tables.set(r, [&ctx, &sst_loader](std::unique_ptr<http::request> req) {
- auto ks = validate_keyspace(ctx, req->param);
+ auto ks = validate_keyspace(ctx, req);
auto cf = req->get_query_param("cf");
auto stream = req->get_query_param("load_and_stream");
auto primary_replica = req->get_query_param("primary_replica_only");
@@ -452,8 +456,8 @@ void unset_sstables_loader(http_context& ctx, routes& r) {

void set_view_builder(http_context& ctx, routes& r, sharded<db::view::view_builder>& vb) {
ss::view_build_statuses.set(r, [&ctx, &vb] (std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
- auto view = req->param["view"];
+ auto keyspace = validate_keyspace(ctx, req);
+ auto view = req->get_path_param("view");
return vb.local().view_build_statuses(std::move(keyspace), std::move(view)).then([] (std::unordered_map<sstring, sstring> status) {
std::vector<storage_service_json::mapper> res;
return make_ready_future<json::json_return_type>(map_to_key_value(std::move(status), res));
@@ -590,7 +594,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::get_range_to_endpoint_map.set(r, [&ctx, &ss](std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
std::vector<ss::maplist_mapper> res;
co_return stream_range_as_array(co_await ss.local().get_range_to_address_map(keyspace),
[](const std::pair<dht::token_range, inet_address_vector_replica_set>& entry){
@@ -615,7 +619,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
ss::get_pending_range_to_endpoint_map.set(r, [&ctx](std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
std::vector<ss::maplist_mapper> res;
return make_ready_future<json::json_return_type>(res);
});
@@ -631,7 +635,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::describe_ring.set(r, [&ctx, &ss](std::unique_ptr<http::request> req) {
- return describe_ring_as_json(ss, validate_keyspace(ctx, req->param));
+ return describe_ring_as_json(ss, validate_keyspace(ctx, req));
});

ss::get_host_id_map.set(r, [&ss](const_req req) {
@@ -664,7 +668,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::get_natural_endpoints.set(r, [&ctx, &ss](const_req req) {
- auto keyspace = validate_keyspace(ctx, req.param);
+ auto keyspace = validate_keyspace(ctx, req);
return container_to_vec(ss.local().get_natural_endpoints(keyspace, req.get_query_param("cf"),
req.get_query_param("key")));
});
@@ -733,7 +737,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_

ss::force_keyspace_cleanup.set(r, [&ctx, &ss](std::unique_ptr<http::request> req) -> future<json::json_return_type> {
auto& db = ctx.db;
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto table_infos = parse_table_infos(keyspace, ctx, req->query_parameters, "cf");
apilog.info("force_keyspace_cleanup: keyspace={} tables={}", keyspace, table_infos);
if (!co_await ss.local().is_cleanup_allowed(keyspace)) {
@@ -796,7 +800,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::force_keyspace_flush.set(r, [&ctx](std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto column_families = parse_tables(keyspace, ctx, req->query_parameters, "cf");
apilog.info("perform_keyspace_flush: keyspace={} tables={}", keyspace, column_families);
auto& db = ctx.db;
@@ -905,7 +909,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
ss::truncate.set(r, [&ctx](std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto column_family = req->get_query_param("cf");
return make_ready_future<json::json_return_type>(json_void());
});
@@ -1039,14 +1043,14 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
ss::bulk_load.set(r, [](std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- auto path = req->param["path"];
+ auto path = req->get_path_param("path");
return make_ready_future<json::json_return_type>(json_void());
});

ss::bulk_load_async.set(r, [](std::unique_ptr<http::request> req) {
//TBD
unimplemented();
- auto path = req->param["path"];
+ auto path = req->get_path_param("path");
return make_ready_future<json::json_return_type>(json_void());
});

@@ -1134,31 +1138,31 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::enable_auto_compaction.set(r, [&ctx](std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto tables = parse_tables(keyspace, ctx, req->query_parameters, "cf");

apilog.info("enable_auto_compaction: keyspace={} tables={}", keyspace, tables);
return set_tables_autocompaction(ctx, keyspace, tables, true);
});

ss::disable_auto_compaction.set(r, [&ctx](std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto tables = parse_tables(keyspace, ctx, req->query_parameters, "cf");

apilog.info("disable_auto_compaction: keyspace={} tables={}", keyspace, tables);
return set_tables_autocompaction(ctx, keyspace, tables, false);
});

ss::enable_tombstone_gc.set(r, [&ctx](std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto tables = parse_tables(keyspace, ctx, req->query_parameters, "cf");

apilog.info("enable_tombstone_gc: keyspace={} tables={}", keyspace, tables);
return set_tables_tombstone_gc(ctx, keyspace, tables, true);
});

ss::disable_tombstone_gc.set(r, [&ctx](std::unique_ptr<http::request> req) {
- auto keyspace = validate_keyspace(ctx, req->param);
+ auto keyspace = validate_keyspace(ctx, req);
auto tables = parse_tables(keyspace, ctx, req->query_parameters, "cf");

apilog.info("disable_tombstone_gc: keyspace={} tables={}", keyspace, tables);
@@ -1254,7 +1258,7 @@ void set_storage_service(http_context& ctx, routes& r, sharded<service::storage_
});

ss::get_effective_ownership.set(r, [&ctx, &ss] (std::unique_ptr<http::request> req) {
- auto keyspace_name = req->param["keyspace"] == "null" ? "" : validate_keyspace(ctx, req->param);
+ auto keyspace_name = req->get_path_param("keyspace") == "null" ? "" : validate_keyspace(ctx, req);
return ss.local().effective_ownership(keyspace_name).then([] (auto&& ownership) {
std::vector<storage_service_json::mapper> res;
return make_ready_future<json::json_return_type>(map_to_key_value(ownership, res));
diff --git a/api/storage_service.hh b/api/storage_service.hh
--- a/api/storage_service.hh
+++ b/api/storage_service.hh
@@ -37,11 +37,11 @@ namespace api {

// verify that the keyspace is found, otherwise a bad_param_exception exception is thrown
// containing the description of the respective keyspace error.
-sstring validate_keyspace(http_context& ctx, sstring ks_name);
+sstring validate_keyspace(const http_context& ctx, sstring ks_name);

// verify that the keyspace parameter is found, otherwise a bad_param_exception exception is thrown
// containing the description of the respective keyspace error.
-sstring validate_keyspace(http_context& ctx, const httpd::parameters& param);
+sstring validate_keyspace(const http_context& ctx, const std::unique_ptr<http::request>& req);

// splits a request parameter assumed to hold a comma-separated list of table names
// verify that the tables are found, otherwise a bad_param_exception exception is thrown
diff --git a/api/stream_manager.cc b/api/stream_manager.cc
--- a/api/stream_manager.cc
+++ b/api/stream_manager.cc
@@ -106,7 +106,7 @@ void set_stream_manager(http_context& ctx, routes& r, sharded<streaming::stream_
});

hs::get_total_incoming_bytes.set(r, [&sm](std::unique_ptr<request> req) {
- gms::inet_address peer(req->param["peer"]);
+ gms::inet_address peer(req->get_path_param("peer"));
return sm.map_reduce0([peer](streaming::stream_manager& sm) {
return sm.get_progress_on_all_shards(peer).then([] (auto sbytes) {
return sbytes.bytes_received;
@@ -127,7 +127,7 @@ void set_stream_manager(http_context& ctx, routes& r, sharded<streaming::stream_
});

hs::get_total_outgoing_bytes.set(r, [&sm](std::unique_ptr<request> req) {
- gms::inet_address peer(req->param["peer"]);
+ gms::inet_address peer(req->get_path_param("peer"));
return sm.map_reduce0([peer] (streaming::stream_manager& sm) {
return sm.get_progress_on_all_shards(peer).then([] (auto sbytes) {
return sbytes.bytes_sent;
diff --git a/api/system.cc b/api/system.cc
--- a/api/system.cc
+++ b/api/system.cc
@@ -119,9 +119,9 @@ void set_system(http_context& ctx, routes& r) {

hs::get_logger_level.set(r, [](const_req req) {
try {
- return logging::level_name(logging::logger_registry().get_logger_level(req.param["name"]));
+ return logging::level_name(logging::logger_registry().get_logger_level(req.get_path_param("name")));
} catch (std::out_of_range& e) {
- throw bad_param_exception("Unknown logger name " + req.param["name"]);
+ throw bad_param_exception("Unknown logger name " + req.get_path_param("name"));
}
// just to keep the compiler happy
return sstring();
@@ -130,9 +130,9 @@ void set_system(http_context& ctx, routes& r) {
hs::set_logger_level.set(r, [](const_req req) {
try {
logging::log_level level = boost::lexical_cast<logging::log_level>(std::string(req.get_query_param("level")));
- logging::logger_registry().set_logger_level(req.param["name"], level);
+ logging::logger_registry().set_logger_level(req.get_path_param("name"), level);
} catch (std::out_of_range& e) {
- throw bad_param_exception("Unknown logger name " + req.param["name"]);
+ throw bad_param_exception("Unknown logger name " + req.get_path_param("name"));
} catch (boost::bad_lexical_cast& e) {
throw bad_param_exception("Unknown logging level " + req.get_query_param("level"));
}
diff --git a/api/task_manager.cc b/api/task_manager.cc
--- a/api/task_manager.cc
+++ b/api/task_manager.cc
@@ -124,7 +124,7 @@ void set_task_manager(http_context& ctx, routes& r, db::config& cfg) {
chunked_stats local_res;
tasks::task_manager::module_ptr module;
try {
- module = tm.find_module(req->param["module"]);
+ module = tm.find_module(req->get_path_param("module"));
} catch (...) {
throw bad_param_exception(fmt::format("{}", std::current_exception()));
}
@@ -157,7 +157,7 @@ void set_task_manager(http_context& ctx, routes& r, db::config& cfg) {
});

tm::get_task_status.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto id = tasks::task_id{utils::UUID{req->param["task_id"]}};
+ auto id = tasks::task_id{utils::UUID{req->get_path_param("task_id")}};
tasks::task_manager::foreign_task_ptr task;
try {
task = co_await tasks::task_manager::invoke_on_task(ctx.tm, id, std::function([] (tasks::task_manager::task_ptr task) -> future<tasks::task_manager::foreign_task_ptr> {
@@ -174,7 +174,7 @@ void set_task_manager(http_context& ctx, routes& r, db::config& cfg) {
});

tm::abort_task.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto id = tasks::task_id{utils::UUID{req->param["task_id"]}};
+ auto id = tasks::task_id{utils::UUID{req->get_path_param("task_id")}};
try {
co_await tasks::task_manager::invoke_on_task(ctx.tm, id, [] (tasks::task_manager::task_ptr task) -> future<> {
if (!task->is_abortable()) {
@@ -189,7 +189,7 @@ void set_task_manager(http_context& ctx, routes& r, db::config& cfg) {
});

tm::wait_task.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto id = tasks::task_id{utils::UUID{req->param["task_id"]}};
+ auto id = tasks::task_id{utils::UUID{req->get_path_param("task_id")}};
tasks::task_manager::foreign_task_ptr task;
try {
task = co_await tasks::task_manager::invoke_on_task(ctx.tm, id, std::function([] (tasks::task_manager::task_ptr task) {
@@ -210,7 +210,7 @@ void set_task_manager(http_context& ctx, routes& r, db::config& cfg) {

tm::get_task_status_recursively.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
auto& _ctx = ctx;
- auto id = tasks::task_id{utils::UUID{req->param["task_id"]}};
+ auto id = tasks::task_id{utils::UUID{req->get_path_param("task_id")}};
std::queue<tasks::task_manager::foreign_task_ptr> q;
utils::chunked_vector<full_task_status> res;

diff --git a/api/task_manager_test.cc b/api/task_manager_test.cc
--- a/api/task_manager_test.cc
+++ b/api/task_manager_test.cc
@@ -83,7 +83,7 @@ void set_task_manager_test(http_context& ctx, routes& r) {
});

tmt::finish_test_task.set(r, [&ctx] (std::unique_ptr<http::request> req) -> future<json::json_return_type> {
- auto id = tasks::task_id{utils::UUID{req->param["task_id"]}};
+ auto id = tasks::task_id{utils::UUID{req->get_path_param("task_id")}};
auto it = req->query_parameters.find("error");
bool fail = it != req->query_parameters.end();
std::string error = fail ? it->second : "";
diff --git a/test/alternator/test_lsi.py b/test/alternator/test_lsi.py
--- a/test/alternator/test_lsi.py
+++ b/test/alternator/test_lsi.py
@@ -10,6 +10,7 @@

import pytest
import time
+import requests
from botocore.exceptions import ClientError
from util import create_test_table, new_test_table, random_string, full_scan, full_query, multiset, list_tables

@@ -538,3 +539,35 @@ def test_lsi_and_gsi_same_name(dynamodb):
}
])
table.delete()
+
+# Test that the LSI table can be addressed in Scylla's REST API (obviously,
+# since this test is for the REST API, it is Scylla-only and can't be run on
+# DynamoDB).
+# At the time this test was written, the LSI's name has a "!" in it, so this
+# test reproduces a bug in URL decoding (#5883). But the goal of this test
+# isn't to insist that a table backing an LSI must have a specific name,
+# but rather that whatever name it does have - it can be addressed.
+def test_lsi_name_rest_api(test_table_lsi_1, rest_api):
+ # See that the LSI is listed in list of tables. It will be a table
+ # whose CQL name contains the Alternator table's name, and the
+ # LSI's name ('hello'). As of this writing, it will actually be
+ # alternator_<name>:<name>!:<lsi> - but the test doesn't enshrine this.
+ resp = requests.get(f'{rest_api}/column_family/name')
+ resp.raise_for_status()
+ lsi_rest_name = None
+ for name in resp.json():
+ if test_table_lsi_1.name in name and 'hello' in name:
+ lsi_rest_name = name
+ break
+ assert lsi_rest_name
+ # Attempt to run a request on this LSI's table name "lsi_rest_name".
+ # We'll use the compaction_strategy request here, but if for some
+ # reason in the future we decide to drop that request, any other
+ # request will be fine.
+ resp = requests.get(f'{rest_api}/column_family/compaction_strategy/{lsi_rest_name}')
+ resp.raise_for_status()
+ # Let's make things difficult for the server by URL encoding the
+ # lsi_rest_name - exposing issue #5883.
+ encoded_lsi_rest_name = requests.utils.quote(lsi_rest_name)
+ resp = requests.get(f'{rest_api}/column_family/compaction_strategy/{encoded_lsi_rest_name}')
+ resp.raise_for_status()

[Commit Bot]

From: Botond Dénes <bde...@scylladb.com>
Committer: Avi Kivity <a...@scylladb.com>

Branch: branch-5.4