From 36b6fda787eeb586cf4059d725fee57544571f62 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Sun, 18 Aug 2024 12:49:24 +1000 Subject: [PATCH] Mail substr index (#2981) --- server/lib/src/be/idl_sqlite.rs | 2 +- server/lib/src/constants/mod.rs | 4 ++- server/lib/src/idm/ldap.rs | 55 +++++++++++++++++++++++++++++ server/lib/src/server/migrations.rs | 6 ++++ server/lib/src/valueset/address.rs | 48 +++++++++++++++++++++---- 5 files changed, 107 insertions(+), 8 deletions(-) diff --git a/server/lib/src/be/idl_sqlite.rs b/server/lib/src/be/idl_sqlite.rs index 2a0f805d1..e6e1e5f9b 100644 --- a/server/lib/src/be/idl_sqlite.rs +++ b/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(), []) diff --git a/server/lib/src/constants/mod.rs b/server/lib/src/constants/mod.rs index e831cbd49..8d4063ff3 100644 --- a/server/lib/src/constants/mod.rs +++ b/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 diff --git a/server/lib/src/idm/ldap.rs b/server/lib/src/idm/ldap.rs index dc0479a6d..7efdb50a4 100644 --- a/server/lib/src/idm/ldap.rs +++ b/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, diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index 53ce734f8..020cbc944 100644 --- a/server/lib/src/server/migrations.rs +++ b/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 { diff --git a/server/lib/src/valueset/address.rs b/server/lib/src/valueset/address.rs index 7991abeaa..b9743e7d8 100644 --- a/server/lib/src/valueset/address.rs +++ b/server/lib/src/valueset/address.rs @@ -357,16 +357,52 @@ impl ValueSetT for ValueSetEmailAddress { } } - fn substring(&self, _pv: &PartialValue) -> bool { - false + fn substring(&self, pv: &PartialValue) -> bool { + 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 { - false + fn startswith(&self, pv: &PartialValue) -> bool { + 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 { - false + fn endswith(&self, pv: &PartialValue) -> bool { + 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 {