Fixed test and take ownership of vars in ctx

Signed-off-by: Alex Snaps <[email protected]>

limitador-server/src/envoy_rls/server.rs

@@ -109,7 +109,7 @@ impl RateLimitService for MyRateLimiter {
             req.hits_addend
         };
 
-        let ctx = (&values).into();
+        let ctx = values.into();
 
         let rate_limited_resp = match &*self.limiter {
             Limiter::Blocking(limiter) => limiter.check_rate_limited_and_update(

limitador-server/src/http_api/server.rs

@@ -122,7 +122,7 @@ async fn check(
         response_headers: _,
     } = request.into_inner();
     let namespace = namespace.into();
-    let ctx = (&values).into();
+    let ctx = values.into();
     let is_rate_limited_result = match state.get_ref().limiter() {
         Limiter::Blocking(limiter) => limiter.is_rate_limited(&namespace, &ctx, delta),
         Limiter::Async(limiter) => limiter.is_rate_limited(&namespace, &ctx, delta).await,
@@ -153,7 +153,7 @@ async fn report(
         response_headers: _,
     } = request.into_inner();
     let namespace = namespace.into();
-    let ctx = (&values).into();
+    let ctx = values.into();
     let update_counters_result = match data.get_ref().limiter() {
         Limiter::Blocking(limiter) => limiter.update_counters(&namespace, &ctx, delta),
         Limiter::Async(limiter) => limiter.update_counters(&namespace, &ctx, delta).await,
@@ -178,7 +178,7 @@ async fn check_and_report(
         response_headers,
     } = request.into_inner();
     let namespace = namespace.into();
-    let ctx = (&values).into();
+    let ctx = values.into();
     let rate_limit_data = data.get_ref();
     let rate_limited_and_update_result = match rate_limit_data.limiter() {
         Limiter::Blocking(limiter) => limiter.check_rate_limited_and_update(

limitador/benches/bench.rs

@@ -7,7 +7,7 @@ use criterion::{black_box, criterion_group, criterion_main, Bencher, BenchmarkId
 use rand::seq::SliceRandom;
 use rand::SeedableRng;
 
-use limitador::limit::Limit;
+use limitador::limit::{Context, Limit};
 #[cfg(feature = "disk_storage")]
 use limitador::storage::disk::{DiskStorage, OptimizeFor};
 #[cfg(feature = "distributed_storage")]
@@ -89,9 +89,9 @@ const TEST_SCENARIOS: &[&TestScenario] = &[
     },
 ];
 
-struct TestCallParams {
+struct TestCallParams<'a> {
     namespace: String,
-    values: HashMap<String, String>,
+    ctx: Context<'a>,
     delta: u64,
 }
 
@@ -329,7 +329,7 @@ fn bench_is_rate_limited(
             rate_limiter
                 .is_rate_limited(
                     &params.namespace.to_owned().into(),
-                    &(&params.values).into(),
+                    &params.ctx,
                     params.delta,
                 )
                 .unwrap(),
@@ -357,7 +357,7 @@ fn async_bench_is_rate_limited<F>(
                 rate_limiter
                     .is_rate_limited(
                         &params.namespace.to_owned().into(),
-                        &(&params.values).into(),
+                        &params.ctx,
                         params.delta,
                     )
                     .await
@@ -383,7 +383,7 @@ fn bench_update_counters(
         rate_limiter
             .update_counters(
                 &params.namespace.to_owned().into(),
-                &(&params.values).into(),
+                &params.ctx,
                 params.delta,
             )
             .unwrap();
@@ -410,7 +410,7 @@ fn async_bench_update_counters<F>(
                 rate_limiter
                     .update_counters(
                         &params.namespace.to_owned().into(),
-                        &(&params.values).into(),
+                        &params.ctx,
                         params.delta,
                     )
                     .await
@@ -437,7 +437,7 @@ fn bench_check_rate_limited_and_update(
             rate_limiter
                 .check_rate_limited_and_update(
                     &params.namespace.to_owned().into(),
-                    &(&params.values).into(),
+                    &params.ctx,
                     params.delta,
                     false,
                 )
@@ -467,7 +467,7 @@ fn async_bench_check_rate_limited_and_update<F>(
                 rate_limiter
                     .check_rate_limited_and_update(
                         &params.namespace.to_owned().into(),
-                        &(&params.values).into(),
+                        &params.ctx,
                         params.delta,
                         false,
                     )
@@ -562,7 +562,7 @@ fn generate_test_limits(scenario: &TestScenario) -> (Vec<Limit>, Vec<TestCallPar
 
         call_params.push(TestCallParams {
             namespace,
-            values: test_values.clone(),
+            ctx: test_values.clone().into(),
             delta: 1,
         });
     }

limitador/src/counter.rs

@@ -154,7 +154,7 @@ mod tests {
             [var.try_into().expect("failed parsing!")],
         );
         let map = HashMap::from([("ts".to_string(), "2019-10-12T13:20:50.52Z".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(limit, &ctx).expect("failed creating counter");
         assert_eq!(
             counter.unwrap().set_variables.get(var),

limitador/src/limit.rs

@@ -260,7 +260,7 @@ mod tests {
         values.insert("x".into(), "5".into());
         values.insert("y".into(), "1".into());
 
-        assert!(limit.applies(&(&values).into()))
+        assert!(limit.applies(&values.into()))
     }
 
     #[test]
@@ -277,7 +277,7 @@ mod tests {
         values.insert("x".into(), "1".into());
         values.insert("y".into(), "1".into());
 
-        assert!(!limit.applies(&(&values).into()))
+        assert!(!limit.applies(&values.into()))
     }
 
     #[test]
@@ -295,7 +295,7 @@ mod tests {
         values.insert("a".into(), "1".into());
         values.insert("y".into(), "1".into());
 
-        assert!(!limit.applies(&(&values).into()))
+        assert!(!limit.applies(&values.into()))
     }
 
     #[test]
@@ -312,7 +312,7 @@ mod tests {
         let mut values: HashMap<String, String> = HashMap::new();
         values.insert("x".into(), "5".into());
 
-        assert!(!limit.applies(&(&values).into()))
+        assert!(!limit.applies(&values.into()))
     }
 
     #[test]
@@ -333,7 +333,7 @@ mod tests {
         values.insert("y".into(), "2".into());
         values.insert("z".into(), "1".into());
 
-        assert!(limit.applies(&(&values).into()))
+        assert!(limit.applies(&values.into()))
     }
 
     #[test]
@@ -354,7 +354,7 @@ mod tests {
         values.insert("y".into(), "2".into());
         values.insert("z".into(), "1".into());
 
-        assert!(!limit.applies(&(&values).into()))
+        assert!(!limit.applies(&values.into()))
     }
 
     #[test]
@@ -434,7 +434,7 @@ mod tests {
             ],
             Vec::default(),
         );
-        assert!(limit.applies(&(&HashMap::default()).into()));
+        assert!(limit.applies(&Context::default()));
 
         let limit = Limit::with_id(
             "my_id",
@@ -462,7 +462,7 @@ mod tests {
             ("foo".to_string(), "nice bar!".to_string()),
             ("bar".to_string(), "foo,baz".to_string()),
         ]);
-        let ctx = Context::new(String::default(), &map);
+        let ctx = map.into();
         assert!(limit.applies(&ctx));
         assert_eq!(
             Counter::new(limit, &ctx)

limitador/src/limit/cel.rs

@@ -78,22 +78,21 @@ pub struct Context<'a> {
 }
 
 impl<'a> Context<'a> {
-    pub(crate) fn new(root: String, values: &HashMap<String, String>) -> Self {
+    pub(crate) fn new(root: String, values: HashMap<String, String>) -> Self {
         let mut ctx = cel_interpreter::Context::default();
+        let mut variables = HashSet::new();
 
         if root.is_empty() {
             for (binding, value) in values {
-                ctx.add_variable_from_value(binding, value.clone())
+                ctx.add_variable_from_value(binding.clone(), value.clone());
+                variables.insert(binding);
             }
         } else {
             let map = cel_interpreter::objects::Map::from(values.clone());
             ctx.add_variable_from_value(root, Value::Map(map));
         }
 
-        Self {
-            variables: values.keys().cloned().collect(),
-            ctx,
-        }
+        Self { variables, ctx }
     }
 
     pub fn list_binding(&mut self, name: String, value: Vec<HashMap<String, String>>) {
@@ -146,12 +145,12 @@ impl<'a> Context<'a> {
 
 impl Default for Context<'_> {
     fn default() -> Self {
-        Self::new(String::default(), &HashMap::default())
+        Self::new(String::default(), HashMap::default())
     }
 }
 
-impl From<&HashMap<String, String>> for Context<'_> {
-    fn from(value: &HashMap<String, String>) -> Self {
+impl From<HashMap<String, String>> for Context<'_> {
+    fn from(value: HashMap<String, String>) -> Self {
         Self::new(String::default(), value)
     }
 }
@@ -411,7 +410,7 @@ mod tests {
     fn predicate_no_key() {
         let pred = Predicate::parse("there.not == 42").expect("failed to parse");
         assert_eq!(
-            pred.test(&(&HashMap::from([("there".to_string(), String::default())])).into()),
+            pred.test(&HashMap::from([("there".to_string(), String::default())]).into()),
             Ok(false)
         );
     }

limitador/src/storage/disk/rocksdb_storage.rs

@@ -250,7 +250,7 @@ mod tests {
             vec!["app_id".try_into().expect("failed parsing!")],
         );
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(limit, &ctx)
             .unwrap()
             .expect("must have a counter");

limitador/src/storage/distributed/mod.rs

@@ -341,7 +341,7 @@ fn encode_limit_to_key(limit: &Limit) -> Vec<u8> {
         .into_iter()
         .map(|k| (k, "".to_string()))
         .collect();
-    let ctx = (&vars).into();
+    let ctx = vars.into();
     let counter = Counter::new(limit.clone(), &ctx)
         .expect("counter creation can't fail! faked vars!")
         .expect("must have a counter");

limitador/src/storage/in_memory.rs

@@ -270,7 +270,7 @@ mod tests {
             vec!["app_id".try_into().expect("failed parsing!")],
         );
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter_1 = Counter::new(limit_1, &ctx)
             .expect("counter creation failed!")
             .expect("Should have a counter");

limitador/src/storage/keys.rs

@@ -154,7 +154,7 @@ mod tests {
             vec!["app_id".try_into().expect("failed parsing!")],
         );
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(limit.clone(), &ctx)
             .expect("counter creation failed!")
             .expect("must have a counter");
@@ -173,7 +173,7 @@ mod tests {
             vec!["app_id".try_into().expect("failed parsing!")],
         );
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(limit.clone(), &ctx)
             .expect("counter creation failed!")
             .expect("must have a counter");
@@ -387,7 +387,7 @@ pub mod bin {
             vars.insert("role".to_string(), "admin".to_string());
             vars.insert("app_id".to_string(), "123".to_string());
             vars.insert("wat".to_string(), "dunno".to_string());
-            let ctx = (&vars).into();
+            let ctx = vars.into();
             let counter = Counter::new(limit.clone(), &ctx)
                 .expect("counter creation failed!")
                 .expect("must have a counter");
@@ -411,7 +411,7 @@ pub mod bin {
             );
             let mut variables = HashMap::default();
             variables.insert("app_id".to_string(), "123".to_string());
-            let ctx = (&variables).into();
+            let ctx = variables.into();
             let counter = Counter::new(limit.clone(), &ctx)
                 .expect("counter creation failed!")
                 .expect("must have a counter");
@@ -430,7 +430,7 @@ pub mod bin {
                 vec!["app_id".try_into().expect("failed parsing!")],
             );
             let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-            let ctx = (&map).into();
+            let ctx = map.into();
             let counter = Counter::new(limit, &ctx)
                 .expect("counter creation failed!")
                 .expect("must have a counter");
@@ -460,7 +460,7 @@ pub mod bin {
             );
 
             let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-            let ctx = (&map).into();
+            let ctx = map.into();
             let counter_with_id = Counter::new(limit_with_id, &ctx)
                 .expect("counter creation failed!")
                 .expect("must have a counter");

limitador/src/storage/redis/counters_cache.rs

@@ -676,7 +676,6 @@ mod tests {
         if let Some(overrides) = other_values {
             values.extend(overrides);
         }
-        let ctx = (&values).into();
         Counter::new(
             Limit::new(
                 "test_namespace",
@@ -685,7 +684,7 @@ mod tests {
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
                 vec!["app_id".try_into().expect("failed parsing!")],
             ),
-            &ctx,
+            &values.into(),
         )
         .expect("failed creating counter")
         .expect("Should have a counter")

limitador/src/storage/redis/redis_cached.rs

@@ -440,7 +440,7 @@ mod tests {
 
         let mut counters_and_deltas = HashMap::new();
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(
             Limit::new(
                 "test_namespace",
@@ -506,7 +506,7 @@ mod tests {
     #[tokio::test]
     async fn flush_batcher_and_update_counters_test() {
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(
             Limit::new(
                 "test_namespace",
@@ -569,7 +569,7 @@ mod tests {
     #[tokio::test]
     async fn flush_batcher_reverts_on_err() {
         let map = HashMap::from([("app_id".to_string(), "foo".to_string())]);
-        let ctx = (&map).into();
+        let ctx = map.into();
         let counter = Counter::new(
             Limit::new(
                 "test_namespace",