From fa285f473b7f357ceb1178f508ec93886007266b Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 1 Sep 2023 15:24:23 +0200 Subject: [PATCH 1/7] Added LocationArgumentResolver --- .../Converter/LocationArgumentResolver.php | 56 +++++++++++++++++++ src/bundle/Core/Resources/config/services.yml | 8 ++- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 src/bundle/Core/Converter/LocationArgumentResolver.php diff --git a/src/bundle/Core/Converter/LocationArgumentResolver.php b/src/bundle/Core/Converter/LocationArgumentResolver.php new file mode 100644 index 0000000000..b65dfd0d9e --- /dev/null +++ b/src/bundle/Core/Converter/LocationArgumentResolver.php @@ -0,0 +1,56 @@ +locationService = $locationService; + } + + public function supports(Request $request, ArgumentMetadata $argument): bool + { + return !$request->attributes->has(self::PARAMETER_LOCATION_ID) + && $request->query->has(self::PARAMETER_LOCATION_ID); + } + + /** + * @return iterable<\Ibexa\Contracts\Core\Repository\Values\Content\Location> + * + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + * @throws \Ibexa\Contracts\Core\Exception\InvalidArgumentException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException + */ + public function resolve(Request $request, ArgumentMetadata $argument): iterable + { + $locationId = $request->query->get(self::PARAMETER_LOCATION_ID); + if (!is_numeric($locationId)) { + throw new InvalidArgumentException( + 'locationId', + 'Expected numeric type, ' . get_debug_type($locationId) . ' given.' + ); + } + + yield $this->locationService->loadLocation((int)$locationId); + } +} diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index c3c6cf156c..f4f7187a59 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/src/bundle/Core/Resources/config/services.yml @@ -195,6 +195,12 @@ services: tags: - { name: request.param_converter, priority: '%ibexa.param_converter.location.priority%', converter: ez_location_converter } + Ibexa\Bundle\Core\Converter\LocationArgumentResolver: + autowire: true + autoconfigure: true + tags: + - { name: controller.argument_value_resolver, priority: '%ibexa.param_converter.location.priority%' } + Ibexa\Bundle\Core\EventListener\ExceptionListener: class: Ibexa\Bundle\Core\EventListener\ExceptionListener arguments: ["@translator"] @@ -361,5 +367,5 @@ services: $repositoryConfigurationProvider: '@Ibexa\Bundle\Core\ApiLoader\RepositoryConfigurationProvider' $defaultConnection: '%doctrine.default_connection%' $entityManagers: '%doctrine.entity_managers%' - + Ibexa\Bundle\Core\Translation\Policy\PolicyTranslationDefinitionProvider: ~ From fd372995e4f2097744ab7e59163b50eab9578103 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 1 Sep 2023 15:24:41 +0200 Subject: [PATCH 2/7] [Tests] Added test coverage --- .../LocationArgumentResolverTest.php | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 tests/bundle/Core/Converter/LocationArgumentResolverTest.php diff --git a/tests/bundle/Core/Converter/LocationArgumentResolverTest.php b/tests/bundle/Core/Converter/LocationArgumentResolverTest.php new file mode 100644 index 0000000000..50d120b662 --- /dev/null +++ b/tests/bundle/Core/Converter/LocationArgumentResolverTest.php @@ -0,0 +1,135 @@ +createMock(LocationService::class); + $this->locationArgumentResolver = new LocationArgumentResolver($locationService); + } + + /** + * @dataProvider provideDataForTestSupports + */ + public function testSupports( + bool $expected, + Request $request, + ArgumentMetadata $argumentMetadata + ): void { + self::assertSame( + $expected, + $this->locationArgumentResolver->supports( + $request, + $argumentMetadata + ) + ); + } + + public function testResolveThrowsInvalidArgumentException(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Argument \'locationId\' is invalid: Expected numeric type, string given.'); + + $generator = $this->locationArgumentResolver->resolve( + new Request( + [ + 'locationId' => 'foo', + ] + ), + $this->createMock(ArgumentMetadata::class) + ); + + self::assertInstanceOf(Generator::class, $generator); + + $generator->getReturn(); + } + + public function testResolve(): void + { + $resolvedArgumentsGenerator = $this->locationArgumentResolver->resolve( + $this->createRequest(true, false, 1), + $this->createMock(ArgumentMetadata::class) + ); + + self::assertInstanceOf(Generator::class, $resolvedArgumentsGenerator); + $resolvedArguments = iterator_to_array($resolvedArgumentsGenerator); + + self::assertCount(1, $resolvedArguments); + + $value = current($resolvedArguments); + self::assertInstanceOf( + Location::class, + $value + ); + } + + /** + * @return iterable + */ + public function provideDataForTestSupports(): iterable + { + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + + yield 'Supported - locationId passed to request query' => [ + true, + $this->createRequest(true, false, 1), + $argumentMetadata, + ]; + + yield 'Not supported - locationId passed to request attributes' => [ + false, + $this->createRequest(false, true, 1), + $argumentMetadata, + ]; + + yield 'Not supported - locationId passed to request attributes and query' => [ + false, + $this->createRequest(true, true, 1), + $argumentMetadata, + ]; + } + + private function createRequest( + bool $addToQuery, + bool $addToAttributes, + ?int $locationId = null + ): Request { + $request = Request::create('/'); + + if ($addToQuery) { + $request->query->set(self::PARAMETER_LOCATION_ID, $locationId); + } + + if ($addToAttributes) { + $request->attributes->set(self::PARAMETER_LOCATION_ID, $locationId); + } + + return $request; + } +} From 9cc613d40799c6a3895520b6c9339b3563f44333 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Mon, 4 Sep 2023 08:24:17 +0200 Subject: [PATCH 3/7] Update tests/bundle/Core/Converter/LocationArgumentResolverTest.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adam Wójs --- tests/bundle/Core/Converter/LocationArgumentResolverTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/bundle/Core/Converter/LocationArgumentResolverTest.php b/tests/bundle/Core/Converter/LocationArgumentResolverTest.php index 50d120b662..bfb0b845e0 100644 --- a/tests/bundle/Core/Converter/LocationArgumentResolverTest.php +++ b/tests/bundle/Core/Converter/LocationArgumentResolverTest.php @@ -4,6 +4,8 @@ * @copyright Copyright (C) Ibexa AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ +declare(strict_types=1); + namespace Ibexa\Tests\Bundle\Core\Converter; use Generator; From dbcb22d843d22d505365f57e73efdee16696506f Mon Sep 17 00:00:00 2001 From: ciastektk Date: Mon, 4 Sep 2023 09:52:19 +0200 Subject: [PATCH 4/7] Update src/bundle/Core/Converter/LocationArgumentResolver.php Co-authored-by: Andrew Longosz --- src/bundle/Core/Converter/LocationArgumentResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bundle/Core/Converter/LocationArgumentResolver.php b/src/bundle/Core/Converter/LocationArgumentResolver.php index b65dfd0d9e..c1905c6d80 100644 --- a/src/bundle/Core/Converter/LocationArgumentResolver.php +++ b/src/bundle/Core/Converter/LocationArgumentResolver.php @@ -38,7 +38,7 @@ public function supports(Request $request, ArgumentMetadata $argument): bool * @return iterable<\Ibexa\Contracts\Core\Repository\Values\Content\Location> * * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException - * @throws \Ibexa\Contracts\Core\Exception\InvalidArgumentException + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException * @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException */ public function resolve(Request $request, ArgumentMetadata $argument): iterable From 78f0ebddaa4bbd84c224c6e2b8c4c5fea9520970 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Mon, 4 Sep 2023 12:45:37 +0200 Subject: [PATCH 5/7] Moved LocationArgumentResolver to ControllerArgumentResolved namespace --- .../LocationArgumentResolver.php | 7 +++-- src/bundle/Core/Resources/config/services.yml | 4 +-- .../LocationArgumentResolverTest.php | 29 +++++++++++++++---- 3 files changed, 30 insertions(+), 10 deletions(-) rename src/bundle/Core/{Converter => ControllerArgumentResolver}/LocationArgumentResolver.php (88%) rename tests/bundle/Core/{Converter => ControllerArgumentResolver}/LocationArgumentResolverTest.php (81%) diff --git a/src/bundle/Core/Converter/LocationArgumentResolver.php b/src/bundle/Core/ControllerArgumentResolver/LocationArgumentResolver.php similarity index 88% rename from src/bundle/Core/Converter/LocationArgumentResolver.php rename to src/bundle/Core/ControllerArgumentResolver/LocationArgumentResolver.php index c1905c6d80..fe4dd8d226 100644 --- a/src/bundle/Core/Converter/LocationArgumentResolver.php +++ b/src/bundle/Core/ControllerArgumentResolver/LocationArgumentResolver.php @@ -6,10 +6,11 @@ */ declare(strict_types=1); -namespace Ibexa\Bundle\Core\Converter; +namespace Ibexa\Bundle\Core\ControllerArgumentResolver; use Ibexa\Contracts\Core\Exception\InvalidArgumentException; use Ibexa\Contracts\Core\Repository\LocationService; +use Ibexa\Contracts\Core\Repository\Values\Content\Location; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; @@ -30,7 +31,9 @@ public function __construct(LocationService $locationService) public function supports(Request $request, ArgumentMetadata $argument): bool { - return !$request->attributes->has(self::PARAMETER_LOCATION_ID) + return + Location::class === $argument->getType() + && !$request->attributes->has(self::PARAMETER_LOCATION_ID) && $request->query->has(self::PARAMETER_LOCATION_ID); } diff --git a/src/bundle/Core/Resources/config/services.yml b/src/bundle/Core/Resources/config/services.yml index f4f7187a59..e65c582af2 100644 --- a/src/bundle/Core/Resources/config/services.yml +++ b/src/bundle/Core/Resources/config/services.yml @@ -195,11 +195,11 @@ services: tags: - { name: request.param_converter, priority: '%ibexa.param_converter.location.priority%', converter: ez_location_converter } - Ibexa\Bundle\Core\Converter\LocationArgumentResolver: + Ibexa\Bundle\Core\ControllerArgumentResolver\LocationArgumentResolver: autowire: true autoconfigure: true tags: - - { name: controller.argument_value_resolver, priority: '%ibexa.param_converter.location.priority%' } + - { name: controller.argument_value_resolver, priority: 50 } Ibexa\Bundle\Core\EventListener\ExceptionListener: class: Ibexa\Bundle\Core\EventListener\ExceptionListener diff --git a/tests/bundle/Core/Converter/LocationArgumentResolverTest.php b/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php similarity index 81% rename from tests/bundle/Core/Converter/LocationArgumentResolverTest.php rename to tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php index bfb0b845e0..cb8fc641db 100644 --- a/tests/bundle/Core/Converter/LocationArgumentResolverTest.php +++ b/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php @@ -6,10 +6,10 @@ */ declare(strict_types=1); -namespace Ibexa\Tests\Bundle\Core\Converter; +namespace Ibexa\Tests\Bundle\Core\ControllerArgumentResolver; use Generator; -use Ibexa\Bundle\Core\Converter\LocationArgumentResolver; +use Ibexa\Bundle\Core\ControllerArgumentResolver\LocationArgumentResolver; use Ibexa\Contracts\Core\Exception\InvalidArgumentException; use Ibexa\Contracts\Core\Repository\LocationService; use Ibexa\Contracts\Core\Repository\Values\Content\Location; @@ -96,27 +96,44 @@ public function testResolve(): void */ public function provideDataForTestSupports(): iterable { - $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $locationBasedArgumentMetadata = $this->createArgumentMetadata(Location::class); yield 'Supported - locationId passed to request query' => [ true, $this->createRequest(true, false, 1), - $argumentMetadata, + $locationBasedArgumentMetadata, + ]; + + yield 'Not supported - type different than Ibexa\Contracts\Core\Repository\Values\Content\Location' => [ + false, + $this->createRequest(true, false, 1), + $this->createArgumentMetadata('foo'), ]; yield 'Not supported - locationId passed to request attributes' => [ false, $this->createRequest(false, true, 1), - $argumentMetadata, + $locationBasedArgumentMetadata, ]; yield 'Not supported - locationId passed to request attributes and query' => [ false, $this->createRequest(true, true, 1), - $argumentMetadata, + $locationBasedArgumentMetadata, ]; } + private function createArgumentMetadata(string $type): ArgumentMetadata + { + $argumentMetadata = $this->createMock(ArgumentMetadata::class); + $argumentMetadata + ->expects(self::atLeastOnce()) + ->method('getType') + ->willReturn($type); + + return $argumentMetadata; + } + private function createRequest( bool $addToQuery, bool $addToAttributes, From 48eba7a23403a9d84449cfa14348f81c5bde0156 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Wed, 6 Sep 2023 07:54:29 +0200 Subject: [PATCH 6/7] Dropped expects from ArgumentMetadata mock --- .../ControllerArgumentResolver/LocationArgumentResolverTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php b/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php index cb8fc641db..36a36c0bdc 100644 --- a/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php +++ b/tests/bundle/Core/ControllerArgumentResolver/LocationArgumentResolverTest.php @@ -127,7 +127,6 @@ private function createArgumentMetadata(string $type): ArgumentMetadata { $argumentMetadata = $this->createMock(ArgumentMetadata::class); $argumentMetadata - ->expects(self::atLeastOnce()) ->method('getType') ->willReturn($type); From 25ace1cfe0eb30846744e19170b6e32cba05ae68 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Thu, 19 Oct 2023 10:04:43 +0200 Subject: [PATCH 7/7] [PHPStan] Fixed errors --- phpstan-baseline.neon | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4180ef7d07..096e27786d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -15157,12 +15157,12 @@ parameters: - message: "#^Cannot access offset int\\|string on Ibexa\\\\Contracts\\\\Core\\\\Persistence\\\\Content\\\\Field\\.$#" - count: 3 + count: 1 path: src/lib/Persistence/Legacy/Content/FieldHandler.php - message: "#^Cannot access offset string on Ibexa\\\\Contracts\\\\Core\\\\Persistence\\\\Content\\\\Field\\.$#" - count: 4 + count: 2 path: src/lib/Persistence/Legacy/Content/FieldHandler.php - @@ -20400,11 +20400,6 @@ parameters: count: 1 path: src/lib/Repository/Mapper/ContentDomainMapper.php - - - message: "#^Cannot access offset mixed on iterable\\\\.$#" - count: 1 - path: src/lib/Repository/Mapper/ContentDomainMapper.php - - message: "#^Cannot call method error\\(\\) on Psr\\\\Log\\\\LoggerInterface\\|null\\.$#" count: 1 @@ -58395,11 +58390,6 @@ parameters: count: 3 path: tests/lib/Repository/Service/Mock/ContentTest.php - - - message: "#^Cannot access offset mixed on Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Field\\.$#" - count: 1 - path: tests/lib/Repository/Service/Mock/ContentTest.php - - message: "#^Cannot access offset string on Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Content\\\\Field\\.$#" count: 1