Skip to content
This repository has been archived by the owner on Dec 18, 2024. It is now read-only.

Commit

Permalink
Avoid using validation on call and add unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@rafaeling
rafaeling committed Oct 4, 2023
1 parent ff58485 commit b0fc44d
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 58 deletions.
56 changes: 55 additions & 1 deletion kuksa_databroker/databroker/src/broker.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use std::time::SystemTime;

use tracing::{debug, info, warn};

use crate::glob;

#[derive(Debug)]
pub enum UpdateError {
NotFound,
Expand All @@ -46,7 +48,7 @@ pub enum ReadError {
PermissionExpired,
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum RegistrationError {
ValidationError,
PermissionDenied,
Expand Down Expand Up @@ -1024,6 +1026,10 @@ impl<'a, 'b> DatabaseWriteAccess<'a, 'b> {
allowed: Option<types::DataValue>,
datapoint: Option<Datapoint>,
) -> Result<i32, RegistrationError> {
if !glob::is_valid_pattern(name.as_str()) {
return Err(RegistrationError::ValidationError);
}

self.permissions
.can_create(&name)
.map_err(|err| match err {
Expand Down Expand Up @@ -2913,4 +2919,52 @@ mod tests {
}
}
}

#[tokio::test]
async fn test_register_invalid_and_valid_path() {
let broker = DataBroker::default();
let broker = broker.authorized_access(&permissions::ALLOW_ALL);

let error = broker
.add_entry(
"test. signal:3".to_owned(),
DataType::String,
ChangeType::OnChange,
EntryType::Sensor,
"Test signal 3".to_owned(),
Some(DataValue::Int32Array(Vec::from([1, 2, 3, 4]))),
)
.await
.unwrap_err();
assert_eq!(error, RegistrationError::ValidationError);

let id = broker
.add_entry(
"Vehicle._kuksa.databroker.GitVersion.Do_you_not_like_smörgåstårta.tschö_mit_ö.東京_Москва_r#true".to_owned(),
DataType::Bool,
ChangeType::OnChange,
EntryType::Sensor,
"Test datapoint".to_owned(),
Some(DataValue::BoolArray(Vec::from([true]))),
)
.await
.expect("Register datapoint should succeed");
{
match broker.get_entry_by_id(id).await {
Ok(entry) => {
assert_eq!(entry.metadata.id, id);
assert_eq!(entry.metadata.path, "Vehicle._kuksa.databroker.GitVersion.Do_you_not_like_smörgåstårta.tschö_mit_ö.東京_Москва_r#true");
assert_eq!(entry.metadata.data_type, DataType::Bool);
assert_eq!(entry.metadata.description, "Test datapoint");
assert_eq!(
entry.metadata.allowed,
Some(DataValue::BoolArray(Vec::from([true])))
);
}
Err(_) => {
panic!("no metadata returned");
}
}
}
}
}
11 changes: 0 additions & 11 deletions kuksa_databroker/databroker/src/grpc/kuksa_val_v1/val.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,6 @@ impl proto::val_server::Val for broker::DataBroker {

// Fill valid_requests structure.
for request in requested {
if !glob::is_valid_pattern(&request.path) {
errors.push(proto::DataEntryError {
path: request.path,
error: Some(proto::Error {
code: 400,
reason: "bad_request".to_owned(),
message: "Bad Path Request".to_owned(),
}),
});
continue;
}
let view = proto::View::from_i32(request.view).ok_or_else(|| {
tonic::Status::invalid_argument(format!("Invalid View (id: {}", request.view))
})?;
Expand Down
88 changes: 42 additions & 46 deletions kuksa_databroker/databroker/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use tracing::{debug, error, info};

use clap::{Arg, ArgAction, Command};

use databroker::{broker, glob, grpc, permissions, vss};
use databroker::{broker, grpc, permissions, vss};

async fn shutdown_handler() {
let mut sigint =
Expand Down Expand Up @@ -111,55 +111,51 @@ async fn read_metadata_file<'a, 'b>(
for (path, entry) in entries {
debug!("Adding VSS datapoint type {}", path);

if glob::is_valid_pattern(&path) {
match database
.add_entry(
path.clone(),
entry.data_type,
databroker::broker::ChangeType::OnChange,
entry.entry_type,
entry.description,
entry.allowed,
)
.await
{
Ok(id) => {
if let Some(default) = entry.default {
let ids = [(
id,
broker::EntryUpdate {
datapoint: Some(broker::Datapoint {
ts: std::time::SystemTime::now(),
value: default,
}),
path: None,
actuator_target: None,
entry_type: None,
data_type: None,
description: None,
allowed: None,
},
)];
if let Err(errors) = database.update_entries(ids).await {
// There's only one error (since we're only trying to set one)
if let Some(error) = errors.get(0) {
info!("Failed to set default value for {}: {:?}", path, error.1);
}
match database
.add_entry(
path.clone(),
entry.data_type,
databroker::broker::ChangeType::OnChange,
entry.entry_type,
entry.description,
entry.allowed,
)
.await
{
Ok(id) => {
if let Some(default) = entry.default {
let ids = [(
id,
broker::EntryUpdate {
datapoint: Some(broker::Datapoint {
ts: std::time::SystemTime::now(),
value: default,
}),
path: None,
actuator_target: None,
entry_type: None,
data_type: None,
description: None,
allowed: None,
},
)];
if let Err(errors) = database.update_entries(ids).await {
// There's only one error (since we're only trying to set one)
if let Some(error) = errors.get(0) {
info!("Failed to set default value for {}: {:?}", path, error.1);
}
}
}
Err(RegistrationError::PermissionDenied) => {
error!("Failed to add entry {path}: Permission denied")
}
Err(RegistrationError::PermissionExpired) => {
error!("Failed to add entry {path}: Permission expired")
}
Err(RegistrationError::ValidationError) => {
error!("Failed to add entry {path}: Validation failed")
}
}
} else {
info!("Not valid path pattern for: {}", path);
Err(RegistrationError::PermissionDenied) => {
error!("Failed to add entry {path}: Permission denied")
}
Err(RegistrationError::PermissionExpired) => {
error!("Failed to add entry {path}: Permission expired")
}
Err(RegistrationError::ValidationError) => {
error!("Failed to add entry {path}: Validation failed")
}
}
}
Ok(())
Expand Down

0 comments on commit b0fc44d

Please sign in to comment.