szaszm commented on code in PR #2186:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2186#discussion_r3613772003
##########
minifi_rust/extensions/minifi_rs_playground/src/controller_services/dog_controller_service.rs:
##########
@@ -0,0 +1,79 @@
+use crate::controller_services::animal_controller_apis::{
+ CanFlyControllerApi, NumberOfLegsControllerApi,
+};
+use minifi_native::ControllerServiceApi;
+use minifi_native::macros::ComponentIdentifier;
+use minifi_native::{
+ ControllerServiceDefinition, EnableControllerService, GetProperty, Logger,
MinifiError,
+ Property, ProvidedInterface, StandardPropertyValidator,
create_provided_interface,
+};
+
+pub(crate) const HAS_JETPACK: Property = Property {
+ name: "Has Jetpack",
+ description: "Whether or not the dog has a jetpack",
+ is_required: true,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: Some("false"),
+ validator: StandardPropertyValidator::BoolValidator,
+ allowed_values: &[],
+ allowed_type: None,
+};
+
+pub(crate) const EXTRA_INFO: Property = Property {
+ name: "Extra information",
+ description: "We need this to verify the casting was done correctly",
+ is_required: false,
+ is_sensitive: false,
+ supports_expr_lang: false,
+ default_value: None,
+ validator: StandardPropertyValidator::AlwaysValidValidator,
+ allowed_values: &[],
+ allowed_type: None,
+};
+
+#[allow(dead_code)] // extra_info is only used by {:?}
+#[derive(Debug, ComponentIdentifier)]
+pub(crate) struct DogControllerRs {
+ has_jetpack: bool,
+ extra_info: String,
+}
+
+impl NumberOfLegsControllerApi for DogControllerRs {
+ fn number_of_legs(&self) -> u8 {
+ 4
+ }
+}
+
+impl CanFlyControllerApi for DogControllerRs {
+ fn can_fly(&self) -> bool {
+ self.has_jetpack
+ }
+}
+
+impl EnableControllerService for DogControllerRs {
+ fn enable<Ctx: GetProperty, L: Logger>(context: &Ctx, _logger: &L) ->
Result<Self, MinifiError>
+ where
+ Self: Sized,
+ {
+ let has_jetpack = context.get_bool_property(&HAS_JETPACK)?.ok_or(
+ MinifiError::missing_required_property("Has jetpack is required"),
+ )?;
+
+ let extra_info =
context.get_property(&EXTRA_INFO)?.unwrap_or("".into());
+
+ Ok(Self {
+ has_jetpack,
+ extra_info,
+ })
+ }
+}
+
+impl ControllerServiceDefinition for DogControllerRs {
+ const DESCRIPTION: &'static str = "Test DogControllerRs";
+ const PROPERTIES: &'static [Property] = &[HAS_JETPACK, EXTRA_INFO];
+ const PROVIDED_APIS: &'static [ProvidedInterface<Self>] = &[
+ create_provided_interface!(dyn CanFlyControllerApi),
+ create_provided_interface!(dyn NumberOfLegsControllerApi),
+ ];
Review Comment:
is there a reason to add the `dyn` prefix here instead of within the macro
at the right places?
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/generate_flow_file/tests.rs:
##########
@@ -0,0 +1,155 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use super::*;
+use crate::processors::generate_flow_file::properties::{
+ BATCH_SIZE, CUSTOM_TEXT, DATA_FORMAT, UNIQUE_FLOW_FILES,
+};
+use minifi_native::{MockLogger, MockProcessContext, MockProcessSession};
+
+#[test]
+fn schedule_succeeds_with_default_values() {
+ assert!(GenerateFlowFileRs::schedule(&MockProcessContext::new(),
&MockLogger::new()).is_ok());
+}
+
+#[test]
+fn generate_flow_file_empty_test() {
+ let logger = MockLogger::new();
+ let mut context = MockProcessContext::new();
+ context
+ .properties
+ .insert(properties::FILE_SIZE.name.to_string(), "0".to_string());
+ context
+ .properties
+ .insert(UNIQUE_FLOW_FILES.name.to_string(), "false".to_string());
+ context
+ .properties
+ .insert(DATA_FORMAT.name.to_string(), "Text".to_string());
+
+ let processor = GenerateFlowFileRs::schedule(&context, &logger).unwrap();
+ let mut session = MockProcessSession::new();
+ assert_eq!(
+ processor
+ .trigger(&mut context, &mut session, &logger)
+ .unwrap(),
+ OnTriggerResult::Ok
+ );
+ let result_flow_files = session.transferred_flow_files.borrow();
+ assert_eq!(result_flow_files.len(), 1);
+ assert_eq!(result_flow_files[0].flow_file.content_len(), 0);
+}
+
+#[test]
+fn generate_custom_text() {
+ let mut context = MockProcessContext::new();
+ context
+ .properties
+ .insert(properties::FILE_SIZE.name.to_string(), "0".to_string());
+ context
+ .properties
+ .insert(UNIQUE_FLOW_FILES.name.to_string(), "false".to_string());
+ context
+ .properties
+ .insert(DATA_FORMAT.name.to_string(), "Text".to_string());
+ context
+ .properties
+ .insert(CUSTOM_TEXT.name.to_string(), "foo bar baz".to_string());
+
+ let logger = MockLogger::new();
+ let processor = GenerateFlowFileRs::schedule(&context, &logger).unwrap();
+
+ let mut session = MockProcessSession::new();
+ assert_eq!(
+ processor
+ .trigger(&mut context, &mut session, &logger)
+ .expect("Should trigger successfully"),
+ OnTriggerResult::Ok
+ );
+ let result_flow_files = session.transferred_flow_files.borrow();
Review Comment:
why is this special borrow call needed? Couldn't we just take a reference?
##########
minifi_rust/extensions/minifi_rs_playground/features/controller_apis.feature:
##########
@@ -0,0 +1,14 @@
+@SUPPORTS_WINDOWS
+Feature: Testing controller service api casting
+
+ Scenario: Zoo has a jetpack dog
+ Given a ZooProcessorRs processor with the "Can fly service" property set
to "Wolfie the magical"
+ And the "Number of Legs service" property of the ZooProcessorRs processor
is set to "Wolfie the magical"
+ And a DogControllerRs controller service named "Wolfie the magical" is set
up and the "Has Jetpack" property set to "true"
+ 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
Review Comment:
do we need to escape the quotes inside the quotes?
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/put_file/tests.rs:
##########
@@ -0,0 +1,160 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use super::*;
+use crate::processors::put_file::relationships::{FAILURE, SUCCESS};
+use minifi_native::{MockLogger, MockProcessContext};
+
+#[test]
+fn schedule_succeeds_with_default_values() {
+ assert!(PutFileRs::schedule(&MockProcessContext::new(),
&MockLogger::new()).is_ok());
+}
+
+#[test]
+fn simple_put_file_test() {
+ let mut context = MockProcessContext::new();
+ let temp_dir = tempfile::tempdir().expect("temp dir is required for
testing PutFile");
+ let put_file_dir = temp_dir.path().join("subdir");
+
+ context.properties.insert(
+ "Directory".to_string(),
+ put_file_dir.to_str().unwrap().to_string(),
+ );
+ let put_file = PutFileRs::schedule(&context,
&MockLogger::new()).expect("Should succeed");
+
+ let mut input_stream = std::io::Cursor::new("test".as_bytes());
+ context
+ .attributes
+ .insert("filename".to_string(), "test.txt".to_string());
+ let result = put_file
+ .transform(&context, &mut input_stream, &MockLogger::new())
+ .expect("Should succeed");
+
+ assert_eq!(result.target_relationship(), SUCCESS.name);
+
+ let expected_path = temp_dir.path().join("subdir/test.txt");
+ assert!(expected_path.exists());
+ assert_eq!(std::fs::read_to_string(expected_path).unwrap(), "test");
+}
+
+#[test]
+fn put_file_without_create_dirs() {
+ let mut context = MockProcessContext::new();
+ let temp_dir = tempfile::tempdir().expect("temp dir is required for
testing PutFile");
+
+ let put_file_dir = temp_dir.path().join("subdir");
+
+ context.properties.insert(
+ "Directory".to_string(),
+ put_file_dir.to_str().unwrap().to_string(),
+ );
+
+ context.properties.insert(
+ "Create Missing Directories".to_string(),
+ "false".to_string(),
+ );
+
+ let put_file = PutFileRs::schedule(&context,
&MockLogger::new()).expect("Should succeed");
+
+ let mut input_stream = std::io::Cursor::new("test".as_bytes());
+ context
+ .attributes
+ .insert("filename".to_string(), "test.txt".to_string());
+ let result = put_file
+ .transform(&context, &mut input_stream, &MockLogger::new())
+ .expect("Should succeed");
+
+ assert_eq!(result.target_relationship(), FAILURE.name);
+
+ let expected_path = temp_dir.path().join("subdir/test.txt");
+ assert!(!expected_path.exists());
+}
+
+#[test]
+fn directory_is_full_counts_only_files() {
+ let mut context = MockProcessContext::new();
+ let temp_dir = tempfile::tempdir().expect("temp dir is required for
testing PutFile");
+
+ context.properties.insert(
+ "Directory".to_string(),
+ temp_dir.path().to_str().unwrap().to_string(),
+ );
+ context
+ .properties
+ .insert("Maximum File Count".to_string(), "2".to_string());
+
+ let put_file = PutFileRs::schedule(&context,
&MockLogger::new()).expect("Should succeed");
+
+ let destination = temp_dir.path().join("test.txt");
+
+ // No files yet → not full
+ assert!(!put_file.directory_is_full(&destination));
+
+ // Create a subdirectory; it must not be counted as a file
+ std::fs::create_dir(temp_dir.path().join("subdir")).unwrap();
+ assert!(!put_file.directory_is_full(&destination));
+
+ // Add two files → full
+ std::fs::write(temp_dir.path().join("a.txt"), b"a").unwrap();
+ std::fs::write(temp_dir.path().join("b.txt"), b"b").unwrap();
+ assert!(put_file.directory_is_full(&destination));
+
+ // Remove one file → not full again
+ std::fs::remove_file(temp_dir.path().join("a.txt")).unwrap();
+ assert!(!put_file.directory_is_full(&destination));
+}
+
+#[cfg(unix)]
+#[test]
+fn put_file_test_permissions() {
+ use std::os::unix::fs::PermissionsExt;
+ let mut context = MockProcessContext::new();
+ let temp_dir = tempfile::tempdir().expect("temp dir is required for
testing PutFile");
+ let put_file_dir = temp_dir.path().join("subdir");
+
+ context.properties.insert(
+ "Directory".to_string(),
+ put_file_dir.to_str().unwrap().to_string(),
+ );
+
+ context
+ .properties
+ .insert("Directory Permissions".to_string(), "0777".to_string());
+
+ context
+ .properties
+ .insert("Permissions".to_string(), "0777".to_string());
+ let put_file = PutFileRs::schedule(&context,
&MockLogger::new()).expect("Should succeed");
+
+ let mut input_stream = std::io::Cursor::new("test".as_bytes());
+ context
+ .attributes
+ .insert("filename".to_string(), "test.txt".to_string());
+ let result = put_file
+ .transform(&context, &mut input_stream, &MockLogger::new())
+ .expect("Should succeed");
+
+ assert_eq!(result.target_relationship(), SUCCESS.name);
+
+ let expected_path = temp_dir.path().join("subdir/test.txt");
+ assert!(expected_path.exists());
+ assert_eq!(std::fs::read_to_string(&expected_path).unwrap(), "test");
+ let parent_permissions =
std::fs::metadata(put_file_dir).unwrap().permissions();
+ let permissions = expected_path.metadata().unwrap().permissions();
+ assert_eq!(permissions.mode(), 0o100777);
+ assert_eq!(parent_permissions.mode(), 0o40777);
Review Comment:
what do the extra numbers means? are they some kind of flags? would be worth
an explanatory code comment
##########
minifi_rust/extensions/minifi_rs_playground/src/controller_services/lorem_ipsum_controller_service/properties.rs:
##########
@@ -0,0 +1,13 @@
+use minifi_native::{Property, StandardPropertyValidator};
+
+pub(crate) const LENGTH: Property = Property {
Review Comment:
I think I'd keep all parts of a processor in the same file, because it's a
single logical unit of code in my view.
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/asciify_german.rs:
##########
@@ -0,0 +1,67 @@
+use crate::processors::asciify_german::relationships::FAILURE;
Review Comment:
I'd prefer to have this explanation in a code comment
##########
minifi_rust/minifi_native/src/api/errors.rs:
##########
@@ -0,0 +1,129 @@
+use minifi_native_sys::minifi_status;
+use std::borrow::Cow;
+use std::ffi::NulError;
+use std::fmt;
+use std::num::{NonZeroU32, ParseIntError};
+use std::str::ParseBoolError;
+
+#[derive(Debug, Clone)]
+pub enum ParseError {
+ Strum(strum::ParseError),
+ Bool(ParseBoolError),
+ Int(ParseIntError),
+ Duration(humantime::DurationError),
+ Size(byte_unit::ParseError),
+ Nul(NulError),
+ Other,
+}
+
+#[derive(Debug)]
+pub enum MinifiError {
+ UnknownError,
+ StatusError((Cow<'static, str>, NonZeroU32)),
+ MissingRequiredProperty(Cow<'static, str>),
+ ControllerServiceError(Cow<'static, str>),
+ ValidationError(Cow<'static, str>),
+ ScheduleError(Cow<'static, str>),
+ TriggerError(Cow<'static, str>),
+ Parse(ParseError),
+ MissingFlowFileError,
+ IoError(std::io::Error),
+}
+
+impl From<std::io::Error> for MinifiError {
+ fn from(error: std::io::Error) -> Self {
+ MinifiError::IoError(error)
+ }
+}
+
+impl From<strum::ParseError> for MinifiError {
+ fn from(err: strum::ParseError) -> Self {
+ MinifiError::Parse(ParseError::Strum(err))
+ }
+}
+
+impl From<ParseBoolError> for MinifiError {
+ fn from(err: ParseBoolError) -> Self {
+ MinifiError::Parse(ParseError::Bool(err))
+ }
+}
+
+impl From<ParseIntError> for MinifiError {
+ fn from(err: ParseIntError) -> Self {
+ MinifiError::Parse(ParseError::Int(err))
+ }
+}
+
+impl From<humantime::DurationError> for MinifiError {
+ fn from(err: humantime::DurationError) -> Self {
+ MinifiError::Parse(ParseError::Duration(err))
+ }
+}
+
+impl From<byte_unit::ParseError> for MinifiError {
+ fn from(err: byte_unit::ParseError) -> Self {
+ MinifiError::Parse(ParseError::Size(err))
+ }
+}
+
+impl From<NulError> for MinifiError {
+ fn from(err: NulError) -> Self {
+ MinifiError::Parse(ParseError::Nul(err))
+ }
+}
+
+impl MinifiError {
+ pub(crate) fn to_status(&self) -> minifi_status {
+ // TODO expand this
+ minifi_native_sys::minifi_status_MINIFI_STATUS_UNKNOWN_ERROR
+ }
Review Comment:
I think it would be nice to finish this or remove it if it's unused
--
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]
