Skip to content

Commit e018adc

Browse files
rafaelingrafaeling
committed
Fix method response structure
1 parent 03d76e0 commit e018adc

File tree

4 files changed

+128
-89
lines changed

4 files changed

+128
-89
lines changed

‎databroker/src/viss/v2/server.rs

+120-81
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ impl Viss for Server {
9292
async fn get(&self, request: GetRequest) -> Result<GetSuccessResponse, GetErrorResponse> {
9393
let request_id = request.request_id;
9494

95-
let mut request_matcher: Vec<(Matcher, String, bool)> = Vec::new();
96-
9795
if let Some(Filter::StaticMetadata(_)) = &request.filter {
9896
// Authorization not required for metadata, don't bail if an
9997
// access token is missing.
@@ -104,99 +102,140 @@ impl Viss for Server {
104102
metadata,
105103
}));
106104
} else if let Some(Filter::Paths(paths_filter)) = &request.filter {
105+
let request_path = request.path.as_ref();
106+
if request_path.contains('*') {
107+
return Err(GetErrorResponse {
108+
request_id,
109+
ts: SystemTime::now().into(),
110+
error: Error::NotFoundInvalidPath,
111+
});
112+
}
113+
114+
let permissions = resolve_permissions(&self.authorization, &request.authorization)
115+
.map_err(|error| GetErrorResponse {
116+
request_id: request_id.clone(),
117+
error,
118+
ts: SystemTime::now().into(),
119+
})?;
120+
let broker = self.broker.authorized_access(&permissions);
121+
122+
let mut request_matcher: Vec<(Matcher, bool)> = Vec::new();
123+
let mut entries_data = Vec::new();
124+
let mut signal_errors = Vec::new();
125+
107126
for path in &paths_filter.parameter {
108-
let new_path = format!("{}{}", request.path.clone().as_ref(), path);
127+
let new_path = format!("{}.{}", request_path, path);
109128
if let Ok(matcher) = Matcher::new(&new_path) {
110-
request_matcher.push((matcher, request.path.clone().into(), false));
129+
request_matcher.push((matcher, false));
111130
}
112131
}
113-
}
114132

115-
let permissions = resolve_permissions(&self.authorization, &request.authorization)
116-
.map_err(|error| GetErrorResponse {
117-
request_id: request_id.clone(),
118-
error,
119-
ts: SystemTime::now().into(),
120-
})?;
121-
let broker = self.broker.authorized_access(&permissions);
122-
123-
let mut entries_data = Vec::new();
124-
let mut signal_errors = Vec::new();
125-
126-
if !request_matcher.is_empty() {
127-
for (matcher, _, is_match) in &mut request_matcher {
128-
broker
129-
.for_each_entry(|entry| {
130-
let glob_path = &entry.metadata().glob_path;
131-
let path = entry.metadata().path.clone();
132-
if matcher.is_match(glob_path) {
133-
match entry.datapoint() {
134-
Ok(datapoint) => {
135-
let dp = DataPoint::from(datapoint.clone());
136-
*is_match = true;
137-
entries_data.push(Data::Object(DataObject {
138-
path: Path::from(path),
139-
dp,
140-
}));
141-
}
142-
Err(_) => {
143-
signal_errors.push(path);
133+
if !request_matcher.is_empty() {
134+
for (matcher, is_match) in &mut request_matcher {
135+
broker
136+
.for_each_entry(|entry| {
137+
let glob_path = &entry.metadata().glob_path;
138+
let path = entry.metadata().path.clone();
139+
if matcher.is_match(glob_path) {
140+
match entry.datapoint() {
141+
Ok(datapoint) => {
142+
let dp = DataPoint::from(datapoint.clone());
143+
*is_match = true;
144+
entries_data.push(DataObject {
145+
path: Path::from(path),
146+
dp,
147+
});
148+
}
149+
Err(_) => {
150+
signal_errors.push(path);
151+
}
144152
}
145153
}
146-
}
147-
})
148-
.await;
149-
150-
// Not found any matches meaning it could be a branch path request
151-
// Only support branches like Vehicle.Cabin.Sunroof but not like **.Sunroof
152-
if !matcher.as_string().starts_with("**")
153-
&& !matcher.as_string().ends_with("/**")
154-
&& !(*is_match)
155-
{
156-
if let Ok(branch_matcher) = Matcher::new(&(matcher.as_string() + "/**")) {
157-
broker
158-
.for_each_entry(|entry| {
159-
let glob_path = &entry.metadata().glob_path;
160-
let path = entry.metadata().path.clone();
161-
if branch_matcher.is_match(glob_path) {
162-
match entry.datapoint() {
163-
Ok(datapoint) => {
164-
let dp = DataPoint::from(datapoint.clone());
165-
*is_match = true;
166-
entries_data.push(Data::Object(DataObject {
167-
path: Path::from(path),
168-
dp,
169-
}));
170-
}
171-
Err(_) => {
172-
signal_errors.push(path);
154+
})
155+
.await;
156+
157+
// Not found any matches meaning it could be a branch path request
158+
// Only support branches like Vehicle.Cabin.Sunroof but not like **.Sunroof
159+
if !matcher.as_string().starts_with("**")
160+
&& !matcher.as_string().ends_with("/**")
161+
&& !(*is_match)
162+
{
163+
if let Ok(branch_matcher) = Matcher::new(&(matcher.as_string() + "/**")) {
164+
broker
165+
.for_each_entry(|entry| {
166+
let glob_path = &entry.metadata().glob_path;
167+
let path = entry.metadata().path.clone();
168+
if branch_matcher.is_match(glob_path) {
169+
match entry.datapoint() {
170+
Ok(datapoint) => {
171+
let dp = DataPoint::from(datapoint.clone());
172+
*is_match = true;
173+
entries_data.push(DataObject {
174+
path: Path::from(path),
175+
dp,
176+
});
177+
}
178+
Err(_) => {
179+
signal_errors.push(path);
180+
}
173181
}
174182
}
175-
}
176-
})
177-
.await;
183+
})
184+
.await;
185+
}
178186
}
179187
}
180188
}
181-
}
182189

183-
// https://w3c.github.io/automotive/spec/VISSv2_Core.html#error-handling
184-
if signal_errors.is_empty() {
185-
Ok(GetSuccessResponse::Data(DataResponse {
186-
request_id,
187-
data: entries_data,
188-
}))
190+
// https://w3c.github.io/automotive/spec/VISSv2_Core.html#error-handling
191+
if signal_errors.is_empty() {
192+
Ok(GetSuccessResponse::Data(DataResponse {
193+
request_id,
194+
data: Data::Array(entries_data),
195+
}))
196+
} else {
197+
Err(GetErrorResponse {
198+
request_id,
199+
ts: SystemTime::now().into(),
200+
error: Error::Forbidden {
201+
msg: Some(format!(
202+
"Permission denied for some signal: {}",
203+
signal_errors.join(", ")
204+
)),
205+
},
206+
})
207+
}
189208
} else {
190-
Err(GetErrorResponse {
191-
request_id,
192-
ts: SystemTime::now().into(),
193-
error: Error::Forbidden {
194-
msg: Some(format!(
195-
"Permission denied for some signal: {}",
196-
signal_errors.join(", ")
197-
)),
198-
},
199-
})
209+
let permissions = resolve_permissions(&self.authorization, &request.authorization)
210+
.map_err(|error| GetErrorResponse {
211+
request_id: request_id.clone(),
212+
error,
213+
ts: SystemTime::now().into(),
214+
})?;
215+
let broker = self.broker.authorized_access(&permissions);
216+
217+
// Get datapoints
218+
match broker.get_datapoint_by_path(request.path.as_ref()).await {
219+
Ok(datapoint) => {
220+
let dp = DataPoint::from(datapoint);
221+
Ok(GetSuccessResponse::Data(DataResponse {
222+
request_id,
223+
data: Data::Object(DataObject {
224+
path: request.path,
225+
dp,
226+
}),
227+
}))
228+
}
229+
Err(err) => Err(GetErrorResponse {
230+
request_id,
231+
ts: SystemTime::now().into(),
232+
error: match err {
233+
broker::ReadError::NotFound => Error::NotFoundInvalidPath,
234+
broker::ReadError::PermissionDenied => Error::Forbidden { msg: None },
235+
broker::ReadError::PermissionExpired => Error::UnauthorizedTokenExpired,
236+
},
237+
}),
238+
}
200239
}
201240
}
202241

‎databroker/src/viss/v2/types.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ pub enum GetSuccessResponse {
6969
#[serde(tag = "action", rename = "get", rename_all = "camelCase")]
7070
pub struct DataResponse {
7171
pub request_id: RequestId,
72-
pub data: Vec<Data>,
72+
pub data: Data,
7373
}
7474

7575
#[derive(Serialize)]

‎integration_test/viss/features/viss_v2_multiple_paths.feature

+6-6
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@ Feature: VISS v2 Compliance Testing - Multiple Paths
2020
@ShouldHave
2121
Scenario: Request for multiple values from a single node
2222
# This scenario can be expanded based on specific use cases.
23-
When I send a read request with path "Vehicle.Cabin.TemperatureSetpoint"
24-
Then I should receive multiple values from a single node
23+
When I search "Vehicle.Cabin" using a path filter "Door.*.*.IsOpen"
24+
Then I should receive multiple data points
2525

2626
# 5.1.2 Read request - Single value request from multiple nodes
2727
# The VISS server must support reading values from multiple nodes using path filters.
2828
@MustHave
2929
Scenario: Request for a single value from multiple nodes
3030
When I search "Vehicle" using a path filter "*"
31-
Then I should receive a single value from multiple nodes
31+
Then I should receive multiple data points
3232

3333
# 5.1.2 Read request - Multiple values from multiple nodes
3434
# The VISS server should support reading multiple values from multiple nodes.
3535
@ShouldHave
36-
Scenario: Request for multiple values from multiple nodes
37-
When I send a read request with path "Vehicle.*"
38-
Then I should receive multiple values from multiple nodes
36+
Scenario: Path request for multiple values must not contain any wildcards
37+
When I search "Vehicle.*" using a path filter "*"
38+
Then I should receive an error response

‎integration_test/viss/features/viss_v2_transport_wss_filter.feature

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Feature: VISS v2 Compliance Testing - Filter
3333
# The VISS server must support searching for multiple signals using a path filter (e.g., "*.IsOpen").
3434
@MustHave
3535
Scenario: Search for multiple signals using path filter
36-
When I search "Vehicle" using a path filter "*.IsOpen"
36+
When I search "Vehicle" using a path filter "*.*.*.IsOpen"
3737
Then I should receive multiple data points
3838

3939
# 5.5.1 Subscribe with change filter

0 commit comments

Comments
 (0)