- Replace .ok_or_else() and .map_err() with anyhow::Context where possible.
- Use .context() to check Option for None when it's an error
- Resultify Chatlist.get_chat_id()
- Add useful .context() to some errors
- IMAP error handling cleanup
`imap` table maps Message-IDs to UIDs on the server. `dc_receive_imf`
no longer gets the UID of the message as an argument and does not
insert the folder and UID of the message into the `msgs`
table. `server_folder` and `server_uid` columns in `msgs` table are
deprecated.
MoveMsg and DeleteMsgOnImap jobs are removed. Now messages are moved
and deleted only in the `fetch_move_delete` procedure that consults
the `target` column of the `imap` table to determine where the message
should go.
Where the message should go is determined after prefetching by the
`imap::target_folder()` procedure. Messages are only downloaded once
they reach their target folder to avoid race conditions in multidevice
setting, such as:
1. One device trying to FETCH the message while the other tries to
MOVE it.
2. One device marking the message as \Seen in the Inbox while the
other has already copied unseen message to the Movebox and is going to
delete the \Seen message in the Inbox.
3. Device downloads the message from the Inbox while there are newer
messages in the Movebox placed there by the other device, thus
processing the messages out of order.
Update gossiped_timestamp when someone else sends autocrypt gossip in
the group, so we postpone sending gossip again ourselves.
- Warn about failures to parse Autocrypt-Gossip header
- Move gossip-related methods into ChatId impl
- Fix a "gossi_pp_ed" typo
* add basic multi-device-sync functions
* generate json
* add context.parse_sync_items()
* add context.execute_sync_items()
* piggyback sync-commands message, add body for human-readable part
* avoid double json renderings
* mimeparser parses incoming .json sync-files
* do not piggyback sync-files
* execute sync items
* return status of send_sync_msg()
* send sync messages as multipart/report
* add a per-item-timestamp and also allow adding other per-item-fields in the future
* if the self-chat does not exist, create it blocked/hidden
* create tokens closer to real qr-code needs
* respect bcc_self setting, add test for that
* sync qr code tokens after promoting groups
* send sync-messages only if an experimental switch is set
* trigger send_sync_msg() after sending messages and after creating/redraw/revive qr-code
* add DC_STR_* constants to deltachat.h
* adapt to refactored qr module as of #2729
* tweak test
* use SendSyncMsgs config name instead of SendExperimentalSyncMsgs - we can remove or rename the config nevertheless, but have the option to keep it without renaming
* tweak docs
* remove currently unused effective timestamp calculation
* clarify when send_sync_msg() is called
* make sure, sync-messages are encrypted and are sent by SELF
* tweak docs, fix typos
The problem was:
When opening the connectivity view while there is no network,
get_connectivity_html() calls schedule_quota_update(), which schedules a
UpdateRecentQuota job. But in update_recent_quota(), connecting fails
and a ConnectivityChanged event is emitted (connectivity changes from
Error to Connecting and back). Therefore the UI calls
get_connectivity_html() again, and the loop is complete.
This made the UI completely unresponsible. To reproduce, just turn wi-fi
off and open the connectivity view.
The fix is:
schedule_quota_update() now only schedules a new job if there is no old
job. To prevent the possible (though probably unlikely) problem that an
old quota update job has a backoff of, like, a day and therefore quota
is not updated, I reduced the backoff time for quota jobs to 10s.
Fixes possibly https://github.com/deltachat/deltachat-android/issues/2043, but we should "re-try" this
* draft a download-api
* basic implementation
* allow partial downloads for protected chats
* use a separate column for download_state
* force a minimal timeout for delete_server_after in combination with partial messages
* add a warning if a possible download may expire by delete_server_after
* test load_imap_deletion_msgid()
* add a test for a partial download
* improve documentation and visibility
* let get_download_limit() return Result<Option>
* rusty getters
* apply MIN_DELETE_SERVER_AFTER to shown availability time
* move stub-creation to download.rs, use stock-strings, nicer logging
* make clippy happy (cargo clippy --tests)
* refine tests and comments
* fix typo
* remove superfluous closure in ffi
* respect partial_download for immediately scheduled DeleteMsgOnImap jobs
* resultify update_recent_quota()
* add a device-message if quota exceeds QUOTA_WARN_THRESHOLD_PERCENTAGE
* check if a quota warning should be added during housekeeping, this is at least once a day
* dc_get_config("quota_exceeding") is useful for bots
* make clippy happy
* reword QuotaExceedingMsgBody message
* avoid lots of warnings if quota jitters around the warning threshold
* allow clippy::assertions_on_constants
these constants depend on each other, it makes sense to check that they are not changed in an incompatible way
* add imap::get_quota_roots()
* schedule quote-checking job on getting connectivity-html
* get quota and debug print it
* basic quota output
* update quota at most once per minute, emit event on changes
* use more meaningful names
* add some comments, move update_recent_quota() to quota.rs
* show root name only if there are several roots
* make clippy happy, some refactorings
* allow only one update-quota job per time
* add now supported QUOTA to standards.md
fix#2539
It's always a bit ambiguous which function should do set_err or set last_send_error, I used this rule here:
If some code returns Status::RetryLater, then it sets last_send_error, because Status::RetryLater means that the user won't see the error directly on the message (if we returned Status::Failed(Err(_)), then the message would be shown as failed to the user) Also, smtp_send always sets last_send_error because, well, sending just failed or succeeded.
Also, remove unused field pending_error.
Contact request chats are not merged into a single virtual "deaddrop"
chat anymore. Instead, they are shown in the chatlist the same way as
other chats, but sending of messages to them is not allowed and MDNs
are not sent automatically until the chat is "accepted" by the user.
New API:
- dc_chat_is_contact_request(): returns true if chat is a contact
request. In this case option to accept and block the chat via
dc_accept_chat() and dc_block_chat() should be shown in the UI.
- dc_accept_chat(): accept contact request and unblock the chat
- dc_block_chat(): decline contact request and block the chat
Removed API:
- dc_create_chat_by_msg_id(): deprecated 2021-02-07 in favor of
dc_decide_on_contact_request()
- dc_marknoticed_contact(): deprecated 2021-02-07 in favor of
dc_decide_on_contact_request()
- dc_decide_on_contact_request(): this call requires a message ID from
deaddrop chat as input. As deaddrop chat is removed, this call can't
be used anymore.
- dc_msg_get_real_chat_id(): use dc_msg_get_chat_id() instead, the
only difference between these calls was in handling of deaddrop chat
- removed DC_CHAT_ID_DEADDROP and DC_STR_DEADDROP constants
See https://support.delta.chat/t/discussion-how-to-show-error-states/1363/10 <!-- comment -->
It turns out that it's pretty easy to distinguish between lots of states (currently Error/NotConnected, Connecting…, Getting new messages… and Connected). What's not that easy is distinguishing between an actual error and no network, because if the server just doesn't respond, it could mean that we don't have network or that we are trying ipv6, but only ipv4 works.
**WRT debouncing:**
Sending of EVENT_CONNECTIVITY_CHANGED is not debounced, but emitted every time one of the 3 threads (Inbox, Mvbox and Sentbox) has a network error, starts fetching data, or is done fetching data.
This means that it is emitted:
- 9 times when dc_maybe_network() is called or we get network connection
- 12 times when we lose network connection
Some measurements: dc_get_connectivity() takes a little more than 1ms (in my measurements back in March), dc_get_connectivity_html() takes 10-20ms. This means that it's no immmediate problem to call them very often, might increase battery drain though. For the UI it may be a lot of work to update the title everytime; at least Android is smart enough to update the title only once.
Possible problems (we don't have to worry about them now I think):
- Due to the scan_folders feature, if the user has lots of folders, the state could be "Connecting..." for quite a long time, generally DC seemed a little unresponsive to me because it took so long for "Connecting..." to go away. Telegram has a state "Updating..." that sometimes comes after "Connecting...".
To be done in other PRs:
- Better handle the case that the password was changed on the server and authenticating fails, see https://github.com/deltachat/deltachat-core-rust/issues/1923 and https://github.com/deltachat/deltachat-core-rust/issues/1768
- maybe event debouncing (except for "Connected" connectivity events)
fix https://github.com/deltachat/deltachat-android/issues/1760
`Imap` structure is always created in a configured state now. There is
no default value for `ImapConfig` anymore.
Also resultify Scheduler::start() to fail on database errors, for
example if IMAP configuration cannot be read from the database during
`start_io()`. Previosuly errors during reading keys such as
`mvbox_watch` were simply ignored and folders were not watched until
the application is completely restarted, now start_io() will fail and
scheduler will only be started at the next start_io() call which
usually happens when app is brought to the foreground.
Using `impl AsRef<str>` as the argument instead of `&str` makes it
possible to call the function with `&str`, `String` and other types
that implement `AsRef` trait.
The cost of it is that compiled binary contains mulitple versions of
the same function, one for each variant of types. If function contains
multiple generic `impl AsRef` arguments, the number of versions possibly
compiled into binary grows exponentially with the number of arguments.
Simple way to avoid it is to call `.as_ref()` on the caller side to
convert the argument to `&str`. In most cases even adding a `&` and
relying on `Deref` coercion is sufficient.
This patch changes many functions that accepted `impl AsRef<str>` and
`impl AsRef<Path>` to accept `&str` and `&Path` instead.
In some places `.clone()` calls are removed. Calling `.clone()` on
`String` and passing `String` to a function accepting `impl
AsRef<str>` is completely unnecessary as `&str` reference could be
passed instead. There is no clippy warning against it yet, but
changing argument type to `&str` allowed to find these cases.
The result of debloating is not impressive, several hundred kilobytes
are saved, which is about 3% of the `.so` binary, but the code is
cleaner too.
This moves the module-level lookup and creation functions to the
types, which make the naming more consistent. Now the lookup_* get_*
and create_* functions all behave similarly.
Peraps even more important the API of the lookup now allows
distinguishing failure from not found. This in turn is important to
be able to remove reliance on a ChatId with a 0 or "unset" value. The
locations where this ChatId(0) is still used is in database queries
which should be solved in an independed commit.
* Remove sql::error submodule
Use anyhow errors instead.
* Remove explicit checks for open SQL connection
An error will be thrown anyway during attempt to execute query.
* Don't use `with_conn()` and remove it
* Remove unused `with_conn_async`
* Resultify markseen_msgs
* Fix#2335 (delete job flooding)
The problem was:
- You are offline, and an ephemeral message is due to delete.
- load_imap_deletion_job() is called and returns a deletion job for it.
- The job fails because, well, we are offline.
- The job is saved into the database.
- load_imap_deletion_job() is called again, and so on.
* Add test
Switches from rusqlite to sqlx to have a fully async based interface
to sqlite.
Co-authored-by: B. Petersen <r10s@b44t.com>
Co-authored-by: Hocuri <hocuri@gmx.de>
Co-authored-by: link2xt <link2xt@testrun.org>
fix#2254: if the DB was closed without calling stop_io() and then an interrupt arrives (e.g. incoming message), the db was corrupted.
* Add result.log() for logging with less boilerplate code
* Bugfix: Resultify housekeeping() to make it abort if the db is closed instead of just deleting everything
* Require the UI to call dc_stop_io() before backup export
* Prepare a bit better for closed-db: Resultify get_uidvalidity and get_uid_next and let job::load_next() wait until the db is open
About the bug (before this PR):
if the DB was closed without calling stop_io() and then an interrupt arrives (e.g. incoming message):
- I don't know if it downloads the message, but of course at some point the process of receiving the message will fail
- In my test, DC is just in the process of moving a message when the imex starts, but then can't delete the job or update the msg server_uid
- Then, when job::load_next() is called, no job can be loaded. That's why it calls `load_housekeeping_job()`. As `load_housekeeping_job()` can't load the time of the last housekeeping, it assumes we never ran housekeeping and returns a new Housekeeping job, which is immediately executed.
- housekeeping can't find any blobs referenced in the db and therefore deletes almost all blobs.
The chat::lookup_by_contact_id call is already resultified, the
database can not contain this since the auto-increment counter is
bumped to 7 by the time the database tables are created.
* Copypaste-merge my old work
* Start implementing mailinglists the new way
* Create pseudo contact
* Fine-tune docs
* Remove some unnecessary changes
* style
* Make a stock str
* Fix a crash. Yes, this line caused a panic when reconfiguring on Android
(without a reasonable error log). Also, I could not receive any messages
anymore.
* rfmt
* Add tests and make them pass
* Even more tests
* rfmt
* Enhance test and fix bug
* Don't update the sender name when prefetching because maybe it's a mailing list
* Use an enum instead of numbers for the decision
* Don't remove anyone from mailing lists
* Fix bug in the regex
* Adjust error msg
* Compile error after rebase
* Correctly emit event
* Add dc_msg_is_mailing_list so that you can find out whether messages in the deaddrop belong to mailing lists.
* Add received headers to unit tests
* Comments, small tweaks
* Use dc_msg_get_override_sender_name instead of dc_msg_get_sender_name
* Add dc_msg_get_sender_first_name() because sometimes the first name was not correctly shown in mailing lists
* small fixes, don't let the user modify mailing list groups
* Hide contacts for addresses like noreply@github.com and reply+AEJ...@reply.github.com
When testing mailing lists, I noticed that sometimes a mailing list
contact got a name (like, hocuri <noreply@github.com>). It turned out
that ages ago, I had accidentally written an email to - in this example
- hocuri <noreply@github.com> and it had been added to the contacts
list.
This hides email addresses from the contacts list that are obviously not
meant to be written at and prevents updating the names.
* Comment, clippy
* Replace u32 with ChatId
* Resolve lost of small issues from the reviews
* remove dc_msg_get_sender_first_name
* add dc_msg_get_real_chat_id()
this allows to check if a contact request belongs to a mailing list
and to show name/avatar/whatever of the mailinglist.
another approach was to duplicate some chat-apis for messages
(eg. dc_msg_is_mailing_list()) however that would require far more new apis.
the idea to change the behavior of dc_msg_get_chat_id() would be
more clean, however, that easily breaks existing implementations
and can led to hard to find bugs.
* remove now unused Message.is_mailing_list()
* if a name for a mailing list is missing, use List-ID
* fix comment
* fix error message
* document how dc_get_chat_contacts() works for mailing lists
* refine decide api (#2185)
* add DC_DECIDE* constants to deltachat.h, tweak documentation
* use StartChat/Block/NotNow instead of Yes/Never/NotNow
* decide_on_contact_request works on ctx/msg-id
functions working on message-objects
usually do not read or write directly on the database.
therefore, decide_on_contact_request()
should not work with message-objects as well,
it is even a bit misleading, as eg. chat-id of the object is not modified.
instead, the function works on context,
similar to dc_send_msg(), dc_create_chat() and so on.
for now, i moved it to context, could maybe be part od MsgId.
* Update src/chatlist.rs
Co-authored-by: Hocuri <hocuri@gmx.de>
Co-authored-by: Hocuri <hocuri@gmx.de>
* refine documentation
* re-add accidentally deleted Param::MailingList
* remove pseudo-contact in domain @mailing.list
1. the pseudo-contact was added to the mailing list contacts,
which is not needed.
might be that we want to add the recent contacts there in a subsequent pr
(we do not know all contacts of a mailing list)
2. the pseudo-contact was used to block
mailing lists; this is done by setting the chat to Blocked::Manually now
3- the pseudo-contact was used for unblocking;
as it is very neat not to require additional ui for mailing list unblocking,
might be that we introduce a similar pseudo-contact for this limited purpose
in a subsequent pr, however, the pseudo-contact needs to exist only
during unblocking then, maybe also the special domain is not needed,
we'll see :)
* Move dc_decide_on_contact_request() up to the dc_context section as it's a member of dc_context now
More specifically, to the "handle messages" section
* re-introduce Chattype::Mailinglist (#2195)
* re-introduce Chattype::Mailinglist
* exhaustive chattype-check in fetch_existing_msgs() and get_summary2()
* exhaustive chattype-check in ndn_maybe_add_info_msg()
* exhaustive chattype-check in message::fill()
* remove dc_chat_is_mailing_list() from ffi
Co-authored-by: B. Petersen <r10s@b44t.com>
This removes the proxy via crate::error to depend on anyhow directly.
There is no benefit to this indirection and this makes it simpler to
see which error types are used.
