Skip to content

Commit

Permalink
Deal with syntax error of conditions
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Snaps <[email protected]>
  • Loading branch information
alexsnaps committed Nov 25, 2024
1 parent 74b7539 commit 539d53c
Show file tree
Hide file tree
Showing 13 changed files with 228 additions and 126 deletions.
15 changes: 10 additions & 5 deletions limitador-server/src/envoy_rls/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ mod tests {
60,
vec!["req.method == 'GET'"],
vec!["app_id"],
);
)
.expect("This must be a valid limit!");

let limiter = RateLimiter::new(10_000);
limiter.add_limit(limit);
Expand Down Expand Up @@ -394,8 +395,10 @@ mod tests {
let namespace = "test_namespace";

vec![
Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["z"]),
Limit::new(namespace, 0, 60, vec!["x == '1'", "y == '2'"], vec!["z"]),
Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["z"])
.expect("This must be a valid limit!"),
Limit::new(namespace, 0, 60, vec!["x == '1'", "y == '2'"], vec!["z"])
.expect("This must be a valid limit!"),
]
.into_iter()
.for_each(|limit| {
Expand Down Expand Up @@ -459,7 +462,8 @@ mod tests {
#[tokio::test]
async fn test_takes_into_account_the_hits_addend_param() {
let namespace = "test_namespace";
let limit = Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["y"]);
let limit = Limit::new(namespace, 10, 60, vec!["x == '1'"], vec!["y"])
.expect("This must be a valid limit!");

let limiter = RateLimiter::new(10_000);
limiter.add_limit(limit);
Expand Down Expand Up @@ -528,7 +532,8 @@ mod tests {
// "hits_addend" is optional according to the spec, and should default
// to 1, However, with the autogenerated structs it defaults to 0.
let namespace = "test_namespace";
let limit = Limit::new(namespace, 1, 60, vec!["x == '1'"], vec!["y"]);
let limit = Limit::new(namespace, 1, 60, vec!["x == '1'"], vec!["y"])
.expect("This must be a valid limit!");

let limiter = RateLimiter::new(10_000);
limiter.add_limit(limit);
Expand Down
14 changes: 8 additions & 6 deletions limitador-server/src/http_api/request_types.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use limitador::counter::Counter as LimitadorCounter;
use limitador::errors::LimitadorError;
use limitador::limit::Limit as LimitadorLimit;
use paperclip::actix::Apiv2Schema;
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, HashMap};

// We need to define the Limit and Counter types. They're basically the same as
// defined in the lib but with some modifications to be able to derive
// Apiv2Schema (needed to generate the OpenAPI specs).
Expand Down Expand Up @@ -41,8 +41,10 @@ impl From<&LimitadorLimit> for Limit {
}
}

impl From<Limit> for LimitadorLimit {
fn from(limit: Limit) -> Self {
impl TryFrom<Limit> for LimitadorLimit {
type Error = LimitadorError;

fn try_from(limit: Limit) -> Result<Self, Self::Error> {
let mut limitador_limit = if let Some(id) = limit.id {
Self::with_id(
id,
Expand All @@ -51,22 +53,22 @@ impl From<Limit> for LimitadorLimit {
limit.seconds,
limit.conditions,
limit.variables,
)
)?
} else {
Self::new(
limit.namespace,
limit.max_value,
limit.seconds,
limit.conditions,
limit.variables,
)
)?
};

if let Some(name) = limit.name {
limitador_limit.set_name(name)
}

limitador_limit
Ok(limitador_limit)
}
}

Expand Down
3 changes: 2 additions & 1 deletion limitador-server/src/http_api/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,8 @@ mod tests {
60,
vec!["req.method == 'GET'"],
vec!["app_id"],
);
)
.expect("This must be a valid limit!");

match &limiter {
Limiter::Blocking(limiter) => limiter.add_limit(limit.clone()),
Expand Down
2 changes: 1 addition & 1 deletion limitador/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ fn generate_test_limits(scenario: &TestScenario) -> (Vec<Limit>, Vec<TestCallPar
((limit_idx * 60) + 10) as u64,
conditions.clone(),
variables.clone(),
))
).expect("This must be a valid limit!"))
}

call_params.push(TestCallParams {
Expand Down
27 changes: 23 additions & 4 deletions limitador/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,51 @@
use crate::limit::ConditionParsingError;
use crate::storage::StorageErr;
use std::convert::Infallible;
use std::error::Error;
use std::fmt::{Display, Formatter};

#[derive(Debug)]
pub enum LimitadorError {
Storage(StorageErr),
StorageError(StorageErr),
InterpreterError(ConditionParsingError),
}

impl Display for LimitadorError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
LimitadorError::Storage(err) => {
LimitadorError::StorageError(err) => {
write!(f, "error while accessing the limits storage: {err:?}")
}
LimitadorError::InterpreterError(err) => {
write!(f, "error parsing condition: {err:?}")
}
}
}
}

impl Error for LimitadorError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
LimitadorError::Storage(err) => Some(err),
LimitadorError::StorageError(err) => Some(err),
LimitadorError::InterpreterError(err) => Some(err),
}
}
}

impl From<StorageErr> for LimitadorError {
fn from(e: StorageErr) -> Self {
Self::Storage(e)
Self::StorageError(e)
}
}

impl From<ConditionParsingError> for LimitadorError {
fn from(err: ConditionParsingError) -> Self {
LimitadorError::InterpreterError(err)
}
}

impl From<Infallible> for LimitadorError {
fn from(value: Infallible) -> Self {
unreachable!("unexpected infallible value: {:?}", value)
}
}
9 changes: 5 additions & 4 deletions limitador/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
//! 60,
//! vec!["req.method == 'GET'"],
//! vec!["user_id"],
//! );
//! ).unwrap();
//! let mut rate_limiter = RateLimiter::new(1000);
//!
//! // Add a limit
Expand Down Expand Up @@ -105,7 +105,7 @@
//! 60,
//! vec!["req.method == 'GET'"],
//! vec!["user_id"],
//! );
//! ).unwrap();
//! rate_limiter.add_limit(limit);
//!
//! // We've defined a limit of 2. So we can report 2 times before being
Expand Down Expand Up @@ -169,7 +169,7 @@
//! 60,
//! vec!["req.method == 'GET'"],
//! vec!["user_id"],
//! );
//! ).unwrap();
//!
//! async {
//! let rate_limiter = AsyncRateLimiter::new_with_storage(
Expand Down Expand Up @@ -708,7 +708,8 @@ mod test {
100,
Vec::<String>::default(),
Vec::<String>::default(),
);
)
.expect("This must be a valid limit!");
rl.add_limit(l.clone());
let limits = rl.get_limits(&namespace.into());
assert_eq!(limits.len(), 1);
Expand Down
Loading

0 comments on commit 539d53c

Please sign in to comment.