Repository navigation
mailcore: let the engine fetch HTML bodies (#1901) - #1904
Merged
Merged
Conversation
An Html engine gets each body native, so a plain-text message comes back as text: content_type alone cannot tell "asked for html, got text" from "fetched as text by an older engine". bodies.fetched_as records what was asked (put_fetched_body), NULL for local writes and pre-column rows, and get_body_for(id, format) answers whether the stored body satisfies a format, so an embedder refetches each message once, not on every open. update_draft_fields now also makes the body html with no fetch format: compose writes html, and a draft fetched as text kept content_type text over html content, which would have invited a refetch over the edits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EngineOptions { body_format } is chosen at spawn (spawn_with_options);
spawn keeps Text, so lookxy is unchanged. Html sends no Prefer header and
stores each body with the format it asked for. Tests pin the Prefer
header per format, the one-time upgrade of a text-stored body, no refetch
of a native-text answer, and inline attachments' contentId under Html.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A body with no recorded fetch format now satisfies both formats when it belongs to a draft. Under Html, a plain-text createReply draft (stored as text) and a draft an older update_draft_fields left as html content under content_type 'text' read as "asked for text", inviting a FetchBody over unsynced edits that a queued SaveDraft would then push back as Graph's old body (m1, m3). A draft is is_draft or filed in the Drafts folder (synced or the local sentinel): a complete delta item resets is_draft to 0, as isDraft is not in MESSAGE_SELECT, so the flag alone would not last past a sync. A body FetchBody recorded keeps the strict rule. The #1901 store tests move to their own body_format_tests module (m2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Owner
Author
|
Merged automatically (auto-merge is on for this checkout): review stopped: round 1 had no Major; all findings fixed; merge rounds: 1 CI rerun (a timing flake in the untouched Follow-ups: #1905 (FTS indexes html bodies' markup under an |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
SyncCommand::FetchBodyalways asked Graph for plain text (Prefer: outlook.body-content-type="text"), so a graphical client embedding the engine got every message as text. Now:sync::engine::EngineOptions { body_format }(BodyFormat::{Text, Html}, defaultText) is chosen at spawn:spawnruns the default (lookxy unchanged),spawn_with_optionstakes one.Htmlsends noPrefer, so each body comes back native (html, or text for a plain-text message).bodies.fetched_as(additive migration) records the formatFetchBodyasked for, not the content type that came back, so a native-text answer to anHtmlask is not refetched forever.Store::get_body_for(id, format)is the embedder's gate: the stored body when it satisfies the format, elseNone(send oneFetchBody). Old rows from the text-only engine read as "asked for text", so an existing store upgrades one message per open. A draft's body with no recorded format (local,createReply, or a legacy compose edit) satisfies both formats, so a refetch is never invited over unsynced edits (draft =is_draft, or filed under Drafts / the local Drafts sentinel, since delta does not selectisDraft).update_draft_fieldsnow also writescontent_type = 'html'and clearsfetched_ason conflict (it used to keep a fetched-as-text draft'stexttype over html content).contentId/isInlinewere already stored and fetchable (FetchAttachments,FetchInlineImage); a test now pins it under anHtmlengine. Reply/forward drafts unchanged.Closes #1901
Testing
cargo fmt --check,cargo clippy -p mailcore -p lookxy --all-targets -- -D warnings: cleancargo test -p mailcore(317 passed),cargo test -p lookxy(333 passed). Only lookxy depends on mailcore.Preferand records text; anHtmlengine sends none and stores html; a legacy text body is fetched as html exactly once through the gate; a native-text answer is not fetched again; inline attachments storecontentId; a plain-text reply draft is not a refetch candidate. Store tests instore::body_format_testscover the rule, the migration and the draft cases. Mutation checks (by hand): recording the returned content type, or the old draft conflict arm, or dropping the draft rule each fail a test.Review record
Html(data loss), same for plain-textcreateReplydrafts, and the store tests moved to their own module.Htmlstore).