ProviderDataTransformer: throw TransformationFailedException on error (…

…#2444)
sonata-project · Mar 8, 2024 · ab5530f · ab5530f
1 parent c3c4cf3
commit ab5530f
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 36 deletions.
diff --git a/src/Form/DataTransformer/ProviderDataTransformer.php b/src/Form/DataTransformer/ProviderDataTransformer.php
@@ -15,16 +15,21 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
-use Psr\Log\NullLogger;
 use Sonata\MediaBundle\Model\MediaInterface;
 use Sonata\MediaBundle\Provider\Pool;
 use Symfony\Component\Form\DataTransformerInterface;
+use Symfony\Component\Form\Exception\TransformationFailedException;
 
 /**
  * @implements DataTransformerInterface<MediaInterface, MediaInterface>
+ *
+ * NEXT_MAJOR: remove LoggerAwareInterface interface
  */
 final class ProviderDataTransformer implements DataTransformerInterface, LoggerAwareInterface
 {
+    /**
+     * NEXT_MAJOR: remove this trait.
+     */
     use LoggerAwareTrait;
 
     /**
@@ -68,6 +73,8 @@ public function transform($value): mixed
     /**
      * @param mixed $value
      *
+     * @throws TransformationFailedException when the transformation fails
+     *
      * @phpstan-param MediaInterface|null $value
      * @phpstan-return MediaInterface|null
      */
@@ -116,15 +123,7 @@ public function reverseTransform($value): mixed
         try {
             $provider->transform($newMedia);
         } catch (\Throwable $e) {
-            $logger = $this->logger ?? new NullLogger();
-
-            // #1107 We must never throw an exception here.
-            // An exception here would prevent us to provide meaningful errors through the Form
-            // Error message inspired from Monolog\ErrorHandler
-            $logger->error(
-                sprintf('Caught Exception %s: "%s" at %s line %s', $e::class, $e->getMessage(), $e->getFile(), $e->getLine()),
-                ['exception' => $e]
-            );
+            throw new TransformationFailedException($e->getMessage(), 0, $e, $e->getMessage());
         }
 
         return $newMedia;

diff --git a/src/Form/Type/MediaType.php b/src/Form/Type/MediaType.php
@@ -15,7 +15,6 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
-use Psr\Log\NullLogger;
 use Sonata\MediaBundle\Form\DataTransformer\ProviderDataTransformer;
 use Sonata\MediaBundle\Model\MediaInterface;
 use Sonata\MediaBundle\Provider\Pool;
@@ -53,7 +52,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
             'empty_on_new' => $options['empty_on_new'],
             'new_on_update' => $options['new_on_update'],
         ]);
-        $dataTransformer->setLogger($this->logger ?? new NullLogger());
 
         $builder->addModelTransformer($dataTransformer);
 

diff --git a/tests/Form/DataTransformer/ProviderDataTransformerTest.php b/tests/Form/DataTransformer/ProviderDataTransformerTest.php
@@ -14,11 +14,11 @@
 namespace Sonata\MediaBundle\Tests\Form\DataTransformer;
 
 use PHPUnit\Framework\TestCase;
-use Psr\Log\LoggerInterface;
 use Sonata\MediaBundle\Form\DataTransformer\ProviderDataTransformer;
 use Sonata\MediaBundle\Model\MediaInterface;
 use Sonata\MediaBundle\Provider\MediaProviderInterface;
 use Sonata\MediaBundle\Provider\Pool;
+use Symfony\Component\Form\Exception\TransformationFailedException;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 
 class ProviderDataTransformerTest extends TestCase
@@ -124,36 +124,16 @@ public function testReverseTransformWithMediaAndUploadFileInstance(): void
         $transformer->reverseTransform($media);
     }
 
-    public function testReverseTransformWithThrowingProviderNoThrow(): void
+    public function testReverseTransformWithThrowingProviderThrowTransformationFailedException(): void
     {
-        $provider = $this->createMock(MediaProviderInterface::class);
-        $provider->expects(static::once())->method('transform')->will(static::throwException(new \Exception()));
-
-        $pool = new Pool('default');
-        $pool->addProvider('default', $provider);
-
-        $media = $this->createMock(MediaInterface::class);
-        $media->expects(static::exactly(3))->method('getProviderName')->willReturn('default');
-        $media->method('getId')->willReturn(1);
-        $media->method('getBinaryContent')->willReturn(new UploadedFile(__FILE__, 'ProviderDataTransformerTest'));
+        $this->expectException(TransformationFailedException::class);
 
-        $transformer = new ProviderDataTransformer($pool, MediaInterface::class, [
-            'new_on_update' => false,
-        ]);
-        $transformer->reverseTransform($media);
-    }
-
-    public function testReverseTransformWithThrowingProviderLogsException(): void
-    {
         $provider = $this->createMock(MediaProviderInterface::class);
         $provider->expects(static::once())->method('transform')->will(static::throwException(new \Exception()));
 
         $pool = new Pool('default');
         $pool->addProvider('default', $provider);
 
-        $logger = $this->createMock(LoggerInterface::class);
-        $logger->expects(static::once())->method('error');
-
         $media = $this->createMock(MediaInterface::class);
         $media->expects(static::exactly(3))->method('getProviderName')->willReturn('default');
         $media->method('getId')->willReturn(1);
@@ -162,7 +142,6 @@ public function testReverseTransformWithThrowingProviderLogsException(): void
         $transformer = new ProviderDataTransformer($pool, MediaInterface::class, [
             'new_on_update' => false,
         ]);
-        $transformer->setLogger($logger);
         $transformer->reverseTransform($media);
     }
 }