Persist send error state #3806
Persist send error state #3806
Conversation
cketti
force-pushed
the
outbox_state
branch
2 times, most recently
from
4c3d073
to
818f8fc
Compare
| @@ -229,6 +242,7 @@ private static void dbCreateDatabaseFromScratch(SQLiteDatabase db) { | |||
| "BEGIN " + | |||
| "DELETE FROM message_parts WHERE root = OLD.message_part_id; " + | |||
| "DELETE FROM messages_fulltext WHERE docid = OLD.id; " + | |||
| "DELETE FROM outbox_state WHERE message_id = OLD.id; " + | |||
There was a problem hiding this comment.
is there a reason we do this with triggers rather than foreign keys? seems a bit odd. (have I asked this before?)
There was a problem hiding this comment.
Hmm... it seems possible to use foreign keys and CASCADE DELETE in sqlite when using foreign_keys PRAGMA... I didn't know that.
There was a problem hiding this comment.
Of course. We use that in OpenKeychain all the time.
There was a problem hiding this comment.
Yes, I see you have foreign keys enabled but I don't see it in K-9 (it isn't used?).
One way or another I think FKs are better approach and the work to enable them would be a good investment anyway :)
There was a problem hiding this comment.
The codebase has never used foreign keys. I believe it's because early Android versions shipped with a Sqlite version that didn't support foreign key constraints.
| } | ||
|
|
||
| private fun createOutboxStateEntries(db: SQLiteDatabase) { | ||
| db.execSQL("INSERT INTO outbox_state (message_id, send_state) " + |
There was a problem hiding this comment.
any reason to avoid """ here?
| @@ -41,6 +56,12 @@ | |||
| private PreviewType previewType; | |||
| private boolean headerNeedsUpdating = false; | |||
|
|
|||
| // Only available for messages in Outbox folder after calling fetchOutboxState() | |||
There was a problem hiding this comment.
this looks pretty ugly to me. could this (and related methods) be kept in an OutboxStateRepository or something? the calls are simple and rare enough that we don't need to worry about caching, plus it would be more easily testable.
|
|
This adds the
outbox_statetable to the database to keep track of the number of send attempts and hard send errors (server declined to send the message as opposed to temporary connection or server error).It's only a first step towards better send error handling. Nicer notifications, better retry logic, displaying error indicators and actions to perform on messages that failed to send are next steps.