szaszm commented on a change in pull request #1253:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1253#discussion_r802573125



##########
File path: encrypt-config/ConfigFile.cpp
##########
@@ -23,11 +23,12 @@
 #include <optional>
 
 #include "utils/StringUtils.h"
+#include "properties/Configuration.h"
 
 namespace {
-constexpr std::array<const char*, 2> 
DEFAULT_SENSITIVE_PROPERTIES{"nifi.security.client.pass.phrase",
-                                                                  
"nifi.rest.api.password"};
-constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME = 
"nifi.sensitive.props.additional.keys";
+constexpr std::array<const char*, 2> 
DEFAULT_SENSITIVE_PROPERTIES{org::apache::nifi::minifi::Configuration::nifi_security_client_pass_phrase,
+                                                                  
org::apache::nifi::minifi::Configuration::nifi_rest_api_password};
+constexpr const char* ADDITIONAL_SENSITIVE_PROPS_PROPERTY_NAME = 
org::apache::nifi::minifi::Configuration::nifi_sensitive_props_additional_keys;

Review comment:
       Why not just move this into the inner namespace and avoid fully 
qualified names?

##########
File path: extensions/civetweb/protocols/RESTReceiver.cpp
##########
@@ -51,8 +52,8 @@ void 
RESTReceiver::initialize(core::controller::ControllerServiceProvider* contr
   logger_->log_trace("Initializing rest receiver");
   if (nullptr != configuration_) {
     std::string listeningPort, rootUri = "/", caCert;
-    configuration_->get("nifi.c2.rest.listener.port", "c2.rest.listener.port", 
listeningPort);
-    configuration_->get("nifi.c2.rest.listener.cacert", 
"c2.rest.listener.cacert", caCert);
+    configuration_->get(minifi::Configuration::nifi_c2_rest_listener_port, 
"c2.rest.listener.port", listeningPort);
+    configuration_->get(minifi::Configuration::nifi_c2_rest_listener_cacert, 
"c2.rest.listener.cacert", caCert);

Review comment:
       The `minifi::` prefix shouldn't be needed here

##########
File path: encrypt-config/tests/ConfigFileTests.cpp
##########
@@ -112,21 +113,21 @@ TEST_CASE("ConfigFile can parse a simple config file", 
"[encrypt-config][constru
 
 TEST_CASE("ConfigFile can test whether a key is present", 
"[encrypt-config][hasValue]") {
   ConfigFile test_file{std::ifstream{"resources/minifi.properties"}};
-  REQUIRE(test_file.hasValue("nifi.version"));
-  REQUIRE(test_file.hasValue("nifi.c2.flow.id"));  // present but blank
-  REQUIRE(!test_file.hasValue("nifi.remote.input.secure"));  // commented out
+  
REQUIRE(test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_version));
+  
REQUIRE(test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_c2_flow_id));
  // present but blank
+  
REQUIRE(!test_file.hasValue(org::apache::nifi::minifi::Configuration::nifi_remote_input_secure));
  // commented out

Review comment:
       Consider using a namespace alias or moving the test cases inside the 
minifi or encrypt_config namespace.

##########
File path: extensions/coap/controllerservice/CoapConnector.cpp
##########
@@ -67,7 +67,7 @@ void CoapConnectorService::onEnable() {
     core::Property::StringToInt(port_str, port_);
   } else {
     // this is the case where we aren't being used in the context of a single 
controller service.
-    if (configuration_->get("nifi.c2.agent.coap.host", host_) && 
configuration_->get("nifi.c2.agent.coap.port", port_str)) {
+    if (configuration_->get(minifi::Configuration::nifi_c2_agent_coap_host, 
host_) && configuration_->get(minifi::Configuration::nifi_c2_agent_coap_port, 
port_str)) {

Review comment:
       The `minifi::` prefix shouldn't be needed here, or anywhere inside the 
`minifi` namespace.

##########
File path: extensions/http-curl/tests/C2NullConfiguration.cpp
##########
@@ -72,15 +72,15 @@ class VerifyC2Server : public HTTPIntegrationBase {
 
     std::string port, scheme, path;
     parse_http_components(url, port, scheme, path);
-    configuration->set("c2.enable", "true");
-    configuration->set("c2.agent.class", "test");
-    configuration->set("c2.agent.protocol.class", "RESTSender");
-    configuration->set("c2.rest.url", "");
-    configuration->set("c2.rest.url.ack", "");
-    configuration->set("c2.agent.heartbeat.reporter.classes", "null");
-    configuration->set("c2.rest.listener.port", "null");
-    configuration->set("c2.agent.heartbeat.period", "null");
-    configuration->set("c2.rest.listener.heartbeat.rooturi", "null");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_enable, 
"true");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_class,
 "test");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_protocol_class,
 "RESTSender");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url, 
"");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_url_ack,
 "");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_reporter_classes,
 "null");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_port,
 "null");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_agent_heartbeat_period,
 "null");
+    
configuration->set(org::apache::nifi::minifi::Configuration::nifi_c2_rest_listener_heartbeat_rooturi,
 "null");

Review comment:
       Consider moving the class instead.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to