You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by lo...@apache.org on 2024/06/25 12:46:15 UTC
(nifi-minifi-cpp) 01/02: MINIFICPP-2393 fix unnecessary throws during Property check
This is an automated email from the ASF dual-hosted git repository.
lordgamez pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit 5b86dd4c0d53ce2545b77bcef24c135ca4c1e627
Author: Martin Zink <ma...@apache.org>
AuthorDate: Wed Jun 5 10:53:25 2024 +0200
MINIFICPP-2393 fix unnecessary throws during Property check
Signed-off-by: Gabor Gyimesi <ga...@gmail.com>
This closes #1825
---
libminifi/include/core/TypedValues.h | 3 ++
libminifi/include/core/flow/Node.h | 22 +++++----
libminifi/include/core/json/JsonNode.h | 86 ++++++++++++++-------------------
libminifi/include/core/yaml/YamlNode.h | 46 ++++++++----------
libminifi/src/properties/Properties.cpp | 56 +++++++++++++--------
5 files changed, 106 insertions(+), 107 deletions(-)
diff --git a/libminifi/include/core/TypedValues.h b/libminifi/include/core/TypedValues.h
index 615a4dd0a..e4c9ef81e 100644
--- a/libminifi/include/core/TypedValues.h
+++ b/libminifi/include/core/TypedValues.h
@@ -123,6 +123,9 @@ class DataSizeValue : public TransformableValue, public state::response::UInt64V
// Convert String to Integer
template<std::integral T>
static bool StringToInt(const std::string &input, T &output) {
+ if (input.empty()) {
+ return false;
+ }
int64_t value;
std::string unit_str;
try {
diff --git a/libminifi/include/core/flow/Node.h b/libminifi/include/core/flow/Node.h
index eed8ce9b6..ed32d8843 100644
--- a/libminifi/include/core/flow/Node.h
+++ b/libminifi/include/core/flow/Node.h
@@ -90,11 +90,12 @@ class Node {
virtual bool isMap() const = 0;
virtual bool isNull() const = 0;
- virtual nonstd::expected<std::string, std::exception_ptr> getString() const = 0;
- virtual nonstd::expected<bool, std::exception_ptr> getBool() const = 0;
- virtual nonstd::expected<int64_t, std::exception_ptr> getInt64() const = 0;
- virtual nonstd::expected<std::string, std::exception_ptr> getIntegerAsString() const = 0;
- virtual nonstd::expected<std::string, std::exception_ptr> getScalarAsString() const = 0;
+ [[nodiscard]] virtual std::optional<std::string> getString() const = 0;
+ [[nodiscard]] virtual std::optional<bool> getBool() const = 0;
+ [[nodiscard]] virtual std::optional<int64_t> getInt64() const = 0;
+
+ [[nodiscard]] virtual std::optional<std::string> getIntegerAsString() const = 0;
+ [[nodiscard]] virtual std::optional<std::string> getScalarAsString() const = 0;
virtual std::string getDebugString() const = 0;
@@ -120,11 +121,12 @@ class Node {
bool isMap() const {return impl_->isMap();}
bool isNull() const {return impl_->isNull();}
- nonstd::expected<std::string, std::exception_ptr> getString() const {return impl_->getString();}
- nonstd::expected<bool, std::exception_ptr> getBool() const {return impl_->getBool();}
- nonstd::expected<int64_t, std::exception_ptr> getInt64() const {return impl_->getInt64();}
- nonstd::expected<std::string, std::exception_ptr> getIntegerAsString() const {return impl_->getIntegerAsString();}
- nonstd::expected<std::string, std::exception_ptr> getScalarAsString() const {return impl_->getScalarAsString();}
+ [[nodiscard]] std::optional<std::string> getString() const {return impl_->getString(); }
+ [[nodiscard]] std::optional<bool> getBool() const { return impl_->getBool(); }
+ [[nodiscard]] std::optional<int64_t> getInt64() const { return impl_->getInt64(); }
+
+ [[nodiscard]] std::optional<std::string> getIntegerAsString() const { return impl_->getIntegerAsString(); }
+ [[nodiscard]] std::optional<std::string> getScalarAsString() const { return impl_->getScalarAsString(); }
// return a string representation of the node (need not be deserializable)
std::string getDebugString() const {return impl_->getDebugString();}
diff --git a/libminifi/include/core/json/JsonNode.h b/libminifi/include/core/json/JsonNode.h
index f6b9b1c9f..c9724e94d 100644
--- a/libminifi/include/core/json/JsonNode.h
+++ b/libminifi/include/core/json/JsonNode.h
@@ -50,72 +50,58 @@ class JsonNode : public flow::Node::NodeImpl {
return flow::Node{std::make_shared<JsonNode>(nullptr)};
}
- nonstd::expected<std::string, std::exception_ptr> getString() const override {
- try {
- if (!node_) {
- throw std::runtime_error(fmt::format("Cannot get string of invalid json value at '{}'", path_));
- }
- if (!node_->IsString()) {
- throw std::runtime_error(fmt::format("Cannot get string of non-string json value at '{}'", path_));
- }
+ [[nodiscard]] std::optional<std::string> getString() const override {
+ if (node_ && node_->IsString()) {
return std::string{node_->GetString(), node_->GetStringLength()};
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
}
+ return std::nullopt;
}
- nonstd::expected<int64_t, std::exception_ptr> getInt64() const override {
- try {
- if (!node_) {
- throw std::runtime_error(fmt::format("Cannot get int64 of invalid json value at '{}'", path_));
- }
- if (!node_->IsInt64()) {
- throw std::runtime_error(fmt::format("Cannot get int64 of non-int64 json value at '{}'", path_));
- }
+ [[nodiscard]] std::optional<int64_t> getInt64() const override {
+ if (node_ && node_->IsInt64()) {
return node_->GetInt64();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
}
+ return std::nullopt;
}
- nonstd::expected<bool, std::exception_ptr> getBool() const override {
- try {
- if (!node_) {
- throw std::runtime_error(fmt::format("Cannot get bool of invalid json value at '{}'", path_));
- }
- if (!node_->IsBool()) {
- throw std::runtime_error(fmt::format("Cannot get bool of non-bool json value at '{}'", path_));
- }
+ [[nodiscard]] std::optional<bool> getBool() const override {
+ if (node_ && node_->IsBool()) {
return node_->GetBool();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
}
+ return std::nullopt;
}
- nonstd::expected<std::string, std::exception_ptr> getIntegerAsString() const override {
- try {
- if (!node_) throw std::runtime_error(fmt::format("Cannot get string from invalid json value at '{}'", path_));
- if (node_->IsInt64()) return std::to_string(node_->GetInt64());
- if (node_->IsUint64()) return std::to_string(node_->GetUint64());
- if (node_->IsString()) return std::string(node_->GetString(), node_->GetStringLength());
- throw std::runtime_error(fmt::format("Cannot get string from non-integer json value at '{}'", path_));
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
+ [[nodiscard]] std::optional<std::string> getIntegerAsString() const override {
+ if (!node_) {
+ return std::nullopt;
+ }
+ if (node_->IsInt64()) {
+ return std::to_string(node_->GetInt64());
+ }
+ if (node_->IsUint64()) {
+ return std::to_string(node_->GetUint64());
}
+ return getString();
}
- nonstd::expected<std::string, std::exception_ptr> getScalarAsString() const override {
- try {
- if (!node_) throw std::runtime_error(fmt::format("Cannot get string from invalid json value at '{}'", path_));
- if (node_->IsBool()) return node_->GetBool() ? "true" : "false";
- if (node_->IsInt64()) return std::to_string(node_->GetInt64());
- if (node_->IsUint64()) return std::to_string(node_->GetUint64());
- if (node_->IsString()) return std::string(node_->GetString(), node_->GetStringLength());
- if (node_->IsDouble()) return std::to_string(node_->GetDouble());
- throw std::runtime_error(fmt::format("Cannot convert non-scalar json value to string at '{}'", path_));
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
+ [[nodiscard]] std::optional<std::string> getScalarAsString() const override {
+ if (!node_) {
+ return std::nullopt;
+ }
+ if (node_->IsInt64()) {
+ return std::to_string(node_->GetInt64());
+ }
+ if (node_->IsUint64()) {
+ return std::to_string(node_->GetUint64());
}
+ if (node_->IsDouble()) {
+ return std::to_string(node_->GetDouble());
+ }
+ if (node_->IsBool()) {
+ return std::to_string(node_->GetDouble());
+ }
+
+ return getString();
}
std::string getDebugString() const override {
diff --git a/libminifi/include/core/yaml/YamlNode.h b/libminifi/include/core/yaml/YamlNode.h
index 193c61d55..c8ca52c91 100644
--- a/libminifi/include/core/yaml/YamlNode.h
+++ b/libminifi/include/core/yaml/YamlNode.h
@@ -53,44 +53,36 @@ class YamlNode : public flow::Node::NodeImpl {
return flow::Node{std::make_shared<YamlNode>(YAML::Node{YAML::NodeType::Undefined})};
}
- nonstd::expected<std::string, std::exception_ptr> getString() const override {
- try {
- return node_.as<std::string>();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
+ std::optional<std::string> getString() const override {
+ if (std::string result; YAML::convert<std::string>::decode(node_, result)) {
+ return result;
}
+ if (node_ && node_.IsNull()) {
+ return "null";
+ }
+ return std::nullopt;
}
- nonstd::expected<bool, std::exception_ptr> getBool() const override {
- try {
- return node_.as<bool>();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
+ std::optional<bool> getBool() const override {
+ if (bool result; YAML::convert<bool>::decode(node_, result)) {
+ return result;
}
+ return std::nullopt;
}
- nonstd::expected<int64_t, std::exception_ptr> getInt64() const override {
- try {
- return node_.as<int64_t>();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
+ std::optional<int64_t> getInt64() const override {
+ if (int64_t result; YAML::convert<int64_t>::decode(node_, result)) {
+ return result;
}
+ return std::nullopt;
}
- nonstd::expected<std::string, std::exception_ptr> getIntegerAsString() const override {
- try {
- return node_.as<std::string>();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
- }
+ std::optional<std::string> getIntegerAsString() const override {
+ return getString();
}
- nonstd::expected<std::string, std::exception_ptr> getScalarAsString() const override {
- try {
- return node_.as<std::string>();
- } catch (...) {
- return nonstd::make_unexpected(std::current_exception());
- }
+ std::optional<std::string> getScalarAsString() const override {
+ return getString();
}
std::string getDebugString() const override {
diff --git a/libminifi/src/properties/Properties.cpp b/libminifi/src/properties/Properties.cpp
index 360fb2a94..45ebda39a 100644
--- a/libminifi/src/properties/Properties.cpp
+++ b/libminifi/src/properties/Properties.cpp
@@ -16,15 +16,16 @@
* limitations under the License.
*/
#include "properties/Properties.h"
+
#include <fstream>
#include <string>
-#include <utility>
-#include "utils/StringUtils.h"
-#include "utils/file/FileUtils.h"
-#include "utils/file/PathUtils.h"
+
#include "core/logging/LoggerConfiguration.h"
-#include "properties/PropertiesFile.h"
#include "properties/Configuration.h"
+#include "properties/PropertiesFile.h"
+#include "range/v3/algorithm/all_of.hpp"
+#include "utils/StringUtils.h"
+#include "utils/file/FileUtils.h"
namespace org::apache::nifi::minifi {
@@ -47,13 +48,10 @@ bool Properties::getString(const std::string &key, std::string &value) const {
}
std::optional<std::string> Properties::getString(const std::string& key) const {
- std::string result;
- const bool found = getString(key, result);
- if (found) {
+ if (std::string result; getString(key, result)) {
return result;
- } else {
- return std::nullopt;
}
+ return std::nullopt;
}
int Properties::getInt(const std::string &key, int default_value) const {
@@ -72,20 +70,34 @@ const core::PropertyValidator* getValidator(const std::string& lookup_value) {
return nullptr;
}
-std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
- if (validator != &core::StandardPropertyTypes::TIME_PERIOD_TYPE)
+// isdigit requires unsigned chars as input
+bool allDigits(const std::string& value) {
+ return ranges::all_of(value, [](const unsigned char c){ return ::isdigit(c); });
+}
+
+// isdigit requires unsigned chars as input
+bool allDigitsOrSpaces(const std::string& value) {
+ return ranges::all_of(value, [](const unsigned char c) { return std::isdigit(c) || std::isspace(c);});
+}
+
+std::optional<std::string> ensureTimePeriodValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, const std::string& value) {
+ if (validator != &core::StandardPropertyTypes::TIME_PERIOD_TYPE) {
return std::nullopt;
- if (value.empty() || !std::all_of(value.begin(), value.end(), [](unsigned char c){ return ::isdigit(c); }))
+ }
+ if (value.empty() || !allDigits(value)) {
return std::nullopt;
+ }
return value + " ms";
}
-std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, std::string& value) {
- if (validator != &core::StandardPropertyTypes::DATA_SIZE_TYPE)
+std::optional<std::string> ensureDataSizeValidatedPropertyHasExplicitUnit(const core::PropertyValidator* const validator, const std::string& value) {
+ if (validator != &core::StandardPropertyTypes::DATA_SIZE_TYPE) {
return std::nullopt;
- if (value.empty() || !std::all_of(value.begin(), value.end(), [](unsigned char c){ return ::isdigit(c); }))
+ }
+ if (value.empty() || !allDigits(value)) {
return std::nullopt;
+ }
return value + " B";
}
@@ -97,14 +109,14 @@ bool integerValidatedProperty(const core::PropertyValidator* const validator) {
|| validator == &core::StandardPropertyTypes::UNSIGNED_LONG_TYPE;
}
-std::optional<int64_t> stringToDataSize(std::string_view input) {
+std::optional<int64_t> stringToDataSize(const std::string_view input) {
int64_t value = 0;
std::string unit_str;
if (!utils::string::splitToValueAndUnit(input, value, unit_str)) {
return std::nullopt;
}
- if (auto unit_multiplier = core::DataSizeValue::getUnitMultiplier(unit_str)) {
+ if (const auto unit_multiplier = core::DataSizeValue::getUnitMultiplier(unit_str)) {
return value * *unit_multiplier;
}
return std::nullopt;
@@ -120,12 +132,16 @@ std::optional<int64_t> stringToDataTransferSpeed(std::string_view input) {
return stringToDataSize(data_size);
}
-std::optional<std::string> ensureIntegerValidatedPropertyHasNoUnit(const core::PropertyValidator* const validator, std::string& value) {
+std::optional<std::string> ensureIntegerValidatedPropertyHasNoUnit(const core::PropertyValidator* const validator, const std::string& value) {
if (!integerValidatedProperty(validator)) {
return std::nullopt;
}
- if (auto parsed_time = utils::timeutils::StringToDuration<std::chrono::milliseconds>(value)) {
+ if (allDigitsOrSpaces(value)) {
+ return std::nullopt;
+ }
+
+ if (const auto parsed_time = utils::timeutils::StringToDuration<std::chrono::milliseconds>(value)) {
return fmt::format("{}", parsed_time->count());
}