You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "martinzink (via GitHub)" <gi...@apache.org> on 2023/03/24 11:06:16 UTC

[GitHub] [nifi-minifi-cpp] martinzink opened a new pull request, #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

martinzink opened a new pull request, #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543

   …adConfigureFile
   
   Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced
        in the commit message?
   
   - [x] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
   
   - [x] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [x] If applicable, have you updated the LICENSE file?
   - [x] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [x] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
   


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155850024


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))

Review Comment:
   nice catch, fixed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-f841426e292e70012a81b0fae92f08df7b13120e17b17a92fc360f2f50ec072fR78



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1494183140

   > There is a build error in one of the CI jobs:
   > 
   > ```
   > /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/SFTPProcessorBase.cpp:142:36: error: ‘port_tmp’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   >   142 |       common_properties.proxy_port = static_cast<uint16_t>(port_tmp);
   > ```
   
   I am not sure why does this only fail in this PR :thinking: , but fixed the issue nontheless in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/423558ad876eea3c8fe0c77021ce62135a419e61


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1151516542


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,6 +63,46 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+void ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return;
+
+  value += " ms";
+  persisted_value = value;
+  need_to_persist_new_value = true;
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+void ensureIntegerValidatedPropertyHasNoUnit(const core::PropertyValidator* const validator, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  if (!integerValidatedProperty(validator))
+    return;
+
+  if (auto parsed_time = utils::timeutils::StringToDuration<std::chrono::milliseconds>(value)) {
+    value = fmt::format("{}", parsed_time->count());
+    persisted_value = value;
+    need_to_persist_new_value = true;
+  }
+}
+
+void formatConfigurationProperty(std::string_view key, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(key);
+  if (configuration_property == Configuration::CONFIGURATION_PROPERTIES.end())
+    return;
+
+  ensureTimePeriodValidatedPropertyHasExplicitUnit(configuration_property->second, persisted_value, value, need_to_persist_new_value);
+  ensureIntegerValidatedPropertyHasNoUnit(configuration_property->second, persisted_value, value, need_to_persist_new_value);
+}
+}  // namespace
+

Review Comment:
   good idea, I've added an explanatory comment in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/ee61abd3c1c0efc9f2d460bd691f854f26e9a5aa#diff-f841426e292e70012a81b0fae92f08df7b13120e17b17a92fc360f2f50ec072fR157-R158



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155856612


##########
libminifi/test/unit/StringUtilsTests.cpp:
##########
@@ -536,4 +536,25 @@ TEST_CASE("StringUtils::matchesSequence works correctly", "[matchesSequence]") {
   REQUIRE(StringUtils::matchesSequence("xxxabcxxxabcxxxdefxxx", {"abc", "abc", "def"}));
   REQUIRE(!StringUtils::matchesSequence("xxxabcxxxdefxxx", {"abc", "abc", "def"}));
 }
+
+TEST_CASE("StringUtils::splitToUnitAndValue tests") {

Review Comment:
   I've included these tests (except for the 017 bananas that would parse as 17 "bananas", and "0x17 bananas" would parse as 0 "x17 bananas")



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1158428649


##########
libminifi/include/utils/StringUtils.h:
##########
@@ -88,7 +87,15 @@ class StringUtils {
    */
   static std::optional<bool> toBool(const std::string& input);
 
-  static std::string toLower(std::string_view str);
+  static inline std::string toLower(std::string_view str) {
+    const auto tolower = [](auto c) { return std::tolower(static_cast<unsigned char>(c)); };
+    return str | ranges::views::transform(tolower) | ranges::to<std::string>();
+  }
+
+  static inline std::string toUpper(std::string_view str)  {
+    const auto toupper = [](auto c) { return std::toupper(static_cast<unsigned char>(c)); };
+    return str | ranges::views::transform(toupper) | ranges::to<std::string>();
+  }

Review Comment:
   The dumber version of taking a `std::string` by value, and doing in-place transform would be more efficient, because it allows NRVO, and better optimization
   demonstration: https://godbolt.org/z/q5qWqGbnx
   above version: 3 allocations, 1 copy
   std::string version: 1 allocation (of the passed-in string)
   
   We need 1 allocation either way, to construct the result, but by taking a std::string by value, we push this responsibility to the caller, who may be able to avoid it by reusing an existing std::string object. If the string is moved in from an existing object, the resulting assembly is basically equivalent to passing in a pointer to the string, and doing an in-place transform, with no extra allocation, but the interface keeps the value-semantics, and falls back to copy when necessary.
   
   ```suggestion
     static inline std::string toLower(std::string str) {
       const auto tolower = [](auto c) { return std::tolower(static_cast<unsigned char>(c)); };
       std::transform(std::begin(str), std::end(str), std::begin(str), tolower);
       return str;
     }
   
     static inline std::string toUpper(std::string str)  {
       const auto toupper = [](auto c) { return std::toupper(static_cast<unsigned char>(c)); };
       std::transform(std::begin(str), std::end(str), std::begin(str), toupper);
       return str;
     }
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1158385546


##########
extensions/standard-processors/tests/unit/ConfigurationTests.cpp:
##########
@@ -53,4 +54,130 @@ TEST_CASE("Configuration can validate values to be assigned to specific properti
   REQUIRE(Configuration::validatePropertyValue("random.property", "random_value"));
 }
 
+TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "nifi.c2.agent.heartbeat.period=1min" << std::endl;
+    properties_file << "nifi.administrative.yield.duration=30000" << std::endl;
+    properties_file.close();
+  }

Review Comment:
   ```suggestion
     std::ofstream{properties_path}
         << "nifi.c2.agent.heartbeat.period=1min\n"
         << "nifi.administrative.yield.duration=30000\n";
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155848273


##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {

Review Comment:
   good idea, changed this to use int64_t in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-e2999aedfde011241ca9ddcd8081eec97c6bd96a5b1f6f883aeb2c13985be13fR113



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1150080892


##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -263,6 +263,7 @@ int main(int argc, char **argv) {
     configure->setHome(minifiHome);
     configure->loadConfigureFile(DEFAULT_NIFI_PROPERTIES_FILE);
 
+    configure->commitChanges();

Review Comment:
   Can we skip this commit, and still have the changed/mapped properties appear as part of the manifest?
   
   It feels wrong to write the config right after reading it, and we will do the mapping of the values anyway while reading. I think persisting them in a batch with the first normal persist is enough, and we can rely on in-memory mapping integers to milliseconds.



##########
libminifi/src/properties/Properties.cpp:
##########
@@ -87,13 +128,17 @@ void Properties::loadConfigureFile(const std::filesystem::path& configuration_fi
     return;
   }
   properties_.clear();
+  dirty_ = false;
   for (const auto& line : PropertiesFile{file}) {
+    auto key = line.getKey();
     auto persisted_value = line.getValue();
     auto value = utils::StringUtils::replaceEnvironmentVariables(persisted_value);
-    properties_[line.getKey()] = {persisted_value, value, false};
+    bool need_to_persist_new_value = false;
+    formatConfigurationProperty(key, persisted_value, value, need_to_persist_new_value);
+    dirty_ |= need_to_persist_new_value;

Review Comment:
   This operator does bitwise OR, not logical OR. It happens to work out fine, due to the memory representation of booleans, but I'd prefer using logical operators with booleans.



##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,6 +63,46 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+void ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return;
+
+  value += " ms";
+  persisted_value = value;
+  need_to_persist_new_value = true;
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+void ensureIntegerValidatedPropertyHasNoUnit(const core::PropertyValidator* const validator, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  if (!integerValidatedProperty(validator))
+    return;
+
+  if (auto parsed_time = utils::timeutils::StringToDuration<std::chrono::milliseconds>(value)) {
+    value = fmt::format("{}", parsed_time->count());
+    persisted_value = value;
+    need_to_persist_new_value = true;
+  }
+}
+
+void formatConfigurationProperty(std::string_view key, std::string& persisted_value, std::string& value, bool& need_to_persist_new_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(key);
+  if (configuration_property == Configuration::CONFIGURATION_PROPERTIES.end())
+    return;
+
+  ensureTimePeriodValidatedPropertyHasExplicitUnit(configuration_property->second, persisted_value, value, need_to_persist_new_value);
+  ensureIntegerValidatedPropertyHasNoUnit(configuration_property->second, persisted_value, value, need_to_persist_new_value);
+}
+}  // namespace
+

Review Comment:
   Could you extend the comment above `loadConfigureFile` to specify the kind of mapping that's happening here during load, and the motivation? Just to avoid confusing future readers of the code.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155850661


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " ms";
+}
+
+std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " B";
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+std::optional<uint64_t> stringToDataSize(std::string_view input) {

Review Comment:
   it even makes sense to use signed for these, so I've changed it to use int64_t in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-f841426e292e70012a81b0fae92f08df7b13120e17b17a92fc360f2f50ec072fR100



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1153389236


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " ms";
+}
+
+std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " B";
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+std::optional<uint64_t> stringToDataSize(std::string_view input) {
+  int64_t value;
+  std::string unit_str;
+  if (!utils::StringUtils::splitToUnitAndValue(input, unit_str, value))
+    return std::nullopt;
+  if (auto unit_multiplier = core::DataSizeValue::getUnitMultiplier(unit_str)) {
+    return value * *unit_multiplier;
+  }

Review Comment:
   Nitpick: inconsistent use of braces on oneliner ifs, same in ensureIntegerValidatedPropertyHasNoUnit



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1151517332


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -87,13 +128,17 @@ void Properties::loadConfigureFile(const std::filesystem::path& configuration_fi
     return;
   }
   properties_.clear();
+  dirty_ = false;
   for (const auto& line : PropertiesFile{file}) {
+    auto key = line.getKey();
     auto persisted_value = line.getValue();
     auto value = utils::StringUtils::replaceEnvironmentVariables(persisted_value);
-    properties_[line.getKey()] = {persisted_value, value, false};
+    bool need_to_persist_new_value = false;
+    formatConfigurationProperty(key, persisted_value, value, need_to_persist_new_value);
+    dirty_ |= need_to_persist_new_value;

Review Comment:
   sure, it will be more readable changed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/ee61abd3c1c0efc9f2d460bd691f854f26e9a5aa#diff-f841426e292e70012a81b0fae92f08df7b13120e17b17a92fc360f2f50ec072fR190



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155847165


##########
libminifi/include/EventDrivenSchedulingAgent.h:
##########
@@ -38,11 +38,15 @@ class EventDrivenSchedulingAgent : public ThreadedSchedulingAgent {
                              std::shared_ptr<core::Repository> flow_repo, std::shared_ptr<core::ContentRepository> content_repo, std::shared_ptr<Configure> configuration,
                              utils::ThreadPool<utils::TaskRescheduleInfo> &thread_pool)
       : ThreadedSchedulingAgent(controller_service_provider, repo, flow_repo, content_repo, configuration, thread_pool) {
-    int slice = configuration->getInt(Configure::nifi_flow_engine_event_driven_time_slice, DEFAULT_TIME_SLICE_MS);
-    if (slice < 10 || 1000 < slice) {
+    using namespace std::literals::chrono_literals;
+
+    time_slice_ = configuration->get(Configure::nifi_flow_engine_event_driven_time_slice)
+        | utils::flatMap(utils::timeutils::StringToDuration<std::chrono::milliseconds>)
+        | utils::valueOrElse([] { return std::chrono::milliseconds(DEFAULT_TIME_SLICE_MS);});

Review Comment:
   good idea changed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-e8d87fb1392695bb61fc71860b0895a816856bc637825368c05d7f4031212c85R48-R49 https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-9d30d2ebf0518c2ec672054d681e45cbee06665a0c0eb377a44d0879f018dc53R26



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155856932


##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " ms";
+}
+
+std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " B";
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+std::optional<uint64_t> stringToDataSize(std::string_view input) {
+  int64_t value;
+  std::string unit_str;
+  if (!utils::StringUtils::splitToUnitAndValue(input, unit_str, value))
+    return std::nullopt;
+  if (auto unit_multiplier = core::DataSizeValue::getUnitMultiplier(unit_str)) {
+    return value * *unit_multiplier;
+  }

Review Comment:
   nice catch, fixed them in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-f841426e292e70012a81b0fae92f08df7b13120e17b17a92fc360f2f50ec072fR114-R116



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1163950530


##########
libminifi/src/Configuration.cpp:
##########
@@ -179,12 +179,10 @@ std::vector<std::string> Configuration::getSensitiveProperties(const std::functi
 }
 
 bool Configuration::validatePropertyValue(const std::string& property_name, const std::string& property_value) {
-  for (const auto& config_property: Configuration::CONFIGURATION_PROPERTIES) {
-    if (config_property.name == property_name) {
-      return config_property.validator->validate(property_name, property_value).valid();
-    }
-  }
-  return true;
+  if (!Configuration::CONFIGURATION_PROPERTIES.contains(property_name))
+    return true;
+
+  return Configuration::CONFIGURATION_PROPERTIES.at(property_name)->validate(property_name, property_value).valid();

Review Comment:
   We should do the lookup only once, not twice.
   ```suggestion
     const auto validator = Configuration::CONFIGURATION_PROPERTIES.find(property_name);
     if (validator == std::end(Configuration::CONFIGURATION_PROPERTIES))
       return true;
   
     return (*validator)->validate(property_name, property_value).valid();
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1151519318


##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -263,6 +263,7 @@ int main(int argc, char **argv) {
     configure->setHome(minifiHome);
     configure->loadConfigureFile(DEFAULT_NIFI_PROPERTIES_FILE);
 
+    configure->commitChanges();

Review Comment:
   Thats a great idea, I've removed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/ee61abd3c1c0efc9f2d460bd691f854f26e9a5aa#diff-5261461cf4d98b7b43439de885b0c39239d283dbe0e089dcf6901c16aa0ca41aL243 I've also tested and it works like you would expect, the property is fixed in memory and the changes (if it didnt involve enviromental variables) will be persisted during the next normal persist.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1153018041


##########
extensions/standard-processors/tests/unit/ConfigurationTests.cpp:
##########
@@ -53,4 +54,130 @@ TEST_CASE("Configuration can validate values to be assigned to specific properti
   REQUIRE(Configuration::validatePropertyValue("random.property", "random_value"));
 }
 
+TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "nifi.c2.agent.heartbeat.period=1min" << std::endl;
+    properties_file << "nifi.administrative.yield.duration=30000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path);
+  CHECK(configure->get("nifi.c2.agent.heartbeat.period") == "60000");
+  CHECK(configure->get("nifi.administrative.yield.duration") == "30000 ms");
+
+  {
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=1min");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=60000");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000 ms");
+  }
+}
+
+TEST_CASE("Configuration can fix misconfigured datasize<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "appender.rolling.max_file_size=6000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path, "nifi.log.");
+  CHECK(configure->get("appender.rolling.max_file_size") == "6000 B");
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000 B");
+  }
+}
+
+
+TEST_CASE("Configuration misconfigured validated properties with in environmental variables") {

Review Comment:
   ```suggestion
   TEST_CASE("Configuration can fix misconfigured validated properties within environmental variables") {
   ```



##########
extensions/http-curl/tests/HTTPHandlers.h:
##########
@@ -556,18 +556,18 @@ class HeartbeatHandler : public ServerAwareHandler {
         std::vector<std::unordered_map<std::string, std::string>> config_properties;
         const auto prop_reader = [this](const std::string& sensitive_props) { return configuration_->getString(sensitive_props); };
         const auto sensitive_props = minifi::Configuration::getSensitiveProperties(prop_reader);
-        for (const auto& property : minifi::Configuration::CONFIGURATION_PROPERTIES) {
-          if (ranges::find(sensitive_props, property.name) != ranges::end(sensitive_props)) {
+        for (const auto& [property_name, property_validator] : minifi::Configuration::CONFIGURATION_PROPERTIES) {
+          if (ranges::find(sensitive_props, property_name) != ranges::end(sensitive_props)) {

Review Comment:
   minor improvement, but this could be written as
   ```suggestion
             if (ranges::contains(sensitive_props, property_name)) {
   ```



##########
libminifi/include/properties/Properties.h:
##########
@@ -113,7 +113,7 @@ class Properties {
    * Load configure file
    * @param fileName path of the configuration file RELATIVE to MINIFI_HOME set by setHome()
    */
-  void loadConfigureFile(const std::filesystem::path& configuration_file);
+  void loadConfigureFile(const std::filesystem::path& configuration_file, const std::string_view prefix = "");

Review Comment:
   the `const` doesn't do anything (except triggering a clang-tidy warning)



##########
libminifi/include/EventDrivenSchedulingAgent.h:
##########
@@ -38,11 +38,15 @@ class EventDrivenSchedulingAgent : public ThreadedSchedulingAgent {
                              std::shared_ptr<core::Repository> flow_repo, std::shared_ptr<core::ContentRepository> content_repo, std::shared_ptr<Configure> configuration,
                              utils::ThreadPool<utils::TaskRescheduleInfo> &thread_pool)
       : ThreadedSchedulingAgent(controller_service_provider, repo, flow_repo, content_repo, configuration, thread_pool) {
-    int slice = configuration->getInt(Configure::nifi_flow_engine_event_driven_time_slice, DEFAULT_TIME_SLICE_MS);
-    if (slice < 10 || 1000 < slice) {
+    using namespace std::literals::chrono_literals;
+
+    time_slice_ = configuration->get(Configure::nifi_flow_engine_event_driven_time_slice)
+        | utils::flatMap(utils::timeutils::StringToDuration<std::chrono::milliseconds>)
+        | utils::valueOrElse([] { return std::chrono::milliseconds(DEFAULT_TIME_SLICE_MS);});

Review Comment:
   `DEFAULT_TIME_SLICE_MS` could be a `constexpr std::chrono::milliseconds` instead of a macro
   
   (also `SCHEDULING_WATCHDOG_DEFAULT_ALERT_PERIOD_MS` below)



##########
libminifi/include/utils/TimeUtil.h:
##########
@@ -201,38 +203,17 @@ std::optional<TargetDuration> cast_to_matching_unit(std::string& unit, const int
   ((result = cast_if_unit_matches<TargetDuration, T>(unit, value)) || ...);
   return result;
 }
-
-inline bool get_unit_and_value(const std::string& input, std::string& unit, int64_t& value) {
-  const char* begin = input.c_str();
-  char *end;
-  errno = 0;
-  value = std::strtoll(begin, &end, 0);
-  if (end == begin || errno == ERANGE) {
-    return false;
-  }
-
-  if (end[0] == '\0') {
-    return false;
-  }
-
-  while (*end == ' ') {
-    // Skip the spaces
-    end++;
-  }
-  unit = std::string(end);
-  std::transform(unit.begin(), unit.end(), unit.begin(), ::tolower);
-  return true;
-}
-
 }  // namespace details
 
 template<class TargetDuration>
 std::optional<TargetDuration> StringToDuration(const std::string& input) {
   std::string unit;
   int64_t value;
-  if (!details::get_unit_and_value(input, unit, value))
+  if (!StringUtils::splitToUnitAndValue(input, unit, value))
     return std::nullopt;
 
+  std::transform(unit.begin(), unit.end(), unit.begin(), ::tolower);

Review Comment:
   we could use `StringUtils::toLower()` here



##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))

Review Comment:
   the same `unsigned char` thingie applies to `isdigit` as to `toupper`/`tolower`



##########
libminifi/include/utils/StringUtils.h:
##########
@@ -505,7 +505,7 @@ class StringUtils {
    */
   static bool matchesSequence(std::string_view str, const std::vector<std::string>& patterns);
 
- private:
+  static bool splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value);

Review Comment:
   can we also switch the arguments, please?
   ```suggestion
     static bool splitToUnitAndValue(std::string_view input, int64_t& value, std::string& unit);
   ```
   that would look more logical to me



##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
     // TODO(adebreceni): this mapping is to preserve backwards compatibility,
     //  we should entertain the idea of moving to standardized units in
     //  the configuration (i.e. K = 1000, Ki = 1024)
     static std::map<std::string, int64_t> unit_map{
-      {"B", 1},
-      {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
-      {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
+        {"B", 1},
+        {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+        {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
     };
+    std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), ::toupper);

Review Comment:
   I know it's old code, but this should be
   ```suggestion
       std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), [](unsigned char c){ return ::toupper(c); });
   ```
   (see https://en.cppreference.com/w/cpp/string/byte/toupper)
   
   EDIT: we have `StringUtils::toLower()` which does this correctly; maybe we should add `StringUtils::toUpper()`, as well, and use that instead of `transform` + `::toupper` directly?
   
   EDIT2: and possibly move both to the `StringUtils` header, so they can be inlined



##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
     // TODO(adebreceni): this mapping is to preserve backwards compatibility,
     //  we should entertain the idea of moving to standardized units in
     //  the configuration (i.e. K = 1000, Ki = 1024)
     static std::map<std::string, int64_t> unit_map{
-      {"B", 1},
-      {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
-      {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
+        {"B", 1},
+        {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+        {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
     };
+    std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), ::toupper);
+    auto unit_multiplier_it = unit_map.find(unit_str);
+    if (unit_multiplier_it == unit_map.end())
+      return std::nullopt;
+    return unit_multiplier_it->second;
+  }
 
+  // Convert String to Integer
+  template<typename T, typename std::enable_if<
+      std::is_integral<T>::value>::type* = nullptr>

Review Comment:
   I can't remember if we can use the `<concepts>` header yet, but
   ```suggestion
     template<std::integral T>
   ```
   would be much nicer



##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {

Review Comment:
   there is no point in converting the return value to `uint64_t`, as it will be converted back to `int64_t` at line 143 (with a "loss of precision" clang-tidy warning)
   ```suggestion
     static std::optional<int64_t> getUnitMultiplier(std::string unit_str) {
   ```



##########
libminifi/src/properties/Properties.cpp:
##########
@@ -62,8 +63,100 @@ int Properties::getInt(const std::string &key, int default_value) const {
   return it != properties_.end() ? std::stoi(it->second.active_value) : default_value;
 }
 
+namespace {
+const core::PropertyValidator* getValidator(const std::string& lookup_value) {
+  auto configuration_property = Configuration::CONFIGURATION_PROPERTIES.find(lookup_value);
+
+  if (configuration_property != Configuration::CONFIGURATION_PROPERTIES.end())
+    return configuration_property->second;
+  return nullptr;
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " ms";
+}
+
+std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
+  if (validator != core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())
+    return std::nullopt;
+  if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
+    return std::nullopt;
+
+  return value + " B";
+}
+
+bool integerValidatedProperty(const core::PropertyValidator* const validator) {
+  return validator == core::StandardValidators::get().INTEGER_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get()
+      || validator == core::StandardValidators::get().LONG_VALIDATOR.get()
+      || validator == core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get();
+}
+
+std::optional<uint64_t> stringToDataSize(std::string_view input) {

Review Comment:
   if we change `getUnitMultiplier()` to return `int64_t`, then this should be changed to `int64_t`, too
   
   otherwise, some `gsl::narrow`s can be added, that is fine, too



##########
libminifi/src/c2/C2Agent.cpp:
##########
@@ -180,17 +180,18 @@ void C2Agent::configure(const std::shared_ptr<Configure> &configure, bool reconf
     try {
       if (auto heartbeat_period_ms = utils::timeutils::StringToDuration<std::chrono::milliseconds>(heartbeat_period)) {
         heart_beat_period_ = *heartbeat_period_ms;
-        logger_->log_debug("Using %u ms as the heartbeat period", heart_beat_period_.count());
       } else {
         heart_beat_period_ = std::chrono::milliseconds(std::stoi(heartbeat_period));
       }
     } catch (const std::invalid_argument &) {
+      logger_->log_error("Invalid heartbeat period: %s", heartbeat_period);
       heart_beat_period_ = 3s;
     }
   } else {
     if (!reconfigure)
       heart_beat_period_ = 3s;
   }
+  logger_->log_debug("Using %u ms as the heartbeat period", heart_beat_period_.count());

Review Comment:
   old code + nitpicking, but
   ```suggestion
     logger_->log_debug("Using %" PRId64 " ms as the heartbeat period", heart_beat_period_.count());
   ```



##########
libminifi/src/utils/StringUtils.cpp:
##########
@@ -510,4 +511,20 @@ bool StringUtils::matchesSequence(std::string_view str, const std::vector<std::s
   return true;
 }
 
+bool StringUtils::splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value) {
+  const char* begin = input.data();
+  const char* end = begin + input.size();

Review Comment:
   minor improvement, but I think
   ```suggestion
     const char* begin = std::begin(input);
     const char* end = std::end(input);
   ```
   would be nicer



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1158383457


##########
extensions/standard-processors/tests/unit/ConfigurationTests.cpp:
##########
@@ -53,4 +54,130 @@ TEST_CASE("Configuration can validate values to be assigned to specific properti
   REQUIRE(Configuration::validatePropertyValue("random.property", "random_value"));
 }
 
+TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "nifi.c2.agent.heartbeat.period=1min" << std::endl;
+    properties_file << "nifi.administrative.yield.duration=30000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path);
+  CHECK(configure->get("nifi.c2.agent.heartbeat.period") == "60000");
+  CHECK(configure->get("nifi.administrative.yield.duration") == "30000 ms");
+
+  {
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=1min");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=60000");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000 ms");
+  }
+}
+
+TEST_CASE("Configuration can fix misconfigured datasize<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "appender.rolling.max_file_size=6000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path, "nifi.log.");
+  CHECK(configure->get("appender.rolling.max_file_size") == "6000 B");
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000 B");
+  }
+}
+
+
+TEST_CASE("Configuration can fix misconfigured validated properties within environmental variables") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  CHECK(minifi::utils::Environment::setEnvironmentVariable("SOME_VARIABLE", "4000"));
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "compression.cached.log.max.size=${SOME_VARIABLE}" << std::endl;
+    properties_file << "compression.compressed.log.max.size=3000" << std::endl;
+    properties_file.close();
+  }

Review Comment:
   `std::endl` flushes the stream. I think it's not necessary, but certainly not twice.
   ```suggestion
       std::ofstream{properties_path}
           << "compression.cached.log.max.size=${SOME_VARIABLE}\n"
           << "compression.compressed.log.max.size=3000\n";
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm closed pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm closed pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155846054


##########
extensions/http-curl/tests/HTTPHandlers.h:
##########
@@ -556,18 +556,18 @@ class HeartbeatHandler : public ServerAwareHandler {
         std::vector<std::unordered_map<std::string, std::string>> config_properties;
         const auto prop_reader = [this](const std::string& sensitive_props) { return configuration_->getString(sensitive_props); };
         const auto sensitive_props = minifi::Configuration::getSensitiveProperties(prop_reader);
-        for (const auto& property : minifi::Configuration::CONFIGURATION_PROPERTIES) {
-          if (ranges::find(sensitive_props, property.name) != ranges::end(sensitive_props)) {
+        for (const auto& [property_name, property_validator] : minifi::Configuration::CONFIGURATION_PROPERTIES) {
+          if (ranges::find(sensitive_props, property_name) != ranges::end(sensitive_props)) {

Review Comment:
   Good idea, changed this and an other one couple lines down to use contains 
   https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-2c92592d65b3446aa7ec5a38236b3068aefb9163714268c77658c11d17dfe1baR560



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1168380116


##########
libminifi/src/Configuration.cpp:
##########
@@ -17,140 +17,140 @@
 #include <algorithm>
 
 #include "properties/Configuration.h"
-#include "core/PropertyBuilder.h"
+#include "core/PropertyValidation.h"
 
 namespace org::apache::nifi::minifi {
 
-const std::vector<core::ConfigurationProperty> Configuration::CONFIGURATION_PROPERTIES{
-  core::ConfigurationProperty{Configuration::nifi_default_directory},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_name},
-  core::ConfigurationProperty{Configuration::nifi_configuration_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_sensitive_props_additional_keys},
-  core::ConfigurationProperty{Configuration::nifi_python_processor_dir},
-  core::ConfigurationProperty{Configuration::nifi_extension_path},
-  core::ConfigurationProperty{Configuration::nifi_security_client_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_client_private_key},
-  core::ConfigurationProperty{Configuration::nifi_security_client_pass_phrase},
-  core::ConfigurationProperty{Configuration::nifi_security_client_ca_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_use_system_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_cert_store_location},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_server_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_cn},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_key_usage},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_user_name},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_password},
-  core::ConfigurationProperty{Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_file_watch},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_id},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_base_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_coap_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_reporter_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_host},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_protocol_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier_fallback},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_trigger_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_class_definitions},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_cacert},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url_ack},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_request_encoding},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_heartbeat_topic},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_update_topic},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path_old},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_framework_dir},
-  core::ConfigurationProperty{Configuration::nifi_jvm_options},
-  core::ConfigurationProperty{Configuration::nifi_nar_directory},
-  core::ConfigurationProperty{Configuration::nifi_nar_deploy_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_pattern},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_file_name},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stdout},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stderr},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_null},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_syslog},
-  core::ConfigurationProperty{Configuration::nifi_log_logger_root},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_url},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_batch_size},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_flush_period},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_filter},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_rate_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_buffer_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_level},
-  core::ConfigurationProperty{Configuration::nifi_asset_directory},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_class},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_prometheus_metrics_publisher_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_metrics},
-  core::ConfigurationProperty{Configuration::controller_socket_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::controller_socket_local_any_interface, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::controller_socket_host},
-  core::ConfigurationProperty{Configuration::controller_socket_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::controller_ssl_context_service}
+const std::unordered_map<std::string_view, gsl::not_null<core::PropertyValidator*>> Configuration::CONFIGURATION_PROPERTIES{
+  {Configuration::nifi_default_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_server_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_configuration_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_sensitive_props_additional_keys, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_python_processor_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_extension_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_private_key, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_pass_phrase, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_ca_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_use_system_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_cert_store_location, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_server_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_cn, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_key_usage, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_user_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_password, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_file_watch, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_id, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_base_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_coap_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_reporter_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_host, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_protocol_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier_fallback, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_trigger_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_class_definitions, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_cacert, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url_ack, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_request_encoding, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_heartbeat_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_update_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::nifi_framework_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_jvm_options, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_deploy_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_pattern, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_file_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stdout, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stderr, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_null, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_syslog, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_logger_root, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_batch_size, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_flush_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_filter, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_rate_limit, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_buffer_limit, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_level, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_asset_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_metrics_publisher_agent_identifier, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_metrics_publisher_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_metrics_publisher_prometheus_metrics_publisher_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_metrics_publisher_metrics, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::controller_socket_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::controller_socket_local_any_interface, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::controller_socket_host,gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::controller_socket_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::controller_ssl_context_service,gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())}

Review Comment:
   ```
   /home/szaszm/nifi-minifi-cpp-3/libminifi/src/Configuration.cpp:151:  Missing space after ,  [whitespace/comma] [3]
   /home/szaszm/nifi-minifi-cpp-3/libminifi/src/Configuration.cpp:153:  Missing space after ,  [whitespace/comma] [3]
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] arpadboda commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "arpadboda (via GitHub)" <gi...@apache.org>.
arpadboda commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1488886936

   There is a build error in one of the CI jobs:
   ```
   /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/SFTPProcessorBase.cpp:142:36: error: ‘port_tmp’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     142 |       common_properties.proxy_port = static_cast<uint16_t>(port_tmp);
   ```


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1152260276


##########
libminifi/src/utils/StringUtils.cpp:
##########
@@ -510,4 +510,26 @@ bool StringUtils::matchesSequence(std::string_view str, const std::vector<std::s
   return true;
 }
 
+bool StringUtils::splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value) {
+  const char* begin = input.data();
+  char *end;
+  errno = 0;
+  value = std::strtoll(begin, &end, 0);

Review Comment:
   I naively just copied this from timeUtils but looking into it strtoull is guranteed to not throw, stoull is the one that throws this only sets the errno.
   But using std::from_chars which doesnt rely on a global error variable is probably a better aproach, so I've refactored and added some tests for it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/471e5dc9ff8473b04d9f801acd4cc9013dbb3db9



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1494523571

   Another error popped up, this time in clang-tidy:
   ```
   /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/../client/SFTPClient.h:20:10: error: 'curl/curl.h' file not found [clang-diagnostic-error]
   #include <curl/curl.h>
            ^~~~~~~~~~~~~
   /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/SFTPProcessorBase.cpp:186:133: error: parameter 'connection' is unused [misc-unused-parameters,-warnings-as-errors]
   void SFTPProcessorBase::addConnectionToCache(const SFTPProcessorBase::ConnectionCacheKey& key, std::unique_ptr<utils::SFTPClient>&& connection) {
                                                                                                                                       ^~~~~~~~~~
                                                                                                                                        /*connection*/
   ```


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155847706


##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
     // TODO(adebreceni): this mapping is to preserve backwards compatibility,
     //  we should entertain the idea of moving to standardized units in
     //  the configuration (i.e. K = 1000, Ki = 1024)
     static std::map<std::string, int64_t> unit_map{
-      {"B", 1},
-      {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
-      {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
+        {"B", 1},
+        {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+        {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
     };
+    std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), ::toupper);

Review Comment:
   good idea, changed these in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-e2999aedfde011241ca9ddcd8081eec97c6bd96a5b1f6f883aeb2c13985be13fR122



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155849368


##########
libminifi/include/utils/TimeUtil.h:
##########
@@ -201,38 +203,17 @@ std::optional<TargetDuration> cast_to_matching_unit(std::string& unit, const int
   ((result = cast_if_unit_matches<TargetDuration, T>(unit, value)) || ...);
   return result;
 }
-
-inline bool get_unit_and_value(const std::string& input, std::string& unit, int64_t& value) {
-  const char* begin = input.c_str();
-  char *end;
-  errno = 0;
-  value = std::strtoll(begin, &end, 0);
-  if (end == begin || errno == ERANGE) {
-    return false;
-  }
-
-  if (end[0] == '\0') {
-    return false;
-  }
-
-  while (*end == ' ') {
-    // Skip the spaces
-    end++;
-  }
-  unit = std::string(end);
-  std::transform(unit.begin(), unit.end(), unit.begin(), ::tolower);
-  return true;
-}
-
 }  // namespace details
 
 template<class TargetDuration>
 std::optional<TargetDuration> StringToDuration(const std::string& input) {
   std::string unit;
   int64_t value;
-  if (!details::get_unit_and_value(input, unit, value))
+  if (!StringUtils::splitToUnitAndValue(input, unit, value))
     return std::nullopt;
 
+  std::transform(unit.begin(), unit.end(), unit.begin(), ::tolower);

Review Comment:
   good idea, changed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-4a76905d55704437ae0a7a4ee434a73f057c105c97bdf8756f5747b4fbc65e9fR216



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155849080


##########
libminifi/include/properties/Properties.h:
##########
@@ -113,7 +113,7 @@ class Properties {
    * Load configure file
    * @param fileName path of the configuration file RELATIVE to MINIFI_HOME set by setHome()
    */
-  void loadConfigureFile(const std::filesystem::path& configuration_file);
+  void loadConfigureFile(const std::filesystem::path& configuration_file, const std::string_view prefix = "");

Review Comment:
   removed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-33abd3468567a377d80f73a7183b9c31a4ecf8b3023bca951ca5c5d696e83caeR116



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1161202029


##########
libminifi/src/Configuration.cpp:
##########
@@ -179,9 +179,9 @@ std::vector<std::string> Configuration::getSensitiveProperties(const std::functi
 }
 
 bool Configuration::validatePropertyValue(const std::string& property_name, const std::string& property_value) {
-  for (const auto& config_property: Configuration::CONFIGURATION_PROPERTIES) {
-    if (config_property.name == property_name) {
-      return config_property.validator->validate(property_name, property_value).valid();
+  for (const auto& [config_property_name, config_property_validator]: Configuration::CONFIGURATION_PROPERTIES) {
+    if (config_property_name == property_name) {
+      return config_property_validator->validate(property_name, property_value).valid();

Review Comment:
   Now that it's not a vector anymore, there is a more efficient way to do lookup in CONFIGURATION_PROPERTIES



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1152156109


##########
libminifi/src/utils/StringUtils.cpp:
##########
@@ -510,4 +510,26 @@ bool StringUtils::matchesSequence(std::string_view str, const std::vector<std::s
   return true;
 }
 
+bool StringUtils::splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value) {
+  const char* begin = input.data();
+  char *end;
+  errno = 0;
+  value = std::strtoll(begin, &end, 0);

Review Comment:
   Can we guarantee that strtoull will not throw?



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "adamdebreceni (via GitHub)" <gi...@apache.org>.
adamdebreceni commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1154138507


##########
libminifi/src/Configuration.cpp:
##########
@@ -17,135 +17,135 @@
 #include <algorithm>
 
 #include "properties/Configuration.h"
-#include "core/PropertyBuilder.h"
+#include "core/PropertyValidation.h"
 
 namespace org::apache::nifi::minifi {
 
-const std::vector<core::ConfigurationProperty> Configuration::CONFIGURATION_PROPERTIES{
-  core::ConfigurationProperty{Configuration::nifi_default_directory},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_name},
-  core::ConfigurationProperty{Configuration::nifi_configuration_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_sensitive_props_additional_keys},
-  core::ConfigurationProperty{Configuration::nifi_python_processor_dir},
-  core::ConfigurationProperty{Configuration::nifi_extension_path},
-  core::ConfigurationProperty{Configuration::nifi_security_client_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_client_private_key},
-  core::ConfigurationProperty{Configuration::nifi_security_client_pass_phrase},
-  core::ConfigurationProperty{Configuration::nifi_security_client_ca_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_use_system_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_cert_store_location},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_server_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_cn},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_key_usage},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_user_name},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_password},
-  core::ConfigurationProperty{Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_file_watch},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_id},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_base_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_coap_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_reporter_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_host},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_protocol_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier_fallback},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_trigger_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_class_definitions},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_cacert},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url_ack},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_request_encoding},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_heartbeat_topic},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_update_topic},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path_old},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_framework_dir},
-  core::ConfigurationProperty{Configuration::nifi_jvm_options},
-  core::ConfigurationProperty{Configuration::nifi_nar_directory},
-  core::ConfigurationProperty{Configuration::nifi_nar_deploy_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_pattern},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_file_name},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stdout},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stderr},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_null},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_syslog},
-  core::ConfigurationProperty{Configuration::nifi_log_logger_root},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_url},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_batch_size},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_flush_period},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_filter},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_rate_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_buffer_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_level},
-  core::ConfigurationProperty{Configuration::nifi_asset_directory},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_class},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_prometheus_metrics_publisher_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_metrics}
+const std::unordered_map<std::string_view, gsl::not_null<core::PropertyValidator*>> Configuration::CONFIGURATION_PROPERTIES{
+  {Configuration::nifi_default_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_server_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_configuration_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_sensitive_props_additional_keys, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_python_processor_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_extension_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_private_key, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_pass_phrase, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_ca_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_use_system_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_cert_store_location, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_server_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_cn, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_key_usage, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_user_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_password, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_file_watch, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_id, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_base_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_coap_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_reporter_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_host, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_protocol_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier_fallback, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_trigger_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_class_definitions, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_cacert, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url_ack, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_request_encoding, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_heartbeat_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_update_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::nifi_framework_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_jvm_options, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_deploy_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_pattern, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_file_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stdout, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stderr, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_null, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_syslog, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_logger_root, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_batch_size, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_flush_period, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},

Review Comment:
   should we also change this to time period validator?



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1153345313


##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
     // TODO(adebreceni): this mapping is to preserve backwards compatibility,
     //  we should entertain the idea of moving to standardized units in
     //  the configuration (i.e. K = 1000, Ki = 1024)
     static std::map<std::string, int64_t> unit_map{
-      {"B", 1},
-      {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
-      {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
+        {"B", 1},
+        {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+        {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
     };
+    std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), ::toupper);
+    auto unit_multiplier_it = unit_map.find(unit_str);
+    if (unit_multiplier_it == unit_map.end())
+      return std::nullopt;
+    return unit_multiplier_it->second;

Review Comment:
   This may be a bit shorter:
   ```
   return unit_map.contains(unit_str) ? unit_map[unit_str] : std::nullopt;
   ```



##########
libminifi/test/unit/StringUtilsTests.cpp:
##########
@@ -536,4 +536,25 @@ TEST_CASE("StringUtils::matchesSequence works correctly", "[matchesSequence]") {
   REQUIRE(StringUtils::matchesSequence("xxxabcxxxabcxxxdefxxx", {"abc", "abc", "def"}));
   REQUIRE(!StringUtils::matchesSequence("xxxabcxxxdefxxx", {"abc", "abc", "def"}));
 }
+
+TEST_CASE("StringUtils::splitToUnitAndValue tests") {

Review Comment:
   I would add a test case like  `1KiB` that is without spaces. It would distinguish this from another solution that would just split the input on the spaces and convert the first part to an integer.
   
   I would also add a test case for invalid cases like
   - an empty string
   - a non-decimal number like `017 bananas`



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155844438


##########
libminifi/src/Configuration.cpp:
##########
@@ -17,135 +17,135 @@
 #include <algorithm>
 
 #include "properties/Configuration.h"
-#include "core/PropertyBuilder.h"
+#include "core/PropertyValidation.h"
 
 namespace org::apache::nifi::minifi {
 
-const std::vector<core::ConfigurationProperty> Configuration::CONFIGURATION_PROPERTIES{
-  core::ConfigurationProperty{Configuration::nifi_default_directory},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_name},
-  core::ConfigurationProperty{Configuration::nifi_configuration_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_flow_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_content_repository_rocksdb_compression},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_class_name},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_provenance_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_directory_default},
-  core::ConfigurationProperty{Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_sensitive_props_additional_keys},
-  core::ConfigurationProperty{Configuration::nifi_python_processor_dir},
-  core::ConfigurationProperty{Configuration::nifi_extension_path},
-  core::ConfigurationProperty{Configuration::nifi_security_client_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_client_private_key},
-  core::ConfigurationProperty{Configuration::nifi_security_client_pass_phrase},
-  core::ConfigurationProperty{Configuration::nifi_security_client_ca_certificate},
-  core::ConfigurationProperty{Configuration::nifi_security_use_system_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_cert_store_location},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_server_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_store},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_cn},
-  core::ConfigurationProperty{Configuration::nifi_security_windows_client_cert_key_usage},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_user_name},
-  core::ConfigurationProperty{Configuration::nifi_rest_api_password},
-  core::ConfigurationProperty{Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_file_watch},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_id},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_flow_base_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_coap_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_heartbeat_reporter_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_host},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_protocol_class},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_identifier_fallback},
-  core::ConfigurationProperty{Configuration::nifi_c2_agent_trigger_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_classes},
-  core::ConfigurationProperty{Configuration::nifi_c2_root_class_definitions},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_listener_cacert},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_url_ack},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_request_encoding},
-  core::ConfigurationProperty{Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_connector_service},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_heartbeat_topic},
-  core::ConfigurationProperty{Configuration::nifi_c2_mqtt_update_topic},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_class_name_old},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path},
-  core::ConfigurationProperty{Configuration::nifi_state_storage_local_path_old},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_framework_dir},
-  core::ConfigurationProperty{Configuration::nifi_jvm_options},
-  core::ConfigurationProperty{Configuration::nifi_nar_directory},
-  core::ConfigurationProperty{Configuration::nifi_nar_deploy_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_pattern},
-  core::ConfigurationProperty{Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_directory},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_file_name},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stdout},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_stderr},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_null},
-  core::ConfigurationProperty{Configuration::nifi_log_appender_syslog},
-  core::ConfigurationProperty{Configuration::nifi_log_logger_root},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_url},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_ssl_context_service},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_batch_size},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_flush_period},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_filter},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_rate_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_buffer_limit},
-  core::ConfigurationProperty{Configuration::nifi_log_alert_level},
-  core::ConfigurationProperty{Configuration::nifi_asset_directory},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_agent_identifier},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_class},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_prometheus_metrics_publisher_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
-  core::ConfigurationProperty{Configuration::nifi_metrics_publisher_metrics}
+const std::unordered_map<std::string_view, gsl::not_null<core::PropertyValidator*>> Configuration::CONFIGURATION_PROPERTIES{
+  {Configuration::nifi_default_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_encrypt, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_configuration_file_backup_update, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_threads, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_alert_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flow_engine_event_driven_time_slice, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_administrative_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_bored_yield_duration, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_graceful_shutdown_seconds, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_flowcontroller_drain_timeout, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_server_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_configuration_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flow_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_content_repository_rocksdb_compression, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_flowfile_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_provenance_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_count, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_max_bytes, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_volatile_repository_options_content_minimal_locking, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_server_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_server_report_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_max_storage_time, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_provenance_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_directory_default, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_flowfile_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_dbcontent_repository_rocksdb_compaction_period, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_remote_input_secure, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_security_need_ClientAuth, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_sensitive_props_additional_keys, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_python_processor_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_extension_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_private_key, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_pass_phrase, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_client_ca_certificate, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_use_system_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_cert_store_location, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_server_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_store, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_cn, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_security_windows_client_cert_key_usage, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_user_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_rest_api_password, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_file_watch, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_id, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_flow_base_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_full_heartbeat, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_coap_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_period, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_heartbeat_reporter_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_host, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_coap_port, gsl::make_not_null(core::StandardValidators::get().PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_protocol_class, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_identifier_fallback, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_agent_trigger_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_classes, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_root_class_definitions, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_port, gsl::make_not_null(core::StandardValidators::get().LISTEN_PORT_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_listener_cacert, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_url_ack, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_request_encoding, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_rest_heartbeat_minimize_updates, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_connector_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_heartbeat_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_c2_mqtt_update_topic, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_class_name_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_always_persist_old, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_auto_persistence_interval_old, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_state_storage_local_path_old, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_enable, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_interval, gsl::make_not_null(core::StandardValidators::get().TIME_PERIOD_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_stop_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::minifi_disk_space_watchdog_restart_threshold, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_LONG_VALIDATOR.get())},
+  {Configuration::nifi_framework_dir, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_jvm_options, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_nar_deploy_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_pattern, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_spdlog_shorten_names, gsl::make_not_null(core::StandardValidators::get().BOOLEAN_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_directory, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_file_name, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_files, gsl::make_not_null(core::StandardValidators::get().UNSIGNED_INT_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_rolling_max_file_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stdout, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_stderr, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_null, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_appender_syslog, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_logger_root, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_cached_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_compression_compressed_log_max_size, gsl::make_not_null(core::StandardValidators::get().DATA_SIZE_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_url, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_ssl_context_service, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_batch_size, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},
+  {Configuration::nifi_log_alert_flush_period, gsl::make_not_null(core::StandardValidators::get().VALID_VALIDATOR.get())},

Review Comment:
   good idea, changed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/e58bc6936e88e251decd060f74f3a4994e372c0a (the usage was already expecting it as a time period)



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155848804


##########
libminifi/include/core/TypedValues.h:
##########
@@ -110,20 +110,26 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
       : state::response::UInt64Value(0) {
   }
 
-
-  // Convert String to Integer
-  template<typename T, typename std::enable_if<
-      std::is_integral<T>::value>::type* = nullptr>
-  static bool StringToInt(const std::string &input, T &output) {
+  static std::optional<uint64_t> getUnitMultiplier(std::string unit_str) {
     // TODO(adebreceni): this mapping is to preserve backwards compatibility,
     //  we should entertain the idea of moving to standardized units in
     //  the configuration (i.e. K = 1000, Ki = 1024)
     static std::map<std::string, int64_t> unit_map{
-      {"B", 1},
-      {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
-      {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
+        {"B", 1},
+        {"K", 1_KB}, {"M", 1_MB}, {"G", 1_GB}, {"T", 1_TB}, {"P", 1_PB},
+        {"KB", 1_KiB}, {"MB", 1_MiB}, {"GB", 1_GiB}, {"TB", 1_TiB}, {"PB", 1_PiB},
     };
+    std::transform(unit_str.begin(), unit_str.end(), unit_str.begin(), ::toupper);
+    auto unit_multiplier_it = unit_map.find(unit_str);
+    if (unit_multiplier_it == unit_map.end())
+      return std::nullopt;
+    return unit_multiplier_it->second;
+  }
 
+  // Convert String to Integer
+  template<typename T, typename std::enable_if<
+      std::is_integral<T>::value>::type* = nullptr>

Review Comment:
   good idea, included this in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-e2999aedfde011241ca9ddcd8081eec97c6bd96a5b1f6f883aeb2c13985be13fR128



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155851899


##########
libminifi/src/utils/StringUtils.cpp:
##########
@@ -510,4 +511,20 @@ bool StringUtils::matchesSequence(std::string_view str, const std::vector<std::s
   return true;
 }
 
+bool StringUtils::splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value) {
+  const char* begin = input.data();
+  const char* end = begin + input.size();

Review Comment:
   It does seem nicer however it failed to compile on MSVC so I've reverted it back https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/c7d10838758e9b96631e8bbb99d5ad307e17bd8c  https://godbolt.org/z/jvs91f44o



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1488093156

   I've also fixed the data-size validated properties similarly to the time-period  validated ones.
   I've changed some properties to be time-period validated in [change nifi_flow_engine_alert_period to be TIME_PERIOD validated](https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/a999bf66b8c9bc1e1c127913db8a292d766bd572) [change nifi_flow_engine_event_driven_time_slice to be TIME_PERIOD val…](https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/e2554b5fa6aa5c61615dfb0c53dc187c1b2f9c13) [change nifi_graceful_shutdown_seconds to be TIME_PERIOD validated](https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/9e3a43ab7315a838bd38f9b99ed2b4aedfe3be21), and also fixed the boolean validated ones to be case-insenstive everywhere (it was inconsistent before).


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1494588704

   > Another error popped up, this time in clang-tidy:
   > 
   > ```
   > /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/../client/SFTPClient.h:20:10: error: 'curl/curl.h' file not found [clang-diagnostic-error]
   > #include <curl/curl.h>
   >          ^~~~~~~~~~~~~
   > /home/runner/work/nifi-minifi-cpp/nifi-minifi-cpp/extensions/sftp/processors/SFTPProcessorBase.cpp:186:133: error: parameter 'connection' is unused [misc-unused-parameters,-warnings-as-errors]
   > void SFTPProcessorBase::addConnectionToCache(const SFTPProcessorBase::ConnectionCacheKey& key, std::unique_ptr<utils::SFTPClient>&& connection) {
   >                                                                                                                                     ^~~~~~~~~~
   >                                                                                                                                      /*connection*/
   > ```
   
   This seems false positive to me...
    [Connection is used (it is moved into connections_)](https://github.com/apache/nifi-minifi-cpp/blob/main/extensions/sftp/processors/SFTPProcessorBase.cpp#L203) 


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#issuecomment-1494824283

   > This seems false positive to me... [Connection is used (it is moved into connections_)](https://github.com/apache/nifi-minifi-cpp/blob/main/extensions/sftp/processors/SFTPProcessorBase.cpp#L203)
   
   This sort of false positive tends to happen when `compile_commands.json` is not found or is corrupted.  I have triggered a re-run of the CI job, let's see what happens.


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155846613


##########
extensions/standard-processors/tests/unit/ConfigurationTests.cpp:
##########
@@ -53,4 +54,130 @@ TEST_CASE("Configuration can validate values to be assigned to specific properti
   REQUIRE(Configuration::validatePropertyValue("random.property", "random_value"));
 }
 
+TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "nifi.c2.agent.heartbeat.period=1min" << std::endl;
+    properties_file << "nifi.administrative.yield.duration=30000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path);
+  CHECK(configure->get("nifi.c2.agent.heartbeat.period") == "60000");
+  CHECK(configure->get("nifi.administrative.yield.duration") == "30000 ms");
+
+  {
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=1min");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=60000");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000 ms");
+  }
+}
+
+TEST_CASE("Configuration can fix misconfigured datasize<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "appender.rolling.max_file_size=6000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path, "nifi.log.");
+  CHECK(configure->get("appender.rolling.max_file_size") == "6000 B");
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000 B");
+  }
+}
+
+
+TEST_CASE("Configuration misconfigured validated properties with in environmental variables") {

Review Comment:
   fixed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-245bd52b73142917446d5f4b4ca2a752e2a7131635f8609b50502849d4159f28R138



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155849770


##########
libminifi/src/c2/C2Agent.cpp:
##########
@@ -180,17 +180,18 @@ void C2Agent::configure(const std::shared_ptr<Configure> &configure, bool reconf
     try {
       if (auto heartbeat_period_ms = utils::timeutils::StringToDuration<std::chrono::milliseconds>(heartbeat_period)) {
         heart_beat_period_ = *heartbeat_period_ms;
-        logger_->log_debug("Using %u ms as the heartbeat period", heart_beat_period_.count());
       } else {
         heart_beat_period_ = std::chrono::milliseconds(std::stoi(heartbeat_period));
       }
     } catch (const std::invalid_argument &) {
+      logger_->log_error("Invalid heartbeat period: %s", heartbeat_period);
       heart_beat_period_ = 3s;
     }
   } else {
     if (!reconfigure)
       heart_beat_period_ = 3s;
   }
+  logger_->log_debug("Using %u ms as the heartbeat period", heart_beat_period_.count());

Review Comment:
   nice catch, fixed it in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002#diff-6efb1f7ecfeb30f06ecb4dbda25c6ea4b7bde897b7a174253ed1c5331ec94ce7R194



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1155852620


##########
libminifi/include/utils/StringUtils.h:
##########
@@ -505,7 +505,7 @@ class StringUtils {
    */
   static bool matchesSequence(std::string_view str, const std::vector<std::string>& patterns);
 
- private:
+  static bool splitToUnitAndValue(std::string_view input, std::string& unit, int64_t& value);

Review Comment:
   sure thing, I've changed the name aswell to better reflect the order of the arguments in https://github.com/apache/nifi-minifi-cpp/pull/1543/commits/64c5975e3fda0911820465778fa4d73f16d12002



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1543: MINIFICPP-2074 Fix time-period/integer validated properties during lo…

Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1543:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1543#discussion_r1158383457


##########
extensions/standard-processors/tests/unit/ConfigurationTests.cpp:
##########
@@ -53,4 +54,130 @@ TEST_CASE("Configuration can validate values to be assigned to specific properti
   REQUIRE(Configuration::validatePropertyValue("random.property", "random_value"));
 }
 
+TEST_CASE("Configuration can fix misconfigured timeperiod<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "nifi.c2.agent.heartbeat.period=1min" << std::endl;
+    properties_file << "nifi.administrative.yield.duration=30000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path);
+  CHECK(configure->get("nifi.c2.agent.heartbeat.period") == "60000");
+  CHECK(configure->get("nifi.administrative.yield.duration") == "30000 ms");
+
+  {
+    CHECK(properties_file_time_after_creation == std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=1min");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    std::string second_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(std::getline(properties_file, second_line));
+    CHECK(first_line == "nifi.c2.agent.heartbeat.period=60000");
+    CHECK(second_line == "nifi.administrative.yield.duration=30000 ms");
+  }
+}
+
+TEST_CASE("Configuration can fix misconfigured datasize<->integer validated properties") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "appender.rolling.max_file_size=6000" << std::endl;
+    properties_file.close();
+  }
+  auto properties_file_time_after_creation = std::filesystem::last_write_time(properties_path);
+  const std::shared_ptr<minifi::Configure> configure = std::make_shared<minifi::Configure>();
+
+  configure->loadConfigureFile(properties_path, "nifi.log.");
+  CHECK(configure->get("appender.rolling.max_file_size") == "6000 B");
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000");
+  }
+
+  CHECK(configure->commitChanges());
+
+  {
+    CHECK(properties_file_time_after_creation <= std::filesystem::last_write_time(properties_path));
+    std::ifstream properties_file(properties_path);
+    std::string first_line;
+    CHECK(std::getline(properties_file, first_line));
+    CHECK(first_line == "appender.rolling.max_file_size=6000 B");
+  }
+}
+
+
+TEST_CASE("Configuration can fix misconfigured validated properties within environmental variables") {
+  LogTestController::getInstance().setInfo<minifi::Configure>();
+  LogTestController::getInstance().setInfo<minifi::Properties>();
+  auto properties_path = std::filesystem::temp_directory_path() /  "test.properties";
+
+  CHECK(minifi::utils::Environment::setEnvironmentVariable("SOME_VARIABLE", "4000"));
+
+  {
+    std::ofstream properties_file(properties_path);
+    properties_file << "compression.cached.log.max.size=${SOME_VARIABLE}" << std::endl;
+    properties_file << "compression.compressed.log.max.size=3000" << std::endl;
+    properties_file.close();
+  }

Review Comment:
   `std::endl` flushes the stream. I think it's not necessary, but certainly not twice.
   ```suggestion
     std::ofstream{properties_path}
         << "compression.cached.log.max.size=${SOME_VARIABLE}\n"
         << "compression.compressed.log.max.size=3000\n";
   ```



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org