Skip to content

Commit

Permalink
Merge branch 'main' into 105-use-ten_value_t-uniformly-to-represent-e…
Browse files Browse the repository at this point in the history
…ach-field-in-the-message
  • Loading branch information
sunxilin authored Oct 23, 2024
2 parents 207a390 + 844b61f commit 96c6cae
Show file tree
Hide file tree
Showing 24 changed files with 1,699 additions and 47 deletions.
5 changes: 3 additions & 2 deletions core/src/ten_manager/src/cmd/cmd_check/cmd_check_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ fn get_graphs_to_be_checked(command: &CheckGraphCommand) -> Result<Vec<Graph>> {
let mut graphs_to_be_checked: Vec<Graph> = Vec::new();

if let Some(graph_str) = &command.graph {
let graph: Graph = Graph::from_str(graph_str)
.with_context(|| "The graph json string is invalid")?;
let graph: Graph = Graph::from_str(graph_str).map_err(|e| {
anyhow::anyhow!("Failed to parse graph string, {}", e)
})?;
graphs_to_be_checked.push(graph);
} else {
let first_app_path = path::Path::new(&command.app[0]);
Expand Down
57 changes: 55 additions & 2 deletions core/src/ten_manager/tests/cmd_check_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ async fn test_cmd_check_app_in_graph_cannot_be_localhost() {
eprintln!("{:?}", result);

let msg = result.err().unwrap().to_string();
assert!(msg
.contains("the app uri should be some string other than 'localhost'"));
assert!(msg.contains("'localhost' is not allowed in graph definition"));
}

#[actix_rt::test]
Expand Down Expand Up @@ -179,3 +178,57 @@ async fn test_cmd_check_unique_extension_in_connections() {
assert!(result.is_err());
eprintln!("{:?}", result);
}

#[actix_rt::test]
async fn test_cmd_check_single_app_node_cannot_be_localhost() {
let tman_config = TmanConfig::default();
let command = CheckGraphCommand {
app: vec!["tests/test_data/cmd_check_single_app_node_cannot_be_localhost".to_string()],
graph: Some(
include_str!(
"test_data/cmd_check_single_app_node_cannot_be_localhost/start_graph.json"
)
.to_string(),
),
predefined_graph_name: None,
};

let result = ten_manager::cmd::cmd_check::cmd_check_graph::execute_cmd(
&tman_config,
command,
)
.await;

assert!(result.is_err());
eprintln!("{:?}", result);

let msg = result.err().unwrap().to_string();
assert!(msg.contains("'localhost' is not allowed in graph definition"));
}

#[actix_rt::test]
async fn test_cmd_check_multi_apps_node_cannot_be_localhost() {
let tman_config = TmanConfig::default();
let command = CheckGraphCommand {
app: vec!["tests/test_data/cmd_check_multi_apps_node_cannot_be_localhost".to_string()],
graph: Some(
include_str!(
"test_data/cmd_check_multi_apps_node_cannot_be_localhost/start_graph.json"
)
.to_string(),
),
predefined_graph_name: None,
};

let result = ten_manager::cmd::cmd_check::cmd_check_graph::execute_cmd(
&tman_config,
command,
)
.await;

assert!(result.is_err());
eprintln!("{:?}", result);

let msg = result.err().unwrap().to_string();
assert!(msg.contains("'localhost' is not allowed in graph definition"));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"type": "app",
"name": "cmd_check_single_app",
"version": "0.1.0",
"dependencies": [
{
"type": "extension",
"name": "addon_a",
"version": "0.1.0"
},
{
"type": "extension",
"name": "addon_b",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"type": "start_graph",
"nodes": [
{
"type": "extension",
"name": "ext_a",
"addon": "addon_a",
"extension_group": "some_group",
"app": "http://localhost:8000"
},
{
"type": "extension",
"name": "ext_b",
"addon": "addon_b",
"extension_group": "some_group",
"app": "localhost"
}
],
"connections": [
{
"extension_group": "some_group",
"extension": "ext_a",
"app": "http://localhost:8000",
"cmd": [
{
"name": "cmd_1",
"dest": [
{
"extension_group": "some_group",
"extension": "ext_b"
}
]
}
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "extension",
"name": "addon_a",
"version": "0.1.0",
"dependencies": [
{
"type": "system",
"name": "ten_runtime_go",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
},
"api": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "extension",
"name": "addon_b",
"version": "0.1.0",
"dependencies": [
{
"type": "system",
"name": "ten_runtime_go",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
},
"api": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"type": "app",
"name": "cmd_check_single_app",
"version": "0.1.0",
"dependencies": [
{
"type": "extension",
"name": "addon_a",
"version": "0.1.0"
},
{
"type": "extension",
"name": "addon_b",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"type": "start_graph",
"nodes": [
{
"type": "extension",
"name": "ext_a",
"addon": "addon_a",
"extension_group": "some_group",
"app": "localhost"
},
{
"type": "extension",
"name": "ext_b",
"addon": "addon_b",
"extension_group": "some_group",
"app": "localhost"
}
],
"connections": [
{
"extension_group": "some_group",
"extension": "ext_a",
"cmd": [
{
"name": "cmd_1",
"dest": [
{
"extension_group": "some_group",
"extension": "ext_b"
}
]
}
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "extension",
"name": "addon_a",
"version": "0.1.0",
"dependencies": [
{
"type": "system",
"name": "ten_runtime_go",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
},
"api": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"type": "extension",
"name": "addon_b",
"version": "0.1.0",
"dependencies": [
{
"type": "system",
"name": "ten_runtime_go",
"version": "0.1.0"
}
],
"package": {
"include": [
"**"
]
},
"api": {}
}
3 changes: 2 additions & 1 deletion core/src/ten_runtime/app/graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ bool ten_app_check_start_graph_cmd_json(ten_app_t *self,
start_graph_cmd_json, TEN_STR_UNDERLINE_TEN, &free_json_string);

const char *err_msg = NULL;
bool rc = ten_rust_check_graph_for_app(base_dir, graph_json_str, &err_msg);
bool rc = ten_rust_check_graph_for_app(base_dir, graph_json_str,
ten_app_get_uri(self), &err_msg);

if (free_json_string) {
TEN_FREE(graph_json_str);
Expand Down
5 changes: 5 additions & 0 deletions core/src/ten_rust/src/pkg_info/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,25 @@ use std::ffi::{c_char, CStr, CString};
pub extern "C" fn ten_rust_check_graph_for_app(
app_base_dir: *const c_char,
graph_json: *const c_char,
app_uri: *const c_char,
out_err_msg: *mut *const c_char,
) -> bool {
assert!(!app_base_dir.is_null(), "Invalid argument.");
assert!(!graph_json.is_null(), "Invalid argument.");
assert!(!app_uri.is_null(), "Invalid argument.");

let c_app_base_dir = unsafe { CStr::from_ptr(app_base_dir) };
let c_graph_json = unsafe { CStr::from_ptr(graph_json) };
let c_app_uri = unsafe { CStr::from_ptr(app_uri) };

let rust_app_base_dir = c_app_base_dir.to_str().unwrap();
let rust_graph_json = c_graph_json.to_str().unwrap();
let rust_app_uri = c_app_uri.to_str().unwrap();

let ret = crate::pkg_info::ten_rust_check_graph_for_app(
rust_app_base_dir,
rust_graph_json,
rust_app_uri,
);
if ret.is_err() {
let err_msg = ret.err().unwrap().to_string();
Expand Down
21 changes: 21 additions & 0 deletions core/src/ten_rust/src/pkg_info/graph/constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//
// 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.
//
pub const ERR_MSG_APP_LOCALHOST_DISALLOWED_SINGLE: &str =
"'localhost' is not allowed in graph definition, and the graph seems to be a single-app graph, just remove the 'app' field";

pub const ERR_MSG_APP_LOCALHOST_DISALLOWED_MULTI: &str =
"'localhost' is not allowed in graph definition, change the content of 'app' field to be consistent with '_ten::uri'";

pub const ERR_MSG_APP_DECLARATION_MISMATCH: &str = "Either all nodes should have 'app' declared, or none should, but not a mix of both.";

pub const ERR_MSG_APP_IS_EMPTY: &str = "the 'app' field can not be empty, remove the field if the graph is a single-app graph";

pub const ERR_MSG_APP_SHOULD_NOT_DECLARED: &str =
"the 'app' should not be declared, as not any node has declared it";

pub const ERR_MSG_APP_SHOULD_DECLARED: &str =
"the 'app' can not be none, as it has been declared in nodes";
Loading

0 comments on commit 96c6cae

Please sign in to comment.