This is an automated email from the ASF dual-hosted git repository.
aboda pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
The following commit(s) were added to refs/heads/master by this push:
new 2b29b7b MINIFICPP-1231 - Early property value validation
MINIFICPP-1231 - Use property validators in MergeContent MINIFICPP-1231 - Fix
MergeContent allowed values.
2b29b7b is described below
commit 2b29b7b5440ae73c20defa91ac471cb44a0d2683
Author: Adam Debreceni <[email protected]>
AuthorDate: Wed May 27 11:03:51 2020 +0200
MINIFICPP-1231 - Early property value validation
MINIFICPP-1231 - Use property validators in MergeContent
MINIFICPP-1231 - Fix MergeContent allowed values.
Signed-off-by: Arpad Boda <[email protected]>
This closes #797
---
.../coap/controllerservice/CoapConnector.cpp | 2 +-
extensions/libarchive/MergeContent.cpp | 27 ++-
extensions/libarchive/MergeContent.h | 2 +-
.../tests/unit/ProcessorTests.cpp | 3 +-
libminifi/include/core/CachedValueValidator.h | 126 +++++++++++
libminifi/include/core/ConfigurableComponent.h | 27 ++-
libminifi/include/core/Property.h | 71 +++---
libminifi/include/core/PropertyValidation.h | 37 ++--
libminifi/include/core/PropertyValue.h | 143 +++++++------
libminifi/include/core/state/Value.h | 105 ++++-----
libminifi/include/utils/PropertyErrors.h | 93 ++++++++
libminifi/include/utils/ValueParser.h | 166 ++++++++++++++
libminifi/src/core/ConfigurableComponent.cpp | 68 +++---
libminifi/src/core/PropertyValidation.cpp | 24 +--
libminifi/test/archive-tests/MergeFileTests.cpp | 12 +-
libminifi/test/unit/PropertyValidationTests.cpp | 238 +++++++++++++++++++++
nanofi/include/blocks/comms.h | 8 +-
17 files changed, 912 insertions(+), 240 deletions(-)
diff --git a/extensions/coap/controllerservice/CoapConnector.cpp
b/extensions/coap/controllerservice/CoapConnector.cpp
index 1d152ff..31ff0a3 100644
--- a/extensions/coap/controllerservice/CoapConnector.cpp
+++ b/extensions/coap/controllerservice/CoapConnector.cpp
@@ -39,7 +39,7 @@ static core::Property RemoteServer;
static core::Property Port;
static core::Property MaxQueueSize;
-core::Property
CoapConnectorService::RemoteServer(core::PropertyBuilder::createProperty("Remote
Server")->withDescription("Remote CoAP server")->isRequired(true)->build());
+core::Property
CoapConnectorService::RemoteServer(core::PropertyBuilder::createProperty("Remote
Server")->withDescription("Remote CoAP server")->isRequired(false)->build());
core::Property CoapConnectorService::Port(
core::PropertyBuilder::createProperty("Remote
Port")->withDescription("Remote CoAP server
port")->withDefaultValue<uint64_t>(8181)->isRequired(true)->build());
core::Property CoapConnectorService::MaxQueueSize(
diff --git a/extensions/libarchive/MergeContent.cpp
b/extensions/libarchive/MergeContent.cpp
index 8bd3f79..137a872 100644
--- a/extensions/libarchive/MergeContent.cpp
+++ b/extensions/libarchive/MergeContent.cpp
@@ -39,14 +39,29 @@ namespace nifi {
namespace minifi {
namespace processors {
-core::Property MergeContent::MergeStrategy("Merge Strategy", "Defragment or
Bin-Packing Algorithm", MERGE_STRATEGY_DEFRAGMENT);
-core::Property MergeContent::MergeFormat("Merge Format", "Merge Format",
MERGE_FORMAT_CONCAT_VALUE);
+core::Property MergeContent::MergeStrategy(
+ core::PropertyBuilder::createProperty("Merge Strategy")
+ ->withDescription("Defragment or Bin-Packing Algorithm")
+ ->withAllowableValues<std::string>({MERGE_STRATEGY_DEFRAGMENT,
MERGE_STRATEGY_BIN_PACK})
+ ->withDefaultValue(MERGE_STRATEGY_DEFRAGMENT)->build());
+core::Property MergeContent::MergeFormat(
+ core::PropertyBuilder::createProperty("Merge Format")
+ ->withDescription("Merge Format")
+ ->withAllowableValues<std::string>({MERGE_FORMAT_CONCAT_VALUE,
MERGE_FORMAT_TAR_VALUE, MERGE_FORMAT_ZIP_VALUE})
+ ->withDefaultValue(MERGE_FORMAT_CONCAT_VALUE)->build());
core::Property MergeContent::CorrelationAttributeName("Correlation Attribute
Name", "Correlation Attribute Name", "");
-core::Property MergeContent::DelimiterStratgey("Delimiter Strategy",
"Determines if Header, Footer, and Demarcator should point to files",
DELIMITER_STRATEGY_FILENAME);
+core::Property MergeContent::DelimiterStrategy(
+ core::PropertyBuilder::createProperty("Delimiter Strategy")
+ ->withDescription("Determines if Header, Footer, and Demarcator should point
to files")
+ ->withAllowableValues<std::string>({DELIMITER_STRATEGY_FILENAME,
DELIMITER_STRATEGY_TEXT})
+ ->withDefaultValue(DELIMITER_STRATEGY_FILENAME)->build());
core::Property MergeContent::Header("Header File", "Filename specifying the
header to use", "");
core::Property MergeContent::Footer("Footer File", "Filename specifying the
footer to use", "");
core::Property MergeContent::Demarcator("Demarcator File", "Filename
specifying the demarcator to use", "");
-core::Property MergeContent::KeepPath("Keep Path", "If using the Zip or Tar
Merge Format, specifies whether or not the FlowFiles' paths should be included
in their entry", "false");
+core::Property MergeContent::KeepPath(
+ core::PropertyBuilder::createProperty("Keep Path")
+ ->withDescription("If using the Zip or Tar Merge Format, specifies whether
or not the FlowFiles' paths should be included in their entry")
+ ->withDefaultValue(false)->build());
core::Relationship MergeContent::Merge("merged", "The FlowFile containing the
merged content");
const char *BinaryConcatenationMerge::mimeType = "application/octet-stream";
const char *TarMerge::mimeType = "application/tar";
@@ -64,7 +79,7 @@ void MergeContent::initialize() {
properties.insert(MergeStrategy);
properties.insert(MergeFormat);
properties.insert(CorrelationAttributeName);
- properties.insert(DelimiterStratgey);
+ properties.insert(DelimiterStrategy);
properties.insert(Header);
properties.insert(Footer);
properties.insert(Demarcator);
@@ -106,7 +121,7 @@ void MergeContent::onSchedule(core::ProcessContext
*context, core::ProcessSessio
this->correlationAttributeName_ = value;
}
value = "";
- if (context->getProperty(DelimiterStratgey.getName(), value) &&
!value.empty()) {
+ if (context->getProperty(DelimiterStrategy.getName(), value) &&
!value.empty()) {
this->delimiterStratgey_ = value;
}
value = "";
diff --git a/extensions/libarchive/MergeContent.h
b/extensions/libarchive/MergeContent.h
index 40df40d..65066bc 100644
--- a/extensions/libarchive/MergeContent.h
+++ b/extensions/libarchive/MergeContent.h
@@ -282,7 +282,7 @@ class MergeContent : public processors::BinFiles {
static core::Property MergeStrategy;
static core::Property MergeFormat;
static core::Property CorrelationAttributeName;
- static core::Property DelimiterStratgey;
+ static core::Property DelimiterStrategy;
static core::Property KeepPath;
static core::Property Header;
static core::Property Footer;
diff --git a/extensions/standard-processors/tests/unit/ProcessorTests.cpp
b/extensions/standard-processors/tests/unit/ProcessorTests.cpp
index 19eb414..f7dc551 100644
--- a/extensions/standard-processors/tests/unit/ProcessorTests.cpp
+++ b/extensions/standard-processors/tests/unit/ProcessorTests.cpp
@@ -39,6 +39,7 @@
#include "core/ProcessSession.h"
#include "core/ProcessorNode.h"
#include "core/reporting/SiteToSiteProvenanceReportingTask.h"
+#include "utils/PropertyErrors.h"
TEST_CASE("Test Creation of GetFile", "[getfileCreate]") {
TestController testController;
@@ -341,7 +342,7 @@ TEST_CASE("LogAttributeTestInvalid", "[TestLogAttribute]") {
plan->setProperty(getfile,
org::apache::nifi::minifi::processors::GetFile::Directory.getName(), dir);
plan->setProperty(getfile,
org::apache::nifi::minifi::processors::GetFile::BatchSize.getName(), "1");
- REQUIRE_THROWS_AS(plan->setProperty(loggattr,
org::apache::nifi::minifi::processors::LogAttribute::FlowFilesToLog.getName(),
"-1"), std::out_of_range);
+ REQUIRE_THROWS_AS(plan->setProperty(loggattr,
org::apache::nifi::minifi::processors::LogAttribute::FlowFilesToLog.getName(),
"-1"), utils::internal::ParseException);
LogTestController::getInstance().reset();
}
diff --git a/libminifi/include/core/CachedValueValidator.h
b/libminifi/include/core/CachedValueValidator.h
new file mode 100644
index 0000000..1076027
--- /dev/null
+++ b/libminifi/include/core/CachedValueValidator.h
@@ -0,0 +1,126 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef LIBMINIFI_INCLUDE_CORE_CACHEDVALUEVALIDATOR_H_
+#define LIBMINIFI_INCLUDE_CORE_CACHEDVALUEVALIDATOR_H_
+
+#include <utility>
+#include <memory>
+#include <string>
+#include "PropertyValidation.h"
+#include "state/Value.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+class PropertyValue;
+
+namespace internal {
+
+class CachedValueValidator {
+ friend class core::PropertyValue;
+
+ public:
+ enum class Result {
+ FAILURE,
+ SUCCESS,
+ RECOMPUTE
+ };
+
+ CachedValueValidator() = default;
+
+ CachedValueValidator(const CachedValueValidator& other) :
validator_(other.validator_) {}
+
+ CachedValueValidator(CachedValueValidator&& other) noexcept :
validator_(std::move(other.validator_)) {}
+
+ CachedValueValidator& operator=(const CachedValueValidator& other) {
+ if (this == &other) {
+ return *this;
+ }
+ setValidator(other.validator_);
+ return *this;
+ }
+
+ CachedValueValidator& operator=(CachedValueValidator&& other) {
+ if (this == &other) {
+ return *this;
+ }
+ setValidator(std::move(other.validator_));
+ return *this;
+ }
+
+ explicit CachedValueValidator(const std::shared_ptr<PropertyValidator>&
other) : validator_(other) {}
+
+ explicit CachedValueValidator(std::shared_ptr<PropertyValidator>&& other) :
validator_(std::move(other)) {}
+
+ CachedValueValidator& operator=(const
gsl::not_null<std::shared_ptr<PropertyValidator>>& new_validator) {
+ setValidator(new_validator);
+ return *this;
+ }
+
+ CachedValueValidator&
operator=(gsl::not_null<std::shared_ptr<PropertyValidator>>&& new_validator) {
+ setValidator(std::move(new_validator));
+ return *this;
+ }
+
+ const gsl::not_null<std::shared_ptr<PropertyValidator>>& operator*() const {
+ return validator_;
+ }
+
+ private:
+ template<typename T>
+ void setValidator(T&& newValidator) {
+ invalidateCachedResult();
+ validator_ = std::forward<T>(newValidator);
+ }
+
+ ValidationResult validate(const std::string& subject, const
std::shared_ptr<state::response::Value>& value) const {
+ if (validation_result_ == CachedValueValidator::Result::SUCCESS) {
+ return ValidationResult::Builder::createBuilder().isValid(true).build();
+ }
+ if (validation_result_ == CachedValueValidator::Result::FAILURE) {
+ return
ValidationResult::Builder::createBuilder().withSubject(subject).withInput(value->getStringValue()).isValid(false).build();
+ }
+ auto result = validator_->validate(subject, value);
+ if (result.valid()) {
+ validation_result_ = Result::SUCCESS;
+ } else {
+ validation_result_ = Result::FAILURE;
+ }
+ return result;
+ }
+
+ void invalidateCachedResult() {
+ validation_result_ = Result::RECOMPUTE;
+ }
+
+ gsl::not_null<std::shared_ptr<PropertyValidator>>
validator_{StandardValidators::VALID_VALIDATOR()};
+ mutable Result validation_result_{Result::RECOMPUTE};
+};
+
+} /* namespace internal */
+} /* namespace core */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
+
+#endif // LIBMINIFI_INCLUDE_CORE_CACHEDVALUEVALIDATOR_H_
diff --git a/libminifi/include/core/ConfigurableComponent.h
b/libminifi/include/core/ConfigurableComponent.h
index 794011f..61357aa 100644
--- a/libminifi/include/core/ConfigurableComponent.h
+++ b/libminifi/include/core/ConfigurableComponent.h
@@ -215,18 +215,23 @@ bool ConfigurableComponent::getProperty(const std::string
name, T &value) const
auto &&it = properties_.find(name);
if (it != properties_.end()) {
- Property item = it->second;
- value = static_cast<T>(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_error("Component %s required property %s is empty", name,
item.getName());
+ throw utils::internal::RequiredPropertyMissingException("Required
property is empty: " + item.getName());
+ }
+ logger_->log_debug("Component %s property name %s, empty value", name,
item.getName());
+ return false;
+ }
+ logger_->log_debug("Component %s property name %s value %s", name,
item.getName(), item.getValue().to_string());
+ // cast throws if the value is invalid
+ value = static_cast<T>(item.getValue());
+ return true;
} else {
- logger_->log_warn("Could not find property %s", name);
- return false;
+ logger_->log_warn("Could not find property %s", name);
+ return false;
}
}
diff --git a/libminifi/include/core/Property.h
b/libminifi/include/core/Property.h
index 142f784..01937e7 100644
--- a/libminifi/include/core/Property.h
+++ b/libminifi/include/core/Property.h
@@ -18,12 +18,9 @@
#ifndef LIBMINIFI_INCLUDE_CORE_PROPERTY_H_
#define LIBMINIFI_INCLUDE_CORE_PROPERTY_H_
-#include <math.h>
-#include <stdlib.h>
-
+#include <cmath>
+#include <cstdlib>
#include <algorithm>
-#include <atomic>
-#include <functional>
#include <map>
#include <memory>
#include <mutex>
@@ -35,11 +32,13 @@
#include <utility>
#include <vector>
+#include "CachedValueValidator.h"
#include "core/Core.h"
#include "PropertyValidation.h"
#include "PropertyValue.h"
#include "utils/StringUtils.h"
#include "utils/TimeUtil.h"
+#include "utils/gsl.h"
namespace org {
namespace apache {
@@ -53,6 +52,9 @@ class Property {
public:
/*!
* Create a new property
+ * Pay special attention to when value is "true" or "false"
+ * as they will get coerced to the bool true and false, causing
+ * further overwrites to inherit the bool validator.
*/
Property(std::string name, std::string description, std::string value, bool
is_required, std::string valid_regex, std::vector<std::string>
dependent_properties,
std::vector<std::pair<std::string, std::string>>
exclusive_of_properties)
@@ -84,9 +86,7 @@ class Property {
is_required_(false),
is_collection_(true),
supports_el_(false),
- is_transient_(false) {
- validator_ = StandardValidators::VALID;
- }
+ is_transient_(false) {}
Property(Property &&other) = default;
@@ -98,9 +98,7 @@ class Property {
is_required_(false),
is_collection_(false),
supports_el_(false),
- is_transient_(false) {
- validator_ = StandardValidators::VALID;
- }
+ is_transient_(false) {}
virtual ~Property() = default;
@@ -130,40 +128,33 @@ class Property {
template<typename T = std::string>
void setValue(const T &value) {
- PropertyValue vn = default_value_;
- vn = value;
- if (validator_) {
- vn.setValidator(validator_);
- ValidationResult result = validator_->validate(name_, vn.getValue());
- if (!result.valid()) {
- // throw some exception?
- }
- } else {
- vn.setValidator(core::StandardValidators::VALID);
- }
if (!is_collection_) {
values_.clear();
- values_.push_back(vn);
+ values_.push_back(default_value_);
} else {
- values_.push_back(vn);
+ values_.push_back(default_value_);
+ }
+ PropertyValue& vn = values_.back();
+ vn.setValidator(validator_);
+ vn = value;
+ ValidationResult result = vn.validate(name_);
+ if (!result.valid()) {
+ throw utils::internal::InvalidValueException(name_ + " value validation
failed");
}
}
- void setValue(PropertyValue &vn) {
- if (validator_) {
- vn.setValidator(validator_);
- ValidationResult result = validator_->validate(name_, vn.getValue());
- if (!result.valid()) {
- // throw some exception?
- }
- } else {
- vn.setValidator(core::StandardValidators::VALID);
- }
+ void setValue(PropertyValue &newValue) {
if (!is_collection_) {
values_.clear();
- values_.push_back(vn);
+ values_.push_back(newValue);
} else {
- values_.push_back(vn);
+ values_.push_back(newValue);
+ }
+ PropertyValue& vn = values_.back();
+ vn.setValidator(validator_);
+ ValidationResult result = vn.validate(name_);
+ if (!result.valid()) {
+ throw utils::internal::InvalidValueException(name_ + " value validation
failed");
}
}
void setSupportsExpressionLanguage(bool supportEl);
@@ -464,7 +455,7 @@ class Property {
validator_ = StandardValidators::getValidator(ret.getValue());
} else {
ret = value;
- validator_ = StandardValidators::VALID;
+ validator_ = StandardValidators::VALID_VALIDATOR();
}
return ret;
}
@@ -478,7 +469,7 @@ class Property {
bool is_collection_;
PropertyValue default_value_;
std::vector<PropertyValue> values_;
- std::shared_ptr<PropertyValidator> validator_;
+ gsl::not_null<std::shared_ptr<PropertyValidator>>
validator_{StandardValidators::VALID_VALIDATOR()};
std::string display_name_;
std::vector<PropertyValue> allowed_values_;
// types represents the allowable types for this property
@@ -529,8 +520,8 @@ class PropertyBuilder : public
std::enable_shared_from_this<PropertyBuilder> {
prop.default_value_ = df;
if (validator != nullptr) {
- prop.default_value_.setValidator(validator);
- prop.validator_ = validator;
+ prop.default_value_.setValidator(gsl::make_not_null(validator));
+ prop.validator_ = gsl::make_not_null(validator);
} else {
prop.validator_ =
StandardValidators::getValidator(prop.default_value_.getValue());
prop.default_value_.setValidator(prop.validator_);
diff --git a/libminifi/include/core/PropertyValidation.h
b/libminifi/include/core/PropertyValidation.h
index 2847369..2f1e593 100644
--- a/libminifi/include/core/PropertyValidation.h
+++ b/libminifi/include/core/PropertyValidation.h
@@ -315,16 +315,12 @@ class TimePeriodValidator : public PropertyValidator {
class StandardValidators {
public:
- static std::shared_ptr<PropertyValidator> VALID;
-
- static const std::shared_ptr<PropertyValidator> &getValidator(const
std::shared_ptr<minifi::state::response::Value> &input) {
+ static const gsl::not_null<std::shared_ptr<PropertyValidator>>
&getValidator(const std::shared_ptr<minifi::state::response::Value> &input) {
static StandardValidators init;
if (std::dynamic_pointer_cast<core::DataSizeValue>(input) != nullptr) {
return init.DATA_SIZE_VALIDATOR;
} else if (std::dynamic_pointer_cast<core::TimePeriodValue>(input) !=
nullptr) {
return init.TIME_PERIOD_VALIDATOR;
- } else if (std::dynamic_pointer_cast<core::DataSizeValue>(input) !=
nullptr) {
- return init.DATA_SIZE_VALIDATOR;
} else if
(std::dynamic_pointer_cast<minifi::state::response::BoolValue>(input) !=
nullptr) {
return init.BOOLEAN_VALIDATOR;
} else if
(std::dynamic_pointer_cast<minifi::state::response::IntValue>(input) !=
nullptr) {
@@ -336,29 +332,34 @@ class StandardValidators {
} else if
(std::dynamic_pointer_cast<minifi::state::response::UInt64Value>(input) !=
nullptr) {
return init.UNSIGNED_LONG_VALIDATOR;
} else {
- return org::apache::nifi::minifi::core::StandardValidators::VALID;
+ return
org::apache::nifi::minifi::core::StandardValidators::VALID_VALIDATOR();
}
}
- static std::shared_ptr<PropertyValidator> PORT_VALIDATOR() {
- static std::shared_ptr<PropertyValidator> validator =
std::make_shared<PortValidator>("PORT_VALIDATOR");
+ static const gsl::not_null<std::shared_ptr<PropertyValidator>>&
VALID_VALIDATOR() {
+ static gsl::not_null<std::shared_ptr<PropertyValidator>>
validator(std::make_shared<AlwaysValid>(true, "VALID"));
+ return validator;
+ }
+
+ static gsl::not_null<std::shared_ptr<PropertyValidator>> PORT_VALIDATOR() {
+ static gsl::not_null<std::shared_ptr<PropertyValidator>>
validator(std::make_shared<PortValidator>("PORT_VALIDATOR"));
return validator;
}
- static std::shared_ptr<PropertyValidator> LISTEN_PORT_VALIDATOR() {
- static std::shared_ptr<PropertyValidator> validator =
std::make_shared<ListenPortValidator>("PORT_VALIDATOR");
+ static gsl::not_null<std::shared_ptr<PropertyValidator>>
LISTEN_PORT_VALIDATOR() {
+ static gsl::not_null<std::shared_ptr<PropertyValidator>>
validator(std::make_shared<ListenPortValidator>("PORT_VALIDATOR"));
return validator;
}
private:
- std::shared_ptr<PropertyValidator> INVALID;
- std::shared_ptr<PropertyValidator> INTEGER_VALIDATOR;
- std::shared_ptr<PropertyValidator> UNSIGNED_INT_VALIDATOR;
- std::shared_ptr<PropertyValidator> LONG_VALIDATOR;
- std::shared_ptr<PropertyValidator> UNSIGNED_LONG_VALIDATOR;
- std::shared_ptr<PropertyValidator> BOOLEAN_VALIDATOR;
- std::shared_ptr<PropertyValidator> DATA_SIZE_VALIDATOR;
- std::shared_ptr<PropertyValidator> TIME_PERIOD_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> INVALID;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> INTEGER_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> UNSIGNED_INT_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> LONG_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> UNSIGNED_LONG_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> BOOLEAN_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> DATA_SIZE_VALIDATOR;
+ gsl::not_null<std::shared_ptr<PropertyValidator>> TIME_PERIOD_VALIDATOR;
StandardValidators();
};
diff --git a/libminifi/include/core/PropertyValue.h
b/libminifi/include/core/PropertyValue.h
index 97e6a4b..61a82ba 100644
--- a/libminifi/include/core/PropertyValue.h
+++ b/libminifi/include/core/PropertyValue.h
@@ -18,10 +18,12 @@
#ifndef LIBMINIFI_INCLUDE_CORE_PROPERTYVALUE_H_
#define LIBMINIFI_INCLUDE_CORE_PROPERTYVALUE_H_
-#include <memory>
-#include <string>
#include <typeindex>
+#include <string>
+#include <utility>
+#include <memory>
+#include "CachedValueValidator.h"
#include "PropertyValidation.h"
#include "state/Value.h"
#include "TypedValues.h"
@@ -61,78 +63,57 @@ static inline std::shared_ptr<state::response::Value>
convert(const std::shared_
* and value translation.
*/
class PropertyValue : public state::response::ValueNode {
+ using CachedValueValidator = internal::CachedValueValidator;
+
public:
PropertyValue()
- : type_id(std::type_index(typeid(std::string))) {
- validator_ = StandardValidators::VALID;
- }
+ : type_id(std::type_index(typeid(std::string))) {}
PropertyValue(const PropertyValue &o) = default;
PropertyValue(PropertyValue &&o) noexcept = default;
- void setValidator(const std::shared_ptr<PropertyValidator> &val) {
+ void setValidator(const gsl::not_null<std::shared_ptr<PropertyValidator>>
&val) {
validator_ = val;
}
std::shared_ptr<PropertyValidator> getValidator() const {
- return validator_;
+ return *validator_;
}
ValidationResult validate(const std::string &subject) const {
- if (validator_) {
- return validator_->validate(subject, getValue());
- } else {
- return ValidationResult::Builder::createBuilder().isValid(true).build();
- }
+ return validator_.validate(subject, getValue());
}
operator uint64_t() const {
- uint64_t res;
- if (value_->convertValue(res)) {
- return res;
- }
- throw std::runtime_error("Invalid conversion to uint64_t for" +
value_->getStringValue());
+ return convertImpl<uint64_t>("uint64_t");
}
operator int64_t() const {
- int64_t res;
- if (value_->convertValue(res)) {
- return res;
- }
- throw std::runtime_error("Invalid conversion to int64_t");
+ return convertImpl<int64_t>("int64_t");
}
operator uint32_t() const {
- uint32_t res;
- if (value_->convertValue(res)) {
- return res;
- }
- throw std::runtime_error("Invalid conversion to uint32_t for" +
value_->getStringValue());
+ return convertImpl<uint32_t>("uint32_t");
}
operator int() const {
- int res;
- if (value_->convertValue(res)) {
- return res;
- }
- throw std::runtime_error("Invalid conversion to int ");
+ return convertImpl<int>("int");
}
operator bool() const {
- bool res;
- if (value_->convertValue(res)) {
- return res;
- }
- throw std::runtime_error("Invalid conversion to bool");
+ return convertImpl<bool>("bool");
}
- PropertyValue &operator=(PropertyValue &&o) = default;
- PropertyValue &operator=(const PropertyValue &o) = default;
-
operator std::string() const {
+ if (!isValueUsable()) {
+ throw utils::internal::InvalidValueException("Cannot convert invalid
value");
+ }
return to_string();
}
+ PropertyValue &operator=(PropertyValue &&o) = default;
+ PropertyValue &operator=(const PropertyValue &o) = default;
+
std::type_index getTypeInfo() const {
return type_id;
}
@@ -142,25 +123,28 @@ class PropertyValue : public state::response::ValueNode {
*/
template<typename T>
auto operator=(const T ref) -> typename std::enable_if<std::is_same<T,
std::string>::value, PropertyValue&>::type {
- if (value_ == nullptr) {
- type_id = std::type_index(typeid(T));
- value_ = minifi::state::response::createValue(ref);
- } else {
- type_id = std::type_index(typeid(T));
- auto ret = convert(value_, ref);
- if (ret != nullptr) {
- value_ = ret;
+ validator_.invalidateCachedResult();
+ return WithAssignmentGuard(ref, [&] () -> PropertyValue& {
+ if (value_ == nullptr) {
+ type_id = std::type_index(typeid(T));
+ value_ = minifi::state::response::createValue(ref);
} else {
- /**
- * This is a protection mechanism that allows us to fail properties
that are strictly defined.
- * To maintain backwards compatibility we allow strings to be set by
way of the internal API
- * We then rely on the developer of the processor to perform the
conversion. We want to get away from
- * this, so this exception will throw an exception, forcefully, when
they specify types in properties.
- */
- throw std::runtime_error("Invalid conversion");
+ type_id = std::type_index(typeid(T));
+ auto ret = convert(value_, ref);
+ if (ret != nullptr) {
+ value_ = ret;
+ } else {
+ /**
+ * This is a protection mechanism that allows us to fail properties
that are strictly defined.
+ * To maintain backwards compatibility we allow strings to be set by
way of the internal API
+ * We then rely on the developer of the processor to perform the
conversion. We want to get away from
+ * this, so this exception will throw an exception, forcefully, when
they specify types in properties.
+ */
+ throw utils::internal::ConversionException("Invalid conversion");
+ }
}
- }
- return *this;
+ return *this;
+ });
}
template<typename T>
@@ -169,6 +153,7 @@ class PropertyValue : public state::response::ValueNode {
std::is_same<T, uint64_t >::value ||
std::is_same<T, int64_t >::value ||
std::is_same<T, bool >::value, PropertyValue&>::type {
+ validator_.invalidateCachedResult();
if (value_ == nullptr) {
type_id = std::type_index(typeid(T));
value_ = minifi::state::response::createValue(ref);
@@ -184,7 +169,7 @@ class PropertyValue : public state::response::ValueNode {
} else {
// this is not the place to perform translation. There are other
places within
// the code where we can do assignments and transformations from "10"
to (int)10;
- throw std::runtime_error("Assigning invalid types");
+ throw utils::internal::ConversionException("Assigning invalid types");
}
}
return *this;
@@ -202,14 +187,48 @@ class PropertyValue : public state::response::ValueNode {
auto operator=(const std::string &ref) -> typename std::enable_if<
std::is_same<T, DataSizeValue >::value ||
std::is_same<T, TimePeriodValue >::value, PropertyValue&>::type {
- value_ = std::make_shared<T>(ref);
- type_id = value_->getTypeIndex();
- return *this;
+ validator_.invalidateCachedResult();
+ return WithAssignmentGuard(ref, [&] () -> PropertyValue& {
+ value_ = std::make_shared<T>(ref);
+ type_id = value_->getTypeIndex();
+ return *this;
+ });
+ }
+
+ private:
+ template<typename T>
+ T convertImpl(const char* const type_name) const {
+ if (!isValueUsable()) {
+ throw utils::internal::InvalidValueException("Cannot convert invalid
value");
+ }
+ T res;
+ if (value_->convertValue(res)) {
+ return res;
+ }
+ throw utils::internal::ConversionException(std::string("Invalid conversion
to ") + type_name + " for " + value_->getStringValue());
+ }
+
+ bool isValueUsable() const {
+ if (!value_) return false;
+ return validate("__unknown__").valid();
+ }
+
+ template<typename Fn>
+ auto WithAssignmentGuard(const std::string& ref, Fn&& functor) ->
decltype(std::forward<Fn>(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<Fn>(functor)();
+ } catch(const utils::internal::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<PropertyValidator> validator_;
+ CachedValueValidator validator_;
};
inline std::string conditional_conversion(const PropertyValue &v) {
diff --git a/libminifi/include/core/state/Value.h
b/libminifi/include/core/state/Value.h
index 162e269..f2b42c7 100644
--- a/libminifi/include/core/state/Value.h
+++ b/libminifi/include/core/state/Value.h
@@ -26,6 +26,7 @@
#include <string>
#include <vector>
#include <typeinfo>
+#include "utils/ValueParser.h"
namespace org {
namespace apache {
@@ -42,6 +43,8 @@ namespace response {
* representation is needed.
*/
class Value {
+ using ParseException = utils::internal::ParseException;
+
public:
explicit Value(const std::string &value)
: string_value(value),
@@ -85,35 +88,57 @@ class Value {
}
virtual bool getValue(uint32_t &ref) {
- const auto negative = string_value.find_first_of('-') != std::string::npos;
- if (negative) {
- return false;
- }
- ref = std::stoul(string_value);
+ try {
+ uint32_t value;
+ utils::internal::ValueParser(string_value).parse(value).parseEnd();
+ ref = value;
+ } catch(const ParseException&) {
+ return false;
+ }
return true;
}
virtual bool getValue(int &ref) {
- ref = std::stol(string_value);
+ try {
+ int value;
+ utils::internal::ValueParser(string_value).parse(value).parseEnd();
+ ref = value;
+ } catch(const ParseException&) {
+ return false;
+ }
return true;
}
virtual bool getValue(int64_t &ref) {
- ref = std::stoll(string_value);
+ try {
+ int64_t value;
+ utils::internal::ValueParser(string_value).parse(value).parseEnd();
+ ref = value;
+ } catch(const ParseException&) {
+ return false;
+ }
return true;
}
virtual bool getValue(uint64_t &ref) {
- const auto negative = string_value.find_first_of('-') != std::string::npos;
- if (negative) {
- return false;
- }
- ref = std::stoull(string_value);
+ try {
+ uint64_t value;
+ utils::internal::ValueParser(string_value).parse(value).parseEnd();
+ ref = value;
+ } catch(const ParseException&) {
+ return false;
+ }
return true;
}
virtual bool getValue(bool &ref) {
- std::istringstream(string_value) >> std::boolalpha >> ref;
+ try {
+ bool value;
+ utils::internal::ValueParser(string_value).parse(value).parseEnd();
+ ref = value;
+ } catch(const ParseException&) {
+ return false;
+ }
return true;
}
@@ -130,17 +155,8 @@ class UInt32Value : public Value {
}
explicit UInt32Value(const std::string &strvalue)
- : Value(strvalue),
- value(std::stoul(strvalue)) {
- /**
- * This is a fundamental change in that we would be changing where this
error occurs.
- * We should be prudent about breaking backwards compatibility, but since
Uint32Value
- * is only created with a validator and type, we **should** be okay.
- */
- const auto negative = strvalue.find_first_of('-') != std::string::npos;
- if (negative) {
- throw std::out_of_range("negative value detected");
- }
+ : Value(strvalue) {
+ utils::internal::ValueParser(strvalue).parse(value).parseEnd();
setTypeId<uint32_t>();
}
@@ -188,8 +204,8 @@ class IntValue : public Value {
}
explicit IntValue(const std::string &strvalue)
- : Value(strvalue),
- value(std::stoi(strvalue)) {
+ : Value(strvalue) {
+ utils::internal::ValueParser(strvalue).parse(value).parseEnd();
}
int getValue() const {
return value;
@@ -202,11 +218,9 @@ class IntValue : public Value {
}
virtual bool getValue(uint32_t &ref) {
- if (value >= 0) {
- ref = value;
- return true;
- }
- return false;
+ if (value < 0) return false;
+ ref = value;
+ return true;
}
virtual bool getValue(int64_t &ref) {
@@ -236,9 +250,7 @@ class BoolValue : public Value {
explicit BoolValue(const std::string &strvalue)
: Value(strvalue) {
- bool l;
- std::istringstream(strvalue) >> std::boolalpha >> l;
- value = l; // avoid warnings
+ utils::internal::ValueParser(strvalue).parse(value).parseEnd();
}
bool getValue() const {
@@ -289,17 +301,8 @@ class UInt64Value : public Value {
}
explicit UInt64Value(const std::string &strvalue)
- : Value(strvalue),
- value(std::stoull(strvalue)) {
- /**
- * This is a fundamental change in that we would be changing where this
error occurs.
- * We should be prudent about breaking backwards compatibility, but since
Uint64Value
- * is only created with a validator and type, we **should** be okay.
- */
- const auto negative = strvalue.find_first_of('-') != std::string::npos;
- if (negative) {
- throw std::out_of_range("negative value detected");
- }
+ : Value(strvalue) {
+ utils::internal::ValueParser(strvalue).parse(value).parseEnd();
setTypeId<uint64_t>();
}
@@ -344,8 +347,8 @@ class Int64Value : public Value {
setTypeId<int64_t>();
}
explicit Int64Value(const std::string &strvalue)
- : Value(strvalue),
- value(std::stoll(strvalue)) {
+ : Value(strvalue) {
+ utils::internal::ValueParser(strvalue).parse(value).parseEnd();
setTypeId<int64_t>();
}
@@ -368,11 +371,9 @@ class Int64Value : public Value {
}
virtual bool getValue(uint64_t &ref) {
- if (value >= 0) {
- ref = value;
- return true;
- }
- return false;
+ if (value < 0) return false;
+ ref = value;
+ return true;
}
virtual bool getValue(bool &ref) {
diff --git a/libminifi/include/utils/PropertyErrors.h
b/libminifi/include/utils/PropertyErrors.h
new file mode 100644
index 0000000..85474a5
--- /dev/null
+++ b/libminifi/include/utils/PropertyErrors.h
@@ -0,0 +1,93 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenseas/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef LIBMINIFI_INCLUDE_UTILS_PROPERTYERRORS_H_
+#define LIBMINIFI_INCLUDE_UTILS_PROPERTYERRORS_H_
+
+#include <string>
+
+#include "Exception.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+
+namespace core {
+
+class PropertyValue;
+class ConfigurableComponent;
+class Property;
+
+} /* namespace core */
+
+namespace utils {
+namespace internal {
+
+class ValueException : public Exception {
+ public:
+ explicit ValueException(const std::string& err) :
Exception(ExceptionType::GENERAL_EXCEPTION, err) {}
+ explicit ValueException(const char* err) :
Exception(ExceptionType::GENERAL_EXCEPTION, err) {}
+
+ // base class already has a virtual destructor
+};
+
+class PropertyException : public Exception {
+ public:
+ explicit PropertyException(const std::string& err) :
Exception(ExceptionType::GENERAL_EXCEPTION, err) {}
+ explicit PropertyException(const char* err) :
Exception(ExceptionType::GENERAL_EXCEPTION, err) {}
+
+ // base class already has a virtual destructor
+};
+
+/**
+ * Thrown during converting from and to Value
+ */
+class ConversionException : public ValueException {
+ using ValueException::ValueException;
+};
+
+/**
+ * Represents std::string -> Value conversion errors
+ */
+class ParseException : public ConversionException {
+ using ConversionException::ConversionException;
+};
+
+/**
+ * Thrown when trying to access invalid Values.
+ */
+class InvalidValueException : public ValueException {
+ using ValueException::ValueException;
+};
+
+/**
+ * When querying missing properties marked required.
+ */
+class RequiredPropertyMissingException : public PropertyException {
+ using PropertyException::PropertyException;
+};
+
+} /* namespace internal */
+} /* namespace utils */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
+
+#endif // LIBMINIFI_INCLUDE_UTILS_PROPERTYERRORS_H_
diff --git a/libminifi/include/utils/ValueParser.h
b/libminifi/include/utils/ValueParser.h
new file mode 100644
index 0000000..dfca9bc
--- /dev/null
+++ b/libminifi/include/utils/ValueParser.h
@@ -0,0 +1,166 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenseas/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef LIBMINIFI_INCLUDE_UTILS_VALUEPARSER_H_
+#define LIBMINIFI_INCLUDE_UTILS_VALUEPARSER_H_
+
+#include <exception>
+#include <string>
+#include <cstring>
+#include <vector>
+#include <cstdlib>
+#include <type_traits>
+#include <limits>
+
+#include "PropertyErrors.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+namespace internal {
+
+class ValueParser {
+ public:
+ explicit ValueParser(const std::string& str, std::size_t offset = 0) :
str(str), offset(offset) {}
+
+ ValueParser& parse(int& out) { // NOLINT
+ long result; // NOLINT
+ auto len = safeCallConverter(std::strtol, result);
+ if (len == 0) {
+ throw ParseException("Couldn't parse int");
+ }
+ if (result < (std::numeric_limits<int>::min)() || result >
(std::numeric_limits<int>::max)()) {
+ throw ParseException("Cannot convert long to int");
+ }
+ offset += len;
+ out = result;
+ return *this;
+ }
+
+ ValueParser& parse(int64_t& out) {
+ long long result; // NOLINT
+ auto len = safeCallConverter(std::strtoll, result);
+ if (len == 0) {
+ throw ParseException("Couldn't parse long long");
+ }
+ if (result < (std::numeric_limits<int64_t>::min)() || result >
(std::numeric_limits<int64_t>::max)()) {
+ throw ParseException("Cannot convert long long to int64_t");
+ }
+ offset += len;
+ out = result;
+ return *this;
+ }
+
+ ValueParser& parse(uint32_t & out) {
+ skipWhitespace();
+ if (offset < str.length() && str[offset] == '-') {
+ throw ParseException("Not an unsigned long");
+ }
+ unsigned long result; // NOLINT
+ auto len = safeCallConverter(std::strtoul, result);
+ if (len == 0) {
+ throw ParseException("Couldn't parse uint32_t");
+ }
+ if (result > (std::numeric_limits<uint32_t>::max)()) {
+ throw ParseException("Cannot convert unsigned long to uint32_t");
+ }
+ offset += len;
+ out = result;
+ return *this;
+ }
+
+ ValueParser& parse(uint64_t& out) {
+ skipWhitespace();
+ if (offset < str.length() && str[offset] == '-') {
+ throw ParseException("Not an unsigned long");
+ }
+ unsigned long long result; // NOLINT
+ auto len = safeCallConverter(std::strtoull, result);
+ if (len == 0) {
+ throw ParseException("Couldn't parse unsigned long long");
+ }
+ if (result > (std::numeric_limits<uint64_t>::max)()) {
+ throw ParseException("Cannot convert unsigned long long to uint64_t");
+ }
+ offset += len;
+ out = result;
+ return *this;
+ }
+
+ ValueParser& parse(bool& out) {
+ skipWhitespace();
+ if (std::strncmp(str.c_str() + offset, "false", std::strlen("false")) ==
0) {
+ offset += std::strlen("false");
+ out = false;
+ } else if (std::strncmp(str.c_str() + offset, "true", std::strlen("true"))
== 0) {
+ offset += std::strlen("true");
+ out = true;
+ } else {
+ throw ParseException("Couldn't parse bool");
+ }
+ return *this;
+ }
+
+ void parseEnd() {
+ skipWhitespace();
+ if (offset < str.length()) {
+ throw ParseException("Expected to parse till the end");
+ }
+ }
+
+ private:
+ /**
+ *
+ * @tparam T
+ * @param converter
+ * @param out
+ * @return the number of characters used during conversion, 0 for error
+ */
+ template<typename T>
+ std::size_t safeCallConverter(T (*converter)(const char* begin, char** end,
int base), T& out) {
+ const char* const begin = str.c_str() + offset;
+ char* end;
+ errno = 0;
+ T result = converter(begin, &end, 10);
+ if (end == begin || errno == ERANGE) {
+ return 0;
+ }
+ out = result;
+ return end - begin;
+ }
+
+ void skipWhitespace() {
+ while (offset < str.length() && std::isspace(str[offset])) {
+ ++offset;
+ }
+ }
+
+ const std::string& str;
+ std::size_t offset;
+};
+
+} /* namespace internal */
+} /* namespace utils */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
+
+#endif // LIBMINIFI_INCLUDE_UTILS_VALUEPARSER_H_
diff --git a/libminifi/src/core/ConfigurableComponent.cpp
b/libminifi/src/core/ConfigurableComponent.cpp
index 0f29f05..20dbaa2 100644
--- a/libminifi/src/core/ConfigurableComponent.cpp
+++ b/libminifi/src/core/ConfigurableComponent.cpp
@@ -24,6 +24,7 @@
#include "core/ConfigurableComponent.h"
#include "core/logging/LoggerConfiguration.h"
+#include "utils/gsl.h"
namespace org {
namespace apache {
@@ -66,15 +67,16 @@ bool ConfigurableComponent::getProperty(const std::string
&name, Property &prop)
*/
bool ConfigurableComponent::setProperty(const std::string name, std::string
value) {
std::lock_guard<std::mutex> lock(configuration_mutex_);
- auto &&it = properties_.find(name);
+ auto it = properties_.find(name);
if (it != properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&]{
+ onPropertyModified(orig_property, new_property);
+ logger_->log_debug("Component %s property name %s value %s", name,
new_property.getName(), value);
+ });
new_property.setValue(value);
- properties_[new_property.getName()] = new_property;
- onPropertyModified(orig_property, new_property);
- logger_->log_debug("Component %s property name %s value %s", name,
new_property.getName(), value);
return true;
} else {
if (accept_all_properties_) {
@@ -102,11 +104,12 @@ bool ConfigurableComponent::updateProperty(const
std::string &name, const std::s
if (it != properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&] {
+ onPropertyModified(orig_property, new_property);
+ logger_->log_debug("Component %s property name %s value %s", name,
new_property.getName(), value);
+ });
new_property.addValue(value);
- properties_[new_property.getName()] = new_property;
- onPropertyModified(orig_property, new_property);
- logger_->log_debug("Component %s property name %s value %s", name,
new_property.getName(), value);
return true;
} else {
return false;
@@ -125,11 +128,12 @@ bool ConfigurableComponent::setProperty(Property &prop,
std::string value) {
if (it != properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&] {
+ onPropertyModified(orig_property, new_property);
+ logger_->log_debug("property name %s value %s and new value is %s",
prop.getName(), value, new_property.getValue().to_string());
+ });
new_property.setValue(value);
- properties_[new_property.getName()] = new_property;
- onPropertyModified(orig_property, new_property);
- logger_->log_debug("property name %s value %s and new value is %s",
prop.getName(), value, new_property.getValue().to_string());
return true;
} else {
if (accept_all_properties_) {
@@ -153,11 +157,12 @@ bool ConfigurableComponent::setProperty(Property &prop,
PropertyValue &value) {
if (it != properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&] {
+ onPropertyModified(orig_property, new_property);
+ logger_->log_debug("property name %s value %s and new value is %s",
prop.getName(), new_property.getName(), value,
new_property.getValue().to_string());
+ });
new_property.setValue(value);
- properties_[new_property.getName()] = new_property;
- onPropertyModified(orig_property, new_property);
- logger_->log_debug("property name %s value %s and new value is %s",
prop.getName(), new_property.getName(), value,
new_property.getValue().to_string());
return true;
} else {
if (accept_all_properties_) {
@@ -212,7 +217,16 @@ bool ConfigurableComponent::getDynamicProperty(const
std::string name, std::stri
auto &&it = dynamic_properties_.find(name);
if (it != dynamic_properties_.end()) {
- Property item = it->second;
+ const Property& item = it->second;
+ if (item.getValue().getValue() == nullptr) {
+ // empty property value
+ if (item.getRequired()) {
+ logger_->log_error("Component %s required dynamic property %s is
empty", name, item.getName());
+ throw std::runtime_error("Required dynamic property is empty: " +
item.getName());
+ }
+ logger_->log_debug("Component %s dynamic property name %s, empty value",
name, item.getName());
+ return false;
+ }
value = item.getValue().to_string();
logger_->log_debug("Component %s dynamic property name %s value %s", name,
item.getName(), value);
return true;
@@ -243,12 +257,13 @@ bool ConfigurableComponent::setDynamicProperty(const
std::string name, std::stri
if (it != dynamic_properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&] {
+ onDynamicPropertyModified(orig_property, new_property);
+ logger_->log_debug("Component %s dynamic property name %s value %s",
name, new_property.getName(), value);
+ });
new_property.setValue(value);
new_property.setSupportsExpressionLanguage(true);
- dynamic_properties_[new_property.getName()] = new_property;
- onDynamicPropertyModified(orig_property, new_property);
- logger_->log_debug("Component %s dynamic property name %s value %s", name,
new_property.getName(), value);
return true;
} else {
return createDynamicProperty(name, value);
@@ -261,12 +276,13 @@ bool ConfigurableComponent::updateDynamicProperty(const
std::string &name, const
if (it != dynamic_properties_.end()) {
Property orig_property = it->second;
- Property new_property = orig_property;
+ Property& new_property = it->second;
+ auto onExit = gsl::finally([&] {
+ onDynamicPropertyModified(orig_property, new_property);
+ logger_->log_debug("Component %s dynamic property name %s value %s",
name, new_property.getName(), value);
+ });
new_property.addValue(value);
new_property.setSupportsExpressionLanguage(true);
- dynamic_properties_[new_property.getName()] = new_property;
- onDynamicPropertyModified(orig_property, new_property);
- logger_->log_debug("Component %s dynamic property name %s value %s", name,
new_property.getName(), value);
return true;
} else {
return createDynamicProperty(name, value);
diff --git a/libminifi/src/core/PropertyValidation.cpp
b/libminifi/src/core/PropertyValidation.cpp
index 177a3c7..304aa39 100644
--- a/libminifi/src/core/PropertyValidation.cpp
+++ b/libminifi/src/core/PropertyValidation.cpp
@@ -23,20 +23,18 @@ namespace nifi {
namespace minifi {
namespace core {
-std::shared_ptr<PropertyValidator> StandardValidators::VALID =
std::make_shared<AlwaysValid>(true, "VALID");
+StandardValidators::StandardValidators()
+ : INVALID(std::make_shared<AlwaysValid>(false, "INVALID")),
+ INTEGER_VALIDATOR(std::make_shared<IntegerValidator>("INTEGER_VALIDATOR")),
+
UNSIGNED_INT_VALIDATOR(std::make_shared<UnsignedIntValidator>("NON_NEGATIVE_INTEGER_VALIDATOR")),
+ LONG_VALIDATOR(std::make_shared<LongValidator>("LONG_VALIDATOR")),
+ // name is used by java nifi validators, so we should keep this LONG and
not change to reflect
+ // its internal use
+
UNSIGNED_LONG_VALIDATOR(std::make_shared<UnsignedLongValidator>("LONG_VALIDATOR")),
+
DATA_SIZE_VALIDATOR(std::make_shared<DataSizeValidator>("DATA_SIZE_VALIDATOR")),
+
TIME_PERIOD_VALIDATOR(std::make_shared<TimePeriodValidator>("TIME_PERIOD_VALIDATOR")),
+ BOOLEAN_VALIDATOR(std::make_shared<BooleanValidator>("BOOLEAN_VALIDATOR"))
{}
-StandardValidators::StandardValidators() {
- INVALID = std::make_shared<AlwaysValid>(false, "INVALID");
- INTEGER_VALIDATOR = std::make_shared<IntegerValidator>("INTEGER_VALIDATOR");
- UNSIGNED_INT_VALIDATOR =
std::make_shared<UnsignedIntValidator>("NON_NEGATIVE_INTEGER_VALIDATOR");
- LONG_VALIDATOR = std::make_shared<LongValidator>("LONG_VALIDATOR");
- // name is used by java nifi validators, so we should keep this LONG and not
change to reflect
- // its internal use
- UNSIGNED_LONG_VALIDATOR =
std::make_shared<UnsignedLongValidator>("LONG_VALIDATOR");
- DATA_SIZE_VALIDATOR =
std::make_shared<DataSizeValidator>("DATA_SIZE_VALIDATOR");
- TIME_PERIOD_VALIDATOR =
std::make_shared<TimePeriodValidator>("TIME_PERIOD_VALIDATOR");
- BOOLEAN_VALIDATOR = std::make_shared<BooleanValidator>("BOOLEAN_VALIDATOR");
-}
} /* namespace core */
} /* namespace minifi */
} /* namespace nifi */
diff --git a/libminifi/test/archive-tests/MergeFileTests.cpp
b/libminifi/test/archive-tests/MergeFileTests.cpp
index a6e8315..e6920fb 100644
--- a/libminifi/test/archive-tests/MergeFileTests.cpp
+++ b/libminifi/test/archive-tests/MergeFileTests.cpp
@@ -224,7 +224,7 @@ TEST_CASE("MergeFileDefragment", "[mergefiletest1]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_CONCAT_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_DEFRAGMENT);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_TEXT);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_TEXT);
core::ProcessSession sessionGenFlowFile(context);
std::shared_ptr<core::FlowFile> record[6];
@@ -326,7 +326,7 @@ TEST_CASE("MergeFileDefragmentDelimiter",
"[mergefiletest2]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_CONCAT_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_DEFRAGMENT);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_FILENAME);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_FILENAME);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::Header,
HEADER_FILE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::Footer,
FOOTER_FILE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::Demarcator,
DEMARCATOR_FILE);
@@ -419,7 +419,7 @@ TEST_CASE("MergeFileDefragmentDropFlow",
"[mergefiletest3]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_CONCAT_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_DEFRAGMENT);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_TEXT);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_TEXT);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MaxBinAge,
"1 sec");
core::ProcessSession sessionGenFlowFile(context);
@@ -516,7 +516,7 @@ TEST_CASE("MergeFileBinPack", "[mergefiletest4]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_CONCAT_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_BIN_PACK);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_TEXT);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_TEXT);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MinSize,
"96");
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::CorrelationAttributeName,
"tag");
@@ -598,7 +598,7 @@ TEST_CASE("MergeFileTar", "[mergefiletest4]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_TAR_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_BIN_PACK);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_TEXT);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_TEXT);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MinSize,
"96");
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::CorrelationAttributeName,
"tag");
@@ -689,7 +689,7 @@ TEST_CASE("MergeFileZip", "[mergefiletest5]") {
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeFormat,
MERGE_FORMAT_ZIP_VALUE);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MergeStrategy,
MERGE_STRATEGY_BIN_PACK);
-
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStratgey,
DELIMITER_STRATEGY_TEXT);
+
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::DelimiterStrategy,
DELIMITER_STRATEGY_TEXT);
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::MinSize,
"96");
context->setProperty(org::apache::nifi::minifi::processors::MergeContent::CorrelationAttributeName,
"tag");
diff --git a/libminifi/test/unit/PropertyValidationTests.cpp
b/libminifi/test/unit/PropertyValidationTests.cpp
new file mode 100644
index 0000000..5a50164
--- /dev/null
+++ b/libminifi/test/unit/PropertyValidationTests.cpp
@@ -0,0 +1,238 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "../TestBase.h"
+#include "core/ConfigurableComponent.h"
+#include "utils/PropertyErrors.h"
+#include "core/PropertyValidation.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+using namespace utils::internal;
+/**
+ * This Tests checks a deprecated behavior that should be removed
+ * in the next major release.
+ */
+TEST_CASE("Some default values get coerced to typed variants") {
+ auto prop = Property("prop", "d", "true");
+ REQUIRE_THROWS_AS(prop.setValue("banana"), ConversionException&);
+
+ const std::string SPACE = " ";
+ auto prop2 = Property("prop", "d", SPACE + "true");
+ prop2.setValue("banana");
+}
+
+TEST_CASE("Converting invalid PropertyValue") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(0)
+ ->build();
+ REQUIRE_THROWS_AS(prop.setValue("not int"), ParseException&);
+ REQUIRE_THROWS_AS(static_cast<int>(prop.getValue()), InvalidValueException&);
+}
+
+TEST_CASE("Parsing int has baggage after") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(0)
+ ->build();
+ REQUIRE_THROWS_AS(prop.setValue("55almost int"), ParseException&);
+}
+
+TEST_CASE("Parsing int has spaces") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(0)
+ ->build();
+ prop.setValue(" 55 ");
+ REQUIRE(static_cast<int>(prop.getValue()) == 55);
+}
+
+TEST_CASE("Parsing int out of range") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(0)
+ ->build();
+ REQUIRE_THROWS_AS(prop.setValue(" 5000000000 "), ParseException&);
+}
+
+TEST_CASE("Parsing bool has baggage after") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<bool>(true)
+ ->build();
+ REQUIRE_THROWS_AS(prop.setValue("false almost bool"), ParseException&);
+}
+
+class TestConfigurableComponent : public ConfigurableComponent {
+ public:
+ bool supportsDynamicProperties() override {
+ return true;
+ }
+
+ bool canEdit() override {
+ return true;
+ }
+
+ void onPropertyModified(const Property &old_property, const Property
&new_property) override {
+ if (onPropertyModifiedCallback) onPropertyModifiedCallback(old_property,
new_property);
+ }
+
+ void onDynamicPropertyModified(const Property &old_property, const Property
&new_property) override {
+ if (onDynamicPropertyModifiedCallback)
onDynamicPropertyModifiedCallback(old_property, new_property);
+ }
+
+ template<typename Fn>
+ void setPropertyModifiedCallback(Fn &&functor) {
+ onPropertyModifiedCallback = std::forward<Fn>(functor);
+ }
+
+ template<typename Fn>
+ void setDynamicPropertyModifiedCallback(Fn &&functor) {
+ onDynamicPropertyModifiedCallback = std::forward<Fn>(functor);
+ }
+
+ private:
+ std::function<void(const Property &, const Property &)>
onPropertyModifiedCallback;
+ std::function<void(const Property &, const Property &)>
onDynamicPropertyModifiedCallback;
+};
+
+TEST_CASE("Missing Required With Default") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->isRequired(true)
+ ->withDefaultValue<std::string>("default")
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ std::string value;
+ REQUIRE(component.getProperty(prop.getName(), value));
+ REQUIRE(value == "default");
+}
+
+TEST_CASE("Missing Required Without Default") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->isRequired(true)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ std::string value;
+ REQUIRE_THROWS_AS(component.getProperty(prop.getName(), value),
RequiredPropertyMissingException&);
+}
+
+TEST_CASE("Missing Optional Without Default") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->isRequired(false)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ std::string value;
+ REQUIRE_FALSE(component.getProperty(prop.getName(), value));
+}
+
+TEST_CASE("Valid Optional Without Default") {
+ // without a default the value will be stored as a string
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->isRequired(false)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ component.setProperty(prop.getName(), "some data");
+ std::string value;
+ REQUIRE(component.getProperty(prop.getName(), value));
+ REQUIRE(value == "some data");
+}
+
+TEST_CASE("Invalid With Default") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<bool>(true)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ REQUIRE_THROWS_AS(component.setProperty("prop", "banana"), ParseException&);
+ std::string value;
+ REQUIRE_THROWS_AS(component.getProperty(prop.getName(), value),
InvalidValueException&);
+}
+
+TEST_CASE("Valid With Default") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(55)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ REQUIRE(component.setProperty("prop", "23"));
+ int value;
+ REQUIRE(component.getProperty(prop.getName(), value));
+ REQUIRE(value == 23);
+}
+
+TEST_CASE("Invalid conversion") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<std::string>("banana")
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ bool value;
+ REQUIRE_THROWS_AS(component.getProperty(prop.getName(), value),
ConversionException&);
+}
+
+TEST_CASE("Write Invalid Then Override With Valid") {
+ // we always base the assignment on the default value
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int>(55)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ REQUIRE_THROWS_AS(component.setProperty(prop.getName(), "banana"),
ConversionException&);
+ component.setProperty(prop.getName(), "98");
+ int value;
+ REQUIRE(component.getProperty(prop.getName(), value));
+ REQUIRE(value == 98);
+}
+
+TEST_CASE("Property Change notification gets called even on erroneous
assignment") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<bool>(true)
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ int callbackCount = 0;
+ component.setPropertyModifiedCallback([&](const Property &, const Property
&) {
+ ++callbackCount;
+ });
+ REQUIRE_THROWS_AS(component.setProperty(prop.getName(), "banana"),
ConversionException&);
+ REQUIRE(callbackCount == 1);
+}
+
+TEST_CASE("Correctly Typed Property With Invalid Validation") {
+ auto prop = PropertyBuilder::createProperty("prop")
+ ->withDefaultValue<int64_t>(5,
std::make_shared<LongValidator>("myValidator", 0, 10))
+ ->build();
+ TestConfigurableComponent component;
+ component.setSupportedProperties({prop});
+ int callbackCount = 0;
+ component.setPropertyModifiedCallback([&](const Property &, const Property
&) {
+ ++callbackCount;
+ });
+ REQUIRE_THROWS_AS(component.setProperty(prop.getName(), "20"),
InvalidValueException&);
+ REQUIRE(callbackCount == 1);
+}
+
+} /* namespace core */
+} /* namespace minifi */
+} /* namespace nifi */
+} /* namespace apache */
+} /* namespace org */
diff --git a/nanofi/include/blocks/comms.h b/nanofi/include/blocks/comms.h
index bfacc3d..818ef27 100644
--- a/nanofi/include/blocks/comms.h
+++ b/nanofi/include/blocks/comms.h
@@ -23,9 +23,11 @@
#include "../api/nanofi.h"
#include "core/processors.h"
-#define SUCCESS 0x00
-#define FINISHED_EARLY 0x01
-#define FAIL 0x02
+enum {
+ SUCCESS = 0x00,
+ FINISHED_EARLY = 0x01,
+ FAIL = 0x02
+};
typedef int transmission_stop(void *);
