diff --git a/Controller/Annotations/View.php b/Controller/Annotations/View.php index 95583c13c..4d33b59c7 100644 --- a/Controller/Annotations/View.php +++ b/Controller/Annotations/View.php @@ -13,6 +13,57 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template; +if (class_exists(Template::class)) { + /** + * Compat class for applications where SensioFrameworkExtraBundle is installed, to be removed when compatibility with the bundle is no longer provided. + * + * @internal + */ + abstract class CompatView extends Template + { + } +} else { + /** + * Compat class for applications where SensioFrameworkExtraBundle is not installed. + * + * @internal + */ + abstract class CompatView + { + /** + * The controller (+action) this annotation is set to. + * + * @var array + * + * @note This property is declared within this compat class to not conflict with the {@see Template::$owner} + * property when SensioFrameworkExtraBundle is present. + */ + protected $owner = []; + + /** + * @note This method is declared within this compat class to not conflict with the {@see Template::setOwner} + * method when SensioFrameworkExtraBundle is present. + */ + public function setOwner(array $owner) + { + $this->owner = $owner; + } + + /** + * The controller (+action) this annotation is attached to. + * + * @return array + * + * @note This method is declared within this compat class to not conflict with the {@see Template::getOwner} + * method when SensioFrameworkExtraBundle is present. + */ + public function getOwner() + { + return $this->owner; + } + } +} + /** * View annotation class. * @@ -20,7 +71,7 @@ * @Target({"METHOD","CLASS"}) */ #[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD)] -class View extends Template +class View extends CompatView { /** * @var int|null @@ -49,12 +100,21 @@ public function __construct( array $serializerGroups = [], bool $serializerEnableMaxDepthChecks = false ) { - parent::__construct($data, $vars, $isStreamable, $owner); + if ($this instanceof Template) { + trigger_deprecation('friendsofsymfony/rest-bundle', '3.7', 'Extending from "%s" in "%s" is deprecated, the $vars and $isStreamable constructor arguments will not be supported when "sensio/framework-extra-bundle" is not installed and will be removed completely in 4.0.', Template::class, static::class); + + parent::__construct($data, $vars, $isStreamable, $owner); + } elseif ([] !== $vars) { + trigger_deprecation('friendsofsymfony/rest-bundle', '3.7', 'Extending from "%s" in "%s" is deprecated and "sensio/framework-extra-bundle" is not installed, the $vars and $isStreamable constructor arguments will be ignored and removed completely in 4.0.', Template::class, static::class); + } $values = is_array($data) ? $data : []; $this->statusCode = $values['statusCode'] ?? $statusCode; $this->serializerGroups = $values['serializerGroups'] ?? $serializerGroups; $this->serializerEnableMaxDepthChecks = $values['serializerEnableMaxDepthChecks'] ?? $serializerEnableMaxDepthChecks; + + // Use the setter to initialize the owner; when extending the Template class, the property will be private + $this->setOwner($values['owner'] ?? $owner); } /** diff --git a/EventListener/ViewResponseListener.php b/EventListener/ViewResponseListener.php index 0f73a372f..89eee18ac 100644 --- a/EventListener/ViewResponseListener.php +++ b/EventListener/ViewResponseListener.php @@ -11,17 +11,20 @@ namespace FOS\RestBundle\EventListener; +use Doctrine\Common\Annotations\Reader; +use Doctrine\Persistence\Proxy; use FOS\RestBundle\Controller\Annotations\View as ViewAnnotation; use FOS\RestBundle\FOSRestBundle; use FOS\RestBundle\View\View; use FOS\RestBundle\View\ViewHandlerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Event\ControllerEvent; use Symfony\Component\HttpKernel\Event\ViewEvent; use Symfony\Component\HttpKernel\KernelEvents; /** - * The ViewResponseListener class handles the View core event as well as the "@extra:Template" annotation. + * The ViewResponseListener class handles the kernel.view event and creates a {@see Response} for a {@see View} provided by the controller result. * * @author Lukas Kahwe Smith * @@ -29,13 +32,91 @@ */ class ViewResponseListener implements EventSubscriberInterface { + /** + * @var ViewHandlerInterface + */ private $viewHandler; + private $forceView; - public function __construct(ViewHandlerInterface $viewHandler, bool $forceView) + /** + * @var Reader|null + */ + private $annotationReader; + + public function __construct(ViewHandlerInterface $viewHandler, bool $forceView, ?Reader $annotationReader = null) { $this->viewHandler = $viewHandler; $this->forceView = $forceView; + $this->annotationReader = $annotationReader; + } + + /** + * Extracts configuration for a {@see ViewAnnotation} from the controller if present. + */ + public function onKernelController(ControllerEvent $event) + { + $request = $event->getRequest(); + + if (!$request->attributes->get(FOSRestBundle::ZONE_ATTRIBUTE, true)) { + return; + } + + $controller = $event->getController(); + + if (!\is_array($controller) && method_exists($controller, '__invoke')) { + $controller = [$controller, '__invoke']; + } + + if (!\is_array($controller)) { + return; + } + + $className = $this->getRealClass(\get_class($controller[0])); + $object = new \ReflectionClass($className); + $method = $object->getMethod($controller[1]); + + /** @var ViewAnnotation|null $classConfiguration */ + $classConfiguration = null; + + /** @var ViewAnnotation|null $methodConfiguration */ + $methodConfiguration = null; + + if (null !== $this->annotationReader) { + $classConfiguration = $this->getViewConfiguration($this->annotationReader->getClassAnnotations($object)); + $methodConfiguration = $this->getViewConfiguration($this->annotationReader->getMethodAnnotations($method)); + } + + if (80000 <= \PHP_VERSION_ID) { + if (null === $classConfiguration) { + $classAttributes = array_map( + function (\ReflectionAttribute $attribute) { + return $attribute->newInstance(); + }, + $object->getAttributes(ViewAnnotation::class, \ReflectionAttribute::IS_INSTANCEOF) + ); + + $classConfiguration = $this->getViewConfiguration($classAttributes); + } + + if (null === $methodConfiguration) { + $methodAttributes = array_map( + function (\ReflectionAttribute $attribute) { + return $attribute->newInstance(); + }, + $method->getAttributes(ViewAnnotation::class, \ReflectionAttribute::IS_INSTANCEOF) + ); + + $methodConfiguration = $this->getViewConfiguration($methodAttributes); + } + } + + // An annotation/attribute on the method takes precedence over the class level + if (null !== $methodConfiguration) { + $request->attributes->set(FOSRestBundle::VIEW_ATTRIBUTE, $methodConfiguration); + } elseif (null !== $classConfiguration) { + $request->attributes->set(FOSRestBundle::VIEW_ATTRIBUTE, $classConfiguration); + } } public function onKernelView(ViewEvent $event): void @@ -46,7 +127,8 @@ public function onKernelView(ViewEvent $event): void return; } - $configuration = $request->attributes->get('_template'); + /** @var ViewAnnotation|null $configuration */ + $configuration = $request->attributes->get(FOSRestBundle::VIEW_ATTRIBUTE); $view = $event->getControllerResult(); if (!$view instanceof View) { @@ -81,16 +163,49 @@ public function onKernelView(ViewEvent $event): void $view->setFormat($request->getRequestFormat()); } - $response = $this->viewHandler->handle($view, $request); - - $event->setResponse($response); + $event->setResponse($this->viewHandler->handle($view, $request)); } public static function getSubscribedEvents(): array { - // Must be executed before SensioFrameworkExtraBundle's listener return [ - KernelEvents::VIEW => ['onKernelView', 30], + KernelEvents::CONTROLLER => 'onKernelController', + KernelEvents::VIEW => ['onKernelView', -128], ]; } + + /** + * @param object[] $annotations + */ + private function getViewConfiguration(array $annotations): ?ViewAnnotation + { + $viewAnnotation = null; + + foreach ($annotations as $annotation) { + if (!$annotation instanceof ViewAnnotation) { + continue; + } + + if (null === $viewAnnotation) { + $viewAnnotation = $annotation; + } else { + throw new \LogicException('Multiple "view" annotations are not allowed.'); + } + } + + return $viewAnnotation; + } + + private function getRealClass(string $class): string + { + if (class_exists(Proxy::class)) { + if (false === $pos = strrpos($class, '\\'.Proxy::MARKER.'\\')) { + return $class; + } + + return substr($class, $pos + Proxy::MARKER_LENGTH + 2); + } + + return $class; + } } diff --git a/FOSRestBundle.php b/FOSRestBundle.php index a45151843..655a15a0a 100644 --- a/FOSRestBundle.php +++ b/FOSRestBundle.php @@ -27,6 +27,7 @@ */ class FOSRestBundle extends Bundle { + const VIEW_ATTRIBUTE = '_fos_rest_view'; const ZONE_ATTRIBUTE = '_fos_rest_zone'; /** diff --git a/Resources/config/view_response_listener.xml b/Resources/config/view_response_listener.xml index c4cff13b9..5ea9b7484 100644 --- a/Resources/config/view_response_listener.xml +++ b/Resources/config/view_response_listener.xml @@ -10,6 +10,7 @@ + diff --git a/Tests/EventListener/ViewResponseListenerTest.php b/Tests/EventListener/ViewResponseListenerTest.php index cbc6b98fd..d383a1a70 100644 --- a/Tests/EventListener/ViewResponseListenerTest.php +++ b/Tests/EventListener/ViewResponseListenerTest.php @@ -11,12 +11,16 @@ namespace FOS\RestBundle\Tests\EventListener; +use Doctrine\Common\Annotations\Reader; use FOS\RestBundle\Controller\Annotations\View as ViewAnnotation; use FOS\RestBundle\EventListener\ViewResponseListener; +use FOS\RestBundle\FOSRestBundle; use FOS\RestBundle\Serializer\Serializer; +use FOS\RestBundle\Tests\Functional\Bundle\TestBundle\Controller\Version2Controller; use FOS\RestBundle\View\View; use FOS\RestBundle\View\ViewHandler; use FOS\RestBundle\View\ViewHandlerInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -39,7 +43,12 @@ class ViewResponseListenerTest extends TestCase public $listener; /** - * @var ViewHandlerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var (MockObject&Reader)|null + */ + public $annotationReader; + + /** + * @var ViewHandlerInterface */ public $viewHandler; @@ -47,23 +56,66 @@ class ViewResponseListenerTest extends TestCase private $serializer; private $requestStack; + protected function getControllerEvent(Request $request, callable $controller): ControllerEvent + { + $kernel = $this->createMock(HttpKernelInterface::class); + + return new ControllerEvent($kernel, $controller, $request, HttpKernelInterface::MASTER_REQUEST); + } + + public function testExtractsViewConfigurationFromAnnotationOnMethod() + { + if (null === $this->annotationReader || 80000 <= \PHP_VERSION_ID) { + $this->markTestSkipped('Test only applies when doctrine/annotations is installed and running on PHP 7'); + } + + $this->createViewResponseListener(); + + $this->annotationReader->expects($this->once()) + ->method('getClassAnnotations') + ->willReturn([]); + + $this->annotationReader->expects($this->once()) + ->method('getMethodAnnotations') + ->willReturn([new ViewAnnotation()]); + + $controller = new Version2Controller(); + + $request = new Request(); + $event = $this->getControllerEvent($request, [$controller, 'versionAction']); + + $this->listener->onKernelController($event); + + $config = $request->attributes->get(FOSRestBundle::VIEW_ATTRIBUTE); + + $this->assertNotNull($config); + $this->assertInstanceOf(ViewAnnotation::class, $config); + } + /** - * @return ControllerEvent|\PHPUnit_Framework_MockObject_MockObject + * @requires PHP 8.0 */ - protected function getFilterEvent(Request $request) + public function testExtractsViewConfigurationFromAttributeOnMethod() { - $controller = new FooController(); - $kernel = $this->createMock(HttpKernelInterface::class); + $this->createViewResponseListener(); + + $controller = new Version2Controller(); + + $request = new Request(); + $event = $this->getControllerEvent($request, [$controller, 'versionAction']); + + $this->listener->onKernelController($event); - return new ControllerEvent($kernel, [$controller, 'viewAction'], $request, null); + $config = $request->attributes->get(FOSRestBundle::VIEW_ATTRIBUTE); + + $this->assertNotNull($config); + $this->assertInstanceOf(ViewAnnotation::class, $config); } /** * @param mixed $result - * - * @return ViewEvent|\PHPUnit_Framework_MockObject_MockObject */ - protected function getResponseEvent(Request $request, $result) + protected function getViewEvent(Request $request, $result): ViewEvent { $kernel = $this->createMock(HttpKernelInterface::class); @@ -75,9 +127,10 @@ public function testOnKernelViewWhenControllerResultIsNotViewObject() $this->createViewResponseListener(); $request = new Request(); - $event = $this->getResponseEvent($request, []); + $event = $this->getViewEvent($request, []); + + $this->listener->onKernelView($event); - $this->assertNull($this->listener->onKernelView($event)); $this->assertNull($event->getResponse()); } @@ -103,13 +156,13 @@ public function testStatusCode($annotationCode, $viewCode, $expectedCode) $request = new Request(); $request->setRequestFormat('json'); - $request->attributes->set('_template', $viewAnnotation); + $request->attributes->set(FOSRestBundle::VIEW_ATTRIBUTE, $viewAnnotation); $view = new View(); $view->setStatusCode($viewCode); $view->setData('foo'); - $event = $this->getResponseEvent($request, $view); + $event = $this->getViewEvent($request, $view); $this->listener->onKernelView($event); $response = $event->getResponse(); @@ -138,11 +191,11 @@ public function testSerializerEnableMaxDepthChecks($enableMaxDepthChecks, $expec $request = new Request(); $request->setRequestFormat('json'); - $request->attributes->set('_template', $viewAnnotation); + $request->attributes->set(FOSRestBundle::VIEW_ATTRIBUTE, $viewAnnotation); $view = new View(); - $event = $this->getResponseEvent($request, $view); + $event = $this->getViewEvent($request, $view); $this->listener->onKernelView($event); @@ -161,6 +214,7 @@ public function getDataForDefaultVarsCopy() protected function setUp(): void { + $this->annotationReader = interface_exists(Reader::class) && 80000 > \PHP_VERSION_ID ? $this->getMockBuilder(Reader::class)->getMock() : null; $this->router = $this->getMockBuilder(RouterInterface::class)->getMock(); $this->serializer = $this->getMockBuilder(Serializer::class)->getMock(); $this->requestStack = new RequestStack(); @@ -169,23 +223,6 @@ protected function setUp(): void private function createViewResponseListener($formats = null) { $this->viewHandler = ViewHandler::create($this->router, $this->serializer, $this->requestStack, $formats); - $this->listener = new ViewResponseListener($this->viewHandler, false); - } -} - -class FooController -{ - /** - * @see testOnKernelView() - */ - public function onKernelViewAction($foo, $halli) - { - } - - /** - * @see testViewWithNoCopyDefaultVars() - */ - public function viewAction($customer) - { + $this->listener = new ViewResponseListener($this->viewHandler, false, $this->annotationReader); } } diff --git a/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php b/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php index ac2e87dd9..32c03aba3 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/ArticleController.php @@ -25,8 +25,11 @@ class ArticleController extends AbstractFOSRestController * @return View view instance * * @Post("/articles.{_format}", name="post_articles") + * * @View() */ + #[Post(path: '/articles.{_format}', name: 'post_articles')] + #[View] public function cpostAction(Request $request) { $view = $this->routeRedirectView('test_redirect_endpoint', ['name' => $request->request->get('name')]); @@ -40,8 +43,11 @@ public function cpostAction(Request $request) * @return View view instance * * @Get("/articles.{_format}", name="get_article", defaults={"_format": "html"}) + * * @View() */ + #[Get(path: '/articles.{_format}', name: 'get_article', defaults: ['_format' => 'html'])] + #[View] public function cgetAction(Request $request) { $view = $this->view(); diff --git a/Tests/Functional/Bundle/TestBundle/Controller/RequestBodyParamConverterController.php b/Tests/Functional/Bundle/TestBundle/Controller/RequestBodyParamConverterController.php index 602413ec9..507a6dc7b 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/RequestBodyParamConverterController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/RequestBodyParamConverterController.php @@ -20,7 +20,8 @@ class RequestBodyParamConverterController extends AbstractController /** * @ParamConverter("post", converter="fos_rest.request_body") */ - public function putPostAction(Post $post, \Datetime $date) + #[ParamConverter(['name' => 'post'], converter: 'fos_rest.request_body')] + public function putPostAction(Post $post, \DateTime $date) { return new Response($post->getName()); } diff --git a/Tests/Functional/Bundle/TestBundle/Controller/SerializerErrorController.php b/Tests/Functional/Bundle/TestBundle/Controller/SerializerErrorController.php index a7915f0c7..0a1f7159d 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/SerializerErrorController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/SerializerErrorController.php @@ -50,6 +50,7 @@ public function customExceptionAction() /** * @View */ + #[View] public function invalidFormAction() { $form = $this->createFormBuilder(null, [ diff --git a/Tests/Functional/Bundle/TestBundle/Controller/Version2Controller.php b/Tests/Functional/Bundle/TestBundle/Controller/Version2Controller.php index f60dee566..8d8b225d3 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/Version2Controller.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/Version2Controller.php @@ -23,8 +23,11 @@ class Version2Controller extends AbstractFOSRestController { /** * @View() + * * @Get(path="/version", condition="request.attributes.get('version') in ['1.2']") */ + #[View] + #[Get(path: '/version', condition: 'request.attributes.get("version") in ["1.2"]')] public function versionAction($version) { return ['version' => 'test annotation']; @@ -32,8 +35,11 @@ public function versionAction($version) /** * @View() + * * @Get(path="/version/{version}", requirements={"version": "1.2"}) */ + #[View] + #[Get(path: '/version/{version}', requirements: ['version' => '1.2'])] public function versionPathAction(Request $request, $version) { $versionExclusion = $this->findExclusionStrategyVersion($request); diff --git a/Tests/Functional/Bundle/TestBundle/Controller/VersionController.php b/Tests/Functional/Bundle/TestBundle/Controller/VersionController.php index 4dc489413..77afbc92a 100644 --- a/Tests/Functional/Bundle/TestBundle/Controller/VersionController.php +++ b/Tests/Functional/Bundle/TestBundle/Controller/VersionController.php @@ -23,6 +23,7 @@ class VersionController extends AbstractFOSRestController /** * @View() */ + #[View] public function versionAction(Request $request, $version) { $versionExclusion = $this->findExclusionStrategyVersion($request); diff --git a/composer.json b/composer.json index 62469eea6..d4cae108f 100644 --- a/composer.json +++ b/composer.json @@ -32,6 +32,7 @@ "php": "^7.2|^8.0", "symfony/config": "^5.4|^6.0", "symfony/dependency-injection": "^5.4|^6.0", + "symfony/deprecation-contracts": "^2.1|^3.0", "symfony/event-dispatcher": "^5.4|^6.0", "symfony/framework-bundle": "^4.4.1|^5.0|^6.0", "symfony/http-foundation": "^5.4|^6.0", @@ -49,7 +50,7 @@ "psr/http-message": "^1.0", "psr/log": "^1.0|^2.0|^3.0", "sensio/framework-extra-bundle": "^6.1", - "symfony/phpunit-bridge": "^5.4|^6.0", + "symfony/phpunit-bridge": "^7.0.1", "symfony/asset": "^5.4|^6.0", "symfony/browser-kit": "^5.4|^6.0", "symfony/css-selector": "^5.4|^6.0", diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 7f9b085e9..b66e830fb 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -2,7 +2,7 @@ - +