Merge pull request #371 from Kuadrant/immut_ids

Immutable Limit.id API

limitador-server/src/http_api/request_types.rs

@@ -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(),
@@ -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)

limitador/src/counter.rs

@@ -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()
     }
 

limitador/src/limit.rs

@@ -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 {
@@ -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"))
     }
 }

limitador/src/storage/keys.rs

@@ -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()
     }
 }
 
@@ -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)
@@ -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(),
             }
         }
@@ -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),
@@ -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() {
@@ -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);

limitador/tests/integration_tests.rs

@@ -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;