Repository: nifi-minifi-cpp Updated Branches: refs/heads/master f812fca2c -> b3818fff6
MINIFICPP-466 Implement enforcement of dependent property validation This closes #342. Signed-off-by: Aldrin Piri <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/commit/b3818fff Tree: http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/tree/b3818fff Diff: http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/diff/b3818fff Branch: refs/heads/master Commit: b3818fff65438a1f32a89067ca7cdb968a619540 Parents: f812fca Author: Andrew I. Christianson <[email protected]> Authored: Fri May 25 10:03:55 2018 -0400 Committer: Aldrin Piri <[email protected]> Committed: Tue Jul 24 16:23:17 2018 -0400 ---------------------------------------------------------------------- libminifi/include/core/yaml/YamlConfiguration.h | 23 +++++ libminifi/src/core/yaml/YamlConfiguration.cpp | 59 +++++++++---- libminifi/test/unit/YamlConfigurationTests.cpp | 92 ++++++++++++++++---- 3 files changed, 143 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/blob/b3818fff/libminifi/include/core/yaml/YamlConfiguration.h ---------------------------------------------------------------------- diff --git a/libminifi/include/core/yaml/YamlConfiguration.h b/libminifi/include/core/yaml/YamlConfiguration.h index 1d0a332..09edd26 100644 --- a/libminifi/include/core/yaml/YamlConfiguration.h +++ b/libminifi/include/core/yaml/YamlConfiguration.h @@ -111,6 +111,18 @@ class YamlConfiguration : public FlowConfiguration { return getYamlRoot(&rootYamlNode); } + /** + * Iterates all component property validation rules and checks that configured state + * is valid. If state is determined to be invalid, conf parsing ends and an error is raised. + * + * @param component + * @param component_name + * @param yaml_section + */ + void validateComponentProperties(const std::shared_ptr<ConfigurableComponent> &component, + const std::string &component_name, + const std::string &yaml_section) const; + protected: /** @@ -312,6 +324,17 @@ class YamlConfiguration : public FlowConfiguration { private: std::shared_ptr<logging::Logger> logger_; static std::shared_ptr<utils::IdGenerator> id_generator_; + + /** + * Raises a human-readable configuration error for the given configuration component/section. + * + * @param component_name + * @param yaml_section + * @param reason + */ + void raiseComponentError(const std::string &component_name, + const std::string &yaml_section, + const std::string &reason) const; }; } /* namespace core */ http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/blob/b3818fff/libminifi/src/core/yaml/YamlConfiguration.cpp ---------------------------------------------------------------------- diff --git a/libminifi/src/core/yaml/YamlConfiguration.cpp b/libminifi/src/core/yaml/YamlConfiguration.cpp index b8d777f..a41b449 100644 --- a/libminifi/src/core/yaml/YamlConfiguration.cpp +++ b/libminifi/src/core/yaml/YamlConfiguration.cpp @@ -782,29 +782,58 @@ void YamlConfiguration::parsePropertiesNodeYaml(YAML::Node *propertiesNode, } } + validateComponentProperties(processor, component_name, yaml_section); +} + +void YamlConfiguration::validateComponentProperties(const std::shared_ptr<ConfigurableComponent> &component, + const std::string &component_name, + const std::string &yaml_section) const { + const auto &component_properties = component->getProperties(); + // Validate required properties - for (const auto &prop_pair : processor->getProperties()) { + for (const auto &prop_pair : component_properties) { if (prop_pair.second.getRequired()) { - const auto &val = prop_pair.second.getValue(); - - if (val.empty()) { - // Build a helpful error message for the user so they can fix the - // invalid YAML config file, using the component name if present - std::string err_msg = - "Unable to parse configuration file for component named '" - + component_name - + "' because required property '" + prop_pair.second.getName() + "' is not set"; - if (!yaml_section.empty()) { - err_msg += " [in '" + yaml_section + "' section of configuration file]"; - } - logging::LOG_ERROR(logger_) << err_msg; + if (prop_pair.second.getValue().empty()) { + std::string reason("required property '"); + reason.append(prop_pair.second.getName()); + reason.append("' is not set"); + raiseComponentError(component_name, yaml_section, reason); + } + } + } - throw std::invalid_argument(err_msg); + // Validate dependent properties + for (const auto &prop_pair : component_properties) { + const auto &dep_props = prop_pair.second.getDependentProperties(); + + for (const auto &dep_prop_key : dep_props) { + if (component_properties.at(dep_prop_key).getValue().empty()) { + std::string reason("property '"); + reason.append(prop_pair.second.getName()); + reason.append("' depends on property '"); + reason.append(dep_prop_key); + reason.append("' which is not set"); + raiseComponentError(component_name, yaml_section, reason); } } } } +void YamlConfiguration::raiseComponentError(const std::string &component_name, + const std::string &yaml_section, + const std::string &reason) const { + std::string err_msg = "Unable to parse configuration file for component named '"; + err_msg.append(component_name); + err_msg.append("' because " + reason); + if (!yaml_section.empty()) { + err_msg.append(" [in '" + yaml_section + "' section of configuration file]"); + } + + logging::LOG_ERROR(logger_) << err_msg; + + throw std::invalid_argument(err_msg); +} + std::string YamlConfiguration::getOrGenerateId(YAML::Node *yamlNode, const std::string &idField) { std::string id; YAML::Node node = yamlNode->as<YAML::Node>(); http://git-wip-us.apache.org/repos/asf/nifi-minifi-cpp/blob/b3818fff/libminifi/test/unit/YamlConfigurationTests.cpp ---------------------------------------------------------------------- diff --git a/libminifi/test/unit/YamlConfigurationTests.cpp b/libminifi/test/unit/YamlConfigurationTests.cpp index b05c402..61fe7f1 100644 --- a/libminifi/test/unit/YamlConfigurationTests.cpp +++ b/libminifi/test/unit/YamlConfigurationTests.cpp @@ -33,8 +33,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") { std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::VolatileContentRepository>(); - core::YamlConfiguration *yamlConfig = - new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); SECTION("loading YAML without optional component IDs works") { static const std::string CONFIG_YAML_WITHOUT_IDS = "" @@ -139,7 +138,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") { " batch size: 1000"; std::istringstream configYamlStream(CONFIG_YAML_WITHOUT_IDS); - std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream); + std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream); REQUIRE(rootFlowConfig); REQUIRE(rootFlowConfig->findProcessor("TailFile")); @@ -187,7 +186,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") { "\n"; std::istringstream configYamlStream(CONFIG_YAML_NO_RPG_PORT_ID); - REQUIRE_THROWS_AS(yamlConfig->getYamlRoot(configYamlStream), std::invalid_argument); + REQUIRE_THROWS_AS(yamlConfig.getYamlRoot(configYamlStream), std::invalid_argument); } } @@ -200,8 +199,7 @@ TEST_CASE("Test YAML v3 Config Processing", "[YamlConfiguration3]") { std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::VolatileContentRepository>(); - core::YamlConfiguration *yamlConfig = - new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); static const std::string TEST_CONFIG_YAML = R"( MiNiFi Config Version: 3 @@ -314,7 +312,7 @@ Remote Process Groups: NiFi Properties Overrides: {} )"; std::istringstream configYamlStream(TEST_CONFIG_YAML); - std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream); + std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream); REQUIRE(rootFlowConfig); REQUIRE(rootFlowConfig->findProcessor("TailFile")); @@ -353,8 +351,7 @@ TEST_CASE("Test Dynamic Unsupported", "[YamlConfigurationDynamicUnsupported]") { std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::VolatileContentRepository>(); - core::YamlConfiguration *yamlConfig = - new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); static const std::string TEST_CONFIG_YAML = R"( Flow Controller: @@ -366,7 +363,7 @@ Processors: Dynamic Property: Bad )"; std::istringstream configYamlStream(TEST_CONFIG_YAML); - std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream); + std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream); REQUIRE(rootFlowConfig); REQUIRE(rootFlowConfig->findProcessor("PutFile")); @@ -391,8 +388,7 @@ TEST_CASE("Test Required Property", "[YamlConfigurationRequiredProperty]") { std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::VolatileContentRepository>(); - core::YamlConfiguration *yamlConfig = - new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); static const std::string TEST_CONFIG_YAML = R"( Flow Controller: @@ -408,7 +404,7 @@ Processors: bool caught_exception = false; try { - std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream); + std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream); REQUIRE(rootFlowConfig); REQUIRE(rootFlowConfig->findProcessor("GetFile")); @@ -436,8 +432,7 @@ TEST_CASE("Test Required Property 2", "[YamlConfigurationRequiredProperty2]") { std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::VolatileContentRepository>(); - core::YamlConfiguration *yamlConfig = - new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); static const std::string TEST_CONFIG_YAML = R"( Flow Controller: @@ -450,10 +445,75 @@ Processors: Batch Size: 1 )"; std::istringstream configYamlStream(TEST_CONFIG_YAML); - std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream); + std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream); REQUIRE(rootFlowConfig); REQUIRE(rootFlowConfig->findProcessor("XYZ")); REQUIRE(NULL != rootFlowConfig->findProcessor("XYZ")->getUUID()); REQUIRE(!rootFlowConfig->findProcessor("XYZ")->getUUIDStr().empty()); } + +class DummyComponent : public core::ConfigurableComponent { + public: + bool supportsDynamicProperties() override { + return false; + } + + bool canEdit() override { + return true; + } +}; + +TEST_CASE("Test Dependent Property", "[YamlConfigurationDependentProperty]") { + TestController test_controller; + + LogTestController &logTestController = LogTestController::getInstance(); + logTestController.setDebug<TestPlan>(); + logTestController.setDebug<core::YamlConfiguration>(); + + std::shared_ptr<core::Repository> testProvRepo = core::createRepository("provenancerepository", true); + std::shared_ptr<core::Repository> testFlowFileRepo = core::createRepository("flowfilerepository", true); + std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>(); + std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); + std::shared_ptr<core::ContentRepository> + content_repo = std::make_shared<core::repository::VolatileContentRepository>(); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + const auto component = std::make_shared<DummyComponent>(); + std::set<core::Property> props; + props.emplace(core::Property("Prop A", "Prop A desc", "val A", true, {}, {})); + props.emplace(core::Property("Prop B", "Prop B desc", "val B", true, {"Prop A"}, {})); + component->setSupportedProperties(std::move(props)); + yamlConfig.validateComponentProperties(component, "component A", "section A"); + REQUIRE(true); // Expected to get here w/o any exceptions +} + +TEST_CASE("Test Dependent Property 2", "[YamlConfigurationDependentProperty2]") { + TestController test_controller; + + LogTestController &logTestController = LogTestController::getInstance(); + logTestController.setDebug<TestPlan>(); + logTestController.setDebug<core::YamlConfiguration>(); + + std::shared_ptr<core::Repository> testProvRepo = core::createRepository("provenancerepository", true); + std::shared_ptr<core::Repository> testFlowFileRepo = core::createRepository("flowfilerepository", true); + std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>(); + std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration); + std::shared_ptr<core::ContentRepository> + content_repo = std::make_shared<core::repository::VolatileContentRepository>(); + core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration); + const auto component = std::make_shared<DummyComponent>(); + std::set<core::Property> props; + props.emplace(core::Property("Prop A", "Prop A desc", "", false, {}, {})); + props.emplace(core::Property("Prop B", "Prop B desc", "val B", true, {"Prop A"}, {})); + component->setSupportedProperties(std::move(props)); + bool config_failed = false; + try { + yamlConfig.validateComponentProperties(component, "component A", "section A"); + } catch (const std::exception &e) { + config_failed = true; + REQUIRE("Unable to parse configuration file for component named 'component A' because property " + "'Prop B' depends on property 'Prop A' which is not set " + "[in 'section A' section of configuration file]" == std::string(e.what())); + } + REQUIRE(config_failed); +}
