From 5b46542b411ca0dba5457f3269adab543971ddec Mon Sep 17 00:00:00 2001 From: Malte Wunsch Date: Tue, 19 Nov 2024 12:50:51 +0100 Subject: [PATCH 1/6] Test that the event (not controller inside the event) is the source for attributes ... and that we behave accordingly, by saving attributes in the event when replacing the controller inside the event. --- tests/NotModified/EventListenerTest.php | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/NotModified/EventListenerTest.php b/tests/NotModified/EventListenerTest.php index 38e312f..a22ed55 100644 --- a/tests/NotModified/EventListenerTest.php +++ b/tests/NotModified/EventListenerTest.php @@ -159,6 +159,42 @@ public function eventListenerDifferentiatesBetweenMultipleRequests(): void self::assertNull($anotherResponse->getLastModified()); } + /** @test */ + public function onKernelControllerSearchesEventInsteadOfControllerForAttribute(): void + { + // setup an event that should lead to a NotModified response + $this->request->headers->set('If-Modified-Since', '-1 hour'); + $this->filterControllerEvent = new ControllerEvent( + $this->kernel, + [DummyController::class, 'oneDayAgoModifiedLastModifiedAction'], + $this->request, + HttpKernelInterface::MAIN_REQUEST + ); + + // now simulate another EventListener has replaced the response of the controller inside the event, but has + // saved the original attributes in the event. + $this->filterControllerEvent->setController( + function () { + return new Response(); + }, + [ReplaceWithNotModifiedResponse::class => [new ReplaceWithNotModifiedResponse([OneDayAgoModifiedLastModifiedDeterminator::class])]] + ); + + $this->eventListener->onKernelController($this->filterControllerEvent); + + self::assertNotModifiedResponse(); + } + + /** @test */ + public function onKernelControllerSavesOriginalControllerAttributesWhenReplacingTheController(): void + { + $this->request->headers->set('If-Modified-Since', '-1 hour'); + + $this->exerciseOnKernelController([DummyController::class, 'oneDayAgoModifiedLastModifiedAction']); + + self::assertNotEmpty($this->filterControllerEvent->getAttributes()); + } + private function exerciseOnKernelController(array $callable): void { $this->callable = $callable; From 2eb78702431901135bceb215febe155cb369a011 Mon Sep 17 00:00:00 2001 From: Malte Wunsch Date: Tue, 19 Nov 2024 12:53:44 +0100 Subject: [PATCH 2/6] Search the event (not controller inside the event) for attributes ... and save attributes of the original controller in the event when replacing the controller. --- src/NotModified/EventListener.php | 37 ++++++++++--------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/src/NotModified/EventListener.php b/src/NotModified/EventListener.php index cd6030a..68b8951 100644 --- a/src/NotModified/EventListener.php +++ b/src/NotModified/EventListener.php @@ -9,7 +9,6 @@ namespace Webfactory\HttpCacheBundle\NotModified; -use ReflectionMethod; use SplObjectStorage; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Response; @@ -46,14 +45,16 @@ public function __construct( */ public function onKernelController(ControllerEvent $event): void { - $annotation = $this->findAnnotation($event->getController()); - if (!$annotation) { + $attributes = $event->getAttributes(ReplaceWithNotModifiedResponse::class); + + if (!$attributes) { return; } + $attribute = $attributes[0]; $request = $event->getRequest(); - $annotation->setContainer($this->container); - $lastModified = $annotation->determineLastModified($request); + $attribute->setContainer($this->container); + $lastModified = $attribute->determineLastModified($request); if (!$lastModified) { return; } @@ -70,9 +71,12 @@ public function onKernelController(ControllerEvent $event): void $response->setLastModified($lastModified); if ($response->isNotModified($request)) { - $event->setController(function () use ($response) { - return $response; - }); + $event->setController( + function () use ($response) { + return $response; + }, + $event->getAttributes() + ); } } @@ -89,21 +93,4 @@ public function onKernelResponse(ResponseEvent $event): void $response->setLastModified($this->lastModified[$request]); } } - - /** - * @param $controllerCallable callable PHP callback pointing to the method to reflect on. - */ - private function findAnnotation(callable $controllerCallable): ?ReplaceWithNotModifiedResponse - { - if (!is_array($controllerCallable)) { - return null; - } - - [$class, $methodName] = $controllerCallable; - $method = new ReflectionMethod($class, $methodName); - - $attributes = $method->getAttributes(ReplaceWithNotModifiedResponse::class); - - return $attributes ? $attributes[0]->newInstance() : null; - } } From 35e735e8d8b79cc89be02e961e286d3e7020aa1f Mon Sep 17 00:00:00 2001 From: Malte Wunsch Date: Tue, 19 Nov 2024 13:01:22 +0100 Subject: [PATCH 3/6] Assert exception if attribute is present more than once --- tests/NotModified/EventListenerTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/NotModified/EventListenerTest.php b/tests/NotModified/EventListenerTest.php index a22ed55..05fc718 100644 --- a/tests/NotModified/EventListenerTest.php +++ b/tests/NotModified/EventListenerTest.php @@ -195,6 +195,15 @@ public function onKernelControllerSavesOriginalControllerAttributesWhenReplacing self::assertNotEmpty($this->filterControllerEvent->getAttributes()); } + /** @test */ + public function onKernelControllerThrowsExceptionIfAttributeIsFoundMoreThanOnce(): void + { + self::expectException(\Error::class); + self::expectExceptionMessageMatches('/ReplaceWithNotModifiedResponse/'); + + $this->exerciseOnKernelController([DummyController::class, 'actionWithMoreThanOneAttribute']); + } + private function exerciseOnKernelController(array $callable): void { $this->callable = $callable; @@ -249,6 +258,13 @@ public static function fixedDateAgoModifiedLastModifiedDeterminatorAction(): Res { return new Response(); } + + #[ReplaceWithNotModifiedResponse([AbstainingLastModifiedDeterminator::class])] + #[ReplaceWithNotModifiedResponse([OneDayAgoModifiedLastModifiedDeterminator::class])] + public static function actionWithMoreThanOneAttribute(): Response + { + return new Response(); + } } final class AbstainingLastModifiedDeterminator implements LastModifiedDeterminator From 98e608c68c89ddc4e814f81781cf8b4daff5b2b4 Mon Sep 17 00:00:00 2001 From: MalteWunsch Date: Tue, 19 Nov 2024 12:03:18 +0000 Subject: [PATCH 4/6] Fix CS with PHP-CS-Fixer --- tests/NotModified/EventListenerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/NotModified/EventListenerTest.php b/tests/NotModified/EventListenerTest.php index 05fc718..c79715c 100644 --- a/tests/NotModified/EventListenerTest.php +++ b/tests/NotModified/EventListenerTest.php @@ -11,6 +11,7 @@ use Closure; use DateTime; +use Error; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -198,7 +199,7 @@ public function onKernelControllerSavesOriginalControllerAttributesWhenReplacing /** @test */ public function onKernelControllerThrowsExceptionIfAttributeIsFoundMoreThanOnce(): void { - self::expectException(\Error::class); + self::expectException(Error::class); self::expectExceptionMessageMatches('/ReplaceWithNotModifiedResponse/'); $this->exerciseOnKernelController([DummyController::class, 'actionWithMoreThanOneAttribute']); From c31f8a1270af726ca9648e892f3de968bf6d483e Mon Sep 17 00:00:00 2001 From: Malte Wunsch Date: Tue, 19 Nov 2024 13:05:44 +0100 Subject: [PATCH 5/6] Bump minimum symfony/http-kernel version accordingly --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index bc07192..ef91709 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,7 @@ "symfony/dependency-injection": "^5.0 | ^6.0 | ^7.0", "symfony/deprecation-contracts": "^2.0|^3.0", "symfony/http-foundation": "^5.0 | ^6.0 | ^7.0", - "symfony/http-kernel": "^5.3 | ^6.0 | ^7.0" + "symfony/http-kernel": "^6.2 | ^7.0" }, "require-dev": { From 9d16b164130f7548752ba7b9464810d6aec809ac Mon Sep 17 00:00:00 2001 From: Malte Wunsch Date: Tue, 19 Nov 2024 13:14:18 +0100 Subject: [PATCH 6/6] Bump minimum symfony/http-kernel version again While ControllerEvent->getAttributes() has been introduced in http-kernel 6.2, we use it's more comfortable signature available in 6.4. --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ef91709..3a3652a 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,7 @@ "symfony/dependency-injection": "^5.0 | ^6.0 | ^7.0", "symfony/deprecation-contracts": "^2.0|^3.0", "symfony/http-foundation": "^5.0 | ^6.0 | ^7.0", - "symfony/http-kernel": "^6.2 | ^7.0" + "symfony/http-kernel": "^6.4 | ^7.0" }, "require-dev": {