8000 feature #47462 [Mime] Simplify adding Parts to an Email (fabpot) · symfony/symfony@527e30f · GitHub
[go: up one dir, main page]
Skip to content

Commit 527e30f

Browse files
fabpotfabpot
committed
feature #47462 [Mime] Simplify adding Parts to an Email (fabpot)
This PR was merged into the 6.2 branch. Discussion ---------- [Mime] Simplify adding Parts to an Email | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | n/a <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | While fixing some MIME bugs, I realized we have a substantial cyclomatic complexity due to the attach/attachFromPath/embed/embedFromPath/attachPart methods on the Email class. This PR simplifies all of that and introduces a way to have a file for TextPart as well (via the new `file://` notation) and it keeps the lazy-loading feature which was why those methods were introduced in the first place. From now, I've kept all the methods, but I'm wondering if we should deprecate all of them and only keep `attachPart()` (which I would like to rename `addPart()`). Commits ------- 0a29d97 [Mime] Simplify adding Parts to an Email
2 parents 59da7ea + 0a29d97 commit 527e30f

File tree

14 files changed

+122
-137
lines changed

14 files changed

+122
-137
lines changed

‎src/Symfony/Bridge/Twig/Tests/Mime/TemplatedEmailTest.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,16 @@ public function testSymfonySerialize()
7474
"htmlCharset": null,
7575
"attachments": [
7676
{
77+
"filename": "test.txt",
78+
"mediaType": "application",
7779
"body": "Some Text file",
80+
"charset": null,
81+
"subtype": "octet-stream",
82+
"disposition": "attachment",
7883
"name": "test.txt",
79-
"content-type": null,
80-
"inline": false
84+
"encoding": "base64",
85+
"headers": [],
86+
"class": "Symfony\\\Component\\\Mime\\\Part\\\DataPart"
8187
}
8288
],
8389
"headers": {

‎src/Symfony/Bridge/Twig/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
"symfony/security-core": "^5.4|^6.0",
4444
"symfony/security-csrf": "^5.4|^6.0",
4545
"symfony/security-http": "^5.4|^6.0",
46-
"symfony/serializer": "^5.4|^6.0",
46+
"symfony/serializer": "^6.2",
4747
"symfony/stopwatch": "^5.4|^6.0",
4848
"symfony/console": "^5.4|^6.0",
4949
"symfony/expression-language": "^5.4|^6.0",

‎src/Symfony/Component/Mailer/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"psr/event-dispatcher": "^1",
2222
"psr/log": "^1|^2|^3",
2323
"symfony/event-dispatcher": "^5.4|^6.0",
24-
"symfony/mime": "^5.4|^6.0",
24+
"symfony/mime": "^6.2",
2525
"symfony/service-contracts": "^1.1|^2|^3"
2626
},
2727
"require-dev": {

‎src/Symfony/Component/Mime/Email.php

Lines changed: 15 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Mime\Exception\LogicException;
1515
use Symfony\Component\Mime\Part\AbstractPart;
16+
use Symfony\Component\Mime\Part\BodyFile;
1617
use Symfony\Component\Mime\Part\DataPart;
1718
use Symfony\Component\Mime\Part\Multipart\AlternativePart;
1819
use Symfony\Component\Mime\Part\Multipart\MixedPart;
@@ -326,25 +327,15 @@ public function getHtmlCharset(): ?string
326327
*/
327328
public function attach($body, string $name = null, string $contentType = null): static
328329
{
329-
if (!\is_string($body) && !\is_resource($body)) {
330-
throw new \TypeError(sprintf('The body must be a string or a resource (got "%s").', get_debug_type($body)));
331-
}
332-
333-
$this->cachedBody = null;
334-
$this->attachments[] = ['body' => $body, 'name' => $name, 'content-type' => $contentType, 'inline' => false];
335-
336-
return $this;
330+
return $this->attachPart(new DataPart($body, $name, $contentType));
337331
}
338332

339333
/**
340334
* @return $this
341335
*/
342336
public function attachFromPath(string $path, string $name = null, string $contentType = null): static
343337
{
344-
$this->cachedBody = null;
345-
$this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => false];
346-
347-
return $this;
338+
return $this->attachPart(new DataPart(new BodyFile($path), $name, $contentType));
348339
}
349340

350341
/**
@@ -354,25 +345,15 @@ public function attachFromPath(string $path, string $name = null, string $conten
354345
*/
355346
public function embed($body, string $name = null, string $contentType = null): static
356347
{
357-
if (!\is_string($body) && !\is_resource($body)) {
358-
throw new \TypeError(sprintf('The body must be a string or a resource (got "%s").', get_debug_type($body)));
359-
}
360-
361-
$this->cachedBody = null;
362-
$this->attachments[] = ['body' => $body, 'name' => $name, 'content-type' => $contentType, 'inline' => true];
363-
364-
return $this;
348+
return $this->attachPart((new DataPart($body, $name, $contentType))->asInline());
365349
}
366350

367351
/**
368352
* @return $this
369353
*/
370354
public function embedFromPath(string $path, string $name = null, string $contentType = null): static
371355
{
372-
$this->cachedBody = null;
373-
$this->attachments[] = ['path' => $path, 'name' => $name, 'content-type' => $contentType, 'inline' => true];
374-
375-
return $this;
356+
return $this->attachPart((new DataPart(new BodyFile($path), $name, $contentType))->asInline());
376357
}
377358

378359
/**
@@ -381,22 +362,17 @@ public function embedFromPath(string $path, string $name = null, string $content
381362
public function attachPart(DataPart $part): static
382363
{
383364
$this->cachedBody = null;
384-
$this->attachments[] = ['part' => $part];
365+
$this->attachments[] = $part;
385366

386367
return $this;
387368
}
388369

389370
/**
390-
* @return array|DataPart[]
371+
* @return DataPart[]
391372
*/
392373
public function getAttachments(): array
393374
{
394-
$parts = [];
395-
foreach ($this->attachments as $attachment) {
396-
$parts[] = $this->createDataPart($attachment);
397-
}
398-
399-
return $parts;
375+
return $this->attachments;
400376
}
401377

402378
public function getBody(): AbstractPart
@@ -502,35 +478,23 @@ private function prepareParts(): ?array
502478
}
503479

504480
$otherParts = $relatedParts = [];
505-
foreach ($this->attachments as $attachment) {
506-
$part = $this->createDataPart($attachment);
507-
if (isset($attachment['part'])) {
508-
$attachment['name'] = $part->getName();
509-
}
510-
511-
$related = false;
481+
foreach ($this->attachments as $part) {
512482
foreach ($names as $name) {
513-
if ($name !== $attachment['name']) {
483+
if ($name !== $part->getName()) {
514484
continue;
515485
}
516486
if (isset($relatedParts[$name])) {
517487
continue 2;
518488
}
519-
$part->setDisposition('inline');
489+
520490
$html = str_replace('cid:'.$name, 'cid:'.$part->getContentId(), $html, $count);
521-
if ($count) {
522-
$related = true;
523-
}
524-
$part->setName($part->getContentId());
491+
$relatedParts[$name] = $part;
492+
$part->setName($part->getContentId())->asInline();
525493

526-
break;
494+
continue 2;
527495
}
528496

529-
if ($related) {
530-
$relatedParts[$attachment['name']] = $part;
531-
} else {
532-
$otherParts[] = $part;
533-
}
497+
$otherParts[] = $part;
534498
}
535499
if (null !== $htmlPart) {
536500
$htmlPart = new TextPart($html, $this->htmlCharset, 'html');
@@ -539,24 +503,6 @@ private function prepareParts(): ?array
539503
return [$htmlPart, $otherParts, array_values($relatedParts)];
540504
}
541505

542-
private function createDataPart(array $attachment): DataPart
543-
{
544-
if (isset($attachment['part'])) {
545-
return $attachment['part'];
546-
}
547-
548-
if (isset($attachment['body'])) {
549-
$part = new DataPart($attachment['body'], $attachment['name'] ?? null, $attachment['content-type'] ?? null);
550-
} else {
551-
$part = DataPart::fromPath($attachment['path'] ?? '', $attachment['name'] ?? null, $attachment['content-type'] ?? null);
552-
}
553-
if ($attachment['inline']) {
554-
$part->asInline();
555-
}
556-
557-
return $part;
558-
}
559-
560506
/**
561507
* @return $this
562508
*/
@@ -606,12 +552,6 @@ public function __serialize(): array
606552
$this->html = (new TextPart($this->html))->getBody();
607553
}
608554

609-
foreach ($this->attachments as $i => $attachment) {
610-
if (isset($attachment['body']) && \is_resource($attachment['body'])) {
611-
$this->attachments[$i]['body'] = (new TextPart($attachment['body']))->getBody();
612-
}
613-
}
614-
615555
return [$this->text, $this->textCharset, $this->html, $this->htmlCharset, $this->attachments, parent::__serialize()];
616556
}
617557

‎src/Symfony/Component/Mime/MessageConverter.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,7 @@ private static function attachParts(Email $email, array $parts): Email
114114
throw new RuntimeException(sprintf('Unable to create an Email from an instance of "%s" as the body is too complex.', get_debug_type($email)));
115115
}
116116

117-
$headers = $part->getPreparedHeaders();
118-
$method = 'inline' === $headers->getHeaderBody('Content-Disposition') ? 'embed' : 'attach';
119-
$name = $headers->getHeaderParameter('Content-Disposition', 'filename');
120-
$email->$method($part->getBody(), $name, $part->getMediaType().'/'.$part->getMediaSubtype());
117+
$email->attachPart($part);
121118
}
122119

123120
return $email;

‎src/Symfony/Component/Mime/Part/BodyFile.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Mime\Part;
13+
14+
use Symfony\Component\Mime\MimeTypes;
15+
16+
/**
17+
* @author Fabien Potencier <fabien@symfony.com>
18+
*/
19+
class BodyFile
20+
{
21+
private static $mimeTypes;
22+
23+
public function __construct(
24+
private string $path,
25+
) {
26+
}
27+
28+
public function getPath(): string
29+
{
30+
return $this->path;
31+
}
32+
33+
public function getContentType(): string
34+
{
35+
$ext = strtolower(pathinfo($this->path, \PATHINFO_EXTENSION));
36+
if (null === self::$mimeTypes) {
37+
self::$mimeTypes = new MimeTypes();
38+
}
39+
40+
return self::$mimeTypes->getMimeTypes($ext)[0] ?? 'application/octet-stream';
41+
}
42+
}

‎src/Symfony/Component/Mime/Part/DataPart.php

Lines changed: 7 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Symfony\Component\Mime\Exception\InvalidArgumentException;
1515
use Symfony\Component\Mime\Header\Headers;
16-
use Symfony\Component\Mime\MimeTypes;
1716

1817
/**
1918
* @author Fabien Potencier <fabien@symfony.com>
@@ -23,22 +22,23 @@ class DataPart extends TextPart
2322
/** @internal */
2423
protected $_parent;
2524

26-
private static $mimeTypes;
27-
2825
private $filename;
2926
private $mediaType;
3027
private $cid;
31-
private $handle;
3228

3329
/**
34-
* @param resource|string $body
30+
* @param resource|string|BodyFile $body Use a BodyFile instance to defer loading the file until rendering
3531
*/
3632
public function __construct($body, string $filename = null, string $contentType = null, string $encoding = null)
3733
{
3834
unset($this->_parent);
3935

36+
if ($body instanceof BodyFile && !$filename) {
37+
$filename = basename($body->getPath());
38+
}
39+
4040
if (null === $contentType) {
41-
$contentType = 'application/octet-stream';
41+
$contentType = $body instanceof BodyFile ? $body->getContentType() : 'application/octet-stream';
4242
}
4343
[$this->mediaType, $subtype] = explode('/', $contentType);
4444

@@ -53,32 +53,7 @@ public function __construct($body, string $filename = null, string $contentType
5353

5454
public static function fromPath(string $path, string $name = null, string $contentType = null): self
5555
{
56-
if (null === $contentType) {
57-
$ext = strtolower(substr($path, strrpos($path, '.') + 1));
58-
if (null === self::$mimeTypes) {
59-
self::$mimeTypes = new MimeTypes();
60-
}
61-
$contentType = self::$mimeTypes->getMimeTypes($ext)[0] ?? 'application/octet-stream';
62-
}
63-
64-
if ((is_file($path) && !is_readable($path)) || is_dir($path)) {
65-
throw new InvalidArgumentException(sprintf('Path "%s" is not readable.', $path));
66-
}
67-
68-
if (false === $handle = @fopen($path, 'r', false)) {
69-
throw new InvalidArgumentException(sprintf('Unable to open path "%s".', $path));
70-
}
71-
72-
if (!is_file($path)) {
73-
$cache = fopen('php://temp', 'r+');
74-
stream_copy_to_stream($handle, $cache);
75-
$handle = $cache;
76-
}
77-
78-
$p = new self($handle, $name ?: basename($path), $contentType);
79-
$p->handle = $handle;
80-
81-
return $p;
56+
return new self(new BodyFile($path), $name, $contentType);
8257
}
8358

8459
/**
@@ -158,13 +133,6 @@ private function generateContentId(): string
158133
return bin2hex(random_bytes(16)).'@symfony';
159134
}
160135

161-
public function __destruct()
162-
{
163-
if (null !== $this->handle && \is_resource($this->handle)) {
164-
fclose($this->handle);
165-
}
166-
}
167-
168136
public function __sleep(): array
169137
{
170138
// converts the body to a string

0 commit comments

Comments
 (0)
0