Skip to content

Migrate to Symfony Mailer - #30349

Merged
blizzz merged 2 commits into
masterfrom
enhancement/migrate_to_symfony_mailer
Feb 2, 2023
Merged

Migrate to Symfony Mailer#30349
blizzz merged 2 commits into
masterfrom
enhancement/migrate_to_symfony_mailer

Conversation

@come-nc

@come-nccome-nc commented Dec 20, 2021•
edited by miaulalala
Loading

Copy link
Copy Markdown
Contributor

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

Fixes#29731

TODO:

As a follow up we could get rid of any RFC validations we're doing with the egulias package as it's the same validator used by Symfony in the Address::__construct

Done
  • lib/private/Mail/Message.php: * SwiftMailer does currently not work with IDN domains, this function therefore converts the domains
  • lib/private/Mail/Message.php: * FIXME: Remove this once SwiftMailer supports IDN
  • lib/private/Mail/Mailer.php: * SwiftMailer does currently not work with IDN domains, this function therefore converts the domains
  • lib/private/Mail/Mailer.php: * FIXME: Remove this once SwiftMailer supports IDN
  • lib/private/Mail/Mailer.php: return new Attachment(new \Swift_Attachment($data, $filename, $contentType));
  • lib/private/Mail/Mailer.php: return new Attachment(\Swift_Attachment::fromPath($path, $contentType));
  • lib/private/Mail/Mailer.php: $mailLogger = new \Swift_Plugins_Loggers_ArrayLogger();
  • lib/private/Mail/Mailer.php: $mailer->registerPlugin(new \Swift_Plugins_LoggerPlugin($mailLogger));
  • lib/private/Mail/Mailer.php: $mailer->send($message->getSwiftMessage(), $failedRecipients);
  • lib/private/Mail/Message.php: * Class Message provides a wrapper around SwiftMail
  • lib/private/Mail/Message.php: $this->swiftMessage->attach($attachment->getSwiftAttachment());
  • lib/private/Mail/Message.php: * Get's the underlying SwiftMessage
  • lib/private/Mail/Message.php: public function setSwiftMessage(\Symfony\Component\Mime\Email $swiftMessage): void {
  • lib/private/Mail/Message.php: * Get's the underlying SwiftMessage
  • lib/private/Mail/Message.php: public function getSwiftMessage(): \Symfony\Component\Mime\Email {
  • lib/private/Mail/Attachment.php: /** @var \Swift_Mime_Attachment */
  • lib/private/Mail/Attachment.php: public function __construct(\Swift_Mime_Attachment $attachment) {
  • lib/private/Mail/Attachment.php: * @return \Swift_Mime_Attachment
  • lib/private/Mail/Attachment.php: public function getSwiftAttachment(): \Swift_Mime_Attachment {
Nothing to do with SwiftMailer (== Done)
  • lib/private/SystemConfig.php: // Legacy Swift (Remove objectstore credentials #17696 (comment))
  • lib/private/SystemConfig.php: // Swift v2
  • lib/private/SystemConfig.php: // Swift v3
  • lib/private/SystemConfig.php: // Swift v2
  • lib/private/SystemConfig.php: // Swift v3

@come-nc
come-ncforce-pushed the enhancement/migrate_to_symfony_mailer branch from b6f89f8 to 43836feCompareDecember 20, 2021 10:41
@come-nc

This comment was marked as duplicate.

Sign in to view
@come-nc
come-nc marked this pull request as draft December 20, 2021 10:43
@come-nc

This comment was marked as outdated.

Sign in to view
@nextcloud-command
nextcloud-commandforce-pushed the enhancement/migrate_to_symfony_mailer branch from 43836fe to bdf3360CompareSeptember 30, 2022 14:37
github-advanced-security[bot]
github-advanced-securityAI found potential problems Sep 30, 2022
View reviewed changes
Comment threadlib/private/Mail/Mailer.php Fixed
Comment threadlib/private/Mail/Mailer.php Fixed
Comment threadlib/private/Mail/Mailer.php Fixed
Comment threadlib/private/Mail/Mailer.php Fixed
Comment threadlib/private/Mail/Mailer.php Fixed
Comment threadlib/private/Mail/Message.php Fixed
Comment threadlib/private/Mail/Message.php Fixed
Comment threadlib/private/Mail/Message.php Fixed
Comment threadlib/private/Mail/Message.php Fixed
Comment threadlib/private/Mail/Mailer.php Fixed
@come-nccome-nc mentioned this pull request Sep 30, 2022
Merged
github-advanced-security[bot]
github-advanced-securityAI found potential problems Sep 30, 2022
View reviewed changes

@github-advanced-securitygithub-advanced-securityAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@github-actions

github-actionsBot commented Oct 3, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@come-nccome-nc changed the title Migrate to Symfony Mailer using rectorMigrate to Symfony MailerOct 4, 2022
@come-nc

come-nc commented Oct 4, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Known missing things:

  • setStreamOptions support. This seems not possible with symfony/mailer. Expose Mailer stream options (setStreamOptions) in configuration #12702 seems to suggest that it is mainly used to accept self-signed certificate, maybe we can look into supporting that one usecase if possible.
  • timeout setting is currently commented out. Probably possible to fix that but I did not find yet where.

@come-nc

come-nc commented Oct 5, 2022

Copy link
Copy Markdown
ContributorAuthor

Stream options and timeout support are back. Just have to use getStream on the transport and work directly on the stream.

So, tests are green, but I did not actually test that this is able to send any email. Anyone has a setup to test this? Does the CI test it?

@come-nc
come-nc marked this pull request as ready for review October 5, 2022 07:55
@github-actions

github-actionsBot commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@come-nc

This comment was marked as outdated.

Sign in to view
@github-actions

github-actionsBot commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@come-nc

This comment was marked as outdated.

Sign in to view
@come-nc

This comment was marked as outdated.

Sign in to view
@come-nc

This comment was marked as outdated.

Sign in to view
@github-actions

github-actionsBot commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

2 similar comments
@github-actions

github-actionsBot commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@github-actions

github-actionsBot commented Oct 11, 2022

Copy link
Copy Markdown
Contributor

Possible performance regression detected

Show Output
1 queries added
= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
= /remote.php/dav/files/test/many_files
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
- UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3, "size" = CASE WHEN "size" > '-1' THEN MAX("size" + :dcValue4, :dcValue5) ELSE "size" END WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "mtime" = MAX("mtime", :dcValue1), "etag" = :dcValue3 WHERE ("storage" = :dcValue2) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1'))
+ UPDATE "oc_filecache" SET "size" = MAX("size" + :dcValue1, :dcValue2) WHERE ("storage" = :dcValue3) AND ("path_hash" IN ('d41d8cd98f00b204e9800998ecf8427e', '45b963397aa40d4a0063e0d85e4fe7a1')) AND ("size" > '-1')
= /remote.php/dav/files/test/new_file.txt

@come-nccome-nc mentioned this pull request Nov 15, 2022
Closed
19 tasks
come-nc
come-nc commented Nov 21, 2022
View reviewed changes
Comment threadlib/private/Mail/Mailer.php Outdated
@come-nc

This comment was marked as off-topic.

Sign in to view
@szaimenszaimen added the 2. developing Work in progress label Nov 21, 2022
github-advanced-security[bot]
github-advanced-securityAI found potential problems Jan 26, 2023
View reviewed changes
Comment threadapps/dav/lib/CalDAV/Schedule/IMipPlugin.php Fixed
@kesselbkesselb mentioned this pull request Jan 27, 2023
Merged
9 tasks
@miaulalala
miaulalalaforce-pushed the enhancement/migrate_to_symfony_mailer branch from eae8353 to f8ee45cCompareJanuary 30, 2023 13:19
come-nc
come-nc commented Jan 30, 2023
View reviewed changes
Comment threadlib/private/Mail/Mailer.php Outdated
@come-nc

come-nc commented Jan 30, 2023

Copy link
Copy Markdown
ContributorAuthor

(I cannot approve as author of the PR, but I hereby approve what @miaulalala did 👍 )

@come-nc
come-ncforce-pushed the enhancement/migrate_to_symfony_mailer branch from db8c3f6 to f8ee45cCompareJanuary 30, 2023 15:21
@miaulalala

miaulalala commented Jan 30, 2023

Copy link
Copy Markdown
Contributor

documentation ticket here: nextcloud/documentation#9600

@miaulalala
miaulalalaforce-pushed the enhancement/migrate_to_symfony_mailer branch from f8ee45c to 90e9c9bCompareJanuary 30, 2023 18:57
@miaulalalamiaulalala added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 30, 2023
@ChristophWurst

ChristophWurst commented Jan 31, 2023

Copy link
Copy Markdown
Member

There were 10 errors:

@miaulalala
miaulalalaforce-pushed the enhancement/migrate_to_symfony_mailer branch 2 times, most recently from 7554b63 to 65709f4CompareFebruary 1, 2023 15:48
ChristophWurst
ChristophWurst reviewed Feb 1, 2023
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks sane otherwise

Comment threadapps/settings/lib/Controller/MailSettingsController.php Outdated
Comment threadconfig/config.sample.php Outdated
Comment threadlib/public/Mail/IMailer.php Outdated
Comment threadlib/public/Mail/IMessage.php Outdated
Comment threadlib/public/Mail/IMessage.php Outdated
@blizzzblizzz mentioned this pull request Feb 2, 2023
Merged
ChristophWurst
ChristophWurst requested changes Feb 2, 2023
View reviewed changes
Comment threadlib/public/Mail/IMailer.php Outdated
@blizzzblizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 2, 2023
come-ncand others added 2 commits February 2, 2023 10:30
@come-nc@blizzz
Migrate to Symfony Mailer
dde5c46
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@blizzz
remove depraction of validateEmailAddress
e123d27
for there are legit use cases to validate an email address without
sending there to
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
blizzzforce-pushed the enhancement/migrate_to_symfony_mailer branch from 2c0b560 to e123d27CompareFebruary 2, 2023 09:30
@blizzz
blizzz merged commit bbd3e2b into masterFeb 2, 2023
@blizzz
blizzz deleted the enhancement/migrate_to_symfony_mailer branch February 2, 2023 10:42
@DaphneMuller

DaphneMuller commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

hello @come-nc ,
Thank you for your work on this pull request! This ticket seems to have the tag 'missing documentation', is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@miaulalala

miaulalala commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

@DaphneMullernextcloud/documentation#9600 is the documentation ticket

@kesselbkesselb removed the pending documentation This pull request needs an associated documentation update label Jun 7, 2023
@joshtrichards

joshtrichards commented Jul 8, 2023

Copy link
Copy Markdown
Member

Indirectly completes #29333

@joshtrichardsjoshtrichards mentioned this pull request Jul 8, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miaulalalamiaulalalamiaulalala approved these changes
@ChristophWurstChristophWurstChristophWurst approved these changes
@tcitworldtcitworldAwaiting requested review from tcitworldtcitworld is a code owner

Assignees

@miaulalalamiaulalala

Labels

4. to releaseReady to be released and/or waiting for tests to finishtechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 26

Development

Successfully merging this pull request may close these issues.

Migrate from Swiftmailer to Symfony Mailer

11 participants

@come-nc@ChristophWurst@juliusknorr@miaulalala@DaphneMuller@joshtrichards@tcitworld@github-advanced-security@blizzz@kesselb@szaimen