Skip to content

Commit a1f6e9f

Browse files
AnotherDanielAnotherDaniel
authored
Implement dsn~usubscription-register-notifications-invalid-topic (#26)
* Implement dsn~usubscription-register-notifications-invalid-topic, improved validation error messages * Improve error type checking in all handlers
1 parent bea6f11 commit a1f6e9f

File tree

6 files changed

+100
-139
lines changed

6 files changed

+100
-139
lines changed

‎up-subscription/src/handlers/fetch_subscribers.rs‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,10 @@ impl RequestHandler for FetchSubscribersRequestHandler {
6161
));
6262
};
6363

64-
// topic input validation
6564
// [impl->dsn~usubscription-fetch-subscribers-invalid-topic~1]
66-
helpers::validate_uri(&topic)?;
65+
helpers::validate_uri(&topic).map_err(|e| {
66+
ServiceInvocationError::InvalidArgument(format!("Invalid topic uri '{topic}': {e}"))
67+
})?;
6768

6869
// Interact with subscription manager backend
6970
let (respond_to, receive_from) = oneshot::channel::<Vec<SubscriberUUri>>();
@@ -187,11 +188,7 @@ mod tests {
187188
)
188189
.await;
189190

190-
assert!(result.is_err());
191-
match result.unwrap_err() {
192-
ServiceInvocationError::InvalidArgument(_) => {}
193-
_ => panic!("Wrong error type"),
194-
}
191+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
195192
}
196193

197194
#[tokio::test]
@@ -216,11 +213,7 @@ mod tests {
216213
)
217214
.await;
218215

219-
assert!(result.is_err());
220-
match result.unwrap_err() {
221-
ServiceInvocationError::InvalidArgument(_) => {}
222-
_ => panic!("Wrong error type"),
223-
}
216+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
224217
}
225218

226219
#[tokio::test]
@@ -242,11 +235,7 @@ mod tests {
242235
.handle_request(RESOURCE_ID_FETCH_SUBSCRIBERS, &message_attributes, None)
243236
.await;
244237

245-
assert!(result.is_err());
246-
match result.unwrap_err() {
247-
ServiceInvocationError::InvalidArgument(_) => {}
248-
_ => panic!("Wrong error type"),
249-
}
238+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
250239
}
251240

252241
#[tokio::test]
@@ -275,11 +264,7 @@ mod tests {
275264
)
276265
.await;
277266

278-
assert!(result.is_err());
279-
match result.unwrap_err() {
280-
ServiceInvocationError::InvalidArgument(_) => {}
281-
_ => panic!("Wrong error type"),
282-
}
267+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
283268
}
284269

285270
// [utest->dsn~usubscription-fetch-subscribers-invalid-topic~1]
@@ -329,10 +314,6 @@ mod tests {
329314
)
330315
.await;
331316

332-
assert!(result.is_err());
333-
match result.unwrap_err() {
334-
ServiceInvocationError::InvalidArgument(_) => {}
335-
_ => panic!("Wrong error type"),
336-
}
317+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
337318
}
338319
}

‎up-subscription/src/handlers/fetch_subscriptions.rs‎

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,21 @@ impl RequestHandler for FetchSubscriptionsRequestHandler {
6262
let request_kind = match request {
6363
Some(Request::Topic(topic)) => {
6464
// [impl->dsn~usubscription-fetch-subscriptions-invalid-topic~1]
65-
helpers::validate_uri(&topic)?;
65+
helpers::validate_uri(&topic).map_err(|e| {
66+
ServiceInvocationError::InvalidArgument(format!(
67+
"Invalid topic uri '{topic}': {e}"
68+
))
69+
})?;
6670
RequestKind::Topic(topic)
6771
}
6872
Some(Request::Subscriber(subscriber)) => {
6973
if let Some(subscriber) = subscriber.uri.into_option() {
7074
// [impl->dsn~usubscription-fetch-subscriptions-invalid-subscriber~1]
71-
helpers::validate_uri(&subscriber)?;
75+
helpers::validate_uri(&subscriber).map_err(|e| {
76+
ServiceInvocationError::InvalidArgument(format!(
77+
"Invalid subscriber uri '{subscriber}': {e}"
78+
))
79+
})?;
7280
RequestKind::Subscriber(subscriber)
7381
} else {
7482
return Err(ServiceInvocationError::InvalidArgument(
@@ -235,11 +243,7 @@ mod tests {
235243
)
236244
.await;
237245

238-
assert!(result.is_err());
239-
match result.unwrap_err() {
240-
ServiceInvocationError::InvalidArgument(_) => {}
241-
_ => panic!("Wrong error type"),
242-
}
246+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
243247
}
244248

245249
#[tokio::test]
@@ -264,11 +268,7 @@ mod tests {
264268
)
265269
.await;
266270

267-
assert!(result.is_err());
268-
match result.unwrap_err() {
269-
ServiceInvocationError::InvalidArgument(_) => {}
270-
_ => panic!("Wrong error type"),
271-
}
271+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
272272
}
273273

274274
#[tokio::test]
@@ -290,11 +290,7 @@ mod tests {
290290
.handle_request(RESOURCE_ID_FETCH_SUBSCRIPTIONS, &message_attributes, None)
291291
.await;
292292

293-
assert!(result.is_err());
294-
match result.unwrap_err() {
295-
ServiceInvocationError::InvalidArgument(_) => {}
296-
_ => panic!("Wrong error type"),
297-
}
293+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
298294
}
299295

300296
#[tokio::test]
@@ -323,11 +319,7 @@ mod tests {
323319
)
324320
.await;
325321

326-
assert!(result.is_err());
327-
match result.unwrap_err() {
328-
ServiceInvocationError::InvalidArgument(_) => {}
329-
_ => panic!("Wrong error type"),
330-
}
322+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
331323
}
332324

333325
// [utest->dsn~usubscription-fetch-subscriptions-invalid-subscriber~1]
@@ -386,11 +378,7 @@ mod tests {
386378
)
387379
.await;
388380

389-
assert!(result.is_err());
390-
match result.unwrap_err() {
391-
ServiceInvocationError::InvalidArgument(_) => {}
392-
_ => panic!("Wrong error type"),
393-
}
381+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
394382
}
395383

396384
// [utest->dsn~usubscription-fetch-subscriptions-invalid-topic~1]
@@ -444,10 +432,6 @@ mod tests {
444432
)
445433
.await;
446434

447-
assert!(result.is_err());
448-
match result.unwrap_err() {
449-
ServiceInvocationError::InvalidArgument(_) => {}
450-
_ => panic!("Wrong error type"),
451-
}
435+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
452436
}
453437
}

‎up-subscription/src/handlers/register_for_notifications.rs‎

Lines changed: 64 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ impl RequestHandler for RegisterNotificationsRequestHandler {
5858
"No topic defined in request".to_string(),
5959
));
6060
};
61+
// [impl->dsn~usubscription-register-notifications-invalid-topic~1]
62+
helpers::validate_uri(topic).map_err(|e| {
63+
ServiceInvocationError::InvalidArgument(format!("Invalid topic uri '{topic}': {e}"))
64+
})?;
6165

6266
// Interact with notification manager backend
6367
let se = NotificationEvent::AddNotifyee {
@@ -84,8 +88,11 @@ impl RequestHandler for RegisterNotificationsRequestHandler {
8488
#[cfg(test)]
8589
mod tests {
8690
use super::*;
91+
use test_case::test_case;
8792
use tokio::sync::mpsc::{self};
8893

94+
use up_rust::UUri;
95+
8996
use crate::{helpers, tests::test_lib};
9097

9198
// [utest->dsn~usubscription-register-notifications-protobuf~1]
@@ -162,11 +169,7 @@ mod tests {
162169
)
163170
.await;
164171

165-
assert!(result.is_err());
166-
match result.unwrap_err() {
167-
ServiceInvocationError::InvalidArgument(_) => {}
168-
_ => panic!("Wrong error type"),
169-
}
172+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
170173
}
171174

172175
#[tokio::test]
@@ -192,11 +195,7 @@ mod tests {
192195
)
193196
.await;
194197

195-
assert!(result.is_err());
196-
match result.unwrap_err() {
197-
ServiceInvocationError::InvalidArgument(_) => {}
198-
_ => panic!("Wrong error type"),
199-
}
198+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
200199
}
201200

202201
#[tokio::test]
@@ -221,11 +220,7 @@ mod tests {
221220
)
222221
.await;
223222

224-
assert!(result.is_err());
225-
match result.unwrap_err() {
226-
ServiceInvocationError::InvalidArgument(_) => {}
227-
_ => panic!("Wrong error type"),
228-
}
223+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
229224
}
230225

231226
#[tokio::test]
@@ -253,10 +248,59 @@ mod tests {
253248
)
254249
.await;
255250

256-
assert!(result.is_err());
257-
match result.unwrap_err() {
258-
ServiceInvocationError::InvalidArgument(_) => {}
259-
_ => panic!("Wrong error type"),
260-
}
251+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
252+
}
253+
254+
// [utest->dsn~usubscription-register-notifications-invalid-topic~1]
255+
#[test_case(UUri::default(); "Bad topic UUri")]
256+
#[test_case(UUri {
257+
authority_name: String::from("*"),
258+
ue_id: test_lib::helpers::TOPIC_LOCAL1_ID,
259+
ue_version_major: test_lib::helpers::TOPIC_LOCAL1_VERSION as u32,
260+
resource_id: test_lib::helpers::TOPIC_LOCAL1_RESOURCE as u32,
261+
..Default::default()
262+
}; "Wildcard authority in topic UUri")]
263+
#[test_case(UUri {
264+
authority_name: test_lib::helpers::LOCAL_AUTHORITY.into(),
265+
ue_id: 0xFFFF_0000,
266+
ue_version_major: test_lib::helpers::TOPIC_LOCAL1_VERSION as u32,
267+
resource_id: test_lib::helpers::TOPIC_LOCAL1_RESOURCE as u32,
268+
..Default::default()
269+
}; "Wildcard entity id in topic UUri")]
270+
#[test_case(UUri {
271+
authority_name: test_lib::helpers::LOCAL_AUTHORITY.into(),
272+
ue_id: test_lib::helpers::TOPIC_LOCAL1_ID,
273+
ue_version_major: test_lib::helpers::TOPIC_LOCAL1_VERSION as u32,
274+
resource_id: 0x0000_FFFF,
275+
..Default::default()
276+
}; "Wildcard resource id in topic UUri")]
277+
#[tokio::test]
278+
async fn test_invalid_topic_uri(topic: UUri) {
279+
helpers::init_once();
280+
281+
// create request and other required object(s)
282+
let notification_request = NotificationsRequest {
283+
topic: Some(topic).into(),
284+
..Default::default()
285+
};
286+
let request_payload = UPayload::try_from_protobuf(notification_request.clone()).unwrap();
287+
let message_attributes = UAttributes {
288+
source: Some(test_lib::helpers::subscriber_uri1()).into(),
289+
..Default::default()
290+
};
291+
let (notification_sender, _) = mpsc::channel::<NotificationEvent>(1);
292+
293+
// create and spawn off handler, to make all the asnync goodness work
294+
let request_handler = RegisterNotificationsRequestHandler::new(notification_sender);
295+
296+
let result = request_handler
297+
.handle_request(
298+
RESOURCE_ID_REGISTER_FOR_NOTIFICATIONS,
299+
&message_attributes,
300+
Some(request_payload),
301+
)
302+
.await;
303+
304+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
261305
}
262306
}

‎up-subscription/src/handlers/subscribe.rs‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,7 @@ mod tests {
206206
)
207207
.await;
208208

209-
assert!(result.is_err());
210-
match result.unwrap_err() {
211-
ServiceInvocationError::InvalidArgument(_) => {}
212-
_ => panic!("Wrong error type"),
213-
}
209+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
214210
}
215211

216212
#[tokio::test]
@@ -235,11 +231,7 @@ mod tests {
235231
)
236232
.await;
237233

238-
assert!(result.is_err());
239-
match result.unwrap_err() {
240-
ServiceInvocationError::InvalidArgument(_) => {}
241-
_ => panic!("Wrong error type"),
242-
}
234+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
243235
}
244236

245237
#[tokio::test]
@@ -261,11 +253,7 @@ mod tests {
261253
.handle_request(RESOURCE_ID_SUBSCRIBE, &message_attributes, None)
262254
.await;
263255

264-
assert!(result.is_err());
265-
match result.unwrap_err() {
266-
ServiceInvocationError::InvalidArgument(_) => {}
267-
_ => panic!("Wrong error type"),
268-
}
256+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
269257
}
270258

271259
#[tokio::test]
@@ -294,11 +282,7 @@ mod tests {
294282
)
295283
.await;
296284

297-
assert!(result.is_err());
298-
match result.unwrap_err() {
299-
ServiceInvocationError::InvalidArgument(_) => {}
300-
_ => panic!("Wrong error type"),
301-
}
285+
assert!(result.is_err_and(|err| matches!(err, ServiceInvocationError::InvalidArgument(_))));
302286
}
303287

304288
#[tokio::test]

0 commit comments

Comments
 (0)