From b4f636ded97d92ddccb0785cdd5e4f47817a5f38 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Fri, 25 Apr 2025 15:10:19 +0900 Subject: [PATCH] server: Introduce True/False for filters This should help clean up the filter debug representations --- crates/domain-handlers/src/handler.rs | 16 ++++----- crates/ldap/src/search.rs | 16 ++++----- .../src/sql_group_backend_handler.rs | 36 ++++++++++--------- .../src/sql_user_backend_handler.rs | 17 +++++---- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/crates/domain-handlers/src/handler.rs b/crates/domain-handlers/src/handler.rs index bb350d2..cd31755 100644 --- a/crates/domain-handlers/src/handler.rs +++ b/crates/domain-handlers/src/handler.rs @@ -70,6 +70,8 @@ impl From for SubStringFilter { #[derive(PartialEq, Eq, Debug, Serialize, Deserialize, Clone)] pub enum UserRequestFilter { + True, + False, And(Vec), Or(Vec), Not(Box), @@ -87,16 +89,14 @@ pub enum UserRequestFilter { impl From for UserRequestFilter { fn from(val: bool) -> Self { - if val { - Self::And(vec![]) - } else { - Self::Not(Box::new(Self::And(vec![]))) - } + if val { Self::True } else { Self::False } } } #[derive(PartialEq, Eq, Debug, Serialize, Deserialize, Clone)] pub enum GroupRequestFilter { + True, + False, And(Vec), Or(Vec), Not(Box), @@ -112,11 +112,7 @@ pub enum GroupRequestFilter { impl From for GroupRequestFilter { fn from(val: bool) -> Self { - if val { - Self::And(vec![]) - } else { - Self::Not(Box::new(Self::And(vec![]))) - } + if val { Self::True } else { Self::False } } } diff --git a/crates/ldap/src/search.rs b/crates/ldap/src/search.rs index 956f07f..15ab210 100644 --- a/crates/ldap/src/search.rs +++ b/crates/ldap/src/search.rs @@ -362,7 +362,7 @@ mod tests { mock.expect_list_users() .with( eq(Some(UserRequestFilter::And(vec![ - true.into(), + UserRequestFilter::And(Vec::new()), UserRequestFilter::UserId(UserId::new("test")), ]))), eq(false), @@ -397,7 +397,7 @@ mod tests { async fn test_search_readonly_user() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_users() - .with(eq(Some(true.into())), eq(false)) + .with(eq(Some(UserRequestFilter::And(Vec::new()))), eq(false)) .times(1) .return_once(|_, _| Ok(vec![])); let ldap_handler = setup_bound_readonly_handler(mock).await; @@ -414,7 +414,7 @@ mod tests { async fn test_search_member_of() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_users() - .with(eq(Some(true.into())), eq(true)) + .with(eq(Some(UserRequestFilter::And(Vec::new()))), eq(true)) .times(1) .return_once(|_, _| { Ok(vec![UserAndGroups { @@ -458,7 +458,7 @@ mod tests { mock.expect_list_users() .with( eq(Some(UserRequestFilter::And(vec![ - true.into(), + UserRequestFilter::And(Vec::new()), UserRequestFilter::UserId(UserId::new("bob")), ]))), eq(false), @@ -656,7 +656,7 @@ mod tests { async fn test_search_groups() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_groups() - .with(eq(Some(true.into()))) + .with(eq(Some(GroupRequestFilter::And(Vec::new())))) .times(1) .return_once(|_| { Ok(vec![ @@ -984,7 +984,7 @@ mod tests { let mut mock = MockTestBackendHandler::new(); mock.expect_list_groups() .with(eq(Some(GroupRequestFilter::And(vec![ - true.into(), + GroupRequestFilter::And(Vec::new()), GroupRequestFilter::DisplayName("rockstars".into()), ])))) .times(1) @@ -1384,7 +1384,7 @@ mod tests { }]) }); mock.expect_list_groups() - .with(eq(Some(true.into()))) + .with(eq(Some(GroupRequestFilter::And(Vec::new())))) .times(1) .return_once(|_| { Ok(vec![Group { @@ -1469,7 +1469,7 @@ mod tests { }]) }); mock.expect_list_groups() - .with(eq(Some(true.into()))) + .with(eq(Some(GroupRequestFilter::And(Vec::new())))) .returning(|_| { Ok(vec![Group { id: GroupId(1), diff --git a/crates/sql-backend-handler/src/sql_group_backend_handler.rs b/crates/sql-backend-handler/src/sql_group_backend_handler.rs index 4ec2f6f..41c1ff2 100644 --- a/crates/sql-backend-handler/src/sql_group_backend_handler.rs +++ b/crates/sql-backend-handler/src/sql_group_backend_handler.rs @@ -39,23 +39,27 @@ fn attribute_condition(name: AttributeName, value: Option) -> Cond { fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond { use GroupRequestFilter::*; let group_table = Alias::new("groups"); + fn bool_to_expr(b: bool) -> Cond { + SimpleExpr::Value(b.into()).into_condition() + } + fn get_repeated_filter( + fs: Vec, + condition: Cond, + default_value: bool, + ) -> Cond { + if fs.is_empty() { + bool_to_expr(default_value) + } else { + fs.into_iter() + .map(get_group_filter_expr) + .fold(condition, Cond::add) + } + } match filter { - And(fs) => { - if fs.is_empty() { - SimpleExpr::Value(true.into()).into_condition() - } else { - fs.into_iter() - .fold(Cond::all(), |c, f| c.add(get_group_filter_expr(f))) - } - } - Or(fs) => { - if fs.is_empty() { - SimpleExpr::Value(false.into()).into_condition() - } else { - fs.into_iter() - .fold(Cond::any(), |c, f| c.add(get_group_filter_expr(f))) - } - } + True => bool_to_expr(true), + False => bool_to_expr(false), + And(fs) => get_repeated_filter(fs, Cond::all(), true), + Or(fs) => get_repeated_filter(fs, Cond::any(), false), Not(f) => get_group_filter_expr(*f).not(), DisplayName(name) => GroupColumn::LowercaseDisplayName .eq(name.as_str().to_lowercase()) diff --git a/crates/sql-backend-handler/src/sql_user_backend_handler.rs b/crates/sql-backend-handler/src/sql_user_backend_handler.rs index dff420f..21e0ab6 100644 --- a/crates/sql-backend-handler/src/sql_user_backend_handler.rs +++ b/crates/sql-backend-handler/src/sql_user_backend_handler.rs @@ -54,13 +54,16 @@ fn user_id_subcondition(filter: Cond) -> Cond { fn get_user_filter_expr(filter: UserRequestFilter) -> Cond { use UserRequestFilter::*; let group_table = Alias::new("r1"); + fn bool_to_expr(b: bool) -> Cond { + SimpleExpr::Value(b.into()).into_condition() + } fn get_repeated_filter( fs: Vec, condition: Cond, default_value: bool, ) -> Cond { if fs.is_empty() { - SimpleExpr::Value(default_value.into()).into_condition() + bool_to_expr(default_value) } else { fs.into_iter() .map(get_user_filter_expr) @@ -68,6 +71,8 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond { } } match filter { + True => bool_to_expr(true), + False => bool_to_expr(false), And(fs) => get_repeated_filter(fs, Cond::all(), true), Or(fs) => get_repeated_filter(fs, Cond::any(), false), Not(f) => get_user_filter_expr(*f).not(), @@ -519,13 +524,7 @@ mod tests { #[tokio::test] async fn test_list_users_false_filter() { let fixture = TestFixture::new().await; - let users = get_user_names( - &fixture.handler, - Some(UserRequestFilter::Not(Box::new(UserRequestFilter::And( - vec![], - )))), - ) - .await; + let users = get_user_names(&fixture.handler, Some(UserRequestFilter::False)).await; assert_eq!(users, Vec::::new()); } @@ -633,7 +632,7 @@ mod tests { let users = get_user_names( &fixture.handler, Some(UserRequestFilter::Or(vec![ - UserRequestFilter::Or(vec![]), + UserRequestFilter::False, UserRequestFilter::Or(vec![ UserRequestFilter::UserId(UserId::new("bob")), UserRequestFilter::UserId(UserId::new("John")),