fixup! Apply review comments on operation ID

Signed-off-by: Rina Fujino <[email protected]>
thin-edge · Aug 27, 2024 · 0136b9e · 0136b9e
1 parent 4c76502
commit 0136b9e
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 23 deletions.
diff --git a/crates/core/tedge_api/src/mqtt_topics.rs b/crates/core/tedge_api/src/mqtt_topics.rs
@@ -804,12 +804,10 @@ impl IdGenerator {
         cmd_id.contains(&self.prefix)
     }
 
-    /// Extract c8y's operation ID from the provided thin-edge command ID.
-    pub fn get_c8y_operation_id(&self, cmd_id: &str) -> Option<String> {
+    pub fn get_value<'a>(&self, cmd_id: &'a str) -> Option<&'a str> {
         cmd_id
-            .strip_prefix(&format!("{}-", self.prefix))
-            .and_then(|s| s.parse::<u32>().ok())
-            .map(|s| s.to_string())
+            .strip_prefix(&self.prefix)
+            .and_then(|s| s.strip_prefix('-'))
     }
 }
 
@@ -1021,13 +1019,13 @@ mod tests {
         );
     }
 
-    #[test_case("1234", Some("1234".into()))]
-    #[test_case("abcd", None)]
-    #[test_case("8dd1d3", None)]
-    fn extract_c8y_operation_id_from_cmd_id(input: &str, expected: Option<String>) {
-        let id_generator = IdGenerator::new("c8y-mapper");
-        let cmd_id = id_generator.new_id_with_str(input);
-        let maybe_id = id_generator.get_c8y_operation_id(&cmd_id);
+    #[test_case("abc-1234", Some("1234"))]
+    #[test_case("abc-", Some(""))]
+    #[test_case("abc", None)]
+    #[test_case("1234", None)]
+    fn extract_value_from_cmd_id(cmd_id: &str, expected: Option<&str>) {
+        let id_generator = IdGenerator::new("abc");
+        let maybe_id = id_generator.get_value(cmd_id);
         assert_eq!(maybe_id, expected);
     }
 }
diff --git a/crates/extensions/c8y_mapper_ext/src/operations/handlers/config_snapshot.rs b/crates/extensions/c8y_mapper_ext/src/operations/handlers/config_snapshot.rs
@@ -113,7 +113,7 @@ impl OperationContext {
                     c8y_binary_url.as_str(),
                     CumulocitySupportedOperations::C8yUploadConfigFile,
                     self.smart_rest_use_operation_id,
-                    self.command_id.get_c8y_operation_id(cmd_id),
+                    self.get_operation_id(cmd_id),
                 );
 
                 let c8y_notification = MqttMessage::new(smartrest_topic, smartrest_response);

diff --git a/crates/extensions/c8y_mapper_ext/src/operations/handlers/log_upload.rs b/crates/extensions/c8y_mapper_ext/src/operations/handlers/log_upload.rs
@@ -94,7 +94,7 @@ impl OperationContext {
                     binary_upload_event_url.as_str(),
                     CumulocitySupportedOperations::C8yLogFileRequest,
                     self.smart_rest_use_operation_id,
-                    self.command_id.get_c8y_operation_id(cmd_id),
+                    self.get_operation_id(cmd_id),
                 );
 
                 let c8y_notification = MqttMessage::new(smartrest_topic, smartrest_response);

diff --git a/crates/extensions/c8y_mapper_ext/src/operations/handlers/mod.rs b/crates/extensions/c8y_mapper_ext/src/operations/handlers/mod.rs
@@ -162,13 +162,12 @@ impl OperationContext {
         ) {
             OperationOutcome::Ignored => {}
             OperationOutcome::Executing { mut extra_messages } => {
-                let c8y_state_executing_payload =
-                    match self.command_id.get_c8y_operation_id(&cmd_id) {
-                        Some(op_id) if self.smart_rest_use_operation_id => {
-                            set_operation_executing_with_id(&op_id)
-                        }
-                        _ => set_operation_executing_with_name(c8y_operation),
-                    };
+                let c8y_state_executing_payload = match self.get_operation_id(&cmd_id) {
+                    Some(op_id) if self.smart_rest_use_operation_id => {
+                        set_operation_executing_with_id(&op_id)
+                    }
+                    _ => set_operation_executing_with_name(c8y_operation),
+                };
 
                 let c8y_state_executing_message =
                     MqttMessage::new(&entity.smartrest_publish_topic, c8y_state_executing_payload);
@@ -202,7 +201,7 @@ impl OperationContext {
         operation: CumulocitySupportedOperations,
         cmd_id: &str,
     ) -> c8y_api::smartrest::smartrest_serializer::SmartRest {
-        match self.command_id.get_c8y_operation_id(cmd_id) {
+        match self.get_operation_id(cmd_id) {
             Some(op_id) if self.smart_rest_use_operation_id => {
                 succeed_operation_with_id_no_parameters(&op_id)
             }
@@ -216,7 +215,7 @@ impl OperationContext {
         reason: &str,
         cmd_id: &str,
     ) -> c8y_api::smartrest::smartrest_serializer::SmartRest {
-        match self.command_id.get_c8y_operation_id(cmd_id) {
+        match self.get_operation_id(cmd_id) {
             Some(op_id) if self.smart_rest_use_operation_id => {
                 fail_operation_with_id(&op_id, reason)
             }
@@ -250,6 +249,13 @@ impl OperationContext {
 
         OperationOutcome::Finished { messages }
     }
+
+    fn get_operation_id(&self, cmd_id: &str) -> Option<String> {
+        self.command_id
+            .get_value(cmd_id)
+            .and_then(|s| s.parse::<u32>().ok()) // Ensure the operation ID is numeric
+            .map(|s| s.to_string())
+    }
 }
 
 async fn clear_command_topic(