diff --git a/src/agents/command_router/BusCommandRouterProcessor.cc b/src/agents/command_router/BusCommandRouterProcessor.cc index be00039d..800df81d 100644 --- a/src/agents/command_router/BusCommandRouterProcessor.cc +++ b/src/agents/command_router/BusCommandRouterProcessor.cc @@ -63,7 +63,6 @@ void BusCommandRouterProcessor::dispatch_http_command( caller_proxy->issued = true; caller_proxy->requestor_id = http_requestor_id; - caller_proxy->parameters = parameters_for_peer(http_requestor_id); caller_proxy->serial = serial; caller_proxy->proxy_port = PortPool::get_port(); if (caller_proxy->proxy_port == 0) { @@ -86,8 +85,9 @@ void BusCommandRouterProcessor::dispatch_http_command( processor_proxy->setup_proxy_node(processor_proxy_node_id, caller_proxy->my_id()); processor_proxy->command = std::move(caller_proxy->command); processor_proxy->args = std::move(caller_proxy->args); + processor_proxy->parameters = caller_proxy->parameters; - this->run_command(processor_proxy); + this->run_command_internal(processor_proxy, false); } void BusCommandRouterProcessor::run_command(shared_ptr proxy) { @@ -96,7 +96,14 @@ void BusCommandRouterProcessor::run_command(shared_ptr proxy) { proxy->raise_error_on_peer("Invalid proxy type for BUS_COMMAND_ROUTER"); return; } - router_proxy->parameters = parameters_for_peer(router_proxy->get_requestor_id()); + this->run_command_internal(router_proxy, true); +} + +void BusCommandRouterProcessor::run_command_internal(shared_ptr router_proxy, + bool load_peer_parameters) { + if (load_peer_parameters) { + router_proxy->parameters = parameters_for_peer(router_proxy->get_requestor_id()); + } try { if (router_proxy->get_args().size() < 2) { RAISE_ERROR("Invalid bus_command_router args: expected {COMMAND, ARG}"); diff --git a/src/agents/command_router/BusCommandRouterProcessor.h b/src/agents/command_router/BusCommandRouterProcessor.h index f8b8b33f..57f228fc 100644 --- a/src/agents/command_router/BusCommandRouterProcessor.h +++ b/src/agents/command_router/BusCommandRouterProcessor.h @@ -36,6 +36,8 @@ class BusCommandRouterProcessor : public BusCommandProcessor { const string& http_requestor_id); private: + void run_command_internal(shared_ptr router_proxy, bool load_peer_parameters); + void handle_get(shared_ptr proxy, const string& arg); void handle_set(shared_ptr proxy, const string& arg); void handle_query(shared_ptr proxy, const string& arg); diff --git a/src/agents/command_router/http_api/BUILD b/src/agents/command_router/http_api/BUILD index 9758010d..5cd65478 100644 --- a/src/agents/command_router/http_api/BUILD +++ b/src/agents/command_router/http_api/BUILD @@ -24,6 +24,30 @@ cc_library( ], ) +cc_library( + name = "proxy_parameters_from_json", + srcs = ["ProxyParametersFromJson.cc"], + hdrs = ["ProxyParametersFromJson.h"], + includes = ["."], + deps = [ + "//commons:commons_lib", + "@nlohmann_json//:json", + ], +) + +cc_library( + name = "http_command_proxy_factory", + srcs = ["HttpCommandProxyFactory.cc"], + hdrs = ["HttpCommandProxyFactory.h"], + includes = ["."], + deps = [ + ":proxy_parameters_from_json", + "//agents/command_router:bus_command_router_proxy", + "//commons:commons_lib", + "@nlohmann_json//:json", + ], +) + cc_library( name = "command_router_http_api_config", srcs = ["CommandRouterHttpAPIConfig.cc"], @@ -44,6 +68,7 @@ cc_library( ":bus_command_router_proxy_stream_poller", ":command_execution", ":command_router_http_api_config", + ":http_command_proxy_factory", "//agents/command_router:bus_command_router_processor", "//agents/command_router:bus_command_router_proxy", "//commons:commons_lib", diff --git a/src/agents/command_router/http_api/CommandExecution.cc b/src/agents/command_router/http_api/CommandExecution.cc index d1f48e5d..893c2c24 100644 --- a/src/agents/command_router/http_api/CommandExecution.cc +++ b/src/agents/command_router/http_api/CommandExecution.cc @@ -5,13 +5,10 @@ using namespace commons; using namespace command_router; CommandExecution::CommandExecution(const string& execution_id, - const string& command_type, - const string& command_text, + const string& command, + const json& params, size_t max_events) - : execution_id(execution_id), - command_type(command_type), - command_text(command_text), - max_events(max_events) { + : execution_id(execution_id), command(command), params(params), max_events(max_events) { if (this->max_events == 0) { RAISE_ERROR("max_events must be greater than 0"); } @@ -125,7 +122,11 @@ void CommandExecution::publish_event_locked(const json& payload) { this->cv_.notify_all(); } -json CommandExecution::lifecycle_event_locked() const { +json CommandExecution::make_envelope_locked(const string& command, json params) const { + return {{"command", command}, {"params", std::move(params)}}; +} + +json CommandExecution::status_params_locked() const { return {{"execution_id", this->execution_id}, {"status", status_to_string(this->status_)}}; } @@ -138,7 +139,8 @@ void CommandExecution::stamp_finished_at_locked() { void CommandExecution::mark_running() { lock_guard lock(this->mtx_); this->status_ = ExecutionStatus::RUNNING; - this->publish_event_locked(this->lifecycle_event_locked()); + this->publish_event_locked( + this->make_envelope_locked(COMMAND_EXECUTION_STATUS, this->status_params_locked())); } void CommandExecution::publish_chunk(int seq, const json& data) { @@ -147,11 +149,11 @@ void CommandExecution::publish_chunk(int seq, const json& data) { RAISE_ERROR("Chunk data must be a JSON array"); } this->received_count_ += static_cast(data.size()); - this->publish_event_locked({{"execution_id", this->execution_id}, - {"type", "chunk"}, - {"seq", seq}, - {"data", data}, - {"received_count", this->received_count_}}); + json params = {{"execution_id", this->execution_id}, + {"seq", seq}, + {"answers", data}, + {"received_count", this->received_count_}}; + this->publish_event_locked(this->make_envelope_locked(COMMAND_QUERY_ANSWERS, std::move(params))); } void CommandExecution::mark_completed(unsigned long duration_ms, int total_items) { @@ -160,10 +162,10 @@ void CommandExecution::mark_completed(unsigned long duration_ms, int total_items this->total_items_ = total_items; this->status_ = ExecutionStatus::COMPLETED; this->stamp_finished_at_locked(); - auto event = this->lifecycle_event_locked(); - event["duration_ms"] = duration_ms; - event["total_items"] = total_items; - this->publish_event_locked(event); + json params = this->status_params_locked(); + params["duration_ms"] = duration_ms; + params["total_items"] = total_items; + this->publish_event_locked(this->make_envelope_locked(COMMAND_EXECUTION_STATUS, std::move(params))); } void CommandExecution::mark_error(const string& message) { @@ -171,16 +173,17 @@ void CommandExecution::mark_error(const string& message) { this->error_message_ = message; this->status_ = ExecutionStatus::ERROR; this->stamp_finished_at_locked(); - auto event = this->lifecycle_event_locked(); - event["message"] = message; - this->publish_event_locked(event); + json params = this->status_params_locked(); + params["message"] = message; + this->publish_event_locked(this->make_envelope_locked(COMMAND_EXECUTION_STATUS, std::move(params))); } void CommandExecution::mark_aborted() { lock_guard lock(this->mtx_); this->status_ = ExecutionStatus::ABORTED; this->stamp_finished_at_locked(); - this->publish_event_locked(this->lifecycle_event_locked()); + this->publish_event_locked( + this->make_envelope_locked(COMMAND_EXECUTION_STATUS, this->status_params_locked())); } void CommandExecution::mark_error_unless_terminal(const string& message) { @@ -191,9 +194,9 @@ void CommandExecution::mark_error_unless_terminal(const string& message) { this->error_message_ = message; this->status_ = ExecutionStatus::ERROR; this->stamp_finished_at_locked(); - auto event = this->lifecycle_event_locked(); - event["message"] = message; - this->publish_event_locked(event); + json params = this->status_params_locked(); + params["message"] = message; + this->publish_event_locked(this->make_envelope_locked(COMMAND_EXECUTION_STATUS, std::move(params))); } void CommandExecution::mark_aborted_unless_terminal() { @@ -203,5 +206,6 @@ void CommandExecution::mark_aborted_unless_terminal() { } this->status_ = ExecutionStatus::ABORTED; this->stamp_finished_at_locked(); - this->publish_event_locked(this->lifecycle_event_locked()); + this->publish_event_locked( + this->make_envelope_locked(COMMAND_EXECUTION_STATUS, this->status_params_locked())); } diff --git a/src/agents/command_router/http_api/CommandExecution.h b/src/agents/command_router/http_api/CommandExecution.h index cfb8c038..1f1128d5 100644 --- a/src/agents/command_router/http_api/CommandExecution.h +++ b/src/agents/command_router/http_api/CommandExecution.h @@ -22,8 +22,8 @@ enum ExecutionStatus { PENDING, RUNNING, COMPLETED, ERROR, ABORTED }; * @brief In-memory state for one asynchronous command run. * * Holds status, progress counters, and a JSON event log (events) consumed by - * GET /executions/{id} and WebSocket replay. Status transitions always emit a - * lifecycle event so clients can rely on status in the stream. + * GET /executions/{id} and WebSocket replay. Stream events use the same envelope + * as HTTP requests: { "command": ..., "params": ... }. * * Thread-safe: callers do not need to lock mtx; public methods synchronize internally. */ @@ -31,9 +31,13 @@ class CommandExecution { public: static constexpr size_t DEFAULT_MAX_EVENTS = 10000; + /** WebSocket / stream command names (same envelope as HTTP requests). */ + static constexpr const char* COMMAND_QUERY_ANSWERS = "query_answers"; + static constexpr const char* COMMAND_EXECUTION_STATUS = "execution_status"; + CommandExecution(const string& execution_id, - const string& command_type, - const string& command_text, + const string& command, + const json& params, size_t max_events = DEFAULT_MAX_EVENTS); ~CommandExecution() = default; @@ -44,8 +48,8 @@ class CommandExecution { static bool is_terminal(ExecutionStatus status); string execution_id; - string command_type; - string command_text; + string command; + json params; size_t max_events; ExecutionStatus status() const; @@ -75,19 +79,19 @@ class CommandExecution { /** @brief True when terminal and finished_at_ms is older than retention_ms. */ bool is_retention_expired(unsigned long now_ms, unsigned long retention_ms) const; - /** @brief Append a chunk event and update received_count. @p data must be a JSON array. */ + /** @brief Append a query_answers event and update received_count. @p data must be a JSON array. */ void publish_chunk(int seq, const json& data); - /** @brief PENDING -> RUNNING; emits a lifecycle event. */ + /** @brief PENDING -> RUNNING; emits an execution_status event. */ void mark_running(); - /** @brief -> COMPLETED; sets duration/totals and emits a lifecycle event. */ + /** @brief -> COMPLETED; sets duration/totals and emits an execution_status event. */ void mark_completed(unsigned long duration_ms, int total_items); - /** @brief -> ERROR; sets error_message and emits a lifecycle event. */ + /** @brief -> ERROR; sets error_message and emits an execution_status event. */ void mark_error(const string& message); - /** @brief -> ABORTED; emits a lifecycle event. */ + /** @brief -> ABORTED; emits an execution_status event. */ void mark_aborted(); void mark_error_unless_terminal(const string& message); @@ -108,7 +112,8 @@ class CommandExecution { void publish_event_locked(const json& payload); void stamp_finished_at_locked(); - json lifecycle_event_locked() const; + json make_envelope_locked(const string& command, json params) const; + json status_params_locked() const; }; } // namespace command_router diff --git a/src/agents/command_router/http_api/CommandRouterHttpAPI.cc b/src/agents/command_router/http_api/CommandRouterHttpAPI.cc index f2b7c77d..8b95ac75 100644 --- a/src/agents/command_router/http_api/CommandRouterHttpAPI.cc +++ b/src/agents/command_router/http_api/CommandRouterHttpAPI.cc @@ -1,12 +1,9 @@ #include "CommandRouterHttpAPI.h" -#include -#include -#include - #include "BusCommandRouterProcessor.h" #include "BusCommandRouterProxy.h" #include "BusCommandRouterProxyStreamPoller.h" +#include "HttpCommandProxyFactory.h" #define LOG_LEVEL INFO_LEVEL #include "Logger.h" @@ -20,8 +17,7 @@ using namespace agents; using json = nlohmann::json; -const unordered_set CommandRouterHttpAPI::VALID_COMMAND_TYPES = { - "query", "evolution", "get", "set"}; +const unordered_set CommandRouterHttpAPI::VALID_COMMAND = {HttpCommandProxyFactory::QUERY}; // ------------------------------------------------------------------------------------------------- // Constructors, destructors @@ -82,10 +78,6 @@ void CommandRouterHttpAPI::setup() { void CommandRouterHttpAPI::stop() { this->shutting_down = true; - { - lock_guard semaphore(this->executions_mtx); - this->execution_slots_cv.notify_all(); - } LOG_INFO("CommandRouter HTTP API stopping on " << this->host << ":" << this->port); this->server.stop(); this->cleanup_finished_executions(); @@ -125,87 +117,34 @@ void CommandRouterHttpAPI::setup_routes() { return; } - if (!body.is_object() || !body.contains("command_type") || - !body["command_type"].is_string() || !body.contains("command_text") || - !body["command_text"].is_string()) { - this->set_json_response( - response, 400, {{"error", "Missing fields: command_type, command_text"}}); + if (!body.is_object() || !body.contains("command") || !body["command"].is_string() || + body["command"].get_ref().empty() || !body.contains("params") || + !body["params"].is_object()) { + this->set_json_response(response, 400, {{"error", "Missing fields: command, params"}}); return; } - string command_type = body["command_type"].get(); - string command_text = body["command_text"].get(); - - if (command_type.empty() || command_text.empty()) { - this->set_json_response( - response, 400, {{"error", "Missing fields: command_type, command_text"}}); - return; - } + const string command = body["command"].get(); + const json params = body["params"]; - if (!this->is_valid_command_type(command_type)) { + if (!this->is_valid_command(command)) { this->set_json_response( - response, - 400, - {{"error", "Invalid command_type. Allowed values: query, evolution, get, set"}}); + response, 400, {{"error", "Invalid command. Allowed values: query"}}); return; } - if (this->is_sync_command_type(command_type)) { - LOG_INFO("CommandRouter HTTP API sync execution type=" << command_type); - - json results = json::array(); - string error_message; - const PollStreamResult poll_result = this->execute_router_command( - command_type, - command_text, - nullptr, - [&](const json& chunk) { - for (const auto& item : chunk) { - results.push_back(item); - } - }, - [&](const string& message) { error_message = message; }, - nullptr); - if (!poll_result.ok) { - if (error_message.empty()) { - error_message = "Command failed"; - } - this->set_json_response(response, 500, {{"error", error_message}}); - return; - } - - if (results.empty()) { - this->set_json_response( - response, 500, {{"error", "Command finished without a response"}}); - return; - } + auto exec = make_shared( + this->generate_execution_id(), command, params, this->settings.max_events_per_execution); - this->set_json_response( - response, 200, {{"command_type", command_type}, {"result", results.front()}}); + if (this->try_admit_execution(exec) == AdmitResult::QueueFull) { + this->set_json_response(response, 503, {{"error", "Execution queue is full"}}); return; } - auto exec = make_shared(this->generate_execution_id(), - command_type, - command_text, - this->settings.max_events_per_execution); - - switch (this->try_admit_execution(exec)) { - case AdmitResult::ConcurrentLimit: - this->set_json_response( - response, 429, {{"error", "Maximum concurrent executions reached"}}); - return; - case AdmitResult::QueueFull: - this->set_json_response(response, 503, {{"error", "Execution queue is full"}}); - return; - case AdmitResult::Admitted: - break; - } - string status_for_response = exec->status_string(); LOG_INFO("CommandRouter HTTP API execution scheduled id=" << exec->execution_id - << " type=" << command_type); + << " command=" << command); try { this->thread_pool->enqueue([this, exec]() { this->run_execution(exec); }); @@ -349,7 +288,7 @@ void CommandRouterHttpAPI::run_execution_inner(const shared_ptrexecute_router_command( - exec->command_type, exec->command_text, should_abort, on_chunk, on_error, on_aborted); + exec->command, exec->params, should_abort, on_chunk, on_error, on_aborted); if (!poll_result.ok) { return; } @@ -361,13 +300,9 @@ void CommandRouterHttpAPI::run_execution_inner(const shared_ptr& should_abort, const function& on_chunk, const function& on_error, @@ -380,14 +315,19 @@ PollStreamResult CommandRouterHttpAPI::execute_router_command( } try { - string router_arg = command_text; - Utils::replace_all(router_arg, "%", "$"); - auto router_proxy = make_shared(command_type, router_arg); + string create_error; + auto router_proxy = HttpCommandProxyFactory::create(command, params, create_error); + if (router_proxy == nullptr) { + if (on_error) { + on_error(create_error); + } + return {}; + } this->router_processor->dispatch_http_command(router_proxy, this->http_requestor_id); return BusCommandRouterProxyStreamPoller::poll_stream(router_proxy, - command_type, + command, this->settings.stream_items_per_chunk, should_abort, on_chunk, @@ -403,46 +343,21 @@ PollStreamResult CommandRouterHttpAPI::execute_router_command( } void CommandRouterHttpAPI::run_execution(const shared_ptr& exec) { - bool acquired_running_slot = false; - - { - unique_lock lock(this->executions_mtx); - - // Block until a concurrent slot is free or the server is shutting down. - this->execution_slots_cv.wait(lock, [&] { - return this->shutting_down.load() || - this->running_executions < this->settings.max_concurrent_executions; - }); - - this->pending_executions--; - - if (!this->shutting_down.load()) { - if (!exec->is_cancel_requested()) { - this->running_executions++; - acquired_running_slot = true; - } - } - } - if (this->shutting_down.load()) { exec->mark_error_unless_terminal("Server is shutting down"); + this->release_pending_execution(); return; } - if (!acquired_running_slot) { + if (exec->is_cancel_requested()) { exec->mark_aborted_unless_terminal(); + this->release_pending_execution(); return; } exec->mark_running(); - this->run_execution_inner(exec); - - { - lock_guard lock(this->executions_mtx); - this->running_executions--; - } - this->execution_slots_cv.notify_one(); + this->release_pending_execution(); } void CommandRouterHttpAPI::cleanup_finished_executions() { @@ -471,16 +386,13 @@ string CommandRouterHttpAPI::generate_execution_id() { return execution_id; } -bool CommandRouterHttpAPI::is_valid_command_type(const string& command_type) const { - return this->VALID_COMMAND_TYPES.find(command_type) != this->VALID_COMMAND_TYPES.end(); +bool CommandRouterHttpAPI::is_valid_command(const string& command) const { + return this->VALID_COMMAND.find(command) != this->VALID_COMMAND.end(); } CommandRouterHttpAPI::AdmitResult CommandRouterHttpAPI::try_admit_execution( const shared_ptr& exec) { lock_guard semaphore(this->executions_mtx); - if (this->running_executions >= this->settings.max_concurrent_executions) { - return AdmitResult::ConcurrentLimit; - } if (this->settings.max_queued_executions > 0 && this->pending_executions >= this->settings.max_queued_executions) { return AdmitResult::QueueFull; @@ -490,6 +402,13 @@ CommandRouterHttpAPI::AdmitResult CommandRouterHttpAPI::try_admit_execution( return AdmitResult::Admitted; } +void CommandRouterHttpAPI::release_pending_execution() { + lock_guard lock(this->executions_mtx); + if (this->pending_executions > 0) { + this->pending_executions--; + } +} + void CommandRouterHttpAPI::set_json_response(httplib::Response& response, int status, const json& body) { response.status = status; string content = body.dump(); diff --git a/src/agents/command_router/http_api/CommandRouterHttpAPI.h b/src/agents/command_router/http_api/CommandRouterHttpAPI.h index f7ff3ec2..32ab3c24 100644 --- a/src/agents/command_router/http_api/CommandRouterHttpAPI.h +++ b/src/agents/command_router/http_api/CommandRouterHttpAPI.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include @@ -32,14 +31,14 @@ class BusCommandRouterProcessor; * @brief HTTP + WebSocket server for command execution. * * Routes: - * POST /command-router/executions — run get/set synchronously, or schedule query/evolution + * POST /command-router/executions — schedule a command ({command, params}) * GET /command-router/executions/{id} — poll status * POST /command-router/executions/{id}/cancel — request cancel * WS /command-router/ws/{id} — stream JSON events * - * Command execution uses the registered BusCommandRouterProcessor via in-process proxy - * dispatch (dispatch_http_command). All HTTP requests share one router parameter store - * (http_requestor_id), matching how busclient reuses the same endpoint across commands. + * The HTTP layer only unpacks the top-level envelope {command, params}. Command-specific + * interpretation of params is delegated to the matching command handler (e.g. query). + * WebSocket stream events use the same {command, params} envelope. * * Runs on a DedicatedThread: thread_one_step() blocks in listen() until stop(). * Each accepted command is enqueued on thread_pool so the listener stays free. @@ -47,14 +46,14 @@ class BusCommandRouterProcessor; */ class CommandRouterHttpAPI : public processor::Processor, public processor::ThreadMethod { public: - static const unordered_set VALID_COMMAND_TYPES; + static const unordered_set VALID_COMMAND; /** * @brief Construct a CommandRouterHttpAPI. * @param host Hostname to bind. * @param port Port to bind. * @param thread_pool Runs command work off the HTTP listener thread. - * @param settings Concurrency, queue, event-buffer, and retention limits. + * @param settings Queue, event-buffer, and retention limits. */ CommandRouterHttpAPI(const string& host, int port, @@ -94,8 +93,6 @@ class CommandRouterHttpAPI : public processor::Processor, public processor::Thre atomic shutting_down{false}; unordered_map> executions; mutex executions_mtx; - condition_variable execution_slots_cv; - size_t running_executions = 0; size_t pending_executions = 0; /** @brief Register all /command-router/ HTTP and WebSocket handlers. */ @@ -106,31 +103,32 @@ class CommandRouterHttpAPI : public processor::Processor, public processor::Thre /** @brief Thread-pool entry point; catches errors and marks the execution failed. */ void run_execution(const shared_ptr& exec); - /** @brief Run command_type/command_text and publish chunk/lifecycle events. */ + /** @brief Run command/params and publish chunk/lifecycle events. */ void run_execution_inner(const shared_ptr& exec); /** @brief Dispatch a router command and poll its response stream. */ - PollStreamResult execute_router_command(const string& command_type, - const string& command_text, + PollStreamResult execute_router_command(const string& command, + const json& params, const function& should_abort, const function& on_chunk, const function& on_error, const function& on_aborted); - static bool is_sync_command_type(const string& command_type); - /** @brief Remove finished executions from the executions map. */ void cleanup_finished_executions(); string generate_execution_id(); - /** @brief Check if the command_type is valid. (Allowed values: query, evolution, get, set) */ - bool is_valid_command_type(const string& command_type) const; + /** @brief Check if the command is valid. (Allowed values: query) */ + bool is_valid_command(const string& command) const; - /** @brief Check limits and register a pending execution. */ - enum class AdmitResult { Admitted, ConcurrentLimit, QueueFull }; + /** @brief Check queue limit and register a pending execution. */ + enum class AdmitResult { Admitted, QueueFull }; AdmitResult try_admit_execution(const shared_ptr& exec); + /** @brief Decrement pending_executions after an execution leaves the in-flight set. */ + void release_pending_execution(); + /** @brief Set response status and JSON body. */ void set_json_response(httplib::Response& res, int status_code, const json& payload); }; diff --git a/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.cc b/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.cc index d3cfec73..685f41b5 100644 --- a/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.cc +++ b/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.cc @@ -26,9 +26,6 @@ static pair parse_host_port(const string& endpoint) { static HttpAPISettings load_http_api_settings(const JsonConfig& command_router_config) { HttpAPISettings settings; - settings.max_concurrent_executions = - command_router_config.at_path("http_api.max_concurrent_executions") - .get_or(settings.max_concurrent_executions); settings.max_queued_executions = command_router_config.at_path("http_api.max_queued_executions") .get_or(settings.max_queued_executions); settings.max_events_per_execution = diff --git a/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.h b/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.h index f4a8871a..6ccfb257 100644 --- a/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.h +++ b/src/agents/command_router/http_api/CommandRouterHttpAPIConfig.h @@ -11,7 +11,7 @@ using namespace commons; namespace command_router { struct HttpAPISettings { - size_t max_concurrent_executions = 100; + // Max admitted executions not yet finished. size_t max_queued_executions = 500; size_t max_events_per_execution = CommandExecution::DEFAULT_MAX_EVENTS; unsigned long execution_retention_ms = 15 * 60 * 1000; diff --git a/src/agents/command_router/http_api/HttpCommandProxyFactory.cc b/src/agents/command_router/http_api/HttpCommandProxyFactory.cc new file mode 100644 index 00000000..e8293540 --- /dev/null +++ b/src/agents/command_router/http_api/HttpCommandProxyFactory.cc @@ -0,0 +1,73 @@ +#include "HttpCommandProxyFactory.h" + +#include + +#include "ProxyParametersFromJson.h" +#include "Utils.h" + +using namespace command_router; +using namespace commons; + +namespace { + +bool parse_query_arg(const json& params, string& query_arg, string& error_message) { + if (!params.contains("query") || !params["query"].is_object()) { + error_message = "params.query must be an object"; + return false; + } + + const json& query = params["query"]; + if (query.contains("syntax") && + (!query["syntax"].is_string() || query["syntax"].get_ref() != "metta")) { + error_message = "params.query.syntax must be \"metta\""; + return false; + } + if (!query.contains("tokens") || !query["tokens"].is_array() || query["tokens"].empty()) { + error_message = "params.query.tokens must be a non-empty array"; + return false; + } + + vector tokens; + tokens.reserve(query["tokens"].size()); + for (const auto& token : query["tokens"]) { + if (!token.is_string() || token.get_ref().empty()) { + error_message = "params.query.tokens entries must be non-empty strings"; + return false; + } + tokens.push_back(token.get()); + } + + query_arg = Utils::join(tokens, ' '); + Utils::replace_all(query_arg, "%", "$"); + return true; +} + +} // namespace + +shared_ptr HttpCommandProxyFactory::create(const string& command, + const json& params, + string& error_message) { + if (!params.is_object()) { + error_message = "params must be an object"; + return nullptr; + } + + string arg; + + if (command == QUERY) { + if (!parse_query_arg(params, arg, error_message)) { + return nullptr; + } + } else { + error_message = "Unsupported command: " + command; + return nullptr; + } + + auto proxy = make_shared(command, arg); + + if (!ProxyParametersFromJson::set(proxy->parameters, params, command, error_message)) { + return nullptr; + } + + return proxy; +} diff --git a/src/agents/command_router/http_api/HttpCommandProxyFactory.h b/src/agents/command_router/http_api/HttpCommandProxyFactory.h new file mode 100644 index 00000000..ba8c2e03 --- /dev/null +++ b/src/agents/command_router/http_api/HttpCommandProxyFactory.h @@ -0,0 +1,35 @@ +#pragma once + +#include +#include + +#include "BusCommandRouterProxy.h" +#include "nlohmann/json.hpp" + +using namespace std; + +using json = nlohmann::json; + +namespace command_router { + +/** + * @brief Builds a BusCommandRouterProxy from an HTTP {command, params} request. + * + * Parses the command-specific structural fields into the bus ARG string, constructs + * the proxy with defaults, then overlays remaining scalar params onto proxy->parameters. + */ +class HttpCommandProxyFactory { + public: + /** Known HTTP command names. */ + static constexpr const char* QUERY = "query"; + + /** + * @brief Create a dispatch-ready proxy for the given HTTP command. + * @return Ready proxy on success; nullptr with error_message on failure. + */ + static shared_ptr create(const string& command, + const json& params, + string& error_message); +}; + +} // namespace command_router diff --git a/src/agents/command_router/http_api/ProxyParametersFromJson.cc b/src/agents/command_router/http_api/ProxyParametersFromJson.cc new file mode 100644 index 00000000..5f406908 --- /dev/null +++ b/src/agents/command_router/http_api/ProxyParametersFromJson.cc @@ -0,0 +1,207 @@ +#include "ProxyParametersFromJson.h" + +#include +#include +#include + +using namespace command_router; +using namespace commons; + +bool ProxyParametersFromJson::set_bool(PropertyValue& current, + const json& value, + const string& key, + string& error_message) { + if (value.is_boolean()) { + current = value.get(); + return true; + } + if (value.is_string()) { + const string& text = value.get_ref(); + if (text == "true" || text == "1") { + current = true; + return true; + } + if (text == "false" || text == "0") { + current = false; + return true; + } + } + if (value.is_number_integer()) { + const long long number = value.get(); + if (number == 0 || number == 1) { + current = (number == 1); + return true; + } + } + error_message = "Parameter '" + key + "' expects bool (true, false, 1, or 0)"; + return false; +} + +bool ProxyParametersFromJson::set_unsigned_int(PropertyValue& current, + const json& value, + const string& key, + string& error_message) { + const string uint_error = "Parameter '" + key + "' expects unsigned integer"; + const auto fits_uint = [](unsigned long long number) { + return static_cast(number) == number; + }; + + if (value.is_number_unsigned()) { + const unsigned long long number = value.get(); + if (!fits_uint(number)) { + error_message = uint_error; + return false; + } + current = static_cast(number); + return true; + } + if (value.is_number_integer()) { + const long long number = value.get(); + if (number < 0 || !fits_uint(static_cast(number))) { + error_message = uint_error; + return false; + } + current = static_cast(number); + return true; + } + if (value.is_string()) { + const string& text = value.get_ref(); + const bool all_digits = !text.empty() && all_of(text.begin(), text.end(), [](unsigned char c) { + return isdigit(c); + }); + if (!all_digits) { + error_message = uint_error; + return false; + } + try { + size_t consumed = 0; + const unsigned long long parsed = stoull(text, &consumed); + if (consumed != text.size() || !fits_uint(parsed)) { + error_message = uint_error; + return false; + } + current = static_cast(parsed); + return true; + } catch (const exception&) { + error_message = uint_error; + return false; + } + } + error_message = uint_error; + return false; +} + +bool ProxyParametersFromJson::set_long(PropertyValue& current, + const json& value, + const string& key, + string& error_message) { + if (value.is_number_integer()) { + current = static_cast(value.get()); + return true; + } + if (value.is_string()) { + try { + size_t consumed = 0; + const long parsed = stol(value.get_ref(), &consumed); + if (consumed != value.get_ref().size()) { + error_message = "Parameter '" + key + "' expects integer"; + return false; + } + current = parsed; + return true; + } catch (const exception&) { + error_message = "Parameter '" + key + "' expects integer"; + return false; + } + } + error_message = "Parameter '" + key + "' expects integer"; + return false; +} + +bool ProxyParametersFromJson::set_double(PropertyValue& current, + const json& value, + const string& key, + string& error_message) { + if (value.is_number()) { + current = value.get(); + return true; + } + if (value.is_string()) { + try { + size_t consumed = 0; + const double parsed = stod(value.get_ref(), &consumed); + if (consumed != value.get_ref().size()) { + error_message = "Parameter '" + key + "' expects number"; + return false; + } + current = parsed; + return true; + } catch (const exception&) { + error_message = "Parameter '" + key + "' expects number"; + return false; + } + } + error_message = "Parameter '" + key + "' expects number"; + return false; +} + +bool ProxyParametersFromJson::set_string(PropertyValue& current, + const json& value, + const string& key, + string& error_message) { + if (!value.is_string()) { + error_message = "Parameter '" + key + "' expects string"; + return false; + } + const string& text = value.get_ref(); + if (text.empty()) { + error_message = "Parameter '" + key + "' expects non-empty string"; + return false; + } + current = text; + return true; +} + +bool ProxyParametersFromJson::set(Properties& proxy_parameters, + const json& params, + const string& command, + string& error_message) { + for (const auto& [key, value] : params.items()) { + if (key == command) { + continue; + } + + auto param_it = proxy_parameters.find(key); + if (param_it == proxy_parameters.end()) { + error_message = "Unknown parameter: '" + key + "'"; + return false; + } + + PropertyValue& current = param_it->second; + if (holds_alternative(current)) { + if (!set_bool(current, value, key, error_message)) { + return false; + } + } else if (holds_alternative(current)) { + if (!set_unsigned_int(current, value, key, error_message)) { + return false; + } + } else if (holds_alternative(current)) { + if (!set_long(current, value, key, error_message)) { + return false; + } + } else if (holds_alternative(current)) { + if (!set_double(current, value, key, error_message)) { + return false; + } + } else if (holds_alternative(current)) { + if (!set_string(current, value, key, error_message)) { + return false; + } + } else { + error_message = "Parameter '" + key + "' has unsupported type"; + return false; + } + } + return true; +} diff --git a/src/agents/command_router/http_api/ProxyParametersFromJson.h b/src/agents/command_router/http_api/ProxyParametersFromJson.h new file mode 100644 index 00000000..1631cf70 --- /dev/null +++ b/src/agents/command_router/http_api/ProxyParametersFromJson.h @@ -0,0 +1,54 @@ +#pragma once + +#include + +#include "Properties.h" +#include "nlohmann/json.hpp" + +using namespace std; + +using json = nlohmann::json; + +namespace command_router { + +class ProxyParametersFromJson { + public: + ProxyParametersFromJson() = delete; + + /** + * @brief Sets the proxy parameters from the HTTP params JSON object. + * @param proxy_parameters The proxy parameters to set. + * @param params The HTTP params JSON object. + * @param command The command name. + * @param error_message The error message to set on failure. + * @return True on success; false on failure. + */ + static bool set(commons::Properties& proxy_parameters, + const json& params, + const string& command, + string& error_message); + + private: + static bool set_bool(commons::PropertyValue& current, + const json& value, + const string& key, + string& error_message); + static bool set_unsigned_int(commons::PropertyValue& current, + const json& value, + const string& key, + string& error_message); + static bool set_long(commons::PropertyValue& current, + const json& value, + const string& key, + string& error_message); + static bool set_double(commons::PropertyValue& current, + const json& value, + const string& key, + string& error_message); + static bool set_string(commons::PropertyValue& current, + const json& value, + const string& key, + string& error_message); +}; + +} // namespace command_router diff --git a/src/tests/cpp/BUILD b/src/tests/cpp/BUILD index 8d7a3ae7..10cd2949 100644 --- a/src/tests/cpp/BUILD +++ b/src/tests/cpp/BUILD @@ -258,7 +258,7 @@ cc_test( cc_test( name = "query_answer_test", - size = "small", + size = "medium", srcs = [ "query_answer_test.cc", "test_utils.cc", @@ -1070,6 +1070,7 @@ cc_test( "//agents/command_router/http_api:command_router_http_api", "//agents/command_router/http_api:command_router_http_api_config", "//agents/command_router/http_api:command_router_http_api_singleton", + "//agents/command_router/http_api:http_command_proxy_factory", "//agents/query_engine:query_answer", "//atomdb:atomdb_singleton", "//commons/processor:processor_lib", diff --git a/src/tests/cpp/command_router_http_api_test.cc b/src/tests/cpp/command_router_http_api_test.cc index b7567618..915ba50b 100644 --- a/src/tests/cpp/command_router_http_api_test.cc +++ b/src/tests/cpp/command_router_http_api_test.cc @@ -1,4 +1,5 @@ #include +#include #include "AtomDBSingleton.h" #include "BaseProxy.h" @@ -11,6 +12,7 @@ #include "CommandRouterHttpAPIConfig.h" #include "CommandRouterHttpAPISingleton.h" #include "DedicatedThread.h" +#include "HttpCommandProxyFactory.h" #include "JsonConfig.h" #include "PatternMatchingQueryProxy.h" #include "PortPool.h" @@ -18,6 +20,7 @@ #include "ServiceBus.h" #include "TestAtomDBJsonConfig.h" #include "TestSystemParams.h" +#include "expression_hasher.h" #include "gtest/gtest.h" #include "httplib.h" #include "processor/ThreadPool.h" @@ -35,13 +38,22 @@ namespace { const string TEST_HOST = "localhost"; const int TEST_PORT = 19001; -const int TEST_PORT_LIMITS = 19006; +const int TEST_PORT_THREAD_POOL = 19007; +const int TEST_PORT_PARALLEL = 19008; const string UNKNOWN_EXECUTION_ID = "exec-00000000000000000000000000000000"; const string SHORT_COMMAND_TEXT = "Blah"; -json make_execution_body(const string& command_type = "query", - const string& command_text = "(Similarity \"human\" %V)") { - return {{"command_type", command_type}, {"command_text", command_text}}; +json make_execution_body(const string& command = "query", + const string& query_token = "(Similarity \"human\" %V)") { + return {{"command", command}, + {"params", {{"query", {{"syntax", "metta"}, {"tokens", json::array({query_token})}}}}}}; +} + +string hash_string(const string& input) { + char* hash = compute_hash(const_cast(input.c_str())); + string result(hash); + delete[] hash; + return result; } class HangingQueryForwardProxy : public BusCommandProxy { @@ -61,6 +73,38 @@ class HangingQueryForwardProcessor : public BusCommandProcessor { void run_command(shared_ptr /*proxy*/) override {} }; +/** Replies with one answer whose handle is hash(query_tokens), plus max_answers-1 extras. */ +class EchoQueryProcessor : public BusCommandProcessor { + public: + EchoQueryProcessor() : BusCommandProcessor({ServiceBus::PATTERN_MATCHING_QUERY}) {} + + shared_ptr factory_empty_proxy() override { + return make_shared(); + } + + void run_command(shared_ptr proxy) override { + auto query = dynamic_pointer_cast(proxy); + if (query == nullptr) { + return; + } + // Bus delivers packed args; real processors untokenize before reading tokens/params. + query->untokenize(query->args); + const string query_key = Utils::join(query->get_query_tokens(), ' '); + unsigned int n = query->parameters.get(BaseQueryProxy::MAX_ANSWERS); + if (n == 0) { + n = 1; + } + std::thread([query, query_key, n]() { + Utils::sleep(5 + (query->get_serial() % 30)); + for (unsigned int i = 0; i < n; ++i) { + query->push( + make_shared(hash_string(query_key + "#" + std::to_string(i)), 0.0)); + } + query->query_processing_finished(); + }).detach(); + } +}; + void initialize_test_service_bus_statics_once() { static bool initialized = false; if (!initialized) { @@ -75,7 +119,10 @@ void initialize_test_service_bus_statics_once() { */ class HttpAPIServerFixture { public: - void start(int port, const HttpAPISettings& settings = {}, unsigned int num_threads = 8) { + void start(int port, + const HttpAPISettings& settings = {}, + unsigned int num_threads = 8, + shared_ptr query_processor = nullptr) { initialize_test_service_bus_statics_once(); const unsigned int query_port = PortPool::get_port(); @@ -83,8 +130,12 @@ class HttpAPIServerFixture { const unsigned int router_port = PortPool::get_port(); const string router_id = TEST_HOST + ":" + std::to_string(router_port); + if (query_processor == nullptr) { + query_processor = make_shared(); + } + this->query_bus = make_shared(query_id); - this->query_bus->register_processor(make_shared()); + this->query_bus->register_processor(query_processor); Utils::sleep(300); this->router_bus = make_shared(router_id, query_id); @@ -115,7 +166,7 @@ class HttpAPIServerFixture { httplib::Client make_client(int port) const { httplib::Client client(TEST_HOST, port); client.set_connection_timeout(2); - client.set_read_timeout(5); + client.set_read_timeout(15); return client; } @@ -176,7 +227,6 @@ JsonConfig make_command_router_config(const json& overrides = json::object()) { {"http_api", {{"endpoint", "localhost:40009"}, {"thread_pool_size", 8}, - {"max_concurrent_executions", 50}, {"max_queued_executions", 200}, {"max_events_per_execution", 5000}, {"stream_items_per_chunk", 25}, @@ -199,40 +249,36 @@ class CommandRouterHttpAPITest : public ::testing::Test { HttpAPIServerFixture CommandRouterHttpAPITest::server; -class CommandRouterHttpAPILimitsTest : public ::testing::Test { +class CommandRouterHttpAPIThreadPoolConcurrencyTest : public ::testing::Test { protected: + static constexpr unsigned int kThreadPoolSize = 2; static HttpAPIServerFixture server; static void SetUpTestSuite() { HttpAPISettings settings; - settings.max_concurrent_executions = 1; - server.start(TEST_PORT_LIMITS, settings); + settings.max_queued_executions = 10; + server.start(TEST_PORT_THREAD_POOL, settings, kThreadPoolSize); } static void TearDownTestSuite() { server.stop(); } - httplib::Client client() { return server.make_client(TEST_PORT_LIMITS); } + httplib::Client client() { return server.make_client(TEST_PORT_THREAD_POOL); } }; -HttpAPIServerFixture CommandRouterHttpAPILimitsTest::server; +HttpAPIServerFixture CommandRouterHttpAPIThreadPoolConcurrencyTest::server; -class CommandRouterHttpAPIQueuedConcurrencyTest : public ::testing::Test { +class CommandRouterHttpAPIParallelTest : public ::testing::Test { protected: static HttpAPIServerFixture server; static void SetUpTestSuite() { - HttpAPISettings settings; - settings.max_concurrent_executions = 2; - settings.max_queued_executions = 10; - server.start(19007, settings, 4); + server.start(TEST_PORT_PARALLEL, {}, 8, make_shared()); } static void TearDownTestSuite() { server.stop(); } - - httplib::Client client() { return server.make_client(19007); } }; -HttpAPIServerFixture CommandRouterHttpAPIQueuedConcurrencyTest::server; +HttpAPIServerFixture CommandRouterHttpAPIParallelTest::server; class CommandRouterHttpAPISingletonTest : public ::testing::Test { void TearDown() override { CommandRouterHttpAPISingleton::provide(nullptr); } @@ -248,14 +294,21 @@ TEST(CommandExecutionTest, status_and_terminal_flags) { } TEST(CommandExecutionTest, terminal_marks_finished_at) { - CommandExecution exec("exec-abc", "query", "(Similarity %V1 %V2)"); + CommandExecution exec( + "exec-abc", + "query", + {{"query", {{"syntax", "metta"}, {"tokens", json::array({"(Similarity %V1 %V2)"})}}}}); exec.mark_completed(100, 5); EXPECT_GT(exec.finished_at_ms(), 0); } TEST(CommandExecutionTest, event_buffer_overflow_raises) { - CommandExecution exec("exec-abc", "query", "(Similarity %V1 %V2)", 2); + CommandExecution exec( + "exec-abc", + "query", + {{"query", {{"syntax", "metta"}, {"tokens", json::array({"(Similarity %V1 %V2)"})}}}}, + 2); exec.mark_running(); exec.publish_chunk(1, json::array({json("a")})); @@ -272,7 +325,6 @@ TEST(CommandRouterHttpAPIConfigTest, from_config_loads_http_api_fields) { EXPECT_EQ(config.port, 40009); EXPECT_EQ(config.thread_pool_size, 8u); EXPECT_EQ(config.bus_host, "localhost"); - EXPECT_EQ(config.settings.max_concurrent_executions, 50u); EXPECT_EQ(config.settings.max_queued_executions, 200u); EXPECT_EQ(config.settings.max_events_per_execution, 5000u); EXPECT_EQ(config.settings.execution_retention_ms, 123456); @@ -307,8 +359,6 @@ TEST(CommandRouterHttpAPIConfigTest, from_config_rejects_trailing_junk_http_api_ // BusCommandRouterProcessor HTTP dispatch TEST(BusCommandRouterProcessorTest, dispatch_http_command_get_returns_params) { - set commands = {ServiceBus::BUS_COMMAND_ROUTER}; - ServiceBus::initialize_statics(commands, 49400, 49499); initialize_test_service_bus_statics_once(); const string router_id = TEST_HOST + ":" + std::to_string(PortPool::get_port()); @@ -325,26 +375,63 @@ TEST(BusCommandRouterProcessorTest, dispatch_http_command_get_returns_params) { EXPECT_TRUE(caller_proxy->finished()); } -TEST(BusCommandRouterProcessorTest, dispatch_http_command_syncs_caller_parameters_from_store) { - set commands = {ServiceBus::BUS_COMMAND_ROUTER}; - ServiceBus::initialize_statics(commands, 49400, 49499); +TEST(BusCommandRouterProcessorTest, dispatch_http_command_preserves_caller_parameters) { initialize_test_service_bus_statics_once(); - const string requestor_id = TEST_HOST + ":http-param-sync-test"; + const string requestor_id = TEST_HOST + ":http-param-preserve-test"; const string router_id = TEST_HOST + ":" + std::to_string(PortPool::get_port()); auto router_bus = make_shared(router_id); auto router_processor = make_shared(router_bus); router_bus->register_processor(router_processor); Utils::sleep(500); + // Peer store would have use_metta_as_query_tokens=true if HTTP still synced from it. auto set_caller = make_shared("set", "param use_metta_as_query_tokens true"); router_processor->dispatch_http_command(set_caller, requestor_id); Utils::sleep(500); auto query_caller = make_shared("query", "(Similarity %V1 %V2)"); - EXPECT_FALSE(query_caller->parameters.get(BaseQueryProxy::USE_METTA_AS_QUERY_TOKENS)); + query_caller->parameters[BaseQueryProxy::USE_METTA_AS_QUERY_TOKENS] = false; + query_caller->parameters[BaseQueryProxy::MAX_ANSWERS] = (unsigned int) 10; router_processor->dispatch_http_command(query_caller, requestor_id); - EXPECT_TRUE(query_caller->parameters.get(BaseQueryProxy::USE_METTA_AS_QUERY_TOKENS)); + + EXPECT_FALSE(query_caller->parameters.get(BaseQueryProxy::USE_METTA_AS_QUERY_TOKENS)); + EXPECT_EQ(query_caller->parameters.get(BaseQueryProxy::MAX_ANSWERS), 10u); +} + +TEST(HttpCommandProxyFactoryTest, create_query_sets_params_onto_proxy_defaults) { + string error; + const json params = { + {"query", {{"syntax", "metta"}, {"tokens", json::array({"(Similarity \"human\" %C)"})}}}, + {"populate_metta_mapping", true}, + {"use_metta_as_query_tokens", "true"}, + {"max_answers", 7}}; + + auto proxy = HttpCommandProxyFactory::create(HttpCommandProxyFactory::QUERY, params, error); + ASSERT_NE(proxy, nullptr) << error; + EXPECT_EQ(proxy->get_args()[0], "query"); + EXPECT_EQ(proxy->get_args()[1], "(Similarity \"human\" $C)"); + EXPECT_TRUE(proxy->parameters.get(BaseQueryProxy::POPULATE_METTA_MAPPING)); + EXPECT_TRUE(proxy->parameters.get(BaseQueryProxy::USE_METTA_AS_QUERY_TOKENS)); + EXPECT_EQ(proxy->parameters.get(BaseQueryProxy::MAX_ANSWERS), 7u); +} + +TEST(HttpCommandProxyFactoryTest, create_rejects_unknown_parameter) { + string error; + const json params = { + {"query", {{"syntax", "metta"}, {"tokens", json::array({"(Similarity \"human\" %C)"})}}}, + {"unknown_key", true}}; + + auto proxy = HttpCommandProxyFactory::create(HttpCommandProxyFactory::QUERY, params, error); + EXPECT_EQ(proxy, nullptr); + EXPECT_NE(error.find("Unknown parameter"), string::npos); +} + +TEST(HttpCommandProxyFactoryTest, create_rejects_unsupported_command) { + string error; + auto proxy = HttpCommandProxyFactory::create("evolution", json::object(), error); + EXPECT_EQ(proxy, nullptr); + EXPECT_NE(error.find("Unsupported command"), string::npos); } // ----------------------------------------------------------------------------- @@ -572,79 +659,70 @@ TEST_F(CommandRouterHttpAPITest, create_execution_returns_202) { EXPECT_EQ(payload["status"], "pending"); } -TEST_F(CommandRouterHttpAPITest, get_params_returns_sync_result) { - auto res = client().Post( - "/command-router/executions", make_execution_body("get", "params").dump(), "application/json"); - ASSERT_TRUE(res); - EXPECT_EQ(res->status, 200); - - auto payload = json::parse(res->body); - EXPECT_EQ(payload["command_type"], "get"); - EXPECT_TRUE(payload.contains("result")); - EXPECT_NE(payload["result"].get().find("use_cache"), string::npos); -} - -TEST_F(CommandRouterHttpAPITest, set_param_returns_sync_ack) { - auto res = client().Post("/command-router/executions", - make_execution_body("set", "param context test-context").dump(), - "application/json"); - ASSERT_TRUE(res); - EXPECT_EQ(res->status, 200); - - auto payload = json::parse(res->body); - EXPECT_EQ(payload["command_type"], "set"); - EXPECT_NE(payload["result"].get().find("context"), string::npos); - - auto get_res = client().Post( - "/command-router/executions", make_execution_body("get", "params").dump(), "application/json"); - ASSERT_TRUE(get_res); - ASSERT_EQ(get_res->status, 200); - EXPECT_NE(json::parse(get_res->body)["result"].get().find("context: test-context"), - string::npos); -} - -TEST_F(CommandRouterHttpAPITest, set_param_persists_for_later_get) { - auto set_res = client().Post("/command-router/executions", - make_execution_body("set", "param populate_metta_mapping true").dump(), - "application/json"); - ASSERT_TRUE(set_res); - ASSERT_EQ(set_res->status, 200); - - auto get_res = client().Post( - "/command-router/executions", make_execution_body("get", "params").dump(), "application/json"); - ASSERT_TRUE(get_res); - ASSERT_EQ(get_res->status, 200); - - const string params = json::parse(get_res->body)["result"].get(); - EXPECT_NE(params.find("populate_metta_mapping: true"), string::npos); -} - -TEST_F(CommandRouterHttpAPITest, set_param_rejects_unknown_key) { - auto res = client().Post("/command-router/executions", - make_execution_body("set", "param unknown_key value").dump(), - "application/json"); - ASSERT_TRUE(res); - EXPECT_EQ(res->status, 500); - - auto payload = json::parse(res->body); - EXPECT_TRUE(payload.contains("error")); - EXPECT_NE(payload["error"].get().find("Unknown parameter"), string::npos); -} - TEST_F(CommandRouterHttpAPITest, create_execution_rejects_invalid_requests) { auto bad_json = client().Post("/command-router/executions", "{bad", "application/json"); ASSERT_TRUE(bad_json); EXPECT_EQ(bad_json->status, 400); auto missing_field = - client().Post("/command-router/executions", R"({"command_type":"query"})", "application/json"); + client().Post("/command-router/executions", R"({"command":"query"})", "application/json"); ASSERT_TRUE(missing_field); EXPECT_EQ(missing_field->status, 400); - auto bad_type = client().Post( - "/command-router/executions", make_execution_body("invalid", "arg").dump(), "application/json"); - ASSERT_TRUE(bad_type); - EXPECT_EQ(bad_type->status, 400); + auto legacy_fields = client().Post( + "/command-router/executions", + json({{"command_type", "query"}, {"command_text", "(Similarity \"human\" %V)"}}).dump(), + "application/json"); + ASSERT_TRUE(legacy_fields); + EXPECT_EQ(legacy_fields->status, 400); + + auto bad_command = client().Post( + "/command-router/executions", make_execution_body("set").dump(), "application/json"); + ASSERT_TRUE(bad_command); + EXPECT_EQ(bad_command->status, 400); + + auto invalid_command = client().Post( + "/command-router/executions", make_execution_body("invalid").dump(), "application/json"); + ASSERT_TRUE(invalid_command); + EXPECT_EQ(invalid_command->status, 400); +} + +TEST(CommandExecutionTest, stream_events_use_command_params_envelope) { + CommandExecution exec( + "exec-abc", + "query", + {{"query", {{"syntax", "metta"}, {"tokens", json::array({"(Similarity %V1 %V2)"})}}}}); + + exec.mark_running(); + const json answer = {{"handles", json::array({json::array({"h1"})})}}; + exec.publish_chunk(1, json::array({answer})); + exec.mark_completed(12, 1); + + size_t next_index = 0; + bool stream_finished = false; + auto running = exec.wait_next_event(next_index, chrono::milliseconds(10), stream_finished); + ASSERT_TRUE(running.has_value()); + auto running_event = json::parse(*running); + EXPECT_EQ(running_event["command"], CommandExecution::COMMAND_EXECUTION_STATUS); + EXPECT_EQ(running_event["params"]["status"], "running"); + EXPECT_EQ(running_event["params"]["execution_id"], "exec-abc"); + + auto answers = exec.wait_next_event(next_index, chrono::milliseconds(10), stream_finished); + ASSERT_TRUE(answers.has_value()); + auto answers_event = json::parse(*answers); + EXPECT_EQ(answers_event["command"], CommandExecution::COMMAND_QUERY_ANSWERS); + EXPECT_EQ(answers_event["params"]["seq"], 1); + EXPECT_EQ(answers_event["params"]["received_count"], 1); + ASSERT_TRUE(answers_event["params"]["answers"].is_array()); + EXPECT_EQ(answers_event["params"]["answers"].size(), 1u); + + auto completed = exec.wait_next_event(next_index, chrono::milliseconds(10), stream_finished); + ASSERT_TRUE(completed.has_value()); + auto completed_event = json::parse(*completed); + EXPECT_EQ(completed_event["command"], CommandExecution::COMMAND_EXECUTION_STATUS); + EXPECT_EQ(completed_event["params"]["status"], "completed"); + EXPECT_EQ(completed_event["params"]["total_items"], 1); + EXPECT_EQ(completed_event["params"]["duration_ms"], 12); } TEST_F(CommandRouterHttpAPITest, get_execution_reports_running_then_unknown_returns_404) { @@ -718,7 +796,7 @@ TEST_F(CommandRouterHttpAPITest, cancel_running_execution_aborts_and_second_canc EXPECT_EQ(json::parse(second_cancel->body)["status"], "aborted"); } -TEST_F(CommandRouterHttpAPIQueuedConcurrencyTest, queued_executions_respect_concurrent_limit) { +TEST_F(CommandRouterHttpAPIThreadPoolConcurrencyTest, running_executions_bounded_by_thread_pool_size) { vector execution_ids; for (int i = 0; i < 4; ++i) { auto create = @@ -726,11 +804,9 @@ TEST_F(CommandRouterHttpAPIQueuedConcurrencyTest, queued_executions_respect_conc make_execution_body("query", SHORT_COMMAND_TEXT + std::to_string(i)).dump(), "application/json"); ASSERT_TRUE(create); - if (create->status == 202) { - execution_ids.push_back(json::parse(create->body)["execution_id"].get()); - } + ASSERT_EQ(create->status, 202); + execution_ids.push_back(json::parse(create->body)["execution_id"].get()); } - ASSERT_GE(execution_ids.size(), 2u); int max_observed_running = 0; for (int attempt = 0; attempt < 100; ++attempt) { @@ -747,36 +823,89 @@ TEST_F(CommandRouterHttpAPIQueuedConcurrencyTest, queued_executions_respect_conc Utils::sleep(50); } - EXPECT_LE(max_observed_running, 2); + EXPECT_GT(max_observed_running, 0); + EXPECT_LE(max_observed_running, static_cast(kThreadPoolSize)); } -TEST_F(CommandRouterHttpAPILimitsTest, rejects_concurrency_limit) { - auto first = client().Post("/command-router/executions", - make_execution_body("query", SHORT_COMMAND_TEXT).dump(), - "application/json"); - ASSERT_TRUE(first); - ASSERT_EQ(first->status, 202); +TEST_F(CommandRouterHttpAPIParallelTest, parallel_queries_keep_params_and_answers_isolated) { + constexpr int N = 20; + vector errors(N); - const auto execution_id = json::parse(first->body)["execution_id"].get(); + auto worker = [&](int i) { + const string token = "(Similarity \"c-" + std::to_string(i) + "\" %V)"; + const string expected_key = "(Similarity \"c-" + std::to_string(i) + "\" $V)"; + const unsigned int max_answers = 1u + static_cast(i % 4); - string status; - for (int attempt = 0; attempt < 50; ++attempt) { - auto get_res = client().Get("/command-router/executions/" + execution_id); - ASSERT_TRUE(get_res); - ASSERT_EQ(get_res->status, 200); - status = json::parse(get_res->body)["status"].get(); - if (status == "running") { - break; + json body = {{"command", "query"}, + {"params", + {{"query", {{"syntax", "metta"}, {"tokens", json::array({token})}}}, + {"use_metta_as_query_tokens", true}, + {"populate_metta_mapping", false}, + {"max_answers", max_answers}}}}; + + httplib::Client http(TEST_HOST, TEST_PORT_PARALLEL); + http.set_connection_timeout(2); + http.set_read_timeout(30); + + auto create = http.Post("/command-router/executions", body.dump(), "application/json"); + if (!create || create->status != 202) { + errors[i] = "create failed"; + return; } - Utils::sleep(100); - } - ASSERT_EQ(status, "running"); + const string id = json::parse(create->body)["execution_id"].get(); + + httplib::ws::WebSocketClient ws("ws://" + TEST_HOST + ":" + std::to_string(TEST_PORT_PARALLEL) + + "/command-router/ws/" + id); + if (!ws.is_valid() || !ws.connect()) { + errors[i] = "ws connect failed"; + return; + } + ws.set_read_timeout(10, 0); - auto second = client().Post("/command-router/executions", - make_execution_body("evolution", SHORT_COMMAND_TEXT).dump(), - "application/json"); - ASSERT_TRUE(second); - EXPECT_EQ(second->status, 429); + vector handles; + string status; + string msg; + while (ws.read(msg)) { + auto event = json::parse(msg); + if (event.value("command", "") == CommandExecution::COMMAND_QUERY_ANSWERS) { + for (const auto& answer : event["params"]["answers"]) { + handles.push_back(answer["handles"][0][0].get()); + } + } else if (event.value("command", "") == CommandExecution::COMMAND_EXECUTION_STATUS) { + status = event["params"].value("status", ""); + } + } + ws.close(); + + if (status != "completed") { + errors[i] = "status=" + status; + return; + } + if (handles.size() != max_answers) { + errors[i] = "count got=" + std::to_string(handles.size()) + + " expected=" + std::to_string(max_answers); + return; + } + for (unsigned int k = 0; k < max_answers; ++k) { + const string expected = hash_string(expected_key + "#" + std::to_string(k)); + if (handles[k] != expected) { + errors[i] = "handle mismatch at " + std::to_string(k) + " got=" + handles[k] + + " expected=" + expected; + return; + } + } + }; + + vector threads; + for (int i = 0; i < N; ++i) { + threads.emplace_back(worker, i); + } + for (auto& t : threads) { + t.join(); + } + for (int i = 0; i < N; ++i) { + EXPECT_TRUE(errors[i].empty()) << "client " << i << ": " << errors[i]; + } } TEST_F(CommandRouterHttpAPITest, websocket_streams_lifecycle_events) { @@ -807,8 +936,11 @@ TEST_F(CommandRouterHttpAPITest, websocket_streams_lifecycle_events) { string msg; while (ws.read(msg)) { auto event = json::parse(msg); - EXPECT_EQ(event["execution_id"].get(), execution_id); - if (event.value("status", "") == "running") { + ASSERT_TRUE(event.contains("command")); + ASSERT_TRUE(event.contains("params")); + EXPECT_EQ(event["params"]["execution_id"].get(), execution_id); + if (event["command"] == CommandExecution::COMMAND_EXECUTION_STATUS && + event["params"].value("status", "") == "running") { saw_running = true; break; } diff --git a/src/tests/scripts/command_router_http_client.py b/src/tests/scripts/command_router_http_client.py index 50704b94..09a9c6cf 100644 --- a/src/tests/scripts/command_router_http_client.py +++ b/src/tests/scripts/command_router_http_client.py @@ -73,7 +73,18 @@ def main() -> int: # POST /command-router/executions response = requests.post( f"{base_url}/command-router/executions", - json={"command_type": "query", "command_text": '(Similarity "human" %V)'}, + json={ + "command": "query", + "params": { + "query": { + "syntax": "metta", + "tokens": ['(Similarity "human" %V)'], + }, + "use_metta_as_query_tokens": True, + "populate_metta_mapping": True, + "max_answers": 1 + }, + }, timeout=10, ) print(f"POST /command-router/executions -> {response.status_code} {response.text}") @@ -92,15 +103,17 @@ def main() -> int: def before_cancel(event: dict, _events: list[dict]) -> bool: nonlocal saw_running, saw_chunk - if event.get("status") == "running": + command = event.get("command") + params = event.get("params") or {} + if command == "execution_status" and params.get("status") == "running": saw_running = True - if event.get("type") == "chunk": + if command == "query_answers": saw_chunk = True return saw_running and saw_chunk read_ws_events(ws, before_cancel, timeout=30.0) assert saw_running, "websocket never received running event" - assert saw_chunk, "websocket never received chunk event" + assert saw_chunk, "websocket never received query_answers event" # POST /command-router/executions/{id}/cancel response = requests.post( @@ -114,12 +127,15 @@ def before_cancel(event: dict, _events: list[dict]) -> bool: assert response.status_code == 200, "cancel failed" def until_aborted(event: dict, _events: list[dict]) -> bool: - return event.get("status") == "aborted" + params = event.get("params") or {} + return event.get("command") == "execution_status" and params.get("status") == "aborted" aborted_events = read_ws_events(ws, until_aborted, timeout=30.0) - assert any(event.get("status") == "aborted" for event in aborted_events), ( - "websocket never received aborted event" - ) + assert any( + event.get("command") == "execution_status" + and (event.get("params") or {}).get("status") == "aborted" + for event in aborted_events + ), "websocket never received aborted event" ws.close() # GET /command-router/executions/{id}