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

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

Reply via email to