Skip to content

Commit fe362a1

Browse files
committed
melib/imap: fix mailbox update logic on expunge
When we received an untagged EXPUNGE response, we should issue a Remove refresh event for the currently selected mailbox only. Fixes #510 ("Deleted emails showing up in the listing") Resolves: https://git.meli-email.org/meli/meli/issues/510 Signed-off-by: Manos Pitsidianakis <[email protected]>
1 parent 269c2cd commit fe362a1

File tree

1 file changed

+76
-81
lines changed

1 file changed

+76
-81
lines changed

melib/src/imap/untagged.rs

Lines changed: 76 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use crate::{
3636
generate_envelope_hash, FetchResponse, ImapLineSplit, RequiredResponses,
3737
UntaggedResponse,
3838
},
39-
sync::cache::ignore_not_found,
39+
sync::cache::{ignore_not_found, ImapCache},
4040
ImapConnection, MailboxSelection, UID,
4141
},
4242
};
@@ -121,54 +121,56 @@ impl ImapConnection {
121121
);
122122
}
123123
let mut events = vec![];
124-
let deleteds = self
125-
.uid_store
126-
.uid_index
127-
.lock()
128-
.unwrap()
129-
.iter()
130-
.filter(|((_, u), _)| !results.contains(u))
131-
.map(|((_, uid), hash)| (*uid, *hash))
132-
.collect::<Vec<(UID, crate::email::EnvelopeHash)>>();
133-
let mut mboxes_to_update = vec![];
134-
for (deleted_uid, deleted_hash) in deleteds {
135-
for (mailbox_hash, mbx) in self.uid_store.mailboxes.lock().await.iter_mut()
136-
{
137-
mbx.exists.lock().unwrap().remove(deleted_hash);
138-
mbx.unseen.lock().unwrap().remove(deleted_hash);
139-
let mut existed = self
140-
.uid_store
124+
{
125+
let mut mboxes = self.uid_store.mailboxes.lock().await;
126+
let deleted_uids_hashes = self
127+
.uid_store
128+
.uid_index
129+
.lock()
130+
.unwrap()
131+
.iter()
132+
.filter(|((mbx, u), _)| *mbx == mailbox_hash && !results.contains(u))
133+
.map(|((_, uid), hash)| (*uid, *hash))
134+
.collect::<Vec<(UID, crate::email::EnvelopeHash)>>();
135+
for (deleted_uid, deleted_hash) in deleted_uids_hashes {
136+
self.uid_store
141137
.uid_index
142138
.lock()
143139
.unwrap()
144-
.remove(&(*mailbox_hash, deleted_uid))
145-
.is_some();
146-
existed |= self
147-
.uid_store
140+
.remove(&(mailbox_hash, deleted_uid));
141+
{
142+
if let Some(mbx) = mboxes.get_mut(&mailbox_hash) {
143+
mbx.exists.lock().unwrap().remove(deleted_hash);
144+
mbx.unseen.lock().unwrap().remove(deleted_hash);
145+
}
146+
}
147+
self.uid_store
148148
.hash_index
149149
.lock()
150150
.unwrap()
151-
.remove(&deleted_hash)
152-
.is_some();
153-
if existed {
154-
mboxes_to_update.push(*mailbox_hash);
155-
events.push((
156-
deleted_uid,
157-
RefreshEvent {
158-
account_hash: self.uid_store.account_hash,
159-
mailbox_hash: *mailbox_hash,
160-
kind: Remove(deleted_hash),
161-
},
162-
));
163-
}
151+
.remove(&deleted_hash);
152+
events.push((
153+
deleted_uid,
154+
RefreshEvent {
155+
account_hash: self.uid_store.account_hash,
156+
mailbox_hash,
157+
kind: Remove(deleted_hash),
158+
},
159+
));
164160
}
165161
}
166-
for mailbox_hash in mboxes_to_update {
167-
use crate::imap::sync::cache::ImapCache;
168-
169-
self.uid_store
170-
.update(mailbox_hash, &events)
171-
.or_else(ignore_not_found)?;
162+
if let Err(err) = self
163+
.uid_store
164+
.update(mailbox_hash, &events)
165+
.or_else(ignore_not_found)
166+
{
167+
log::error!(
168+
"Could not update cache for mailbox_hash = {:?} uid, events = {:?}: \
169+
err = {}",
170+
mailbox_hash,
171+
events,
172+
err
173+
);
172174
}
173175
for (_, event) in events {
174176
self.add_refresh_event(event);
@@ -188,53 +190,52 @@ impl ImapConnection {
188190
return Ok(true);
189191
};
190192
imap_log!(trace, self, "expunge {}, UID = {}", n, deleted_uid);
191-
let deleted_hash: crate::email::EnvelopeHash = match self
193+
let Some(deleted_hash) = self
192194
.uid_store
193195
.uid_index
194196
.lock()
195197
.unwrap()
196198
.remove(&(mailbox_hash, deleted_uid))
197-
{
198-
Some(v) => v,
199-
None => return Ok(true),
199+
else {
200+
return Ok(true);
200201
};
201-
let mut mboxes_to_update = vec![];
202-
for (mailbox_hash, mbx) in self.uid_store.mailboxes.lock().await.iter_mut() {
203-
if mbx.exists.lock().unwrap().remove(deleted_hash) {
204-
mboxes_to_update.push(*mailbox_hash);
202+
{
203+
let mut mboxes = self.uid_store.mailboxes.lock().await;
204+
if let Some(mbx) = mboxes.get_mut(&mailbox_hash) {
205+
mbx.exists.lock().unwrap().remove(deleted_hash);
206+
mbx.unseen.lock().unwrap().remove(deleted_hash);
205207
}
206-
mbx.unseen.lock().unwrap().remove(deleted_hash);
207208
}
208209
self.uid_store
209210
.hash_index
210211
.lock()
211212
.unwrap()
212213
.remove(&deleted_hash);
213-
let events = mboxes_to_update
214-
.into_iter()
215-
.map(|mailbox_hash| {
216-
(
217-
mailbox_hash,
218-
[(
219-
deleted_uid,
220-
RefreshEvent {
221-
account_hash: self.uid_store.account_hash,
222-
mailbox_hash,
223-
kind: Remove(deleted_hash),
224-
},
225-
)],
226-
)
227-
})
228-
.collect::<Vec<(_, [(UID, RefreshEvent); 1])>>();
229-
for (mailbox_hash, pair) in events {
230-
use crate::imap::sync::cache::ImapCache;
214+
let pair = [(
215+
deleted_uid,
216+
RefreshEvent {
217+
account_hash: self.uid_store.account_hash,
218+
mailbox_hash,
219+
kind: Remove(deleted_hash),
220+
},
221+
)];
231222

232-
self.uid_store
233-
.update(mailbox_hash, &pair)
234-
.or_else(ignore_not_found)?;
235-
let [(_, event)] = pair;
236-
self.add_refresh_event(event);
223+
if let Err(err) = self
224+
.uid_store
225+
.update(mailbox_hash, &pair)
226+
.or_else(ignore_not_found)
227+
{
228+
log::error!(
229+
"Could not update cache for mailbox_hash = {:?} uid = {:?}, events = \
230+
{:?}: err = {}",
231+
mailbox_hash,
232+
deleted_uid,
233+
pair,
234+
err
235+
);
237236
}
237+
let [(_, event)] = pair;
238+
self.add_refresh_event(event);
238239
}
239240
UntaggedResponse::Exists(n) => {
240241
imap_log!(trace, self, "exists {}", n);
@@ -300,7 +301,7 @@ impl ImapConnection {
300301
.unwrap()
301302
.entry(mailbox_hash)
302303
.or_default()
303-
.insert(*message_sequence_number, uid);
304+
.insert(message_sequence_number.saturating_sub(1), uid);
304305
}
305306
self.uid_store
306307
.hash_index
@@ -322,8 +323,6 @@ impl ImapConnection {
322323
);
323324
}
324325
{
325-
use crate::imap::sync::cache::ImapCache;
326-
327326
if let Err(err) = self
328327
.uid_store
329328
.insert_envelopes(mailbox_hash, &v)
@@ -430,8 +429,6 @@ impl ImapConnection {
430429
mailbox.exists.lock().unwrap().insert_new(env.hash());
431430
}
432431
{
433-
use crate::imap::sync::cache::ImapCache;
434-
435432
if let Err(err) = self
436433
.uid_store
437434
.insert_envelopes(mailbox_hash, &v)
@@ -467,7 +464,7 @@ impl ImapConnection {
467464
.unwrap()
468465
.entry(mailbox_hash)
469466
.or_default()
470-
.insert(message_sequence_number, uid);
467+
.insert(message_sequence_number.saturating_sub(1), uid);
471468
}
472469
self.uid_store
473470
.hash_index
@@ -582,8 +579,6 @@ impl ImapConnection {
582579
},
583580
)];
584581
{
585-
use crate::imap::sync::cache::ImapCache;
586-
587582
self.uid_store.update(mailbox_hash, &event)?;
588583
}
589584
self.add_refresh_event(std::mem::replace(

0 commit comments

Comments
 (0)