bug #40739 [Notifier] Inject Mailer instead of service locator for Fa…

…keSms and FakeChat (OskarStark)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Notifier] Inject Mailer instead of service locator for FakeSms and FakeChat

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fixes #40731
| License       | MIT
| Doc PR        | symfony/symfony-docs#15206
| Recipe PR        | symfony/recipes#930

Until now the locator was not injected and therefore not working.

We decided to make the transport name configurable instead of the service_id.

[How is it working?](#40739 (comment))

### Todos
* [x] add tests
* [x] test in a real project

Commits
-------

0f6d507 [Notifier] Inject Mailer instead of service locator for FakeSms and FakeChat

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -2423,6 +2423,16 @@ private function registerNotifierConfiguration(array $config, ContainerBuilder $
             $container->removeDefinition($classToServices[MercureTransportFactory::class]);
         }
 
+        if (ContainerBuilder::willBeAvailable('symfony/fake-chat-notifier', FakeSmsTransportFactory::class, ['symfony/framework-bundle']) && ContainerBuilder::willBeAvailable('symfony/fake-chat-notifier', FakeSmsTransportFactory::class, ['symfony/notifier']) && ContainerBuilder::willBeAvailable('symfony/fake-chat-notifier', FakeSmsTransportFactory::class, ['symfony/mailer'])) {
+            $container->getDefinition($classToServices[FakeChatTransportFactory::class])
+                ->replaceArgument('$mailer', new Reference('mailer'));
+        }
+
+        if (ContainerBuilder::willBeAvailable('symfony/fake-sms-notifier', FakeSmsTransportFactory::class, ['symfony/framework-bundle']) && ContainerBuilder::willBeAvailable('symfony/fake-sms-notifier', FakeSmsTransportFactory::class, ['symfony/notifier']) && ContainerBuilder::willBeAvailable('symfony/fake-sms-notifier', FakeSmsTransportFactory::class, ['symfony/mailer'])) {
+            $container->getDefinition($classToServices[FakeSmsTransportFactory::class])
+                ->replaceArgument('$mailer', new Reference('mailer'));
+        }
+
         if (isset($config['admin_recipients'])) {
             $notifier = $container->getDefinition('notifier');
             foreach ($config['admin_recipients'] as $i => $recipient) {

src/Symfony/Component/Notifier/Bridge/FakeChat/.gitignore

@@ -0,0 +1,3 @@
+vendor/
+composer.lock
+phpunit.xml

src/Symfony/Component/Notifier/Bridge/FakeChat/FakeChatEmailTransport.php

@@ -27,6 +27,8 @@
  */
 final class FakeChatEmailTransport extends AbstractTransport
 {
+    protected const HOST = 'default';
+
     private $mailer;
     private $to;
     private $from;
@@ -61,13 +63,22 @@ protected function doSend(MessageInterface $message): SentMessage
             throw new UnsupportedMessageTypeException(__CLASS__, ChatMessage::class, $message);
         }
 
+        $subject = 'New Chat message without specified recipient!';
+        if (null !== $message->getRecipientId()) {
+            $subject = sprintf('New Chat message for recipient: %s', $message->getRecipientId());
+        }
+
         $email = (new Email())
             ->from($this->from)
             ->to($this->to)
-            ->subject(sprintf('New Chat message for recipient: %s', $message->getRecipientId()))
+            ->subject($subject)
             ->html($message->getSubject())
             ->text($message->getSubject());
 
+        if ('default' !== $transportName = $this->getEndpoint()) {
+            $email->getHeaders()->addTextHeader('X-Transport', $transportName);
+        }
+
         $this->mailer->send($email);
 
         return new SentMessage($message, (string) $this);

src/Symfony/Component/Notifier/Bridge/FakeChat/FakeChatTransportFactory.php

@@ -11,24 +11,24 @@
 
 namespace Symfony\Component\Notifier\Bridge\FakeChat;
 
+use Symfony\Component\Mailer\MailerInterface;
 use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
 use Symfony\Component\Notifier\Transport\AbstractTransportFactory;
 use Symfony\Component\Notifier\Transport\Dsn;
 use Symfony\Component\Notifier\Transport\TransportInterface;
-use Symfony\Contracts\Service\ServiceProviderInterface;
 
 /**
  * @author Oskar Stark <oskarstark@googlemail.com>
  */
 final class FakeChatTransportFactory extends AbstractTransportFactory
 {
-    protected $serviceProvider;
+    protected $mailer;
 
-    public function __construct(ServiceProviderInterface $serviceProvider)
+    public function __construct(MailerInterface $mailer)
     {
         parent::__construct();
 
-        $this->serviceProvider = $serviceProvider;
+        $this->mailer = $mailer;
     }
 
     /**
@@ -43,11 +43,11 @@ public function create(Dsn $dsn): TransportInterface
         }
 
         if ('fakechat+email' === $scheme) {
-            $serviceId = $dsn->getHost();
+            $mailerTransport = $dsn->getHost();
             $to = $dsn->getRequiredOption('to');
             $from = $dsn->getRequiredOption('from');
 
-            return (new FakeChatEmailTransport($this->serviceProvider->get($serviceId), $to, $from))->setHost($serviceId);
+            return (new FakeChatEmailTransport($this->mailer, $to, $from))->setHost($mailerTransport);
         }
     }
 

src/Symfony/Component/Notifier/Bridge/FakeChat/README.md

@@ -6,14 +6,18 @@ Provides Fake Chat (as email during development) integration for Symfony Notifie
 #### DSN example
 
 ```
-FAKE_CHAT_DSN=fakechat+email://MAILER_SERVICE_ID?to=TO&from=FROM
+FAKE_CHAT_DSN=fakechat+email://default?to=TO&from=FROM
 ```
 
 where:
- - `MAILER_SERVICE_ID` is mailer service id (use `mailer` by default)
  - `TO` is email who receive Chat message during development
  - `FROM` is email who send Chat message during development
 
+To use a custom mailer transport:
+```
+FAKE_CHAT_DSN=fakechat+email://mailchimp?to=TO&from=FROM
+```
+
 Resources
 ---------
 

src/Symfony/Component/Notifier/Bridge/FakeChat/Tests/FakeChatEmailTransportTest.php

@@ -12,27 +12,34 @@
 namespace Symfony\Component\Notifier\Bridge\FakeChat\Tests;
 
 use Symfony\Component\Mailer\MailerInterface;
+use Symfony\Component\Mime\Email;
 use Symfony\Component\Notifier\Bridge\FakeChat\FakeChatEmailTransport;
 use Symfony\Component\Notifier\Message\ChatMessage;
 use Symfony\Component\Notifier\Message\MessageInterface;
 use Symfony\Component\Notifier\Message\SmsMessage;
 use Symfony\Component\Notifier\Test\TransportTestCase;
+use Symfony\Component\Notifier\Tests\Fixtures\TestOptions;
+use Symfony\Component\Notifier\Tests\Mailer\DummyMailer;
 use Symfony\Component\Notifier\Transport\TransportInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
-/**
- * @author Oskar Stark <oskarstark@googlemail.com>
- */
 final class FakeChatEmailTransportTest extends TransportTestCase
 {
-    public function createTransport(?HttpClientInterface $client = null): TransportInterface
+    public function createTransport(?HttpClientInterface $client = null, ?string $transportName = null): TransportInterface
     {
-        return (new FakeChatEmailTransport($this->createMock(MailerInterface::class), 'recipient@email.net', 'from@email.net'))->setHost('mailer');
+        $transport = (new FakeChatEmailTransport($this->createMock(MailerInterface::class), 'recipient@email.net', 'sender@email.net', $client ?? $this->createMock(HttpClientInterface::class)));
+
+        if (null !== $transportName) {
+            $transport->setHost($transportName);
+        }
+
+        return $transport;
     }
 
     public function toStringProvider(): iterable
     {
-        yield ['fakechat+email://mailer?to=recipient@email.net&from=from@email.net', $this->createTransport()];
+        yield ['fakechat+email://default?to=recipient@email.net&from=sender@email.net', $this->createTransport()];
+        yield ['fakechat+email://mailchimp?to=recipient@email.net&from=sender@email.net', $this->createTransport(null, 'mailchimp')];
     }
 
     public function supportedMessagesProvider(): iterable
@@ -45,4 +52,98 @@ public function unsupportedMessagesProvider(): iterable
         yield [new SmsMessage('0611223344', 'Hello!')];
         yield [$this->createMock(MessageInterface::class)];
     }
+
+    public function testSendWithDefaultTransportAndWithRecipient()
+    {
+        $transportName = null;
+
+        $message = new ChatMessage($subject = 'Hello!', new TestOptions(['recipient_id' => $recipient = 'Oskar']));
+
+        $mailer = new DummyMailer();
+
+        $transport = (new FakeChatEmailTransport($mailer, $to = 'recipient@email.net', $from = 'sender@email.net'));
+        $transport->setHost($transportName);
+
+        $transport->send($message);
+
+        /** @var Email $sentEmail */
+        $sentEmail = $mailer->getSentEmail();
+        $this->assertInstanceOf(Email::class, $sentEmail);
+        $this->assertSame($to, $sentEmail->getTo()[0]->getEncodedAddress());
+        $this->assertSame($from, $sentEmail->getFrom()[0]->getEncodedAddress());
+        $this->assertSame(sprintf('New Chat message for recipient: %s', $recipient), $sentEmail->getSubject());
+        $this->assertSame($subject, $sentEmail->getTextBody());
+        $this->assertFalse($sentEmail->getHeaders()->has('X-Transport'));
+    }
+
+    public function testSendWithDefaultTransportAndWithoutRecipient()
+    {
+        $transportName = null;
+
+        $message = new ChatMessage($subject = 'Hello!');
+
+        $mailer = new DummyMailer();
+
+        $transport = (new FakeChatEmailTransport($mailer, $to = 'recipient@email.net', $from = 'sender@email.net'));
+        $transport->setHost($transportName);
+
+        $transport->send($message);
+
+        /** @var Email $sentEmail */
+        $sentEmail = $mailer->getSentEmail();
+        $this->assertInstanceOf(Email::class, $sentEmail);
+        $this->assertSame($to, $sentEmail->getTo()[0]->getEncodedAddress());
+        $this->assertSame($from, $sentEmail->getFrom()[0]->getEncodedAddress());
+        $this->assertSame('New Chat message without specified recipient!', $sentEmail->getSubject());
+        $this->assertSame($subject, $sentEmail->getTextBody());
+        $this->assertFalse($sentEmail->getHeaders()->has('X-Transport'));
+    }
+
+    public function testSendWithCustomTransportAndWithRecipient()
+    {
+        $transportName = 'mailchimp';
+
+        $message = new ChatMessage($subject = 'Hello!', new TestOptions(['recipient_id' => $recipient = 'Oskar']));
+
+        $mailer = new DummyMailer();
+
+        $transport = (new FakeChatEmailTransport($mailer, $to = 'recipient@email.net', $from = 'sender@email.net'));
+        $transport->setHost($transportName);
+
+        $transport->send($message);
+
+        /** @var Email $sentEmail */
+        $sentEmail = $mailer->getSentEmail();
+        $this->assertInstanceOf(Email::class, $sentEmail);
+        $this->assertSame($to, $sentEmail->getTo()[0]->getEncodedAddress());
+        $this->assertSame($from, $sentEmail->getFrom()[0]->getEncodedAddress());
+        $this->assertSame(sprintf('New Chat message for recipient: %s', $recipient), $sentEmail->getSubject());
+        $this->assertSame($subject, $sentEmail->getTextBody());
+        $this->assertTrue($sentEmail->getHeaders()->has('X-Transport'));
+        $this->assertSame($transportName, $sentEmail->getHeaders()->get('X-Transport')->getBodyAsString());
+    }
+
+    public function testSendWithCustomTransportAndWithoutRecipient()
+    {
+        $transportName = 'mailchimp';
+
+        $message = new ChatMessage($subject = 'Hello!');
+
+        $mailer = new DummyMailer();
+
+        $transport = (new FakeChatEmailTransport($mailer, $to = 'recipient@email.net', $from = 'sender@email.net'));
+        $transport->setHost($transportName);
+
+        $transport->send($message);
+
+        /** @var Email $sentEmail */
+        $sentEmail = $mailer->getSentEmail();
+        $this->assertInstanceOf(Email::class, $sentEmail);
+        $this->assertSame($to, $sentEmail->getTo()[0]->getEncodedAddress());
+        $this->assertSame($from, $sentEmail->getFrom()[0]->getEncodedAddress());
+        $this->assertSame('New Chat message without specified recipient!', $sentEmail->getSubject());
+        $this->assertSame($subject, $sentEmail->getTextBody());
+        $this->assertTrue($sentEmail->getHeaders()->has('X-Transport'));
+        $this->assertSame($transportName, $sentEmail->getHeaders()->get('X-Transport')->getBodyAsString());
+    }
 }

src/Symfony/Component/Notifier/Bridge/FakeChat/Tests/FakeChatTransportFactoryTest.php

@@ -15,53 +15,50 @@
 use Symfony\Component\Notifier\Bridge\FakeChat\FakeChatTransportFactory;
 use Symfony\Component\Notifier\Test\TransportFactoryTestCase;
 use Symfony\Component\Notifier\Transport\TransportFactoryInterface;
-use Symfony\Contracts\Service\ServiceProviderInterface;
 
-/**
- * @author Oskar Stark <oskarstark@googlemail.com>
- */
 final class FakeChatTransportFactoryTest extends TransportFactoryTestCase
 {
     /**
      * @return FakeChatTransportFactory
      */
     public function createFactory(): TransportFactoryInterface
     {
-        $serviceProvider = $this->createMock(ServiceProviderInterface::class);
-        $serviceProvider->method('has')->willReturn(true);
-        $serviceProvider->method('get')->willReturn($this->createMock(MailerInterface::class));
-
-        return new FakeChatTransportFactory($serviceProvider);
+        return new FakeChatTransportFactory($this->createMock(MailerInterface::class));
     }
 
     public function createProvider(): iterable
     {
         yield [
-            'fakechat+email://mailer?to=recipient@email.net&from=sender@email.net',
-            'fakechat+email://mailer?to=recipient@email.net&from=sender@email.net',
+            'fakechat+email://default?to=recipient@email.net&from=sender@email.net',
+            'fakechat+email://default?to=recipient@email.net&from=sender@email.net',
+        ];
+
+        yield [
+            'fakechat+email://mailchimp?to=recipient@email.net&from=sender@email.net',
+            'fakechat+email://mailchimp?to=recipient@email.net&from=sender@email.net',
         ];
     }
 
     public function missingRequiredOptionProvider(): iterable
     {
-        yield 'missing option: from' => ['fakechat+email://mailer?to=recipient@email.net'];
-        yield 'missing option: to' => ['fakechat+email://mailer?from=sender@email.net'];
+        yield 'missing option: from' => ['fakechat+email://default?to=recipient@email.net'];
+        yield 'missing option: to' => ['fakechat+email://default?from=sender@email.net'];
     }
 
     public function supportsProvider(): iterable
     {
-        yield [true, 'fakechat+email://mailer?to=recipient@email.net&from=sender@email.net'];
-        yield [false, 'somethingElse://mailer?to=recipient@email.net&from=sender@email.net'];
+        yield [true, 'fakechat+email://default?to=recipient@email.net&from=sender@email.net'];
+        yield [false, 'somethingElse://default?to=recipient@email.net&from=sender@email.net'];
     }
 
     public function incompleteDsnProvider(): iterable
     {
-        yield 'missing from' => ['fakechat+email://mailer?to=recipient@email.net'];
-        yield 'missing to' => ['fakechat+email://mailer?from=recipient@email.net'];
+        yield 'missing from' => ['fakechat+email://default?to=recipient@email.net'];
+        yield 'missing to' => ['fakechat+email://default?from=recipient@email.net'];
     }
 
     public function unsupportedSchemeProvider(): iterable
     {
-        yield ['somethingElse://mailer?to=recipient@email.net&from=sender@email.net'];
+        yield ['somethingElse://default?to=recipient@email.net&from=sender@email.net'];
     }
 }

src/Symfony/Component/Notifier/Bridge/FakeSms/FakeSmsEmailTransport.php

@@ -24,9 +24,12 @@
 
 /**
  * @author James Hemery <james@yieldstudio.fr>
+ * @author Oskar Stark <oskarstark@googlemail.com>
  */
 final class FakeSmsEmailTransport extends AbstractTransport
 {
+    protected const HOST = 'default';
+
     private $mailer;
     private $to;
     private $from;
@@ -68,6 +71,10 @@ protected function doSend(MessageInterface $message): SentMessage
             ->html($message->getSubject())
             ->text($message->getSubject());
 
+        if ('default' !== $transportName = $this->getEndpoint()) {
+            $email->getHeaders()->addTextHeader('X-Transport', $transportName);
+        }
+
         $this->mailer->send($email);
 
         return new SentMessage($message, (string) $this);

src/Symfony/Component/Notifier/Bridge/FakeSms/FakeSmsTransportFactory.php

@@ -11,24 +11,25 @@
 
 namespace Symfony\Component\Notifier\Bridge\FakeSms;
 
+use Symfony\Component\Mailer\MailerInterface;
 use Symfony\Component\Notifier\Exception\UnsupportedSchemeException;
 use Symfony\Component\Notifier\Transport\AbstractTransportFactory;
 use Symfony\Component\Notifier\Transport\Dsn;
 use Symfony\Component\Notifier\Transport\TransportInterface;
-use Symfony\Contracts\Service\ServiceProviderInterface;
 
 /**
  * @author James Hemery <james@yieldstudio.fr>
+ * @author Oskar Stark <oskarstark@googlemail.com>
  */
 final class FakeSmsTransportFactory extends AbstractTransportFactory
 {
-    protected $serviceProvider;
+    protected $mailer;
 
-    public function __construct(ServiceProviderInterface $serviceProvider)
+    public function __construct(MailerInterface $mailer)
     {
         parent::__construct();
 
-        $this->serviceProvider = $serviceProvider;
+        $this->mailer = $mailer;
     }
 
     /**
@@ -43,11 +44,11 @@ public function create(Dsn $dsn): TransportInterface
         }
 
         if ('fakesms+email' === $scheme) {
-            $serviceId = $dsn->getHost();
+            $mailerTransport = $dsn->getHost();
             $to = $dsn->getRequiredOption('to');
             $from = $dsn->getRequiredOption('from');
 
-            return (new FakeSmsEmailTransport($this->serviceProvider->get($serviceId), $to, $from))->setHost($serviceId);
+            return (new FakeSmsEmailTransport($this->mailer, $to, $from))->setHost($mailerTransport);
         }
     }