Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Split multiplart email sending into a dedicated handler #9977

Merged
merged 6 commits into from
May 17, 2021
Merged

Split multiplart email sending into a dedicated handler #9977

merged 6 commits into from
May 17, 2021

Conversation

babolivier
Copy link
Contributor

In the context of making it available to the module API, cf #9884

@babolivier babolivier marked this pull request as ready for review May 13, 2021 13:23
@babolivier babolivier requested a review from a team May 13, 2021 13:23
@babolivier babolivier mentioned this pull request May 13, 2021
Merged
anoadragon453
anoadragon453 previously requested changes May 13, 2021
View reviewed changes
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Some small notes below. My main question is: why can the module API not call Mailer.send_email? Perhaps because it would need to provide a text and HTML templates?

Essentially I don't currently know why this PR is necessary, even after looking at the comment chain in #9884 (comment)

synapse/handlers/account_validity.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Outdated Show resolved Hide resolved
synapse/handlers/send_email.py Show resolved Hide resolved
changelog.d/9977.misc Outdated Show resolved Hide resolved
@babolivier
Copy link
Contributor Author

My main question is: why can the module API not call Mailer.send_email? Perhaps because it would need to provide a text and HTML templates?

I think there are two answers to this question. One is that Mailer is the push mailer, which is intended to only be used to send email push notifications (though I realise it's also used for 3pid management but I believe the right fix would be to move that code outside of the push notification code and to a more sensible location, not to pollute the push code even more with features that aren't related to push). The other is that the Mailer is created with the templates, but for modules it's more convenient to have the module load its templates, then in its own time do the substitutions it needs and then call send_email. I guess another solution could have been to split out push.Mailer.send_mail into two functions (still in the Mailer), and make the Mailer's send_mail function be a kind of proxy that adds a bunch of push logic and then call a second function to actually send the email, but I preferred killing two birds with one stone here.

@babolivier babolivier requested a review from anoadragon453 May 13, 2021 15:39
@babolivier
Copy link
Contributor Author

Essentially I don't currently know why this PR is necessary, even after looking at the comment chain in #9884 (comment)

The issue here, which this PR aims to fix, is that currently when we want to send non-push emails in Synapse we either pollute the push code with non-push stuff, or we end up copying the same snippet around.

anoadragon453
anoadragon453 approved these changes May 17, 2021
View reviewed changes
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Thank you for the explanation, that all makes sense! This lgtm then.

@babolivier babolivier merged commit 41ac128 into develop May 17, 2021
@babolivier babolivier deleted the babolivier/send_mail branch May 17, 2021 10:33
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jun 5, 2021
@Midar
Update chat/matrix-synapses to 1.35.1
7f7c8d5 
Synapse 1.35.1 (2021-06-03)
===========================

Bugfixes
--------

- Fix a bug introduced in v1.35.0 where invite-only rooms would be shown to all users in a space, regardless of if the user had access to it. ([\#10109](matrix-org/synapse#10109))


Synapse 1.35.0 (2021-06-01)
===========================

Note that [the tag](https://github.com/matrix-org/synapse/releases/tag/v1.35.0rc3) and [docker images](https://hub.docker.com/layers/matrixdotorg/synapse/v1.35.0rc3/images/sha256-34ccc87bd99a17e2cbc0902e678b5937d16bdc1991ead097eee6096481ecf2c4?context=explore) for `v1.35.0rc3` were incorrectly built. If you are experiencing issues with either, it is recommended to upgrade to the equivalent tag or docker image for the `v1.35.0` release.

Deprecations and Removals
-------------------------

- The core Synapse development team plan to drop support for the [unstable API of MSC2858](https://github.com/matrix-org/matrix-doc/blob/master/proposals/2858-Multiple-SSO-Identity-Providers.md#unstable-prefix), including the undocumented `experimental.msc2858_enabled` config option, in August 2021. Client authors should ensure that their clients are updated to use the stable API (which has been supported since Synapse 1.30) well before that time, to give their users time to upgrade. ([\#10101](matrix-org/synapse#10101))

Bugfixes
--------

- Fixed a bug causing replication requests to fail when receiving a lot of events via federation. Introduced in v1.33.0. ([\#10082](matrix-org/synapse#10082))
- Fix HTTP response size limit to allow joining very large rooms over federation. Introduced in v1.33.0. ([\#10093](matrix-org/synapse#10093))


Internal Changes
----------------

- Log method and path when dropping request due to size limit. ([\#10091](matrix-org/synapse#10091))


Synapse 1.35.0rc2 (2021-05-27)
==============================

Bugfixes
--------

- Fix a bug introduced in v1.35.0rc1 when calling the spaces summary API via a GET request. ([\#10079](matrix-org/synapse#10079))


Synapse 1.35.0rc1 (2021-05-25)
==============================

Features
--------

- Add experimental support to allow a user who could join a restricted room to view it in the spaces summary. ([\#9922](matrix-org/synapse#9922), [\#10007](matrix-org/synapse#10007), [\#10038](matrix-org/synapse#10038))
- Reduce memory usage when joining very large rooms over federation. ([\#9958](matrix-org/synapse#9958))
- Add a configuration option which allows enabling opentracing by user id. ([\#9978](matrix-org/synapse#9978))
- Enable experimental support for [MSC2946](matrix-org/matrix-spec-proposals#2946) (spaces summary API) and [MSC3083](matrix-org/matrix-spec-proposals#3083) (restricted join rules) by default. ([\#10011](matrix-org/synapse#10011))


Bugfixes
--------

- Fix a bug introduced in v1.26.0 which meant that `synapse_port_db` would not correctly initialise some postgres sequences, requiring manual updates afterwards. ([\#9991](matrix-org/synapse#9991))
- Fix `synctl`'s `--no-daemonize` parameter to work correctly with worker processes. ([\#9995](matrix-org/synapse#9995))
- Fix a validation bug introduced in v1.34.0 in the ordering of spaces in the space summary API. ([\#10002](matrix-org/synapse#10002))
- Fixed deletion of new presence stream states from database. ([\#10014](matrix-org/synapse#10014), [\#10033](matrix-org/synapse#10033))
- Fixed a bug with very high resolution image uploads throwing internal server errors. ([\#10029](matrix-org/synapse#10029))


Updates to the Docker image
---------------------------

- Fix bug introduced in Synapse 1.33.0 which caused a `Permission denied: '/homeserver.log'` error when starting Synapse with the generated log configuration. Contributed by Sergio Miguéns Iglesias. ([\#10045](matrix-org/synapse#10045))


Improved Documentation
----------------------

- Add hardened systemd files as proposed in [#9760](matrix-org/synapse#9760) and added them to `contrib/`. Change the docs to reflect the presence of these files. ([\#9803](matrix-org/synapse#9803))
- Clarify documentation around SSO mapping providers generating unique IDs and localparts. ([\#9980](matrix-org/synapse#9980))
- Updates to the PostgreSQL documentation (`postgres.md`). ([\#9988](matrix-org/synapse#9988), [\#9989](matrix-org/synapse#9989))
- Fix broken link in user directory documentation. Contributed by @junquera. ([\#10016](matrix-org/synapse#10016))
- Add missing room state entry to the table of contents of room admin API. ([\#10043](matrix-org/synapse#10043))


Deprecations and Removals
-------------------------

- Removed support for the deprecated `tls_fingerprints` configuration setting. Contributed by Jerin J Titus. ([\#9280](matrix-org/synapse#9280))


Internal Changes
----------------

- Allow sending full presence to users via workers other than the one that called `ModuleApi.send_local_online_presence_to`. ([\#9823](matrix-org/synapse#9823))
- Update comments in the space summary handler. ([\#9974](matrix-org/synapse#9974))
- Minor enhancements to the `@cachedList` descriptor. ([\#9975](matrix-org/synapse#9975))
- Split multipart email sending into a dedicated handler. ([\#9977](matrix-org/synapse#9977))
- Run `black` on files in the `scripts` directory. ([\#9981](matrix-org/synapse#9981))
- Add missing type hints to `synapse.util` module. ([\#9982](matrix-org/synapse#9982))
- Simplify a few helper functions. ([\#9984](matrix-org/synapse#9984), [\#9985](matrix-org/synapse#9985), [\#9986](matrix-org/synapse#9986))
- Remove unnecessary property from SQLBaseStore. ([\#9987](matrix-org/synapse#9987))
- Remove `keylen` param on `LruCache`. ([\#9993](matrix-org/synapse#9993))
- Update the Grafana dashboard in `contrib/`. ([\#10001](matrix-org/synapse#10001))
- Add a batching queue implementation. ([\#10017](matrix-org/synapse#10017))
- Reduce memory usage when verifying signatures on large numbers of events at once. ([\#10018](matrix-org/synapse#10018))
- Properly invalidate caches for destination retry timings every (instead of expiring entries every 5 minutes). ([\#10036](matrix-org/synapse#10036))
- Fix running complement tests with Synapse workers. ([\#10039](matrix-org/synapse#10039))
- Fix typo in `get_state_ids_for_event` docstring where the return type was incorrect. ([\#10050](matrix-org/synapse#10050))
babolivier added a commit to matrix-org/synapse-dinsic that referenced this pull request Sep 1, 2021
@babolivier
Merge tag 'v1.35.0' into babolivier/dinsic_1.41.0
200ee12 
Synapse 1.35.0 (2021-06-01)
===========================

Note that [the tag](https://github.com/matrix-org/synapse/releases/tag/v1.35.0rc3) and [docker images](https://hub.docker.com/layers/matrixdotorg/synapse/v1.35.0rc3/images/sha256-34ccc87bd99a17e2cbc0902e678b5937d16bdc1991ead097eee6096481ecf2c4?context=explore) for `v1.35.0rc3` were incorrectly built. If you are experiencing issues with either, it is recommended to upgrade to the equivalent tag or docker image for the `v1.35.0` release.

Deprecations and Removals
-------------------------

- The core Synapse development team plan to drop support for the [unstable API of MSC2858](https://github.com/matrix-org/matrix-doc/blob/master/proposals/2858-Multiple-SSO-Identity-Providers.md#unstable-prefix), including the undocumented `experimental.msc2858_enabled` config option, in August 2021. Client authors should ensure that their clients are updated to use the stable API (which has been supported since Synapse 1.30) well before that time, to give their users time to upgrade. ([\#10101](matrix-org/synapse#10101))

Bugfixes
--------

- Fixed a bug causing replication requests to fail when receiving a lot of events via federation. Introduced in v1.33.0. ([\#10082](matrix-org/synapse#10082))
- Fix HTTP response size limit to allow joining very large rooms over federation. Introduced in v1.33.0. ([\#10093](matrix-org/synapse#10093))

Internal Changes
----------------

- Log method and path when dropping request due to size limit. ([\#10091](matrix-org/synapse#10091))

Synapse 1.35.0rc2 (2021-05-27)
==============================

Bugfixes
--------

- Fix a bug introduced in v1.35.0rc1 when calling the spaces summary API via a GET request. ([\#10079](matrix-org/synapse#10079))

Synapse 1.35.0rc1 (2021-05-25)
==============================

Features
--------

- Add experimental support to allow a user who could join a restricted room to view it in the spaces summary. ([\#9922](matrix-org/synapse#9922), [\#10007](matrix-org/synapse#10007), [\#10038](matrix-org/synapse#10038))
- Reduce memory usage when joining very large rooms over federation. ([\#9958](matrix-org/synapse#9958))
- Add a configuration option which allows enabling opentracing by user id. ([\#9978](matrix-org/synapse#9978))
- Enable experimental support for [MSC2946](matrix-org/matrix-spec-proposals#2946) (spaces summary API) and [MSC3083](matrix-org/matrix-spec-proposals#3083) (restricted join rules) by default. ([\#10011](matrix-org/synapse#10011))

Bugfixes
--------

- Fix a bug introduced in v1.26.0 which meant that `synapse_port_db` would not correctly initialise some postgres sequences, requiring manual updates afterwards. ([\#9991](matrix-org/synapse#9991))
- Fix `synctl`'s `--no-daemonize` parameter to work correctly with worker processes. ([\#9995](matrix-org/synapse#9995))
- Fix a validation bug introduced in v1.34.0 in the ordering of spaces in the space summary API. ([\#10002](matrix-org/synapse#10002))
- Fixed deletion of new presence stream states from database. ([\#10014](matrix-org/synapse#10014), [\#10033](matrix-org/synapse#10033))
- Fixed a bug with very high resolution image uploads throwing internal server errors. ([\#10029](matrix-org/synapse#10029))

Updates to the Docker image
---------------------------

- Fix bug introduced in Synapse 1.33.0 which caused a `Permission denied: '/homeserver.log'` error when starting Synapse with the generated log configuration. Contributed by Sergio Miguéns Iglesias. ([\#10045](matrix-org/synapse#10045))

Improved Documentation
----------------------

- Add hardened systemd files as proposed in [#9760](matrix-org/synapse#9760) and added them to `contrib/`. Change the docs to reflect the presence of these files. ([\#9803](matrix-org/synapse#9803))
- Clarify documentation around SSO mapping providers generating unique IDs and localparts. ([\#9980](matrix-org/synapse#9980))
- Updates to the PostgreSQL documentation (`postgres.md`). ([\#9988](matrix-org/synapse#9988), [\#9989](matrix-org/synapse#9989))
- Fix broken link in user directory documentation. Contributed by @junquera. ([\#10016](matrix-org/synapse#10016))
- Add missing room state entry to the table of contents of room admin API. ([\#10043](matrix-org/synapse#10043))

Deprecations and Removals
-------------------------

- Removed support for the deprecated `tls_fingerprints` configuration setting. Contributed by Jerin J Titus. ([\#9280](matrix-org/synapse#9280))

Internal Changes
----------------

- Allow sending full presence to users via workers other than the one that called `ModuleApi.send_local_online_presence_to`. ([\#9823](matrix-org/synapse#9823))
- Update comments in the space summary handler. ([\#9974](matrix-org/synapse#9974))
- Minor enhancements to the `@cachedList` descriptor. ([\#9975](matrix-org/synapse#9975))
- Split multipart email sending into a dedicated handler. ([\#9977](matrix-org/synapse#9977))
- Run `black` on files in the `scripts` directory. ([\#9981](matrix-org/synapse#9981))
- Add missing type hints to `synapse.util` module. ([\#9982](matrix-org/synapse#9982))
- Simplify a few helper functions. ([\#9984](matrix-org/synapse#9984), [\#9985](matrix-org/synapse#9985), [\#9986](matrix-org/synapse#9986))
- Remove unnecessary property from SQLBaseStore. ([\#9987](matrix-org/synapse#9987))
- Remove `keylen` param on `LruCache`. ([\#9993](matrix-org/synapse#9993))
- Update the Grafana dashboard in `contrib/`. ([\#10001](matrix-org/synapse#10001))
- Add a batching queue implementation. ([\#10017](matrix-org/synapse#10017))
- Reduce memory usage when verifying signatures on large numbers of events at once. ([\#10018](matrix-org/synapse#10018))
- Properly invalidate caches for destination retry timings every (instead of expiring entries every 5 minutes). ([\#10036](matrix-org/synapse#10036))
- Fix running complement tests with Synapse workers. ([\#10039](matrix-org/synapse#10039))
- Fix typo in `get_state_ids_for_event` docstring where the return type was incorrect. ([\#10050](matrix-org/synapse#10050))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anoadragon453 anoadragon453 anoadragon453 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@babolivier @anoadragon453