Mail substr index (#2981)

server/lib/src/be/idl_sqlite.rs

@@ -1204,6 +1204,7 @@ impl IdlSqliteWriteTransaction {
         })
     }
 
+    #[instrument(level = "debug", skip(self))]
     pub fn create_idx(&self, attr: Attribute, itype: IndexType) -> Result<(), OperationError> {
         // Is there a better way than formatting this? I can't seem
         // to template into the str.
@@ -1215,7 +1216,6 @@ impl IdlSqliteWriteTransaction {
             itype.as_idx_str(),
             attr
         );
-        trace!(idx = %idx_stmt, "creating index");
 
         self.get_conn()?
             .execute(idx_stmt.as_str(), [])

server/lib/src/constants/mod.rs

@@ -23,7 +23,9 @@ use std::time::Duration;
 // This value no longer requires incrementing during releases. It only
 // serves as a "once off" marker so that we know when the initial db
 // index is performed on first-run.
-pub const SYSTEM_INDEX_VERSION: i64 = 31;
+//
+// It's also useful if we need to force a reindex due to a bug though :)
+pub const SYSTEM_INDEX_VERSION: i64 = 32;
 
 /*
  * domain functional levels

server/lib/src/idm/ldap.rs

@@ -1353,6 +1353,61 @@ mod tests {
                         LDAP_ATTR_MAIL_ALTERNATIVE,
                         "testperson1.alternative@example.com"
                     ),
+                    (LDAP_ATTR_EMAIL_PRIMARY, "testperson1@example.com"),
+                    (
+                        LDAP_ATTR_EMAIL_ALTERNATIVE,
+                        "testperson1.alternative@example.com"
+                    )
+                );
+            }
+            _ => assert!(false),
+        };
+
+        // ======= test with a substring search
+
+        let sr = SearchRequest {
+            msgid: 2,
+            base: "dc=example,dc=com".to_string(),
+            scope: LdapSearchScope::Subtree,
+            filter: LdapFilter::And(vec![
+                LdapFilter::Equality(Attribute::Class.to_string(), "posixAccount".to_string()),
+                LdapFilter::Substring(
+                    LDAP_ATTR_MAIL.to_string(),
+                    LdapSubstringFilter {
+                        initial: None,
+                        any: vec![],
+                        final_: Some("@example.com".to_string()),
+                    },
+                ),
+            ]),
+            attrs: vec![
+                LDAP_ATTR_NAME,
+                LDAP_ATTR_MAIL,
+                LDAP_ATTR_MAIL_PRIMARY,
+                LDAP_ATTR_MAIL_ALTERNATIVE,
+            ]
+            .into_iter()
+            .map(|s| s.to_string())
+            .collect(),
+        };
+
+        let r1 = ldaps
+            .do_search(idms, &sr, &sa_lbt, Source::Internal)
+            .await
+            .unwrap();
+
+        assert!(r1.len() == 2);
+        match &r1[0].op {
+            LdapOp::SearchResultEntry(lsre) => {
+                assert_entry_contains!(
+                    lsre,
+                    "spn=testperson1@example.com,dc=example,dc=com",
+                    (Attribute::Name.as_ref(), "testperson1"),
+                    (Attribute::Mail.as_ref(), "testperson1@example.com"),
+                    (
+                        Attribute::Mail.as_ref(),
+                        "testperson1.alternative@example.com"
+                    ),
                     (LDAP_ATTR_MAIL_PRIMARY, "testperson1@example.com"),
                     (
                         LDAP_ATTR_MAIL_ALTERNATIVE,

server/lib/src/server/migrations.rs

@@ -139,6 +139,9 @@ impl QueryServer {
             // Reload if anything in migrations requires it - this triggers the domain migrations
             // which in turn can trigger schema reloads etc.
             write_txn.reload()?;
+            // Force a reindex here since schema probably changed and we aren't at the
+            // runtime phase where it will trigger on its own yet.
+            write_txn.reindex()?;
         } else if domain_development_taint {
             // This forces pre-release versions to re-migrate each start up. This solves
             // the domain-version-sprawl issue so that during a development cycle we can
@@ -152,6 +155,9 @@ impl QueryServer {
             // We did not already need a version migration as above
             write_txn.domain_remigrate(DOMAIN_PREVIOUS_TGT_LEVEL)?;
             write_txn.reload()?;
+            // Force a reindex here since schema probably changed and we aren't at the
+            // runtime phase where it will trigger on its own yet.
+            write_txn.reindex()?;
         }
 
         if domain_target_level >= DOMAIN_LEVEL_7 && domain_patch_level < DOMAIN_TGT_PATCH_LEVEL {

server/lib/src/valueset/address.rs

@@ -357,16 +357,52 @@ impl ValueSetT for ValueSetEmailAddress {
         }
     }
 
-    fn substring(&self, _pv: &PartialValue) -> bool {
+    fn substring(&self, pv: &PartialValue) -> bool {
-        false
+        match pv {
+            PartialValue::EmailAddress(s2) => {
+                // We lowercase as LDAP and similar expect case insensitive searches here.
+                let s2_lower = s2.to_lowercase();
+                self.set
+                    .iter()
+                    .any(|s1| s1.to_lowercase().contains(&s2_lower))
+            }
+            _ => {
+                debug_assert!(false);
+                false
+            }
+        }
     }
 
-    fn startswith(&self, _pv: &PartialValue) -> bool {
+    fn startswith(&self, pv: &PartialValue) -> bool {
-        false
+        match pv {
+            PartialValue::EmailAddress(s2) => {
+                // We lowercase as LDAP and similar expect case insensitive searches here.
+                let s2_lower = s2.to_lowercase();
+                self.set
+                    .iter()
+                    .any(|s1| s1.to_lowercase().starts_with(&s2_lower))
+            }
+            _ => {
+                debug_assert!(false);
+                false
+            }
+        }
     }
 
-    fn endswith(&self, _pv: &PartialValue) -> bool {
+    fn endswith(&self, pv: &PartialValue) -> bool {
-        false
+        match pv {
+            PartialValue::EmailAddress(s2) => {
+                // We lowercase as LDAP and similar expect case insensitive searches here.
+                let s2_lower = s2.to_lowercase();
+                self.set
+                    .iter()
+                    .any(|s1| s1.to_lowercase().ends_with(&s2_lower))
+            }
+            _ => {
+                debug_assert!(false);
+                false
+            }
+        }
     }
 
     fn lessthan(&self, _pv: &PartialValue) -> bool {