From 7e76bcd7f1fc0d1c0d740a3154f621a8526306c4 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Sun, 18 May 2025 09:47:46 +0200 Subject: [PATCH] Fix purge and enterprise tests --- crates/email/src/message/delete.rs | 2 +- crates/imap/src/op/fetch.rs | 4 ++-- crates/imap/src/op/status.rs | 3 ++- crates/jmap/src/changes/get.rs | 6 +++++ crates/store/src/dispatch/store.rs | 23 +++--------------- tests/src/jmap/enterprise.rs | 10 ++++---- tests/src/jmap/mod.rs | 8 +++---- tests/src/jmap/purge.rs | 38 ++++++++++++++++++------------ 8 files changed, 46 insertions(+), 48 deletions(-) diff --git a/crates/email/src/message/delete.rs b/crates/email/src/message/delete.rs index 072e6b09..3d892431 100644 --- a/crates/email/src/message/delete.rs +++ b/crates/email/src/message/delete.rs @@ -198,7 +198,7 @@ impl EmailDeletion for Server { collection: Collection::Email.into(), document_id: 0, field: Property::ReceivedAt.into(), - key: now().saturating_sub(period.as_secs() * 2).serialize(), + key: 0u64.serialize(), }, IndexKey { account_id, diff --git a/crates/imap/src/op/fetch.rs b/crates/imap/src/op/fetch.rs index e8130f8f..6cf94137 100644 --- a/crates/imap/src/op/fetch.rs +++ b/crates/imap/src/op/fetch.rs @@ -500,12 +500,12 @@ impl SessionData { } Attribute::EmailId => { items.push(DataItem::EmailId { - email_id: Id::from_parts(data.thread_id, id).to_string(), + email_id: Id::from_parts(account_id, id).to_string(), }); } Attribute::ThreadId => { items.push(DataItem::ThreadId { - thread_id: Id::from_parts(0, data.thread_id).to_string(), + thread_id: Id::from_parts(account_id, data.thread_id).to_string(), }); } } diff --git a/crates/imap/src/op/status.rs b/crates/imap/src/op/status.rs index b1f2df23..b574497f 100644 --- a/crates/imap/src/op/status.rs +++ b/crates/imap/src/op/status.rs @@ -192,7 +192,8 @@ impl SessionData { items_response.push(( *item, StatusItemType::String( - Id::from_parts(0, mailbox.mailbox_id).to_string(), + Id::from_parts(mailbox.account_id, mailbox.mailbox_id) + .to_string(), ), )); } diff --git a/crates/jmap/src/changes/get.rs b/crates/jmap/src/changes/get.rs index 49051cba..a926ee80 100644 --- a/crates/jmap/src/changes/get.rs +++ b/crates/jmap/src/changes/get.rs @@ -130,6 +130,12 @@ impl ChangesLookup for Server { } }; + if changelog.is_truncated && request.since_state != State::Initial { + return Err(trc::JmapEvent::CannotCalculateChanges + .into_err() + .details("Changelog has been truncated")); + } + let mut changes = changelog .changes .into_iter() diff --git a/crates/store/src/dispatch/store.rs b/crates/store/src/dispatch/store.rs index 4b426dfa..b25d012d 100644 --- a/crates/store/src/dispatch/store.rs +++ b/crates/store/src/dispatch/store.rs @@ -15,8 +15,8 @@ use trc::{AddContext, StoreEvent}; use crate::{ BitmapKey, Deserialize, IterateParams, Key, QueryResult, SUBSPACE_BITMAP_ID, - SUBSPACE_BITMAP_TAG, SUBSPACE_BITMAP_TEXT, SUBSPACE_INDEXES, SUBSPACE_LOGS, Store, U32_LEN, - Value, ValueKey, + SUBSPACE_BITMAP_TAG, SUBSPACE_BITMAP_TEXT, SUBSPACE_COUNTER, SUBSPACE_INDEXES, SUBSPACE_LOGS, + Store, U32_LEN, Value, ValueKey, write::{ AnyClass, AnyKey, AssignedIds, Batch, BatchBuilder, BitmapClass, BitmapHash, Operation, ReportClass, ValueClass, ValueOp, @@ -387,6 +387,7 @@ impl Store { SUBSPACE_BITMAP_TEXT, SUBSPACE_LOGS, SUBSPACE_INDEXES, + SUBSPACE_COUNTER, ] { self.delete_range( AnyKey { @@ -434,24 +435,6 @@ impl Store { .caused_by(trc::location!())?; } - // Delete property counters (TODO: make this more elegant) - self.delete_range( - ValueKey { - account_id, - collection: 1, - document_id: 0, - class: ValueClass::Property(84), - }, - ValueKey { - account_id, - collection: 1, - document_id: u32::MAX, - class: ValueClass::Property(84), - }, - ) - .await - .caused_by(trc::location!())?; - Ok(()) } diff --git a/tests/src/jmap/enterprise.rs b/tests/src/jmap/enterprise.rs index 9ee2b1c0..2f889c0a 100644 --- a/tests/src/jmap/enterprise.rs +++ b/tests/src/jmap/enterprise.rs @@ -47,23 +47,23 @@ use super::{JMAPTest, ManagementApi, delivery::AssertResult}; const METRICS_CONFIG: &str = r#" [metrics.alerts.expected] enable = true -condition = "domain_count > 1 && cluster_error > 3" +condition = "domain_count > 1 && cluster_publisher_error > 3" [metrics.alerts.expected.notify.event] enable = true -message = "Yikes! Found %{cluster.error}% cluster errors!" +message = "Yikes! Found %{cluster.publisher-error}% cluster errors!" [metrics.alerts.expected.notify.email] enable = true from-name = "Alert Subsystem" from-addr = "alert@example.com" to = ["jdoe@example.com"] -subject = "Found %{cluster.error}% cluster errors" -body = "Sorry for the bad news, but we found %{domain.count}% domains and %{cluster.error}% cluster errors." +subject = "Found %{cluster.publisher-error}% cluster errors" +body = "Sorry for the bad news, but we found %{domain.count}% domains and %{cluster.publisher-error}% cluster errors." [metrics.alerts.unexpected] enable = true -condition = "domain_count < 1 || cluster_error < 3" +condition = "domain_count < 1 || cluster_publisher_error < 3" [metrics.alerts.unexpected.notify.event] enable = true diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index 171a7d14..e7693bb8 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -101,7 +101,7 @@ async fn jmap_tests_() { .await; webhooks::test(&mut params).await; - /*email_query::test(&mut params, delete).await; + email_query::test(&mut params, delete).await; email_get::test(&mut params).await; email_set::test(&mut params).await; email_parse::test(&mut params).await; @@ -119,7 +119,7 @@ async fn jmap_tests_() { event_source::test(&mut params).await; push_subscription::test(&mut params).await; sieve_script::test(&mut params).await; - vacation_response::test(&mut params).await;*/ + vacation_response::test(&mut params).await; email_submission::test(&mut params).await; websocket::test(&mut params).await; quota::test(&mut params).await; @@ -907,8 +907,8 @@ attempts.interval = "500ms" [jmap.email] auto-expunge = "1s" -[jmap.protocol.changes] -max-history = "1s" +[changes] +max-history = "1" [store."auth"] type = "sqlite" diff --git a/tests/src/jmap/purge.rs b/tests/src/jmap/purge.rs index d9293186..83e2a31f 100644 --- a/tests/src/jmap/purge.rs +++ b/tests/src/jmap/purge.rs @@ -87,7 +87,9 @@ pub async fn test(params: &mut JMAPTest) { } if pass == 1 { - changes = get_changes(&server).await; + let (changes_, is_truncated) = get_changes(&server).await; + assert!(!is_truncated); + changes = changes_; tokio::time::sleep(std::time::Duration::from_secs(1)).await; } else { break; @@ -142,14 +144,16 @@ pub async fn test(params: &mut JMAPTest) { .assert_contains("\"Junk Mail\" (MESSAGES 1)"); // Compare changes - let new_changes = get_changes(&server).await; + let (new_changes, is_truncated) = get_changes(&server).await; assert!(!changes.is_empty()); assert!(!new_changes.is_empty()); - for change in changes { + assert!(is_truncated); + for change in &changes { assert!( - !new_changes.contains(&change), - "Change {:?} was not purged", - change + !new_changes.contains(change), + "Change {change:?} was not purged, expected {} changes, got {}", + changes.len(), + new_changes.len() ); } @@ -164,8 +168,9 @@ pub async fn test(params: &mut JMAPTest) { assert_is_empty(server).await; } -async fn get_changes(server: &Server) -> AHashSet<(u64, u8)> { +async fn get_changes(server: &Server) -> (AHashSet<(u64, u8)>, bool) { let mut changes = AHashSet::new(); + let mut is_truncated = false; server .core .storage @@ -183,17 +188,20 @@ async fn get_changes(server: &Server) -> AHashSet<(u64, u8)> { change_id: u64::MAX, }, ) - .ascending() - .no_values(), - |key, _| { - changes.insert(( - key.deserialize_be_u64(key.len() - U64_LEN).unwrap(), - key[U32_LEN], - )); + .ascending(), + |key, value| { + if !value.is_empty() { + changes.insert(( + key.deserialize_be_u64(key.len() - U64_LEN).unwrap(), + key[U32_LEN], + )); + } else { + is_truncated = true; + } Ok(true) }, ) .await .unwrap(); - changes + (changes, is_truncated) }