Fix https://github.com/chatmail/core/issues/8042
The problem was that after receiving the bcc_self'ed pre-message in
`receive_imf`, the logic there only looked for a pending
`smtp`-table-entry that matches the rfc724_mid, and if there was none
then it thought "Great, apparently the message is fully sent out, we can
mark it as delivered!".
But with pre-messages, the same message can have two `smtp` entries (one
for the pre-message and one for the post-message), and the message
should only be marked as delivered once both of them are sent out.
Now, I changed the logic to look for all entries with the same msg_id.
This is actually the same SQL query used in smtp.rs, so, I extracted it
into a new function; feel free to suggest a better name for it.
I tested on Android that it now works fine.
I'll add a test in a follow-up PR.
There are a lot of other problems with sending large files, though:
- The pre-message is sent before the post-message, so that for the
receiver it looks as if the message arrived, but stays in
"downloading..." forever
- There is quite a time delay between clicking on "Send" and the
outgoing message appearing in the chat
- The message shortly gets a letter icon right after it is sent
- I'm wondering if there is a way to give feedback to the user
immediately if the message is too big
- It's unclear when exactly we want to send read receipts
I'll open a follow-up issue for these.
This is to avoid sorting incoming messages that
are slightly in the past above system messages
about SecureJoin. SecureJoin messages are
timed according to smeared timestamp,
so even in the local tests they are in the future
by a few seconds.
We set timestamp of this info message to 0
to make it always appear in the beginning of the chat.
To avoid new chats being sorted to the end of the chatlist,
we ignore such 0 and use chat creation timestamp
when sorting the chatlist.
We know which message was added from the return value
of receive_imf(). It may be that the first chat
in the chatlist is not the one where the message was received
if there is a pinned chat or if
just received message is old.
It is not clear now what this is testing.
Golden test shows messages ordered
incorrectly according to the timestamps,
they should be ordered the other way round.
Comment talks about fetching from mvbox and inbox
in paralell which is a rare case that
could have happened if one message is left in the inbox
and the other message is a chat message moved to mvbox.
We never download anything that is not moved to the target folder.
The test also resides in "verified chats" tests
which are all legacy tests we kept after
replacing the concept of verified/protected chats
with key contacts in 2.x.
If our key is gossiped, the message is intended for us.
The check for address is redundant for incoming messages as
if we received the message then it was addressed to us.
This whole protection code can eventually be removed
as we have intended recipient fingerprints already,
it only protects against forwarding of messages
sent by old clients.
With `ORDER BY` statement SQLite searches
the `imap` table by `transport_id` and for each found row
scans the whole `imap_markseen` table.
Number of `imap` entries for each `transport_id`
is usually large as we need to know
which UIDs to delete on IMAP server
when deleting a message.
```
sqlite> EXPLAIN QUERY PLAN
SELECT imap.id, uid, folder FROM imap, imap_markseen
WHERE imap.id = imap_markseen.id
AND imap.transport_id=?
AND target = folder
ORDER BY folder, uid;
QUERY PLAN
|--SEARCH imap USING INDEX sqlite_autoindex_imap_1 (transport_id=?)
`--SCAN imap_markseen
```
Without `ORDER BY` statement SQLite scans `imap_markseen`
table which is expected to be small,
and then searches `imap` table by `rowid` for each found result.
```
sqlite> EXPLAIN QUERY PLAN
SELECT imap.id, uid, folder FROM imap, imap_markseen
WHERE imap.id = imap_markseen.id
AND imap.transport_id=?
AND target = folder;
QUERY PLAN
|--SCAN imap_markseen
`--SEARCH imap USING INTEGER PRIMARY KEY (rowid=?)
```
Query planning was tested with SQLite 3.52.0.
It is possible to explictly make
query planner move sorting to the last step
with `ORDER +folder, +uid`, but this is not recommended
in SQLite documentation
(see <https://www.sqlite.org/optoverview.html#uplus>).
It is also possible to add indexes,
but indexes use space,
adding them requires an SQL migration,
and each index needs to be updated so it will slow down writes.
Only inbox loop is changed because non-inbox loop is going to be removed
together with `mvbox_move`.
Added transport IDs to the log and logging around quota updates.
Removed some logs that add noise,
like logging that IDLE is supported each time right before using it.
The original idea was to always lock `write_mutex` before acquiring an `InnerPool.semaphore` permit
to avoid ABBA deadlocks, but when refactoring a PR for b696a242fc,
that was forgotten.
This doesn't really change the program flow as we have `Context::housekeeping_mutex` anyway,
just simplifies the code.
If we learn about this message being available on IMAP later,
we will add another available_post_msgs row.
If we don't delete the row, we will keep failing each time
until IMAP entry becomes available and it may not happen.
