fix: cache warming error on invalid class in transformer's `getSuppor…

…tedTransformation` (#79) * fix: cache warming error on invalid class in transformer's `getSupportedTransformation` * bump psr/log dep
rekalogika · Jun 25, 2024 · 9d8cd5c · 9d8cd5c
1 parent fe3e23b
commit 9d8cd5c
Show file tree

Hide file tree

Showing 8 changed files with 253 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * 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
@@ -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
@@ -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
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * 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
@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of rekalogika/mapper package.
+ *
+ * (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
+ *
+ * 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);
+        }
+    }
+}