improve API validation error messages (#2278)

config/packages/fos_rest.yaml

@@ -18,8 +18,6 @@ fos_rest:
         enabled: true
         map_exception_codes: true
         serializer_error_renderer: true
-        serialize_exceptions: false
-        exception_listener: false
         codes:
             'Symfony\Component\HttpKernel\Exception\BadRequestHttpException': 400
             'App\Validator\ValidationFailedException': 400

config/services.yaml

@@ -82,6 +82,9 @@ services:
         arguments:
             $dataDir: '%kimai.data_dir%'
 
+    App\API\Serializer\ValidationFailedExceptionErrorHandler:
+        arguments: ['@translator.default', '@fos_rest.serializer.flatten_exception_handler']
+
     # ================================================================================
     # DATABASE
     # ================================================================================

src/API/Serializer/ValidationFailedExceptionErrorHandler.php

@@ -10,9 +10,12 @@
 namespace App\API\Serializer;
 
 use App\Validator\ValidationFailedException;
+use FOS\RestBundle\Serializer\Normalizer\FlattenExceptionHandler;
+use JMS\Serializer\Context;
 use JMS\Serializer\GraphNavigatorInterface;
 use JMS\Serializer\Handler\SubscribingHandlerInterface;
-use JMS\Serializer\Visitor\SerializationVisitorInterface;
+use JMS\Serializer\JsonSerializationVisitor;
+use Symfony\Component\ErrorHandler\Exception\FlattenException;
 use Symfony\Component\Validator\ConstraintViolationInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -22,25 +25,52 @@ class ValidationFailedExceptionErrorHandler implements SubscribingHandlerInterfa
      * @var TranslatorInterface
      */
     private $translator;
+    /**
+     * @var FlattenExceptionHandler
+     */
+    private $exceptionHandler;
 
-    public function __construct(TranslatorInterface $translator)
+    public function __construct(TranslatorInterface $translator, FlattenExceptionHandler $exceptionHandler)
     {
         $this->translator = $translator;
+        $this->exceptionHandler = $exceptionHandler;
     }
 
     public static function getSubscribingMethods()
     {
         return [[
             'direction' => GraphNavigatorInterface::DIRECTION_SERIALIZATION,
-            'type' => ValidationFailedException::class,
+            'type' => FlattenException::class,
             'format' => 'json',
             'method' => 'serializeExceptionToJson',
+            'priority' => -1
+        ], [
+            'direction' => GraphNavigatorInterface::DIRECTION_SERIALIZATION,
+            'type' => ValidationFailedException::class,
+            'format' => 'json',
+            'method' => 'serializeValidationExceptionToJson',
+            'priority' => -1
         ]];
     }
 
-    public function serializeExceptionToJson(SerializationVisitorInterface $visitor, ValidationFailedException $exception, array $type)
+    public function serializeExceptionToJson(JsonSerializationVisitor $visitor, FlattenException $exception, array $type, Context $context)
+    {
+        if ($exception->getClass() !== ValidationFailedException::class) {
+            return $this->exceptionHandler->serializeToJson($visitor, $exception, $type, $context);
+        }
+
+        $original = $context->getAttribute('exception');
+        if ($original instanceof ValidationFailedException) {
+            return $this->serializeValidationExceptionToJson($visitor, $original, $type, $context);
+        }
+
+        return $this->exceptionHandler->serializeToJson($visitor, $exception, $type, $context);
+    }
+
+    public function serializeValidationExceptionToJson(JsonSerializationVisitor $visitor, ValidationFailedException $exception, array $type, Context $context)
     {
         $errors = [];
+
         /** @var ConstraintViolationInterface $error */
         foreach (iterator_to_array($exception->getViolations()) as $error) {
             $errors[$error->getPropertyPath()]['errors'][] = $this->getErrorMessage($error);

tests/API/Serializer/ValidationFailedExceptionErrorHandlerTest.php

@@ -11,9 +11,12 @@ namespace App\Tests\API\Serializer;
 
 use App\API\Serializer\ValidationFailedExceptionErrorHandler;
 use App\Validator\ValidationFailedException;
+use FOS\RestBundle\Serializer\Normalizer\FlattenExceptionHandler;
 use JMS\Serializer\GraphNavigatorInterface;
-use JMS\Serializer\Visitor\SerializationVisitorInterface;
+use JMS\Serializer\JsonSerializationVisitor;
+use JMS\Serializer\SerializationContext;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\ErrorHandler\Exception\FlattenException;
 use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\ConstraintViolationList;
 use Symfony\Contracts\Translation\TranslatorInterface;
@@ -27,21 +30,28 @@ class ValidationFailedExceptionErrorHandlerTest extends TestCase
     {
         self::assertEquals([[
             'direction' => GraphNavigatorInterface::DIRECTION_SERIALIZATION,
-            'type' => 'App\Validator\ValidationFailedException',
+            'type' => FlattenException::class,
             'format' => 'json',
             'method' => 'serializeExceptionToJson',
+            'priority' => -1
+        ], [
+            'direction' => GraphNavigatorInterface::DIRECTION_SERIALIZATION,
+            'type' => ValidationFailedException::class,
+            'format' => 'json',
+            'method' => 'serializeValidationExceptionToJson',
+            'priority' => -1
         ]], ValidationFailedExceptionErrorHandler::getSubscribingMethods());
     }
 
     public function testWithEmptyConstraintsList()
     {
         $translator = $this->createMock(TranslatorInterface::class);
-        $sut = new ValidationFailedExceptionErrorHandler($translator);
+        $handler = $this->createMock(FlattenExceptionHandler::class);
+        $sut = new ValidationFailedExceptionErrorHandler($translator, $handler);
 
         $constraints = new ConstraintViolationList();
         $validations = new ValidationFailedException($constraints, 'Uuups, that is broken');
 
-        $serialization = $this->createMock(SerializationVisitorInterface::class);
         $expected = [
             'code' => '400',
             'message' => null,
@@ -49,13 +59,30 @@ class ValidationFailedExceptionErrorHandlerTest extends TestCase
                 'children' => []
             ]
         ];
-        self::assertEquals($expected, $sut->serializeExceptionToJson($serialization, $validations, []));
+        self::assertEquals($expected, $sut->serializeValidationExceptionToJson(new JsonSerializationVisitor(), $validations, [], new SerializationContext()));
+    }
+
+    public function testWithUnsupportedException()
+    {
+        $translator = $this->createMock(TranslatorInterface::class);
+        $handler = $this->createMock(FlattenExceptionHandler::class);
+        $handler->method('serializeToJson')->willReturn('foooo');
+        $sut = new ValidationFailedExceptionErrorHandler($translator, $handler);
+        $actual = $sut->serializeExceptionToJson(
+            new JsonSerializationVisitor(),
+            FlattenException::createFromThrowable(new \Exception('sdfsdf')),
+            [],
+            new SerializationContext()
+        );
+
+        self::assertEquals('foooo', $actual);
     }
 
     public function testWithConstraintsList()
     {
         $translator = $this->createMock(TranslatorInterface::class);
-        $sut = new ValidationFailedExceptionErrorHandler($translator);
+        $handler = $this->createMock(FlattenExceptionHandler::class);
+        $sut = new ValidationFailedExceptionErrorHandler($translator, $handler);
         $translator->method('trans')->willReturnArgument(0);
 
         $constraints = new ConstraintViolationList();
@@ -64,7 +91,9 @@ class ValidationFailedExceptionErrorHandlerTest extends TestCase
         $constraints->add(new ConstraintViolation('missing tests', 'test %wuuf% 123', ['%wuuf%' => 'xcv'], '$root', 'end', 4, 3, null, null, '$cause'));
         $validations = new ValidationFailedException($constraints, 'Uuups, that is broken');
 
-        $serialization = $this->createMock(SerializationVisitorInterface::class);
+        $context = new SerializationContext();
+        $context->setAttribute('exception', $validations);
+
         $expected = [
             'code' => '400',
             'message' => 'Uuups, that is broken',
@@ -84,6 +113,36 @@ class ValidationFailedExceptionErrorHandlerTest extends TestCase
                 ]
             ]
         ];
-        self::assertEquals($expected, $sut->serializeExceptionToJson($serialization, $validations, []));
+        self::assertEquals($expected, $sut->serializeExceptionToJson(
+            new JsonSerializationVisitor(),
+            FlattenException::createFromThrowable($validations),
+            [],
+            $context
+        ));
+    }
+
+    public function testWithConstraintsListAndWrongException()
+    {
+        $translator = $this->createMock(TranslatorInterface::class);
+        $handler = $this->createMock(FlattenExceptionHandler::class);
+        $handler->method('serializeToJson')->willReturn('foooo');
+        $sut = new ValidationFailedExceptionErrorHandler($translator, $handler);
+        $translator->method('trans')->willReturnArgument(0);
+
+        $constraints = new ConstraintViolationList();
+        $constraints->add(new ConstraintViolation('toooo many tests', 'abc.def', [], '$root', 'begin', 4, null, null, null, '$cause'));
+        $constraints->add(new ConstraintViolation('missing tests', 'abc.def', [], '$root', 'begin', 4, null, null, null, '$cause'));
+        $constraints->add(new ConstraintViolation('missing tests', 'test %wuuf% 123', ['%wuuf%' => 'xcv'], '$root', 'end', 4, 3, null, null, '$cause'));
+        $validations = new ValidationFailedException($constraints, 'Uuups, that is broken');
+
+        $context = new SerializationContext();
+        $context->setAttribute('exception', new \Exception('sdfsdf'));
+
+        self::assertEquals('foooo', $sut->serializeExceptionToJson(
+            new JsonSerializationVisitor(),
+            FlattenException::createFromThrowable($validations),
+            [],
+            $context
+        ));
     }
 }