Skip to content

Make "mailer" service optional #2801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions DependencyInjection/Compiler/CheckForMailerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
* Checks to see if the mailer service exists.
*
* @author Ryan Weaver <[email protected]>
*
* @deprecated since 2.1, "mailer" service should be optional
*/
class CheckForMailerPass implements CompilerPassInterface
{
Expand Down
2 changes: 0 additions & 2 deletions FOSUserBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use Doctrine\Bundle\CouchDBBundle\DependencyInjection\Compiler\DoctrineCouchDBMappingsPass;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\DoctrineOrmMappingsPass;
use Doctrine\Bundle\MongoDBBundle\DependencyInjection\Compiler\DoctrineMongoDBMappingsPass;
use FOS\UserBundle\DependencyInjection\Compiler\CheckForMailerPass;
use FOS\UserBundle\DependencyInjection\Compiler\CheckForSessionPass;
use FOS\UserBundle\DependencyInjection\Compiler\InjectRememberMeServicesPass;
use FOS\UserBundle\DependencyInjection\Compiler\InjectUserCheckerPass;
Expand All @@ -38,7 +37,6 @@ public function build(ContainerBuilder $container)
$container->addCompilerPass(new InjectUserCheckerPass());
$container->addCompilerPass(new InjectRememberMeServicesPass());
$container->addCompilerPass(new CheckForSessionPass());
$container->addCompilerPass(new CheckForMailerPass());

$this->addRegisterMappingsPass($container);
}
Expand Down
7 changes: 7 additions & 0 deletions Mailer/Mailer.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ public function sendResettingEmailMessage(UserInterface $user)
*/
protected function sendEmailMessage($renderedTemplate, $fromEmail, $toEmail)
{
if (null === $this->mailer) {
throw new \RuntimeException(
'Sending email requires the "mailer" service to be available. '.
'Run "composer require symfony/swiftmailer-bundle" to install Swiftmailer.'
);
}

// Render the email, use the first line as the subject, and the rest as the body
$renderedLines = explode("\n", trim($renderedTemplate));
$subject = array_shift($renderedLines);
Expand Down
129 changes: 129 additions & 0 deletions Mailer/TwigMailer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
<?php

/*
* This file is part of the FOSUserBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\UserBundle\Mailer;

use FOS\UserBundle\Model\UserInterface;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

/**
* @author Christophe Coevoet <[email protected]>
*/
class TwigMailer implements MailerInterface
{
/**
* @var \Swift_Mailer
*/
protected $mailer;

/**
* @var UrlGeneratorInterface
*/
protected $router;

/**
* @var \Twig_Environment
*/
protected $twig;

/**
* @var array
*/
protected $parameters;

/**
* TwigMailer constructor.
*
* @param \Swift_Mailer $mailer
* @param UrlGeneratorInterface $router
* @param \Twig_Environment $twig
* @param array $parameters
*/
public function __construct($mailer, UrlGeneratorInterface $router, \Twig_Environment $twig, array $parameters)

Choose a reason for hiding this comment

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

$mailer Should be typed as \Swift_Mailer

Copy link
Author

Choose a reason for hiding this comment

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

No it should not =) With a type hint it would not be possible to pass null value. Old TwigSwiftMailer had that type hint. I could not remove it from there because it would be BC break. So - i created a new class TwigMailer, a parent for old TwigSwiftMailer

Choose a reason for hiding this comment

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

Ok, what's about

if(null !== $mailer && !($mailer instanceof \Swift_Mailer)
{
    // throw ....
}

?

Currently you can pass anything as $mailer... :D

Copy link
Author

@covex-nn covex-nn May 30, 2018

Choose a reason for hiding this comment

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

It turned out that there are more then one mailer! I found this out in @stof's comment last year in #2562 (comment) =) That is a reason why symfony/swiftmailer-bundle is not required dependency, btw. So, it is not necessary, i guess

Choose a reason for hiding this comment

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

I think that there is no another mailer which uses the Swift_Message class (which comes from the swiftmailer package)
https://github.com/covex-nn/FOSUserBundle/blob/b3000d0088536a1384b7057aa0888a9caa42a45e/Mailer/TwigMailer.php#L115

Copy link
Author

@covex-nn covex-nn May 30, 2018

Choose a reason for hiding this comment

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

@devtronic actualy, i agree with you, and i am on your side! May be symfony/swiftmailer-bundle should be in require section of composer.json! Create a PR and i will be very glad if it will be accepted, then i will close this one =)

But, Swift_Message comes from swiftmailer/swiftmailer and it is not a bundle. So, it is possible to create a bundle, that provides mailer service and uses Swift_Message as a first argument of send method at the same time.

{
$this->mailer = $mailer;
$this->router = $router;
$this->twig = $twig;
$this->parameters = $parameters;
}

/**
* {@inheritdoc}
*/
public function sendConfirmationEmailMessage(UserInterface $user)
{
$template = $this->parameters['template']['confirmation'];
$url = $this->router->generate('fos_user_registration_confirm', array('token' => $user->getConfirmationToken()), UrlGeneratorInterface::ABSOLUTE_URL);

$context = array(
'user' => $user,
'confirmationUrl' => $url,
);

$this->sendMessage($template, $context, $this->parameters['from_email']['confirmation'], (string) $user->getEmail());
}

/**
* {@inheritdoc}
*/
public function sendResettingEmailMessage(UserInterface $user)
{
$template = $this->parameters['template']['resetting'];
$url = $this->router->generate('fos_user_resetting_reset', array('token' => $user->getConfirmationToken()), UrlGeneratorInterface::ABSOLUTE_URL);

$context = array(
'user' => $user,
'confirmationUrl' => $url,
);

$this->sendMessage($template, $context, $this->parameters['from_email']['resetting'], (string) $user->getEmail());
}

/**
* @param string $templateName
* @param array $context
* @param array $fromEmail
* @param string $toEmail
*/
protected function sendMessage($templateName, $context, $fromEmail, $toEmail)
{
if (null === $this->mailer) {
throw new \RuntimeException(
'Sending email requires the "mailer" service to be available. '.
'Run "composer require symfony/swiftmailer-bundle" to install Swiftmailer.'
);
}

$template = $this->twig->load($templateName);
$subject = $template->renderBlock('subject', $context);
$textBody = $template->renderBlock('body_text', $context);

$htmlBody = '';

if ($template->hasBlock('body_html', $context)) {
$htmlBody = $template->renderBlock('body_html', $context);
}

$message = (new \Swift_Message())
->setSubject($subject)
->setFrom($fromEmail)
->setTo($toEmail);

if (!empty($htmlBody)) {
$message->setBody($htmlBody, 'text/html')
->addPart($textBody, 'text/plain');
} else {
$message->setBody($textBody);
}

$this->mailer->send($message);
}
}
97 changes: 7 additions & 90 deletions Mailer/TwigSwiftMailer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,15 @@

namespace FOS\UserBundle\Mailer;

use FOS\UserBundle\Model\UserInterface;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;

/**
* @author Christophe Coevoet <[email protected]>
*
* @deprecated TwigSwiftMailer class is deprecated since 2.1, use TwigMailer instead.
*/
class TwigSwiftMailer implements MailerInterface
class TwigSwiftMailer extends TwigMailer
{
/**
* @var \Swift_Mailer
*/
protected $mailer;

/**
* @var UrlGeneratorInterface
*/
protected $router;

/**
* @var \Twig_Environment
*/
protected $twig;

/**
* @var array
*/
protected $parameters;

/**
* TwigSwiftMailer constructor.
*
Expand All @@ -49,74 +30,10 @@ class TwigSwiftMailer implements MailerInterface
*/
public function __construct(\Swift_Mailer $mailer, UrlGeneratorInterface $router, \Twig_Environment $twig, array $parameters)
{
$this->mailer = $mailer;
$this->router = $router;
$this->twig = $twig;
$this->parameters = $parameters;
}

/**
* {@inheritdoc}
*/
public function sendConfirmationEmailMessage(UserInterface $user)
{
$template = $this->parameters['template']['confirmation'];
$url = $this->router->generate('fos_user_registration_confirm', array('token' => $user->getConfirmationToken()), UrlGeneratorInterface::ABSOLUTE_URL);

$context = array(
'user' => $user,
'confirmationUrl' => $url,
);

$this->sendMessage($template, $context, $this->parameters['from_email']['confirmation'], (string) $user->getEmail());
}

/**
* {@inheritdoc}
*/
public function sendResettingEmailMessage(UserInterface $user)
{
$template = $this->parameters['template']['resetting'];
$url = $this->router->generate('fos_user_resetting_reset', array('token' => $user->getConfirmationToken()), UrlGeneratorInterface::ABSOLUTE_URL);

$context = array(
'user' => $user,
'confirmationUrl' => $url,
);

$this->sendMessage($template, $context, $this->parameters['from_email']['resetting'], (string) $user->getEmail());
}

/**
* @param string $templateName
* @param array $context
* @param array $fromEmail
* @param string $toEmail
*/
protected function sendMessage($templateName, $context, $fromEmail, $toEmail)
{
$template = $this->twig->load($templateName);
$subject = $template->renderBlock('subject', $context);
$textBody = $template->renderBlock('body_text', $context);

$htmlBody = '';

if ($template->hasBlock('body_html', $context)) {
$htmlBody = $template->renderBlock('body_html', $context);
}

$message = (new \Swift_Message())
->setSubject($subject)
->setFrom($fromEmail)
->setTo($toEmail);

if (!empty($htmlBody)) {
$message->setBody($htmlBody, 'text/html')
->addPart($textBody, 'text/plain');
} else {
$message->setBody($textBody);
}
@trigger_error(sprintf(
'%s is deprecated, use %s instead.', __CLASS__, TwigMailer::class
), E_USER_DEPRECATED);

$this->mailer->send($message);
parent::__construct($mailer, $router, $twig, $parameters);
}
}
6 changes: 3 additions & 3 deletions Resources/config/mailer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

<services>
<service id="fos_user.mailer.default" class="FOS\UserBundle\Mailer\Mailer" public="false">
<argument type="service" id="mailer" />
<argument type="service" id="mailer" on-invalid="null" />
<argument type="service" id="router" />
<argument type="service" id="templating" />
<argument type="collection">
Expand All @@ -31,8 +31,8 @@
<tag name="fos_user.requires_swift" />
</service>

<service id="fos_user.mailer.twig_swift" class="FOS\UserBundle\Mailer\TwigSwiftMailer" public="false">
<argument type="service" id="mailer" />
<service id="fos_user.mailer.twig_swift" class="FOS\UserBundle\Mailer\TwigMailer" public="false">
<argument type="service" id="mailer" on-invalid="null" />
<argument type="service" id="router" />
<argument type="service" id="twig" />
<argument type="collection">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@

namespace FOS\UserBundle\Tests\Mailer;

use FOS\UserBundle\Mailer\TwigSwiftMailer;
use FOS\UserBundle\Mailer\TwigMailer;
use PHPUnit\Framework\TestCase;
use Swift_Mailer;
use Swift_Transport_NullTransport;

class TwigSwiftMailerTest extends TestCase
class TwigMailerTest extends TestCase
{
/**
* @dataProvider goodEmailProvider
*/
public function testSendConfirmationEmailMessageWithGoodEmails($emailAddress)
{
$mailer = $this->getTwigSwiftMailer();
$mailer = $this->getTwigMailer();
$mailer->sendConfirmationEmailMessage($this->getUser($emailAddress));

$this->assertTrue(true);
Expand All @@ -35,7 +35,7 @@ public function testSendConfirmationEmailMessageWithGoodEmails($emailAddress)
*/
public function testSendConfirmationEmailMessageWithBadEmails($emailAddress)
{
$mailer = $this->getTwigSwiftMailer();
$mailer = $this->getTwigMailer();
$mailer->sendConfirmationEmailMessage($this->getUser($emailAddress));
}

Expand All @@ -44,7 +44,7 @@ public function testSendConfirmationEmailMessageWithBadEmails($emailAddress)
*/
public function testSendResettingEmailMessageWithGoodEmails($emailAddress)
{
$mailer = $this->getTwigSwiftMailer();
$mailer = $this->getTwigMailer();
$mailer->sendResettingEmailMessage($this->getUser($emailAddress));

$this->assertTrue(true);
Expand All @@ -56,7 +56,7 @@ public function testSendResettingEmailMessageWithGoodEmails($emailAddress)
*/
public function testSendResettingEmailMessageWithBadEmails($emailAddress)
{
$mailer = $this->getTwigSwiftMailer();
$mailer = $this->getTwigMailer();
$mailer->sendResettingEmailMessage($this->getUser($emailAddress));
}

Expand All @@ -78,9 +78,9 @@ public function badEmailProvider()
);
}

private function getTwigSwiftMailer()
private function getTwigMailer()
{
return new TwigSwiftMailer(
return new TwigMailer(
new Swift_Mailer(
new Swift_Transport_NullTransport(
$this->getMockBuilder('Swift_Events_EventDispatcher')->getMock()
Expand Down