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

All i64 to unsigned ones #332

Merged
merged 2 commits into from
May 16, 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
16 changes: 4 additions & 12 deletions limitador-server/src/envoy_rls/server.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use opentelemetry::global;
use opentelemetry::propagation::Extractor;
use std::cmp::Ordering;
use std::collections::HashMap;
use std::sync::Arc;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand All @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions limitador-server/src/http_api/request_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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>,
}

Expand Down
14 changes: 3 additions & 11 deletions limitador-server/src/http_api/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down
91 changes: 23 additions & 68 deletions limitador-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
))),
Expand All @@ -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 = {
Expand Down Expand Up @@ -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}")),
}
}
Expand Down Expand Up @@ -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);
}
}
4 changes: 2 additions & 2 deletions limitador/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const TEST_SCENARIOS: &[&TestScenario] = &[
struct TestCallParams {
namespace: String,
values: HashMap<String, String>,
delta: i64,
delta: u64,
}

impl Display for TestScenario {
Expand Down Expand Up @@ -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(),
Expand Down
8 changes: 4 additions & 4 deletions limitador/src/counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down Expand Up @@ -53,7 +53,7 @@ impl Counter {
&self.limit
}

pub fn max_value(&self) -> i64 {
pub fn max_value(&self) -> u64 {
self.limit.max_value()
}

Expand All @@ -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)
}

Expand Down
12 changes: 6 additions & 6 deletions limitador/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand All @@ -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)?;

Expand All @@ -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)?;
Expand Down Expand Up @@ -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?;

Expand All @@ -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?;

Expand All @@ -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().
Expand Down
8 changes: 4 additions & 4 deletions limitador/src/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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>>,
Expand All @@ -335,7 +335,7 @@ impl Limit {
&self.namespace
}

pub fn max_value(&self) -> i64 {
pub fn max_value(&self) -> u64 {
self.max_value
}

Expand All @@ -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;
}

Expand Down
Loading
Loading