3bd17d0 fix: apply configSchema defaults at write time and read time CMakeLists.txt | 1 + lib/common/config_defaults.cpp | 39 +++++++++++++++++++++++++ lib/common/config_defaults.hpp | 27 +++++++++++++++++ src/runner/workflow_engine.cpp | 9 +++++- src/webserver/api/workflow_controller.cpp | 39 +++++++++++++++++++++++-- src/webserver/api/workflow_controller.hpp | 4 +++ src/webserver/webserver_service.cpp | 7 +++-- tests/nodes/config-defaults-fill.json | 14 +++++++++ tests/nodes/config-defaults-preserve-falsy.json | 14 +++++++++ 9 files changed, 148 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4499987..6aef080 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,20 +12,21 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") include(CompilerFlags) include(Dependencies) include(FindPackages) # Common library add_library(smartbotic_common STATIC lib/common/uuid.cpp lib/common/time_utils.cpp lib/common/error.cpp lib/common/string_utils.cpp + lib/common/config_defaults.cpp ) target_include_directories(smartbotic_common PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/lib ) target_link_libraries(smartbotic_common PUBLIC nlohmann_json::nlohmann_json OpenSSL::Crypto ) # Logging library diff --git a/lib/common/config_defaults.cpp b/lib/common/config_defaults.cpp new file mode 100644 index 0000000..a5bbfd8 --- /dev/null +++ b/lib/common/config_defaults.cpp @@ -0,0 +1,39 @@ +#include "common/config_defaults.hpp" + +namespace smartbotic::common { + +nlohmann::json applyConfigDefaults(const nlohmann::json& config, + const nlohmann::json& config_schema) { + nlohmann::json result = config.is_object() ? config : nlohmann::json::object(); + + if (!config_schema.is_object()) { + return result; + } + + auto properties_it = config_schema.find("properties"); + if (properties_it == config_schema.end() || !properties_it->is_object()) { + return result; + } + + for (auto it = properties_it->begin(); it != properties_it->end(); ++it) { + const std::string& key = it.key(); + const nlohmann::json& property_schema = it.value(); + + if (!property_schema.is_object()) { + continue; + } + + auto default_it = property_schema.find("default"); + if (default_it == property_schema.end()) { + continue; + } + + if (!result.contains(key)) { + result[key] = *default_it; + } + } + + return result; +} + +} // namespace smartbotic::common diff --git a/lib/common/config_defaults.hpp b/lib/common/config_defaults.hpp new file mode 100644 index 0000000..ad504da --- /dev/null +++ b/lib/common/config_defaults.hpp @@ -0,0 +1,27 @@ +#pragma once + +#include + +namespace smartbotic::common { + +// Applies configSchema-declared defaults to a node config. +// +// Returns a copy of `config` with any property from +// `config_schema["properties"]` that declares a "default" filled in, but +// only when `config` does not already contain that key. A key that is +// present - even with a falsy value like false, 0, null, or "" - is a +// deliberate, stored value and is never overwritten. +// +// Scope limit (by design, not an oversight): only top-level properties are +// considered. Defaults nested inside object properties or array item +// schemas are NOT applied. Nodes are not currently written with nested +// config shapes that rely on defaults, so recursing was left out to keep +// this function's behaviour easy to reason about; revisit if that changes. +// +// Tolerant of a missing/null/non-object schema, or one with no +// "properties" - in all of those cases the config is returned unchanged +// rather than throwing. +nlohmann::json applyConfigDefaults(const nlohmann::json& config, + const nlohmann::json& config_schema); + +} // namespace smartbotic::common diff --git a/src/runner/workflow_engine.cpp b/src/runner/workflow_engine.cpp index e475d9c..bf2a85d 100644 --- a/src/runner/workflow_engine.cpp +++ b/src/runner/workflow_engine.cpp @@ -1,13 +1,14 @@ #include "workflow_engine.hpp" #include "common/uuid.hpp" #include "common/time_utils.hpp" +#include "common/config_defaults.hpp" #include "logging/logger.hpp" #include #include #include #include namespace smartbotic::runner { using namespace common; @@ -641,21 +642,27 @@ Result WorkflowEngine::execute(const Workflow& workflow, {"nodeId", node_id}, {"status", "completed"}, {"output", node_result.output}, {"fromCache", true} }); } } else { // Evaluate expressions in node config WorkflowNode evaluated_node = *node; t_disabled_reference_error.clear(); - evaluated_node.config = evaluateExpressions(node->config, input, result.node_results, workflow); + nlohmann::json defaulted_config = node->config; + auto node_def_for_defaults = registry_.getNode(node->type); + if (node_def_for_defaults) { + defaulted_config = smartbotic::common::applyConfigDefaults( + node->config, node_def_for_defaults->config_schema); + } + evaluated_node.config = evaluateExpressions(defaulted_config, input, result.node_results, workflow); if (!t_disabled_reference_error.empty()) { node_result.node_id = node_id; node_result.status = NodeStatus::Failed; node_result.input = input; node_result.output = nlohmann::json::object(); node_result.error = t_disabled_reference_error; node_result.started_at = TimeUtils::nowMs(); node_result.finished_at = node_result.started_at; } else { diff --git a/src/webserver/api/workflow_controller.cpp b/src/webserver/api/workflow_controller.cpp index 4be6024..aa0021c 100644 --- a/src/webserver/api/workflow_controller.cpp +++ b/src/webserver/api/workflow_controller.cpp @@ -1,13 +1,14 @@ #include "workflow_controller.hpp" #include "common/uuid.hpp" #include "common/time_utils.hpp" +#include "common/config_defaults.hpp" #include "logging/logger.hpp" #include namespace smartbotic::webserver::api { using namespace common; WorkflowController::WorkflowController(storage::StorageClient& storage, auth::AuthMiddleware& middleware, runners::RunnerRegistry& registry, @@ -16,20 +17,45 @@ WorkflowController::WorkflowController(storage::StorageClient& storage, WorkflowScheduler& scheduler, nodes::NodeStore& node_store) : storage_(storage) , middleware_(middleware) , registry_(registry) , load_balancer_(load_balancer) , ws_server_(ws_server) , scheduler_(scheduler) , node_store_(node_store) {} +void WorkflowController::materializeNodeConfigDefaults(nlohmann::json& body) { + if (!body.contains("nodes") || !body["nodes"].is_array()) { + return; + } + + for (auto& node : body["nodes"]) { + if (!node.is_object()) { + continue; + } + + std::string node_type = node.value("type", ""); + if (node_type.empty()) { + continue; + } + + auto node_result = node_store_.get(node_type); + if (node_result.failed()) { + continue; + } + + auto config = node.value("config", nlohmann::json::object()); + node["config"] = common::applyConfigDefaults(config, node_result.value().config_schema); + } +} + void WorkflowController::registerRoutes(httplib::Server& server) { server.Get("/api/v1/workflows", [this](const httplib::Request& req, httplib::Response& res) { middleware_.requireAuth(req, res, [this](auto& req, auto& res, auto& ctx) { listWorkflows(req, res, ctx); }); }); server.Get(R"(/api/v1/workflows/([^/]+))", [this](const httplib::Request& req, httplib::Response& res) { middleware_.requireAuth(req, res, [this](auto& req, auto& res, auto& ctx) { getWorkflow(req, res, ctx); @@ -165,20 +191,22 @@ void WorkflowController::createWorkflow(const httplib::Request& req, httplib::Re // Set owner - metadata fields (_id, _createdAt, _updatedAt, _version) are handled by database body["ownerId"] = ctx.user_id; body["active"] = body.value("active", false); // Validate required fields if (!body.contains("name") || body["name"].get().empty()) { sendError(res, "Name is required", 400); return; } + materializeNodeConfigDefaults(body); + auto result = storage_.insert("workflows", body, id); if (result.failed()) { sendError(res, result.error().message(), 500); return; } // Get the created workflow with metadata auto workflow = storage_.get("workflows", id); if (workflow.failed()) { sendError(res, "Failed to retrieve created workflow", 500); @@ -215,20 +243,22 @@ void WorkflowController::updateWorkflow(const httplib::Request& req, httplib::Re // Note: updatedAt is managed by database automatically // Remember the current run state so the reconciliation below can tell // whether this update actually flipped it. bool was_active = false; auto before = storage_.get("workflows", id); if (before.ok()) { was_active = before.value().value("active", false); } + materializeNodeConfigDefaults(body); + auto result = storage_.update("workflows", id, body, 0, true); if (result.failed()) { sendError(res, result.error().message(), 404); return; } // Get updated workflow auto workflow = storage_.get("workflows", id); if (workflow.ok()) { // Reconcile the scheduler in both directions. Registering on active @@ -402,22 +432,24 @@ void WorkflowController::updateScheduledTriggers(const std::string& workflow_id, } auto& node_def = node_result.value(); // Check if node has scheduling capability (is_scheduled flag or pollInterval config) bool is_scheduled = node_def.is_scheduled; if (!is_scheduled) { continue; } - // Get interval from node config - auto config = node.value("config", nlohmann::json::object()); + // Get interval from node config, filling in any schema defaults the + // stored config is missing. + auto config = common::applyConfigDefaults( + node.value("config", nlohmann::json::object()), node_def.config_schema); int interval = config.value("pollInterval", 0); if (interval > 0) { scheduler_.registerWorkflow( workflow_id, workflow_name, node_id, node_type, interval ); @@ -430,21 +462,22 @@ void WorkflowController::updateScheduledTriggers(const std::string& workflow_id, int WorkflowController::getScheduledInterval(const nlohmann::json& workflow) { auto nodes = workflow.value("nodes", nlohmann::json::array()); for (const auto& node : nodes) { std::string node_type = node.value("type", ""); auto node_result = node_store_.get(node_type); if (node_result.failed() || !node_result.value().is_trigger || !node_result.value().is_scheduled) { continue; } - auto config = node.value("config", nlohmann::json::object()); + auto config = common::applyConfigDefaults( + node.value("config", nlohmann::json::object()), node_result.value().config_schema); return config.value("pollInterval", 0); } return 0; } void WorkflowController::sendJson(httplib::Response& res, const nlohmann::json& data, int status) { res.status = status; res.set_content(data.dump(), "application/json"); } diff --git a/src/webserver/api/workflow_controller.hpp b/src/webserver/api/workflow_controller.hpp index 2447ceb..a748ae1 100644 --- a/src/webserver/api/workflow_controller.hpp +++ b/src/webserver/api/workflow_controller.hpp @@ -53,13 +53,17 @@ private: auth::AuthMiddleware& middleware_; runners::RunnerRegistry& registry_; runners::LoadBalancer& load_balancer_; WebSocketServer& ws_server_; WorkflowScheduler& scheduler_; nodes::NodeStore& node_store_; // Helper to check for scheduled triggers and register/unregister with scheduler void updateScheduledTriggers(const std::string& workflow_id, bool activate); int getScheduledInterval(const nlohmann::json& workflow); + + // Fills in each node's stored config with any missing configSchema + // defaults, in place, so a saved workflow document is self-describing. + void materializeNodeConfigDefaults(nlohmann::json& body); }; } // namespace smartbotic::webserver::api diff --git a/src/webserver/webserver_service.cpp b/src/webserver/webserver_service.cpp index a7d7f13..124419a 100644 --- a/src/webserver/webserver_service.cpp +++ b/src/webserver/webserver_service.cpp @@ -10,20 +10,21 @@ #include "api/database_controller.hpp" #include "api/file_controller.hpp" #include "api/proxy_controller.hpp" #include "api/credential_controller.hpp" #include "nodes/node_store.hpp" #include "grpc/node_sync_service.hpp" #include "grpc/credential_service.hpp" #include "credentials/credential_store.hpp" #include "scheduler/workflow_scheduler.hpp" #include "common/time_utils.hpp" +#include "common/config_defaults.hpp" #include "logging/logger.hpp" #include #include "proto/runner.grpc.pb.h" namespace smartbotic::webserver { WebServerService::WebServerService(const WebServerServiceConfig& config) : config_(config) { // Initialize storage client @@ -306,22 +307,24 @@ void WebServerService::loadScheduledWorkflows() { auto node_result = node_store_->get(node_type); if (node_result.failed()) { continue; } const auto& node_def = node_result.value(); if (!node_def.is_trigger || !node_def.is_scheduled) { continue; } - // Get interval from node config - auto config = node.value("config", nlohmann::json::object()); + // Get interval from node config, filling in any schema defaults + // the stored config is missing (e.g. an untouched form field). + auto config = smartbotic::common::applyConfigDefaults( + node.value("config", nlohmann::json::object()), node_def.config_schema); int interval = config.value("pollInterval", 0); if (interval > 0) { auto policy = overlapPolicyFromString( config.value("overlapPolicy", std::string("skip"))); scheduler_->registerWorkflow( workflow_id, workflow_name, node_id, diff --git a/tests/nodes/config-defaults-fill.json b/tests/nodes/config-defaults-fill.json new file mode 100644 index 0000000..ec715f0 --- /dev/null +++ b/tests/nodes/config-defaults-fill.json @@ -0,0 +1,14 @@ +{ + "name": "verify-config-defaults-fill", + "nodes": [ + {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}}, + {"id": "n2", "name": "NoConfig", "type": "code", "position": {"x": 0, "y": 100}, + "config": {}} + ], + "connections": [ + {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "n2", "targetInput": "data"} + ], + "expect": { + "n2": {"status": "completed", "output": {"result": {"processed": true}}} + } +} diff --git a/tests/nodes/config-defaults-preserve-falsy.json b/tests/nodes/config-defaults-preserve-falsy.json new file mode 100644 index 0000000..6b84e8c --- /dev/null +++ b/tests/nodes/config-defaults-preserve-falsy.json @@ -0,0 +1,14 @@ +{ + "name": "verify-config-defaults-preserve-falsy", + "nodes": [ + {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}}, + {"id": "n2", "name": "EmptyCode", "type": "code", "position": {"x": 0, "y": 100}, + "config": {"code": ""}} + ], + "connections": [ + {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "n2", "targetInput": "data"} + ], + "expect": { + "n2": {"status": "failed", "errorContains": "No code provided"} + } +}