refactor: replace update_graph_connections_all_fields across multiple modules (#795)

Author: halajohn

core/src/ten_manager/src/designer/graphs/connections/add.rs

@@ -29,7 +29,8 @@ use crate::{
 };
 
 use crate::graph::{
-    graphs_cache_find_by_id_mut, update_graph_connections_all_fields,
+    graphs_cache_find_by_id_mut,
+    update_graph_connections_in_property_all_fields,
 };
 
 #[derive(Serialize, Deserialize)]
@@ -97,27 +98,6 @@ fn create_graph_connection(
     connection
 }
 
-/// Update the property.json file with the new connection.
-fn update_property_file(
-    base_dir: &str,
-    property: &mut ten_rust::pkg_info::property::Property,
-    graph_name: &str,
-    connection: &GraphConnection,
-) -> Result<()> {
-    // Update only with the new connection.
-    let connections_to_add = vec![connection.clone()];
-
-    // Update the property_all_fields map and write to property.json.
-    update_graph_connections_all_fields(
-        base_dir,
-        &mut property.all_fields,
-        graph_name,
-        Some(&connections_to_add),
-        None,
-        None,
-    )
-}
-
 pub async fn add_graph_connection_endpoint(
     request_payload: web::Json<AddGraphConnectionRequestPayload>,
     state: web::Data<Arc<DesignerState>>,
@@ -166,15 +146,19 @@ pub async fn add_graph_connection_endpoint(
     {
         // Update property.json file with the updated graph.
         if let Some(property) = &mut pkg_info.property {
-            // Create a new connection object.
-            let connection = create_graph_connection(&request_payload);
+            // Create a new connection object, and update only with the new
+            // connection.
+            let connections_to_add =
+                vec![create_graph_connection(&request_payload)];
 
-            // Update the property.json file.
-            if let Err(e) = update_property_file(
+            // Update the property_all_fields map and write to property.json.
+            if let Err(e) = update_graph_connections_in_property_all_fields(
                 &pkg_info.url,
-                property,
+                &mut property.all_fields,
                 graph_info.name.as_ref().unwrap(),
-                &connection,
+                Some(&connections_to_add),
+                None,
+                None,
             ) {
                 eprintln!(
                     "Warning: Failed to update property.json file: {}",

core/src/ten_manager/src/designer/graphs/connections/delete.rs

@@ -24,7 +24,10 @@ use crate::{
         response::{ApiResponse, ErrorResponse, Status},
         DesignerState,
     },
-    graph::{graphs_cache_find_by_id_mut, update_graph_connections_all_fields},
+    graph::{
+        graphs_cache_find_by_id_mut,
+        update_graph_connections_in_property_all_fields,
+    },
     pkg_info::belonging_pkg_info_find_by_graph_info_mut,
 };
 
@@ -45,7 +48,7 @@ pub struct DeleteGraphConnectionResponsePayload {
     pub success: bool,
 }
 
-fn delete_connection(
+fn graph_delete_connection(
     graph: &mut Graph,
     src_app: Option<String>,
     src_extension: String,
@@ -164,7 +167,7 @@ pub async fn delete_graph_connection_endpoint(
     };
 
     // Delete the connection.
-    match delete_connection(
+    if let Err(err) = graph_delete_connection(
         &mut graph_info.graph,
         request_payload.src_app.clone(),
         request_payload.src_extension.clone(),
@@ -173,89 +176,83 @@ pub async fn delete_graph_connection_endpoint(
         request_payload.dest_app.clone(),
         request_payload.dest_extension.clone(),
     ) {
-        Ok(_) => {
-            if let Ok(Some(pkg_info)) =
-                belonging_pkg_info_find_by_graph_info_mut(
-                    &mut pkgs_cache,
-                    graph_info,
-                )
-            {
-                // Update property.json file to remove the connection.
-                if let Some(property) = &mut pkg_info.property {
-                    // Create a GraphConnection representing what we
-                    // want to remove.
-                    let mut connection = GraphConnection {
-                        app: request_payload.src_app.clone(),
-                        extension: request_payload.src_extension.clone(),
-                        cmd: None,
-                        data: None,
-                        audio_frame: None,
-                        video_frame: None,
-                    };
-
-                    // Create destination for the message flow.
-                    let destination = GraphDestination {
-                        app: request_payload.dest_app.clone(),
-                        extension: request_payload.dest_extension.clone(),
-                        msg_conversion: None,
-                    };
+        let error_response = ErrorResponse {
+            status: Status::Fail,
+            message: format!("Failed to delete connection: {}", err),
+            error: None,
+        };
+        return Ok(HttpResponse::BadRequest().json(error_response));
+    }
 
-                    // Create the message flow with destination.
-                    let message_flow = GraphMessageFlow {
-                        name: request_payload.msg_name.clone(),
-                        dest: vec![destination],
-                    };
+    if let Ok(Some(pkg_info)) =
+        belonging_pkg_info_find_by_graph_info_mut(&mut pkgs_cache, graph_info)
+    {
+        // Update property.json file to remove the connection.
+        if let Some(property) = &mut pkg_info.property {
+            // Create a GraphConnection representing what we
+            // want to remove.
+            let mut connection = GraphConnection {
+                app: request_payload.src_app.clone(),
+                extension: request_payload.src_extension.clone(),
+                cmd: None,
+                data: None,
+                audio_frame: None,
+                video_frame: None,
+            };
 
-                    // Set the appropriate message type field.
-                    match request_payload.msg_type {
-                        MsgType::Cmd => {
-                            connection.cmd = Some(vec![message_flow]);
-                        }
-                        MsgType::Data => {
-                            connection.data = Some(vec![message_flow]);
-                        }
-                        MsgType::AudioFrame => {
-                            connection.audio_frame = Some(vec![message_flow]);
-                        }
-                        MsgType::VideoFrame => {
-                            connection.video_frame = Some(vec![message_flow]);
-                        }
-                    }
+            // Create destination for the message flow.
+            let destination = GraphDestination {
+                app: request_payload.dest_app.clone(),
+                extension: request_payload.dest_extension.clone(),
+                msg_conversion: None,
+            };
 
-                    let connections_to_remove = vec![connection];
+            // Create the message flow with destination.
+            let message_flow = GraphMessageFlow {
+                name: request_payload.msg_name.clone(),
+                dest: vec![destination],
+            };
 
-                    // Write the updated property_all_fields map to
-                    // property.json.
-                    if let Err(e) = update_graph_connections_all_fields(
-                        &pkg_info.url,
-                        &mut property.all_fields,
-                        graph_info.name.as_ref().unwrap(),
-                        None,
-                        Some(&connections_to_remove),
-                        None,
-                    ) {
-                        eprintln!(
-                            "Warning: Failed to update property.json file: {}",
-                            e
-                        );
-                    }
+            // Set the appropriate message type field.
+            match request_payload.msg_type {
+                MsgType::Cmd => {
+                    connection.cmd = Some(vec![message_flow]);
+                }
+                MsgType::Data => {
+                    connection.data = Some(vec![message_flow]);
+                }
+                MsgType::AudioFrame => {
+                    connection.audio_frame = Some(vec![message_flow]);
+                }
+                MsgType::VideoFrame => {
+                    connection.video_frame = Some(vec![message_flow]);
                 }
             }
 
-            let response = ApiResponse {
-                status: Status::Ok,
-                data: DeleteGraphConnectionResponsePayload { success: true },
-                meta: None,
-            };
-            Ok(HttpResponse::Ok().json(response))
-        }
-        Err(err) => {
-            let error_response = ErrorResponse {
-                status: Status::Fail,
-                message: format!("Failed to delete connection: {}", err),
-                error: None,
-            };
-            Ok(HttpResponse::BadRequest().json(error_response))
+            let connections_to_remove = vec![connection];
+
+            // Write the updated property_all_fields map to
+            // property.json.
+            if let Err(e) = update_graph_connections_in_property_all_fields(
+                &pkg_info.url,
+                &mut property.all_fields,
+                graph_info.name.as_ref().unwrap(),
+                None,
+                Some(&connections_to_remove),
+                None,
+            ) {
+                eprintln!(
+                    "Warning: Failed to update property.json file: {}",
+                    e
+                );
+            }
         }
     }
+
+    let response = ApiResponse {
+        status: Status::Ok,
+        data: DeleteGraphConnectionResponsePayload { success: true },
+        meta: None,
+    };
+    Ok(HttpResponse::Ok().json(response))
 }

core/src/ten_manager/src/designer/graphs/connections/msg_conversion/update.rs

@@ -30,7 +30,8 @@ use crate::{
         connections::validate::{
             validate_connection_schema, MsgConversionValidateInfo,
         },
-        graphs_cache_find_by_id_mut, update_graph_connections_all_fields,
+        graphs_cache_find_by_id_mut,
+        update_graph_connections_in_property_all_fields,
     },
     pkg_info::belonging_pkg_info_find_by_graph_info_mut,
 };
@@ -161,7 +162,7 @@ fn update_property_all_fields(
             let connections_to_modify = vec![connection];
 
             // Update the property.json file.
-            if let Err(e) = update_graph_connections_all_fields(
+            if let Err(e) = update_graph_connections_in_property_all_fields(
                 &pkg_info.url,
                 &mut property.all_fields,
                 graph_info.name.as_ref().unwrap(),

core/src/ten_manager/src/designer/graphs/nodes/delete.rs

@@ -4,7 +4,6 @@
 // Licensed under the Apache License, Version 2.0, with certain conditions.
 // Refer to the "LICENSE" file in the root directory for more information.
 //
-use std::collections::HashMap;
 use std::sync::Arc;
 
 use actix_web::{web, HttpResponse, Responder};
@@ -13,8 +12,7 @@ use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
 use ten_rust::{
-    base_dir_pkg_info::PkgsInfoInApp,
-    graph::{graph_info::GraphInfo, node::GraphNode, Graph},
+    graph::{node::GraphNode, Graph},
     pkg_info::{pkg_type::PkgType, pkg_type_and_name::PkgTypeAndName},
 };
 
@@ -177,15 +175,46 @@ pub async fn delete_graph_node_endpoint(
         return Ok(HttpResponse::BadRequest().json(error_response));
     }
 
-    // Try to update property.json file if possible
-    update_property_json_if_needed(
-        &mut pkgs_cache,
-        graph_info,
-        &request_payload.name,
-        &request_payload.addon,
-        request_payload.extension_group.as_deref(),
-        request_payload.app.as_deref(),
-    );
+    // Try to update property.json file if possible.
+    // First, try to find the belonging package info
+    if let Ok(Some(pkg_info)) =
+        belonging_pkg_info_find_by_graph_info_mut(&mut pkgs_cache, graph_info)
+    {
+        // Check if property exists
+        if let Some(property) = &mut pkg_info.property {
+            // Create the GraphNode we want to remove
+            let node_to_remove = GraphNode {
+                type_and_name: PkgTypeAndName {
+                    pkg_type: PkgType::Extension,
+                    name: request_payload.name.to_string(),
+                },
+                addon: request_payload.addon.to_string(),
+                extension_group: request_payload
+                    .extension_group
+                    .as_deref()
+                    .map(String::from),
+                app: request_payload.app.as_deref().map(String::from),
+                property: None,
+            };
+
+            let nodes_to_remove = vec![node_to_remove];
+
+            // Update property.json file
+            if let Err(e) = update_graph_node_all_fields(
+                &pkg_info.url,
+                &mut property.all_fields,
+                graph_info.name.as_ref().unwrap(),
+                None,
+                Some(&nodes_to_remove),
+                None,
+            ) {
+                eprintln!(
+                    "Warning: Failed to update property.json file: {}",
+                    e
+                );
+            }
+        }
+    }
 
     // Return success response
     let response = ApiResponse {
@@ -195,50 +224,3 @@ pub async fn delete_graph_node_endpoint(
     };
     Ok(HttpResponse::Ok().json(response))
 }
-
-/// Helper function to update property.json after node deletion
-fn update_property_json_if_needed(
-    pkgs_cache: &mut HashMap<String, PkgsInfoInApp>,
-    graph_info: &mut GraphInfo,
-    node_name: &str,
-    addon: &str,
-    extension_group: Option<&str>,
-    app: Option<&str>,
-) {
-    // Try to find the belonging package info
-    if let Ok(Some(pkg_info)) =
-        belonging_pkg_info_find_by_graph_info_mut(pkgs_cache, graph_info)
-    {
-        // Check if property exists
-        let property = match &mut pkg_info.property {
-            Some(property) => property,
-            None => return,
-        };
-
-        // Create the GraphNode we want to remove
-        let node_to_remove = GraphNode {
-            type_and_name: PkgTypeAndName {
-                pkg_type: PkgType::Extension,
-                name: node_name.to_string(),
-            },
-            addon: addon.to_string(),
-            extension_group: extension_group.map(String::from),
-            app: app.map(String::from),
-            property: None,
-        };
-
-        let nodes_to_remove = vec![node_to_remove];
-
-        // Update property.json file
-        if let Err(e) = update_graph_node_all_fields(
-            &pkg_info.url,
-            &mut property.all_fields,
-            graph_info.name.as_ref().unwrap(),
-            None,
-            Some(&nodes_to_remove),
-            None,
-        ) {
-            eprintln!("Warning: Failed to update property.json file: {}", e);
-        }
-    }
-}