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.

Sign up for GitHub

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

Impl From<&str> for Namespace instead of FromStr as it can't fail #76

Merged
merged 2 commits into from
Jun 9, 2022
Merged

Impl From<&str> for Namespace instead of FromStr as it can't fail #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01ca3da
Impl From<&str> for Namespace, it can't fail
alexsnaps Jun 8, 2022
c2e0add
No need to convert early
alexsnaps Jun 8, 2022
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
2 changes: 1 addition & 1 deletion limitador-server/src/http_api/request_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl From<&LimitadorLimit> for Limit {
impl From<Limit> for LimitadorLimit {
fn from(limit: Limit) -> Self {
let mut limitador_limit = Self::new(
limit.namespace.as_str(),
limit.namespace,
limit.max_value,
limit.seconds,
limit.conditions,
Expand Down
92 changes: 29 additions & 63 deletions limitador-server/src/http_api/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::http_api::request_types::{CheckAndReportInfo, Counter, Limit};
use crate::Limiter;
use actix_web::{http::StatusCode, ResponseError};
use actix_web::{App, HttpServer};
use limitador::limit::Namespace;
use paperclip::actix::{
api_v2_errors,
api_v2_operation,
Expand Down Expand Up @@ -75,14 +74,8 @@ async fn get_limits(
namespace: web::Path<String>,
) -> Result<web::Json<Vec<Limit>>, ErrorResponse> {
let get_limits_result = match data.get_ref().as_ref() {
Limiter::Blocking(limiter) => {
limiter.get_limits(namespace.into_inner().parse::<Namespace>().unwrap())
}
Limiter::Async(limiter) => {
limiter
.get_limits(namespace.into_inner().parse::<Namespace>().unwrap())
.await
}
Limiter::Blocking(limiter) => limiter.get_limits(namespace.into_inner()),
Limiter::Async(limiter) => limiter.get_limits(namespace.into_inner()).await,
};

match get_limits_result {
Expand Down Expand Up @@ -116,14 +109,8 @@ async fn delete_limits(
namespace: web::Path<String>,
) -> Result<web::Json<()>, ErrorResponse> {
let delete_limits_result = match data.get_ref().as_ref() {
Limiter::Blocking(limiter) => {
limiter.delete_limits(namespace.into_inner().parse::<Namespace>().unwrap())
}
Limiter::Async(limiter) => {
limiter
.delete_limits(namespace.into_inner().parse::<Namespace>().unwrap())
.await
}
Limiter::Blocking(limiter) => limiter.delete_limits(namespace.into_inner()),
Limiter::Async(limiter) => limiter.delete_limits(namespace.into_inner()).await,
};

match delete_limits_result {
Expand All @@ -138,14 +125,8 @@ async fn get_counters(
namespace: web::Path<String>,
) -> Result<web::Json<Vec<Counter>>, ErrorResponse> {
let get_counters_result = match data.get_ref().as_ref() {
Limiter::Blocking(limiter) => {
limiter.get_counters(namespace.into_inner().parse::<Namespace>().unwrap())
}
Limiter::Async(limiter) => {
limiter
.get_counters(namespace.into_inner().parse::<Namespace>().unwrap())
.await
}
Limiter::Blocking(limiter) => limiter.get_counters(namespace.into_inner()),
Limiter::Async(limiter) => limiter.get_counters(namespace.into_inner()).await,
};

match get_counters_result {
Expand All @@ -165,21 +146,14 @@ async fn check(
state: web::Data<Arc<Limiter>>,
request: web::Json<CheckAndReportInfo>,
) -> Result<web::Json<()>, ErrorResponse> {
let CheckAndReportInfo {
namespace,
values,
delta,
} = request.into_inner();
let is_rate_limited_result = match state.get_ref().as_ref() {
Limiter::Blocking(limiter) => limiter.is_rate_limited(
request.namespace.parse::<Namespace>().unwrap(),
&request.values,
request.delta,
),
Limiter::Async(limiter) => {
limiter
.is_rate_limited(
request.namespace.parse::<Namespace>().unwrap(),
&request.values,
request.delta,
)
.await
}
Limiter::Blocking(limiter) => limiter.is_rate_limited(namespace, &values, delta),
Limiter::Async(limiter) => limiter.is_rate_limited(namespace, &values, delta).await,
};

match is_rate_limited_result {
Expand All @@ -199,21 +173,14 @@ async fn report(
data: web::Data<Arc<Limiter>>,
request: web::Json<CheckAndReportInfo>,
) -> Result<web::Json<()>, ErrorResponse> {
let CheckAndReportInfo {
namespace,
values,
delta,
} = request.into_inner();
let update_counters_result = match data.get_ref().as_ref() {
Limiter::Blocking(limiter) => limiter.update_counters(
request.namespace.parse::<Namespace>().unwrap(),
&request.values,
request.delta,
),
Limiter::Async(limiter) => {
limiter
.update_counters(
request.namespace.parse::<Namespace>().unwrap(),
&request.values,
request.delta,
)
.await
}
Limiter::Blocking(limiter) => limiter.update_counters(namespace, &values, delta),
Limiter::Async(limiter) => limiter.update_counters(namespace, &values, delta).await,
};

match update_counters_result {
Expand All @@ -227,19 +194,18 @@ async fn check_and_report(
data: web::Data<Arc<Limiter>>,
request: web::Json<CheckAndReportInfo>,
) -> Result<web::Json<()>, ErrorResponse> {
let CheckAndReportInfo {
namespace,
values,
delta,
} = request.into_inner();
let rate_limited_and_update_result = match data.get_ref().as_ref() {
Limiter::Blocking(limiter) => limiter.check_rate_limited_and_update(
request.namespace.parse::<Namespace>().unwrap(),
&request.values,
request.delta,
),
Limiter::Blocking(limiter) => {
limiter.check_rate_limited_and_update(namespace, &values, delta)
}
Limiter::Async(limiter) => {
limiter
.check_rate_limited_and_update(
request.namespace.as_str(),
&request.values,
request.delta,
)
.check_rate_limited_and_update(namespace, &values, delta)
.await
}
};
Expand Down
24 changes: 12 additions & 12 deletions limitador/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,28 +319,28 @@ impl RateLimiter {
self.storage.delete_limit(limit).map_err(|err| err.into())
}

pub fn get_limits<N: TryInto<Namespace>>(
pub fn get_limits<N: Into<Namespace>>(
&self,
namespace: N,
) -> Result<HashSet<Limit>, LimitadorError>
where
<N as TryInto<Namespace>>::Error: Debug,
{
self.storage
.get_limits(&namespace.try_into().unwrap())
.get_limits(&namespace.into())
.map_err(|err| err.into())
}

pub fn delete_limits<N: TryInto<Namespace>>(&self, namespace: N) -> Result<(), LimitadorError>
pub fn delete_limits<N: Into<Namespace>>(&self, namespace: N) -> Result<(), LimitadorError>
where
<N as TryInto<Namespace>>::Error: Debug,
{
self.storage
.delete_limits(&namespace.try_into().unwrap())
.delete_limits(&namespace.into())
.map_err(|err| err.into())
}

pub fn is_rate_limited<N: TryInto<Namespace>>(
pub fn is_rate_limited<N: Into<Namespace>>(
&self,
namespace: N,
values: &HashMap<String, String>,
Expand All @@ -349,7 +349,7 @@ impl RateLimiter {
where
<N as TryInto<Namespace>>::Error: Debug,
{
let namespace = namespace.try_into().unwrap();
let namespace = namespace.into();
let counters = self.counters_that_apply(namespace.clone(), values)?;

for counter in counters {
Expand All @@ -369,7 +369,7 @@ impl RateLimiter {
Ok(false)
}

pub fn update_counters<N: TryInto<Namespace>>(
pub fn update_counters<N: Into<Namespace>>(
&self,
namespace: N,
values: &HashMap<String, String>,
Expand All @@ -386,7 +386,7 @@ impl RateLimiter {
.map_err(|err| err.into())
}

pub fn check_rate_limited_and_update<N: TryInto<Namespace>>(
pub fn check_rate_limited_and_update<N: Into<Namespace>>(
&self,
namespace: N,
values: &HashMap<String, String>,
Expand All @@ -395,7 +395,7 @@ impl RateLimiter {
where
<N as TryInto<Namespace>>::Error: Debug,
{
let namespace = namespace.try_into().unwrap();
let namespace = namespace.into();
let counters = self.counters_that_apply(namespace.clone(), values)?;

if counters.is_empty() {
Expand Down Expand Up @@ -467,7 +467,7 @@ impl RateLimiter {
self.prometheus_metrics.gather_metrics()
}

fn counters_that_apply<N: TryInto<Namespace>>(
fn counters_that_apply<N: Into<Namespace>>(
&self,
namespace: N,
values: &HashMap<String, String>,
Expand Down Expand Up @@ -588,7 +588,7 @@ impl AsyncRateLimiter {
Ok(())
}

pub async fn check_rate_limited_and_update<N: TryInto<Namespace>>(
pub async fn check_rate_limited_and_update<N: Into<Namespace>>(
&self,
namespace: N,
values: &HashMap<String, String>,
Expand All @@ -598,7 +598,7 @@ impl AsyncRateLimiter {
<N as TryInto<Namespace>>::Error: Debug,
{
// the above where-clause is needed in order to call unwrap().
let namespace = namespace.try_into().unwrap();
let namespace = namespace.into();
let counters = self.counters_that_apply(namespace.clone(), values).await?;

if counters.is_empty() {
Expand Down
37 changes: 7 additions & 30 deletions limitador/src/limit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,13 @@
use serde::{Deserialize, Serialize, Serializer};
use std::collections::{BTreeSet, HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::str::FromStr;

#[derive(Debug, Hash, Eq, PartialEq, Clone, Serialize, Deserialize)]
pub struct Namespace(String);

impl FromStr for Namespace {
type Err = core::convert::Infallible;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(Self(s.into()))
}
}

impl TryFrom<&str> for Namespace {
type Error = <Self as FromStr>::Err;

fn try_from(s: &str) -> Result<Self, Self::Error> {
s.parse()
impl From<&str> for Namespace {
fn from(s: &str) -> Namespace {
Self(s.into())
}
}

Expand Down Expand Up @@ -58,7 +47,7 @@ where
}

impl Limit {
pub fn new<N: TryInto<Namespace>>(
pub fn new<N: Into<Namespace>>(
namespace: N,
max_value: i64,
seconds: u64,
Expand All @@ -70,7 +59,7 @@ impl Limit {
{
// the above where-clause is needed in order to call unwrap().
Self {
namespace: namespace.try_into().unwrap(),
namespace: namespace.into(),
max_value,
seconds,
name: None,
Expand Down Expand Up @@ -181,13 +170,7 @@ mod tests {

#[test]
fn limit_can_have_an_optional_name() {
let mut limit = Limit::new(
"test_namespace".parse::<Namespace>().unwrap(),
10,
60,
vec!["x == 5"],
vec!["y"],
);
let mut limit = Limit::new("test_namespace", 10, 60, vec!["x == 5"], vec!["y"]);
assert!(limit.name.is_none());

let name = "Test Limit";
Expand All @@ -197,13 +180,7 @@ mod tests {

#[test]
fn limit_applies() {
let limit = Limit::new(
"test_namespace".parse::<Namespace>().unwrap(),
10,
60,
vec!["x == 5"],
vec!["y"],
);
let limit = Limit::new("test_namespace", 10, 60, vec!["x == 5"], vec!["y"]);

let mut values: HashMap<String, String> = HashMap::new();
values.insert("x".into(), "5".into());
Expand Down
14 changes: 7 additions & 7 deletions limitador/src/prometheus_metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ mod tests {
let prometheus_metrics = PrometheusMetrics::new();

let namespaces_with_auth_counts = [
("some_namespace".parse().unwrap(), 2),
("another_namespace".parse().unwrap(), 3),
("some_namespace".into(), 2),
("another_namespace".into(), 3),
];

namespaces_with_auth_counts
Expand Down Expand Up @@ -170,8 +170,8 @@ mod tests {
let prometheus_metrics = PrometheusMetrics::new();

let namespaces_with_limited_counts = [
("some_namespace".parse().unwrap(), 2),
("another_namespace".parse().unwrap(), 3),
("some_namespace".into(), 2),
("another_namespace".into(), 3),
];

namespaces_with_limited_counts
Expand Down Expand Up @@ -200,8 +200,8 @@ mod tests {
let prometheus_metrics = PrometheusMetrics::new_with_counters_by_limit_name();

let limits_with_counts = [
("some_namespace".parse().unwrap(), "Some limit", 2),
("some_namespace".parse().unwrap(), "Another limit", 3),
("some_namespace".into(), "Some limit", 2),
("some_namespace".into(), "Another limit", 3),
];

limits_with_counts
Expand Down Expand Up @@ -231,7 +231,7 @@ mod tests {
#[test]
fn incr_limited_calls_uses_empty_string_when_no_name() {
let prometheus_metrics = PrometheusMetrics::new_with_counters_by_limit_name();
let namespace = "some namespace".parse().unwrap();
let namespace = "some namespace".into();
prometheus_metrics.incr_limited_calls(&namespace, None);

let metrics_output = prometheus_metrics.gather_metrics();
Expand Down
2 changes: 1 addition & 1 deletion limitador/src/storage/infinispan/infinispan_storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl AsyncStorage for InfinispanStorage {
.get_set(key_for_namespaces_set())
.await?
.iter()
.map(|ns| ns.parse().unwrap())
.map(|ns| ns.as_str().into())
.collect())
}

Expand Down
2 changes: 1 addition & 1 deletion limitador/src/storage/redis/redis_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl AsyncStorage for AsyncRedisStorage {
.smembers::<String, HashSet<String>>(key_for_namespaces_set())
.await?;

Ok(namespaces.iter().map(|ns| ns.parse().unwrap()).collect())
Ok(namespaces.iter().map(|ns| ns.as_str().into()).collect())
}

async fn add_limit(&self, limit: &Limit) -> Result<(), StorageErr> {
Expand Down
2 changes: 1 addition & 1 deletion limitador/src/storage/redis/redis_sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl Storage for RedisStorage {

let namespaces = con.smembers::<String, HashSet<String>>(key_for_namespaces_set())?;

Ok(namespaces.iter().map(|ns| ns.parse().unwrap()).collect())
Ok(namespaces.iter().map(|ns| ns.as_str().into()).collect())
}

fn add_limit(&self, limit: &Limit) -> Result<(), StorageErr> {
Expand Down
Loading