Merge pull request #332 from Kuadrant/u64

All i64 to unsigned ones

limitador-server/src/envoy_rls/server.rs

@@ -1,6 +1,5 @@
 use opentelemetry::global;
 use opentelemetry::propagation::Extractor;
-use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::sync::Arc;
 
@@ -100,15 +99,15 @@ impl RateLimitService for MyRateLimiter {
             Limiter::Blocking(limiter) => limiter.check_rate_limited_and_update(
                 &namespace,
                 &values,
-                i64::from(hits_addend),
+                u64::from(hits_addend),
                 self.rate_limit_headers != RateLimitHeaders::None,
             ),
             Limiter::Async(limiter) => {
                 limiter
                     .check_rate_limited_and_update(
                         &namespace,
                         &values,
-                        i64::from(hits_addend),
+                        u64::from(hits_addend),
                         self.rate_limit_headers != RateLimitHeaders::None,
                     )
                     .await
@@ -170,11 +169,7 @@ pub fn to_response_header(
             counters.sort_by(|a, b| {
                 let a_remaining = a.remaining().unwrap_or(a.max_value());
                 let b_remaining = b.remaining().unwrap_or(b.max_value());
-                if a_remaining - b_remaining < 0 {
-                    Ordering::Less
-                } else {
-                    Ordering::Greater
-                }
+                a_remaining.cmp(&b_remaining)
             });
 
             let mut all_limits_text = String::with_capacity(20 * counters.len());
@@ -194,10 +189,7 @@ pub fn to_response_header(
                     value: format!("{}{}", counter.max_value(), all_limits_text),
                 });
 
-                let mut remaining = counter.remaining().unwrap_or(counter.max_value());
-                if remaining < 0 {
-                    remaining = 0
-                }
+                let remaining = counter.remaining().unwrap_or(counter.max_value());
                 headers.push(HeaderValue {
                     key: "X-RateLimit-Remaining".to_string(),
                     value: format!("{}", remaining),

limitador-server/src/http_api/request_types.rs

@@ -12,14 +12,14 @@ use std::collections::HashMap;
 pub struct CheckAndReportInfo {
     pub namespace: String,
     pub values: HashMap<String, String>,
-    pub delta: i64,
+    pub delta: u64,
     pub response_headers: Option<String>,
 }
 
 #[derive(Debug, Eq, PartialEq, Serialize, Deserialize, Apiv2Schema)]
 pub struct Limit {
     namespace: String,
-    max_value: i64,
+    max_value: u64,
     seconds: u64,
     name: Option<String>,
     conditions: Vec<String>,
@@ -61,7 +61,7 @@ impl From<Limit> for LimitadorLimit {
 pub struct Counter {
     limit: Limit,
     set_variables: HashMap<String, String>,
-    remaining: Option<i64>,
+    remaining: Option<u64>,
     expires_in_seconds: Option<u64>,
 }
 

limitador-server/src/http_api/server.rs

@@ -11,7 +11,6 @@ use paperclip::actix::{
     // extension trait for actix_web::App and proc-macro attributes
     OpenApiExt,
 };
-use std::cmp::Ordering;
 use std::fmt;
 use std::sync::Arc;
 
@@ -248,11 +247,7 @@ pub fn add_response_header(
             counters.sort_by(|a, b| {
                 let a_remaining = a.remaining().unwrap_or(a.max_value());
                 let b_remaining = b.remaining().unwrap_or(b.max_value());
-                if a_remaining - b_remaining < 0 {
-                    Ordering::Less
-                } else {
-                    Ordering::Greater
-                }
+                a_remaining.cmp(&b_remaining)
             });
 
             let mut all_limits_text = String::with_capacity(20 * counters.len());
@@ -272,10 +267,7 @@ pub fn add_response_header(
                     format!("{}{}", counter.max_value(), all_limits_text),
                 ));
 
-                let mut remaining = counter.remaining().unwrap_or(counter.max_value());
-                if remaining < 0 {
-                    remaining = 0
-                }
+                let remaining = counter.remaining().unwrap_or(counter.max_value());
                 resp.insert_header((
                     "X-RateLimit-Remaining".to_string(),
                     format!("{}", remaining),
@@ -581,7 +573,7 @@ mod tests {
         assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS);
     }
 
-    async fn create_test_limit(limiter: &Limiter, namespace: &str, max: i64) -> LimitadorLimit {
+    async fn create_test_limit(limiter: &Limiter, namespace: &str, max: u64) -> LimitadorLimit {
         // Create a limit
         let limit = LimitadorLimit::new(
             namespace,

limitador-server/src/main.rs

@@ -162,21 +162,16 @@ impl Limiter {
             Ok(f) => {
                 let parsed_limits: Result<Vec<Limit>, _> = serde_yaml::from_reader(f);
                 match parsed_limits {
-                    Ok(limits) => match find_first_negative_limit(&limits) {
-                        None => {
-                            match &self {
-                                Self::Blocking(limiter) => limiter.configure_with(limits)?,
-                                Self::Async(limiter) => limiter.configure_with(limits).await?,
-                            }
-                            if limitador::limit::check_deprecated_syntax_usages_and_reset() {
-                                error!("You are using deprecated syntax for your conditions! See the migration guide https://docs.kuadrant.io/limitador/doc/migrations/conditions/")
-                            }
-                            Ok(())
+                    Ok(limits) => {
+                        match &self {
+                            Self::Blocking(limiter) => limiter.configure_with(limits)?,
+                            Self::Async(limiter) => limiter.configure_with(limits).await?,
+                        }
+                        if limitador::limit::check_deprecated_syntax_usages_and_reset() {
+                            error!("You are using deprecated syntax for your conditions! See the migration guide https://docs.kuadrant.io/limitador/doc/migrations/conditions/")
                         }
-                        Some(index) => Err(LimitadorServerError::ConfigFile(format!(
-                            ".[{index}]: invalid value for `max_value`: positive integer expected"
-                        ))),
-                    },
+                        Ok(())
+                    }
                     Err(e) => Err(LimitadorServerError::ConfigFile(format!(
                         "Couldn't parse: {e}"
                     ))),
@@ -191,15 +186,6 @@ impl Limiter {
     }
 }
 
-fn find_first_negative_limit(limits: &[Limit]) -> Option<usize> {
-    for (index, limit) in limits.iter().enumerate() {
-        if limit.max_value() < 0 {
-            return Some(index);
-        }
-    }
-    None
-}
-
 #[actix_rt::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let config = {
@@ -588,28 +574,23 @@ fn create_config() -> (Configuration, &'static str) {
             Ok(f) => {
                 let parsed_limits: Result<Vec<Limit>, _> = serde_yaml::from_reader(f);
                 match parsed_limits {
-                    Ok(limits) => match find_first_negative_limit(&limits) {
-                        Some(index) => LimitadorServerError::ConfigFile(format!(
-                            ".[{index}]: invalid value for `max_value`: positive integer expected"
-                        )),
-                        None => {
-                            if limitador::limit::check_deprecated_syntax_usages_and_reset() {
-                                eprintln!("Deprecated syntax for conditions corrected!\n")
-                            }
+                    Ok(limits) => {
+                        if limitador::limit::check_deprecated_syntax_usages_and_reset() {
+                            eprintln!("Deprecated syntax for conditions corrected!\n")
+                        }
 
-                            let output: Vec<http_api::LimitVO> =
-                                limits.iter().map(|l| l.into()).collect();
-                            match serde_yaml::to_string(&output) {
-                                Ok(cfg) => {
-                                    println!("{cfg}");
-                                }
-                                Err(err) => {
-                                    eprintln!("Config file is valid, but can't be output: {err}");
-                                }
+                        let output: Vec<http_api::LimitVO> =
+                            limits.iter().map(|l| l.into()).collect();
+                        match serde_yaml::to_string(&output) {
+                            Ok(cfg) => {
+                                println!("{cfg}");
+                            }
+                            Err(err) => {
+                                eprintln!("Config file is valid, but can't be output: {err}");
                             }
-                            process::exit(0);
                         }
-                    },
+                        process::exit(0);
+                    }
                     Err(e) => LimitadorServerError::ConfigFile(format!("Couldn't parse: {e}")),
                 }
             }
@@ -738,29 +719,3 @@ fn guess_cache_size() -> Option<u64> {
 fn leak<D: Display>(s: D) -> &'static str {
     return Box::leak(format!("{}", s).into_boxed_str());
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::find_first_negative_limit;
-    use limitador::limit::Limit;
-
-    #[test]
-    fn finds_negative_limits() {
-        let variables: [&str; 0] = [];
-        let mut limits: Vec<Limit> = vec![
-            Limit::new::<_, &str>("foo", 42, 10, [], variables),
-            Limit::new::<_, &str>("foo", -42, 10, [], variables),
-        ];
-
-        assert_eq!(find_first_negative_limit(&limits), Some(1));
-        limits[0].set_max_value(-42);
-        assert_eq!(find_first_negative_limit(&limits), Some(0));
-        limits[1].set_max_value(42);
-        assert_eq!(find_first_negative_limit(&limits), Some(0));
-        limits[0].set_max_value(42);
-        assert_eq!(find_first_negative_limit(&limits), None);
-
-        let nothing: [Limit; 0] = [];
-        assert_eq!(find_first_negative_limit(&nothing), None);
-    }
-}

limitador/benches/bench.rs

@@ -56,7 +56,7 @@ const TEST_SCENARIOS: &[&TestScenario] = &[
 struct TestCallParams {
     namespace: String,
     values: HashMap<String, String>,
-    delta: i64,
+    delta: u64,
 }
 
 impl Display for TestScenario {
@@ -280,7 +280,7 @@ fn generate_test_data(
         for limit_idx in 0..scenario.n_limits_per_ns {
             test_limits.push(Limit::new(
                 namespace.clone(),
-                i64::MAX,
+                u64::MAX,
                 ((limit_idx * 60) + 10) as u64,
                 conditions.clone(),
                 variables.clone(),

limitador/src/counter.rs

@@ -13,7 +13,7 @@ pub struct Counter {
     #[serde(serialize_with = "ordered_map")]
     set_variables: HashMap<String, String>,
 
-    remaining: Option<i64>,
+    remaining: Option<u64>,
     expires_in: Option<Duration>,
 }
 
@@ -53,7 +53,7 @@ impl Counter {
         &self.limit
     }
 
-    pub fn max_value(&self) -> i64 {
+    pub fn max_value(&self) -> u64 {
         self.limit.max_value()
     }
 
@@ -80,11 +80,11 @@ impl Counter {
         &self.set_variables
     }
 
-    pub fn remaining(&self) -> Option<i64> {
+    pub fn remaining(&self) -> Option<u64> {
         self.remaining
     }
 
-    pub fn set_remaining(&mut self, remaining: i64) {
+    pub fn set_remaining(&mut self, remaining: u64) {
         self.remaining = Some(remaining)
     }
 

limitador/src/lib.rs

@@ -310,7 +310,7 @@ impl RateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
     ) -> Result<bool, LimitadorError> {
         let counters = self.counters_that_apply(namespace, values)?;
 
@@ -332,7 +332,7 @@ impl RateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
     ) -> Result<(), LimitadorError> {
         let counters = self.counters_that_apply(namespace, values)?;
 
@@ -346,7 +346,7 @@ impl RateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
         load_counters: bool,
     ) -> Result<CheckResult, LimitadorError> {
         let mut counters = self.counters_that_apply(namespace, values)?;
@@ -482,7 +482,7 @@ impl AsyncRateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
     ) -> Result<bool, LimitadorError> {
         let counters = self.counters_that_apply(namespace, values).await?;
 
@@ -503,7 +503,7 @@ impl AsyncRateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
     ) -> Result<(), LimitadorError> {
         let counters = self.counters_that_apply(namespace, values).await?;
 
@@ -518,7 +518,7 @@ impl AsyncRateLimiter {
         &self,
         namespace: &Namespace,
         values: &HashMap<String, String>,
-        delta: i64,
+        delta: u64,
         load_counters: bool,
     ) -> Result<CheckResult, LimitadorError> {
         // the above where-clause is needed in order to call unwrap().

limitador/src/limit.rs

@@ -53,7 +53,7 @@ impl From<String> for Namespace {
 pub struct Limit {
     namespace: Namespace,
     #[serde(skip_serializing, default)]
-    max_value: i64,
+    max_value: u64,
     seconds: u64,
     #[serde(skip_serializing, default)]
     name: Option<String>,
@@ -308,7 +308,7 @@ where
 impl Limit {
     pub fn new<N: Into<Namespace>, T: TryInto<Condition>>(
         namespace: N,
-        max_value: i64,
+        max_value: u64,
         seconds: u64,
         conditions: impl IntoIterator<Item = T>,
         variables: impl IntoIterator<Item = impl Into<String>>,
@@ -335,7 +335,7 @@ impl Limit {
         &self.namespace
     }
 
-    pub fn max_value(&self) -> i64 {
+    pub fn max_value(&self) -> u64 {
         self.max_value
     }
 
@@ -351,7 +351,7 @@ impl Limit {
         self.name = Some(name)
     }
 
-    pub fn set_max_value(&mut self, value: i64) {
+    pub fn set_max_value(&mut self, value: u64) {
         self.max_value = value;
     }