diff --git a/databroker/src/viss/v2/server.rs b/databroker/src/viss/v2/server.rs index 465f4983..1af851f3 100644 --- a/databroker/src/viss/v2/server.rs +++ b/databroker/src/viss/v2/server.rs @@ -29,6 +29,7 @@ use tracing::warn; use crate::{ authorization::Authorization, broker::{self, AuthorizedAccess, UpdateError}, + glob::Matcher, permissions::{self, Permissions}, }; @@ -100,36 +101,141 @@ impl Viss for Server { request_id, metadata, })); - } - let permissions = resolve_permissions(&self.authorization, &request.authorization) - .map_err(|error| GetErrorResponse { - request_id: request_id.clone(), - error, - ts: SystemTime::now().into(), - })?; - let broker = self.broker.authorized_access(&permissions); + } else if let Some(Filter::Paths(paths_filter)) = &request.filter { + let request_path = request.path.as_ref(); + if request_path.contains('*') { + return Err(GetErrorResponse { + request_id, + ts: SystemTime::now().into(), + error: Error::NotFoundInvalidPath, + }); + } + + let permissions = resolve_permissions(&self.authorization, &request.authorization) + .map_err(|error| GetErrorResponse { + request_id: request_id.clone(), + error, + ts: SystemTime::now().into(), + })?; + let broker = self.broker.authorized_access(&permissions); + + let mut request_matcher: Vec<(Matcher, bool)> = Vec::new(); + let mut entries_data = Vec::new(); + let mut signal_errors = Vec::new(); + + for path in &paths_filter.parameter { + let new_path = format!("{}.{}", request_path, path); + if let Ok(matcher) = Matcher::new(&new_path) { + request_matcher.push((matcher, false)); + } + } + + if !request_matcher.is_empty() { + for (matcher, is_match) in &mut request_matcher { + broker + .for_each_entry(|entry| { + let glob_path = &entry.metadata().glob_path; + let path = entry.metadata().path.clone(); + if matcher.is_match(glob_path) { + match entry.datapoint() { + Ok(datapoint) => { + let dp = DataPoint::from(datapoint.clone()); + *is_match = true; + entries_data.push(DataObject { + path: Path::from(path), + dp, + }); + } + Err(_) => { + signal_errors.push(path); + } + } + } + }) + .await; + + // Not found any matches meaning it could be a branch path request + // Only support branches like Vehicle.Cabin.Sunroof but not like **.Sunroof + if !matcher.as_string().starts_with("**") + && !matcher.as_string().ends_with("/**") + && !(*is_match) + { + if let Ok(branch_matcher) = Matcher::new(&(matcher.as_string() + "/**")) { + broker + .for_each_entry(|entry| { + let glob_path = &entry.metadata().glob_path; + let path = entry.metadata().path.clone(); + if branch_matcher.is_match(glob_path) { + match entry.datapoint() { + Ok(datapoint) => { + let dp = DataPoint::from(datapoint.clone()); + *is_match = true; + entries_data.push(DataObject { + path: Path::from(path), + dp, + }); + } + Err(_) => { + signal_errors.push(path); + } + } + } + }) + .await; + } + } + } + } - // Get datapoints - match broker.get_datapoint_by_path(request.path.as_ref()).await { - Ok(datapoint) => { - let dp = DataPoint::from(datapoint); + // https://w3c.github.io/automotive/spec/VISSv2_Core.html#error-handling + if signal_errors.is_empty() { Ok(GetSuccessResponse::Data(DataResponse { request_id, - data: Data::Object(DataObject { - path: request.path, - dp, - }), + data: Data::Array(entries_data), })) + } else { + Err(GetErrorResponse { + request_id, + ts: SystemTime::now().into(), + error: Error::Forbidden { + msg: Some(format!( + "Permission denied for some signal: {}", + signal_errors.join(", ") + )), + }, + }) + } + } else { + let permissions = resolve_permissions(&self.authorization, &request.authorization) + .map_err(|error| GetErrorResponse { + request_id: request_id.clone(), + error, + ts: SystemTime::now().into(), + })?; + let broker = self.broker.authorized_access(&permissions); + + // Get datapoints + match broker.get_datapoint_by_path(request.path.as_ref()).await { + Ok(datapoint) => { + let dp = DataPoint::from(datapoint); + Ok(GetSuccessResponse::Data(DataResponse { + request_id, + data: Data::Object(DataObject { + path: request.path, + dp, + }), + })) + } + Err(err) => Err(GetErrorResponse { + request_id, + ts: SystemTime::now().into(), + error: match err { + broker::ReadError::NotFound => Error::NotFoundInvalidPath, + broker::ReadError::PermissionDenied => Error::Forbidden { msg: None }, + broker::ReadError::PermissionExpired => Error::UnauthorizedTokenExpired, + }, + }), } - Err(err) => Err(GetErrorResponse { - request_id, - ts: SystemTime::now().into(), - error: match err { - broker::ReadError::NotFound => Error::NotFoundInvalidPath, - broker::ReadError::PermissionDenied => Error::Forbidden, - broker::ReadError::PermissionExpired => Error::UnauthorizedTokenExpired, - }, - }), } } @@ -211,7 +317,7 @@ impl Viss for Server { UpdateError::UnsupportedType => Error::BadRequest { msg: Some("Unsupported data type.".into()), }, - UpdateError::PermissionDenied => Error::Forbidden, + UpdateError::PermissionDenied => Error::Forbidden { msg: None }, UpdateError::PermissionExpired => Error::UnauthorizedTokenExpired, } } else { diff --git a/databroker/src/viss/v2/types.rs b/databroker/src/viss/v2/types.rs index c0fe23d7..3f926d4e 100644 --- a/databroker/src/viss/v2/types.rs +++ b/databroker/src/viss/v2/types.rs @@ -196,6 +196,14 @@ pub struct GenericErrorResponse { pub enum Filter { #[serde(rename = "static-metadata")] StaticMetadata(StaticMetadataFilter), + #[serde(rename = "paths")] + Paths(PathsFilter), +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PathsFilter { + pub parameter: Vec, } #[derive(Deserialize)] @@ -389,7 +397,7 @@ pub enum Error { UnauthorizedTokenInvalid, UnauthorizedTokenMissing, UnauthorizedReadOnly, - Forbidden, + Forbidden { msg: Option }, NotFoundInvalidPath, NotFoundUnavailableData, NotFoundInvalidSubscriptionId, @@ -437,10 +445,10 @@ impl From for ErrorSpec { message: "The desired signal cannot be set since it is a read only signal.".into(), }, // Forbidden 403 user_forbidden The user is not permitted to access the requested resource. Retrying does not help. - Error::Forbidden => ErrorSpec { + Error::Forbidden{ msg: custom_msg } => ErrorSpec { number: 403, reason: "user_forbidden".into(), - message: "The user is not permitted to access the requested resource. Retrying does not help.".into(), + message: custom_msg.unwrap_or("The user is not permitted to access the requested resource. Retrying does not help.".into()), }, // Forbidden 403 user_unknown The user is unknown. Retrying does not help. // Forbidden 403 device_forbidden The device is not permitted to access the requested resource. Retrying does not help. diff --git a/integration_test/viss/features/viss_v2_multiple_paths.feature b/integration_test/viss/features/viss_v2_multiple_paths.feature index 57ba3473..c69a3aee 100644 --- a/integration_test/viss/features/viss_v2_multiple_paths.feature +++ b/integration_test/viss/features/viss_v2_multiple_paths.feature @@ -20,19 +20,19 @@ Feature: VISS v2 Compliance Testing - Multiple Paths @ShouldHave Scenario: Request for multiple values from a single node # This scenario can be expanded based on specific use cases. - When I send a read request with path "Vehicle.Cabin.TemperatureSetpoint" - Then I should receive multiple values from a single node + When I search "Vehicle.Cabin" using a path filter "Door.*.*.IsOpen" + Then I should receive multiple data points # 5.1.2 Read request - Single value request from multiple nodes # The VISS server must support reading values from multiple nodes using path filters. @MustHave Scenario: Request for a single value from multiple nodes When I search "Vehicle" using a path filter "*" - Then I should receive a single value from multiple nodes + Then I should receive multiple data points # 5.1.2 Read request - Multiple values from multiple nodes # The VISS server should support reading multiple values from multiple nodes. @ShouldHave - Scenario: Request for multiple values from multiple nodes - When I send a read request with path "Vehicle.*" - Then I should receive multiple values from multiple nodes + Scenario: Path request for multiple values must not contain any wildcards + When I search "Vehicle.*" using a path filter "*" + Then I should receive an error response diff --git a/integration_test/viss/features/viss_v2_transport_wss_filter.feature b/integration_test/viss/features/viss_v2_transport_wss_filter.feature index 36a9c3ea..2da4e298 100644 --- a/integration_test/viss/features/viss_v2_transport_wss_filter.feature +++ b/integration_test/viss/features/viss_v2_transport_wss_filter.feature @@ -33,7 +33,7 @@ Feature: VISS v2 Compliance Testing - Filter # The VISS server must support searching for multiple signals using a path filter (e.g., "*.IsOpen"). @MustHave Scenario: Search for multiple signals using path filter - When I search "Vehicle" using a path filter "*.IsOpen" + When I search "Vehicle" using a path filter "*.*.*.IsOpen" Then I should receive multiple data points # 5.5.1 Subscribe with change filter diff --git a/integration_test/viss/tests/test_steps/viss_v2_steps.py b/integration_test/viss/tests/test_steps/viss_v2_steps.py index 3fe691f6..3c78f90e 100644 --- a/integration_test/viss/tests/test_steps/viss_v2_steps.py +++ b/integration_test/viss/tests/test_steps/viss_v2_steps.py @@ -377,7 +377,7 @@ def receive_ws_read_multiple_datapoints(connected_clients,request_id): responses = connected_clients.find_messages(request_id=request_id) # TODO: Unpack envelope # TODO: Count number of valid "dp" items - actual_count = len(responses) + actual_count = len(responses[0]["body"]["data"]) assert actual_count > 1, f"Expected multiple messages but only got {actual_count}" @then("I should receive a single value from multiple nodes")