From e0a5707018e741aa098b6624a312e1d811916bc6 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Mon, 9 Feb 2026 15:01:50 +0400 Subject: [PATCH 1/5] AttachmentController --- composer.json | 2 +- .../Controller/AttachmentController.php | 59 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 src/Messaging/Controller/AttachmentController.php diff --git a/composer.json b/composer.json index 0934eda..c9f9ea7 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-dev", + "phplist/core": "dev-feat/attachment", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4", diff --git a/src/Messaging/Controller/AttachmentController.php b/src/Messaging/Controller/AttachmentController.php new file mode 100644 index 0000000..f06f362 --- /dev/null +++ b/src/Messaging/Controller/AttachmentController.php @@ -0,0 +1,59 @@ + '\\d+'], methods: ['GET'])] + #[OA\Get( + path: '/api/v2/attachments/{id}/download', + description: 'Download an attachment by ID. `uid` query parameter is required.', + summary: 'Download attachment', + tags: ['campaigns'], + parameters: [ + new OA\Parameter( + name: 'id', + description: 'Attachment ID', + in: 'path', + required: true, + schema: new OA\Schema(type: 'integer') + ), + new OA\Parameter( + name: 'uid', + description: 'Download token', + in: 'query', + required: true, + schema: new OA\Schema(type: 'string') + ), + ], + responses: [ + new OA\Response(response: 200, description: 'File stream'), + new OA\Response(response: 403, description: 'Unauthorized'), + new OA\Response(response: 404, description: 'Not found'), + ] + )] + public function download(Attachment $attachment): BinaryFileResponse + { + $this->attachmentDownloadService->getDownloadable($attachment); + } +} From 41f9c2dd016b1e953b7c0a6d5a9b3f183f19d9cd Mon Sep 17 00:00:00 2001 From: Tatevik Date: Tue, 10 Feb 2026 13:08:19 +0400 Subject: [PATCH 2/5] AttachmentController return StreamedResponse --- .../EventListener/ExceptionListener.php | 14 +++++- .../Controller/AttachmentController.php | 50 ++++++++++++++++--- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/Common/EventListener/ExceptionListener.php b/src/Common/EventListener/ExceptionListener.php index d66b852..87097b0 100644 --- a/src/Common/EventListener/ExceptionListener.php +++ b/src/Common/EventListener/ExceptionListener.php @@ -6,7 +6,9 @@ use Exception; use PhpList\Core\Domain\Identity\Exception\AdminAttributeCreationException; +use PhpList\Core\Domain\Messaging\Exception\AttachmentFileNotFoundException; use PhpList\Core\Domain\Messaging\Exception\MessageNotReceivedException; +use PhpList\Core\Domain\Messaging\Exception\SubscriberNotFoundException; use PhpList\Core\Domain\Subscription\Exception\AttributeDefinitionCreationException; use PhpList\Core\Domain\Subscription\Exception\SubscriptionCreationException; use Symfony\Component\HttpFoundation\JsonResponse; @@ -67,7 +69,17 @@ public function onKernelException(ExceptionEvent $event): void 'message' => $exception->getMessage(), ], 422); $event->setResponse($response); - } elseif ($exception instanceof Exception) { + } elseif ($exception instanceof AttachmentFileNotFoundException) { + $response = new JsonResponse([ + 'message' => $exception->getMessage(), + ], 404); + $event->setResponse($response); + } elseif ($exception instanceof SubscriberNotFoundException) { + $response = new JsonResponse([ + 'message' => $exception->getMessage(), + ], 404); + $event->setResponse($response); + } elseif ($exception instanceof Exception) { $response = new JsonResponse([ 'message' => $exception->getMessage(), ], 500); diff --git a/src/Messaging/Controller/AttachmentController.php b/src/Messaging/Controller/AttachmentController.php index f06f362..a9c9142 100644 --- a/src/Messaging/Controller/AttachmentController.php +++ b/src/Messaging/Controller/AttachmentController.php @@ -10,7 +10,10 @@ use PhpList\Core\Security\Authentication; use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Validator\RequestValidator; -use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Symfony\Bridge\Doctrine\Attribute\MapEntity; +use Symfony\Component\HttpFoundation\ResponseHeaderBag; +use Symfony\Component\HttpFoundation\StreamedResponse; +use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; use Symfony\Component\Routing\Attribute\Route; #[Route('/attachments', name: 'attachments_')] @@ -24,12 +27,12 @@ public function __construct( parent::__construct($authentication, $validator); } - #[Route('/{id}/download', name: 'download', requirements: ['id' => '\\d+'], methods: ['GET'])] + #[Route('/download/{id}', name: 'download', requirements: ['id' => '\\d+'], methods: ['GET'])] #[OA\Get( - path: '/api/v2/attachments/{id}/download', + path: '/api/v2/attachments/download/{id}', description: 'Download an attachment by ID. `uid` query parameter is required.', summary: 'Download attachment', - tags: ['campaigns'], + tags: ['attachments'], parameters: [ new OA\Parameter( name: 'id', @@ -40,7 +43,7 @@ public function __construct( ), new OA\Parameter( name: 'uid', - description: 'Download token', + description: 'Download token (subscriber email or word "forwarded")', in: 'query', required: true, schema: new OA\Schema(type: 'string') @@ -52,8 +55,39 @@ public function __construct( new OA\Response(response: 404, description: 'Not found'), ] )] - public function download(Attachment $attachment): BinaryFileResponse - { - $this->attachmentDownloadService->getDownloadable($attachment); + public function download( + #[MapEntity(mapping: ['id' => 'id'])] Attachment $attachment, + #[MapQueryParameter] string $uid + ): StreamedResponse { + $downloadable = $this->attachmentDownloadService->getDownloadable($attachment, $uid); + + $headers = [ + 'Content-Type' => $downloadable->mimeType, + 'Content-Disposition' => ResponseHeaderBag::makeDisposition( + disposition: ResponseHeaderBag::DISPOSITION_ATTACHMENT, + fileName: $downloadable->filename + ), + ]; + + if ($downloadable->size !== null) { + $headers['Content-Length'] = (string) $downloadable->size; + } + + return new StreamedResponse( + callback: function () use ($downloadable) { + $stream = $downloadable->content; + + if ($stream->isSeekable()) { + $stream->rewind(); + } + + while (!$stream->eof()) { + echo $stream->read(8192); + flush(); + } + }, + status: 200, + headers: $headers + ); } } From 2a1479678bd8a0a55ccf211f66dfd7312ccd31ab Mon Sep 17 00:00:00 2001 From: Tatevik Date: Tue, 10 Feb 2026 13:17:44 +0400 Subject: [PATCH 3/5] Add test --- .../EventListener/ExceptionListener.php | 2 +- .../Controller/AttachmentController.php | 5 +- .../Controller/AttachmentControllerTest.php | 125 ++++++++++++++++++ .../Messaging/Fixtures/AttachmentFixture.php | 33 +++++ 4 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 tests/Integration/Messaging/Controller/AttachmentControllerTest.php create mode 100644 tests/Integration/Messaging/Fixtures/AttachmentFixture.php diff --git a/src/Common/EventListener/ExceptionListener.php b/src/Common/EventListener/ExceptionListener.php index 87097b0..f7f5fb5 100644 --- a/src/Common/EventListener/ExceptionListener.php +++ b/src/Common/EventListener/ExceptionListener.php @@ -79,7 +79,7 @@ public function onKernelException(ExceptionEvent $event): void 'message' => $exception->getMessage(), ], 404); $event->setResponse($response); - } elseif ($exception instanceof Exception) { + } elseif ($exception instanceof Exception) { $response = new JsonResponse([ 'message' => $exception->getMessage(), ], 500); diff --git a/src/Messaging/Controller/AttachmentController.php b/src/Messaging/Controller/AttachmentController.php index a9c9142..b7e90fe 100644 --- a/src/Messaging/Controller/AttachmentController.php +++ b/src/Messaging/Controller/AttachmentController.php @@ -11,6 +11,7 @@ use PhpList\RestBundle\Common\Controller\BaseController; use PhpList\RestBundle\Common\Validator\RequestValidator; use Symfony\Bridge\Doctrine\Attribute\MapEntity; +use Symfony\Component\HttpFoundation\HeaderUtils; use Symfony\Component\HttpFoundation\ResponseHeaderBag; use Symfony\Component\HttpFoundation\StreamedResponse; use Symfony\Component\HttpKernel\Attribute\MapQueryParameter; @@ -63,9 +64,9 @@ public function download( $headers = [ 'Content-Type' => $downloadable->mimeType, - 'Content-Disposition' => ResponseHeaderBag::makeDisposition( + 'Content-Disposition' => HeaderUtils::makeDisposition( disposition: ResponseHeaderBag::DISPOSITION_ATTACHMENT, - fileName: $downloadable->filename + filename: $downloadable->filename ), ]; diff --git a/tests/Integration/Messaging/Controller/AttachmentControllerTest.php b/tests/Integration/Messaging/Controller/AttachmentControllerTest.php new file mode 100644 index 0000000..e00bc7c --- /dev/null +++ b/tests/Integration/Messaging/Controller/AttachmentControllerTest.php @@ -0,0 +1,125 @@ +repoPath = (string) self::getContainer()->getParameter('phplist.attachment_repository_path'); + if (!is_dir($this->repoPath)) { + mkdir($this->repoPath, 0777, true); + } + } + + protected function tearDown(): void + { + // Clean up any test file we might have created + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + if (is_file($file)) { + unlink($file); + } + + parent::tearDown(); + } + + public function testControllerIsAvailableViaContainer(): void + { + self::assertInstanceOf( + AttachmentController::class, + self::getContainer()->get(AttachmentController::class) + ); + } + + public function testDownloadReturnsFileStreamWithHeaders(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Prepare the actual file in the repository path + $content = 'Hello Attachment'; + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + file_put_contents($file, $content); + + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + Attachment::FORWARD + ) + ); + + $response = self::getClient()->getResponse(); + + // StreamedResponse should be 200 with correct headers + self::assertSame(200, $response->getStatusCode()); + self::assertSame('text/plain; charset=UTF-8', $response->headers->get('Content-Type')); + self::assertStringContainsString( + 'attachment; filename=' . AttachmentFixture::FILENAME, + (string) $response->headers->get('Content-Disposition') + ); + self::assertSame((string) strlen($content), $response->headers->get('Content-Length')); + + $callback = $response->getCallback(); + ob_start(); + $callback(); + $body = ob_get_clean(); + + self::assertSame($content, $body); + } + + public function testDownloadReturnsNotFoundWhenAttachmentEntityMissing(): void + { + self::getClient()->request('GET', '/api/v2/attachments/download/999999?uid=' . Attachment::FORWARD); + $this->assertHttpNotFound(); + } + + public function testDownloadReturnsNotFoundWhenUidEmailNotFound(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Do not create the file; the uid validation happens first and should 404 + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + 'does-not-exist@example.com' + ) + ); + + $this->assertHttpNotFound(); + } + + public function testDownloadReturnsNotFoundWhenFileMissing(): void + { + $this->loadFixtures([AttachmentFixture::class]); + + // Ensure no file exists + $file = $this->repoPath . DIRECTORY_SEPARATOR . AttachmentFixture::FILENAME; + if (is_file($file)) { + unlink($file); + } + + self::getClient()->request( + 'GET', + sprintf( + '/api/v2/attachments/download/%d?uid=%s', + AttachmentFixture::ATTACHMENT_ID, + Attachment::FORWARD + ) + ); + + $this->assertHttpNotFound(); + } +} diff --git a/tests/Integration/Messaging/Fixtures/AttachmentFixture.php b/tests/Integration/Messaging/Fixtures/AttachmentFixture.php new file mode 100644 index 0000000..7d23319 --- /dev/null +++ b/tests/Integration/Messaging/Fixtures/AttachmentFixture.php @@ -0,0 +1,33 @@ +setSubjectId($attachment, self::ATTACHMENT_ID); + $manager->persist($attachment); + $manager->flush(); + } +} From c85162a72632e92bf5675007a98e55837f7714e2 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Wed, 11 Feb 2026 13:36:40 +0400 Subject: [PATCH 4/5] Ref: ExceptionListener --- .../EventListener/ExceptionListener.php | 96 +++++++------------ 1 file changed, 37 insertions(+), 59 deletions(-) diff --git a/src/Common/EventListener/ExceptionListener.php b/src/Common/EventListener/ExceptionListener.php index f7f5fb5..f272462 100644 --- a/src/Common/EventListener/ExceptionListener.php +++ b/src/Common/EventListener/ExceptionListener.php @@ -20,71 +20,49 @@ class ExceptionListener { - /** - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - */ + private const EXCEPTION_STATUS_MAP = [ + SubscriptionCreationException::class => null, + AttributeDefinitionCreationException::class => null, + AdminAttributeCreationException::class => null, + ValidatorException::class => 400, + AccessDeniedException::class => 403, + AccessDeniedHttpException::class => 403, + AttachmentFileNotFoundException::class => 404, + SubscriberNotFoundException::class => 404, + MessageNotReceivedException::class => 422, + ]; + public function onKernelException(ExceptionEvent $event): void { $exception = $event->getThrowable(); - if ($exception instanceof AccessDeniedHttpException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 403); - - $event->setResponse($response); - } elseif ($exception instanceof HttpExceptionInterface) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); + foreach (self::EXCEPTION_STATUS_MAP as $class => $statusCode) { + if ($exception instanceof $class) { + $status = $statusCode ?? $exception->getStatusCode(); + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], $status) + ); + return; + } + } - $event->setResponse($response); - } elseif ($exception instanceof SubscriptionCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof AdminAttributeCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof AttributeDefinitionCreationException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], $exception->getStatusCode()); - $event->setResponse($response); - } elseif ($exception instanceof ValidatorException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 400); - $event->setResponse($response); - } elseif ($exception instanceof AccessDeniedException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 403); - $event->setResponse($response); - } elseif ($exception instanceof MessageNotReceivedException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 422); - $event->setResponse($response); - } elseif ($exception instanceof AttachmentFileNotFoundException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 404); - $event->setResponse($response); - } elseif ($exception instanceof SubscriberNotFoundException) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 404); - $event->setResponse($response); - } elseif ($exception instanceof Exception) { - $response = new JsonResponse([ - 'message' => $exception->getMessage(), - ], 500); + if ($exception instanceof HttpExceptionInterface) { + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], $exception->getStatusCode()) + ); + return; + } - $event->setResponse($response); + if ($exception instanceof Exception) { + $event->setResponse( + new JsonResponse([ + 'message' => $exception->getMessage() + ], 500) + ); } } } From 951bf4f101e0310f9cdb47b1ba28ad1c4547b047 Mon Sep 17 00:00:00 2001 From: Tatevik Date: Wed, 11 Feb 2026 14:03:19 +0400 Subject: [PATCH 5/5] Version bump to dev-dev --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index c9f9ea7..0934eda 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ }, "require": { "php": "^8.1", - "phplist/core": "dev-feat/attachment", + "phplist/core": "dev-dev", "friendsofsymfony/rest-bundle": "*", "symfony/test-pack": "^1.0", "symfony/process": "^6.4",