Introduce a mailer interface and the respective factory

[laoneo]

Summary of Changes

Introduces a new MailerInterface which does contain the most common functions which are needed to send a mail. A factory which is served through the container and the corresponding aware interface and trait are also shipped with this pr.

It also fixes an inconsistency, getting a mailer with the global configuration settings. Mail::getInstance() delivers the same object, but with a different configuration, either if Factory::getMailer() is called before or not. Here is an example:

Factory::getConfig()->set('mailfrom', '[email protected]');
echo Mail::getInstance()->From; // Prints an empty string
echo Factory::getMailer()->From; // Prints [email protected]
echo Mail::getInstance()->From; // Prints now [email protected]

With this pr all calls do return as From value the EMail address from the global configuration.

Testing Instructions

Send a test mail while editing the global Joomla configuration.

Actual result BEFORE applying this Pull Request

Test mail is sent.

Expected result AFTER applying this Pull Request

Test mail is sent.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org: Add documentation for the new mailer interface Manual#116

• No documentation changes for manual.joomla.org needed

[wilsonge]

I don't think this is the correct implementation for this.

Note that factory doesn't actually set the mailer into it's list of statics - all it's doing is populating the data from JApplication. Funnily enough as things stand if all applications only called Mail::getInstance instead of Factory::getMailer then they wouldn't have this data populated despite the doc blocks. Now all instances of the mailer will have this information injected if they are using the factory which is a big behaviour change.

This is a much simpler case than the mvc classes. I think you just need an interface on the mail class and on top of that a new method in the application that bootstraps a mail instance that can be injected. This will also make the application dependent code much easier to unit test and from the models perspective it makes no diference anyhow whether that bootstrap of the from sender etc has actually taken place or not when the mail object is being injected.

[laoneo]

More or less I was there at the beginning. Then I though that mostly you want to have the settings in the mailer instance from the global config.

Note that factory doesn't actually set the mailer into it's list of statics - all it's doing is populating the data from JApplication

This is not correct, it calls Mail::getInstance() which is cached, so it basically sets that config on the cached instance. A subsequent call on Mail::getInstance() delivers a mailer with the global settings. So actually this is a pretty inconsistent behaviour as you can get two different mailer, depending if Factory::getMailer() is called already or not. I'v updated the pr description to illustrate the current problem.

a new method in the application

Please no more functions which do make the application instance an even bigger god object.

[wilsonge]

This is not correct, it calls Mail::getInstance() which is cached, so it basically sets that config on the cached instance. A subsequent call on Mail::getInstance() delivers a mailer with the global settings. So actually this is a pretty inconsistent behaviour as you can get two different mailer, depending if Factory::getMailer() is called already or not. I'v updated the pr description to illustrate the current problem.

This was what I was trying to say - just badly!

More or less I was there at the beginning. Then I though that mostly you want to have the settings in the mailer instance from the global config.

OK but what is the value of having that central object stored in the container? Like the container is there to do dependency management not configuration of objects

Please no more functions which do make the application instance an even bigger god object

OK Then put it in JConfiguration as the only alternative - which is where all the variables come from. I don't see any benefit in the abstraction your providing...

[laoneo]

OK but what is the value of having that central object stored in the container? Like the container is there to do dependency management not configuration of objects

Only the factory is in the container, not the mailer instance.

[ChrisHoefliger]

I have tested this unsuccessfully on localhost /w mailer set to SMPT

[laoneo]

Does it work without the patch? Just want to make sure that the settings itself do work.

[ChrisHoefliger]

Does it work without the patch? Just want to make sure that the settings itself do work.

Yeah, I tried without the patch first, that worked, then with patch applied and finally with patch removed. Only with applied patch no luck. This is the latest nighly build on PHP 8.2.1.

[laoneo]

Did you try with the send test mail button?

[ChrisHoefliger]

Does it work without the patch? Just want to make sure that the settings itself do work.

It says 'Send a test mail while editing the global Joomla configuration.'
I assume that means press the send Test Mail button - that's what I did. Without the patch it says 'Test mail sent'; applying the patch, there's an error message and no mail gets sent.

[ChrisHoefliger]

I have now re-tested this successfully on a test site, using SMTP mail on PHP 8.2

[ChrisHoefliger]

I have tested this item ✅ successfully on a4df9b9

Tested successfully with SMTP

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40560.}

[toivo]

I have tested this item ✅ successfully on a4df9b9

Tested successfully in Joomla 4.4.0-dev of 28 May in Wampserver 3.3.1 64-bit on Windows 11 using SMTP and PHP 8.1.6.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40560.}

[alikon]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40560.}

[heelc29]

ParamSignatureMismatch of MailerInterface to Joomla\CMS\Mail\Mail

libraries/src/Mail/Mail.php:134 PhanParamSignatureMismatch Declaration of function Send() : bool should be compatible with function send() : void defined in libraries/src/Mail/MailerInterface.php:34
libraries/src/Mail/Mail.php:197 PhanParamSignatureMismatch Declaration of function setSender(mixed $from, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function setSender(string $fromEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:48
libraries/src/Mail/Mail.php:233 PhanParamSignatureMismatch Declaration of function setSubject(string $subject) : \Joomla\CMS\Mail\Mail should be compatible with function setSubject(string $subject) : void defined in libraries/src/Mail/MailerInterface.php:59
libraries/src/Mail/Mail.php:249 PhanParamSignatureMismatch Declaration of function setBody(string $content) : \Joomla\CMS\Mail\Mail should be compatible with function setBody(string $content) : void defined in libraries/src/Mail/MailerInterface.php:70
libraries/src/Mail/Mail.php:332 PhanParamSignatureMismatch Declaration of function addRecipient(mixed $recipient, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addRecipient(string $recipientEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:84
libraries/src/Mail/Mail.php:349 PhanParamSignatureMismatch Declaration of function addCc(mixed $cc, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addCc(string $ccEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:98
libraries/src/Mail/Mail.php:371 PhanParamSignatureMismatch Declaration of function addBcc(mixed $bcc, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addBcc(string $bccEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:112
libraries/src/Mail/Mail.php:397 PhanParamSignatureMismatch Declaration of function addAttachment(mixed $path, mixed|string $name = '', mixed|string $encoding = 'base64', mixed|string $type = 'application/octet-stream', string $disposition = 'attachment') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addAttachment(string $data, string $name = '', string $encoding = 'base64', string $type = 'application/octet-stream') : void defined in libraries/src/Mail/MailerInterface.php:126
libraries/src/Mail/Mail.php:481 PhanParamSignatureMismatch Declaration of function addReplyTo(mixed $replyto, mixed|string $name = '') : \Joomla\CMS\Mail\Mail|bool should be compatible with function addReplyTo(string $replyToEmail, string $name = '') : void defined in libraries/src/Mail/MailerInterface.php:140

[laoneo]

Then phan messages can be ignored for now as the interface should be decoupled from the actual Mail implementation. Till the interface gets wider acceptance and we can change the methods in mailer we have to leave with the mismatch. PHP itself is accepting that and this is what counts for me at the moment. The target is to have an independent mailer interface without any reference to an implementation so we can exchange it on some point when there is a need for it.