refactor Email::generateBody()

kbond · kbond · commit ad833aaaedf4 · 2021-12-21T19:57:10.000-05:00
diff --git a/src/Symfony/Component/Mailer/Mailer.php b/src/Symfony/Component/Mailer/Mailer.php
@@ -13,11 +13,9 @@
 
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Mailer\Event\MessageEvent;
-use Symfony\Component\Mailer\Exception\LogicException;
 use Symfony\Component\Mailer\Messenger\SendEmailMessage;
 use Symfony\Component\Mailer\Transport\TransportInterface;
 use Symfony\Component\Messenger\MessageBusInterface;
-use Symfony\Component\Mime\Message;
 use Symfony\Component\Mime\RawMessage;
 
 /**
@@ -38,10 +36,6 @@ public function __construct(TransportInterface $transport, MessageBusInterface $
 
     public function send(RawMessage $message, Envelope $envelope = null): void
     {
-        if ($message instanceof Message && '1' === $message->getHeaders()->getHeaderBody('X-Unsent')) {
-            throw new LogicException('Cannot send messages marked as "draft".');
-        }
-
         if (null === $this->bus) {
             $this->transport->send($message, $envelope);
 
diff --git a/src/Symfony/Component/Mailer/Tests/MailerTest.php b/src/Symfony/Component/Mailer/Tests/MailerTest.php
@@ -69,15 +69,4 @@ public function dispatch($message, array $stamps = []): Envelope
         self::assertCount(1, $bus->messages);
         self::assertSame($email, $bus->messages[0]->getMessage());
     }
-
-    public function testCannotSendDraftEmails()
-    {
-        $transport = new Mailer($this->createMock(TransportInterface::class), $this->createMock(MessageBusInterface::class), $this->createMock(EventDispatcherInterface::class));
-        $email = new Email();
-        $email->getHeaders()->addTextHeader('X-Unsent', '1');
-
-        $this->expectException(LogicException::class);
-
-        $transport->send($email);
-    }
 }
diff --git a/src/Symfony/Component/Mime/Email.php b/src/Symfony/Component/Mime/Email.php
@@ -386,17 +386,22 @@ public function getBody(): AbstractPart
 
     public function ensureValidity()
     {
-        if (null === $this->text && null === $this->html && !$this->attachments) {
-            throw new LogicException('A message must have a text or an HTML part or attachments.');
-        }
+        $this->ensureBodyValid();
 
         if ('1' === $this->getHeaders()->getHeaderBody('X-Unsent')) {
-            return;
+            throw new LogicException('Cannot send messages marked as "draft".');
         }
 
         parent::ensureValidity();
     }
 
+    private function ensureBodyValid(): void
+    {
+        if (null === $this->text && null === $this->html && !$this->attachments) {
+            throw new LogicException('A message must have a text or an HTML part or attachments.');
+        }
+    }
+
     /**
      * Generates an AbstractPart based on the raw body of a message.
      *
@@ -419,7 +424,7 @@ public function ensureValidity()
      */
     private function generateBody(): AbstractPart
     {
-        $this->ensureValidity();
+        $this->ensureBodyValid();
 
         [$htmlPart, $attachmentParts, $inlineParts] = $this->prepareParts();
 
diff --git a/src/Symfony/Component/Mime/Tests/DraftEmailTest.php b/src/Symfony/Component/Mime/Tests/DraftEmailTest.php
@@ -42,4 +42,17 @@ public function testMustHaveBody()
 
         (new DraftEmail())->toString();
     }
+
+    public function testEnsureValidityAlwaysFails()
+    {
+        $email = (new DraftEmail())
+            ->to('alice@example.com')
+            ->from('webmaster@example.com')
+            ->text('some text')
+        ;
+
+        $this->expectException(LogicException::class);
+
+        $email->ensureValidity();
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -386,17 +386,22 @@ public function getBody(): AbstractPart`
`386`	`386`
`387`	`387`	`public function ensureValidity()`
`388`	`388`	`{`
`389`		`- if (null === $this->text && null === $this->html && !$this->attachments) {`
`390`		`- throw new LogicException('A message must have a text or an HTML part or attachments.');`
`391`		`- }`
	`389`	`+ $this->ensureBodyValid();`
`392`	`390`
`393`	`391`	`if ('1' === $this->getHeaders()->getHeaderBody('X-Unsent')) {`
`394`		`- return;`
	`392`	`+ throw new LogicException('Cannot send messages marked as "draft".');`
`395`	`393`	`}`
`396`	`394`
`397`	`395`	`parent::ensureValidity();`
`398`	`396`	`}`
`399`	`397`
	`398`	`+ private function ensureBodyValid(): void`
	`399`	`+ {`
	`400`	`+ if (null === $this->text && null === $this->html && !$this->attachments) {`
	`401`	`+ throw new LogicException('A message must have a text or an HTML part or attachments.');`
	`402`	`+ }`
	`403`	`+ }`
	`404`	`+`
`400`	`405`	`/**`
`401`	`406`	`* Generates an AbstractPart based on the raw body of a message.`
`402`	`407`	`*`
`@@ -419,7 +424,7 @@ public function ensureValidity()`
`419`	`424`	`*/`
`420`	`425`	`private function generateBody(): AbstractPart`
`421`	`426`	`{`
`422`		`- $this->ensureValidity();`
	`427`	`+ $this->ensureBodyValid();`
`423`	`428`
`424`	`429`	`[$htmlPart, $attachmentParts, $inlineParts] = $this->prepareParts();`
`425`	`430`