From 1652ba872f9bd080d8f2aa8b75957cbaa4b863ac Mon Sep 17 00:00:00 2001
From: Alex Snaps <alex@wcgw.dev>
Date: Wed, 27 Nov 2024 14:59:01 -0500
Subject: [PATCH] Parse Limit's variables as exp and resolve

Signed-off-by: Alex Snaps <alex@wcgw.dev>
---
 limitador-server/src/envoy_rls/server.rs      | 10 +--
 .../src/http_api/request_types.rs             |  8 +-
 limitador-server/src/http_api/server.rs       |  2 +-
 limitador/benches/bench.rs                    |  2 +-
 limitador/src/counter.rs                      | 52 ++++++++++-
 limitador/src/lib.rs                          | 24 +++--
 limitador/src/limit.rs                        | 79 +++++++++++-----
 limitador/src/limit/cel.rs                    | 29 +++++-
 limitador/src/storage/disk/rocksdb_storage.rs |  8 +-
 limitador/src/storage/distributed/mod.rs      | 14 ++-
 limitador/src/storage/in_memory.rs            | 19 ++--
 limitador/src/storage/keys.rs                 | 90 +++++++++++++------
 limitador/src/storage/redis/counters_cache.rs |  3 +-
 limitador/src/storage/redis/redis_cached.rs   | 21 +++--
 limitador/tests/integration_tests.rs          | 84 ++++++++---------
 15 files changed, 304 insertions(+), 141 deletions(-)

diff --git a/limitador-server/src/envoy_rls/server.rs b/limitador-server/src/envoy_rls/server.rs
index b6c3e7a0..8f252ce5 100644
--- a/limitador-server/src/envoy_rls/server.rs
+++ b/limitador-server/src/envoy_rls/server.rs
@@ -254,7 +254,7 @@ mod tests {
             1,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let limiter = RateLimiter::new(10_000);
@@ -399,7 +399,7 @@ mod tests {
                 10,
                 60,
                 vec!["x == '1'".try_into().expect("failed parsing!")],
-                vec!["z"],
+                vec!["z".try_into().expect("failed parsing!")],
             ),
             Limit::new(
                 namespace,
@@ -409,7 +409,7 @@ mod tests {
                     "x == '1'".try_into().expect("failed parsing!"),
                     "y == '2'".try_into().expect("failed parsing!"),
                 ],
-                vec!["z"],
+                vec!["z".try_into().expect("failed parsing!")],
             ),
         ]
         .into_iter()
@@ -479,7 +479,7 @@ mod tests {
             10,
             60,
             vec!["x == '1'".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         let limiter = RateLimiter::new(10_000);
@@ -554,7 +554,7 @@ mod tests {
             1,
             60,
             vec!["x == '1'".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         let limiter = RateLimiter::new(10_000);
diff --git a/limitador-server/src/http_api/request_types.rs b/limitador-server/src/http_api/request_types.rs
index 99adc8b6..6939ba38 100644
--- a/limitador-server/src/http_api/request_types.rs
+++ b/limitador-server/src/http_api/request_types.rs
@@ -1,5 +1,5 @@
 use limitador::counter::Counter as LimitadorCounter;
-use limitador::limit::{Limit as LimitadorLimit, ParseError, Predicate};
+use limitador::limit::{Expression, Limit as LimitadorLimit, ParseError, Predicate};
 use paperclip::actix::Apiv2Schema;
 use serde::{Deserialize, Serialize};
 use std::collections::{BTreeMap, HashMap};
@@ -46,6 +46,8 @@ impl TryFrom<Limit> for LimitadorLimit {
     fn try_from(limit: Limit) -> Result<Self, Self::Error> {
         let conditions: Result<Vec<Predicate>, ParseError> =
             limit.conditions.into_iter().map(|p| p.try_into()).collect();
+        let variables: Result<Vec<Expression>, ParseError> =
+            limit.variables.into_iter().map(|v| v.try_into()).collect();
 
         let mut limitador_limit = if let Some(id) = limit.id {
             Self::with_id(
@@ -54,7 +56,7 @@ impl TryFrom<Limit> for LimitadorLimit {
                 limit.max_value,
                 limit.seconds,
                 conditions?,
-                limit.variables,
+                variables?,
             )
         } else {
             Self::new(
@@ -62,7 +64,7 @@ impl TryFrom<Limit> for LimitadorLimit {
                 limit.max_value,
                 limit.seconds,
                 conditions?,
-                limit.variables,
+                variables?,
             )
         };
 
diff --git a/limitador-server/src/http_api/server.rs b/limitador-server/src/http_api/server.rs
index d2c26b03..d436e2a0 100644
--- a/limitador-server/src/http_api/server.rs
+++ b/limitador-server/src/http_api/server.rs
@@ -554,7 +554,7 @@ mod tests {
             max,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         match &limiter {
diff --git a/limitador/benches/bench.rs b/limitador/benches/bench.rs
index edccd9e2..b6c0cf70 100644
--- a/limitador/benches/bench.rs
+++ b/limitador/benches/bench.rs
@@ -540,7 +540,7 @@ fn generate_test_limits(scenario: &TestScenario) -> (Vec<Limit>, Vec<TestCallPar
     let mut variables = vec![];
     for idx_var in 0..scenario.n_vars_per_limit {
         let var_name = format!("var_{idx_var}");
-        variables.push(var_name.clone());
+        variables.push(var_name.clone().try_into().expect("failed parsing!"));
         test_values.insert(var_name, "1".into());
     }
 
diff --git a/limitador/src/counter.rs b/limitador/src/counter.rs
index e9f2b71a..2c5494bf 100644
--- a/limitador/src/counter.rs
+++ b/limitador/src/counter.rs
@@ -1,4 +1,5 @@
 use crate::limit::{Limit, Namespace};
+use crate::LimitadorResult;
 use serde::{Deserialize, Serialize};
 use std::collections::{BTreeMap, HashMap};
 use std::hash::{Hash, Hasher};
@@ -16,19 +17,37 @@ pub struct Counter {
 }
 
 impl Counter {
-    pub fn new<L: Into<Arc<Limit>>>(limit: L, set_variables: HashMap<String, String>) -> Self {
-        // TODO: check that all the variables defined in the limit are set.
+    pub fn new<L: Into<Arc<Limit>>>(
+        limit: L,
+        set_variables: HashMap<String, String>,
+    ) -> LimitadorResult<Self> {
+        let limit = limit.into();
+        let mut vars = set_variables;
+        vars.retain(|var, _| limit.has_variable(var));
 
+        let variables = limit.resolve_variables(vars)?;
+        Ok(Self {
+            limit,
+            set_variables: variables,
+            remaining: None,
+            expires_in: None,
+        })
+    }
+
+    pub(super) fn resolved_vars<L: Into<Arc<Limit>>>(
+        limit: L,
+        set_variables: HashMap<String, String>,
+    ) -> LimitadorResult<Self> {
         let limit = limit.into();
         let mut vars = set_variables;
         vars.retain(|var, _| limit.has_variable(var));
 
-        Self {
+        Ok(Self {
             limit,
             set_variables: vars.into_iter().collect(),
             remaining: None,
             expires_in: None,
-        }
+        })
     }
 
     #[cfg(any(feature = "redis_storage", feature = "disk_storage"))]
@@ -120,3 +139,28 @@ impl PartialEq for Counter {
         self.limit == other.limit && self.set_variables == other.set_variables
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::counter::Counter;
+    use crate::limit::Limit;
+    use std::collections::HashMap;
+
+    #[test]
+    fn resolves_variables() {
+        let limit = Limit::new(
+            "",
+            10,
+            60,
+            Vec::default(),
+            ["int(x) * 3".try_into().expect("failed parsing!")],
+        );
+        let key = "x".to_string();
+        let counter = Counter::new(limit, HashMap::from([(key.clone(), "14".to_string())]))
+            .expect("failed creating counter");
+        assert_eq!(
+            counter.set_variables.get(&key),
+            Some("42".to_string()).as_ref()
+        );
+    }
+}
diff --git a/limitador/src/lib.rs b/limitador/src/lib.rs
index 66a82e35..c7a652b4 100644
--- a/limitador/src/lib.rs
+++ b/limitador/src/lib.rs
@@ -55,7 +55,7 @@
 //!      10,
 //!      60,
 //!      vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-//!      vec!["user_id"],
+//!      vec!["user_id".try_into().expect("failed parsing!")],
 //! );
 //! ```
 //!
@@ -72,7 +72,7 @@
 //!      10,
 //!      60,
 //!      vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-//!      vec!["user_id"],
+//!      vec!["user_id".try_into().expect("failed parsing!")],
 //! );
 //! let mut rate_limiter = RateLimiter::new(1000);
 //!
@@ -104,7 +104,7 @@
 //!      2,
 //!      60,
 //!      vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-//!      vec!["user_id"],
+//!      vec!["user_id".try_into().expect("failed parsing!")],
 //! );
 //! rate_limiter.add_limit(limit);
 //!
@@ -168,7 +168,7 @@
 //!      10,
 //!      60,
 //!      vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-//!      vec!["user_id"],
+//!      vec!["user_id".try_into().expect("failed parsing!")],
 //! );
 //!
 //! async {
@@ -481,13 +481,11 @@ impl RateLimiter {
     ) -> LimitadorResult<Vec<Counter>> {
         let limits = self.storage.get_limits(namespace);
 
-        let counters = limits
+        limits
             .iter()
             .filter(|lim| lim.applies(values))
             .map(|lim| Counter::new(Arc::clone(lim), values.clone()))
-            .collect();
-
-        Ok(counters)
+            .collect()
     }
 }
 
@@ -660,13 +658,11 @@ impl AsyncRateLimiter {
     ) -> LimitadorResult<Vec<Counter>> {
         let limits = self.storage.get_limits(namespace);
 
-        let counters = limits
+        limits
             .iter()
             .filter(|lim| lim.applies(values))
             .map(|lim| Counter::new(Arc::clone(lim), values.clone()))
-            .collect();
-
-        Ok(counters)
+            .collect()
     }
 }
 
@@ -693,7 +689,7 @@ fn classify_limits_by_namespace(
 
 #[cfg(test)]
 mod test {
-    use crate::limit::Limit;
+    use crate::limit::{Expression, Limit};
     use crate::RateLimiter;
     use std::collections::HashMap;
 
@@ -702,7 +698,7 @@ mod test {
         let rl = RateLimiter::new(100);
         let namespace = "foo";
 
-        let l = Limit::new(namespace, 42, 100, vec![], Vec::<String>::default());
+        let l = Limit::new(namespace, 42, 100, vec![], Vec::<Expression>::default());
         rl.add_limit(l.clone());
         let limits = rl.get_limits(&namespace.into());
         assert_eq!(limits.len(), 1);
diff --git a/limitador/src/limit.rs b/limitador/src/limit.rs
index afc3f054..a67af038 100644
--- a/limitador/src/limit.rs
+++ b/limitador/src/limit.rs
@@ -1,7 +1,7 @@
 use cel::Context;
 use serde::{Deserialize, Serialize};
 use std::cmp::Ordering;
-use std::collections::{BTreeSet, HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
 
@@ -45,7 +45,7 @@ pub struct Limit {
     // Need to sort to generate the same object when using the JSON as a key or
     // value in Redis.
     conditions: BTreeSet<Predicate>,
-    variables: BTreeSet<String>,
+    variables: BTreeSet<Expression>,
 }
 
 impl Limit {
@@ -54,7 +54,7 @@ impl Limit {
         max_value: u64,
         seconds: u64,
         conditions: impl IntoIterator<Item = Predicate>,
-        variables: impl IntoIterator<Item = impl Into<String>>,
+        variables: impl IntoIterator<Item = Expression>,
     ) -> Self
     where
         <N as TryInto<Namespace>>::Error: Debug,
@@ -66,7 +66,7 @@ impl Limit {
             seconds,
             name: None,
             conditions: conditions.into_iter().collect(),
-            variables: variables.into_iter().map(|var| var.into()).collect(),
+            variables: variables.into_iter().collect(),
         }
     }
 
@@ -76,7 +76,7 @@ impl Limit {
         max_value: u64,
         seconds: u64,
         conditions: impl IntoIterator<Item = Predicate>,
-        variables: impl IntoIterator<Item = impl Into<String>>,
+        variables: impl IntoIterator<Item = Expression>,
     ) -> Self {
         Self {
             id: Some(id.into()),
@@ -85,7 +85,7 @@ impl Limit {
             seconds,
             name: None,
             conditions: conditions.into_iter().collect(),
-            variables: variables.into_iter().map(|var| var.into()).collect(),
+            variables: variables.into_iter().collect(),
         }
     }
 
@@ -125,21 +125,41 @@ impl Limit {
     }
 
     pub fn variables(&self) -> HashSet<String> {
-        self.variables.iter().map(|var| var.into()).collect()
+        self.variables
+            .iter()
+            .map(|var| var.source().into())
+            .collect()
+    }
+
+    pub fn resolve_variables(
+        &self,
+        vars: HashMap<String, String>,
+    ) -> Result<BTreeMap<String, String>, EvaluationError> {
+        let ctx = Context::new(self, String::default(), &vars);
+        let mut map = BTreeMap::new();
+        for variable in &self.variables {
+            let name = variable.variables().concat().to_string();
+            let value = variable.eval(&ctx)?;
+            map.insert(name, value);
+        }
+        Ok(map)
     }
 
     #[cfg(feature = "disk_storage")]
     pub(crate) fn variables_for_key(&self) -> Vec<&str> {
         let mut variables = Vec::with_capacity(self.variables.len());
         for var in &self.variables {
-            variables.push(var.as_str());
+            variables.push(var.source());
         }
         variables.sort();
         variables
     }
 
     pub fn has_variable(&self, var: &str) -> bool {
-        self.variables.contains(var)
+        self.variables
+            .iter()
+            .flat_map(|v| v.variables())
+            .any(|v| v.as_str() == var)
     }
 
     pub fn applies(&self, values: &HashMap<String, String>) -> bool {
@@ -149,7 +169,10 @@ impl Limit {
             .iter()
             .all(|predicate| predicate.test(&ctx).unwrap());
 
-        let all_vars_are_set = self.variables.iter().all(|var| values.contains_key(var));
+        let all_vars_are_set = self
+            .variables
+            .iter()
+            .all(|var| values.contains_key(var.source()));
 
         all_conditions_apply && all_vars_are_set
     }
@@ -206,7 +229,7 @@ mod tests {
             10,
             60,
             vec!["x == \"5\"".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
         assert!(limit.name.is_none());
 
@@ -222,7 +245,7 @@ mod tests {
             10,
             60,
             vec!["x == \"5\"".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         let mut values: HashMap<String, String> = HashMap::new();
@@ -239,7 +262,7 @@ mod tests {
             10,
             60,
             vec!["x == \"5\"".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         let mut values: HashMap<String, String> = HashMap::new();
@@ -256,7 +279,7 @@ mod tests {
             10,
             60,
             vec!["x == \"5\"".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         // Notice that "x" is not set
@@ -274,7 +297,7 @@ mod tests {
             10,
             60,
             vec!["x == \"5\"".try_into().expect("failed parsing!")],
-            vec!["y"],
+            vec!["y".try_into().expect("failed parsing!")],
         );
 
         // Notice that "y" is not set
@@ -294,7 +317,7 @@ mod tests {
                 "x == \"5\"".try_into().expect("failed parsing!"),
                 "y == \"2\"".try_into().expect("failed parsing!"),
             ],
-            vec!["z"],
+            vec!["z".try_into().expect("failed parsing!")],
         );
 
         let mut values: HashMap<String, String> = HashMap::new();
@@ -315,7 +338,7 @@ mod tests {
                 "x == \"5\"".try_into().expect("failed parsing!"),
                 "y == \"2\"".try_into().expect("failed parsing!"),
             ],
-            vec!["z"],
+            vec!["z".try_into().expect("failed parsing!")],
         );
 
         let mut values: HashMap<String, String> = HashMap::new();
@@ -334,7 +357,7 @@ mod tests {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         assert_eq!(limit.id(), Some("test_id"))
@@ -348,7 +371,7 @@ mod tests {
             42,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let mut limit2 = Limit::new(
@@ -364,6 +387,18 @@ mod tests {
         assert_eq!(limit1, limit2);
     }
 
+    #[test]
+    fn resolves_variables() {
+        let limit = Limit::new(
+            "",
+            10,
+            60,
+            Vec::default(),
+            ["int(x) * 3".try_into().expect("failed parsing!")],
+        );
+        assert!(limit.has_variable("x"));
+    }
+
     #[test]
     fn conditions_have_limit_info() {
         let mut limit = Limit::new(
@@ -373,7 +408,7 @@ mod tests {
             vec!["limit.name == 'named_limit'"
                 .try_into()
                 .expect("failed parsing!")],
-            Vec::<String>::default(),
+            Vec::default(),
         );
         assert!(!limit.applies(&HashMap::default()));
 
@@ -389,7 +424,7 @@ mod tests {
                 "limit.id == 'my_id'".try_into().expect("failed parsing!"),
                 "limit.name == null".try_into().expect("failed parsing!"),
             ],
-            Vec::<String>::default(),
+            Vec::default(),
         );
         assert!(limit.applies(&HashMap::default()));
 
@@ -401,7 +436,7 @@ mod tests {
             vec!["limit.id == 'other_id'"
                 .try_into()
                 .expect("failed parsing!")],
-            Vec::<String>::default(),
+            Vec::default(),
         );
         assert!(!limit.applies(&HashMap::default()));
     }
diff --git a/limitador/src/limit/cel.rs b/limitador/src/limit/cel.rs
index d910c0c9..df0f77f2 100644
--- a/limitador/src/limit/cel.rs
+++ b/limitador/src/limit/cel.rs
@@ -149,6 +149,19 @@ impl Expression {
     pub fn resolve(&self, ctx: &Context) -> Result<Value, ExecutionError> {
         Value::resolve(&self.expression, &ctx.ctx)
     }
+
+    pub fn source(&self) -> &str {
+        self.source.as_str()
+    }
+
+    pub fn variables(&self) -> Vec<String> {
+        self.expression
+            .references()
+            .variables()
+            .into_iter()
+            .map(String::from)
+            .collect()
+    }
 }
 
 fn err_on_value(val: Value) -> EvaluationError {
@@ -178,6 +191,14 @@ impl TryFrom<String> for Expression {
     }
 }
 
+impl TryFrom<&str> for Predicate {
+    type Error = ParseError;
+
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        Self::parse(value)
+    }
+}
+
 impl From<Expression> for String {
     fn from(value: Expression) -> Self {
         value.source
@@ -192,6 +213,12 @@ impl PartialEq<Self> for Expression {
 
 impl Eq for Expression {}
 
+impl Hash for Expression {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.source.hash(state);
+    }
+}
+
 impl PartialOrd<Self> for Expression {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))
@@ -276,7 +303,7 @@ impl TryFrom<String> for Predicate {
     }
 }
 
-impl TryFrom<&str> for Predicate {
+impl TryFrom<&str> for Expression {
     type Error = ParseError;
 
     fn try_from(value: &str) -> Result<Self, Self::Error> {
diff --git a/limitador/src/storage/disk/rocksdb_storage.rs b/limitador/src/storage/disk/rocksdb_storage.rs
index 68ef7369..05d04f04 100644
--- a/limitador/src/storage/disk/rocksdb_storage.rs
+++ b/limitador/src/storage/disk/rocksdb_storage.rs
@@ -247,9 +247,13 @@ mod tests {
             1,
             2,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
-        let counter = Counter::new(limit, HashMap::default());
+        let counter = Counter::new(
+            limit,
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .unwrap();
 
         let tmp = TempDir::new().expect("We should have a dir!");
         {
diff --git a/limitador/src/storage/distributed/mod.rs b/limitador/src/storage/distributed/mod.rs
index 020918c5..81d9d6ed 100644
--- a/limitador/src/storage/distributed/mod.rs
+++ b/limitador/src/storage/distributed/mod.rs
@@ -47,7 +47,8 @@ impl CounterStorage for CrInMemoryStorage {
             let key = encode_limit_to_key(limit);
             limits.entry(key.clone()).or_insert(Arc::new(CounterEntry {
                 key,
-                counter: Counter::new(limit.clone(), HashMap::default()),
+                counter: Counter::new(limit.clone(), HashMap::default())
+                    .expect("counter creation can't fail! no vars to resolve!"),
                 value: CrCounterValue::new(
                     self.identifier.clone(),
                     limit.max_value(),
@@ -333,6 +334,15 @@ fn encode_counter_to_key(counter: &Counter) -> Vec<u8> {
 }
 
 fn encode_limit_to_key(limit: &Limit) -> Vec<u8> {
-    let counter = Counter::new(limit.clone(), HashMap::default());
+    // fixme this is broken!
+    let counter = Counter::new(
+        limit.clone(),
+        limit
+            .variables()
+            .into_iter()
+            .map(|k| (k, "".to_string()))
+            .collect(),
+    )
+    .expect("counter creation can't fail! faked vars!");
     key_for_counter_v2(&counter)
 }
diff --git a/limitador/src/storage/in_memory.rs b/limitador/src/storage/in_memory.rs
index dca294b7..ac5e2c07 100644
--- a/limitador/src/storage/in_memory.rs
+++ b/limitador/src/storage/in_memory.rs
@@ -211,7 +211,8 @@ impl InMemoryStorage {
         for (limit, counter) in self.simple_limits.read().unwrap().iter() {
             if limit.namespace() == namespace {
                 res.insert(
-                    Counter::new(limit.clone(), HashMap::default()),
+                    // todo fixme
+                    Counter::new(limit.clone(), HashMap::default()).unwrap(),
                     counter.clone(),
                 );
             }
@@ -257,17 +258,25 @@ mod tests {
             1,
             1,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
         let limit_2 = Limit::new(
             namespace,
             1,
             10,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
-        let counter_1 = Counter::new(limit_1, HashMap::default());
-        let counter_2 = Counter::new(limit_2, HashMap::default());
+        let counter_1 = Counter::new(
+            limit_1,
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
+        let counter_2 = Counter::new(
+            limit_2,
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
         storage.update_counter(&counter_1, 1).unwrap();
         storage.update_counter(&counter_2, 1).unwrap();
 
diff --git a/limitador/src/storage/keys.rs b/limitador/src/storage/keys.rs
index 9bbeddfa..9d4e1d83 100644
--- a/limitador/src/storage/keys.rs
+++ b/limitador/src/storage/keys.rs
@@ -120,7 +120,7 @@ mod tests {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
         assert_eq!(
             "namespace:{example.com},counters_of_limit:{\"namespace\":\"example.com\",\"seconds\":60,\"conditions\":[\"req_method == 'GET'\"],\"variables\":[\"app_id\"]}".as_bytes(),
@@ -135,7 +135,7 @@ mod tests {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
         assert_eq!(
             "\u{2}\u{7}test_id".as_bytes(),
@@ -151,9 +151,13 @@ mod tests {
             1,
             1,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
-        let counter = Counter::new(limit.clone(), HashMap::default());
+        let counter = Counter::new(
+            limit.clone(),
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
         let raw = key_for_counter(&counter);
         assert_eq!(counter, partial_counter_from_counter_key(&raw));
     }
@@ -166,9 +170,13 @@ mod tests {
             1,
             1,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
-        let counter = Counter::new(limit.clone(), HashMap::default());
+        let counter = Counter::new(
+            limit.clone(),
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
         let mut other = counter.clone();
         other.set_remaining(123);
         other.set_expires_in(Duration::from_millis(456));
@@ -286,11 +294,11 @@ pub mod bin {
                     seconds,
                     conditions
                         .into_iter()
-                        .map(|p| p.try_into().expect("condition corrupted!"))
-                        .collect::<Vec<Predicate>>(),
-                    map.keys(),
+                        .map(|p| p.try_into().expect("condition corrupted!")),
+                    map.keys()
+                        .map(|var| var.as_str().try_into().expect("variable corrupted!")),
                 );
-                Counter::new(limit, map)
+                Counter::resolved_vars(limit, map).expect("counter creation failed!")
             }
             2u8 => {
                 let IdCounterKey { id, variables } = postcard::from_bytes(key).unwrap();
@@ -300,9 +308,16 @@ pub mod bin {
                     .collect();
 
                 // we are not able to rebuild the full limit since we only have the id and variables.
-                let limit =
-                    Limit::with_id::<&str, &str>(id, "", u64::default(), 0, vec![], map.keys());
-                Counter::new(limit, map)
+                let limit = Limit::with_id::<&str, &str>(
+                    id,
+                    "",
+                    u64::default(),
+                    0,
+                    vec![],
+                    map.keys()
+                        .map(|var| var.as_str().try_into().expect("variable corrupted!")),
+                );
+                Counter::resolved_vars(limit, map).expect("counter creation failed!")
             }
             _ => panic!("Unknown version: {}", version),
         }
@@ -338,9 +353,10 @@ pub mod bin {
                 .into_iter()
                 .map(|p| p.try_into().expect("condition corrupted!"))
                 .collect::<Vec<Predicate>>(),
-            map.keys(),
+            map.keys()
+                .map(|p| p.as_str().try_into().expect("variable corrupted!")),
         );
-        Counter::new(limit, map)
+        Counter::resolved_vars(limit, map).unwrap()
     }
 
     #[cfg(test)]
@@ -361,13 +377,17 @@ pub mod bin {
                 1,
                 2,
                 vec!["foo == 'bar'".try_into().expect("failed parsing!")],
-                vec!["app_id", "role", "wat"],
+                vec![
+                    "app_id".try_into().expect("failed parsing!"),
+                    "role".try_into().expect("failed parsing!"),
+                    "wat".try_into().expect("failed parsing!"),
+                ],
             );
             let mut vars = HashMap::default();
             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 counter = Counter::new(limit.clone(), vars);
+            let counter = Counter::new(limit.clone(), vars).expect("counter creation failed!");
 
             let raw = key_for_counter(&counter);
             let key_back: CounterKey =
@@ -384,11 +404,11 @@ pub mod bin {
                 1,
                 1,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             );
             let mut variables = HashMap::default();
             variables.insert("app_id".to_string(), "123".to_string());
-            let counter = Counter::new(limit.clone(), variables);
+            let counter = Counter::new(limit.clone(), variables).expect("counter creation failed!");
             let raw = key_for_counter(&counter);
             assert_eq!(counter, partial_counter_from_counter_key(&raw));
         }
@@ -401,9 +421,13 @@ pub mod bin {
                 1,
                 1,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             );
-            let counter = Counter::new(limit, HashMap::default());
+            let counter = Counter::new(
+                limit,
+                HashMap::from([("app_id".to_string(), "foo".to_string())]),
+            )
+            .expect("counter creation failed!");
             let serialized_counter = key_for_counter(&counter);
 
             let prefix = prefix_for_namespace(namespace);
@@ -418,7 +442,7 @@ pub mod bin {
                 1,
                 1,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             );
             let limit_with_id = Limit::with_id(
                 "id200",
@@ -426,27 +450,35 @@ pub mod bin {
                 1,
                 1,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             );
 
-            let counter_with_id = Counter::new(limit_with_id, HashMap::default());
+            let counter_with_id = Counter::new(
+                limit_with_id,
+                HashMap::from([("app_id".to_string(), "foo".to_string())]),
+            )
+            .expect("counter creation failed!");
             let serialized_with_id_counter = key_for_counter(&counter_with_id);
 
-            let counter_without_id = Counter::new(limit_without_id, HashMap::default());
+            let counter_without_id = Counter::new(
+                limit_without_id,
+                HashMap::from([("app_id".to_string(), "foo".to_string())]),
+            )
+            .expect("counter creation failed!");
             let serialized_without_id_counter = key_for_counter(&counter_without_id);
 
             // the original key_for_counter continues to encode kinda big
-            assert_eq!(serialized_without_id_counter.len(), 35);
-            assert_eq!(serialized_with_id_counter.len(), 35);
+            assert_eq!(serialized_without_id_counter.len(), 46);
+            assert_eq!(serialized_with_id_counter.len(), 46);
 
             // serialized_counter_v2 will only encode the id.... so it will be smaller for
             // counters with an id.
             let serialized_counter_with_id_v2 = key_for_counter_v2(&counter_with_id);
-            assert_eq!(serialized_counter_with_id_v2.clone().len(), 8);
+            assert_eq!(serialized_counter_with_id_v2.clone().len(), 19);
 
             // but continues to be large for counters without an id.
             let serialized_counter_without_id_v2 = key_for_counter_v2(&counter_without_id);
-            assert_eq!(serialized_counter_without_id_v2.clone().len(), 36);
+            assert_eq!(serialized_counter_without_id_v2.clone().len(), 47);
         }
     }
 }
diff --git a/limitador/src/storage/redis/counters_cache.rs b/limitador/src/storage/redis/counters_cache.rs
index 1924d63f..37653024 100644
--- a/limitador/src/storage/redis/counters_cache.rs
+++ b/limitador/src/storage/redis/counters_cache.rs
@@ -681,9 +681,10 @@ mod tests {
                 max_val,
                 60,
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
             values,
         )
+        .expect("failed creating counter")
     }
 }
diff --git a/limitador/src/storage/redis/redis_cached.rs b/limitador/src/storage/redis/redis_cached.rs
index 742d2986..b9b12ca7 100644
--- a/limitador/src/storage/redis/redis_cached.rs
+++ b/limitador/src/storage/redis/redis_cached.rs
@@ -445,10 +445,11 @@ mod tests {
                 10,
                 60,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
-            Default::default(),
-        );
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
 
         let arc = Arc::new(CachedCounterValue::from_authority(
             &counter,
@@ -507,10 +508,11 @@ mod tests {
                 10,
                 60,
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
-            Default::default(),
-        );
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
 
         let mock_response = Value::Array(vec![
             Value::Int(8),
@@ -566,10 +568,11 @@ mod tests {
                 10,
                 60,
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
-            Default::default(),
-        );
+            HashMap::from([("app_id".to_string(), "foo".to_string())]),
+        )
+        .expect("counter creation failed!");
 
         let error: RedisError = io::Error::new(io::ErrorKind::TimedOut, "That was long!").into();
         assert!(error.is_timeout());
diff --git a/limitador/tests/integration_tests.rs b/limitador/tests/integration_tests.rs
index 7572d766..7a71372c 100644
--- a/limitador/tests/integration_tests.rs
+++ b/limitador/tests/integration_tests.rs
@@ -221,14 +221,14 @@ mod test {
                 10,
                 60,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
             Limit::new(
                 "second_namespace",
                 20,
                 60,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
         ];
 
@@ -253,7 +253,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let lim2 = Limit::new(
@@ -261,7 +261,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         for limit in [&lim1, &lim2] {
@@ -285,7 +285,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -305,7 +305,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            Vec::<String>::new(),
+            Vec::default(),
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -327,7 +327,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let limit_2 = Limit::new(
@@ -335,7 +335,7 @@ mod test {
             5,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit_1).await;
@@ -354,7 +354,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -371,7 +371,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -406,14 +406,14 @@ mod test {
                 10,
                 60,
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
             Limit::new(
                 namespace,
                 5,
                 60,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
         ];
 
@@ -438,7 +438,7 @@ mod test {
                 10,
                 60,
                 vec!["x == '10'".try_into().expect("failed parsing!")],
-                vec!["z"],
+                vec!["z".try_into().expect("failed parsing!")],
             ))
             .await;
         rate_limiter
@@ -447,7 +447,7 @@ mod test {
                 5,
                 60,
                 vec!["x == '10'".try_into().expect("failed parsing!")],
-                vec!["z"],
+                vec!["z".try_into().expect("failed parsing!")],
             ))
             .await;
 
@@ -464,7 +464,7 @@ mod test {
             5,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -498,7 +498,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -535,7 +535,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -572,14 +572,14 @@ mod test {
                 max_hits,
                 60,
                 vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
             Limit::new(
                 namespace,
                 max_hits + 1,
                 60,
                 vec!["req_method == 'POST'".try_into().expect("failed parsing!")],
-                vec!["app_id"],
+                vec!["app_id".try_into().expect("failed parsing!")],
             ),
         ];
 
@@ -640,7 +640,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -675,7 +675,7 @@ mod test {
             max,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -698,7 +698,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -752,7 +752,7 @@ mod test {
             0, // So reporting 1 more would not be allowed
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -776,7 +776,7 @@ mod test {
             0, // So reporting 1 more would not be allowed
             60,
             Vec::default(), // unconditional
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -799,7 +799,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -836,7 +836,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -886,7 +886,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -915,7 +915,7 @@ mod test {
             0, // So reporting 1 more would not be allowed
             60,
             Vec::default(), // unconditional
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -943,7 +943,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -1000,7 +1000,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -1021,7 +1021,7 @@ mod test {
             10,
             limit_time,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -1046,7 +1046,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let second_limit = Limit::new(
@@ -1054,7 +1054,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter
@@ -1085,7 +1085,7 @@ mod test {
             max_value,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit).await;
@@ -1124,7 +1124,7 @@ mod test {
             10,
             1,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let limit_to_be_deleted = Limit::new(
@@ -1132,7 +1132,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         for limit in [&limit_to_be_kept, &limit_to_be_deleted].iter() {
@@ -1158,7 +1158,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let limit_update = Limit::new(
@@ -1166,7 +1166,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         rate_limiter.add_limit(&limit_orig).await;
@@ -1190,7 +1190,7 @@ mod test {
             10,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let limit_2 = Limit::new(
@@ -1198,7 +1198,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         let mut limit_3 = Limit::new(
@@ -1206,7 +1206,7 @@ mod test {
             20,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
         limit_3.set_name("Name is irrelevant too".to_owned());
 
@@ -1235,7 +1235,7 @@ mod test {
             max_hits,
             60,
             vec!["req_method == 'GET'".try_into().expect("failed parsing!")],
-            vec!["app_id"],
+            vec!["app_id".try_into().expect("failed parsing!")],
         );
 
         for rate_limiter in rate_limiters.iter() {