You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/08/17 12:32:59 UTC
[GitHub] [nifi-minifi-cpp] hunyadi-dev opened a new pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
hunyadi-dev opened a new pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874
This is a refactor and test only PR - done as prework for name and UUID usage rework. It is probably simplest to **review this PR on a per commit basis**.
This PR fixes faulty scenarios for parsing configuration values related to connections.
1. We agreed to fix this one:
https://issues.apache.org/jira/browse/MINIFICPP-1331
1. The others are documented here:
**[https://cloudera.atlassian.net/wiki/spaces/ENG/pages/796853901/MiNiFi+known+configuration+issues](https://cloudera.atlassian.net/wiki/spaces/ENG/pages/796853901/MiNiFi+known+configuration+issues)**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480146776
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
+ }
+ SECTION("Source and destination names and uuids are read") {
+ const utils::Identifier expected_source_id = utils::generateUUID();
+ const utils::Identifier expected_destination_id = utils::generateUUID();
+ std::string serialized_yaml;
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_1", expected_source_id)));
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_2", expected_destination_id)));
+ SECTION("Directly from configuration") {
+ serialized_yaml = std::string {
+ "source id: " + expected_source_id.to_string() + "\n"
+ "destination id: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Using UUID as remote port id") {
+ serialized_yaml = std::string {
+ "source name: " + expected_source_id.to_string() + "\n"
+ "destination name: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Via processor name lookup") {
+ serialized_yaml = std::string {
+ "source name: TailFile_1\n"
+ "destination name: TailFile_2\n" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(expected_source_id == yaml_connection_parser.getSourceUUIDFromYaml());
+ REQUIRE(expected_destination_id == yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Flow file expiration is read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "flowfile expiration: 2 min\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(120000 == yaml_connection_parser.getFlowFileExpirationFromYaml()); // 2 * 60 * 1000 ms
+ }
+ SECTION("Drop empty value is read") {
+ SECTION("When config contains true value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: true\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(true == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("When config contains false value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: false\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(false == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ }
+ SECTION("Errors are handled properly when configuration lines are missing") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ SECTION("With empty configuration") {
+ YAML::Node connection_node = YAML::Load(std::string(""));
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that lists keys but has no assigned values") {
+ std::string serialized_yaml;
+ SECTION("Single relationship name left empty") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ // This seems incorrect, but we do not want to ruin backward compatibility
+ CHECK_NOTHROW(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("List of relationship names contains empty item") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source relationship names:\n"
+ "- \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("Source and destination lookup from via id") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source id: \n"
+ "destination id: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Source and destination lookup via name") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: \n"
+ "max work queue data size: \n"
+ "flowfile expiration: \n"
+ "drop empty: \n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that has values of incorrect format") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 2 KB\n"
+ "max work queue data size: 10 Incorrect\n"
+ "flowfile expiration: 12\n"
+ "drop empty: sup\n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
Review comment:
I understand we would like to maintain backward compatibility, so would we only throw when the strict parsing is ready?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r483683083
##########
File path: libminifi/src/core/yaml/CheckRequiredField.cpp
##########
@@ -0,0 +1,55 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdexcept>
+
+#include "core/yaml/CheckRequiredField.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+namespace yaml {
+
+void checkRequiredField(const YAML::Node *yamlNode, const std::string &fieldName, const std::shared_ptr<logging::Logger>& logger, const std::string &yamlSection, const std::string &errorMessage) {
+ std::string errMsg = errorMessage;
+ if (!yamlNode->as<YAML::Node>()[fieldName]) {
+ if (errMsg.empty()) {
+ const YAML::Node name_node = yamlNode->as<YAML::Node>()["name"];
+ // Build a helpful error message for the user so they can fix the
+ // invalid YAML config file, using the component name if present
+ errMsg =
+ name_node ?
+ "Unable to parse configuration file for component named '" + name_node.as<std::string>() + "' as required field '" + fieldName + "' is missing" :
+ "Unable to parse configuration file as required field '" + fieldName + "' is missing";
+ if (!yamlSection.empty()) {
+ errMsg += " [in '" + yamlSection + "' section of configuration file]";
+ }
Review comment:
Added as requested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480192949
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
But the code \*is\* self-documenting, the number it shows is obvious. I would definitely change this if I saw any chance that anyone who reads this couldn't immediately understand what the number means from the context.
Just for reference, I asked my girlfriend who has nothing to do with coding what it could mean that we read in 12 MB and get the number 12582912 and she could immediately tell that this is probably the number of bytes.
However, I decided to go further than this, and add an explanation, and my choice of style is a comment for the reason mentioned above. I call this 100% bikeshedding: https://www.wikiwand.com/en/Law_of_triviality
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480147201
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
Both versions are obvious and - maybe I am wrong but - I have the gut feeling that this compiles ever-so-slightly quicker. I will change it if you convince me there is no compile-time penalty :D
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480144761
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
+ }
+ SECTION("Source and destination names and uuids are read") {
+ const utils::Identifier expected_source_id = utils::generateUUID();
+ const utils::Identifier expected_destination_id = utils::generateUUID();
+ std::string serialized_yaml;
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_1", expected_source_id)));
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_2", expected_destination_id)));
+ SECTION("Directly from configuration") {
+ serialized_yaml = std::string {
+ "source id: " + expected_source_id.to_string() + "\n"
+ "destination id: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Using UUID as remote port id") {
+ serialized_yaml = std::string {
+ "source name: " + expected_source_id.to_string() + "\n"
+ "destination name: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Via processor name lookup") {
+ serialized_yaml = std::string {
+ "source name: TailFile_1\n"
+ "destination name: TailFile_2\n" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(expected_source_id == yaml_connection_parser.getSourceUUIDFromYaml());
+ REQUIRE(expected_destination_id == yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Flow file expiration is read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "flowfile expiration: 2 min\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(120000 == yaml_connection_parser.getFlowFileExpirationFromYaml()); // 2 * 60 * 1000 ms
+ }
+ SECTION("Drop empty value is read") {
+ SECTION("When config contains true value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: true\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(true == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("When config contains false value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: false\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(false == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ }
+ SECTION("Errors are handled properly when configuration lines are missing") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ SECTION("With empty configuration") {
+ YAML::Node connection_node = YAML::Load(std::string(""));
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that lists keys but has no assigned values") {
+ std::string serialized_yaml;
+ SECTION("Single relationship name left empty") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ // This seems incorrect, but we do not want to ruin backward compatibility
+ CHECK_NOTHROW(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("List of relationship names contains empty item") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source relationship names:\n"
+ "- \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("Source and destination lookup from via id") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source id: \n"
+ "destination id: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Source and destination lookup via name") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: \n"
+ "max work queue data size: \n"
+ "flowfile expiration: \n"
+ "drop empty: \n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that has values of incorrect format") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 2 KB\n"
+ "max work queue data size: 10 Incorrect\n"
+ "flowfile expiration: 12\n"
+ "drop empty: sup\n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
Review comment:
why are we verifying incorrect behavior?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480151533
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
leave it if you can tell me what `64 * 1024 * 1024` is faster than the compiler, it is about having a single source of truth, and in this case we could express in code, what is written in comment (me moving ~~in my chair~~ the cursor during compilation probably has a greater effect on the compilation time than this multiplication)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480189463
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
This is how I solved the same problem some time ago: https://github.com/apache/nifi-minifi-cpp/pull/875/files#diff-42dd8bec1f7b514486d8de629c9a608eR64
Take what you like.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev closed pull request #874: MINIFICPP-1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev closed pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480136156
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
we could move the comment and let the compiler do the multiplication
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480170258
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
> But remember: while comments are very important, the best code is self-documenting.
from [this](https://google.github.io/styleguide/cppguide.html#Comments)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
arpadboda commented on pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#issuecomment-675077375
@hunyadi-dev : quite a lot of CI failures, could you take a look at them before people start reviewing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480164518
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
I do not think you not-liking comments because they can lie is a good excuse for asking this change. Using comments for explaining a calculation is my choice of style (and I gave you the reason, I think it theoretically - albeit rather slowly - might increase compilation time).
Why do you think as the author of this PR, I should follow your favourite way of doing things, when the current proposal is just as expressive? Are you not being a bit arrogant?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480146776
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
+ }
+ SECTION("Source and destination names and uuids are read") {
+ const utils::Identifier expected_source_id = utils::generateUUID();
+ const utils::Identifier expected_destination_id = utils::generateUUID();
+ std::string serialized_yaml;
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_1", expected_source_id)));
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_2", expected_destination_id)));
+ SECTION("Directly from configuration") {
+ serialized_yaml = std::string {
+ "source id: " + expected_source_id.to_string() + "\n"
+ "destination id: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Using UUID as remote port id") {
+ serialized_yaml = std::string {
+ "source name: " + expected_source_id.to_string() + "\n"
+ "destination name: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Via processor name lookup") {
+ serialized_yaml = std::string {
+ "source name: TailFile_1\n"
+ "destination name: TailFile_2\n" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(expected_source_id == yaml_connection_parser.getSourceUUIDFromYaml());
+ REQUIRE(expected_destination_id == yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Flow file expiration is read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "flowfile expiration: 2 min\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(120000 == yaml_connection_parser.getFlowFileExpirationFromYaml()); // 2 * 60 * 1000 ms
+ }
+ SECTION("Drop empty value is read") {
+ SECTION("When config contains true value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: true\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(true == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("When config contains false value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: false\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(false == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ }
+ SECTION("Errors are handled properly when configuration lines are missing") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ SECTION("With empty configuration") {
+ YAML::Node connection_node = YAML::Load(std::string(""));
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that lists keys but has no assigned values") {
+ std::string serialized_yaml;
+ SECTION("Single relationship name left empty") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ // This seems incorrect, but we do not want to ruin backward compatibility
+ CHECK_NOTHROW(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("List of relationship names contains empty item") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source relationship names:\n"
+ "- \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("Source and destination lookup from via id") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source id: \n"
+ "destination id: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Source and destination lookup via name") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: \n"
+ "max work queue data size: \n"
+ "flowfile expiration: \n"
+ "drop empty: \n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that has values of incorrect format") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 2 KB\n"
+ "max work queue data size: 10 Incorrect\n"
+ "flowfile expiration: 12\n"
+ "drop empty: sup\n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
Review comment:
I understand we would like to maintain backward compatibility, so would we only throw when the strict parsing is ready?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480145721
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
+ }
+ SECTION("Source and destination names and uuids are read") {
+ const utils::Identifier expected_source_id = utils::generateUUID();
+ const utils::Identifier expected_destination_id = utils::generateUUID();
+ std::string serialized_yaml;
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_1", expected_source_id)));
+ parent.addProcessor(std::static_pointer_cast<core::Processor>(std::make_shared<processors::TailFile>("TailFile_2", expected_destination_id)));
+ SECTION("Directly from configuration") {
+ serialized_yaml = std::string {
+ "source id: " + expected_source_id.to_string() + "\n"
+ "destination id: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Using UUID as remote port id") {
+ serialized_yaml = std::string {
+ "source name: " + expected_source_id.to_string() + "\n"
+ "destination name: " + expected_destination_id.to_string() + "\n" };
+ }
+ SECTION("Via processor name lookup") {
+ serialized_yaml = std::string {
+ "source name: TailFile_1\n"
+ "destination name: TailFile_2\n" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(expected_source_id == yaml_connection_parser.getSourceUUIDFromYaml());
+ REQUIRE(expected_destination_id == yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Flow file expiration is read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "flowfile expiration: 2 min\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(120000 == yaml_connection_parser.getFlowFileExpirationFromYaml()); // 2 * 60 * 1000 ms
+ }
+ SECTION("Drop empty value is read") {
+ SECTION("When config contains true value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: true\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(true == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("When config contains false value") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "drop empty: false\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(false == yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ }
+ SECTION("Errors are handled properly when configuration lines are missing") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ SECTION("With empty configuration") {
+ YAML::Node connection_node = YAML::Load(std::string(""));
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that lists keys but has no assigned values") {
+ std::string serialized_yaml;
+ SECTION("Single relationship name left empty") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ // This seems incorrect, but we do not want to ruin backward compatibility
+ CHECK_NOTHROW(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("List of relationship names contains empty item") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source relationship names:\n"
+ "- \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection));
+ }
+ SECTION("Source and destination lookup from via id") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source id: \n"
+ "destination id: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ SECTION("Source and destination lookup via name") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "source name: \n"
+ "destination name: \n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getSourceUUIDFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDestinationUUIDFromYaml());
+ }
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: \n"
+ "max work queue data size: \n"
+ "flowfile expiration: \n"
+ "drop empty: \n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getFlowFileExpirationFromYaml());
+ CHECK_THROWS(yaml_connection_parser.getDropEmptyFromYaml());
+ }
+ SECTION("With a configuration that has values of incorrect format") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 2 KB\n"
+ "max work queue data size: 10 Incorrect\n"
+ "flowfile expiration: 12\n"
+ "drop empty: sup\n"});
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueSizeFromYaml());
+ CHECK_NOTHROW(yaml_connection_parser.getWorkQueueDataSizeFromYaml());
Review comment:
We need to be backward compatible with configuration files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480165898
##########
File path: libminifi/src/core/yaml/CheckRequiredField.cpp
##########
@@ -0,0 +1,55 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdexcept>
+
+#include "core/yaml/CheckRequiredField.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+namespace yaml {
+
+void checkRequiredField(const YAML::Node *yamlNode, const std::string &fieldName, const std::shared_ptr<logging::Logger>& logger, const std::string &yamlSection, const std::string &errorMessage) {
+ std::string errMsg = errorMessage;
+ if (!yamlNode->as<YAML::Node>()[fieldName]) {
+ if (errMsg.empty()) {
+ const YAML::Node name_node = yamlNode->as<YAML::Node>()["name"];
+ // Build a helpful error message for the user so they can fix the
+ // invalid YAML config file, using the component name if present
+ errMsg =
+ name_node ?
+ "Unable to parse configuration file for component named '" + name_node.as<std::string>() + "' as required field '" + fieldName + "' is missing" :
+ "Unable to parse configuration file as required field '" + fieldName + "' is missing";
+ if (!yamlSection.empty()) {
+ errMsg += " [in '" + yamlSection + "' section of configuration file]";
+ }
Review comment:
it seems like we could access the line and column number of the `YAML::Node` with the `Mark` method, could this provide even more info to the user?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on pull request #874: MINIFICPP-1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#issuecomment-736764240
Closing this PR and opening a new one for the same content rebased.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
hunyadi-dev commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480147201
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
Maybe I am wrong, but I have the gut feeling that this compiles ever-so-slightly quicker. I will change it if you convince me there is no compile-time penalty :D
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #874: Minificpp 1325 - Refactor and test YAML connection parsing
Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #874:
URL: https://github.com/apache/nifi-minifi-cpp/pull/874#discussion_r480185712
##########
File path: extensions/standard-processors/tests/unit/YamlConnectionParserTest.cpp
##########
@@ -0,0 +1,200 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "core/yaml/YamlConnectionParser.h"
+
+#include "core/yaml/YamlConfiguration.h"
+#include "TailFile.h"
+#include "TestBase.h"
+#include "utils/TestUtils.h"
+
+namespace {
+
+using org::apache::nifi::minifi::core::yaml::YamlConnectionParser;
+using org::apache::nifi::minifi::core::YamlConfiguration;
+using RetryFlowFile = org::apache::nifi::minifi::processors::TailFile;
+
+TEST_CASE("Connections components are parsed from yaml.", "[YamlConfiguration]") {
+ const std::shared_ptr<logging::Logger> logger = logging::LoggerFactory<YamlConfiguration>::getLogger();
+ core::ProcessGroup parent(core::ProcessGroupType::ROOT_PROCESS_GROUP, "root");
+ gsl::not_null<core::ProcessGroup*> parent_ptr{ &parent };
+
+ SECTION("Source relationships are read") {
+ const auto connection = std::make_shared<minifi::Connection>(nullptr, nullptr, "name");
+ std::string serialized_yaml;
+ std::vector<std::string> expectations;
+ SECTION("Single relationship name") {
+ serialized_yaml = std::string { "source relationship name: success\n" };
+ expectations = { "success" };
+ }
+ SECTION("List of relationship names") {
+ serialized_yaml = std::string {
+ "source relationship names:\n"
+ "- success\n"
+ "- failure\n"
+ "- something_else\n" };
+ expectations = { "success", "failure", "something_else" };
+ }
+ YAML::Node connection_node = YAML::Load(serialized_yaml);
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ yaml_connection_parser.configureConnectionSourceRelationshipsFromYaml(connection);
+ const std::set<core::Relationship>& relationships = connection->getRelationships();
+ REQUIRE(expectations.size() == relationships.size());
+ for (const auto& expected_relationship_name : expectations) {
+ const auto relationship_name_matches = [&] (const core::Relationship& relationship) { return expected_relationship_name == relationship.getName(); };
+ const std::size_t relationship_count = std::count_if(relationships.cbegin(), relationships.cend(), relationship_name_matches);
+ REQUIRE(1 == relationship_count);
+ }
+ }
+ SECTION("Queue size limits are read") {
+ YAML::Node connection_node = YAML::Load(std::string {
+ "max work queue size: 231\n"
+ "max work queue data size: 12 MB\n" });
+ YamlConnectionParser yaml_connection_parser(connection_node, "test_node", parent_ptr, logger);
+ REQUIRE(231 == yaml_connection_parser.getWorkQueueSizeFromYaml());
+ REQUIRE(12582912 == yaml_connection_parser.getWorkQueueDataSizeFromYaml()); // 12 * 1024 * 1024 B
Review comment:
Please remember the Apache motto "Community Over Code". (from https://www.apache.org/theapacheway/)
Good code is high priority but keeping a healthy community is higher. We should be open to constructive criticism, but also accept when we cannot reach a consensus and not sacrifice the community morale on the altar of tiny technical disagreements.
My decision making preference is: consensus > healthy debate > majority vote.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org