ldap: Simplify boolean expressions derived from filters

.coderabbit.yml

@@ -25,6 +25,16 @@ reviews:
     unit_tests:
       enabled: false
 
+  pre_merge_checks:
+    docstrings:
+      mode: "off"
+    title:
+      mode: "off"
+    description:
+      mode: "off"
+    issue_assessment:
+      mode: "off"
+
 chat:
   art: false
   auto_reply: false

.github/workflows/docker-build-static.yml

@@ -24,7 +24,7 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
-  MSRV: "1.89"
+  MSRV: "1.89.0"
 
 ### CI Docs
 

.github/workflows/rust.yml

@@ -8,7 +8,7 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
-  MSRV: "1.89"
+  MSRV: "1.89.0"
 
 jobs:
   pre_job:

Cargo.toml

@@ -16,7 +16,7 @@ edition = "2024"
 homepage = "https://github.com/lldap/lldap"
 license = "GPL-3.0-only"
 repository = "https://github.com/lldap/lldap"
-rust-version = "1.89"
+rust-version = "1.89.0"
 
 [profile.release]
 lto = true

crates/ldap/src/core/group.rs

@@ -184,11 +184,11 @@ fn get_group_attribute_equality_filter(
         ]),
         (Ok(_), Err(e)) => {
             warn!("Invalid value for attribute {} (lowercased): {}", field, e);
-            GroupRequestFilter::from(false)
+            GroupRequestFilter::False
         }
         (Err(e), _) => {
             warn!("Invalid value for attribute {}: {}", field, e);
-            GroupRequestFilter::from(false)
+            GroupRequestFilter::False
         }
     }
 }
@@ -209,7 +209,7 @@ fn convert_group_filter(
                     .map(|id| GroupRequestFilter::GroupId(GroupId(id)))
                     .unwrap_or_else(|_| {
                         warn!("Given group id is not a valid integer: {}", value_lc);
-                        GroupRequestFilter::from(false)
+                        GroupRequestFilter::False
                     })),
                 GroupFieldType::DisplayName => Ok(GroupRequestFilter::DisplayName(value_lc.into())),
                 GroupFieldType::Uuid => Uuid::try_from(value_lc.as_str())
@@ -226,7 +226,7 @@ fn convert_group_filter(
                 .map(GroupRequestFilter::Member)
                 .unwrap_or_else(|e| {
                     warn!("Invalid member filter on group: {}", e);
-                    GroupRequestFilter::from(false)
+                    GroupRequestFilter::False
                 })),
                 GroupFieldType::ObjectClass => Ok(GroupRequestFilter::from(
                     get_default_group_object_classes()
@@ -246,7 +246,7 @@ fn convert_group_filter(
                     .map(GroupRequestFilter::DisplayName)
                     .unwrap_or_else(|_| {
                         warn!("Invalid dn filter on group: {}", value_lc);
-                        GroupRequestFilter::from(false)
+                        GroupRequestFilter::False
                     }))
                 }
                 GroupFieldType::NoMatch => {
@@ -257,7 +257,7 @@ fn convert_group_filter(
                             field
                         );
                     }
-                    Ok(GroupRequestFilter::from(false))
+                    Ok(GroupRequestFilter::False)
                 }
                 GroupFieldType::Attribute(field, typ, is_list) => Ok(
                     get_group_attribute_equality_filter(&field, typ, is_list, value),
@@ -272,21 +272,55 @@ fn convert_group_filter(
                 }),
             }
         }
-        LdapFilter::And(filters) => Ok(GroupRequestFilter::And(
+        LdapFilter::And(filters) => {
-            filters.iter().map(rec).collect::<LdapResult<_>>()?,
+            let res = filters
-        )),
+                .iter()
-        LdapFilter::Or(filters) => Ok(GroupRequestFilter::Or(
+                .map(rec)
-            filters.iter().map(rec).collect::<LdapResult<_>>()?,
+                .filter(|f| !matches!(f, Ok(GroupRequestFilter::False)))
-        )),
+                .flat_map(|f| match f {
-        LdapFilter::Not(filter) => Ok(GroupRequestFilter::Not(Box::new(rec(filter)?))),
+                    Ok(GroupRequestFilter::And(v)) => v.into_iter().map(Ok).collect(),
+                    f => vec![f],
+                })
+                .collect::<LdapResult<Vec<_>>>()?;
+            if res.is_empty() {
+                Ok(GroupRequestFilter::True)
+            } else if res.len() == 1 {
+                Ok(res.into_iter().next().unwrap())
+            } else {
+                Ok(GroupRequestFilter::And(res))
+            }
+        }
+        LdapFilter::Or(filters) => {
+            let res = filters
+                .iter()
+                .map(rec)
+                .filter(|c| !matches!(c, Ok(GroupRequestFilter::True)))
+                .flat_map(|f| match f {
+                    Ok(GroupRequestFilter::Or(v)) => v.into_iter().map(Ok).collect(),
+                    f => vec![f],
+                })
+                .collect::<LdapResult<Vec<_>>>()?;
+            if res.is_empty() {
+                Ok(GroupRequestFilter::False)
+            } else if res.len() == 1 {
+                Ok(res.into_iter().next().unwrap())
+            } else {
+                Ok(GroupRequestFilter::Or(res))
+            }
+        }
+        LdapFilter::Not(filter) => Ok(match rec(filter)? {
+            GroupRequestFilter::True => GroupRequestFilter::False,
+            GroupRequestFilter::False => GroupRequestFilter::True,
+            f => GroupRequestFilter::Not(Box::new(f)),
+        }),
         LdapFilter::Present(field) => {
             let field = AttributeName::from(field.as_str());
             Ok(match map_group_field(&field, schema) {
                 GroupFieldType::Attribute(name, _, _) => {
                     GroupRequestFilter::CustomAttributePresent(name)
                 }
-                GroupFieldType::NoMatch => GroupRequestFilter::from(false),
+                GroupFieldType::NoMatch => GroupRequestFilter::False,
-                _ => GroupRequestFilter::from(true),
+                _ => GroupRequestFilter::True,
             })
         }
         LdapFilter::Substring(field, substring_filter) => {
@@ -295,7 +329,7 @@ fn convert_group_filter(
                 GroupFieldType::DisplayName => Ok(GroupRequestFilter::DisplayNameSubString(
                     substring_filter.clone().into(),
                 )),
-                GroupFieldType::NoMatch => Ok(GroupRequestFilter::from(false)),
+                GroupFieldType::NoMatch => Ok(GroupRequestFilter::False),
                 _ => Err(LdapError {
                     code: LdapResultCode::UnwillingToPerform,
                     message: format!(

crates/ldap/src/core/user.rs

@@ -202,11 +202,11 @@ fn get_user_attribute_equality_filter(
         ]),
         (Ok(_), Err(e)) => {
             warn!("Invalid value for attribute {} (lowercased): {}", field, e);
-            UserRequestFilter::from(false)
+            UserRequestFilter::False
         }
         (Err(e), _) => {
             warn!("Invalid value for attribute {}: {}", field, e);
-            UserRequestFilter::from(false)
+            UserRequestFilter::False
         }
     }
 }
@@ -218,13 +218,47 @@ fn convert_user_filter(
 ) -> LdapResult<UserRequestFilter> {
     let rec = |f| convert_user_filter(ldap_info, f, schema);
     match filter {
-        LdapFilter::And(filters) => Ok(UserRequestFilter::And(
+        LdapFilter::And(filters) => {
-            filters.iter().map(rec).collect::<LdapResult<_>>()?,
+            let res = filters
-        )),
+                .iter()
-        LdapFilter::Or(filters) => Ok(UserRequestFilter::Or(
+                .map(rec)
-            filters.iter().map(rec).collect::<LdapResult<_>>()?,
+                .filter(|c| !matches!(c, Ok(UserRequestFilter::False)))
-        )),
+                .flat_map(|f| match f {
-        LdapFilter::Not(filter) => Ok(UserRequestFilter::Not(Box::new(rec(filter)?))),
+                    Ok(UserRequestFilter::And(v)) => v.into_iter().map(Ok).collect(),
+                    f => vec![f],
+                })
+                .collect::<LdapResult<Vec<_>>>()?;
+            if res.is_empty() {
+                Ok(UserRequestFilter::True)
+            } else if res.len() == 1 {
+                Ok(res.into_iter().next().unwrap())
+            } else {
+                Ok(UserRequestFilter::And(res))
+            }
+        }
+        LdapFilter::Or(filters) => {
+            let res = filters
+                .iter()
+                .map(rec)
+                .filter(|c| !matches!(c, Ok(UserRequestFilter::True)))
+                .flat_map(|f| match f {
+                    Ok(UserRequestFilter::Or(v)) => v.into_iter().map(Ok).collect(),
+                    f => vec![f],
+                })
+                .collect::<LdapResult<Vec<_>>>()?;
+            if res.is_empty() {
+                Ok(UserRequestFilter::False)
+            } else if res.len() == 1 {
+                Ok(res.into_iter().next().unwrap())
+            } else {
+                Ok(UserRequestFilter::Or(res))
+            }
+        }
+        LdapFilter::Not(filter) => Ok(match rec(filter)? {
+            UserRequestFilter::True => UserRequestFilter::False,
+            UserRequestFilter::False => UserRequestFilter::True,
+            f => UserRequestFilter::Not(Box::new(f)),
+        }),
         LdapFilter::Equality(field, value) => {
             let field = AttributeName::from(field.as_str());
             let value_lc = value.to_ascii_lowercase();
@@ -250,7 +284,7 @@ fn convert_user_filter(
                             field
                         );
                     }
-                    Ok(UserRequestFilter::from(false))
+                    Ok(UserRequestFilter::False)
                 }
                 UserFieldType::ObjectClass => Ok(UserRequestFilter::from(
                     get_default_user_object_classes()
@@ -269,7 +303,7 @@ fn convert_user_filter(
                 .map(UserRequestFilter::MemberOf)
                 .unwrap_or_else(|e| {
                     warn!("Invalid memberOf filter: {}", e);
-                    UserRequestFilter::from(false)
+                    UserRequestFilter::False
                 })),
                 UserFieldType::EntryDn | UserFieldType::Dn => {
                     Ok(get_user_id_from_distinguished_name_or_plain_name(
@@ -280,7 +314,7 @@ fn convert_user_filter(
                     .map(UserRequestFilter::UserId)
                     .unwrap_or_else(|_| {
                         warn!("Invalid dn filter on user: {}", value_lc);
-                        UserRequestFilter::from(false)
+                        UserRequestFilter::False
                     }))
                 }
             }
@@ -291,8 +325,8 @@ fn convert_user_filter(
                 UserFieldType::Attribute(name, _, _) => {
                     UserRequestFilter::CustomAttributePresent(name)
                 }
-                UserFieldType::NoMatch => UserRequestFilter::from(false),
+                UserFieldType::NoMatch => UserRequestFilter::False,
-                _ => UserRequestFilter::from(true),
+                _ => UserRequestFilter::True,
             })
         }
         LdapFilter::Substring(field, substring_filter) => {
@@ -311,7 +345,7 @@ fn convert_user_filter(
                     code: LdapResultCode::UnwillingToPerform,
                     message: format!("Unsupported user attribute for substring filter: {field:?}"),
                 }),
-                UserFieldType::NoMatch => Ok(UserRequestFilter::from(false)),
+                UserFieldType::NoMatch => Ok(UserRequestFilter::False),
                 UserFieldType::PrimaryField(UserColumn::Email) => Ok(UserRequestFilter::SubString(
                     UserColumn::LowercaseEmail,
                     substring_filter.clone().into(),

crates/ldap/src/search.rs

@@ -289,16 +289,10 @@ pub fn make_ldap_subschema_entry(schema: PublicSchema) -> LdapOp {
         ],
     })
 }
-
 pub(crate) fn is_root_dse_request(request: &LdapSearchRequest) -> bool {
-    if request.base.is_empty()
+    request.base.is_empty()
         && request.scope == LdapSearchScope::Base
-        && let LdapFilter::Present(attribute) = &request.filter
+        && matches!(&request.filter, LdapFilter::Present(attr) if attr.eq_ignore_ascii_case("objectclass"))
-        && attribute.eq_ignore_ascii_case("objectclass")
-    {
-        return true;
-    }
-    false
 }
 
 pub(crate) fn is_subschema_entry_request(request: &LdapSearchRequest) -> bool {
@@ -672,7 +666,7 @@ mod tests {
         mock.expect_list_users()
             .with(
                 eq(Some(UserRequestFilter::And(vec![
-                    UserRequestFilter::And(Vec::new()),
+                    UserRequestFilter::True,
                     UserRequestFilter::UserId(UserId::new("test")),
                 ]))),
                 eq(false),
@@ -707,7 +701,7 @@ mod tests {
     async fn test_search_readonly_user() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_users()
-            .with(eq(Some(UserRequestFilter::And(Vec::new()))), eq(false))
+            .with(eq(Some(UserRequestFilter::True)), eq(false))
             .times(1)
             .return_once(|_, _| Ok(vec![]));
         let ldap_handler = setup_bound_readonly_handler(mock).await;
@@ -724,7 +718,7 @@ mod tests {
     async fn test_search_member_of() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_users()
-            .with(eq(Some(UserRequestFilter::And(Vec::new()))), eq(true))
+            .with(eq(Some(UserRequestFilter::True)), eq(true))
             .times(1)
             .return_once(|_, _| {
                 Ok(vec![UserAndGroups {
@@ -769,7 +763,7 @@ mod tests {
         mock.expect_list_users()
             .with(
                 eq(Some(UserRequestFilter::And(vec![
-                    UserRequestFilter::And(Vec::new()),
+                    UserRequestFilter::True,
                     UserRequestFilter::UserId(UserId::new("bob")),
                 ]))),
                 eq(false),
@@ -975,7 +969,7 @@ mod tests {
     async fn test_search_groups() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_groups()
-            .with(eq(Some(GroupRequestFilter::And(Vec::new()))))
+            .with(eq(Some(GroupRequestFilter::True)))
             .times(1)
             .return_once(|_| {
                 Ok(vec![
@@ -1114,14 +1108,12 @@ mod tests {
                 GroupRequestFilter::DisplayName("group_1".into()),
                 GroupRequestFilter::Member(UserId::new("bob")),
                 GroupRequestFilter::DisplayName("rockstars".into()),
-                false.into(),
                 GroupRequestFilter::Uuid(uuid!("04ac75e0-2900-3e21-926c-2f732c26b3fc")),
                 true.into(),
                 true.into(),
                 true.into(),
                 true.into(),
-                GroupRequestFilter::Not(Box::new(false.into())),
+                true.into(),
-                false.into(),
                 GroupRequestFilter::DisplayNameSubString(SubStringFilter {
                     initial: Some("iNIt".to_owned()),
                     any: vec!["1".to_owned(), "2aA".to_owned()],
@@ -1195,11 +1187,9 @@ mod tests {
     async fn test_search_groups_filter_2() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_groups()
-            .with(eq(Some(GroupRequestFilter::Or(vec![
+            .with(eq(Some(GroupRequestFilter::Not(Box::new(
-                GroupRequestFilter::Not(Box::new(GroupRequestFilter::DisplayName(
+                GroupRequestFilter::DisplayName("group_2".into()),
-                    "group_2".into(),
+            )))))
-                ))),
-            ]))))
             .times(1)
             .return_once(|_| {
                 Ok(vec![Group {
@@ -1309,7 +1299,7 @@ mod tests {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_groups()
             .with(eq(Some(GroupRequestFilter::And(vec![
-                GroupRequestFilter::And(Vec::new()),
+                GroupRequestFilter::True,
                 GroupRequestFilter::DisplayName("rockstars".into()),
             ]))))
             .times(1)
@@ -1370,11 +1360,9 @@ mod tests {
     async fn test_search_groups_error() {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_groups()
-            .with(eq(Some(GroupRequestFilter::Or(vec![
+            .with(eq(Some(GroupRequestFilter::Not(Box::new(
-                GroupRequestFilter::Not(Box::new(GroupRequestFilter::DisplayName(
+                GroupRequestFilter::DisplayName("group_2".into()),
-                    "group_2".into(),
+            )))))
-                ))),
-            ]))))
             .times(1)
             .return_once(|_| {
                 Err(lldap_domain_model::error::DomainError::InternalError(
@@ -1422,44 +1410,35 @@ mod tests {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_users()
             .with(
-                eq(Some(UserRequestFilter::And(vec![UserRequestFilter::Or(
+                eq(Some(UserRequestFilter::Or(vec![
-                    vec![
+                    UserRequestFilter::Not(Box::new(UserRequestFilter::UserId(UserId::new("bob")))),
-                        UserRequestFilter::Not(Box::new(UserRequestFilter::UserId(UserId::new(
+                    UserRequestFilter::UserId("bob_1".to_string().into()),
-                            "bob",
+                    false.into(),
-                        )))),
+                    false.into(),
-                        UserRequestFilter::UserId("bob_1".to_string().into()),
+                    false.into(),
-                        false.into(),
+                    UserRequestFilter::AttributeEquality(
-                        true.into(),
+                        AttributeName::from("first_name"),
-                        false.into(),
+                        "FirstName".to_string().into(),
-                        true.into(),
+                    ),
-                        true.into(),
+                    UserRequestFilter::AttributeEquality(
-                        false.into(),
+                        AttributeName::from("first_name"),
-                        UserRequestFilter::Or(vec![
+                        "firstname".to_string().into(),
-                            UserRequestFilter::AttributeEquality(
+                    ),
-                                AttributeName::from("first_name"),
+                    false.into(),
-                                "FirstName".to_string().into(),
+                    UserRequestFilter::UserIdSubString(SubStringFilter {
-                            ),
+                        initial: Some("iNIt".to_owned()),
-                            UserRequestFilter::AttributeEquality(
+                        any: vec!["1".to_owned(), "2aA".to_owned()],
-                                AttributeName::from("first_name"),
+                        final_: Some("finAl".to_owned()),
-                                "firstname".to_string().into(),
+                    }),
-                            ),
+                    UserRequestFilter::SubString(
-                        ]),
+                        UserColumn::DisplayName,
-                        false.into(),
+                        SubStringFilter {
-                        UserRequestFilter::UserIdSubString(SubStringFilter {
                             initial: Some("iNIt".to_owned()),
                             any: vec!["1".to_owned(), "2aA".to_owned()],
                             final_: Some("finAl".to_owned()),
-                        }),
+                        },
-                        UserRequestFilter::SubString(
+                    ),
-                            UserColumn::DisplayName,
+                ]))),
-                            SubStringFilter {
-                                initial: Some("iNIt".to_owned()),
-                                any: vec!["1".to_owned(), "2aA".to_owned()],
-                                final_: Some("finAl".to_owned()),
-                            },
-                        ),
-                    ],
-                )]))),
                 eq(false),
             )
             .times(1)
@@ -1598,11 +1577,9 @@ mod tests {
         let mut mock = MockTestBackendHandler::new();
         mock.expect_list_users()
             .with(
-                eq(Some(UserRequestFilter::And(vec![UserRequestFilter::Or(
+                eq(Some(UserRequestFilter::Not(Box::new(
-                    vec![UserRequestFilter::Not(Box::new(
+                    UserRequestFilter::Equality(UserColumn::DisplayName, "bob".to_string()),
-                        UserRequestFilter::Equality(UserColumn::DisplayName, "bob".to_string()),
+                )))),
-                    ))],
-                )]))),
                 eq(false),
             )
             .times(1)
@@ -1709,7 +1686,7 @@ mod tests {
             }])
         });
         mock.expect_list_groups()
-            .with(eq(Some(GroupRequestFilter::And(Vec::new()))))
+            .with(eq(Some(GroupRequestFilter::True)))
             .times(1)
             .return_once(|_| {
                 Ok(vec![Group {
@@ -1795,7 +1772,7 @@ mod tests {
             }])
         });
         mock.expect_list_groups()
-            .with(eq(Some(GroupRequestFilter::And(Vec::new()))))
+            .with(eq(Some(GroupRequestFilter::True)))
             .returning(|_| {
                 Ok(vec![Group {
                     id: GroupId(1),

rust-toolchain.toml

@@ -1,2 +1,2 @@
 [toolchain]
-channel = "1.89"
+channel = "1.89.0"