config: ensure yaml schema is validated

SilKit/source/config/ParticipantConfigurationFromXImpl.cpp

@@ -654,6 +654,9 @@ void ProcessIncludes(const ParticipantConfiguration& config, ConfigIncludeData&
 auto PaticipantConfigurationWithIncludes(const std::string& text, struct ConfigIncludeData& configData)
     -> SilKit::Config::ParticipantConfiguration
 {
+    // Validate the root configuration against the schema (included files are validated in ProcessIncludes)
+    SilKit::Config::Validate(text);
+
     auto configuration = SilKit::Config::Deserialize<ParticipantConfiguration>(text);
     if (configuration.schemaVersion.empty())
     {

SilKit/source/config/Test_ParticipantConfiguration.cpp

@@ -140,6 +140,33 @@ Description: Example include configuration for CAN Controllers
     ASSERT_EQ(participantConfig, participantConfigRef);
     ASSERT_TRUE(participantConfig.logging.sinks.at(0).enabledTopics.at(0) == SilKit::Services::Logging::Topic::Can);
 }
+
+TEST_F(Test_ParticipantConfiguration, participant_config_from_string_root_schema_validated)
+{
+    // Regression: the root configuration (not just included files) must be schema-validated.
+    // A misplaced known keyword at the top level must be rejected.
+    const auto configString = R"raw(
+---
+ParticipantName: P1
+# FlushLevel is a Logging sub-element, not valid at the document root
+FlushLevel: Info
+)raw";
+
+    EXPECT_THROW(SilKit::Config::ParticipantConfigurationFromStringImpl(configString), SilKit::ConfigurationError);
+}
+
+TEST_F(Test_ParticipantConfiguration, participant_config_from_string_unknown_field_warns_but_loads)
+{
+    // Unknown, non-reserved fields are warned about but must not prevent loading the config.
+    const auto configString = R"raw(
+---
+ParticipantName: P1
+Foobar: true
+)raw";
+
+    EXPECT_NO_THROW(SilKit::Config::ParticipantConfigurationFromStringImpl(configString));
+}
+
 TEST_F(Test_ParticipantConfiguration, participant_config_from_string_includes)
 {
     const auto configString = R"raw(

SilKit/source/config/Test_YamlParser.cpp

@@ -352,6 +352,31 @@ TEST_F(Test_YamlParser, yaml_native_type_conversions)
     }
 }
 
+TEST_F(Test_YamlParser, yaml_metrics_update_interval)
+{
+    // UpdateInterval is read from YAML as an integer count of seconds
+    auto config = Deserialize<ParticipantConfiguration>(R"(
+Experimental:
+  Metrics:
+    CollectFromRemote: true
+    UpdateInterval: 42
+)");
+    EXPECT_EQ(config.experimental.metrics.updateInterval, 42s);
+
+    // ... and round-trips through serialization
+    auto txt = Serialize(config);
+    auto config2 = Deserialize<ParticipantConfiguration>(txt);
+    EXPECT_EQ(config2.experimental.metrics.updateInterval, 42s);
+
+    // An absent UpdateInterval keeps the default
+    auto configDefault = Deserialize<ParticipantConfiguration>(R"(
+Experimental:
+  Metrics:
+    CollectFromRemote: true
+)");
+    EXPECT_EQ(configDefault.experimental.metrics.updateInterval, 1s);
+}
+
 TEST_F(Test_YamlParser, middleware_convert)
 {
     auto config = Deserialize<Middleware>(R"(

SilKit/source/config/Test_YamlValidator.cpp

@@ -55,7 +55,93 @@ Description: Sample configuration for CAN
     std::cout << "Yaml Validator warnings: " << warnings.str() << std::endl;
     EXPECT_TRUE(yamlValid) << "We ignore non-keyword errors and typos, but generate warnings!";
     EXPECT_GT(warnings.str().size(), 0u) << "Yaml Validator warnings: '" << warnings.str() << "'";
-    ;
+    // The warning must be complete: it names the field and the schema path it is ignored under
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("CanControllerss"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("is being ignored"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("schema path \"/\""));
+}
+
+TEST_F(Test_YamlValidator, validate_unknown_toplevel_arbitrary)
+{
+    auto yamlString = R"yaml(
+schemaVersion: 1
+ParticipantName: P1
+# a completely unknown field, not a near-miss of any keyword
+Foobar: true
+)yaml";
+
+    std::stringstream warnings;
+    bool yamlValid = ValidateWithSchema(yamlString, warnings);
+    std::cout << "Yaml Validator warnings: " << warnings.str() << std::endl;
+    EXPECT_TRUE(yamlValid) << "Unknown fields are warned about, not rejected";
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("Foobar"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("is being ignored"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("schema path \"/\""));
+}
+
+TEST_F(Test_YamlValidator, validate_unknown_nested)
+{
+    auto yamlString = R"yaml(
+schemaVersion: 1
+ParticipantName: P1
+Middleware:
+  Foobar: true
+CanControllers:
+  - Name: CAN1
+    Foobaz: 42
+)yaml";
+
+    std::stringstream warnings;
+    bool yamlValid = ValidateWithSchema(yamlString, warnings);
+    std::cout << "Yaml Validator warnings: " << warnings.str() << std::endl;
+    EXPECT_TRUE(yamlValid) << "Unknown nested fields are warned about, not rejected";
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("Foobar"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("schema path \"/Middleware\""));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("Foobaz"));
+    EXPECT_THAT(warnings.str(), testing::HasSubstr("schema path \"/CanControllers\""));
+}
+
+TEST_F(Test_YamlValidator, validate_logging_sink_experimental)
+{
+    // Regression: the reader reads EnabledTopics/DisabledTopics under a nested Experimental
+    // node, so the schema must accept that nesting without warnings.
+    auto yamlString = R"yaml(
+schemaVersion: 1
+ParticipantName: P1
+Logging:
+  Sinks:
+  - Type: Remote
+    Experimental:
+      EnabledTopics:
+      - TopicA
+      DisabledTopics:
+      - TopicB
+)yaml";
+
+    std::stringstream warnings;
+    bool yamlValid = ValidateWithSchema(yamlString, warnings);
+    EXPECT_TRUE(yamlValid) << "YamlValidator warnings: " << warnings.str();
+    EXPECT_EQ(warnings.str(), "") << "Nested Sink Experimental topics must validate cleanly";
+}
+
+TEST_F(Test_YamlValidator, validate_rpc_deprecated_channel_alias)
+{
+    // Regression: deprecated but still-supported RPC keys Channel/RpcChannel must not warn.
+    auto yamlString = R"yaml(
+schemaVersion: 1
+ParticipantName: P1
+RpcServers:
+  - Name: Server1
+    Channel: FuncA
+RpcClients:
+  - Name: Client1
+    RpcChannel: FuncA
+)yaml";
+
+    std::stringstream warnings;
+    bool yamlValid = ValidateWithSchema(yamlString, warnings);
+    EXPECT_TRUE(yamlValid) << "YamlValidator warnings: " << warnings.str();
+    EXPECT_EQ(warnings.str(), "") << "Deprecated RPC Channel aliases must validate cleanly";
 }
 
 TEST_F(Test_YamlValidator, validate_duplicate_element)
@@ -74,6 +160,32 @@ TEST_F(Test_YamlValidator, validate_duplicate_element)
     EXPECT_GT(warnings.str().size(), 0u);
 }
 
+TEST_F(Test_YamlValidator, validate_repeated_container_keys_in_sequence)
+{
+    // Regression: the same container key (e.g. Replay) appearing in different elements of a
+    // sequence must NOT be reported as a duplicate. Duplicate detection is per-map only.
+    auto yamlString = R"yaml(
+schemaVersion: 1
+ParticipantName: P1
+CanControllers:
+  - Name: CAN1
+    Replay:
+      UseTraceSource: Source1
+      MdfChannel:
+        ChannelName: Ch1
+  - Name: CAN2
+    Replay:
+      UseTraceSource: Source2
+      MdfChannel:
+        ChannelName: Ch2
+)yaml";
+
+    std::stringstream warnings;
+    bool yamlValid = ValidateWithSchema(yamlString, warnings);
+    EXPECT_TRUE(yamlValid) << "YamlValidator warnings: " << warnings.str();
+    EXPECT_EQ(warnings.str(), "") << "Repeated keys across sequence elements are not duplicates";
+}
+
 TEST_F(Test_YamlValidator, validate_unnamed_children)
 {
     auto yamlString = R"yaml(

SilKit/source/config/YamlReader.cpp

@@ -272,6 +272,11 @@ void YamlReader::Read(SilKit::Config::Metrics& obj)
 {
     OptionalRead(obj.sinks, "Sinks");
     OptionalRead(obj.collectFromRemote, "CollectFromRemote");
+
+    // UpdateInterval is an integer count of seconds; keep the default if the key is absent
+    std::chrono::seconds::rep updateIntervalSeconds{obj.updateInterval.count()};
+    OptionalRead(updateIntervalSeconds, "UpdateInterval");
+    obj.updateInterval = std::chrono::seconds{updateIntervalSeconds};
 }
 
 void YamlReader::Read(SilKit::Config::MdfChannel& obj)

SilKit/source/config/YamlValidator.cpp

@@ -143,6 +143,7 @@ const std::set<std::string> schemaPaths_v1 = {
     "/Experimental/Metrics/Sinks",
     "/Experimental/Metrics/Sinks/Name",
     "/Experimental/Metrics/Sinks/Type",
+    "/Experimental/Metrics/UpdateInterval",
     "/Experimental/TimeSynchronization",
     "/Experimental/TimeSynchronization/AnimationFactor",
     "/Experimental/TimeSynchronization/EnableMessageAggregation",
@@ -314,8 +315,9 @@ const std::set<std::string> schemaPaths_v1 = {
     "/Logging/Sinks/Level",
     "/Logging/Sinks/LogName",
     "/Logging/Sinks/Type",
-    "/Logging/Sinks/EnabledTopics",
-    "/Logging/Sinks/DisabledTopics",
+    "/Logging/Sinks/Experimental",
+    "/Logging/Sinks/Experimental/EnabledTopics",
+    "/Logging/Sinks/Experimental/DisabledTopics",
     "/Middleware",
     "/Middleware/AcceptorUris",
     "/Middleware/ConnectAttempts",
@@ -330,7 +332,9 @@ const std::set<std::string> schemaPaths_v1 = {
     "/Middleware/TcpSendBufferSize",
     "/ParticipantName",
     "/RpcClients",
+    "/RpcClients/Channel",
     "/RpcClients/FunctionName",
+    "/RpcClients/RpcChannel",
     "/RpcClients/Labels",
     "/RpcClients/Labels/Key",
     "/RpcClients/Labels/Kind",
@@ -348,7 +352,9 @@ const std::set<std::string> schemaPaths_v1 = {
     "/RpcClients/Replay/UseTraceSource",
     "/RpcClients/UseTraceSinks",
     "/RpcServers",
+    "/RpcServers/Channel",
     "/RpcServers/FunctionName",
+    "/RpcServers/RpcChannel",
     "/RpcServers/Labels",
     "/RpcServers/Labels/Key",
     "/RpcServers/Labels/Kind",
@@ -460,7 +466,6 @@ struct ValidatingVisitor
     std::ostream& warnings;
     std::string currentNodePath;
     std::deque<std::string> nodePaths;
-    std::set<std::string> userDefinedPaths;
     ryml::Parser& parser;
 
     bool ok{true};
@@ -477,12 +482,6 @@ struct ValidatingVisitor
     ValidatingVisitor(const ValidatingVisitor&) = delete;
     ValidatingVisitor& operator=(const ValidatingVisitor&) = delete;
 
-    bool PathIsAlreadyDefined(const std::string& path)
-    {
-        auto it = userDefinedPaths.insert(path);
-        return !std::get<1>(it);
-    }
-
     auto GetCurrentLocation(ryml::ConstNodeRef node) -> std::string
     {
         auto&& location = parser.location(node);
@@ -503,20 +502,35 @@ struct ValidatingVisitor
 
     void push(ryml::ConstNodeRef node, ryml::id_type /* level */)
     {
+        // Detect keys defined more than once within the same map (real YAML duplicate keys).
+        // This is scoped to a single map's direct children; the same key appearing in
+        // different elements of a sequence is not a duplicate.
+        if (node.is_map())
+        {
+            std::set<std::string> seenKeys;
+            for (auto child : node.children())
+            {
+                if (!child.has_key())
+                {
+                    continue;
+                }
+                auto childKey = to_string(child.key());
+                if (!seenKeys.insert(childKey).second)
+                {
+                    warnings << "At " << GetCurrentLocation(child) << ": Element \"" << childKey << "\""
+                             << " is already defined in path \"" << currentNodePath << "\"\n";
+                    ok &= false;
+                }
+            }
+        }
+
         if (!node.has_key())
         {
             return;
         }
 
         auto nodeKey = to_string(node.key());
         auto nodePath = MakePath(currentNodePath, nodeKey);
-        if (PathIsAlreadyDefined(nodePath))
-        {
-            warnings << "At " << GetCurrentLocation(node) << ": Element \"" << nodePath << "\""
-                     << " is already defined in path \"" << currentNodePath << "\"\n";
-            ok &= false;
-        }
-
         nodePaths.push_back(nodePath);
         currentNodePath = nodePaths.back();
         return;
@@ -546,9 +560,10 @@ struct ValidatingVisitor
             }
             else
             {
-                // We only report error if the element is a reserved keyword
+                // Unknown, non-reserved elements are only warned about, not treated as errors
                 warnings << "At " << GetCurrentLocation(node) << ": Element \"" << nodeName << "\""
-                         << " is being ignored. It is not a sub-element of schema path \"";
+                         << " is being ignored. It is not a sub-element of schema path \"" << currentNodePath
+                         << "\"\n";
             }
         }
     }
@@ -627,6 +642,12 @@ bool ValidateWithSchema(const std::string& yamlString, std::ostream& warnings)
             }
         }
 
+        if (!root.is_map() && !root.is_seq())
+        {
+            // Empty or scalar documents have no fields to validate against the schema
+            return true;
+        }
+
         ValidatingVisitor visitor{parser, warnings};
         visit_stacked(root, visitor);
         return visitor.IsValid();

SilKit/source/config/YamlWriter.cpp

@@ -283,12 +283,15 @@ void YamlWriter::Write(const SilKit::Config::MetricsSink& obj)
 
 void YamlWriter::Write(const SilKit::Config::Metrics& obj)
 {
+    static const SilKit::Config::Metrics defaultObj{};
     MakeMap();
     OptionalWrite(obj.sinks, "Sinks");
     if (obj.collectFromRemote.has_value())
     {
         WriteKeyValue("CollectFromRemote", obj.collectFromRemote.value());
     }
+    // UpdateInterval is serialized as an integer count of seconds
+    NonDefaultWrite(obj.updateInterval.count(), "UpdateInterval", defaultObj.updateInterval.count());
 }