From bdafb7d3b1ca181aca9ce4d17cd549ca35a10f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Fri, 3 Nov 2023 08:47:58 +0100 Subject: [PATCH 1/6] Add ResourceToIdentifierCacheableTransformer form data transformer --- ...sourceToIdentifierCacheableTransformer.php | 82 +++++++++++++++++++ ...ceToIdentifierCacheableTransformerSpec.php | 78 ++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php create mode 100644 src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php diff --git a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php new file mode 100644 index 000000000..1d84b8096 --- /dev/null +++ b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php @@ -0,0 +1,82 @@ + $repository */ + private RepositoryInterface $repository; + + private string $identifier; + + /** @var array */ + private static array $cache; + + public function __construct(RepositoryInterface $repository, ?string $identifier = null) + { + $this->repository = $repository; + $this->identifier = $identifier ?? 'id'; + } + + /** + * @psalm-suppress MissingParamType + * + * @param object|null $value + */ + public function transform($value) + { + if (null === $value) { + return null; + } + + /** @psalm-suppress ArgumentTypeCoercion */ + Assert::isInstanceOf($value, $this->repository->getClassName()); + + return PropertyAccess::createPropertyAccessor()->getValue($value, $this->identifier); + } + + /** @param int|string|null $value */ + public function reverseTransform($value): ?ResourceInterface + { + if (null === $value) { + return null; + } + + if (isset(self::$cache[$value])) { + return self::$cache[$value]; + } + + /** @var ResourceInterface|null $resource */ + $resource = $this->repository->findOneBy([$this->identifier => $value]); + if (null === $resource) { + throw new TransformationFailedException(sprintf( + 'Object "%s" with identifier "%s"="%s" does not exist.', + $this->repository->getClassName(), + $this->identifier, + $value, + )); + } + + self::$cache[$value] = $resource; + + return $resource; + } +} diff --git a/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php new file mode 100644 index 000000000..2cfef21dd --- /dev/null +++ b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php @@ -0,0 +1,78 @@ +beConstructedWith($repository, 'id'); + } + + function it_does_not_reverses_null_value(RepositoryInterface $repository): void + { + $repository->findOneBy(Argument::any())->shouldNotBeCalled(); + + $this->reverseTransform(null)->shouldReturn(null); + } + + function it_throws_an_exception_on_non_existing_resource(RepositoryInterface $repository): void + { + $repository->getClassName()->willReturn(ResourceInterface::class); + $repository->findOneBy(['id' => 6])->willReturn(null); + + $this->shouldThrow(TransformationFailedException::class)->during('reverseTransform', [6]); + } + + function it_reverse_transform_identifier_to_resource(RepositoryInterface $repository, ResourceInterface $resource): void + { + $repository->findOneBy(['id' => 5])->willReturn($resource); + + $this->reverseTransform(5)->shouldReturn($resource); + } + + function it_reverse_transform_identifier_to_resource_using_cache( + RepositoryInterface $repository, + ResourceInterface $resource + ): void { + $repository->findOneBy(['id' => 5]) + ->willReturn($resource) + ->shouldBeCalledOnce(); + + $this->reverseTransform(5)->shouldReturn($resource); + $this->reverseTransform(5)->shouldReturn($resource); + } + + function it_transforms_null_value_to_empty_string(RepositoryInterface $repository): void + { + $repository->getClassName()->willReturn(ResourceInterface::class); + + $this->transform(null)->shouldReturn(null); + } + + function it_transforms_resource_in_identifier(RepositoryInterface $repository, ResourceInterface $resource): void + { + $repository->getClassName()->willReturn(ResourceInterface::class); + + $resource->getId()->willReturn(6); + + $this->transform($resource)->shouldReturn(6); + } +} From 05f4d6f6b307c4efd22ca59c9de1351b9880d350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 20 Nov 2023 11:10:14 +0100 Subject: [PATCH 2/6] Trying to fix Psalm issues --- .../ResourceToIdentifierCacheableTransformer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php index 1d84b8096..6631e046d 100644 --- a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php +++ b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php @@ -39,7 +39,7 @@ public function __construct(RepositoryInterface $repository, ?string $identifier /** * @psalm-suppress MissingParamType * - * @param object|null $value + * @param mixed $value */ public function transform($value) { @@ -53,7 +53,7 @@ public function transform($value) return PropertyAccess::createPropertyAccessor()->getValue($value, $this->identifier); } - /** @param int|string|null $value */ + /** @param mixed $value */ public function reverseTransform($value): ?ResourceInterface { if (null === $value) { From 363700b7702a77f26546592612f78fda1dc504c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 20 Nov 2023 11:11:53 +0100 Subject: [PATCH 3/6] Fix ECS issues --- .../ResourceToIdentifierCacheableTransformer.php | 4 ++-- .../ResourceToIdentifierCacheableTransformerSpec.php | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php index 6631e046d..1b0c0f2f1 100644 --- a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php +++ b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php @@ -3,7 +3,7 @@ /* * This file is part of the Sylius package. * - * (c) Sylius Sp. z o.o. + * (c) Paweł Jędrzejewski * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -22,7 +22,7 @@ final class ResourceToIdentifierCacheableTransformer implements DataTransformerInterface { - /** @var RepositoryInterface $repository */ + /** @var RepositoryInterface */ private RepositoryInterface $repository; private string $identifier; diff --git a/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php index 2cfef21dd..25503333b 100644 --- a/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php +++ b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php @@ -3,7 +3,7 @@ /* * This file is part of the Sylius package. * - * (c) Sylius Sp. z o.o. + * (c) Paweł Jędrzejewski * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -50,11 +50,12 @@ function it_reverse_transform_identifier_to_resource(RepositoryInterface $reposi function it_reverse_transform_identifier_to_resource_using_cache( RepositoryInterface $repository, - ResourceInterface $resource + ResourceInterface $resource, ): void { $repository->findOneBy(['id' => 5]) ->willReturn($resource) - ->shouldBeCalledOnce(); + ->shouldBeCalledOnce() + ; $this->reverseTransform(5)->shouldReturn($resource); $this->reverseTransform(5)->shouldReturn($resource); From 0e5a9bd42ed76ea413fc73e46ac90746636f3168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 20 Nov 2023 11:17:19 +0100 Subject: [PATCH 4/6] Fix PHPStan issues --- .../ResourceToIdentifierCacheableTransformer.php | 1 - src/Bundle/Storage/CookieStorage.php | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php index 1b0c0f2f1..6f01215e8 100644 --- a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php +++ b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php @@ -22,7 +22,6 @@ final class ResourceToIdentifierCacheableTransformer implements DataTransformerInterface { - /** @var RepositoryInterface */ private RepositoryInterface $repository; private string $identifier; diff --git a/src/Bundle/Storage/CookieStorage.php b/src/Bundle/Storage/CookieStorage.php index f259b8e30..d895f3322 100644 --- a/src/Bundle/Storage/CookieStorage.php +++ b/src/Bundle/Storage/CookieStorage.php @@ -44,10 +44,13 @@ public static function getSubscribedEvents(): array public function onKernelRequest(RequestEvent $event): void { + $isMainRequest = false; + /** @phpstan-ignore-next-line */ if (\method_exists($event, 'isMainRequest')) { $isMainRequest = $event->isMainRequest(); - } else { + /** @phpstan-ignore-next-line */ + } else if(\method_exists($event, 'isMasterRequest')) { $isMainRequest = $event->isMasterRequest(); } if (!$isMainRequest) { @@ -60,10 +63,13 @@ public function onKernelRequest(RequestEvent $event): void public function onKernelResponse(ResponseEvent $event): void { + $isMainRequest = false; + /** @phpstan-ignore-next-line */ if (\method_exists($event, 'isMainRequest')) { $isMainRequest = $event->isMainRequest(); - } else { + /** @phpstan-ignore-next-line */ + } else if(\method_exists($event, 'isMasterRequest')) { $isMainRequest = $event->isMasterRequest(); } if (!$isMainRequest) { From 9efbb6baac1519f0b75f82a15c7abe979a5f9833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 20 Nov 2023 11:18:41 +0100 Subject: [PATCH 5/6] Fix ECS issues --- src/Bundle/Storage/CookieStorage.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bundle/Storage/CookieStorage.php b/src/Bundle/Storage/CookieStorage.php index d895f3322..e797da11a 100644 --- a/src/Bundle/Storage/CookieStorage.php +++ b/src/Bundle/Storage/CookieStorage.php @@ -50,7 +50,7 @@ public function onKernelRequest(RequestEvent $event): void if (\method_exists($event, 'isMainRequest')) { $isMainRequest = $event->isMainRequest(); /** @phpstan-ignore-next-line */ - } else if(\method_exists($event, 'isMasterRequest')) { + } elseif (\method_exists($event, 'isMasterRequest')) { $isMainRequest = $event->isMasterRequest(); } if (!$isMainRequest) { @@ -69,7 +69,7 @@ public function onKernelResponse(ResponseEvent $event): void if (\method_exists($event, 'isMainRequest')) { $isMainRequest = $event->isMainRequest(); /** @phpstan-ignore-next-line */ - } else if(\method_exists($event, 'isMasterRequest')) { + } elseif (\method_exists($event, 'isMasterRequest')) { $isMainRequest = $event->isMasterRequest(); } if (!$isMainRequest) { From b04a64a5e116a2a00f6234ad96c515201bdc29b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Kukli=C5=84ski?= Date: Mon, 20 Nov 2023 11:42:50 +0100 Subject: [PATCH 6/6] Fix PHPSpec issues --- .../ResourceToIdentifierCacheableTransformer.php | 5 +++++ .../ResourceToIdentifierCacheableTransformerSpec.php | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php index 6f01215e8..bd33396f7 100644 --- a/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php +++ b/src/Bundle/Form/DataTransformer/ResourceToIdentifierCacheableTransformer.php @@ -78,4 +78,9 @@ public function reverseTransform($value): ?ResourceInterface return $resource; } + + public function clear(): void + { + self::$cache = []; + } } diff --git a/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php index 25503333b..bd956f3e9 100644 --- a/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php +++ b/src/Bundle/spec/Form/DataTransformer/ResourceToIdentifierCacheableTransformerSpec.php @@ -48,10 +48,12 @@ function it_reverse_transform_identifier_to_resource(RepositoryInterface $reposi $this->reverseTransform(5)->shouldReturn($resource); } - function it_reverse_transform_identifier_to_resource_using_cache( + function it_reverse_transforms_identifier_to_resource_using_cache( RepositoryInterface $repository, ResourceInterface $resource, ): void { + $this->clear(); + $repository->findOneBy(['id' => 5]) ->willReturn($resource) ->shouldBeCalledOnce()