Opened 6 months ago

Last modified 3 weeks ago

#35581 assigned Cleanup/optimization

Upgrade django.core.mail to use Python's modern email API

Reported by: Mike Edmunds Owned by: Mike Edmunds
Component: Core (Mail) Version: dev
Severity: Normal Keywords: email, compat32
Cc: bcail Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.core.mail.EmailMessage currently uses Python's "legacy" email API. Updating that code to Python's newer "modern" email API would fix bugs and substantially simplify it.

django-developers discussion

The update should be mostly transparent for users of Django's documented mail APIs, but will rework the internals of django.core.mail.message in ways that may require changes to some third-party email backends and other code that borrows undocumented (but popular) functions from that module. In addition, parts of Django's mail test suite will need reworking to avoid dependence on legacy implementation specifics.

Some relevant posts from django-developers:

Change History (17)

comment:1 by Tim Graham, 6 months ago

Triage Stage: UnreviewedAccepted
Version: dev

comment:2 by YashRaj1506, 4 months ago

Owner: changed from Mike Edmunds to YashRaj1506

I will try to solve this.

comment:3 by Mike Edmunds, 4 months ago

Owner: changed from YashRaj1506 to Mike Edmunds

@YashRaj1506 I have a bunch of work underway on this and will open a PR soon. Don't want you to needlessly duplicate effort. Code review on the PR would be very welcome once it's ready.

Part of the delay is I've run into a handful of open issues in Python's modern email API that may require further discussion:

Also trying to decide how best to handle Django's EmailMessage.encoding. It's not documented, but a bunch of tests cover setting it and checking the results. (The only relevant docs are this note about the DEFAULT_CHARSET setting also applying to django.core.mail.)

comment:4 by Mike Edmunds, 4 months ago

Has patch: set

I've opened two sequential PRs, as suggested in the django-developers discussion. Both are ready for review:

(The second PR is in my own Django fork so that it can be based on the first PR without overlap. I'll reopen it upstream once the first PR is closed. But reviews are welcome now if you don't mind working in my fork; just note any discussion history there won't transfer to the eventual upstream PR.)

Some additional notes are in the PR comments.

comment:5 by Mike Edmunds, 4 months ago

Patch needs improvement: set

comment:6 by Mike Edmunds, 4 months ago

Patch needs improvement: unset

comment:7 by bcail, 2 months ago

Cc: bcail added

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 8 weeks ago

In 889be2f:

Refs #35581 -- Clarified some test names and comments in mail tests.

comment:9 by Sarah Boyce <42296566+sarahboyce@…>, 8 weeks ago

In 4d76adfa:

Refs #35581 -- Verified attachments in the generated message in mail tests.

This also removed send() calls, as this doesn't check the serialized content, and
the backend tests cover sending.

comment:10 by Sarah Boyce <42296566+sarahboyce@…>, 8 weeks ago

In 00861c4c:

Refs #35581 -- Identified mail tests that check for Python 2 behavior.

This also removed a duplicate CTE case (that used to be distinct in Python 2).

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 8 weeks ago

In cf4d902:

Refs #35581 -- Reduced boilerplate in mail tests.

comment:12 by Sarah Boyce, 3 weeks ago

Triage Stage: AcceptedReady for checkin

comment:13 by Sarah Boyce <42296566+sarahboyce@…>, 3 weeks ago

In bddd35c:

Refs #35581 -- Improved reporting for failing tests in mail tests.

  • Converted HeadersCheckMixin to MailTestsMixin for all shared helpers:
    • Hoisted assertStartsWith() from BaseEmailBackendTests.
    • Added matching assertEndsWith().
    • Hoisted get_decoded_attachments() from MailTests.
    • Improved failure reporting in assertMessageHasHeaders().
  • Used unittest subTest() to improve handling of compound test cases.
  • Replaced assertTrue(test on string) with custom assertions, so that failure reporting is more informative than True != False.

comment:14 by Sarah Boyce <42296566+sarahboyce@…>, 3 weeks ago

In 5d7001b:

Refs #35581 -- Used modern email parser and helpers in mail tests.

  • Used modern email API (policy.default) for tests that reparse generated messages, and switched to modern accessors where helpful.
  • Split get_raw_attachments() helper out of get_decoded_attachments(), and used modern iter_attachments() to avoid finding nested attachments in attached message/* emails.
  • Stopped using legacy parseaddr.

comment:15 by Sarah Boyce <42296566+sarahboyce@…>, 3 weeks ago

In ea34de3b:

Refs #35581 -- Added tests for email parameters, attachments, MIME structure, bcc header, encoding and sending.

comment:16 by Sarah Boyce, 3 weeks ago

Triage Stage: Ready for checkinAccepted

comment:17 by Sarah Boyce, 3 weeks ago

Has patch: unset
Note: See TracTickets for help on using tickets.
Back to Top