Skip to content

fix: make create_send_msg_jobs actually return row IDs - #8585

Merged
link2xt merged 3 commits into
mainfrom
link2xt/create_send_msg_jobs-row_id
Aug 26, 2026
Merged

link2xt merged 3 commits into
mainfrom
link2xt/create_send_msg_jobs-row_id

Conversation

@link2xt

@link2xt link2xt commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator

.execute() was returning the number of rows, so usually 1.
.insert() is returning the row ID.

In most cases it does not matter because the result is checked with .is_empty(), but send_msg_sync() actually uses the row IDs.

@link2xt

This comment was marked as outdated.

@link2xt
link2xt force-pushed the link2xt/create_send_msg_jobs-row_id branch from 20ae3c5 to 889578d Compare August 14, 2026 11:40
@link2xt
link2xt force-pushed the link2xt/create_send_msg_jobs-row_id branch 3 times, most recently from 1ca89ff to e657a72 Compare August 25, 2026 14:08
@link2xt
link2xt marked this pull request as ready for review August 25, 2026 14:08
chat1.send_msg_sync(msg1)

lp.sec("ac2: receive message")
msg_in = ac2._evtracker.wait_next_incoming_message()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test gets stuck without the fix, likely here because the message is never sent out as it tries to send rowid 1 from smtp table.

@link2xt

link2xt commented Aug 25, 2026 •

Copy link
Copy Markdown
Collaborator Author

Ended up writing a python test for dc_send_msg_sync() because it is the only API that is affected and only exists as CFFI. IIRC it is used by iOS for share extension.

Comment thread python/src/deltachat/chat.py Outdated
sent_id = lib.dc_send_msg_sync(self.account._dc_context, self.id, msg._dc_msg)
if sent_id == 0:
raise ValueError("message could not be sent")
# modify message in place to avoid bad state for the caller

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is copy-pasted from a function above.

@hpk42 hpk42 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good enough (see review comments). Also i think it'd be good to have an offline Rust test queuing two messages and testing the row-id behaviour.

Comment thread python/tests/test_1_online.py
Comment thread python/src/deltachat/chat.py Outdated
Comment on lines +262 to +305
if sent_msg is None:
raise ValueError("cannot load just sent message from the database")
msg._dc_msg = sent_msg._dc_msg
return msg

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is almost complete duplication. except for one different lib call. Please consider avoiding it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplicated it for these two functions with _reload_sent_msg. There are other functions like send_text etc. but they don't take existing message as an input, so don't need to modify it in place.

It is not called from outside the "chat" module.
.execute() was returning the number of rows, so usually 1.
.insert() is returning the row ID.

In most cases it does not matter because the result is checked with .is_empty(),
but send_msg_sync() actually uses the row IDs.
@link2xt
link2xt force-pushed the link2xt/create_send_msg_jobs-row_id branch from e657a72 to a67a5b0 Compare August 26, 2026 14:36
@link2xt

link2xt commented Aug 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

Also i think it'd be good to have an offline Rust test queuing two messages and testing the row-id behaviour.

I tried to do it before writing a python test, but prepare_send_msg is private and create_send_msg_jobs should have been private (i pushed a commit changing to from pub(crate) to private now), so passing row IDs around happens only inside the chat module and is an implementation detail. It is possible to test the implementation, but then it will stand in the way of any refactoring that removes prepare_send_msg, which could use some refactoring as the concept of "preparing" a message is not a thing anymore and is already removed from all public APIs.

@link2xt
link2xt merged commit 9a33555 into main Aug 26, 2026
30 checks passed
@link2xt
link2xt deleted the link2xt/create_send_msg_jobs-row_id branch August 26, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants