Просмотр исходного кода

fix: apply configSchema defaults at write time and read time

A node's configSchema could declare "default" for a property, but nothing
ever applied it - not the runner, not the scheduler, not on save. It was
documentation for the editor's form only. This caused a live bug: workflow
wf_520f6f05-3256-4419-a8a5-42d7e7c3830f has an imap-trigger whose stored
config has no pollInterval key. loadScheduledWorkflows read it as 0 via
config.value("pollInterval", 0) and never registered the workflow with the
scheduler, so the mailbox was never polled, even though the schema declares
default: 5.

Add a single shared helper, smartbotic::common::applyConfigDefaults in
lib/common/config_defaults.{hpp,cpp}, that fills in missing top-level
config keys from configSchema properties that declare a default. A key
that is present, even with a falsy value like false, 0, null or "", is
left alone - only a missing key gets defaulted.

Call it from every site that reads or stores node config:
- src/runner/workflow_engine.cpp, before expression evaluation, so a node
  never executes with an unset field that has a schema default.
- src/webserver/webserver_service.cpp loadScheduledWorkflows, and
  src/webserver/api/workflow_controller.cpp updateScheduledTriggers and
  getScheduledInterval, so the scheduler sees the same defaults an editor
  would have shown.
- src/webserver/api/workflow_controller.cpp createWorkflow and
  updateWorkflow now materialise defaults into each node's stored config,
  so a saved workflow document is self-describing.

Verified the production workflow now registers: "Loaded 2 scheduled
workflows" at webserver startup, up from 1, including wf_520f6f05 with
its imap-trigger, without editing that workflow.

Added tests/nodes/config-defaults-fill.json and
config-defaults-preserve-falsy.json to prove the read-time path: a code
node with no "code" key runs the schema's default code, while a code
node with an explicit empty string keeps it and fails. Full fixture
suite: 39/39 passing (37 pre-existing plus these 2), no node behaviour
changed.
fszontagh 1 месяц назад
Родитель
Сommit
3bd17d0698

+ 1 - 0
CMakeLists.txt

@@ -19,6 +19,7 @@ add_library(smartbotic_common STATIC
     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

+ 39 - 0
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

+ 27 - 0
lib/common/config_defaults.hpp

@@ -0,0 +1,27 @@
+#pragma once
+
+#include <nlohmann/json.hpp>
+
+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

+ 8 - 1
src/runner/workflow_engine.cpp

@@ -1,6 +1,7 @@
 #include "workflow_engine.hpp"
 #include "common/uuid.hpp"
 #include "common/time_utils.hpp"
+#include "common/config_defaults.hpp"
 #include "logging/logger.hpp"
 #include <algorithm>
 #include <functional>
@@ -648,7 +649,13 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 // 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;

+ 36 - 3
src/webserver/api/workflow_controller.cpp

@@ -1,6 +1,7 @@
 #include "workflow_controller.hpp"
 #include "common/uuid.hpp"
 #include "common/time_utils.hpp"
+#include "common/config_defaults.hpp"
 #include "logging/logger.hpp"
 #include <grpcpp/grpcpp.h>
 
@@ -23,6 +24,31 @@ WorkflowController::WorkflowController(storage::StorageClient& storage,
     , 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) {
@@ -172,6 +198,8 @@ void WorkflowController::createWorkflow(const httplib::Request& req, httplib::Re
             return;
         }
 
+        materializeNodeConfigDefaults(body);
+
         auto result = storage_.insert("workflows", body, id);
         if (result.failed()) {
             sendError(res, result.error().message(), 500);
@@ -222,6 +250,8 @@ void WorkflowController::updateWorkflow(const httplib::Request& req, httplib::Re
             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);
@@ -409,8 +439,10 @@ void WorkflowController::updateScheduledTriggers(const std::string& workflow_id,
             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) {
@@ -437,7 +469,8 @@ int WorkflowController::getScheduledInterval(const nlohmann::json& workflow) {
             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;

+ 4 - 0
src/webserver/api/workflow_controller.hpp

@@ -60,6 +60,10 @@ private:
     // 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

+ 5 - 2
src/webserver/webserver_service.cpp

@@ -17,6 +17,7 @@
 #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 <grpcpp/grpcpp.h>
 #include "proto/runner.grpc.pb.h"
@@ -313,8 +314,10 @@ void WebServerService::loadScheduledWorkflows() {
                 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) {

+ 14 - 0
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}}}
+  }
+}

+ 14 - 0
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"}
+  }
+}