[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #797: MINIFICPP-1231 - General property validation + use them in MergeContent.
arpadboda commented on a change in pull request #797:
URL: https://github.com/apache/nifi-minifi-cpp/pull/797#discussion_r445648023
##
File path: libminifi/include/core/ConfigurableComponent.h
##
@@ -215,18 +215,23 @@ bool ConfigurableComponent::getProperty(const std::string
name, T ) const
auto & = properties_.find(name);
if (it != properties_.end()) {
- Property item = it->second;
- value = static_cast(item.getValue());
- if (item.getValue().getValue() != nullptr) {
- logger_->log_debug("Component %s property name %s value %s", name,
item.getName(), item.getValue().to_string());
- return true;
- } else {
- logger_->log_warn("Component %s property name %s, empty value", name,
item.getName());
- return false;
- }
+const Property& item = it->second;
+if (item.getValue().getValue() == nullptr) {
+ // empty value
+ if (item.getRequired()) {
+logger_->log_debug("Component %s required property %s is empty", name,
item.getName());
+throw utils::internal::RequiredPropertyMissingException("Required
property is empty: " + item.getName());
Review comment:
I think this just adds a secondary safety net, processors are already
prepared to handle configuration errors (and throwing exceptions in
onSchedule), so I don't mind this change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #797: MINIFICPP-1231 - General property validation + use them in MergeContent.
arpadboda commented on a change in pull request #797:
URL: https://github.com/apache/nifi-minifi-cpp/pull/797#discussion_r445648023
##
File path: libminifi/include/core/ConfigurableComponent.h
##
@@ -215,18 +215,23 @@ bool ConfigurableComponent::getProperty(const std::string
name, T ) const
auto & = properties_.find(name);
if (it != properties_.end()) {
- Property item = it->second;
- value = static_cast(item.getValue());
- if (item.getValue().getValue() != nullptr) {
- logger_->log_debug("Component %s property name %s value %s", name,
item.getName(), item.getValue().to_string());
- return true;
- } else {
- logger_->log_warn("Component %s property name %s, empty value", name,
item.getName());
- return false;
- }
+const Property& item = it->second;
+if (item.getValue().getValue() == nullptr) {
+ // empty value
+ if (item.getRequired()) {
+logger_->log_debug("Component %s required property %s is empty", name,
item.getName());
+throw utils::internal::RequiredPropertyMissingException("Required
property is empty: " + item.getName());
Review comment:
I think the it just adds a secondary safety net, processors are already
prepared to handle configuration errors (and throwing exceptions in
onSchedule), so I don't mind this change.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #797: MINIFICPP-1231 - General property validation + use them in MergeContent.
arpadboda commented on a change in pull request #797:
URL: https://github.com/apache/nifi-minifi-cpp/pull/797#discussion_r443498165
##
File path: libminifi/include/core/PropertyValue.h
##
@@ -202,14 +196,50 @@ class PropertyValue : public state::response::ValueNode {
auto operator=(const std::string ) -> typename std::enable_if<
std::is_same::value ||
std::is_same::value, PropertyValue&>::type {
-value_ = std::make_shared(ref);
-type_id = value_->getTypeIndex();
-return *this;
+validator_.clearValidationResult();
+return WithAssignmentGuard(ref, [&] () -> PropertyValue& {
+ value_ = std::make_shared(ref);
+ type_id = value_->getTypeIndex();
+ return *this;
+});
+ }
+
+ private:
+ template
+ T convertImpl(const char* const type_name) const {
+if (!isValueUsable()) {
+ throw utils::InvalidValueException("Cannot convert invalid value");
+}
+T res;
+if (value_->convertValue(res)) {
+ return res;
+}
+throw utils::ConversionException(std::string("Invalid conversion to ") +
type_name + " for " + value_->getStringValue());
+ }
+
+ bool isValueUsable() const {
+if (!value_) return false;
+if (validator_.isValid() == CachedValueValidator::Result::FAILURE) return
false;
+if (validator_.isValid() == CachedValueValidator::Result::SUCCESS) return
true;
+return validate("__unknown__").valid();
+ }
+
+ template
+ auto WithAssignmentGuard(const std::string& ref, Fn&& functor) ->
decltype(std::forward(functor)()) {
+// TODO(adebreceni): as soon as c++17 comes jump to a RAII implementation
+// as we will need std::uncaught_exceptions()
+try {
+ return std::forward(functor)();
+} catch(const utils::ValueException&) {
+ type_id = std::type_index(typeid(std::string));
+ value_ = minifi::state::response::createValue(ref);
+ throw;
+}
}
protected:
std::type_index type_id;
- std::shared_ptr validator_;
+ CachedValueValidator validator_;
Review comment:
I can definitely live with this :)
Property validation has always been lagging behind, seems to be properly
done finally.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
