From 4f3b32e212221da8951ba4f671e5d74c7f4b5905 Mon Sep 17 00:00:00 2001 From: leo Date: Tue, 24 Sep 2024 13:31:18 +0800 Subject: [PATCH] refactor: reuse PropertyPredefinedGraph in tman pkg (#40) --- .../src/dev_server/graphs/connections.rs | 84 +++++++-------- .../ten_manager/src/dev_server/graphs/mod.rs | 7 +- .../src/dev_server/graphs/update.rs | 33 +++--- .../predefined_graphs/connection.rs | 52 ++++----- core/src/ten_rust/src/pkg_info/graph/mod.rs | 64 +---------- .../pkg_info/predefined_graphs/connection.rs | 88 --------------- .../pkg_info/predefined_graphs/extension.rs | 2 +- .../src/pkg_info/predefined_graphs/mod.rs | 102 ++++-------------- core/src/ten_rust/tests/graph_check.rs | 9 +- 9 files changed, 118 insertions(+), 323 deletions(-) delete mode 100644 core/src/ten_rust/src/pkg_info/predefined_graphs/connection.rs diff --git a/core/src/ten_manager/src/dev_server/graphs/connections.rs b/core/src/ten_manager/src/dev_server/graphs/connections.rs index 81f69761de..117ac0f57e 100644 --- a/core/src/ten_manager/src/dev_server/graphs/connections.rs +++ b/core/src/ten_manager/src/dev_server/graphs/connections.rs @@ -8,15 +8,14 @@ use std::sync::{Arc, RwLock}; use actix_web::{web, HttpResponse, Responder}; use serde::{Deserialize, Serialize}; +use ten_rust::pkg_info::graph::{ + GraphConnection, GraphDestination, GraphMessageFlow, +}; use crate::dev_server::get_all_pkgs::get_all_pkgs; use crate::dev_server::response::{ApiResponse, ErrorResponse, Status}; use crate::dev_server::DevServerState; use ten_rust::pkg_info::pkg_type::PkgType; -use ten_rust::pkg_info::predefined_graphs::connection::PkgDestination; -use ten_rust::pkg_info::predefined_graphs::connection::{ - PkgConnection, PkgMessageFlow, -}; use ten_rust::pkg_info::predefined_graphs::pkg_predefined_graphs_find; #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] @@ -38,36 +37,24 @@ pub struct DevServerConnection { pub video_frame: Option>, } -impl From for DevServerConnection { - fn from(conn: PkgConnection) -> Self { +impl From for DevServerConnection { + fn from(conn: GraphConnection) -> Self { DevServerConnection { app: conn.app, extension_group: conn.extension_group, extension: conn.extension, - cmd: if conn.cmd.is_empty() { - None - } else { - Some(get_dev_server_msg_flow_from_pkg(conn.cmd.clone())) - }, - - data: if conn.data.is_empty() { - None - } else { - Some(get_dev_server_msg_flow_from_pkg(conn.data.clone())) - }, - - audio_frame: if conn.audio_frame.is_empty() { - None - } else { - Some(get_dev_server_msg_flow_from_pkg(conn.audio_frame.clone())) - }, - - video_frame: if conn.video_frame.is_empty() { - None - } else { - Some(get_dev_server_msg_flow_from_pkg(conn.video_frame.clone())) - }, + cmd: conn.cmd.map(get_dev_server_msg_flow_from_property), + + data: conn.data.map(get_dev_server_msg_flow_from_property), + + audio_frame: conn + .audio_frame + .map(get_dev_server_msg_flow_from_property), + + video_frame: conn + .video_frame + .map(get_dev_server_msg_flow_from_property), } } } @@ -78,18 +65,22 @@ pub struct DevServerMessageFlow { pub dest: Vec, } -impl From for DevServerMessageFlow { - fn from(msg_flow: PkgMessageFlow) -> Self { +impl From for DevServerMessageFlow { + fn from(msg_flow: GraphMessageFlow) -> Self { DevServerMessageFlow { name: msg_flow.name, - dest: get_dev_server_destination_from_pkg(msg_flow.dest), + dest: get_dev_server_destination_from_property(msg_flow.dest), } } } -fn get_dev_server_msg_flow_from_pkg( - msg_flow: Vec, +fn get_dev_server_msg_flow_from_property( + msg_flow: Vec, ) -> Vec { + if msg_flow.is_empty() { + return vec![]; + } + msg_flow.into_iter().map(|v| v.into()).collect() } @@ -100,8 +91,8 @@ pub struct DevServerDestination { pub extension: String, } -impl From for DevServerDestination { - fn from(destination: PkgDestination) -> Self { +impl From for DevServerDestination { + fn from(destination: GraphDestination) -> Self { DevServerDestination { app: destination.app, extension_group: destination.extension_group, @@ -110,8 +101,8 @@ impl From for DevServerDestination { } } -fn get_dev_server_destination_from_pkg( - destinations: Vec, +fn get_dev_server_destination_from_property( + destinations: Vec, ) -> Vec { destinations.into_iter().map(|v| v.into()).collect() } @@ -142,15 +133,20 @@ pub async fn get_graph_connections( // specified graph_name. if let Some(graph) = pkg_predefined_graphs_find(&app_pkg.predefined_graphs, |g| { - g.name == graph_name + g.prop_predefined_graph.name == graph_name }) { // Convert the connections field to RespConnection. - let resp_connections: Vec = graph - .connections - .iter() - .map(|conn| conn.clone().into()) - .collect(); + let connections = + graph.prop_predefined_graph.graph.connections.as_ref(); + let resp_connections: Vec = + match connections { + Some(connections) => connections + .iter() + .map(|conn| conn.clone().into()) + .collect(), + None => vec![], + }; let response = ApiResponse { status: Status::Ok, diff --git a/core/src/ten_manager/src/dev_server/graphs/mod.rs b/core/src/ten_manager/src/dev_server/graphs/mod.rs index 704811c207..fde853c1fb 100644 --- a/core/src/ten_manager/src/dev_server/graphs/mod.rs +++ b/core/src/ten_manager/src/dev_server/graphs/mod.rs @@ -50,8 +50,11 @@ pub async fn get_graphs( .predefined_graphs .iter() .map(|graph| RespGraph { - name: graph.name.clone(), - auto_start: graph.auto_start, + name: graph.prop_predefined_graph.name.clone(), + auto_start: graph + .prop_predefined_graph + .auto_start + .unwrap_or(false), }) .collect(); diff --git a/core/src/ten_manager/src/dev_server/graphs/update.rs b/core/src/ten_manager/src/dev_server/graphs/update.rs index 02fdea7cda..4a5a4b8106 100644 --- a/core/src/ten_manager/src/dev_server/graphs/update.rs +++ b/core/src/ten_manager/src/dev_server/graphs/update.rs @@ -39,8 +39,7 @@ fn update_graph_to_property(app_pkg: &mut PkgInfo) { app_pkg .predefined_graphs .iter() - .cloned() - .map(|g| g.into()) + .map(|g| g.prop_predefined_graph.clone()) .collect(), ); } else { @@ -49,8 +48,7 @@ fn update_graph_to_property(app_pkg: &mut PkgInfo) { app_pkg .predefined_graphs .iter() - .cloned() - .map(|g| g.into()) + .map(|g| g.prop_predefined_graph.clone()) .collect(), ), uri: None, @@ -65,8 +63,7 @@ fn update_graph_to_property(app_pkg: &mut PkgInfo) { app_pkg .predefined_graphs .iter() - .cloned() - .map(|g| g.into()) + .map(|g| g.prop_predefined_graph.clone()) .collect(), ), uri: None, @@ -129,11 +126,11 @@ pub async fn update_graph( if let Some(old_graph) = app_pkg .predefined_graphs .iter_mut() - .find(|g| g.name == graph_name) + .find(|g| g.prop_predefined_graph.name == graph_name) { - old_graph.auto_start = new_graph.auto_start; old_graph.nodes = new_graph.nodes; - old_graph.connections = new_graph.connections; + old_graph.prop_predefined_graph = + new_graph.prop_predefined_graph; } update_graph_to_property(app_pkg); @@ -256,12 +253,24 @@ mod tests { let predefined_graph = app_pkg .predefined_graphs .iter() - .find(|g| g.name == "0") + .find(|g| g.prop_predefined_graph.name == "0") .unwrap(); - assert!(!predefined_graph.auto_start); + assert!(!predefined_graph + .prop_predefined_graph + .auto_start + .unwrap()); assert_eq!(predefined_graph.nodes.len(), 2); - assert_eq!(predefined_graph.connections.len(), 1); + assert_eq!( + predefined_graph + .prop_predefined_graph + .graph + .connections + .as_ref() + .unwrap() + .len(), + 1 + ); let property = app_pkg.property.as_ref().unwrap(); let property_predefined_graphs = property diff --git a/core/src/ten_manager/src/package_info/predefined_graphs/connection.rs b/core/src/ten_manager/src/package_info/predefined_graphs/connection.rs index 162b39082d..bb08be572d 100644 --- a/core/src/ten_manager/src/package_info/predefined_graphs/connection.rs +++ b/core/src/ten_manager/src/package_info/predefined_graphs/connection.rs @@ -7,50 +7,42 @@ use crate::dev_server::graphs::connections::{ DevServerConnection, DevServerDestination, DevServerMessageFlow, }; -use ten_rust::pkg_info::predefined_graphs::connection::{ - PkgConnection, PkgDestination, PkgMessageFlow, +use ten_rust::pkg_info::graph::{ + GraphConnection, GraphDestination, GraphMessageFlow, }; -impl From for PkgConnection { +impl From for GraphConnection { fn from(dev_server_connection: DevServerConnection) -> Self { - PkgConnection { + GraphConnection { app: dev_server_connection.app, extension_group: dev_server_connection.extension_group, extension: dev_server_connection.extension, - cmd: match dev_server_connection.cmd { - Some(cmd) => get_pkg_msg_flow_from_dev_server(cmd), - None => Vec::new(), - }, - data: match dev_server_connection.data { - Some(data) => get_pkg_msg_flow_from_dev_server(data), - None => Vec::new(), - }, - audio_frame: match dev_server_connection.audio_frame { - Some(audio_frame) => { - get_pkg_msg_flow_from_dev_server(audio_frame) - } - None => Vec::new(), - }, - video_frame: match dev_server_connection.video_frame { - Some(video_frame) => { - get_pkg_msg_flow_from_dev_server(video_frame) - } - None => Vec::new(), - }, + cmd: dev_server_connection + .cmd + .map(get_property_msg_flow_from_dev_server), + data: dev_server_connection + .data + .map(get_property_msg_flow_from_dev_server), + audio_frame: dev_server_connection + .audio_frame + .map(get_property_msg_flow_from_dev_server), + video_frame: dev_server_connection + .video_frame + .map(get_property_msg_flow_from_dev_server), } } } -fn get_pkg_msg_flow_from_dev_server( +fn get_property_msg_flow_from_dev_server( msg_flow: Vec, -) -> Vec { +) -> Vec { msg_flow.into_iter().map(|v| v.into()).collect() } -impl From for PkgMessageFlow { +impl From for GraphMessageFlow { fn from(dev_server_msg_flow: DevServerMessageFlow) -> Self { - PkgMessageFlow { + GraphMessageFlow { name: dev_server_msg_flow.name, dest: dev_server_msg_flow .dest @@ -61,9 +53,9 @@ impl From for PkgMessageFlow { } } -impl From for PkgDestination { +impl From for GraphDestination { fn from(dev_server_destination: DevServerDestination) -> Self { - PkgDestination { + GraphDestination { app: dev_server_destination.app, extension_group: dev_server_destination.extension_group, extension: dev_server_destination.extension, diff --git a/core/src/ten_rust/src/pkg_info/graph/mod.rs b/core/src/ten_rust/src/pkg_info/graph/mod.rs index d5f2273dd7..11fa530263 100644 --- a/core/src/ten_rust/src/pkg_info/graph/mod.rs +++ b/core/src/ten_rust/src/pkg_info/graph/mod.rs @@ -11,14 +11,7 @@ use std::{collections::HashMap, str::FromStr}; use anyhow::Result; use serde::{Deserialize, Serialize}; -use super::{ - pkg_type::PkgType, - predefined_graphs::{ - connection::{PkgConnection, PkgDestination, PkgMessageFlow}, - node::PkgNode, - }, - PkgInfo, -}; +use super::{pkg_type::PkgType, predefined_graphs::node::PkgNode, PkgInfo}; use crate::pkg_info::default_app_loc; #[derive(Serialize, Deserialize, Debug, Clone)] @@ -157,61 +150,6 @@ pub struct GraphDestination { pub extension: String, } -fn get_property_msg_flow_from_pkg( - msg_flow: Vec, -) -> Vec { - msg_flow.into_iter().map(|v| v.into()).collect() -} - -impl From for GraphConnection { - fn from(pkg_msg_flow: PkgConnection) -> Self { - GraphConnection { - app: pkg_msg_flow.app.clone(), - extension_group: pkg_msg_flow.extension_group.clone(), - extension: pkg_msg_flow.extension.clone(), - cmd: if pkg_msg_flow.cmd.is_empty() { - None - } else { - Some(get_property_msg_flow_from_pkg(pkg_msg_flow.cmd)) - }, - data: if pkg_msg_flow.data.is_empty() { - None - } else { - Some(get_property_msg_flow_from_pkg(pkg_msg_flow.data)) - }, - audio_frame: if pkg_msg_flow.audio_frame.is_empty() { - None - } else { - Some(get_property_msg_flow_from_pkg(pkg_msg_flow.audio_frame)) - }, - video_frame: if pkg_msg_flow.video_frame.is_empty() { - None - } else { - Some(get_property_msg_flow_from_pkg(pkg_msg_flow.video_frame)) - }, - } - } -} - -impl From for GraphMessageFlow { - fn from(pkg_msg_flow: PkgMessageFlow) -> Self { - GraphMessageFlow { - name: pkg_msg_flow.name.clone(), - dest: pkg_msg_flow.dest.iter().map(|d| d.clone().into()).collect(), - } - } -} - -impl From for GraphDestination { - fn from(pkg_destination: PkgDestination) -> Self { - GraphDestination { - app: pkg_destination.app.clone(), - extension_group: pkg_destination.extension_group.clone(), - extension: pkg_destination.extension.clone(), - } - } -} - #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/core/src/ten_rust/src/pkg_info/predefined_graphs/connection.rs b/core/src/ten_rust/src/pkg_info/predefined_graphs/connection.rs deleted file mode 100644 index 17fbb7feec..0000000000 --- a/core/src/ten_rust/src/pkg_info/predefined_graphs/connection.rs +++ /dev/null @@ -1,88 +0,0 @@ -// -// Copyright © 2024 Agora -// This file is part of TEN Framework, an open source project. -// Licensed under the Apache License, Version 2.0, with certain conditions. -// Refer to the "LICENSE" file in the root directory for more information. -// -use crate::pkg_info::graph::{ - GraphConnection, GraphDestination, GraphMessageFlow, -}; - -#[derive(Debug, Clone)] -pub struct PkgConnection { - pub app: String, - pub extension_group: String, - pub extension: String, - - pub cmd: Vec, - pub data: Vec, - pub audio_frame: Vec, - pub video_frame: Vec, -} - -impl From for PkgConnection { - fn from(manifest_connection: GraphConnection) -> Self { - PkgConnection { - app: manifest_connection.app, - extension_group: manifest_connection.extension_group, - extension: manifest_connection.extension, - - cmd: match manifest_connection.cmd { - Some(cmd) => cmd.into_iter().map(|m| m.into()).collect(), - None => Vec::new(), - }, - data: match manifest_connection.data { - Some(data) => data.into_iter().map(|m| m.into()).collect(), - None => Vec::new(), - }, - audio_frame: match manifest_connection.audio_frame { - Some(audio_frame) => { - audio_frame.into_iter().map(|m| m.into()).collect() - } - None => Vec::new(), - }, - video_frame: match manifest_connection.video_frame { - Some(video_frame) => { - video_frame.into_iter().map(|m| m.into()).collect() - } - None => Vec::new(), - }, - } - } -} - -#[derive(Debug, Clone)] -pub struct PkgMessageFlow { - pub name: String, - pub dest: Vec, -} - -impl From for PkgMessageFlow { - fn from(manifest_message_flow: GraphMessageFlow) -> Self { - PkgMessageFlow { - name: manifest_message_flow.name, - dest: manifest_message_flow - .dest - .into_iter() - .map(|d| d.into()) - .collect(), - } - } -} - -#[derive(Debug, Clone)] -pub struct PkgDestination { - pub app: String, - pub extension_group: String, - pub extension: String, -} - -impl From for PkgDestination { - fn from(manifest_destination: GraphDestination) -> Self { - PkgDestination { - app: manifest_destination.app, - extension_group: manifest_destination.extension_group, - extension: manifest_destination.extension, - } - } -} diff --git a/core/src/ten_rust/src/pkg_info/predefined_graphs/extension.rs b/core/src/ten_rust/src/pkg_info/predefined_graphs/extension.rs index 3e86564c9e..67334268f5 100644 --- a/core/src/ten_rust/src/pkg_info/predefined_graphs/extension.rs +++ b/core/src/ten_rust/src/pkg_info/predefined_graphs/extension.rs @@ -39,7 +39,7 @@ pub fn get_extension_nodes_in_graph( // package. if let Some(graph) = pkg_predefined_graphs_find(&app_pkg.predefined_graphs, |graph| { - graph.name == *graph_name + graph.prop_predefined_graph.name == *graph_name }) { // Collect all extension nodes from the graph. diff --git a/core/src/ten_rust/src/pkg_info/predefined_graphs/mod.rs b/core/src/ten_rust/src/pkg_info/predefined_graphs/mod.rs index 0ca004f2c0..bad9f9f2c1 100644 --- a/core/src/ten_rust/src/pkg_info/predefined_graphs/mod.rs +++ b/core/src/ten_rust/src/pkg_info/predefined_graphs/mod.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. // -pub mod connection; pub mod extension; pub mod node; @@ -14,9 +13,20 @@ use crate::pkg_info::{ graph::Graph, property::{predefined_graph::PropertyPredefinedGraph, Property}, }; -use connection::PkgConnection; use node::PkgNode; +use super::graph::GraphConnection; + +#[derive(Debug, Clone)] +pub struct PkgPredefinedGraph { + pub prop_predefined_graph: PropertyPredefinedGraph, + + // TODO(Liu): + // * Using GraphNode instead. + // * Add Vec to store the extra pkg info for nodes in this graph. + pub nodes: Vec, +} + pub fn get_pkg_predefined_graphs_from_property( property: &Property, ) -> Result> { @@ -26,10 +36,6 @@ pub fn get_pkg_predefined_graphs_from_property( if let Some(predefined_graphs) = &ten.predefined_graphs { for property_predefined_graph in predefined_graphs { graphs.push(PkgPredefinedGraph { - name: property_predefined_graph.name.clone(), - auto_start: property_predefined_graph - .auto_start - .unwrap_or(false), nodes: property_predefined_graph .graph .nodes @@ -37,17 +43,7 @@ pub fn get_pkg_predefined_graphs_from_property( .cloned() .map(|m| m.into()) .collect(), - connections: match &property_predefined_graph - .graph - .connections - { - Some(connections) => connections - .iter() - .cloned() - .map(|m| m.into()) - .collect(), - None => Vec::new(), - }, + prop_predefined_graph: property_predefined_graph.clone(), }); } } @@ -70,73 +66,19 @@ pub fn get_pkg_predefined_graph_from_nodes_and_connections( graph_name: &str, auto_start: bool, nodes: &Vec, - connections: &Vec, + connections: &Vec, ) -> Result { Ok(PkgPredefinedGraph { - name: graph_name.to_owned(), - auto_start, - nodes: nodes.to_vec(), - connections: connections.to_vec(), - }) -} - -#[derive(Debug, Clone)] -pub struct PkgPredefinedGraph { - pub name: String, - pub auto_start: bool, - pub nodes: Vec, - pub connections: Vec, -} - -impl From for PkgPredefinedGraph { - fn from(property_graph: PropertyPredefinedGraph) -> Self { - PkgPredefinedGraph { - name: property_graph.name, - auto_start: property_graph.auto_start.unwrap_or(false), - nodes: property_graph - .graph - .nodes - .into_iter() - .map(|v| v.into()) - .collect(), - connections: match property_graph.graph.connections { - Some(connections) => { - connections.into_iter().map(|v| v.into()).collect() - } - None => Vec::new(), - }, - } - } -} - -impl From for PropertyPredefinedGraph { - fn from(pkg_predefined_graph: PkgPredefinedGraph) -> Self { - PropertyPredefinedGraph { - name: pkg_predefined_graph.name.clone(), - auto_start: Some(pkg_predefined_graph.auto_start), + prop_predefined_graph: PropertyPredefinedGraph { + name: graph_name.to_string(), + auto_start: Some(auto_start), graph: Graph { - nodes: pkg_predefined_graph - .nodes - .iter() - .map(|n| n.clone().into()) - .collect(), + nodes: nodes.iter().map(|n| n.clone().into()).collect(), connections: Some( - pkg_predefined_graph - .connections - .iter() - .map(|c| c.clone().into()) - .collect(), + connections.iter().map(|c| c.clone().into()).collect(), ), }, - } - } -} - -pub fn get_property_predefined_graphs_from_pkg( - pkg_predefined_graphs: Vec, -) -> Vec { - pkg_predefined_graphs - .into_iter() - .map(|v| v.into()) - .collect() + }, + nodes: nodes.to_vec(), + }) } diff --git a/core/src/ten_rust/tests/graph_check.rs b/core/src/ten_rust/tests/graph_check.rs index 67eb64dbb1..cf12da0fdd 100644 --- a/core/src/ten_rust/tests/graph_check.rs +++ b/core/src/ten_rust/tests/graph_check.rs @@ -23,7 +23,8 @@ fn test_graph_check_extension_not_installed() { .last(); let app_pkg = app_pkg_info.unwrap(); let pkg_graph = app_pkg.predefined_graphs.first().unwrap(); - let predefined_graph: PropertyPredefinedGraph = pkg_graph.clone().into(); + let predefined_graph: PropertyPredefinedGraph = + pkg_graph.prop_predefined_graph.clone(); let graph = &predefined_graph.graph; let mut pkg_info_map: HashMap> = HashMap::new(); @@ -47,7 +48,8 @@ fn test_graph_check_predefined_graph_success() { .last(); let app_pkg = app_pkg_info.unwrap(); let pkg_graph = app_pkg.predefined_graphs.first().unwrap(); - let predefined_graph: PropertyPredefinedGraph = pkg_graph.clone().into(); + let predefined_graph: PropertyPredefinedGraph = + pkg_graph.prop_predefined_graph.clone(); let graph = &predefined_graph.graph; let mut pkg_info_map: HashMap> = HashMap::new(); @@ -70,7 +72,8 @@ fn test_graph_check_all_msgs_schema_incompatible() { .last(); let app_pkg = app_pkg_info.unwrap(); let pkg_graph = app_pkg.predefined_graphs.first().unwrap(); - let predefined_graph: PropertyPredefinedGraph = pkg_graph.clone().into(); + let predefined_graph: PropertyPredefinedGraph = + pkg_graph.prop_predefined_graph.clone(); let graph = &predefined_graph.graph; let mut pkg_info_map: HashMap> = HashMap::new();