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.

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

Immutable Limit.id API #371

Merged
merged 2 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 19 additions & 12 deletions limitador-server/src/http_api/request_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct Limit {
impl From<&LimitadorLimit> for Limit {
fn from(ll: &LimitadorLimit) -> Self {
Self {
id: ll.id().clone(),
id: ll.id().map(|id| id.to_string()),
namespace: ll.namespace().as_ref().to_string(),
max_value: ll.max_value(),
seconds: ll.seconds(),
Expand All @@ -43,17 +43,24 @@ impl From<&LimitadorLimit> for Limit {

impl From<Limit> for LimitadorLimit {
fn from(limit: Limit) -> Self {
let mut limitador_limit = Self::new(
limit.namespace,
limit.max_value,
limit.seconds,
limit.conditions,
limit.variables,
);

if let Some(id) = limit.id {
limitador_limit.set_id(id);
}
let mut limitador_limit = if let Some(id) = limit.id {
Self::with_id(
id,
limit.namespace,
limit.max_value,
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)
Expand Down
2 changes: 1 addition & 1 deletion limitador/src/counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl Counter {
Duration::from_secs(self.limit.seconds())
}

pub fn id(&self) -> &Option<String> {
pub fn id(&self) -> Option<&str> {
self.limit.id()
}

Expand Down
40 changes: 31 additions & 9 deletions limitador/src/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,16 +334,38 @@ impl Limit {
}
}

pub fn namespace(&self) -> &Namespace {
&self.namespace
pub fn with_id<S: Into<String>, N: Into<Namespace>, T: TryInto<Condition>>(
id: S,
namespace: N,
max_value: u64,
seconds: u64,
conditions: impl IntoIterator<Item = T>,
variables: impl IntoIterator<Item = impl Into<String>>,
) -> Self
where
<N as TryInto<Namespace>>::Error: core::fmt::Debug,
<T as TryInto<Condition>>::Error: core::fmt::Debug,
{
Self {
id: Some(id.into()),
namespace: namespace.into(),
max_value,
seconds,
name: None,
conditions: conditions
.into_iter()
.map(|cond| cond.try_into().expect("Invalid condition"))
.collect(),
variables: variables.into_iter().map(|var| var.into()).collect(),
}
}

pub fn set_id(&mut self, value: String) {
self.id = Some(value);
pub fn namespace(&self) -> &Namespace {
&self.namespace
}

pub fn id(&self) -> &Option<String> {
&self.id
pub fn id(&self) -> Option<&str> {
self.id.as_deref()
}

pub fn max_value(&self) -> u64 {
Expand Down Expand Up @@ -1012,15 +1034,15 @@ mod tests {

#[test]
fn limit_id() {
let mut limit = Limit::new(
let limit = Limit::with_id(
"test_id",
"test_namespace",
10,
60,
vec!["req.method == 'GET'"],
vec!["app_id"],
);
limit.set_id("test_id".to_string());

assert_eq!(limit.id().clone(), Some("test_id".to_string()))
assert_eq!(limit.id(), Some("test_id"))
}
}
53 changes: 31 additions & 22 deletions limitador/src/storage/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,25 @@ pub fn key_for_counter(counter: &Counter) -> Vec<u8> {
}

pub fn key_for_counters_of_limit(limit: &Limit) -> Vec<u8> {
if limit.id().is_none() {
let namespace = limit.namespace().as_ref();
format!(
"namespace:{{{namespace}}},counters_of_limit:{}",
serde_json::to_string(limit).unwrap()
)
.into_bytes()
} else {
if let Some(id) = limit.id() {
#[derive(PartialEq, Debug, Serialize, Deserialize)]
struct IdLimitKey<'a> {
id: &'a str,
}

let id = limit.id().as_ref().unwrap();
let key = IdLimitKey { id: id.as_ref() };
let key = IdLimitKey { id };

let mut encoded_key = Vec::new();
encoded_key = postcard::to_extend(&2u8, encoded_key).unwrap();
encoded_key = postcard::to_extend(&key, encoded_key).unwrap();
encoded_key
} else {
let namespace = limit.namespace().as_ref();
format!(
"namespace:{{{namespace}}},counters_of_limit:{}",
serde_json::to_string(limit).unwrap()
)
.into_bytes()
}
}

Expand Down Expand Up @@ -130,14 +129,14 @@ mod tests {

#[test]
fn key_for_limit_with_id_format() {
let mut limit = Limit::new(
let limit = Limit::with_id(
"test_id",
"example.com",
10,
60,
vec!["req.method == 'GET'"],
vec!["app_id"],
);
limit.set_id("test_id".to_string());
assert_eq!(
"\u{2}\u{7}test_id".as_bytes(),
key_for_counters_of_limit(&limit)
Expand Down Expand Up @@ -182,7 +181,7 @@ pub mod bin {
impl<'a> From<&'a Counter> for IdCounterKey<'a> {
fn from(counter: &'a Counter) -> Self {
IdCounterKey {
id: counter.id().as_ref().unwrap().as_ref(),
id: counter.id().unwrap(),
variables: counter.variables_for_key(),
}
}
Expand Down Expand Up @@ -280,8 +279,14 @@ pub mod bin {
.collect();

// we are not able to rebuild the full limit since we only have the id and variables.
let mut limit = Limit::new::<&str, &str>("", u64::default(), 0, vec![], map.keys());
limit.set_id(id.to_string());
let limit = Limit::with_id::<&str, &str, &str>(
id,
"",
u64::default(),
0,
vec![],
map.keys(),
);
Counter::new(limit, map)
}
_ => panic!("Unknown version: {}", version),
Expand Down Expand Up @@ -316,14 +321,13 @@ pub mod bin {

#[cfg(test)]
mod tests {
use crate::counter::Counter;
use crate::Limit;
use std::collections::HashMap;

use super::{
key_for_counter, key_for_counter_v2, partial_counter_from_counter_key,
prefix_for_namespace, CounterKey,
};
use crate::counter::Counter;
use crate::Limit;
use std::collections::HashMap;

#[test]
fn counter_key_serializes_and_back() {
Expand Down Expand Up @@ -375,9 +379,14 @@ pub mod bin {
let namespace = "ns_counter:";
let limit_without_id =
Limit::new(namespace, 1, 1, vec!["req.method == 'GET'"], vec!["app_id"]);
let mut limit_with_id =
Limit::new(namespace, 1, 1, vec!["req.method == 'GET'"], vec!["app_id"]);
limit_with_id.set_id("id200".to_string());
let limit_with_id = Limit::with_id(
"id200",
namespace,
1,
1,
vec!["req.method == 'GET'"],
vec!["app_id"],
);

let counter_with_id = Counter::new(limit_with_id, HashMap::default());
let serialized_with_id_counter = key_for_counter(&counter_with_id);
Expand Down
4 changes: 2 additions & 2 deletions limitador/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,14 @@ mod test {
async fn rate_limited_id_counter(rate_limiter: &mut TestsLimiter) {
let namespace = "test_namespace";
let max_hits = 3;
let mut limit = Limit::new(
let limit = Limit::with_id(
"test-rate_limited_id_counter",
namespace,
max_hits,
60,
vec!["req.method == 'GET'"],
vec!["app_id"],
);
limit.set_id("test-rate_limited_id_counter".to_string());

rate_limiter.add_limit(&limit).await;

Expand Down
Loading