Преглед изворни кода

Merge branch 'config-defaults': apply configSchema defaults platform-wide

Nothing applied a node's configSchema "default:" - not the runner, not the
scheduler, not workflow save. Every node had to defend each config read in
code, and where one forgot, an omitted key silently became 0 or undefined.

That is what stopped workflow wf_520f6f05 from ever firing: its stored config
had no pollInterval, loadScheduledWorkflows read 0, and the workflow was never
registered as scheduled. The schema said default 5 the whole time.

Adds lib/common/config_defaults.{hpp,cpp} - one applyConfigDefaults - and calls
it at both write time (workflow create/update) and read time (runner node
execution, loop bodies, and the two scheduler paths), so existing stored
workflows are fixed without being rewritten.

Also makes two TTL fallbacks agree with their schemas rather than contradict
them; see ccdc442 for the behaviour change that implies.
fszontagh пре 1 месец
родитељ
комит
837c926d77

+ 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

+ 18 - 0
docs/nodes.md

@@ -88,6 +88,15 @@ The comment block at the top of the file defines node metadata:
 
 The `configSchema` defines what options users can configure in the node editor.
 
+A top-level property's `default` is applied automatically whenever the
+stored config omits that key - both when the workflow is saved and again
+before the node executes - so `execute()` can rely on the key being
+present even for a workflow saved before the field existed. A default is
+resolved through the same expression evaluation as any other stored
+config value, so a `default:` containing `{{ }}` will be evaluated as an
+expression rather than kept as a literal string. No current node relies on
+this; keep it in mind if you're the first to try it.
+
 ### Supported Field Types
 
 ```javascript
@@ -415,6 +424,15 @@ smartbotic.storage.update('collection', 'document-id', { key: 'new-value' });
 smartbotic.storage.delete('collection', 'document-id');
 ```
 
+`http-request` (Store Download) and `imap-extract-attachments` (Store in
+Database) both write through `smartbotic.storage.insert` with a TTL. Both
+default that TTL field to 24 hours, so downloaded files and extracted
+attachments stored by those nodes now expire and are auto-deleted a day
+after they're stored, unless the workflow sets the TTL field to `0`
+explicitly - `0` means keep forever. Any workflow that was relying on
+permanent storage under an unset TTL field needs that `0` set explicitly,
+or it will start losing data a day later.
+
 ### Filesystem
 
 ```javascript

+ 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

+ 36 - 0
lib/common/config_defaults.hpp

@@ -0,0 +1,36 @@
+#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.
+//
+// Write-time materialisation is permanent: once a workflow is saved with a
+// default baked into its stored config, that key now counts as "present",
+// so a later change to the schema's default will never reach that already-
+// saved workflow - the read path correctly leaves a present key alone. This
+// is an accepted consequence of applying defaults at write time, not a bug;
+// it only affects workflows saved after write-time materialisation shipped,
+// since a workflow saved before that never had the key baked in to begin
+// with, and still picks up schema default changes at read time.
+nlohmann::json applyConfigDefaults(const nlohmann::json& config,
+                                    const nlohmann::json& config_schema);
+
+} // namespace smartbotic::common

+ 2 - 2
nodes/core/http-request.js

@@ -81,7 +81,7 @@ const configSchema = {
       type: 'number',
       title: 'TTL (hours)',
       default: 24,
-      description: 'Auto-delete stored file after hours (0 = never)'
+      description: 'Auto-delete stored file after this many hours. Default is 24 hours; set to 0 to keep it forever.'
     }
   },
   required: ['url']
@@ -335,7 +335,7 @@ async function execute(config, input, context) {
       // Store in database if configured
       if (config.storeDownload) {
         const collection = config.downloadCollection || 'downloads';
-        const ttlHours = config.downloadTtlHours || 0;
+        const ttlHours = config.downloadTtlHours ?? 24;
         const ttlMs = ttlHours > 0 ? ttlHours * 60 * 60 * 1000 : 0;
 
         const checksum = contentHash;

+ 2 - 2
nodes/imap/imap-extract-attachments.js

@@ -58,7 +58,7 @@ const configSchema = {
             type: 'number',
             title: 'TTL (hours)',
             default: 24,
-            description: 'Auto-delete stored files after hours (0 = never)'
+            description: 'Auto-delete stored files after this many hours. Default is 24 hours; set to 0 to keep them forever.'
         },
         binaryStoragePath: {
             type: 'string',
@@ -439,7 +439,7 @@ module.exports = {
         // Store metadata in database if configured (without binary data)
         if (config.storeInDatabase && attachments.length > 0) {
             const collection = config.storageCollection || 'email_attachments';
-            const ttlHours = config.storageTtlHours || 0;
+            const ttlHours = config.storageTtlHours ?? 24;
             const ttlMs = ttlHours > 0 ? ttlHours * 60 * 60 * 1000 : 0;
 
             for (const att of attachments) {

+ 33 - 8
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;
@@ -659,7 +666,8 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     node_result.started_at = TimeUtils::nowMs();
                     node_result.finished_at = node_result.started_at;
                 } else {
-                    node_result = executeNode(evaluated_node, input, result.execution_id, workflow);
+                    node_result = executeNode(evaluated_node, input, result.execution_id, workflow,
+                                               node_def_for_defaults);
                 }
                 result.node_results[node_id] = node_result;
 
@@ -1112,15 +1120,22 @@ std::vector<std::string> WorkflowEngine::topologicalSort(
 NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
                                                 const nlohmann::json& input,
                                                 const std::string& execution_id,
-                                                const Workflow& workflow) {
+                                                const Workflow& workflow,
+                                                const std::optional<NodeDefinition>& prefetched_node_def) {
     NodeExecutionResult result;
     result.node_id = node.id;
     result.input = input;
     result.started_at = TimeUtils::nowMs();
     result.status = NodeStatus::Running;
 
-    // Get node definition
-    auto node_def = registry_.getNode(node.type);
+    // Get node definition - reuse the caller's lookup if it already made one
+    // (it did, to apply config defaults before expression evaluation) rather
+    // than looking it up, and copying its JavaScript source, a second time.
+    std::optional<NodeDefinition> looked_up_node_def;
+    if (!prefetched_node_def) {
+        looked_up_node_def = registry_.getNode(node.type);
+    }
+    const std::optional<NodeDefinition>& node_def = prefetched_node_def ? prefetched_node_def : looked_up_node_def;
     if (!node_def) {
         result.status = NodeStatus::Failed;
         result.error = "Node type not found: " + node.type;
@@ -2032,10 +2047,19 @@ bool WorkflowEngine::executeLoopBody(
                 merged_results[key] = value;
             }
 
-            // Evaluate expressions in body node config before execution
+            // Evaluate expressions in body node config before execution. Apply
+            // config defaults first, same as the main walk does, so a loop
+            // body node also gets its schema defaults on an already-saved
+            // workflow rather than only on nodes outside a loop.
             WorkflowNode evaluated_body_node = *body_node;
             t_disabled_reference_error.clear();
-            evaluated_body_node.config = evaluateExpressions(body_node->config, node_input, merged_results, workflow);
+            nlohmann::json defaulted_body_config = body_node->config;
+            auto body_node_def_for_defaults = registry_.getNode(body_node->type);
+            if (body_node_def_for_defaults) {
+                defaulted_body_config = smartbotic::common::applyConfigDefaults(
+                    body_node->config, body_node_def_for_defaults->config_schema);
+            }
+            evaluated_body_node.config = evaluateExpressions(defaulted_body_config, node_input, merged_results, workflow);
 
             NodeExecutionResult body_result;
             if (!t_disabled_reference_error.empty()) {
@@ -2047,7 +2071,8 @@ bool WorkflowEngine::executeLoopBody(
                 body_result.started_at = TimeUtils::nowMs();
                 body_result.finished_at = body_result.started_at;
             } else {
-                body_result = executeNode(evaluated_body_node, node_input, result.execution_id, workflow);
+                body_result = executeNode(evaluated_body_node, node_input, result.execution_id, workflow,
+                                           body_node_def_for_defaults);
             }
 
             rejectPauseInLoopBody(body_result);

+ 8 - 1
src/runner/workflow_engine.hpp

@@ -8,6 +8,7 @@
 #include <condition_variable>
 #include <thread>
 #include <atomic>
+#include <optional>
 #include <nlohmann/json.hpp>
 #include "node_registry.hpp"
 #include "error_handler.hpp"
@@ -212,10 +213,16 @@ private:
     std::vector<std::string> topologicalSort(const Workflow& workflow,
                                              const std::unordered_map<std::string, std::vector<std::string>>& dependencies);
 
+    // prefetched_node_def: when the caller already looked up the node's
+    // definition (e.g. to apply config defaults before evaluating
+    // expressions), pass it here so executeNode reuses it instead of
+    // looking it up - and copying its JavaScript source - a second time.
+    // Left empty, executeNode looks it up itself.
     NodeExecutionResult executeNode(const WorkflowNode& node,
                                    const nlohmann::json& input,
                                    const std::string& execution_id,
-                                   const Workflow& workflow);
+                                   const Workflow& workflow,
+                                   const std::optional<NodeDefinition>& prefetched_node_def = std::nullopt);
 
     nlohmann::json collectNodeInput(const std::string& node_id,
                                     const Workflow& workflow,

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

+ 20 - 0
tests/nodes/config-defaults-loop-body.json

@@ -0,0 +1,20 @@
+{
+  "name": "verify-config-defaults-loop-body",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "items", "name": "Items", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { items: [1, 2] };"}},
+    {"id": "loop", "name": "Loop", "type": "loop", "position": {"x": 0, "y": 200},
+     "config": {"inputField": "data.result.items"}},
+    {"id": "body", "name": "NoConfigInLoop", "type": "code", "position": {"x": 0, "y": 300},
+     "config": {}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "items", "targetInput": "data"},
+    {"sourceNodeId": "items", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "loop", "targetNodeId": "body", "targetInput": "data"}
+  ],
+  "expect": {
+    "body": {"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"}
+  }
+}

+ 20 - 0
tests/nodes/config-defaults-ttl-zero-http-request.json

@@ -0,0 +1,20 @@
+{
+  "name": "verify-config-defaults-ttl-zero-http-request",
+  "settings": {
+    "storagePermissions": {
+      "collections": { "ttl0_download_probe": "read-write" }
+    }
+  },
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "dl", "name": "Download", "type": "http-request", "position": {"x": 0, "y": 100},
+     "config": {"method": "GET", "url": "http://localhost:8090/index.html", "responseMode": "binary",
+       "storeDownload": true, "downloadCollection": "ttl0_download_probe", "downloadTtlHours": 0}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "dl", "targetInput": "data"}
+  ],
+  "expect": {
+    "dl": {"status": "completed", "output": {"storage": {"collection": "ttl0_download_probe"}}}
+  }
+}

+ 23 - 0
tests/nodes/config-defaults-ttl-zero-imap-extract-attachments.json

@@ -0,0 +1,23 @@
+{
+  "name": "verify-config-defaults-ttl-zero-imap-extract-attachments",
+  "settings": {
+    "storagePermissions": {
+      "collections": { "ttl0_attach_probe": "read-write" }
+    }
+  },
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "mail", "name": "Mail", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "var boundary = 'BOUNDARY123'; var b64 = smartbotic.utils.base64Encode('hello world'); var raw = 'Content-Type: multipart/mixed; boundary=\"' + boundary + '\"\\n\\n' + '--' + boundary + '\\n' + 'Content-Type: text/plain\\n\\n' + 'body text\\n' + '--' + boundary + '\\n' + 'Content-Type: text/plain; name=\"test.txt\"\\n' + 'Content-Disposition: attachment; filename=\"test.txt\"\\n' + 'Content-Transfer-Encoding: base64\\n\\n' + b64 + '\\n' + '--' + boundary + '--'; return { raw: raw };"}},
+    {"id": "extract", "name": "Extract", "type": "imap-extract-attachments", "position": {"x": 0, "y": 200},
+     "config": {"emailSource": "{{data.result.raw}}", "deduplicateByHash": false, "storeInDatabase": true,
+       "storageCollection": "ttl0_attach_probe", "storageTtlHours": 0}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "mail", "targetInput": "data"},
+    {"sourceNodeId": "mail", "sourceOutput": "main", "targetNodeId": "extract", "targetInput": "data"}
+  ],
+  "expect": {
+    "extract": {"status": "completed", "output": {"count": 1}}
+  }
+}