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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to