From e426c696ddef5664016fd057afd2a877295c6438 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Fri, 3 Jan 2025 12:47:08 +0100 Subject: [PATCH] Replaced Param Converters with Value Resolvers --- .../Core/Converter/ContentParamConverter.php | 37 -------- .../Core/Converter/LocationParamConverter.php | 50 ---------- .../Converter/RepositoryParamConverter.php | 54 ----------- src/bundle/Core/Resources/config/services.yml | 20 +--- .../Resources/config/value_resolvers.yaml | 11 +++ .../ValueResolver/ContentValueResolver.php | 46 ++++++++++ .../ValueResolver/LocationValueResolver.php | 51 +++++++++++ .../Converter/AbstractParamConverterTest.php | 36 -------- .../Converter/ContentParamConverterTest.php | 75 --------------- .../Converter/LocationParamConverterTest.php | 78 ---------------- .../ContentValueResolverTest.php | 79 ++++++++++++++++ .../LocationValueResolverTest.php | 91 +++++++++++++++++++ 12 files changed, 279 insertions(+), 349 deletions(-) delete mode 100644 src/bundle/Core/Converter/ContentParamConverter.php delete mode 100644 src/bundle/Core/Converter/LocationParamConverter.php delete mode 100644 src/bundle/Core/Converter/RepositoryParamConverter.php create mode 100644 src/bundle/Core/Resources/config/value_resolvers.yaml create mode 100644 src/bundle/Core/ValueResolver/ContentValueResolver.php create mode 100644 src/bundle/Core/ValueResolver/LocationValueResolver.php delete mode 100644 tests/bundle/Core/Converter/AbstractParamConverterTest.php delete mode 100644 tests/bundle/Core/Converter/ContentParamConverterTest.php delete mode 100644 tests/bundle/Core/Converter/LocationParamConverterTest.php create mode 100644 tests/bundle/Core/ValueResolver/ContentValueResolverTest.php create mode 100644 tests/bundle/Core/ValueResolver/LocationValueResolverTest.php diff --git a/src/bundle/Core/Converter/ContentParamConverter.php b/src/bundle/Core/Converter/ContentParamConverter.php deleted file mode 100644 index 51082caab5..0000000000 --- a/src/bundle/Core/Converter/ContentParamConverter.php +++ /dev/null @@ -1,37 +0,0 @@ -contentService = $contentService; - } - - protected function getSupportedClass(): string - { - return Content::class; - } - - protected function getPropertyName(): string - { - return 'contentId'; - } - - protected function loadValueObject($id) - { - return $this->contentService->loadContent($id); - } -} diff --git a/src/bundle/Core/Converter/LocationParamConverter.php b/src/bundle/Core/Converter/LocationParamConverter.php deleted file mode 100644 index d2e74e0889..0000000000 --- a/src/bundle/Core/Converter/LocationParamConverter.php +++ /dev/null @@ -1,50 +0,0 @@ -locationService = $locationService; - $this->contentPreviewHelper = $contentPreviewHelper; - } - - protected function getSupportedClass(): string - { - return Location::class; - } - - protected function getPropertyName(): string - { - return 'locationId'; - } - - /** - * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException - * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException - */ - protected function loadValueObject($id): Location - { - $prioritizedLanguages = $this->contentPreviewHelper->isPreviewActive() ? Language::ALL : null; - - return $this->locationService->loadLocation($id, $prioritizedLanguages); - } -} diff --git a/src/bundle/Core/Converter/RepositoryParamConverter.php b/src/bundle/Core/Converter/RepositoryParamConverter.php deleted file mode 100644 index 23b064330c..0000000000 --- a/src/bundle/Core/Converter/RepositoryParamConverter.php +++ /dev/null @@ -1,54 +0,0 @@ -getClass(), $this->getSupportedClass(), true); - } - - abstract protected function getSupportedClass(); - - /** - * @return string property name used in the method of the controller needing param conversion - */ - abstract protected function getPropertyName(); - - /** - * @return \Ibexa\Contracts\Core\Repository\Values\ValueObject - */ - abstract protected function loadValueObject($id); - - /** - * @param \Symfony\Component\HttpFoundation\Request $request - * @param \Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter $configuration - * - * @return bool - */ - public function apply(Request $request, ParamConverter $configuration) - { - if (!$request->attributes->has($this->getPropertyName())) { - return false; - } - - $valueObjectId = $request->attributes->get($this->getPropertyName()); - if (!$valueObjectId && $configuration->isOptional()) { - return false; - } - - $request->attributes->set($configuration->getName(), $this->loadValueObject($valueObjectId)); - - return true; - } -} diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index 2fbd41d98c..ef4f19fead 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/src/bundle/Core/Resources/config/services.yml @@ -1,14 +1,11 @@ imports: - { resource: commands.yml } + - { resource: value_resolvers.yaml } parameters: ibexa.site_access.default.name: default ibexa.config.default_scope: ibexa.site_access.config - # Param converters - ibexa.param_converter.content.priority: -2 - ibexa.param_converter.location.priority: -2 - services: # Siteaccess is injected in the container at runtime Ibexa\Core\MVC\Symfony\SiteAccess: @@ -183,21 +180,6 @@ services: tags: - { name: kernel.fragment_renderer, alias: !php/const Ibexa\Bundle\Core\Fragment\DirectFragmentRenderer::NAME } - Ibexa\Bundle\Core\Converter\ContentParamConverter: - class: Ibexa\Bundle\Core\Converter\ContentParamConverter - arguments: - - '@ibexa.siteaccessaware.service.content' - tags: - - { name: request.param_converter, priority: '%ibexa.param_converter.content.priority%', converter: ez_content_converter } - - Ibexa\Bundle\Core\Converter\LocationParamConverter: - class: Ibexa\Bundle\Core\Converter\LocationParamConverter - arguments: - $locationService: '@ibexa.siteaccessaware.service.location' - $contentPreviewHelper: '@Ibexa\Core\Helper\ContentPreviewHelper' - tags: - - { name: request.param_converter, priority: '%ibexa.param_converter.location.priority%', converter: ez_location_converter } - Ibexa\Bundle\Core\ControllerArgumentResolver\LocationArgumentResolver: autowire: true autoconfigure: true diff --git a/src/bundle/Core/Resources/config/value_resolvers.yaml b/src/bundle/Core/Resources/config/value_resolvers.yaml new file mode 100644 index 0000000000..dbe9a251e4 --- /dev/null +++ b/src/bundle/Core/Resources/config/value_resolvers.yaml @@ -0,0 +1,11 @@ +services: + _defaults: + autowire: true + autoconfigure: true + public: false + + Ibexa\Bundle\Core\ValueResolver\: + resource: "../../ValueResolver/*" + tags: + - name: controller.argument_value_resolver + priority: -2 diff --git a/src/bundle/Core/ValueResolver/ContentValueResolver.php b/src/bundle/Core/ValueResolver/ContentValueResolver.php new file mode 100644 index 0000000000..2b0f2f558a --- /dev/null +++ b/src/bundle/Core/ValueResolver/ContentValueResolver.php @@ -0,0 +1,46 @@ + + * + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException + */ + public function resolve(Request $request, ArgumentMetadata $argument): iterable + { + if ($argument->getType() !== Content::class) { + return []; + } + + $contentId = $request->attributes->get(self::ATTRIBUTE_CONTENT_ID); + + if (!is_numeric($contentId)) { + return []; + } + + yield $this->contentService->loadContent((int)$contentId); + } +} diff --git a/src/bundle/Core/ValueResolver/LocationValueResolver.php b/src/bundle/Core/ValueResolver/LocationValueResolver.php new file mode 100644 index 0000000000..93578dae58 --- /dev/null +++ b/src/bundle/Core/ValueResolver/LocationValueResolver.php @@ -0,0 +1,51 @@ + + * + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException + */ + public function resolve(Request $request, ArgumentMetadata $argument): iterable + { + if ($argument->getType() !== Location::class) { + return []; + } + + $locationId = $request->attributes->get(self::ATTRIBUTE_LOCATION_ID); + + if (!is_numeric($locationId)) { + return []; + } + + $prioritizedLanguages = $this->contentPreviewHelper->isPreviewActive() ? Language::ALL : null; + + yield $this->locationService->loadLocation((int)$locationId, $prioritizedLanguages); + } +} diff --git a/tests/bundle/Core/Converter/AbstractParamConverterTest.php b/tests/bundle/Core/Converter/AbstractParamConverterTest.php deleted file mode 100644 index bd4bc6ce6a..0000000000 --- a/tests/bundle/Core/Converter/AbstractParamConverterTest.php +++ /dev/null @@ -1,36 +0,0 @@ -getMockBuilder(ParamConverter::class) - ->setMethods(['getClass', 'getAliasName', 'getOptions', 'getName', 'allowArray', 'isOptional']) - ->disableOriginalConstructor() - ->getMock(); - - if ($name !== null) { - $config->expects(self::any()) - ->method('getName') - ->will(self::returnValue($name)); - } - if ($class !== null) { - $config->expects(self::any()) - ->method('getClass') - ->will(self::returnValue($class)); - } - - return $config; - } -} diff --git a/tests/bundle/Core/Converter/ContentParamConverterTest.php b/tests/bundle/Core/Converter/ContentParamConverterTest.php deleted file mode 100644 index 5a018b4793..0000000000 --- a/tests/bundle/Core/Converter/ContentParamConverterTest.php +++ /dev/null @@ -1,75 +0,0 @@ -contentServiceMock = $this->createMock(ContentService::class); - $this->converter = new ContentParamConverter($this->contentServiceMock); - } - - public function testSupports() - { - $config = $this->createConfiguration(self::CONTENT_CLASS); - self::assertTrue($this->converter->supports($config)); - - $config = $this->createConfiguration(__CLASS__); - self::assertFalse($this->converter->supports($config)); - - $config = $this->createConfiguration(); - self::assertFalse($this->converter->supports($config)); - } - - public function testApplyContent() - { - $id = 42; - $valueObject = $this->createMock(Content::class); - - $this->contentServiceMock - ->expects(self::once()) - ->method('loadContent') - ->with($id) - ->will(self::returnValue($valueObject)); - - $request = new Request([], [], [self::PROPERTY_NAME => $id]); - $config = $this->createConfiguration(self::CONTENT_CLASS, 'content'); - - $this->converter->apply($request, $config); - - self::assertInstanceOf(self::CONTENT_CLASS, $request->attributes->get('content')); - } - - public function testApplyContentOptionalWithEmptyAttribute() - { - $request = new Request([], [], [self::PROPERTY_NAME => null]); - $config = $this->createConfiguration(self::CONTENT_CLASS, 'content'); - - $config->expects(self::once()) - ->method('isOptional') - ->will(self::returnValue(true)); - - self::assertFalse($this->converter->apply($request, $config)); - self::assertNull($request->attributes->get('content')); - } -} diff --git a/tests/bundle/Core/Converter/LocationParamConverterTest.php b/tests/bundle/Core/Converter/LocationParamConverterTest.php deleted file mode 100644 index 6bd287fb98..0000000000 --- a/tests/bundle/Core/Converter/LocationParamConverterTest.php +++ /dev/null @@ -1,78 +0,0 @@ -locationServiceMock = $this->createMock(LocationService::class); - $contentPreviewHelper = $this->createMock(ContentPreviewHelper::class); - - $this->converter = new LocationParamConverter($this->locationServiceMock, $contentPreviewHelper); - } - - public function testSupports() - { - $config = $this->createConfiguration(self::LOCATION_CLASS); - self::assertTrue($this->converter->supports($config)); - - $config = $this->createConfiguration(__CLASS__); - self::assertFalse($this->converter->supports($config)); - - $config = $this->createConfiguration(); - self::assertFalse($this->converter->supports($config)); - } - - public function testApplyLocation() - { - $id = 42; - $valueObject = $this->createMock(Location::class); - - $this->locationServiceMock - ->expects(self::once()) - ->method('loadLocation') - ->with($id) - ->will(self::returnValue($valueObject)); - - $request = new Request([], [], [self::PROPERTY_NAME => $id]); - $config = $this->createConfiguration(self::LOCATION_CLASS, 'location'); - - $this->converter->apply($request, $config); - - self::assertInstanceOf(self::LOCATION_CLASS, $request->attributes->get('location')); - } - - public function testApplyLocationOptionalWithEmptyAttribute() - { - $request = new Request([], [], [self::PROPERTY_NAME => null]); - $config = $this->createConfiguration(self::LOCATION_CLASS, 'location'); - - $config->expects(self::once()) - ->method('isOptional') - ->will(self::returnValue(true)); - - self::assertFalse($this->converter->apply($request, $config)); - self::assertNull($request->attributes->get('location')); - } -} diff --git a/tests/bundle/Core/ValueResolver/ContentValueResolverTest.php b/tests/bundle/Core/ValueResolver/ContentValueResolverTest.php new file mode 100644 index 0000000000..fdc46a07cf --- /dev/null +++ b/tests/bundle/Core/ValueResolver/ContentValueResolverTest.php @@ -0,0 +1,79 @@ +contentServiceMock = $this->createMock(ContentService::class); + $this->resolver = new ContentValueResolver($this->contentServiceMock); + } + + public function testResolveWithValidContentId(): void + { + $request = new Request([], [], ['contentId' => '123']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn(Content::class); + + $mockContent = $this->createMock(Content::class); + + $this->contentServiceMock + ->expects(self::once()) + ->method('loadContent') + ->with(123) + ->willReturn($mockContent); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([$mockContent], $result); + } + + public function testResolveWithInvalidContentId(): void + { + $request = new Request([], [], ['contentId' => 'invalid']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn(Content::class); + + $this->contentServiceMock + ->expects(self::never()) + ->method('loadContent'); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([], $result); + } + + public function testResolveWithNonContentType(): void + { + $request = new Request([], [], ['contentId' => '123']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn('OtherClass'); + + $this->contentServiceMock + ->expects(self::never()) + ->method('loadContent'); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([], $result); + } +} diff --git a/tests/bundle/Core/ValueResolver/LocationValueResolverTest.php b/tests/bundle/Core/ValueResolver/LocationValueResolverTest.php new file mode 100644 index 0000000000..692aa36727 --- /dev/null +++ b/tests/bundle/Core/ValueResolver/LocationValueResolverTest.php @@ -0,0 +1,91 @@ +locationServiceMock = $this->createMock(LocationService::class); + $this->contentPreviewHelperMock = $this->createMock(ContentPreviewHelper::class); + $this->resolver = new LocationValueResolver( + $this->locationServiceMock, + $this->contentPreviewHelperMock + ); + } + + public function testResolveWithValidLocationId(): void + { + $request = new Request([], [], ['locationId' => '456']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn(Location::class); + + $mockLocation = $this->createMock(Location::class); + + $this->contentPreviewHelperMock + ->method('isPreviewActive') + ->willReturn(true); + + $this->locationServiceMock + ->expects(self::once()) + ->method('loadLocation') + ->with(456, Language::ALL) + ->willReturn($mockLocation); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([$mockLocation], $result); + } + + public function testResolveWithInvalidLocationId(): void + { + $request = new Request([], [], ['locationId' => 'invalid']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn(Location::class); + + $this->locationServiceMock + ->expects(self::never()) + ->method('loadLocation'); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([], $result); + } + + public function testResolveWithNonLocationType(): void + { + $request = new Request([], [], ['locationId' => '456']); + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata->method('getType')->willReturn('OtherClass'); + + $this->locationServiceMock + ->expects(self::never()) + ->method('loadLocation'); + + $result = iterator_to_array($this->resolver->resolve($request, $argumentMetadata)); + + self::assertSame([], $result); + } +}