szaszm commented on code in PR #1589:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1589#discussion_r1263795580
##########
libminifi/include/core/ProcessContext.h:
##########
@@ -102,18 +103,44 @@ class ProcessContext : public
controller::ControllerServiceLookup, public core::
return value;
}
+ template<typename T = std::string>
+ std::enable_if_t<std::is_default_constructible<T>::value, std::optional<T>>
+ getProperty(const PropertyReference& property) const {
+ T value;
+ try {
+ if (!getProperty(property.name, value)) return std::nullopt;
+ } catch (const utils::internal::ValueException&) {
+ return std::nullopt;
+ }
+ return value;
+ }
+
+ template<typename T>
+ requires(!std::is_convertible_v<T&, const FlowFile&> &&
!std::is_convertible_v<T&, const std::shared_ptr<FlowFile>&>)
+ bool getProperty(std::string_view name, T &value) const {
Review Comment:
```suggestion
template<typename T>
concept NotAFlowFile = !std::convertible_to<T&, const FlowFile&> &&
!std::convertible_to<T&, const std::shared_ptr<FlowFile>&>;
bool getProperty(std::string_view name, NotAFlowFile auto& value) const {
```
##########
libminifi/include/core/ProcessContext.h:
##########
@@ -102,18 +103,44 @@ class ProcessContext : public
controller::ControllerServiceLookup, public core::
return value;
}
+ template<typename T = std::string>
+ std::enable_if_t<std::is_default_constructible<T>::value, std::optional<T>>
+ getProperty(const PropertyReference& property) const {
Review Comment:
Let's convert this to concept constraints, since we're touching it, and the
new one below is also written like this.
```suggestion
template<std::default_initializable T = std::string>
std::optional<T> getProperty(const PropertyReference& property) const {
```
##########
libminifi/include/core/ProcessContext.h:
##########
@@ -102,18 +103,44 @@ class ProcessContext : public
controller::ControllerServiceLookup, public core::
return value;
}
+ template<typename T = std::string>
+ std::enable_if_t<std::is_default_constructible<T>::value, std::optional<T>>
+ getProperty(const PropertyReference& property) const {
+ T value;
+ try {
+ if (!getProperty(property.name, value)) return std::nullopt;
+ } catch (const utils::internal::ValueException&) {
+ return std::nullopt;
+ }
+ return value;
+ }
+
+ template<typename T>
+ requires(!std::is_convertible_v<T&, const FlowFile&> &&
!std::is_convertible_v<T&, const std::shared_ptr<FlowFile>&>)
+ bool getProperty(std::string_view name, T &value) const {
+ return getPropertyImp<typename
std::common_type<T>::type>(std::string{name}, value);
+ }
+
template<typename T>
- std::enable_if_t<!std::is_convertible_v<T&, const FlowFile&> &&
!std::is_convertible_v<T&, const std::shared_ptr<FlowFile>&>,
- bool> getProperty(const std::string &name, T &value) const {
- return getPropertyImp<typename std::common_type<T>::type>(name, value);
+ requires(!std::is_convertible_v<T&, const FlowFile&> &&
!std::is_convertible_v<T&, const std::shared_ptr<FlowFile>&>)
+ bool getProperty(const PropertyReference& property, T &value) const {
Review Comment:
```suggestion
bool getProperty(const PropertyReference& property, NotAFlowFile auto&
value) const {
```
##########
libminifi/src/core/Property.cpp:
##########
@@ -90,4 +94,41 @@ std::vector<std::pair<std::string, std::string>>
Property::getExclusiveOfPropert
return exclusive_of_properties_;
}
+namespace {
+std::vector<PropertyValue> createPropertyValues(gsl::span<const
std::string_view> values, const core::PropertyParser& property_parser) {
+ return ranges::views::transform(values, [&property_parser](const auto&
value) {
+ return property_parser.parse(value);
+ }) | ranges::to<std::vector>;
+}
+
+inline std::vector<std::string> createStrings(gsl::span<const
std::string_view> string_views) {
+ return ranges::views::transform(string_views, [](const auto& string_view) {
return std::string{string_view}; })
+ | ranges::to<std::vector>;
+}
+
+inline std::vector<std::pair<std::string, std::string>>
createStrings(gsl::span<const std::pair<std::string_view, std::string_view>>
pairs_of_string_views) {
+ return ranges::views::transform(pairs_of_string_views, [](const auto&
pair_of_string_views) { return std::pair<std::string,
std::string>(pair_of_string_views); })
+ | ranges::to<std::vector>;
+}
+} // namespace
+
+Property::Property(const PropertyReference& compile_time_property)
+ : name_(compile_time_property.name),
+ description_(compile_time_property.description),
+ is_required_(compile_time_property.is_required),
+ valid_regex_(compile_time_property.valid_regex),
Review Comment:
Is this `valid_regex_` ever used? The builder doesn't seem to set it. If
it's always empty, we might as well drop it.
##########
libminifi/include/core/PropertyType.h:
##########
@@ -90,12 +47,19 @@ class PropertyValidator {
[[nodiscard]] virtual ValidationResult validate(const std::string &subject,
const std::shared_ptr<minifi::state::response::Value> &input) const = 0;
[[nodiscard]] virtual ValidationResult validate(const std::string &subject,
const std::string &input) const = 0;
+};
+
+class PropertyType : public PropertyParser, public PropertyValidator {
Review Comment:
I think composition would be better design here, than multiple inheritance.
Most property types have names changed to end with "_TYPE", except
`UnsignedIntPropertyType`. This particular case may be an oversight.
But the pattern ultimately overrides the `getName` method of
`PropertyValidator`, so it should retuirn a validator name (like the old
version), but a method called `getName` in `PropertyType` has no business
returning a validator name. I'm also not sure how the type name gets converted
back to a validator name when used with a validator-aware C2 server.
##########
libminifi/test/unit/CoreTests.cpp:
##########
@@ -0,0 +1,49 @@
+/**
+ * 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 "../Catch.h"
+#include "core/Core.h"
+
+namespace org::apache::nifi::minifi::test {
+
+struct DummyStruct {};
+class DummyClass {};
+template<typename T> struct DummyStructTemplate {};
+template<typename T> class DummyClassTemplate {};
+
+TEST_CASE("getClassName() works correctly") {
+ CHECK(core::getClassName<DummyStruct>() ==
"org::apache::nifi::minifi::test::DummyStruct");
+ CHECK(core::getClassName<DummyClass>() ==
"org::apache::nifi::minifi::test::DummyClass");
+ CHECK(core::getClassName<DummyStructTemplate<int>>() ==
"org::apache::nifi::minifi::test::DummyStructTemplate<int>");
+ CHECK(core::getClassName<DummyClassTemplate<int>>() ==
"org::apache::nifi::minifi::test::DummyClassTemplate<int>");
+}
+
+TEST_CASE("className() works correctly and is constexpr") {
+ static constexpr auto struct_name = core::className<DummyStruct>();
+ static_assert(struct_name == "org::apache::nifi::minifi::test::DummyStruct");
Review Comment:
Can we remove `getClassName` and replace its usages with `className`? It
seems like it's doing the same thing, except that `className` can also work at
compile-time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
