You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "martinzink (via GitHub)" <gi...@apache.org> on 2026/07/24 16:04:30 UTC
[PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
martinzink opened a new pull request, #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3781992445
##########
minifi_rust/minifi_native/src/c_ffi/c_ffi_process_context.rs:
##########
@@ -65,9 +65,9 @@ unsafe extern "C" fn get_property_callback(
impl<'a> ProcessContext for CffiProcessContext<'a> {
type FlowFile = CffiFlowFile<'a>; // FlowFile shouldn't outlive the ProcessContext
- fn get_property(
+ fn get_raw_property<K: PropertySchema + ?Sized>(
Review Comment:
i dont think it stands for anything, but u are right maybe P for Property? [K -> P in Property context](https://github.com/apache/nifi-minifi-cpp/pull/2220/commits/644ccc8273fb29cca6b4490c89fe5a10b78b09b2)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3646721111
##########
minifi_rust/minifi_native/src/api/processor_wrappers/flow_file_transform.rs:
##########
@@ -67,6 +67,19 @@ impl<'a> TransformedFlowFile<'a> {
pub fn attributes_to_add(&self) -> &HashMap<String, String> {
&self.attributes_to_add
}
+
+ #[cfg(any(test, feature = "test-utils"))]
+ pub fn into_bytes(self) -> std::io::Result<Option<Vec<u8>>> {
+ match self.new_content {
+ Some(Content::Buffer(vec)) => Ok(Some(vec)),
+ Some(Content::Stream(mut stream)) => {
+ let mut buffer = Vec::new();
+ stream.read_to_end(&mut buffer)?;
+ Ok(Some(buffer))
Review Comment:
`read_to_end` is a `std::io::Read` trait method, but `Read` is not imported in this module. This will fail to compile when `cfg(test)`/`test-utils` enables `into_bytes`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3852391151
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<K2: ?Sized + PropertySchema>(&self) -> Property<K2> {
Review Comment:
Even after reading this a couple of times, looking at the code, and jumping around to gather context, I still find this really confusing. Could you write a comment somewhere in the code that explains the part from a Property declaration to retrieving a controller service, including jumping through types and traits? I imagine 2-3 paragraphs could be enough to explain this, and I believe it would help future readers too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3795183092
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<K2: ?Sized + PropertySchema>(&self) -> Property<K2> {
Review Comment:
this is used for the wrapped the controller services, with this we can change the type of the property (creating an alternative one) during get_controller_service calls with this so one could simply write
```
pub(crate) const DUMMY_CONTROLLER_SERVICE: Property<Option<DummyControllerService>> = Property::new(
"Dummy Controller Service",
"Optional dummy controller service",
);
```
and the output type can be an Option<&DummyControllerService> via the ControllerServiceValue PropertySchema
also this is how the separation is done so the controllerservices can only be queried via get_controller_service and not get_property
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3852428708
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,39 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ fn get_raw_controller_service<Cs, P>(
Review Comment:
What is the difference between a "raw" controller service and one that's not "raw"? I remember that with properties, you get the string value instead of the parsed one, but that doesn't make sense here. Could you add short comments above the respective functions, explaining what they do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3789973261
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/generate_flow_file/properties.rs:
##########
@@ -15,64 +15,34 @@
// specific language governing permissions and limitations
// under the License.
-use minifi_native::{Property, StandardPropertyValidator};
+use super::DataFormat;
+use minifi_native::{DataSize, Property};
-pub(crate) const FILE_SIZE: Property = Property {
- name: "File Size",
- description: "The size of the file that will be used",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: true,
- default_value: Some("1 kB"),
- validator: StandardPropertyValidator::DataSizeValidator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const FILE_SIZE: Property<DataSize> =
+ Property::new("File Size", "The size of the file that will be used")
+ .supports_expression_language()
+ .with_default("1 kB");
-pub(crate) const BATCH_SIZE: Property = Property {
- name: "Batch Size",
- description: "The number of FlowFiles to be transferred in each invocation",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: false,
- default_value: Some("1"),
- validator: StandardPropertyValidator::U64Validator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const BATCH_SIZE: Property<u64> = Property::new(
+ "Batch Size",
+ "The number of FlowFiles to be transferred in each invocation",
+)
+.with_default("1");
Review Comment:
tbh it's fine to leave it as is, but the formatting seems like a formatter bug to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3777291661
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/generate_flow_file/properties.rs:
##########
@@ -15,64 +15,34 @@
// specific language governing permissions and limitations
// under the License.
-use minifi_native::{Property, StandardPropertyValidator};
+use super::DataFormat;
+use minifi_native::{DataSize, Property};
-pub(crate) const FILE_SIZE: Property = Property {
- name: "File Size",
- description: "The size of the file that will be used",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: true,
- default_value: Some("1 kB"),
- validator: StandardPropertyValidator::DataSizeValidator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const FILE_SIZE: Property<DataSize> =
+ Property::new("File Size", "The size of the file that will be used")
+ .supports_expression_language()
+ .with_default("1 kB");
-pub(crate) const BATCH_SIZE: Property = Property {
- name: "Batch Size",
- description: "The number of FlowFiles to be transferred in each invocation",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: false,
- default_value: Some("1"),
- validator: StandardPropertyValidator::U64Validator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const BATCH_SIZE: Property<u64> = Property::new(
+ "Batch Size",
+ "The number of FlowFiles to be transferred in each invocation",
+)
+.with_default("1");
Review Comment:
it doesnt seem that way no, cargo fmt formatted this i think its based on this
> [Multi-line elements](https://doc.rust-lang.org/style-guide/expressions.html#multi-line-elements)
If any element in a chain is formatted across multiple lines, put that element and any later elements on their own lines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3844211899
##########
minifi_rust/minifi_native/src/api/processor_wrappers/flow_file_transform.rs:
##########
@@ -200,3 +213,42 @@ where
}
}
}
+
+#[macro_export]
+macro_rules! unwrap_or_route {
Review Comment:
Because this macro is exported, crate-owned names must use `$crate`. Its expansions currently reference invocation-site `TransformedFlowFile` and `error!` names (and one arm hard-codes `minifi_native`), so downstream callers that import only `unwrap_or_route!`—or rename the dependency—will fail to compile. Qualify all three arms with `$crate::TransformedFlowFile` and `$crate::error!`.
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/put_file/unix_permissions.rs:
##########
@@ -0,0 +1,62 @@
+use minifi_native::{MinifiError, PropertyConstraints, PropertySchema, PropertyType};
Review Comment:
This new source file is missing the Apache license header used by the surrounding Rust sources (for example, `put_file.rs:1-16` and `unix_only_properties.rs:1-16`). Add the standard header so the file satisfies the repository's licensing convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3785046576
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/generate_flow_file/properties.rs:
##########
@@ -15,64 +15,34 @@
// specific language governing permissions and limitations
// under the License.
-use minifi_native::{Property, StandardPropertyValidator};
+use super::DataFormat;
+use minifi_native::{DataSize, Property};
-pub(crate) const FILE_SIZE: Property = Property {
- name: "File Size",
- description: "The size of the file that will be used",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: true,
- default_value: Some("1 kB"),
- validator: StandardPropertyValidator::DataSizeValidator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const FILE_SIZE: Property<DataSize> =
+ Property::new("File Size", "The size of the file that will be used")
+ .supports_expression_language()
+ .with_default("1 kB");
-pub(crate) const BATCH_SIZE: Property = Property {
- name: "Batch Size",
- description: "The number of FlowFiles to be transferred in each invocation",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: false,
- default_value: Some("1"),
- validator: StandardPropertyValidator::U64Validator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const BATCH_SIZE: Property<u64> = Property::new(
+ "Batch Size",
+ "The number of FlowFiles to be transferred in each invocation",
+)
+.with_default("1");
Review Comment:
It's one thing to put it on its own line, and I have nothing against that. It's another thing how you indent it, and here a statement continuation doesn't get indented as a continuation. Maybe splitting the call within the parentheses to multiple lines caused the formatter to end up undoing the continuation indentation for further lines. I believe this is a problem with the formatter, but I didn't find any clear confirmation of that in the rules.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3717596196
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/generate_flow_file/properties.rs:
##########
@@ -15,64 +15,34 @@
// specific language governing permissions and limitations
// under the License.
-use minifi_native::{Property, StandardPropertyValidator};
+use super::DataFormat;
+use minifi_native::{DataSize, Property};
-pub(crate) const FILE_SIZE: Property = Property {
- name: "File Size",
- description: "The size of the file that will be used",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: true,
- default_value: Some("1 kB"),
- validator: StandardPropertyValidator::DataSizeValidator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const FILE_SIZE: Property<DataSize> =
+ Property::new("File Size", "The size of the file that will be used")
+ .supports_expression_language()
+ .with_default("1 kB");
-pub(crate) const BATCH_SIZE: Property = Property {
- name: "Batch Size",
- description: "The number of FlowFiles to be transferred in each invocation",
- is_required: true,
- is_sensitive: false,
- supports_expr_lang: false,
- default_value: Some("1"),
- validator: StandardPropertyValidator::U64Validator,
- allowed_values: &[],
- allowed_type: None,
-};
+pub(crate) const BATCH_SIZE: Property<u64> = Property::new(
+ "Batch Size",
+ "The number of FlowFiles to be transferred in each invocation",
+)
+.with_default("1");
Review Comment:
shouldn't this be indented 2 levels as a continuation?
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<K: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<K>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<K: ?Sized + PropertySchema> Property<K> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: K::IS_REQUIRED || self.default_value.is_some(),
Review Comment:
I'd say don't try to be smart here, and wire it through directly. That way if the C++ implementation changes behavior around edge cases (like #2241), the Rust implementation will automatically work right too.
##########
minifi_rust/minifi_native/src/c_ffi/c_ffi_process_context.rs:
##########
@@ -65,9 +65,9 @@ unsafe extern "C" fn get_property_callback(
impl<'a> ProcessContext for CffiProcessContext<'a> {
type FlowFile = CffiFlowFile<'a>; // FlowFile shouldn't outlive the ProcessContext
- fn get_property(
+ fn get_raw_property<K: PropertySchema + ?Sized>(
Review Comment:
What does K stand for? Key for something? Because I think that's supposed to be the type of the property value, not the property key (name).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3864121705
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ /// Returns the RawControllerService (ControllerService wrapper whose lifetime is managed by the agent)
+ fn get_raw_controller_service<RawCs, P>(
&self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
-
- fn get_duration_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
- }
- }
-
- fn get_size_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(byte_unit::Byte::from_str(&property_val)?.as_u64()))
- } else {
- Ok(None)
- }
- }
-
- fn get_u64_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != U64Validator {
- return Err(MinifiError::validation_err(format!(
- "to use get_u64_property {:?} must have U64Validator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(u64::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ property: &Property<P>,
+ ) -> Result<Option<&RawCs>, MinifiError>
+ where
+ RawCs: RawControllerService + ComponentIdentifier + 'static,
+ P: PropertySchema + ?Sized;
- fn get_raw_controller_service<Cs>(
+ /// Returns the enabled ControllerService (managed by RawControllerService)
+ fn get_controller_service<Cs>(
Review Comment:
I think the processor shouldn't need to care about the dynamic type of a controller service, only what type it expects to get, and if those match, then the conversion is a no-op / identity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink merged PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3646814035
##########
minifi_rust/minifi_native/src/api/errors.rs:
##########
@@ -89,6 +92,18 @@ impl From<NulError> for MinifiError {
}
}
+impl From<ParseFloatError> for MinifiError {
+ fn from(err: ParseFloatError) -> Self {
+ MinifiError::Parse(ParseError::Float(err))
+ }
+}
+
+impl From<std::convert::Infallible> for MinifiError {
+ fn from(_: std::convert::Infallible) -> Self {
+ unreachable!("Infallible errors can never happen")
+ }
+}
+
impl MinifiError {
pub(crate) fn to_status(&self) -> minifi_status {
match self {
Review Comment:
missing required attribute is not validation error
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3860800378
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,39 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ fn get_raw_controller_service<Cs, P>(
Review Comment:
raw means agent managed in rust context, so there are RawProcessors and RawControllerServices these are the objects are created and destroyed in minifi code (via callbacks) these wrap the user written Processors and ControllerServices (in options) so the user written Processors are created during schedule and user Written ControllerServices are created during enable.
The C api strictly oeprates with these raw types, but the extensions writer shouldnt need to know about these.
In property context, since the agent deals with strings only raw_property means the actual string set, and property is the parsed verified typed object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3863699154
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,303 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<P2: ?Sized + PropertySchema>(&self) -> Property<P2> {
+ Property {
+ name: self.name,
+ description: self.description,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ marker: PhantomData,
}
}
+}
+
+/// Trait required to register Property with the agent
+/// These values will be translated to fill out the
+/// validator, allowed_value, allowed_types, is_required on the agent side
+pub trait PropertySchema {
+ const CONSTRAINT: Option<PropertyConstraints>;
+ const IS_REQUIRED: bool;
+}
+
+/// The requiredness of the property is enforced via this Option impl
+/// If the property is required it should be registered as Property<T>
+/// If the property is not required it should be registered as Property<Option<T>
+impl<T: PropertySchema> PropertySchema for Option<T> {
+ const CONSTRAINT: Option<PropertyConstraints> = T::CONSTRAINT;
+ const IS_REQUIRED: bool = false;
+}
- fn get_size_property(&self, property: &Property) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
+/// Trait required to register property as Property<T> or Property<Option<T>
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2220/changes/288e205a9fef79af4737904a69f88f0110ee41b6
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,303 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<P2: ?Sized + PropertySchema>(&self) -> Property<P2> {
+ Property {
+ name: self.name,
+ description: self.description,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ marker: PhantomData,
}
}
+}
+
+/// Trait required to register Property with the agent
+/// These values will be translated to fill out the
+/// validator, allowed_value, allowed_types, is_required on the agent side
+pub trait PropertySchema {
+ const CONSTRAINT: Option<PropertyConstraints>;
+ const IS_REQUIRED: bool;
+}
+
+/// The requiredness of the property is enforced via this Option impl
+/// If the property is required it should be registered as Property<T>
+/// If the property is not required it should be registered as Property<Option<T>
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2220/changes/288e205a9fef79af4737904a69f88f0110ee41b6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3861562700
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ /// Returns the RawControllerService (ControllerService wrapper whose lifetime is managed by the agent)
+ fn get_raw_controller_service<RawCs, P>(
&self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
-
- fn get_duration_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
- }
- }
-
- fn get_size_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(byte_unit::Byte::from_str(&property_val)?.as_u64()))
- } else {
- Ok(None)
- }
- }
-
- fn get_u64_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != U64Validator {
- return Err(MinifiError::validation_err(format!(
- "to use get_u64_property {:?} must have U64Validator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(u64::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ property: &Property<P>,
+ ) -> Result<Option<&RawCs>, MinifiError>
+ where
+ RawCs: RawControllerService + ComponentIdentifier + 'static,
+ P: PropertySchema + ?Sized;
- fn get_raw_controller_service<Cs>(
+ /// Returns the enabled ControllerService (managed by RawControllerService)
+ fn get_controller_service<Cs>(
Review Comment:
Whose responsibility is it to cast the controller service to the interface type expected by the property and the extension? Because when a controller service uses multiple inheritance or encapsulation, the pointer value between the pointer to the interface and the pointer to the dynamic typed controller service object can differ. I see get_controller_service_api is supposed to handle this, but why is get_controller_service not the same as get_controller_service_api, doing the conversion from the potentially processor-unknown to the processor-known type?
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,303 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<P2: ?Sized + PropertySchema>(&self) -> Property<P2> {
+ Property {
+ name: self.name,
+ description: self.description,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ marker: PhantomData,
}
}
+}
+
+/// Trait required to register Property with the agent
+/// These values will be translated to fill out the
+/// validator, allowed_value, allowed_types, is_required on the agent side
+pub trait PropertySchema {
+ const CONSTRAINT: Option<PropertyConstraints>;
+ const IS_REQUIRED: bool;
+}
+
+/// The requiredness of the property is enforced via this Option impl
+/// If the property is required it should be registered as Property<T>
+/// If the property is not required it should be registered as Property<Option<T>
Review Comment:
```suggestion
/// If the property is not required it should be registered as Property<Option<T>>
```
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,303 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<P2: ?Sized + PropertySchema>(&self) -> Property<P2> {
+ Property {
+ name: self.name,
+ description: self.description,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ marker: PhantomData,
}
}
+}
+
+/// Trait required to register Property with the agent
+/// These values will be translated to fill out the
+/// validator, allowed_value, allowed_types, is_required on the agent side
+pub trait PropertySchema {
+ const CONSTRAINT: Option<PropertyConstraints>;
+ const IS_REQUIRED: bool;
+}
+
+/// The requiredness of the property is enforced via this Option impl
+/// If the property is required it should be registered as Property<T>
+/// If the property is not required it should be registered as Property<Option<T>
+impl<T: PropertySchema> PropertySchema for Option<T> {
+ const CONSTRAINT: Option<PropertyConstraints> = T::CONSTRAINT;
+ const IS_REQUIRED: bool = false;
+}
- fn get_size_property(&self, property: &Property) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
+/// Trait required to register property as Property<T> or Property<Option<T>
Review Comment:
```suggestion
/// Trait required to register property as Property<T> or Property<Option<T>>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3785355853
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<K2: ?Sized + PropertySchema>(&self) -> Property<K2> {
Review Comment:
What does this do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3646721152
##########
minifi_rust/minifi_native/src/api/errors.rs:
##########
@@ -89,6 +92,18 @@ impl From<NulError> for MinifiError {
}
}
+impl From<ParseFloatError> for MinifiError {
+ fn from(err: ParseFloatError) -> Self {
+ MinifiError::Parse(ParseError::Float(err))
+ }
+}
+
+impl From<std::convert::Infallible> for MinifiError {
+ fn from(_: std::convert::Infallible) -> Self {
+ unreachable!("Infallible errors can never happen")
+ }
+}
+
impl MinifiError {
pub(crate) fn to_status(&self) -> minifi_status {
match self {
Review Comment:
`MissingRequiredAttribute` is now part of `MinifiError`, but `to_status()` currently maps it via the catch-all to UNKNOWN. It should be mapped explicitly (e.g., to VALIDATION_FAILED) so C-FFI callers get a meaningful status code.
##########
minifi_rust/minifi_native/src/api/processor_wrappers/flow_file_transform.rs:
##########
@@ -200,3 +213,42 @@ where
}
}
}
+
+#[macro_export]
+macro_rules! unwrap_or_route {
+ ($result:expr, $route:expr) => {
+ match $result {
+ Ok(v) => v,
+ Err(e) => {
+ return Ok(TransformedFlowFile::route_without_changes($route));
+ }
+ }
Review Comment:
In this macro arm, the `Err(e)` binding is unused (warning) and `TransformedFlowFile` is unqualified, so callers must have it in scope. Prefer `Err(_)` and `$crate::TransformedFlowFile` for macro hygiene.
This issue also appears in the following locations of the same file:
- line 232
- line 237
- line 246
- line 250
##########
minifi_rust/minifi_native/src/c_ffi/c_ffi_property.rs:
##########
@@ -76,21 +77,24 @@ impl Property {
fn create_c_allowed_values_vec_vec(properties: &[Self]) -> Vec<Vec<minifi_string_view>> {
properties
.iter()
- .map(|p| {
- p.allowed_values
+ .map(|p| match p.constraints {
+ PropertyConstraints::AllowedValues(allowed_values) => allowed_values
Review Comment:
This `match p.constraints` moves `constraints` out of `&Property` (not `Copy`), which will not compile. Match on a reference instead.
This issue also appears on line 94 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3862227570
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ /// Returns the RawControllerService (ControllerService wrapper whose lifetime is managed by the agent)
+ fn get_raw_controller_service<RawCs, P>(
&self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
-
- fn get_duration_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
- }
- }
-
- fn get_size_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(byte_unit::Byte::from_str(&property_val)?.as_u64()))
- } else {
- Ok(None)
- }
- }
-
- fn get_u64_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != U64Validator {
- return Err(MinifiError::validation_err(format!(
- "to use get_u64_property {:?} must have U64Validator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(u64::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ property: &Property<P>,
+ ) -> Result<Option<&RawCs>, MinifiError>
+ where
+ RawCs: RawControllerService + ComponentIdentifier + 'static,
+ P: PropertySchema + ?Sized;
- fn get_raw_controller_service<Cs>(
+ /// Returns the enabled ControllerService (managed by RawControllerService)
+ fn get_controller_service<Cs>(
Review Comment:
this one uses exact types, the get_controller_service_api is the one that casts to dyn Trait so not used in producetion yet, usually the caller knows the exact ControllerService type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3646800376
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -44,69 +58,100 @@ pub struct Property {
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: PropertyConstraints,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+pub trait PropertyType {
+ type Output;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints = PropertyConstraints::NoConstraints;
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ fn parse(s: &str) -> Result<Self::Output, MinifiError>;
+}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
+macro_rules! impl_from_str_property {
+ ($t:ty, $validator:expr) => {
+ impl PropertyType for $t {
+ type Output = $t;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints = $validator;
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ s.parse::<$t>().map_err(Into::into)
+ }
}
+ };
+ ($t:ty) => {
+ impl_from_str_property!($t, PropertyConstraints::NoConstraints);
+ };
+}
+
+impl_from_str_property!(String);
+impl_from_str_property!(std::path::PathBuf);
+impl_from_str_property!(f64);
+impl_from_str_property!(f32);
+impl_from_str_property!(i64);
+impl_from_str_property!(i32);
+impl_from_str_property!(bool, PropertyConstraints::Validator(BoolValidator));
+impl_from_str_property!(u64, PropertyConstraints::Validator(U64Validator));
+impl_from_str_property!(u32, PropertyConstraints::Validator(U64Validator));
+impl_from_str_property!(usize, PropertyConstraints::Validator(U64Validator));
+
+impl PropertyType for Duration {
+ type Output = Duration;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints =
+ PropertyConstraints::Validator(TimePeriodValidator);
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ humantime::parse_duration(s).map_err(Into::into)
+ }
+}
+
+pub struct DataSize;
+impl PropertyType for DataSize {
+ type Output = u64;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints =
+ PropertyConstraints::Validator(DataSizeValidator);
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ byte_unit::Byte::from_str(s)
+ .map(|b| b.as_u64())
+ .map_err(Into::into)
}
+}
+
+pub trait GetProperty {
+ fn get_raw_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_size_property(&self, property: &Property) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
+ fn get_property<T: PropertyType>(
+ &self,
+ property: &Property,
+ ) -> Result<Option<T::Output>, MinifiError> {
+ if T::EXPECTED_CONSTRAINTS != NoConstraints
+ && T::EXPECTED_CONSTRAINTS != property.constraints
+ {
Review Comment:
it doesnt move out and it does compile, the property is already reference
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3781987405
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<K: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<K>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<K: ?Sized + PropertySchema> Property<K> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: K::IS_REQUIRED || self.default_value.is_some(),
Review Comment:
👍 good idea, [is_required: K::IS_REQUIRED](https://github.com/apache/nifi-minifi-cpp/pull/2220/commits/d4d650cdbf041bd5054957ddac1d17feb328ea22)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3674116241
##########
minifi_rust/extensions/minifi_rs_playground/src/controller_services/dog_controller_service.rs:
##########
@@ -32,9 +32,7 @@ pub(crate) const HAS_JETPACK: Property = Property {
is_sensitive: false,
supports_expr_lang: false,
default_value: Some("false"),
- validator: StandardPropertyValidator::BoolValidator,
- allowed_values: &[],
- allowed_type: None,
+ constraints: property_constraint::<bool>(),
Review Comment:
After our private discussion, I'll wait to see if you can refactor this to move the type to a Property<T> type parameter, but the current version is an improvement too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3646721062
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -44,69 +58,100 @@ pub struct Property {
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: PropertyConstraints,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+pub trait PropertyType {
+ type Output;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints = PropertyConstraints::NoConstraints;
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ fn parse(s: &str) -> Result<Self::Output, MinifiError>;
+}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
+macro_rules! impl_from_str_property {
+ ($t:ty, $validator:expr) => {
+ impl PropertyType for $t {
+ type Output = $t;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints = $validator;
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ s.parse::<$t>().map_err(Into::into)
+ }
}
+ };
+ ($t:ty) => {
+ impl_from_str_property!($t, PropertyConstraints::NoConstraints);
+ };
+}
+
+impl_from_str_property!(String);
+impl_from_str_property!(std::path::PathBuf);
+impl_from_str_property!(f64);
+impl_from_str_property!(f32);
+impl_from_str_property!(i64);
+impl_from_str_property!(i32);
+impl_from_str_property!(bool, PropertyConstraints::Validator(BoolValidator));
+impl_from_str_property!(u64, PropertyConstraints::Validator(U64Validator));
+impl_from_str_property!(u32, PropertyConstraints::Validator(U64Validator));
+impl_from_str_property!(usize, PropertyConstraints::Validator(U64Validator));
+
+impl PropertyType for Duration {
+ type Output = Duration;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints =
+ PropertyConstraints::Validator(TimePeriodValidator);
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ humantime::parse_duration(s).map_err(Into::into)
+ }
+}
+
+pub struct DataSize;
+impl PropertyType for DataSize {
+ type Output = u64;
+ const EXPECTED_CONSTRAINTS: PropertyConstraints =
+ PropertyConstraints::Validator(DataSizeValidator);
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ byte_unit::Byte::from_str(s)
+ .map(|b| b.as_u64())
+ .map_err(Into::into)
}
+}
+
+pub trait GetProperty {
+ fn get_raw_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_size_property(&self, property: &Property) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
+ fn get_property<T: PropertyType>(
+ &self,
+ property: &Property,
+ ) -> Result<Option<T::Output>, MinifiError> {
+ if T::EXPECTED_CONSTRAINTS != NoConstraints
+ && T::EXPECTED_CONSTRAINTS != property.constraints
+ {
Review Comment:
`property.constraints` is accessed by value from a borrowed `&Property`, which will not compile because it attempts to move a non-`Copy` field out of a shared reference. Compare constraints by reference (or make `PropertyConstraints` `Copy`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3743266803
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<K: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<K>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<K: ?Sized + PropertySchema> Property<K> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: K::IS_REQUIRED || self.default_value.is_some(),
Review Comment:
I know the existing C++ code is inconsistent, but if the property has a default value, then I would say it should *not* be required, meaning that Minifi should accept flow definitions in which the property is not specified (and return the default value from `get_property`).
```suggestion
is_required: K::IS_REQUIRED && self.default_value.is_none(),
```
Also, if `IS_REQUIRED` is not always the same as `is_required`, they should have different names. I would negate `IS_REQUIRED` and rename it to `IS_OPTIONAL`.
##########
minifi_rust/extensions/minifi_rs_playground/features/controller_apis.feature:
##########
@@ -23,6 +23,6 @@ Feature: Testing controller service api casting
And the "Extra information" property of the Wolfie the magical controller service is set to "The dog (Canis familiaris or Canis lupus familiaris) is a domesticated descendant of wolves."
When the MiNiFi instance starts up
- Then the Minifi logs contain the following message: "[minifi_rs_playground::processors::zoo_processor::ZooProcessorRs] [critical] Can DogControllerRs { has_jetpack: true, extra_info: "The dog (Canis familiaris or Canis lupus familiaris) is a domesticated descendant of wolves." } fly? true" in less than 10 seconds
+ Then the Minifi logs contain the following message: "[minifi_rs_playground::processors::zoo_processor::ZooProcessorRs] [critical] Can DogControllerRs { has_jetpack: true, extra_info: Some("The dog (Canis familiaris or Canis lupus familiaris) is a domesticated descendant of wolves.") } fly?" in less than 10 seconds
Review Comment:
the ` true` at the end should not be removed, I think
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3795183092
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<K2: ?Sized + PropertySchema>(&self) -> Property<K2> {
Review Comment:
this is used for the wrapped the controller services, with this we can change the type of the property (creating an alternative one) during get_controller_service calls with this so one could simply write
```
pub(crate) const DUMMY_CONTROLLER_SERVICE: Property<Option<DummyControllerService>> = Property::new(
"Dummy Controller Service",
"Optional dummy controller service",
);
```
and the output type can be an Option<&DummyControllerService> via the ControllerServiceValue PropertySchema
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3871520860
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -16,102 +16,291 @@
// under the License.
use crate::StandardPropertyValidator::{
- BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+ BoolValidator, DataSizeValidator, NonBlankValidator, TimePeriodValidator, U64Validator,
};
use crate::{
ComponentIdentifier, ControllerServiceDefinition, EnableControllerService, MinifiError,
};
+use minifi_native::StandardPropertyValidator::{F64Validator, I64Validator};
+use std::marker::PhantomData;
use std::str::FromStr;
use std::time::Duration;
#[derive(Debug, Eq, PartialEq)]
pub enum StandardPropertyValidator {
- AlwaysValidValidator,
NonBlankValidator,
TimePeriodValidator,
BoolValidator,
I64Validator,
U64Validator,
DataSizeValidator,
PortValidator,
+ F64Validator,
}
-#[derive(Debug)]
-pub struct Property {
+#[derive(Debug, PartialEq)]
+pub enum PropertyConstraints {
+ Validator(StandardPropertyValidator),
+ AllowedValues(&'static [&'static str]),
+ ControllerService(&'static str),
+}
+
+pub struct PropertyDefinition {
pub name: &'static str,
pub description: &'static str,
pub is_required: bool,
pub is_sensitive: bool,
pub supports_expr_lang: bool,
pub default_value: Option<&'static str>,
- pub validator: StandardPropertyValidator,
- pub allowed_values: &'static [&'static str],
- pub allowed_type: Option<&'static str>,
+ pub constraints: Option<PropertyConstraints>,
}
-pub trait GetProperty {
- fn get_property(&self, property: &Property) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(&self, property: &Property) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
+#[macro_export]
+macro_rules! property_definitions {
+ ($($property:expr),* $(,)?) => {
+ &[$($property.definition()),*]
+ };
+}
+
+pub struct Property<P: ?Sized + PropertySchema> {
+ pub(crate) name: &'static str,
+ pub(crate) description: &'static str,
+ pub(crate) is_sensitive: bool,
+ pub(crate) supports_expr_lang: bool,
+ pub(crate) default_value: Option<&'static str>,
+ pub(crate) marker: PhantomData<P>,
+}
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
+impl<P: ?Sized + PropertySchema> Property<P> {
+ pub const fn new(name: &'static str, description: &'static str) -> Self {
+ Property {
+ name,
+ description,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ marker: PhantomData,
}
}
- fn get_duration_property(&self, property: &Property) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
+ pub const fn sensitive(mut self) -> Self {
+ self.is_sensitive = true;
+ self
+ }
+
+ pub const fn supports_expression_language(mut self) -> Self {
+ self.supports_expr_lang = true;
+ self
+ }
+
+ pub const fn with_default(mut self, default_value: &'static str) -> Self {
+ self.default_value = Some(default_value);
+ self
+ }
+
+ pub const fn name(&self) -> &'static str {
+ self.name
+ }
+
+ pub const fn definition(&self) -> PropertyDefinition {
+ PropertyDefinition {
+ name: self.name,
+ description: self.description,
+ is_required: P::IS_REQUIRED,
+ is_sensitive: self.is_sensitive,
+ supports_expr_lang: self.supports_expr_lang,
+ default_value: self.default_value,
+ constraints: P::CONSTRAINT,
}
+ }
- if let Some(property_val) = self.get_property(property)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
+ pub(crate) const fn with_marker<K2: ?Sized + PropertySchema>(&self) -> Property<K2> {
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2220/changes/f71f18fcc02dfaa9600f1a1a9c86f0565e049911#r3871519483
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2869 Improve rust api property validation and property getters [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2220#discussion_r3871519483
##########
minifi_rust/minifi_native/src/api/process_context.rs:
##########
@@ -15,115 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-use crate::StandardPropertyValidator::*;
use crate::api::RawControllerService;
use crate::api::component_definition_traits::ComponentIdentifier;
use crate::api::flow_file::FlowFile;
-use crate::api::property::GetControllerService;
-use crate::{
- ControllerServiceApi, ControllerServiceDefinition, EnableControllerService, GetProperty,
- MinifiError, Property,
-};
-use std::str::FromStr;
-use std::time::Duration;
+use crate::api::property::{ControllerServiceValue, GetControllerService, PropertySchema};
+use crate::{ControllerServiceApi, EnableControllerService, GetProperty, MinifiError, Property};
pub trait ProcessContext {
type FlowFile: FlowFile;
- fn get_property(
+ fn get_raw_property<P: PropertySchema + ?Sized>(
&self,
- property: &Property,
+ property: &Property<P>,
flow_file: Option<&Self::FlowFile>,
) -> Result<Option<String>, MinifiError>;
- fn get_bool_property(
+ /// Returns the RawControllerService (ControllerService wrapper whose lifetime is managed by the agent)
+ fn get_raw_controller_service<RawCs, P>(
&self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<bool>, MinifiError> {
- if property.validator != BoolValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_bool_property {:?} must have BoolValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(bool::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
-
- fn get_duration_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<Duration>, MinifiError> {
- if property.validator != TimePeriodValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_duration_property {:?} must have TimePeriodValidator",
- property
- )));
- }
-
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(humantime::parse_duration(property_val.as_str())?))
- } else {
- Ok(None)
- }
- }
-
- fn get_size_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != DataSizeValidator {
- return Err(MinifiError::validation_err(format!(
- "to use get_size_property {:?} must have DataSizeValidator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(byte_unit::Byte::from_str(&property_val)?.as_u64()))
- } else {
- Ok(None)
- }
- }
-
- fn get_u64_property(
- &self,
- property: &Property,
- flow_file: Option<&Self::FlowFile>,
- ) -> Result<Option<u64>, MinifiError> {
- if property.validator != U64Validator {
- return Err(MinifiError::validation_err(format!(
- "to use get_u64_property {:?} must have U64Validator",
- property
- )));
- }
- if let Some(property_val) = self.get_property(property, flow_file)? {
- Ok(Some(u64::from_str(&property_val)?))
- } else {
- Ok(None)
- }
- }
+ property: &Property<P>,
+ ) -> Result<Option<&RawCs>, MinifiError>
+ where
+ RawCs: RawControllerService + ComponentIdentifier + 'static,
+ P: PropertySchema + ?Sized;
- fn get_raw_controller_service<Cs>(
+ /// Returns the enabled ControllerService (managed by RawControllerService)
+ fn get_controller_service<Cs>(
Review Comment:
yeah but the return type of these differ enough so its not trivial to merge them, one returns a simple Option<&T> the other Option<Box<&Trait>>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org