Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace all unwraps #147

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Replace all unwraps #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c3ea61
Replace all unwraps
adam-cattermole Nov 14, 2024
b3d4307
Expect in tests
adam-cattermole Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,7 @@ impl TryFrom<PluginConfiguration> for FilterConfig {
.expect("Predicates must not be compiled yet!");

for datum in &action.data {
let result = datum.item.compile();
if result.is_err() {
return Err(result.err().unwrap());
}
datum.item.compile()?;
}
}

Expand Down Expand Up @@ -204,7 +201,10 @@ impl<'de> Visitor<'de> for TimeoutVisitor {
E: Error,
{
match duration(Arc::new(string)) {
Ok(Value::Duration(duration)) => Ok(Timeout(duration.to_std().unwrap())),
Ok(Value::Duration(duration)) => duration
.to_std()
.map(Timeout)
.map_err(|e| E::custom(e.to_string())),
Err(e) => Err(E::custom(e)),
_ => Err(E::custom("Unsupported Duration Value")),
}
Expand Down Expand Up @@ -277,7 +277,7 @@ mod test {
}
assert!(res.is_ok());

let filter_config = res.unwrap();
let filter_config = res.expect("result is ok");
assert_eq!(filter_config.action_sets.len(), 1);

let services = &filter_config.services;
Expand Down Expand Up @@ -364,7 +364,7 @@ mod test {
}
assert!(res.is_ok());

let filter_config = res.unwrap();
let filter_config = res.expect("result is ok");
assert_eq!(filter_config.action_sets.len(), 0);
}

Expand Down Expand Up @@ -410,12 +410,15 @@ mod test {
}
assert!(res.is_ok());

let filter_config = res.unwrap();
let filter_config = res.expect("result is ok");
assert_eq!(filter_config.action_sets.len(), 1);

let services = &filter_config.services;
assert_eq!(
services.get("limitador").unwrap().timeout,
services
.get("limitador")
.expect("limitador service to be set")
.timeout,
Timeout(Duration::from_millis(20))
);

Expand Down Expand Up @@ -510,7 +513,7 @@ mod test {
}
assert!(res.is_ok());

let result = FilterConfig::try_from(res.unwrap());
let result = FilterConfig::try_from(res.expect("result is ok"));
let filter_config = result.expect("That didn't work");
let rlp_option = filter_config
.index
Expand Down
10 changes: 5 additions & 5 deletions src/configuration/action_set_index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ mod tests {

let val = index.get_longest_match_action_sets("example.com");
assert!(val.is_some());
assert_eq!(val.unwrap()[0].name, "rlp1");
assert_eq!(val.expect("value must be some")[0].name, "rlp1");
}

#[test]
Expand All @@ -90,7 +90,7 @@ mod tests {
let val = index.get_longest_match_action_sets("test.example.com");

assert!(val.is_some());
assert_eq!(val.unwrap()[0].name, "rlp1");
assert_eq!(val.expect("value must be some")[0].name, "rlp1");
}

#[test]
Expand All @@ -103,11 +103,11 @@ mod tests {

let val = index.get_longest_match_action_sets("test.example.com");
assert!(val.is_some());
assert_eq!(val.unwrap()[0].name, "rlp2");
assert_eq!(val.expect("value must be some")[0].name, "rlp2");

let val = index.get_longest_match_action_sets("example.com");
assert!(val.is_some());
assert_eq!(val.unwrap()[0].name, "rlp1");
assert_eq!(val.expect("value must be some")[0].name, "rlp1");
}

#[test]
Expand All @@ -118,6 +118,6 @@ mod tests {

let val = index.get_longest_match_action_sets("test.example.com");
assert!(val.is_some());
assert_eq!(val.unwrap()[0].name, "rlp1");
assert_eq!(val.expect("value must be some")[0].name, "rlp1");
}
}
5 changes: 4 additions & 1 deletion src/data/attribute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ fn process_metadata(s: &Struct, prefix: String) -> Vec<(String, String)> {
let nested_struct = value.get_struct_value();
result.extend(process_metadata(nested_struct, current_prefix));
} else if let Some(v) = json {
result.push((current_prefix, serde_json::to_string(&v).unwrap()));
match serde_json::to_string(&v) {
Ok(ser) => result.push((current_prefix, ser)),
Err(e) => error!("failed to serialize json Value: {e:?}"),
}
}
}
result
Expand Down
86 changes: 58 additions & 28 deletions src/data/cel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ use cel_parser::{parse, Expression as CelExpression, Member, ParseError};
use chrono::{DateTime, FixedOffset};
#[cfg(feature = "debug-host-behaviour")]
use log::debug;
use log::warn;
use proxy_wasm::types::{Bytes, Status};
use serde_json::Value as JsonValue;
use std::borrow::Cow;
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::fmt::{Debug, Formatter};
Expand Down Expand Up @@ -87,7 +89,7 @@ impl Expression {

/// Decodes the query string and returns a Map where the key is the parameter's name and
/// the value is either a [`Value::String`] or a [`Value::List`] if the parameter's name is repeated
/// and the second arg is set not set to `false`.
/// and the second arg is not set to `false`.
/// see [`tests::decodes_query_string`]
fn decode_query_string(This(s): This<Arc<String>>, Arguments(args): Arguments) -> ResolveResult {
let allow_repeats = if args.len() == 2 {
Expand All @@ -102,8 +104,22 @@ fn decode_query_string(This(s): This<Arc<String>>, Arguments(args): Arguments) -
for part in s.split('&') {
let mut kv = part.split('=');
if let (Some(key), Some(value)) = (kv.next(), kv.next().or(Some(""))) {
let new_v: Value = decode(value).unwrap().into_owned().into();
match map.entry(decode(key).unwrap().into_owned().into()) {
let new_v: Value = decode(value)
.unwrap_or_else(|e| {
warn!("failed to decode query value, using default: {e:?}");
Cow::from(value)
})
.into_owned()
.into();
match map.entry(
decode(key)
.unwrap_or_else(|e| {
warn!("failed to decode query key, using default: {e:?}");
Cow::from(key)
})
.into_owned()
.into(),
) {
Entry::Occupied(mut e) => {
if allow_repeats {
if let Value::List(ref mut list) = e.get_mut() {
Expand All @@ -118,7 +134,15 @@ fn decode_query_string(This(s): This<Arc<String>>, Arguments(args): Arguments) -
}
}
Entry::Vacant(e) => {
e.insert(decode(value).unwrap().into_owned().into());
e.insert(
decode(value)
.unwrap_or_else(|e| {
warn!("failed to decode query value, using default: {e:?}");
Cow::from(value)
})
.into_owned()
.into(),
);
}
}
}
Expand Down Expand Up @@ -296,11 +320,11 @@ fn json_to_cel(json: &str) -> Value {
JsonValue::Bool(b) => b.into(),
JsonValue::Number(n) => {
if n.is_u64() {
n.as_u64().unwrap().into()
n.as_u64().expect("Unreachable: number must be u64").into()
} else if n.is_i64() {
n.as_i64().unwrap().into()
n.as_i64().expect("Unreachable: number must be i64").into()
} else {
n.as_f64().unwrap().into()
n.as_f64().expect("Unreachable: number must be f64").into()
}
}
JsonValue::String(str) => str.into(),
Expand Down Expand Up @@ -533,10 +557,14 @@ pub mod data {
fn it_works() {
let map = AttributeMap::new(
[
known_attribute_for(&"request.method".into()).unwrap(),
known_attribute_for(&"request.referer".into()).unwrap(),
known_attribute_for(&"source.address".into()).unwrap(),
known_attribute_for(&"destination.port".into()).unwrap(),
known_attribute_for(&"request.method".into())
.expect("request.method known attribute exists"),
known_attribute_for(&"request.referer".into())
.expect("request.referer known attribute exists"),
known_attribute_for(&"source.address".into())
.expect("source.address known attribute exists"),
known_attribute_for(&"destination.port".into())
.expect("destination.port known attribute exists"),
]
.into(),
);
Expand All @@ -548,38 +576,38 @@ pub mod data {
assert!(map.data.contains_key("destination"));
assert!(map.data.contains_key("request"));

match map.data.get("source").unwrap() {
match map.data.get("source").expect("source is some") {
Token::Node(map) => {
assert_eq!(map.len(), 1);
match map.get("address").unwrap() {
match map.get("address").expect("address is some") {
Token::Node(_) => panic!("Not supposed to get here!"),
Token::Value(v) => assert_eq!(v.path, "source.address".into()),
}
}
Token::Value(_) => panic!("Not supposed to get here!"),
}

match map.data.get("destination").unwrap() {
match map.data.get("destination").expect("destination is some") {
Token::Node(map) => {
assert_eq!(map.len(), 1);
match map.get("port").unwrap() {
match map.get("port").expect("port is some") {
Token::Node(_) => panic!("Not supposed to get here!"),
Token::Value(v) => assert_eq!(v.path, "destination.port".into()),
}
}
Token::Value(_) => panic!("Not supposed to get here!"),
}

match map.data.get("request").unwrap() {
match map.data.get("request").expect("request is some") {
Token::Node(map) => {
assert_eq!(map.len(), 2);
assert!(map.get("method").is_some());
match map.get("method").unwrap() {
match map.get("method").expect("method is some") {
Token::Node(_) => panic!("Not supposed to get here!"),
Token::Value(v) => assert_eq!(v.path, "request.method".into()),
}
assert!(map.get("referer").is_some());
match map.get("referer").unwrap() {
match map.get("referer").expect("referer is some") {
Token::Node(_) => panic!("Not supposed to get here!"),
Token::Value(v) => assert_eq!(v.path, "request.referer".into()),
}
Expand Down Expand Up @@ -611,7 +639,7 @@ mod tests {
let value = Expression::new(
"auth.identity.anonymous && auth.identity != null && auth.identity.foo > 3",
)
.unwrap();
.expect("This is valid CEL!");
assert_eq!(value.attributes.len(), 3);
assert_eq!(value.attributes[0].path, "auth.identity".into());
}
Expand All @@ -626,7 +654,7 @@ mod tests {
"true".bytes().collect(),
)));
let value = Expression::new("auth.identity.anonymous")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, true.into());
Expand All @@ -636,7 +664,7 @@ mod tests {
"42".bytes().collect(),
)));
let value = Expression::new("auth.identity.age")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, 42.into());
Expand All @@ -646,7 +674,7 @@ mod tests {
"42.3".bytes().collect(),
)));
let value = Expression::new("auth.identity.age")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, 42.3.into());
Expand All @@ -656,7 +684,7 @@ mod tests {
"\"John\"".bytes().collect(),
)));
let value = Expression::new("auth.identity.age")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, "John".into());
Expand All @@ -666,7 +694,7 @@ mod tests {
"-42".bytes().collect(),
)));
let value = Expression::new("auth.identity.name")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, (-42).into());
Expand All @@ -677,7 +705,7 @@ mod tests {
"some random crap".bytes().collect(),
)));
let value = Expression::new("auth.identity.age")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, "some random crap".into());
Expand Down Expand Up @@ -751,12 +779,14 @@ mod tests {
80_i64.to_le_bytes().into(),
)));
let value = known_attribute_for(&"destination.port".into())
.unwrap()
.expect("destination.port known attribute exists")
.get();
assert_eq!(value, 80.into());
property::test::TEST_PROPERTY_VALUE
.set(Some(("request.method".into(), "GET".bytes().collect())));
let value = known_attribute_for(&"request.method".into()).unwrap().get();
let value = known_attribute_for(&"request.method".into())
.expect("request.method known attribute exists")
.get();
assert_eq!(value, "GET".into());
}

Expand All @@ -778,7 +808,7 @@ mod tests {
b"\xCA\xFE".to_vec(),
)));
let value = Expression::new("getHostProperty(['foo', 'bar.baz'])")
.unwrap()
.expect("This is valid CEL!")
.eval()
.expect("This must evaluate!");
assert_eq!(value, Value::Bytes(Arc::new(b"\xCA\xFE".to_vec())));
Expand Down
3 changes: 2 additions & 1 deletion src/filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ extern "C" fn start() {

proxy_wasm::set_log_level(LogLevel::Trace);
std::panic::set_hook(Box::new(|panic_info| {
proxy_wasm::hostcalls::log(LogLevel::Critical, &panic_info.to_string()).unwrap();
proxy_wasm::hostcalls::log(LogLevel::Critical, &panic_info.to_string())
.expect("failed to log panic_info");
}));
proxy_wasm::set_root_context(|context_id| -> Box<dyn RootContext> {
info!("#{} set_root_context", context_id);
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ mod tests {
.push(header("test", "some value"));
resp.response_headers_to_add
.push(header("other", "header value"));
let buffer = resp.write_to_bytes().unwrap();
let buffer = resp
.write_to_bytes()
.expect("must be able to write RateLimitResponse to bytes");
let expected: [u8; 45] = [
8, 1, 26, 18, 10, 4, 116, 101, 115, 116, 18, 10, 115, 111, 109, 101, 32, 118, 97, 108,
117, 101, 26, 21, 10, 5, 111, 116, 104, 101, 114, 18, 12, 104, 101, 97, 100, 101, 114,
Expand Down
Loading
Loading