From 080db2fffddf2a3e8e4911ff7d969ab3118d27ec Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Tue, 17 Sep 2024 18:19:08 +0200 Subject: [PATCH 1/8] Testing Ruist --- data/vss-core/README.md | 2 +- databroker/src/grpc/kuksa_val_v2/val.rs | 153 +++++++++++++++++++++++- 2 files changed, 148 insertions(+), 7 deletions(-) diff --git a/data/vss-core/README.md b/data/vss-core/README.md index dbba2cff..73289905 100644 --- a/data/vss-core/README.md +++ b/data/vss-core/README.md @@ -60,7 +60,7 @@ use the full name. When official release is created replace the copied *.json-fi Build and run kuksa_databroker using the new VSS file according to [documentation](../../README.md), e.g. ```sh -$cargo run --bin databroker -- --metadata ../data/vss-core/vss_release_4.0.json +$cargo run --bin databroker -- --metadata ./data/vss-core/vss_release_4.0.json ``` Use the client to verify that changes in VSS are reflected, by doing e.g. set/get on some new or renamed signals. diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 95f2061b..db390007 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -14,7 +14,7 @@ use std::{collections::HashMap, pin::Pin}; use crate::{ - broker::{self, AuthorizedAccess, SubscriptionError}, + broker::{self, AuthorizedAccess, SubscriptionError, ReadError}, glob::Matcher, permissions::Permissions, }; @@ -37,20 +37,58 @@ const MAX_REQUEST_PATH_LENGTH: usize = 1000; #[tonic::async_trait] impl proto::val_server::Val for broker::DataBroker { + + /// Get the latest (current) value of a signal + /// + /// Returns (GRPC error code): + /// NOT_FOUND if the requested signal doesn't exist + /// PERMISSION_DENIED if access is denied async fn get_value( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, tonic::Status> { - Err(tonic::Status::new( - tonic::Code::Unimplemented, - "Unimplemented", - )) + + debug!(?request); + let permissions = match request.extensions().get::() { + Some(permissions) => { + debug!(?permissions); + permissions.clone() + } + None => return Err(tonic::Status::unauthenticated("Unauthenticated")), + }; + + let broker = self.authorized_access(&permissions); + + let request = request.into_inner(); + + let signal_id = match get_signal(request.signal_id, &broker).await { + Ok(signal_id) => signal_id, + Err(err) => return Err(err), + }; + + let datapoint = match broker.get_datapoint(signal_id).await { + Ok(datapoint) => datapoint, + Err(ReadError::NotFound) => + return Err(tonic::Status::new(tonic::Code::NotFound, "Path not found")), + Err(ReadError::PermissionDenied | ReadError::PermissionExpired) => + return Err(tonic::Status::new(tonic::Code::PermissionDenied, "Permission Denied")), + }; + + // TODO - if this works - refactor so that we can reuse the lookup snippet for the methods below + + Ok(tonic::Response::new(proto::GetValueResponse { + data_point: datapoint.into(), + })) + } async fn get_values( &self, _request: tonic::Request, ) -> Result, tonic::Status> { + + // Permissions + Err(tonic::Status::new( tonic::Code::Unimplemented, "Unimplemented", @@ -753,6 +791,109 @@ mod tests { } } + #[tokio::test] + async fn test_get_value_erik() { + let broker = DataBroker::default(); + let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); + let f = false; + + let entry_id = authorized_access + .add_entry( + "test.datapoint1".to_owned(), + broker::DataType::Int32, + broker::ChangeType::OnChange, + broker::EntryType::Sensor, + "Test datapoint 1".to_owned(), + None, + None, + ) + .await + .unwrap(); + + + //let timestamp = Some(std::time::SystemTime::now().into()); + let timestamp2= std::time::SystemTime::now(); + + let value = proto::Value { + typed_value: Some(proto::value::TypedValue::Int32(-64)), + }; + + let _ = authorized_access + .update_entries([( + entry_id, + broker::EntryUpdate { + path: None, + datapoint: Some(broker::Datapoint { + //ts: std::time::SystemTime::now(), + ts: timestamp2, + source_ts: None, + value: broker::types::DataValue::Int32(-64), + }), + actuator_target: None, + entry_type: None, + data_type: None, + description: None, + allowed: None, + unit: None, + }, + )]) + .await; + + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Id(entry_id)), + }), + } + ; + + + // Manually insert permissions + let mut get_value_request = tonic::Request::new(request); + get_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + + + match broker.get_value(get_value_request).await { + Ok(response) => { + // Handle the successful response + let get_response = response.into_inner(); + // TODO : Which is preferred - just checking value + match get_response.data_point.clone().unwrap().value_state { + Some(proto::datapoint::ValueState::Value(value)) => { + assert_eq!(value.typed_value.unwrap(), proto::value::TypedValue::Int32(-64)); + } + Some(proto::datapoint::ValueState::Failure(_failure)) => { + // TODO: When do we expect a failure + assert!(f, "Did not expect failure"); + } + None => { + // Handle the error from the publish_value function + assert!(f, "Expected a value"); + } + } + // TODO : Which is preferred - compare response as such + assert_eq!(get_response, proto::GetValueResponse { + + data_point: { + + Some(proto::Datapoint { + timestamp : Some(timestamp2.into()), + value_state: Some(proto::datapoint::ValueState::Value(value)), + }) + }, + + }); + } + Err(status) => { + // Handle the error from the publish_value function + assert!(f, "Get failed with status: {:?}", status); + } + } + } + #[tokio::test] async fn test_publish_value_signal_id_not_found() { let broker = DataBroker::default(); From c96076e7037c4e63a4070cdf72f36ab702912ce5 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 09:16:47 +0200 Subject: [PATCH 2/8] Increment 2 --- .../src/grpc/kuksa_val_v2/conversions.rs | 6 - databroker/src/grpc/kuksa_val_v2/val.rs | 123 +++++++++--------- proto/kuksa/val/v2/types.proto | 26 +++- proto/kuksa/val/v2/val.proto | 6 +- 4 files changed, 86 insertions(+), 75 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/conversions.rs b/databroker/src/grpc/kuksa_val_v2/conversions.rs index 44a990cc..47e9b046 100644 --- a/databroker/src/grpc/kuksa_val_v2/conversions.rs +++ b/databroker/src/grpc/kuksa_val_v2/conversions.rs @@ -196,10 +196,7 @@ impl From<&i32> for broker::ValueFailure { fn from_i32(value: i32) -> proto::ValueFailure { // Use a match statement to convert the i32 to the corresponding enum variant match value { - 1 => proto::ValueFailure::InvalidValue, 2 => proto::ValueFailure::NotProvided, - 3 => proto::ValueFailure::UnknownSignal, - 4 => proto::ValueFailure::AccessDenied, 5 => proto::ValueFailure::InternalError, _ => proto::ValueFailure::Unspecified, } @@ -209,10 +206,7 @@ impl From<&proto::ValueFailure> for broker::ValueFailure { fn from(value_failure: &proto::ValueFailure) -> Self { match value_failure { proto::ValueFailure::Unspecified => broker::ValueFailure::Unspecified, - proto::ValueFailure::InvalidValue => broker::ValueFailure::InvalidValue, proto::ValueFailure::NotProvided => broker::ValueFailure::NotProvided, - proto::ValueFailure::UnknownSignal => broker::ValueFailure::UnknownSignal, - proto::ValueFailure::AccessDenied => broker::ValueFailure::AccessDenied, proto::ValueFailure::InternalError => broker::ValueFailure::InternalError, } } diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index db390007..36e29f36 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -14,7 +14,7 @@ use std::{collections::HashMap, pin::Pin}; use crate::{ - broker::{self, AuthorizedAccess, SubscriptionError, ReadError}, + broker::{self, AuthorizedAccess, ReadError, SubscriptionError}, glob::Matcher, permissions::Permissions, }; @@ -37,7 +37,6 @@ const MAX_REQUEST_PATH_LENGTH: usize = 1000; #[tonic::async_trait] impl proto::val_server::Val for broker::DataBroker { - /// Get the latest (current) value of a signal /// /// Returns (GRPC error code): @@ -47,7 +46,6 @@ impl proto::val_server::Val for broker::DataBroker { &self, request: tonic::Request, ) -> Result, tonic::Status> { - debug!(?request); let permissions = match request.extensions().get::() { Some(permissions) => { @@ -58,7 +56,7 @@ impl proto::val_server::Val for broker::DataBroker { }; let broker = self.authorized_access(&permissions); - + let request = request.into_inner(); let signal_id = match get_signal(request.signal_id, &broker).await { @@ -68,25 +66,29 @@ impl proto::val_server::Val for broker::DataBroker { let datapoint = match broker.get_datapoint(signal_id).await { Ok(datapoint) => datapoint, - Err(ReadError::NotFound) => - return Err(tonic::Status::new(tonic::Code::NotFound, "Path not found")), - Err(ReadError::PermissionDenied | ReadError::PermissionExpired) => - return Err(tonic::Status::new(tonic::Code::PermissionDenied, "Permission Denied")), + Err(ReadError::NotFound) => { + return Err(tonic::Status::new(tonic::Code::NotFound, "Path not found")) + } + Err(ReadError::PermissionDenied | ReadError::PermissionExpired) => { + return Err(tonic::Status::new( + tonic::Code::PermissionDenied, + "Permission Denied", + )) + } }; // TODO - if this works - refactor so that we can reuse the lookup snippet for the methods below - + // TODO Check what happens if NOT available + // DataValue::NotAvailable Ok(tonic::Response::new(proto::GetValueResponse { data_point: datapoint.into(), })) - } async fn get_values( &self, _request: tonic::Request, ) -> Result, tonic::Status> { - // Permissions Err(tonic::Status::new( @@ -810,82 +812,79 @@ mod tests { .await .unwrap(); - //let timestamp = Some(std::time::SystemTime::now().into()); - let timestamp2= std::time::SystemTime::now(); + let timestamp2 = std::time::SystemTime::now(); let value = proto::Value { typed_value: Some(proto::value::TypedValue::Int32(-64)), }; - let _ = authorized_access - .update_entries([( - entry_id, - broker::EntryUpdate { - path: None, - datapoint: Some(broker::Datapoint { - //ts: std::time::SystemTime::now(), - ts: timestamp2, - source_ts: None, - value: broker::types::DataValue::Int32(-64), - }), - actuator_target: None, - entry_type: None, - data_type: None, - description: None, - allowed: None, - unit: None, - }, - )]) - .await; - + let _ = authorized_access + .update_entries([( + entry_id, + broker::EntryUpdate { + path: None, + datapoint: Some(broker::Datapoint { + //ts: std::time::SystemTime::now(), + ts: timestamp2, + source_ts: None, + value: broker::types::DataValue::Int32(-64), + }), + actuator_target: None, + entry_type: None, + data_type: None, + description: None, + allowed: None, + unit: None, + }, + )]) + .await; let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { signal: Some(proto::signal_id::Signal::Id(entry_id)), - }), - } - ; - + }), + }; // Manually insert permissions let mut get_value_request = tonic::Request::new(request); get_value_request - .extensions_mut() - .insert(permissions::ALLOW_ALL.clone()); - - + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); match broker.get_value(get_value_request).await { Ok(response) => { // Handle the successful response let get_response = response.into_inner(); - // TODO : Which is preferred - just checking value + // TODO : Which is preferred - just checking value match get_response.data_point.clone().unwrap().value_state { - Some(proto::datapoint::ValueState::Value(value)) => { - assert_eq!(value.typed_value.unwrap(), proto::value::TypedValue::Int32(-64)); - } - Some(proto::datapoint::ValueState::Failure(_failure)) => { + Some(proto::datapoint::ValueState::Value(value)) => { + assert_eq!( + value.typed_value.unwrap(), + proto::value::TypedValue::Int32(-64) + ); + } + Some(proto::datapoint::ValueState::Failure(_failure)) => { // TODO: When do we expect a failure assert!(f, "Did not expect failure"); - } + } None => { - // Handle the error from the publish_value function - assert!(f, "Expected a value"); - } + // Handle the error from the publish_value function + assert!(f, "Expected a value"); } + } // TODO : Which is preferred - compare response as such - assert_eq!(get_response, proto::GetValueResponse { - - data_point: { - - Some(proto::Datapoint { - timestamp : Some(timestamp2.into()), - value_state: Some(proto::datapoint::ValueState::Value(value)), - }) - }, - - }); + assert_eq!( + get_response, + proto::GetValueResponse { + data_point: { + Some(proto::Datapoint { + timestamp: Some(timestamp2.into()), + value_state: Some(proto::datapoint::ValueState::Value(value)), + }) + }, + } + ); } Err(status) => { // Handle the error from the publish_value function diff --git a/proto/kuksa/val/v2/types.proto b/proto/kuksa/val/v2/types.proto index 9adc6053..46a46881 100644 --- a/proto/kuksa/val/v2/types.proto +++ b/proto/kuksa/val/v2/types.proto @@ -22,7 +22,12 @@ message Datapoint { google.protobuf.Timestamp timestamp = 1; oneof value_state { + // When reading - returned if the signal exists but has no value, + // or some other error prevents a value from being provided + // When writing - could theoretically be used to "delete" a value ValueFailure failure = 2; + // When reading - this is returned if Databroker has an actual value for the signal + // When writing - this shall be used to provide a value to databroker Value value = 3; } } @@ -50,7 +55,14 @@ message Value { message SignalID { oneof signal { + // Numeric identifier to the signal + // As of today Databroker assigns arbitrary unique numbers to each registered signal + // at startup, meaning that identifiers may change after restarting Databroker. + // A mechanism for static identifiers may be introduced in the future. int32 id = 1; + // Full VSS-style path to a specific signal, like "Vehicle.Speed" + // Wildcards and paths to branches are not supported. + // The given path must be known by the Databroker. string path = 2; } } @@ -147,18 +159,20 @@ message ValueRestrictionString { repeated string allowed_values = 1; } +// Used to indicate status of a signal in Databroker +// This is given as an alternative to Value, so if there is a valid value +// ValueFailure shall not be specified. +// +// Scenarios where a signal does not exist or a user does not have access to a signal +// shall typically not be represented as a ValueFailure, as those secnarios +// should result in a GRPC error code being returned to the caller. enum ValueFailure { // Unspecified value failure, reserved for gRPC backwards compatibility // (see https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum) UNSPECIFIED = 0; - // The signal is known and provided, but doesn't have a valid value - INVALID_VALUE = 1; // The signal is known, but no value is provided currently + // Can theoretically also be used in write request to "delete" a value NOT_PROVIDED = 2; - // The referred signal is unknown on the system - UNKNOWN_SIGNAL = 3; - // The client does not have the necessary access rights to the signal - ACCESS_DENIED = 4; // Unexpected internal error INTERNAL_ERROR = 5; } diff --git a/proto/kuksa/val/v2/val.proto b/proto/kuksa/val/v2/val.proto index 67d1a86f..b33a5fac 100644 --- a/proto/kuksa/val/v2/val.proto +++ b/proto/kuksa/val/v2/val.proto @@ -25,6 +25,10 @@ service VAL { // Returns (GRPC error code): // NOT_FOUND if the requested signal doesn't exist // PERMISSION_DENIED if access is denied + // + // If the signal exist but does not have a valid value + // a ValueFailure datapoint will be returned. + // rpc GetValue(GetValueRequest) returns (GetValueResponse); // Get the latest values of a set of signals. @@ -41,7 +45,7 @@ service VAL { // is allowed to read are included (everything else is ignored). // // Returns (GRPC error code): - // NOT_FOUND if the specified root branch does not exist. + // NOT_FOUND if any of the requested signals doesn't exist. rpc ListValues(ListValuesRequest) returns (ListValuesResponse); // Subscribe to a set of signals using string path parameters From cb7402195a58d1c9b223874c7cbf5cd04d54618c Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 10:57:15 +0200 Subject: [PATCH 3/8] Increment 3 --- databroker/src/grpc/kuksa_val_v2/val.rs | 158 +++++++++++++++++++++++- 1 file changed, 154 insertions(+), 4 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 36e29f36..77344ada 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -793,15 +793,59 @@ mod tests { } } + // Helper for adding an int32 siignal and adding value + async fn helper_add_int32(name: &str, value: i32, timestamp: std::time::SystemTime) -> i32 { + let broker = DataBroker::default(); + let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); + + let entry_id = authorized_access + .add_entry( + name.to_owned(), + broker::DataType::Int32, + broker::ChangeType::OnChange, + broker::EntryType::Sensor, + "Some Description hat Does Not Matter".to_owned(), + None, + None, + ) + .await + .unwrap(); + + + + let _ = authorized_access + .update_entries([( + entry_id, + broker::EntryUpdate { + path: None, + datapoint: Some(broker::Datapoint { + //ts: std::time::SystemTime::now(), + ts: timestamp, + source_ts: None, + value: broker::types::DataValue::Int32(value), + }), + actuator_target: None, + entry_type: None, + data_type: None, + description: None, + allowed: None, + unit: None, + }, + )]) + .await; + + return entry_id; + } + #[tokio::test] - async fn test_get_value_erik() { + async fn test_get_value_id() { let broker = DataBroker::default(); let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); let f = false; - let entry_id = authorized_access + let entry_id2 = authorized_access .add_entry( - "test.datapoint1".to_owned(), + "test.datapoint1b".to_owned(), broker::DataType::Int32, broker::ChangeType::OnChange, broker::EntryType::Sensor, @@ -821,7 +865,111 @@ mod tests { let _ = authorized_access .update_entries([( - entry_id, + entry_id2, + broker::EntryUpdate { + path: None, + datapoint: Some(broker::Datapoint { + //ts: std::time::SystemTime::now(), + ts: timestamp2, + source_ts: None, + value: broker::types::DataValue::Int32(-64), + }), + actuator_target: None, + entry_type: None, + data_type: None, + description: None, + allowed: None, + unit: None, + }, + )]) + .await; + + let entry_id = helper_add_int32("test.datapoint1", -64, timestamp2).await; + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Id(entry_id)), + }), + }; + + // Manually insert permissions + let mut get_value_request = tonic::Request::new(request); + get_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + match broker.get_value(get_value_request).await { + Ok(response) => { + // Handle the successful response + let get_response = response.into_inner(); + // TODO : Which is preferred - just checking value + match get_response.data_point.clone().unwrap().value_state { + Some(proto::datapoint::ValueState::Value(value)) => { + assert_eq!( + value.typed_value.unwrap(), + proto::value::TypedValue::Int32(-64) + ); + } + Some(proto::datapoint::ValueState::Failure(_failure)) => { + // TODO: When do we expect a failure + assert!(f, "Did not expect failure"); + } + None => { + // Handle the error from the publish_value function + assert!(f, "Expected a value"); + } + } + // TODO : Which is preferred - compare response as such + assert_eq!( + get_response, + proto::GetValueResponse { + data_point: { + Some(proto::Datapoint { + timestamp: Some(timestamp2.into()), + value_state: Some(proto::datapoint::ValueState::Value(value)), + }) + }, + } + ); + } + Err(status) => { + // Handle the error from the publish_value function + assert!(f, "Get failed with status: {:?}", status); + } + } + } + + + #[tokio::test] + // Later to be removed + async fn test_get_value_id_obsolete() { + let broker = DataBroker::default(); + let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); + let f = false; + + let entry_id2 = authorized_access + .add_entry( + "test.datapoint1b".to_owned(), + broker::DataType::Int32, + broker::ChangeType::OnChange, + broker::EntryType::Sensor, + "Test datapoint 1".to_owned(), + None, + None, + ) + .await + .unwrap(); + + //let timestamp = Some(std::time::SystemTime::now().into()); + let timestamp2 = std::time::SystemTime::now(); + + let value = proto::Value { + typed_value: Some(proto::value::TypedValue::Int32(-64)), + }; + + let _ = authorized_access + .update_entries([( + entry_id2, broker::EntryUpdate { path: None, datapoint: Some(broker::Datapoint { @@ -840,6 +988,8 @@ mod tests { )]) .await; + let entry_id = helper_add_int32("test.datapoint1", -64, timestamp2).await; + let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { signal: Some(proto::signal_id::Signal::Id(entry_id)), From 5ba49623f0cae42913b283353dff54ef03f43741 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 13:06:36 +0200 Subject: [PATCH 4/8] Increment 4 --- .../src/grpc/kuksa_val_v2/conversions.rs | 5 +- databroker/src/grpc/kuksa_val_v2/val.rs | 177 ++++++++---------- proto/kuksa/val/v2/val.proto | 6 + 3 files changed, 86 insertions(+), 102 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/conversions.rs b/databroker/src/grpc/kuksa_val_v2/conversions.rs index 47e9b046..8fc0f971 100644 --- a/databroker/src/grpc/kuksa_val_v2/conversions.rs +++ b/databroker/src/grpc/kuksa_val_v2/conversions.rs @@ -46,7 +46,10 @@ impl From<&proto::Datapoint> for broker::Datapoint { impl From for Option { fn from(from: broker::Datapoint) -> Self { match from.value { - broker::DataValue::NotAvailable => None, + broker::DataValue::NotAvailable => Some(proto::Datapoint { + value_state: Some(proto::datapoint::ValueState::Failure(proto::ValueFailure::NotProvided.into())), + timestamp: None + }), broker::DataValue::Bool(value) => Some(proto::Datapoint { value_state: Some(proto::datapoint::ValueState::Value(proto::Value { typed_value: Some(proto::value::TypedValue::Bool(value)), diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 77344ada..1d0271d1 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -672,7 +672,14 @@ fn convert_to_proto_stream( let mut entries: HashMap = HashMap::with_capacity(size); for update in item.updates { let update_datapoint: Option = match update.update.datapoint { - Some(datapoint) => datapoint.into(), + Some(datapoint) => { + // For subscribe streams we do not want to return NotVailable + // even if the values is not available when subscribe starts + match datapoint.value { + broker::DataValue::NotAvailable => None, + _ => datapoint.into(), + } + }, None => None, }; if let Some(dp) = update_datapoint { @@ -793,11 +800,10 @@ mod tests { } } - // Helper for adding an int32 siignal and adding value - async fn helper_add_int32(name: &str, value: i32, timestamp: std::time::SystemTime) -> i32 { - let broker = DataBroker::default(); + // Helper for adding an int32 signal and adding value + async fn helper_add_int32(broker : &DataBroker, name: &str, value: i32, timestamp: std::time::SystemTime) -> i32 { + let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); - let entry_id = authorized_access .add_entry( name.to_owned(), @@ -811,8 +817,6 @@ mod tests { .await .unwrap(); - - let _ = authorized_access .update_entries([( entry_id, @@ -840,51 +844,13 @@ mod tests { #[tokio::test] async fn test_get_value_id() { let broker = DataBroker::default(); - let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); - let f = false; - - let entry_id2 = authorized_access - .add_entry( - "test.datapoint1b".to_owned(), - broker::DataType::Int32, - broker::ChangeType::OnChange, - broker::EntryType::Sensor, - "Test datapoint 1".to_owned(), - None, - None, - ) - .await - .unwrap(); - //let timestamp = Some(std::time::SystemTime::now().into()); - let timestamp2 = std::time::SystemTime::now(); + + let timestamp = std::time::SystemTime::now(); - let value = proto::Value { - typed_value: Some(proto::value::TypedValue::Int32(-64)), - }; - let _ = authorized_access - .update_entries([( - entry_id2, - broker::EntryUpdate { - path: None, - datapoint: Some(broker::Datapoint { - //ts: std::time::SystemTime::now(), - ts: timestamp2, - source_ts: None, - value: broker::types::DataValue::Int32(-64), - }), - actuator_target: None, - entry_type: None, - data_type: None, - description: None, - allowed: None, - unit: None, - }, - )]) - .await; - let entry_id = helper_add_int32("test.datapoint1", -64, timestamp2).await; + let entry_id = helper_add_int32(&broker, "test.datapoint1", -64, timestamp).await; let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { @@ -912,20 +878,24 @@ mod tests { } Some(proto::datapoint::ValueState::Failure(_failure)) => { // TODO: When do we expect a failure - assert!(f, "Did not expect failure"); + assert!(false, "Did not expect failure"); } None => { // Handle the error from the publish_value function - assert!(f, "Expected a value"); + assert!(false, "Expected a value"); } } // TODO : Which is preferred - compare response as such + + let value = proto::Value { + typed_value: Some(proto::value::TypedValue::Int32(-64)), + }; assert_eq!( get_response, proto::GetValueResponse { data_point: { Some(proto::Datapoint { - timestamp: Some(timestamp2.into()), + timestamp: Some(timestamp.into()), value_state: Some(proto::datapoint::ValueState::Value(value)), }) }, @@ -934,61 +904,33 @@ mod tests { } Err(status) => { // Handle the error from the publish_value function - assert!(f, "Get failed with status: {:?}", status); + assert!(false, "Get failed with status: {:?}", status); } } } - #[tokio::test] - // Later to be removed - async fn test_get_value_id_obsolete() { + async fn test_get_value_id_no_value() { + + // Define signal but do not assign any value + + let broker = DataBroker::default(); let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); - let f = false; - - let entry_id2 = authorized_access + let entry_id = authorized_access .add_entry( - "test.datapoint1b".to_owned(), + "test.datapoint1".to_string(), broker::DataType::Int32, broker::ChangeType::OnChange, broker::EntryType::Sensor, - "Test datapoint 1".to_owned(), + "Some Description hat Does Not Matter".to_owned(), None, None, ) .await .unwrap(); - //let timestamp = Some(std::time::SystemTime::now().into()); - let timestamp2 = std::time::SystemTime::now(); - - let value = proto::Value { - typed_value: Some(proto::value::TypedValue::Int32(-64)), - }; - - let _ = authorized_access - .update_entries([( - entry_id2, - broker::EntryUpdate { - path: None, - datapoint: Some(broker::Datapoint { - //ts: std::time::SystemTime::now(), - ts: timestamp2, - source_ts: None, - value: broker::types::DataValue::Int32(-64), - }), - actuator_target: None, - entry_type: None, - data_type: None, - description: None, - allowed: None, - unit: None, - }, - )]) - .await; - - let entry_id = helper_add_int32("test.datapoint1", -64, timestamp2).await; + // Now try to get it let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { @@ -1008,29 +950,27 @@ mod tests { let get_response = response.into_inner(); // TODO : Which is preferred - just checking value match get_response.data_point.clone().unwrap().value_state { - Some(proto::datapoint::ValueState::Value(value)) => { - assert_eq!( - value.typed_value.unwrap(), - proto::value::TypedValue::Int32(-64) - ); + Some(proto::datapoint::ValueState::Value(_value)) => { + assert!(false, "Did not expect success"); } - Some(proto::datapoint::ValueState::Failure(_failure)) => { - // TODO: When do we expect a failure - assert!(f, "Did not expect failure"); + Some(proto::datapoint::ValueState::Failure(failure)) => { + let res:i32 = proto::ValueFailure::NotProvided.into(); + assert_eq!(failure, res); } None => { // Handle the error from the publish_value function - assert!(f, "Expected a value"); + assert!(false, "Did not expect this error"); } } // TODO : Which is preferred - compare response as such + assert_eq!( get_response, proto::GetValueResponse { data_point: { Some(proto::Datapoint { - timestamp: Some(timestamp2.into()), - value_state: Some(proto::datapoint::ValueState::Value(value)), + timestamp: None, + value_state: Some(proto::datapoint::ValueState::Failure(proto::ValueFailure::NotProvided.into())), }) }, } @@ -1038,7 +978,40 @@ mod tests { } Err(status) => { // Handle the error from the publish_value function - assert!(f, "Get failed with status: {:?}", status); + assert!(false, "Get failed with status: {:?}", status); + } + } + } + + + #[tokio::test] + async fn test_get_value_id_not_defined() { + + + let broker = DataBroker::default(); + // Just use some arbitrary number + let entry_id: i32 = 12345; + + // Now try to get it + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Id(entry_id)), + }), + }; + + // Manually insert permissions + let mut get_value_request = tonic::Request::new(request); + get_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + match broker.get_value(get_value_request).await { + Ok(_response) => { + assert!(false, "Did not expect success"); + } + Err(status) => { + assert_eq!(status.code(), tonic::Code::NotFound) } } } @@ -1129,6 +1102,8 @@ mod tests { .extensions_mut() .insert(permissions::ALLOW_ALL.clone()); + // Note: We subscribe before the signal test.datapoint1 has any value + // but we do not expect to get a NOT_AVAILABLE message back! let result = tokio::task::block_in_place(|| { // Blocking operation here // Since broker.subscribe is async, you need to run it in an executor diff --git a/proto/kuksa/val/v2/val.proto b/proto/kuksa/val/v2/val.proto index b33a5fac..4e603e28 100644 --- a/proto/kuksa/val/v2/val.proto +++ b/proto/kuksa/val/v2/val.proto @@ -52,6 +52,12 @@ service VAL { // Returns (GRPC error code): // NOT_FOUND if any of the signals are non-existant. // PERMISSION_DENIED if access is denied for any of the signals. + // + // When subscribing the Broker shall immediately return the value for all + // subscribed entries. If no value is available when subscribing a value shall be returned + // first when a value is available. + // TODO - Verify logic above? + // If we have the availability to "delete" values then getting a NotAvailable message sort of makes sense rpc Subscribe(SubscribeRequest) returns (stream SubscribeResponse); // Subscribe to a set of signals using i32 id parameters From 87bbd2bd3e51cbf48cbbd439b1710b87ed272ba5 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 13:55:30 +0200 Subject: [PATCH 5/8] Increment 5 --- .../src/grpc/kuksa_val_v2/conversions.rs | 6 +- databroker/src/grpc/kuksa_val_v2/val.rs | 274 +++++++++++------- 2 files changed, 175 insertions(+), 105 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/conversions.rs b/databroker/src/grpc/kuksa_val_v2/conversions.rs index 8fc0f971..874f7181 100644 --- a/databroker/src/grpc/kuksa_val_v2/conversions.rs +++ b/databroker/src/grpc/kuksa_val_v2/conversions.rs @@ -47,8 +47,10 @@ impl From for Option { fn from(from: broker::Datapoint) -> Self { match from.value { broker::DataValue::NotAvailable => Some(proto::Datapoint { - value_state: Some(proto::datapoint::ValueState::Failure(proto::ValueFailure::NotProvided.into())), - timestamp: None + value_state: Some(proto::datapoint::ValueState::Failure( + proto::ValueFailure::NotProvided.into(), + )), + timestamp: None, }), broker::DataValue::Bool(value) => Some(proto::Datapoint { value_state: Some(proto::datapoint::ValueState::Value(proto::Value { diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 1d0271d1..64652efd 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -679,7 +679,7 @@ fn convert_to_proto_stream( broker::DataValue::NotAvailable => None, _ => datapoint.into(), } - }, + } None => None, }; if let Some(dp) = update_datapoint { @@ -744,65 +744,14 @@ mod tests { } } - #[tokio::test] - async fn test_publish_value() { - let broker = DataBroker::default(); - let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); - let f = false; - - let entry_id = authorized_access - .add_entry( - "test.datapoint1".to_owned(), - broker::DataType::Bool, - broker::ChangeType::OnChange, - broker::EntryType::Sensor, - "Test datapoint 1".to_owned(), - None, - None, - ) - .await - .unwrap(); - - let request = proto::PublishValueRequest { - signal_id: Some(proto::SignalId { - signal: Some(proto::signal_id::Signal::Id(entry_id)), - }), - data_point: { - let timestamp = Some(std::time::SystemTime::now().into()); - - let value = proto::Value { - typed_value: Some(proto::value::TypedValue::Bool(true)), - }; - - Some(proto::Datapoint { - timestamp, - value_state: Some(proto::datapoint::ValueState::Value(value)), - }) - }, - }; - - // Manually insert permissions - let mut publish_value_request = tonic::Request::new(request); - publish_value_request - .extensions_mut() - .insert(permissions::ALLOW_ALL.clone()); - - match broker.publish_value(publish_value_request).await { - Ok(response) => { - // Handle the successful response - let publish_response = response.into_inner(); - assert_eq!(publish_response, proto::PublishValueResponse {}) - } - Err(status) => { - // Handle the error from the publish_value function - assert!(f, "Publish failed with status: {:?}", status); - } - } - } // Helper for adding an int32 signal and adding value - async fn helper_add_int32(broker : &DataBroker, name: &str, value: i32, timestamp: std::time::SystemTime) -> i32 { - + async fn helper_add_int32( + broker: &DataBroker, + name: &str, + value: i32, + timestamp: std::time::SystemTime, + ) -> i32 { let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); let entry_id = authorized_access .add_entry( @@ -842,14 +791,11 @@ mod tests { } #[tokio::test] - async fn test_get_value_id() { + async fn test_get_value_id_ok() { let broker = DataBroker::default(); - let timestamp = std::time::SystemTime::now(); - - let entry_id = helper_add_int32(&broker, "test.datapoint1", -64, timestamp).await; let request = proto::GetValueRequest { @@ -868,27 +814,56 @@ mod tests { Ok(response) => { // Handle the successful response let get_response = response.into_inner(); - // TODO : Which is preferred - just checking value - match get_response.data_point.clone().unwrap().value_state { - Some(proto::datapoint::ValueState::Value(value)) => { - assert_eq!( - value.typed_value.unwrap(), - proto::value::TypedValue::Int32(-64) - ); - } - Some(proto::datapoint::ValueState::Failure(_failure)) => { - // TODO: When do we expect a failure - assert!(false, "Did not expect failure"); - } - None => { - // Handle the error from the publish_value function - assert!(false, "Expected a value"); + + let value = proto::Value { + typed_value: Some(proto::value::TypedValue::Int32(-64)), + }; + assert_eq!( + get_response, + proto::GetValueResponse { + data_point: { + Some(proto::Datapoint { + timestamp: Some(timestamp.into()), + value_state: Some(proto::datapoint::ValueState::Value(value)), + }) + }, } - } - // TODO : Which is preferred - compare response as such + ); + } + Err(status) => { + // Handle the error from the publish_value function + assert!(false, "Get failed with status: {:?}", status); + } + } + } + + #[tokio::test] + async fn test_get_value_name_ok() { + let broker = DataBroker::default(); + + let timestamp = std::time::SystemTime::now(); + + let _entry_id = helper_add_int32(&broker, "test.datapoint1", -64, timestamp).await; + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Path("test.datapoint1".to_string())), + }), + }; + + // Manually insert permissions + let mut get_value_request = tonic::Request::new(request); + get_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + match broker.get_value(get_value_request).await { + Ok(response) => { + // Handle the successful response + let get_response = response.into_inner(); let value = proto::Value { - typed_value: Some(proto::value::TypedValue::Int32(-64)), + typed_value: Some(proto::value::TypedValue::Int32(-64)), }; assert_eq!( get_response, @@ -910,11 +885,36 @@ mod tests { } #[tokio::test] - async fn test_get_value_id_no_value() { + async fn test_get_value_id_not_authorized() { + let broker = DataBroker::default(); + + let timestamp = std::time::SystemTime::now(); + let entry_id = helper_add_int32(&broker, "test.datapoint1", -64, timestamp).await; + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Id(entry_id)), + }), + }; + + // Do not insert permissions + let get_value_request = tonic::Request::new(request); + + match broker.get_value(get_value_request).await { + Ok(_response) => { + assert!(false, "Did not expect success"); + } + Err(status) => { + assert_eq!(status.code(), tonic::Code::Unauthenticated) + } + } + } + + #[tokio::test] + async fn test_get_value_id_no_value() { // Define signal but do not assign any value - let broker = DataBroker::default(); let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); let entry_id = authorized_access @@ -930,7 +930,7 @@ mod tests { .await .unwrap(); - // Now try to get it + // Now try to get it let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { @@ -948,21 +948,6 @@ mod tests { Ok(response) => { // Handle the successful response let get_response = response.into_inner(); - // TODO : Which is preferred - just checking value - match get_response.data_point.clone().unwrap().value_state { - Some(proto::datapoint::ValueState::Value(_value)) => { - assert!(false, "Did not expect success"); - } - Some(proto::datapoint::ValueState::Failure(failure)) => { - let res:i32 = proto::ValueFailure::NotProvided.into(); - assert_eq!(failure, res); - } - None => { - // Handle the error from the publish_value function - assert!(false, "Did not expect this error"); - } - } - // TODO : Which is preferred - compare response as such assert_eq!( get_response, @@ -970,7 +955,9 @@ mod tests { data_point: { Some(proto::Datapoint { timestamp: None, - value_state: Some(proto::datapoint::ValueState::Failure(proto::ValueFailure::NotProvided.into())), + value_state: Some(proto::datapoint::ValueState::Failure( + proto::ValueFailure::NotProvided.into(), + )), }) }, } @@ -983,16 +970,13 @@ mod tests { } } - #[tokio::test] async fn test_get_value_id_not_defined() { - - let broker = DataBroker::default(); // Just use some arbitrary number let entry_id: i32 = 12345; - // Now try to get it + // Now try to get it let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { @@ -1016,6 +1000,90 @@ mod tests { } } + #[tokio::test] + async fn test_get_value_name_not_defined() { + let broker = DataBroker::default(); + + // Now try to get it + + let request = proto::GetValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Path("test.datapoint1".to_string())), + }), + }; + + // Manually insert permissions + let mut get_value_request = tonic::Request::new(request); + get_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + match broker.get_value(get_value_request).await { + Ok(_response) => { + assert!(false, "Did not expect success"); + } + Err(status) => { + assert_eq!(status.code(), tonic::Code::NotFound) + } + } + } + + #[tokio::test] + async fn test_publish_value() { + let broker = DataBroker::default(); + let authorized_access = broker.authorized_access(&permissions::ALLOW_ALL); + let f = false; + + let entry_id = authorized_access + .add_entry( + "test.datapoint1".to_owned(), + broker::DataType::Bool, + broker::ChangeType::OnChange, + broker::EntryType::Sensor, + "Test datapoint 1".to_owned(), + None, + None, + ) + .await + .unwrap(); + + let request = proto::PublishValueRequest { + signal_id: Some(proto::SignalId { + signal: Some(proto::signal_id::Signal::Id(entry_id)), + }), + data_point: { + let timestamp = Some(std::time::SystemTime::now().into()); + + let value = proto::Value { + typed_value: Some(proto::value::TypedValue::Bool(true)), + }; + + Some(proto::Datapoint { + timestamp, + value_state: Some(proto::datapoint::ValueState::Value(value)), + }) + }, + }; + + // Manually insert permissions + let mut publish_value_request = tonic::Request::new(request); + publish_value_request + .extensions_mut() + .insert(permissions::ALLOW_ALL.clone()); + + match broker.publish_value(publish_value_request).await { + Ok(response) => { + // Handle the successful response + let publish_response = response.into_inner(); + assert_eq!(publish_response, proto::PublishValueResponse {}) + } + Err(status) => { + // Handle the error from the publish_value function + assert!(f, "Publish failed with status: {:?}", status); + } + } + } + #[tokio::test] async fn test_publish_value_signal_id_not_found() { let broker = DataBroker::default(); From b168f020c96745ae6aa96d6a4e51fdadfbb6a968 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 14:03:03 +0200 Subject: [PATCH 6/8] Incr 6 --- databroker/src/grpc/kuksa_val_v2/val.rs | 11 +++++++---- proto/kuksa/val/v2/types.proto | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 64652efd..6bf008b8 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -744,7 +744,6 @@ mod tests { } } - // Helper for adding an int32 signal and adding value async fn helper_add_int32( broker: &DataBroker, @@ -847,7 +846,9 @@ mod tests { let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { - signal: Some(proto::signal_id::Signal::Path("test.datapoint1".to_string())), + signal: Some(proto::signal_id::Signal::Path( + "test.datapoint1".to_string(), + )), }), }; @@ -1008,7 +1009,9 @@ mod tests { let request = proto::GetValueRequest { signal_id: Some(proto::SignalId { - signal: Some(proto::signal_id::Signal::Path("test.datapoint1".to_string())), + signal: Some(proto::signal_id::Signal::Path( + "test.datapoint1".to_string(), + )), }), }; @@ -1083,7 +1086,7 @@ mod tests { } } } - + #[tokio::test] async fn test_publish_value_signal_id_not_found() { let broker = DataBroker::default(); diff --git a/proto/kuksa/val/v2/types.proto b/proto/kuksa/val/v2/types.proto index 46a46881..1258aed8 100644 --- a/proto/kuksa/val/v2/types.proto +++ b/proto/kuksa/val/v2/types.proto @@ -58,7 +58,7 @@ message SignalID { // Numeric identifier to the signal // As of today Databroker assigns arbitrary unique numbers to each registered signal // at startup, meaning that identifiers may change after restarting Databroker. - // A mechanism for static identifiers may be introduced in the future. + // A mechanism for static identifiers may be introduced in the future. int32 id = 1; // Full VSS-style path to a specific signal, like "Vehicle.Speed" // Wildcards and paths to branches are not supported. @@ -159,8 +159,8 @@ message ValueRestrictionString { repeated string allowed_values = 1; } -// Used to indicate status of a signal in Databroker -// This is given as an alternative to Value, so if there is a valid value +// Used to indicate status of a signal in Databroker +// This is given as an alternative to Value, so if there is a valid value // ValueFailure shall not be specified. // // Scenarios where a signal does not exist or a user does not have access to a signal From f0d3c1ead370bfb46e31d2c1b42db935196e8ccc Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 14:25:42 +0200 Subject: [PATCH 7/8] Incr 7 --- databroker-cli/src/kuksa_cli.rs | 6 +++--- databroker-cli/src/sdv_cli.rs | 6 +++--- databroker/src/grpc/kuksa_val_v2/val.rs | 16 +++++++--------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/databroker-cli/src/kuksa_cli.rs b/databroker-cli/src/kuksa_cli.rs index 5dd91099..1b86f054 100644 --- a/databroker-cli/src/kuksa_cli.rs +++ b/databroker-cli/src/kuksa_cli.rs @@ -312,7 +312,7 @@ pub async fn kuksa_main(_cli: Cli) -> Result<(), Box> { } } Err(err) => { - cli::print_error(cmd, &format!("Malformed token: {err}"))? + cli::print_error(cmd, format!("Malformed token: {err}"))? } } } @@ -350,12 +350,12 @@ pub async fn kuksa_main(_cli: Cli) -> Result<(), Box> { } } Err(err) => { - cli::print_error(cmd, &format!("Malformed token: {err}"))? + cli::print_error(cmd, format!("Malformed token: {err}"))? } }, Err(err) => cli::print_error( cmd, - &format!( + format!( "Failed to open token file \"{token_filename}\": {err}" ), )?, diff --git a/databroker-cli/src/sdv_cli.rs b/databroker-cli/src/sdv_cli.rs index e1913e29..0bf91b0d 100644 --- a/databroker-cli/src/sdv_cli.rs +++ b/databroker-cli/src/sdv_cli.rs @@ -256,7 +256,7 @@ pub async fn sdv_main(_cli: Cli) -> Result<(), Box> { } } Err(err) => { - cli::print_error(cmd, &format!("Malformed token: {err}"))? + cli::print_error(cmd, format!("Malformed token: {err}"))? } } } @@ -295,12 +295,12 @@ pub async fn sdv_main(_cli: Cli) -> Result<(), Box> { } } Err(err) => { - cli::print_error(cmd, &format!("Malformed token: {err}"))? + cli::print_error(cmd, format!("Malformed token: {err}"))? } }, Err(err) => cli::print_error( cmd, - &format!( + format!( "Failed to open token file \"{token_filename}\": {err}" ), )?, diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 6bf008b8..5a635cde 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -786,7 +786,7 @@ mod tests { )]) .await; - return entry_id; + entry_id } #[tokio::test] @@ -830,8 +830,7 @@ mod tests { ); } Err(status) => { - // Handle the error from the publish_value function - assert!(false, "Get failed with status: {:?}", status); + panic!("Get failed with status: {:?}", status); } } } @@ -879,8 +878,7 @@ mod tests { ); } Err(status) => { - // Handle the error from the publish_value function - assert!(false, "Get failed with status: {:?}", status); + panic!("Get failed with status: {:?}", status); } } } @@ -904,7 +902,7 @@ mod tests { match broker.get_value(get_value_request).await { Ok(_response) => { - assert!(false, "Did not expect success"); + panic!("Did not expect success"); } Err(status) => { assert_eq!(status.code(), tonic::Code::Unauthenticated) @@ -966,7 +964,7 @@ mod tests { } Err(status) => { // Handle the error from the publish_value function - assert!(false, "Get failed with status: {:?}", status); + panic!("Get failed with status: {:?}", status); } } } @@ -993,7 +991,7 @@ mod tests { match broker.get_value(get_value_request).await { Ok(_response) => { - assert!(false, "Did not expect success"); + panic!("Did not expect success"); } Err(status) => { assert_eq!(status.code(), tonic::Code::NotFound) @@ -1023,7 +1021,7 @@ mod tests { match broker.get_value(get_value_request).await { Ok(_response) => { - assert!(false, "Did not expect success"); + panic!("Did not expect success"); } Err(status) => { assert_eq!(status.code(), tonic::Code::NotFound) From 692cfd073e8f61aec9243cc1dd928f0aa9f4e655 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 18 Sep 2024 15:15:22 +0200 Subject: [PATCH 8/8] Incr 8 --- databroker/src/grpc/kuksa_val_v2/val.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/databroker/src/grpc/kuksa_val_v2/val.rs b/databroker/src/grpc/kuksa_val_v2/val.rs index 5a635cde..5037ccf1 100644 --- a/databroker/src/grpc/kuksa_val_v2/val.rs +++ b/databroker/src/grpc/kuksa_val_v2/val.rs @@ -37,11 +37,6 @@ const MAX_REQUEST_PATH_LENGTH: usize = 1000; #[tonic::async_trait] impl proto::val_server::Val for broker::DataBroker { - /// Get the latest (current) value of a signal - /// - /// Returns (GRPC error code): - /// NOT_FOUND if the requested signal doesn't exist - /// PERMISSION_DENIED if access is denied async fn get_value( &self, request: tonic::Request, @@ -77,9 +72,6 @@ impl proto::val_server::Val for broker::DataBroker { } }; - // TODO - if this works - refactor so that we can reuse the lookup snippet for the methods below - // TODO Check what happens if NOT available - // DataValue::NotAvailable Ok(tonic::Response::new(proto::GetValueResponse { data_point: datapoint.into(), })) @@ -89,8 +81,6 @@ impl proto::val_server::Val for broker::DataBroker { &self, _request: tonic::Request, ) -> Result, tonic::Status> { - // Permissions - Err(tonic::Status::new( tonic::Code::Unimplemented, "Unimplemented", @@ -673,7 +663,7 @@ fn convert_to_proto_stream( for update in item.updates { let update_datapoint: Option = match update.update.datapoint { Some(datapoint) => { - // For subscribe streams we do not want to return NotVailable + // For subscribe streams we do not want to return NotAvailable // even if the values is not available when subscribe starts match datapoint.value { broker::DataValue::NotAvailable => None,