This is an automated email from the ASF dual-hosted git repository. martinzink pushed a commit to branch minifi-api-property in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit c7c7a6bf31a417664b894a4031ba26bdf5ab3245 Author: Martin Zink <[email protected]> AuthorDate: Tue Mar 18 13:57:57 2025 +0100 Fail on invalid property sequence and tests --- .../tests/unit/FlowJsonTests.cpp | 39 ++++++++++++++++++++++ .../tests/unit/YamlConfigurationTests.cpp | 36 ++++++++++++++++++++ .../src/core/flow/StructuredConfiguration.cpp | 15 +++++++-- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/extensions/standard-processors/tests/unit/FlowJsonTests.cpp b/extensions/standard-processors/tests/unit/FlowJsonTests.cpp index 2e8db59a4..97ac9056c 100644 --- a/extensions/standard-processors/tests/unit/FlowJsonTests.cpp +++ b/extensions/standard-processors/tests/unit/FlowJsonTests.cpp @@ -32,6 +32,8 @@ #include "utils/crypto/property_encryption/PropertyEncryptionUtils.h" #include "unit/TestUtils.h" #include "unit/DummyParameterProvider.h" +#include "catch2/generators/catch_generators.hpp" + using namespace std::literals::chrono_literals; @@ -190,6 +192,43 @@ TEST_CASE("NiFi flow json format is correctly parsed") { CHECK(connection2->getRelationships() == (std::set<core::Relationship>{{"success", ""}})); } +TEST_CASE("Cannot use invalid property values") { + ConfigurationTestController test_controller; + core::flow::AdaptiveConfiguration config(test_controller.getContext()); + auto [file_size_property_str, throws] = GENERATE( + std::make_tuple(R"("1 kB")", false), + std::make_tuple(R"([{"value": "1 kB"},{"value": "2 kB"}])", false), + std::make_tuple(R"("foo")", true), + std::make_tuple(R"([{"value": "1 kB"},{"value": "bar"}])", true)); + const std::string CONFIG_JSON = fmt::format( + R"( +{{ + "rootGroup": {{ + "name": "MiNiFi Flow", + "processors": [{{ + "identifier": "00000000-0000-0000-0000-000000000001", + "name": "MyGenFF", + "type": "org.apache.nifi.processors.standard.GenerateFlowFile", + "concurrentlySchedulableTaskCount": 15, + "schedulingStrategy": "TIMER_DRIVEN", + "schedulingPeriod": "3 sec", + "penaltyDuration": "12 sec", + "yieldDuration": "4 sec", + "runDurationMillis": 12, + "autoTerminatedRelationships": ["success"], + "properties": {{ + "File Size": {} + }} + }}] + }} +}})", file_size_property_str); + if (throws) { + REQUIRE_THROWS_WITH(config.getRootFromPayload(CONFIG_JSON), "Unable to parse configuration file for component named 'MyGenFF' because ValidationFailed"); + } else { + REQUIRE_NOTHROW(config.getRootFromPayload(CONFIG_JSON)); + } +} + TEST_CASE("Parameters from different parameter contexts should not be replaced") { ConfigurationTestController test_controller; diff --git a/extensions/standard-processors/tests/unit/YamlConfigurationTests.cpp b/extensions/standard-processors/tests/unit/YamlConfigurationTests.cpp index 09ecab861..1232657c6 100644 --- a/extensions/standard-processors/tests/unit/YamlConfigurationTests.cpp +++ b/extensions/standard-processors/tests/unit/YamlConfigurationTests.cpp @@ -32,6 +32,7 @@ #include "core/Resource.h" #include "utils/crypto/property_encryption/PropertyEncryptionUtils.h" #include "unit/DummyProcessor.h" +#include "catch2/generators/catch_generators.hpp" using namespace std::literals::chrono_literals; @@ -2364,4 +2365,39 @@ Parameter Context Name: dummycontext "exists with no parameter provider or generated by other parameter provider"); } +TEST_CASE("Cannot use invalid property values") { + ConfigurationTestController test_controller; + core::flow::AdaptiveConfiguration config(test_controller.getContext()); + auto [file_size_property_str, throws] = GENERATE( + std::make_tuple(R"(1 kB)", false), + std::make_tuple(R"( + - value: 1 kB + - value: 2 MB)", false), + std::make_tuple(R"(foo)", true), + std::make_tuple(R"( + - value: 1 kB + - value: bar KB)", true)); + const std::string CONFIG_YAML = fmt::format( + R"( +MiNiFi Config Version: 3 +Flow Controller: + name: root + comment: '' +Processors: +- id: 0eac51eb-d76c-4ba6-9f0c-351795b2d243 + name: GenerateFlowFile1 + class: org.apache.nifi.minifi.processors.GenerateFlowFile + max concurrent tasks: 1 + scheduling strategy: TIMER_DRIVEN + scheduling period: 10000 ms + Properties: + File Size: {} +)", file_size_property_str); + if (throws) { + REQUIRE_THROWS_WITH(config.getRootFromPayload(CONFIG_YAML), "Unable to parse configuration file for component named 'GenerateFlowFile1' because ValidationFailed"); + } else { + REQUIRE_NOTHROW(config.getRootFromPayload(CONFIG_YAML)); + } +} + } // namespace org::apache::nifi::minifi::test diff --git a/libminifi/src/core/flow/StructuredConfiguration.cpp b/libminifi/src/core/flow/StructuredConfiguration.cpp index bb572930f..44f6f14bd 100644 --- a/libminifi/src/core/flow/StructuredConfiguration.cpp +++ b/libminifi/src/core/flow/StructuredConfiguration.cpp @@ -32,6 +32,7 @@ #include "utils/TimeUtil.h" #include "utils/crypto/property_encryption/PropertyEncryptionUtils.h" #include "utils/PropertyErrors.h" +#include "utils/Error.h" // for more readable std::error_code fmt::formatter namespace org::apache::nifi::minifi::core::flow { @@ -787,13 +788,21 @@ void StructuredConfiguration::parsePropertyValueSequence(const std::string& prop logger_->log_debug("Found property {}", property_name); const auto append_prop_result = component.appendProperty(property_name, rawValueString); - if (!append_prop_result && append_prop_result.error() == make_error_code(PropertyErrorCode::NotSupportedProperty)) { + if (append_prop_result) { + continue; + } + + if (append_prop_result.error() == make_error_code(PropertyErrorCode::NotSupportedProperty)) { logger_->log_warn("Received property {} with value {} but is not one of the properties for {}. Attempting to add as dynamic property.", property_name, rawValueString, component.getName()); - if (!component.appendDynamicProperty(property_name, rawValueString)) { - logger_->log_warn("Unable to set the dynamic property {}", property_name); + const auto append_dynamic_prop_result = component.appendDynamicProperty(property_name, rawValueString); + if (!append_dynamic_prop_result) { + logger_->log_error("Unable to set the dynamic property {} due to {}", property_name, append_dynamic_prop_result.error()); } else { logger_->log_warn("Dynamic property {} has been set", property_name); } + } else { + logger_->log_error("Failed to set {} property due to {}", property_name, append_prop_result.error()); + raiseComponentError(component.getName(), "", append_prop_result.error().message()); } } }
