From 9d8cd5ce594e571b2c151eb4d84c1b5ae0405b59 Mon Sep 17 00:00:00 2001 From: Priyadi Iman Nurcahyo <1102197+priyadi@users.noreply.github.com> Date: Tue, 25 Jun 2024 20:11:21 +0700 Subject: [PATCH] fix: cache warming error on invalid class in transformer's `getSupportedTransformation` (#79) * fix: cache warming error on invalid class in transformer's `getSupportedTransformation` * bump psr/log dep --- composer.json | 1 + config/services.php | 4 +- config/tests.php | 11 +++ src/Exception/InvalidClassException.php | 32 +++++++ src/Mapping/Implementation/MappingFactory.php | 66 +++++++++++++- src/Util/TypeFactory.php | 3 +- .../InvalidTransformer/InvalidTransformer.php | 49 ++++++++++ tests/Service/TestLogger.php | 91 +++++++++++++++++++ 8 files changed, 253 insertions(+), 4 deletions(-) create mode 100644 src/Exception/InvalidClassException.php create mode 100644 tests/Fixtures/InvalidTransformer/InvalidTransformer.php create mode 100644 tests/Service/TestLogger.php diff --git a/composer.json b/composer.json index f2edee8..98bb9d6 100644 --- a/composer.json +++ b/composer.json @@ -53,6 +53,7 @@ "phpstan/phpstan-phpunit": "^1.3", "phpunit/phpunit": "^10.5", "psalm/plugin-phpunit": "^0.19.0", + "psr/log": "^3.0", "ramsey/uuid": "^3.0 || ^4.0", "symfony/framework-bundle": "^6.4 || ^7.0", "symfony/http-kernel": "^6.4 || ^7.0", diff --git a/config/services.php b/config/services.php index a9473ef..5c3b59d 100644 --- a/config/services.php +++ b/config/services.php @@ -11,6 +11,7 @@ * that was distributed with this source code. */ +use Psr\Log\LoggerInterface; use Ramsey\Uuid\UuidInterface; use Rekalogika\Mapper\Command\MappingCommand; use Rekalogika\Mapper\Command\TryCommand; @@ -204,7 +205,8 @@ ->set('rekalogika.mapper.mapping_factory', MappingFactory::class) ->args([ tagged_iterator('rekalogika.mapper.transformer', 'key'), - service('rekalogika.mapper.type_resolver') + service('rekalogika.mapper.type_resolver'), + service(LoggerInterface::class)->nullOnInvalid() ]); $services diff --git a/config/tests.php b/config/tests.php index 05cad79..2f3c1f6 100644 --- a/config/tests.php +++ b/config/tests.php @@ -11,8 +11,10 @@ * that was distributed with this source code. */ +use Psr\Log\LoggerInterface; use Rekalogika\Mapper\MapperInterface; use Rekalogika\Mapper\Tests\Common\TestKernel; +use Rekalogika\Mapper\Tests\Fixtures\InvalidTransformer\InvalidTransformer; use Rekalogika\Mapper\Tests\Fixtures\Money\MoneyToMoneyDtoTransformer; use Rekalogika\Mapper\Tests\Fixtures\ObjectMapper\MoneyObjectMapper; use Rekalogika\Mapper\Tests\Fixtures\ObjectMapper\PersonToPersonDtoMapper; @@ -26,6 +28,7 @@ use Rekalogika\Mapper\Tests\Fixtures\Remove\MemberDtoToMemberMapper; use Rekalogika\Mapper\Tests\Fixtures\Remove\MemberRepository; use Rekalogika\Mapper\Tests\Fixtures\TransformerOverride\OverrideTransformer; +use Rekalogika\Mapper\Tests\Service\TestLogger; use Rekalogika\Mapper\Transformer\Implementation\ScalarToScalarTransformer; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -68,6 +71,14 @@ ->args([ '$transformer' => service(ScalarToScalarTransformer::class), ]); + $services->set(InvalidTransformer::class) + ->tag('rekalogika.mapper.transformer'); + + $services->set(TestLogger::class) + ->decorate(LoggerInterface::class) + ->args([ + '$logger' => service('.inner'), + ]); $services->set(RememberingMapper::class) ->args([ diff --git a/src/Exception/InvalidClassException.php b/src/Exception/InvalidClassException.php new file mode 100644 index 0000000..afe8a83 --- /dev/null +++ b/src/Exception/InvalidClassException.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE file + * that was distributed with this source code. + */ + +namespace Rekalogika\Mapper\Exception; + +class InvalidClassException extends InvalidArgumentException +{ + public function __construct( + private string $class, + int $code = 0, + ?\Throwable $previous = null + ) { + $message = sprintf('Class "%s" does not exist.', $class); + + parent::__construct($message, $code, $previous); + } + + public function getClass(): string + { + return $this->class; + } +} diff --git a/src/Mapping/Implementation/MappingFactory.php b/src/Mapping/Implementation/MappingFactory.php index 7488e7d..9f947c4 100644 --- a/src/Mapping/Implementation/MappingFactory.php +++ b/src/Mapping/Implementation/MappingFactory.php @@ -13,6 +13,8 @@ namespace Rekalogika\Mapper\Mapping\Implementation; +use Psr\Log\LoggerInterface; +use Rekalogika\Mapper\Exception\InvalidClassException; use Rekalogika\Mapper\Mapping\Mapping; use Rekalogika\Mapper\Mapping\MappingFactoryInterface; use Rekalogika\Mapper\Transformer\AbstractTransformerDecorator; @@ -33,7 +35,8 @@ final class MappingFactory implements MappingFactoryInterface */ public function __construct( private iterable $transformers, - private TypeResolverInterface $typeResolver + private TypeResolverInterface $typeResolver, + private ?LoggerInterface $logger = null, ) { } @@ -66,7 +69,61 @@ private function addMapping( string $id, TransformerInterface $transformer ): void { - foreach ($transformer->getSupportedTransformation() as $typeMapping) { + try { + $supportedTransformation = $transformer->getSupportedTransformation(); + } catch (InvalidClassException $e) { + // if we get invalid class exception here, we ignore the transformer + + $this->logger?->warning( + 'Transformer "{transformer}" has a mapping involving an invalid class "{class}", ignoring the transformer.', + [ + 'transformer' => get_class($transformer), + 'class' => $e->getClass(), + ], + ); + + return; + } + + // convert to iterator, so that we can catch an exception and able to + // continue the iteration + + if (is_array($supportedTransformation)) { + $supportedTransformation = new \ArrayIterator($supportedTransformation); + } else { + $supportedTransformation = new \IteratorIterator($supportedTransformation); + } + + try { + $supportedTransformation->rewind(); + } catch (InvalidClassException $e) { + $this->logger?->warning( + 'Transformer "{transformer}" has a mapping involving an invalid class "{class}", ignoring the invalid mapping.', + [ + 'transformer' => get_class($transformer), + 'class' => $e->getClass(), + ], + ); + + // if the error happens here, we ignore + } + + while ($supportedTransformation->valid()) { + try { + $typeMapping = $supportedTransformation->current(); + } catch (InvalidClassException $e) { + $this->logger?->warning( + 'Transformer "{transformer}" has a mapping involving an invalid class "{class}", ignoring the invalid mapping.', + [ + 'transformer' => get_class($transformer), + 'class' => $e->getClass(), + ], + ); + + // if the error happens here, we continue to the next mapping + continue; + } + $sourceTypes = $this->getSimpleTypes($typeMapping->getSourceType()); $targetTypes = $this->getSimpleTypes($typeMapping->getTargetType()); $isVariantTargetType = $typeMapping->isVariantTargetType(); @@ -91,7 +148,12 @@ class: get_class($transformer), ); } } + + $supportedTransformation->next(); } + + // foreach ($supportedTransformation as $typeMapping) { + // } } /** diff --git a/src/Util/TypeFactory.php b/src/Util/TypeFactory.php index 6d76a29..3c7e42a 100644 --- a/src/Util/TypeFactory.php +++ b/src/Util/TypeFactory.php @@ -14,6 +14,7 @@ namespace Rekalogika\Mapper\Util; use Rekalogika\Mapper\Exception\InvalidArgumentException; +use Rekalogika\Mapper\Exception\InvalidClassException; use Rekalogika\Mapper\Transformer\MixedType; use Symfony\Component\PropertyInfo\Type; @@ -289,7 +290,7 @@ public static function objectOfClass(string $class): Type } if (!class_exists($class) && !interface_exists($class) && !enum_exists($class)) { - throw new InvalidArgumentException(sprintf('"%s" is not a valid class.', $class)); + throw new InvalidClassException($class); } return self::$instancesOfObjectOfClass[$class] = new Type( diff --git a/tests/Fixtures/InvalidTransformer/InvalidTransformer.php b/tests/Fixtures/InvalidTransformer/InvalidTransformer.php new file mode 100644 index 0000000..d7a7927 --- /dev/null +++ b/tests/Fixtures/InvalidTransformer/InvalidTransformer.php @@ -0,0 +1,49 @@ + + * + * For the full copyright and license information, please view the LICENSE file + * that was distributed with this source code. + */ + +namespace Rekalogika\Mapper\Tests\Fixtures\InvalidTransformer; + +use Rekalogika\Mapper\Context\Context; +use Rekalogika\Mapper\Exception\InvalidArgumentException; +use Rekalogika\Mapper\Transformer\TransformerInterface; +use Rekalogika\Mapper\Transformer\TypeMapping; +use Rekalogika\Mapper\Util\TypeFactory; +use Symfony\Component\PropertyInfo\Type; + +class InvalidTransformer implements TransformerInterface +{ + public function getSupportedTransformation(): iterable + { + /** + * @psalm-suppress InvalidClass + * @psalm-suppress UndefinedClass + * @psalm-suppress MixedArgument + */ + yield new TypeMapping( + // @phpstan-ignore-next-line + TypeFactory::objectOfClass(InvalidClass::class), + // @phpstan-ignore-next-line + TypeFactory::objectOfClass(AnotherInvalidClass::class) + ); + } + + public function transform( + mixed $source, + mixed $target, + ?Type $sourceType, + ?Type $targetType, + Context $context + ): mixed { + throw new InvalidArgumentException('Should never reach here'); + } +} diff --git a/tests/Service/TestLogger.php b/tests/Service/TestLogger.php new file mode 100644 index 0000000..149c7d6 --- /dev/null +++ b/tests/Service/TestLogger.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE file + * that was distributed with this source code. + */ + +namespace Rekalogika\Mapper\Tests\Service; + +use Psr\Log\LoggerInterface; + +class TestLogger implements LoggerInterface +{ + public function __construct(private LoggerInterface $logger) + { + } + + private function isSuppressed(string|\Stringable $message): bool + { + return str_contains((string)$message, 'has a mapping involving an invalid class'); + } + + public function emergency(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->emergency($message, $context); + } + } + + public function alert(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->alert($message, $context); + } + } + + public function critical(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->critical($message, $context); + } + } + + public function error(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->error($message, $context); + } + } + + public function warning(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->warning($message, $context); + } + } + + public function notice(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->notice($message, $context); + } + } + + public function info(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->info($message, $context); + } + } + + public function debug(string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->debug($message, $context); + } + } + + public function log($level, string|\Stringable $message, array $context = []): void + { + if (!$this->isSuppressed($message)) { + $this->logger->log($level, $message, $context); + } + } +}