From ec4dbb585a7a43a89d87950badb2504ba597f96c Mon Sep 17 00:00:00 2001
From: BacLuc <lucius.bachmann@gmx.ch>
Date: Thu, 9 Jan 2025 22:24:21 +0100
Subject: [PATCH] api: set rfc_7807_compliant_errors to true

---
 api/config/packages/api_platform.yaml         |   2 +-
 api/config/services.yaml                      |   6 +-
 api/src/DTO/ValidationError.php               |  30 +++
 .../TranslationInfoOfConstraintViolation.php  |   4 +-
 ...ationConstraintViolationListNormalizer.php |  85 -------
 .../Normalizer/ValidationErrorNormalizer.php  |  35 +++
 api/src/State/ValidationErrorProvider.php     |  74 ++++++
 ...ViolationListNormalizerIntegrationTest.php | 219 ------------------
 8 files changed, 145 insertions(+), 310 deletions(-)
 create mode 100644 api/src/DTO/ValidationError.php
 delete mode 100644 api/src/Serializer/Normalizer/TranslationConstraintViolationListNormalizer.php
 create mode 100644 api/src/Serializer/Normalizer/ValidationErrorNormalizer.php
 create mode 100644 api/src/State/ValidationErrorProvider.php
 delete mode 100644 api/tests/Integration/Serializer/Normalizer/TranslationConstraintViolationListNormalizerIntegrationTest.php

diff --git a/api/config/packages/api_platform.yaml b/api/config/packages/api_platform.yaml
index 28980e4b83..92dc27e7b0 100644
--- a/api/config/packages/api_platform.yaml
+++ b/api/config/packages/api_platform.yaml
@@ -30,7 +30,7 @@ api_platform:
         stateless: true
         extra_properties:
             standard_put: true
-            rfc_7807_compliant_errors: false
+            rfc_7807_compliant_errors: true
         pagination_enabled: false
         itemOperations: [ 'get', 'patch', 'delete' ]
         collection_operations:
diff --git a/api/config/services.yaml b/api/config/services.yaml
index 7c2d7d2558..aa6cafb29d 100644
--- a/api/config/services.yaml
+++ b/api/config/services.yaml
@@ -73,10 +73,10 @@ services:
     App\Serializer\Normalizer\CollectionItemsNormalizer:
         decorates: 'api_platform.hal.normalizer.collection'
 
-    App\Serializer\Normalizer\TranslationConstraintViolationListNormalizer:
+    App\State\ValidationErrorProvider:
+        decorates: 'api_platform.validator.state.error_provider'
         arguments:
-            - '@api_platform.hydra.normalizer.constraint_violation_list'
-            - '@api_platform.problem.normalizer.constraint_violation_list'
+            - '@.inner'
 
     App\Serializer\SerializerContextBuilder:
         decorates: 'api_platform.serializer.context_builder'
diff --git a/api/src/DTO/ValidationError.php b/api/src/DTO/ValidationError.php
new file mode 100644
index 0000000000..23abdc2022
--- /dev/null
+++ b/api/src/DTO/ValidationError.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace App\DTO;
+
+use ApiPlatform\ApiResource\Error;
+use ApiPlatform\Metadata\ApiProperty;
+
+class ValidationError extends Error {
+    public function __construct(
+        private readonly ?string $title,
+        private readonly int $status,
+        private readonly ?string $detail,
+        private readonly ?string $instance,
+        private readonly array $violations = [],
+        private readonly string $type = 'about:blank',
+    ) {
+        parent::__construct(
+            type: $this->type,
+            title: $this->title,
+            status: $this->status,
+            detail: $this->detail,
+            instance: $this->instance,
+        );
+    }
+
+    #[ApiProperty]
+    public function getViolations(): array {
+        return $this->violations;
+    }
+}
diff --git a/api/src/Serializer/Normalizer/Error/TranslationInfoOfConstraintViolation.php b/api/src/Serializer/Normalizer/Error/TranslationInfoOfConstraintViolation.php
index 31765f3353..dd8e968ed0 100644
--- a/api/src/Serializer/Normalizer/Error/TranslationInfoOfConstraintViolation.php
+++ b/api/src/Serializer/Normalizer/Error/TranslationInfoOfConstraintViolation.php
@@ -2,10 +2,10 @@
 
 namespace App\Serializer\Normalizer\Error;
 
-use Symfony\Component\Validator\ConstraintViolation;
+use Symfony\Component\Validator\ConstraintViolationInterface;
 
 class TranslationInfoOfConstraintViolation {
-    public function extract(ConstraintViolation $constraintViolation): TranslationInfo {
+    public function extract(ConstraintViolationInterface $constraintViolation): TranslationInfo {
         $constraint = $constraintViolation->getConstraint();
         $constraintClass = get_class($constraint);
         $key = str_replace('\\', '.', $constraintClass);
diff --git a/api/src/Serializer/Normalizer/TranslationConstraintViolationListNormalizer.php b/api/src/Serializer/Normalizer/TranslationConstraintViolationListNormalizer.php
deleted file mode 100644
index 64acb29eca..0000000000
--- a/api/src/Serializer/Normalizer/TranslationConstraintViolationListNormalizer.php
+++ /dev/null
@@ -1,85 +0,0 @@
-<?php
-
-/** @noinspection PhpInternalEntityUsedInspection */
-
-namespace App\Serializer\Normalizer;
-
-use ApiPlatform\Serializer\AbstractConstraintViolationListNormalizer;
-use App\Serializer\Normalizer\Error\TranslationInfoOfConstraintViolation;
-use App\Service\TranslateToAllLocalesService;
-use Doctrine\Common\Collections\ArrayCollection;
-use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
-use Symfony\Component\Validator\ConstraintViolation;
-use Symfony\Component\Validator\ConstraintViolationList;
-
-class TranslationConstraintViolationListNormalizer implements NormalizerInterface {
-    public function __construct(
-        private readonly AbstractConstraintViolationListNormalizer $hydraNormalizer,
-        private readonly AbstractConstraintViolationListNormalizer $problemNormalizer,
-        private readonly TranslationInfoOfConstraintViolation $translationInfoOfConstraintViolation,
-        private readonly TranslateToAllLocalesService $translateToAllLocalesService
-    ) {}
-
-    public function supportsNormalization(mixed $data, ?string $format = null, array $context = []): bool {
-        return $this->getNormalizerCollection()->exists(fn ($_, $elem) => $elem->supportsNormalization($data, $format, $context));
-    }
-
-    public function normalize(mixed $data, ?string $format = null, array $context = []): null|array|\ArrayObject|bool|float|int|string {
-        $normalizer = $this->getNormalizerCollection()->filter(fn ($elem) => $elem->supportsNormalization($data, $format, $context))->first();
-        if (false === $normalizer) {
-            throw new \RuntimeException("Did not find a normalizer to normalize response to format {$format}");
-        }
-        $result = $normalizer->normalize($data, $format, $context);
-
-        /** @var ConstraintViolationList $data */
-        foreach ($data as $violation) {
-            foreach ($result['violations'] as &$resultItem) {
-                $code = $resultItem['code'] ?? null;
-                $propertyPath = $resultItem['propertyPath'];
-                $message = $resultItem['message'] ?? null;
-
-                if ($violation->getPropertyPath() === $propertyPath
-                    && $violation->getCode() === $code
-                    && $violation->getMessage() == $message) {
-                    $translationInfo = [];
-                    if ($violation instanceof ConstraintViolation) {
-                        $translationInfo = $this->translationInfoOfConstraintViolation->extract($violation);
-                    }
-
-                    $translations = $this->translateToAllLocalesService->translate(
-                        $violation->getMessageTemplate(),
-                        array_merge(
-                            $violation->getPlural() ? ['%count%' => $violation->getPlural()] : [],
-                            $violation->getParameters()
-                        )
-                    );
-
-                    $resultItem = [
-                        'i18n' => [
-                            ...(array) $translationInfo,
-                            'translations' => $translations,
-                        ],
-                        ...$resultItem,
-                    ];
-
-                    break;
-                }
-            }
-        }
-
-        return $result;
-    }
-
-    public function getSupportedTypes(?string $format): array {
-        return $this->getNormalizerCollection()
-            ->map(function (AbstractConstraintViolationListNormalizer $normalizer) use ($format) {
-                return $normalizer->getSupportedTypes($format);
-            })
-            ->reduce(fn (?array $left, array $right) => array_merge($left ?? [], $right), [])
-        ;
-    }
-
-    private function getNormalizerCollection(): ArrayCollection {
-        return new ArrayCollection([$this->hydraNormalizer, $this->problemNormalizer]);
-    }
-}
diff --git a/api/src/Serializer/Normalizer/ValidationErrorNormalizer.php b/api/src/Serializer/Normalizer/ValidationErrorNormalizer.php
new file mode 100644
index 0000000000..083a687d81
--- /dev/null
+++ b/api/src/Serializer/Normalizer/ValidationErrorNormalizer.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace App\Serializer\Normalizer;
+
+use ApiPlatform\Problem\Serializer\ErrorNormalizerTrait;
+use App\DTO\ValidationError;
+use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
+
+class ValidationErrorNormalizer implements NormalizerInterface {
+    use ErrorNormalizerTrait;
+
+    public const TYPE = 'type';
+    public const TITLE = 'title';
+    private array $defaultContext = [
+        self::TYPE => 'https://tools.ietf.org/html/rfc2616#section-10',
+        self::TITLE => 'An error occurred',
+    ];
+
+    public function normalize(mixed $data, ?string $format = null, array $context = []): null|array|\ArrayObject|bool|float|int|string {
+        return [
+            'type' => $context[self::TYPE] ?? $this->defaultContext[self::TYPE],
+            'title' => $context[self::TITLE] ?? $this->defaultContext[self::TITLE],
+            'detail' => $data->getDetail(),
+            'violations' => $data->getViolations(),
+        ];
+    }
+
+    public function supportsNormalization(mixed $data, ?string $format = null, array $context = []): bool {
+        return $data instanceof ValidationError;
+    }
+
+    public function getSupportedTypes(?string $format): array {
+        return [ValidationError::class => true];
+    }
+}
diff --git a/api/src/State/ValidationErrorProvider.php b/api/src/State/ValidationErrorProvider.php
new file mode 100644
index 0000000000..a5b3a6b472
--- /dev/null
+++ b/api/src/State/ValidationErrorProvider.php
@@ -0,0 +1,74 @@
+<?php
+
+namespace App\State;
+
+use ApiPlatform\Metadata\HttpOperation;
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ApiResource\Error;
+use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\Validator\Exception\ValidationException;
+use App\DTO\ValidationError;
+use App\Serializer\Normalizer\Error\TranslationInfoOfConstraintViolation;
+use App\Service\TranslateToAllLocalesService;
+
+/**
+ * @psalm-suppress MissingTemplateParam
+ */
+class ValidationErrorProvider implements ProviderInterface {
+    public function __construct(
+        private readonly ProviderInterface $decorated,
+        private readonly TranslationInfoOfConstraintViolation $translationInfoOfConstraintViolation,
+        private readonly TranslateToAllLocalesService $translateToAllLocalesService
+    ) {}
+
+    /**
+     * @psalm-suppress InvalidReturnStatement
+     */
+    public function provide(Operation $operation, array $uriVariables = [], array $context = []): null|array|object {
+        $request = $context['request'];
+        $exception = $request?->attributes->get('exception');
+        if (!($request ?? null) || !$operation instanceof HttpOperation || null === $exception) {
+            throw new \RuntimeException('Not an HTTP request');
+        }
+
+        $status = $operation->getStatus() ?? 500;
+        $error = Error::createFromException($exception, $status);
+        if (!$exception instanceof ValidationException) {
+            return $this->decorated->provide($operation, $uriVariables, $context);
+        }
+
+        /**
+         * @var ValidationException $exception
+         */
+        $violationInfos = [];
+        foreach ($exception->getConstraintViolationList() as $violation) {
+            $violationInfo = $this->translationInfoOfConstraintViolation->extract($violation);
+            $translations = $this->translateToAllLocalesService->translate(
+                $violation->getMessageTemplate(),
+                array_merge(
+                    $violation->getPlural() ? ['%count%' => $violation->getPlural()] : [],
+                    $violation->getParameters()
+                )
+            );
+
+            $violationInfos[] = [
+                'code' => $violation->getCode(),
+                'propertyPath' => $violation->getPropertyPath(),
+                'message' => $violation->getMessage(),
+                'i18n' => [
+                    ...(array) $violationInfo,
+                    'translations' => $translations,
+                ],
+            ];
+        }
+
+        return new ValidationError(
+            type: $error->getType(),
+            title: $error->getTitle(),
+            status: $status,
+            detail: $error->getDetail(),
+            instance: $error->getInstance(),
+            violations: $violationInfos
+        );
+    }
+}
diff --git a/api/tests/Integration/Serializer/Normalizer/TranslationConstraintViolationListNormalizerIntegrationTest.php b/api/tests/Integration/Serializer/Normalizer/TranslationConstraintViolationListNormalizerIntegrationTest.php
deleted file mode 100644
index 00e3849a4d..0000000000
--- a/api/tests/Integration/Serializer/Normalizer/TranslationConstraintViolationListNormalizerIntegrationTest.php
+++ /dev/null
@@ -1,219 +0,0 @@
-<?php
-
-namespace App\Tests\Integration\Serializer\Normalizer;
-
-use ApiPlatform\Hydra\Serializer\ConstraintViolationListNormalizer as HydraConstraintViolationListNormalizer;
-use ApiPlatform\Problem\Serializer\ConstraintViolationListNormalizer as JsonProblemConstraintViolationListNormalizer;
-use ApiPlatform\Symfony\Bundle\Test\ApiTestAssertionsTrait;
-use App\Entity\CampCollaboration;
-use App\Serializer\Normalizer\TranslationConstraintViolationListNormalizer;
-use App\Validator\AllowTransition\AssertAllowTransitions;
-use PHPUnit\Framework\Attributes\DataProvider;
-use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
-use Symfony\Component\Serializer\Exception\ExceptionInterface;
-use Symfony\Component\Validator\Constraint;
-use Symfony\Component\Validator\Constraints\NotBlank;
-use Symfony\Component\Validator\Constraints\NotNull;
-use Symfony\Component\Validator\ConstraintViolation;
-use Symfony\Component\Validator\ConstraintViolationList;
-
-/**
- * @internal
- */
-class TranslationConstraintViolationListNormalizerIntegrationTest extends KernelTestCase {
-    use ApiTestAssertionsTrait;
-
-    private TranslationConstraintViolationListNormalizer $translationConstraintViolationListNormalizer;
-
-    /**
-     * @throws \Exception
-     */
-    protected function setUp(): void {
-        self::bootKernel();
-        parent::setUp();
-
-        /** @var TranslationConstraintViolationListNormalizer $obj */
-        $obj = self::getContainer()->get('App\Serializer\Normalizer\TranslationConstraintViolationListNormalizer');
-        $this->translationConstraintViolationListNormalizer = $obj;
-    }
-
-    /**
-     * @throws ExceptionInterface
-     * @throws \Exception
-     */
-    #[DataProvider('getFormats')]
-    public function testAddsTranslationKeyAndParameters(string $format) {
-        $constraintViolationList = new ConstraintViolationList(self::getConstraintViolations());
-
-        $result = $this->translationConstraintViolationListNormalizer->normalize(
-            $constraintViolationList,
-            $format,
-            ['api_error_resource' => true]
-        );
-
-        self::assertArraySubset(['violations' => [
-            [
-                'i18n' => [
-                    'key' => 'app.validator.allowtransition.assertallowtransitions',
-                    'parameters' => [
-                        'to' => 'inactive',
-                        'value' => 'established',
-                    ],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'key' => 'symfony.component.validator.constraints.notblank',
-                    'parameters' => [
-                        'value' => '""',
-                    ],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'key' => 'symfony.component.validator.constraints.notnull',
-                    'parameters' => [],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'key' => 'app.tests.integration.serializer.normalizer.myconstraint',
-                    'parameters' => [],
-                ],
-            ],
-        ]], $result);
-    }
-
-    /**
-     * @throws ExceptionInterface
-     * @throws \Exception
-     */
-    #[DataProvider('getFormats')]
-    public function testAddsTranslations(string $format) {
-        $constraintViolationList = new ConstraintViolationList(self::getConstraintViolations());
-
-        $result = $this->translationConstraintViolationListNormalizer->normalize(
-            $constraintViolationList,
-            $format,
-            ['api_error_resource' => true]
-        );
-
-        self::assertArraySubset(['violations' => [
-            [
-                'i18n' => [
-                    'translations' => [
-                        'en' => 'value must be one of inactive, was established',
-                        'de' => 'Der Wert muss einer aus inactive sein, aber er war established',
-                        'fr' => 'La valeur doit être l\'une des suivantes : inactive, a été established',
-                        'it' => 'deve essere uno dei seguenti valori: inactive, è established',
-                    ],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'translations' => [
-                        'en' => 'This value should not be blank.',
-                        'de' => 'Dieser Wert sollte nicht leer sein.',
-                        'fr' => 'Cette valeur ne doit pas être vide.',
-                        'it' => 'Questo valore non dovrebbe essere vuoto.',
-                    ],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'translations' => [
-                        'en' => 'This value should not be null.',
-                        'de' => 'Dieser Wert sollte nicht null sein.',
-                        'fr' => 'Cette valeur ne doit pas être nulle.',
-                        'it' => 'Questo valore non dovrebbe essere nullo.',
-                    ],
-                ],
-            ],
-            [
-                'i18n' => [
-                    'translations' => [
-                        'en' => 'en',
-                        'en_CH_scout' => 'en_CH_scout',
-                        'de' => 'de',
-                        'de_CH_scout' => 'de_CH_scout',
-                        'fr' => 'fr',
-                        'fr_CH_scout' => 'fr_CH_scout',
-                        'it' => 'it',
-                        'it_CH_scout' => 'it_CH_scout',
-                        'rm' => 'rm',
-                        'rm_CH_scout' => 'rm_CH_scout',
-                    ],
-                ],
-            ],
-        ]], $result);
-    }
-
-    public static function getFormats() {
-        $hydra = HydraConstraintViolationListNormalizer::FORMAT;
-        $problem = JsonProblemConstraintViolationListNormalizer::FORMAT;
-
-        return [
-            $hydra => [$hydra],
-            $problem => [$problem],
-        ];
-    }
-
-    public static function getConstraintViolations(): array {
-        return [
-            new ConstraintViolation(
-                message: 'value must be one of inactive, was established',
-                messageTemplate: 'value must be one of {{ to }}, was {{ value }}',
-                parameters: ['{{ to }}' => 'inactive', '{{ value }}' => 'established'],
-                root: new CampCollaboration(),
-                propertyPath: 'status',
-                invalidValue: 'established',
-                plural: null,
-                code: null,
-                constraint: new AssertAllowTransitions(transitions: [])
-            ),
-            new ConstraintViolation(
-                message: 'This value should not be blank.',
-                messageTemplate: 'This value should not be blank.',
-                parameters: ['{{ value }}' => '""'],
-                root: new CampCollaboration(),
-                propertyPath: 'name',
-                invalidValue: '',
-                plural: null,
-                code: 'c1051bb4-d103-4f74-8988-acbcafc7fdc3',
-                constraint: new NotBlank()
-            ),
-            new ConstraintViolation(
-                message: 'This value should not be null.',
-                messageTemplate: 'This value should not be null.',
-                parameters: [],
-                root: new CampCollaboration(),
-                propertyPath: 'name',
-                invalidValue: '',
-                plural: null,
-                code: 'c1051bb4-d103-4f74-8988-acbcafc7fdc3',
-                constraint: new NotNull()
-            ),
-            new ConstraintViolation(
-                message: 'This is a test message for i18n variants',
-                messageTemplate: 'This is a test message for i18n variants',
-                parameters: [],
-                root: new CampCollaboration(),
-                propertyPath: 'name',
-                invalidValue: '',
-                plural: null,
-                code: 'c1051bb4-d103-4f74-8988-acbcafc7fdc5',
-                constraint: new MyConstraint()
-            ),
-        ];
-    }
-}
-
-class MyConstraint extends Constraint {
-    public function __construct(
-        ?array $options = null,
-        ?array $groups = null,
-        $payload = null
-    ) {
-        parent::__construct($options ?? [], $groups, $payload);
-    }
-}