test: fix tests

api-platform · Aug 10, 2023 · 456ba81 · 456ba81
1 parent f062fe7
commit 456ba81
Show file tree

Hide file tree

Showing 15 changed files with 198 additions and 119 deletions.
diff --git a/api/composer.lock b/api/composer.lock
diff --git a/api/config/packages/api_platform.yaml b/api/config/packages/api_platform.yaml
@@ -13,6 +13,8 @@ api_platform:
         stateless: true
         cache_headers:
             vary: ['Content-Type', 'Authorization', 'Origin']
+        extra_properties:
+            standard_put: true
     oauth:
         enabled: true
         clientId: '%env(OIDC_SWAGGER_CLIENT_ID)%'

diff --git a/api/src/Entity/Book.php b/api/src/Entity/Book.php
@@ -60,7 +60,6 @@
         ),
     ],
     normalizationContext: [
-        'item_uri_template' => '/admin/books/{id}{._format}',
         'groups' => ['Book:read:admin', 'Enum:read'],
         'skip_null_values' => true,
     ],
@@ -76,7 +75,6 @@
         new Get(),
     ],
     normalizationContext: [
-        'item_uri_template' => '/books/{id}{._format}',
         'groups' => ['Book:read', 'Enum:read'],
         'skip_null_values' => true,
     ]

diff --git a/api/src/Entity/Review.php b/api/src/Entity/Review.php
@@ -58,7 +58,6 @@
             'book' => '/admin/books/{id}{._format}',
             'user' => '/admin/users/{id}{._format}',
         ],
-        'item_uri_template' => '/admin/reviews/{id}{._format}',
         'skip_null_values' => true,
         'groups' => ['Review:read', 'Review:read:admin'],
     ],
@@ -115,7 +114,6 @@
             'book' => '/books/{id}{._format}',
             'user' => '/users/{id}{._format}',
         ],
-        'item_uri_template' => '/books/{bookId}/reviews/{id}{._format}',
         'skip_null_values' => true,
         'groups' => ['Review:read'],
     ],

diff --git a/api/src/State/Processor/BookPersistProcessor.php b/api/src/State/Processor/BookPersistProcessor.php
@@ -42,10 +42,9 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         }
 
         // save entity
-        $this->persistProcessor->process($data, $operation, $uriVariables, $context);
+        $data = $this->persistProcessor->process($data, $operation, $uriVariables, $context);
 
         // publish on Mercure
-        // todo find a way to do it in API Platform
         foreach (['/admin/books/{id}{._format}', '/books/{id}{._format}'] as $uriTemplate) {
             $this->mercureProcessor->process(
                 $data,

diff --git a/api/src/State/Processor/BookRemoveProcessor.php b/api/src/State/Processor/BookRemoveProcessor.php
@@ -6,7 +6,7 @@
 
 use ApiPlatform\Api\IriConverterInterface;
 use ApiPlatform\Api\UrlGeneratorInterface;
-use ApiPlatform\Doctrine\Common\State\RemoveProcessor as DoctrineRemoveProcessor;
+use ApiPlatform\Doctrine\Common\State\RemoveProcessor;
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\State\ProcessorInterface;
@@ -16,7 +16,7 @@
 final readonly class BookRemoveProcessor implements ProcessorInterface
 {
     public function __construct(
-        #[Autowire(service: DoctrineRemoveProcessor::class)]
+        #[Autowire(service: RemoveProcessor::class)]
         private ProcessorInterface $removeProcessor,
         #[Autowire(service: MercureProcessor::class)]
         private ProcessorInterface $mercureProcessor,
@@ -36,7 +36,6 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         $this->removeProcessor->process($data, $operation, $uriVariables, $context);
 
         // publish on Mercure
-        // todo find a way to do it in API Platform
         foreach (['/admin/books/{id}{._format}', '/books/{id}{._format}'] as $uriTemplate) {
             $iri = $this->iriConverter->getIriFromResource(
                 $object,

diff --git a/api/src/State/Processor/MercureProcessor.php b/api/src/State/Processor/MercureProcessor.php
@@ -46,9 +46,9 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
             $context['data'] = $this->serializer->serialize(
                 $data,
                 key($this->formats),
-                ($operation->getNormalizationContext() ?? [] + [
-                    'item_uri_template' => $context['item_uri_template'] ?? null,
-                ])
+                ($operation->getNormalizationContext() ?? []) + (isset($context['item_uri_template']) ? [
+                    'item_uri_template' => $context['item_uri_template'],
+                ] : [])
             );
         }
 

diff --git a/api/src/State/Processor/ReviewPersistProcessor.php b/api/src/State/Processor/ReviewPersistProcessor.php
@@ -4,19 +4,18 @@
 
 namespace App\State\Processor;
 
+use ApiPlatform\Doctrine\Common\State\PersistProcessor;
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\State\ProcessorInterface;
 use App\Entity\Review;
-use App\Repository\ReviewRepository;
-use Doctrine\Persistence\ObjectRepository;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\DependencyInjection\Attribute\Autowire;
 
 final readonly class ReviewPersistProcessor implements ProcessorInterface
 {
     public function __construct(
-        #[Autowire(service: ReviewRepository::class)]
-        private ObjectRepository $repository,
+        #[Autowire(service: PersistProcessor::class)]
+        private ProcessorInterface $persistProcessor,
         private Security $security,
         #[Autowire(service: MercureProcessor::class)]
         private ProcessorInterface $mercureProcessor
@@ -32,10 +31,9 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         $data->publishedAt = new \DateTimeImmutable();
 
         // save entity
-        $this->repository->save($data, true);
+        $data = $this->persistProcessor->process($data, $operation, $uriVariables, $context);
 
         // publish on Mercure
-        // todo find a way to do it in API Platform
         foreach (['/admin/reviews/{id}{._format}', '/books/{bookId}/reviews/{id}{._format}'] as $uriTemplate) {
             $this->mercureProcessor->process(
                 $data,

diff --git a/api/src/State/Processor/ReviewRemoveProcessor.php b/api/src/State/Processor/ReviewRemoveProcessor.php
@@ -6,7 +6,7 @@
 
 use ApiPlatform\Api\IriConverterInterface;
 use ApiPlatform\Api\UrlGeneratorInterface;
-use ApiPlatform\Doctrine\Common\State\RemoveProcessor as DoctrineRemoveProcessor;
+use ApiPlatform\Doctrine\Common\State\RemoveProcessor;
 use ApiPlatform\Metadata\Operation;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\State\ProcessorInterface;
@@ -16,7 +16,7 @@
 final readonly class ReviewRemoveProcessor implements ProcessorInterface
 {
     public function __construct(
-        #[Autowire(service: DoctrineRemoveProcessor::class)]
+        #[Autowire(service: RemoveProcessor::class)]
         private ProcessorInterface $removeProcessor,
         #[Autowire(service: MercureProcessor::class)]
         private ProcessorInterface $mercureProcessor,
@@ -36,7 +36,6 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
         $this->removeProcessor->process($data, $operation, $uriVariables, $context);
 
         // publish on Mercure
-        // todo find a way to do it in API Platform
         foreach (['/admin/reviews/{id}{._format}', '/books/{bookId}/reviews/{id}{._format}'] as $uriTemplate) {
             $iri = $this->iriConverter->getIriFromResource(
                 $object,

diff --git a/api/tests/Api/Admin/BookTest.php b/api/tests/Api/Admin/BookTest.php
@@ -250,9 +250,9 @@ public function testAsNonAdminUserICannotCreateABook(int $expectedCode, string $
     }
 
     /**
-     * @dataProvider getInvalidData
+     * @dataProvider getInvalidDataOnCreate
      */
-    public function testAsAdminUserICannotCreateABookWithInvalidData(array $data, array $violations): void
+    public function testAsAdminUserICannotCreateABookWithInvalidData(array $data, int $statusCode, array $expected): void
     {
         $token = self::getContainer()->get(OidcTokenGenerator::class)->generate([
             'email' => UserFactory::createOneAdmin()->email,
@@ -263,40 +263,78 @@ public function testAsAdminUserICannotCreateABookWithInvalidData(array $data, ar
             'json' => $data,
         ]);
 
-        self::assertResponseStatusCodeSame(Response::HTTP_UNPROCESSABLE_ENTITY);
+        self::assertResponseStatusCodeSame($statusCode);
         self::assertResponseHeaderSame('content-type', 'application/ld+json; charset=utf-8');
-        self::assertJsonContains([
-            '@context' => '/contexts/ConstraintViolationList',
-            '@type' => 'ConstraintViolationList',
-            'hydra:title' => 'An error occurred',
-            'violations' => $violations,
-        ]);
+        self::assertJsonContains($expected);
     }
 
-    public function getInvalidData(): iterable
+    public function getInvalidDataOnCreate(): iterable
     {
-        yield [
+        yield 'no data' => [
             [],
+            Response::HTTP_UNPROCESSABLE_ENTITY,
             [
-                [
-                    'propertyPath' => 'book',
-                    'message' => 'This value should not be blank.',
-                ],
-                [
-                    'propertyPath' => 'condition',
-                    'message' => 'This value should not be null.',
+                '@context' => '/contexts/ConstraintViolationList',
+                '@type' => 'ConstraintViolationList',
+                'hydra:title' => 'An error occurred',
+                'violations' => [
+                    [
+                        'propertyPath' => 'book',
+                        'message' => 'This value should not be blank.',
+                    ],
+                    [
+                        'propertyPath' => 'condition',
+                        'message' => 'This value should not be null.',
+                    ],
                 ],
             ],
         ];
-        yield [
+        yield from $this->getInvalidData();
+    }
+
+    public function getInvalidData(): iterable
+    {
+        yield 'empty data' => [
+            [
+                'book' => '',
+                'condition' => '',
+            ],
+            Response::HTTP_BAD_REQUEST,
+            [
+                '@context' => '/contexts/Error',
+                '@type' => 'hydra:Error',
+                'hydra:title' => 'An error occurred',
+                'hydra:description' => 'The data must belong to a backed enumeration of type '.BookCondition::class,
+            ],
+        ];
+        yield 'invalid condition' => [
+            [
+                'book' => 'https://openlibrary.org/books/OL28346544M.json',
+                'condition' => 'invalid condition',
+            ],
+            Response::HTTP_BAD_REQUEST,
+            [
+                '@context' => '/contexts/Error',
+                '@type' => 'hydra:Error',
+                'hydra:title' => 'An error occurred',
+                'hydra:description' => 'The data must belong to a backed enumeration of type '.BookCondition::class,
+            ],
+        ];
+        yield 'invalid book' => [
             [
                 'book' => 'invalid book',
                 'condition' => BookCondition::NewCondition->value,
             ],
+            Response::HTTP_UNPROCESSABLE_ENTITY,
             [
-                [
-                    'propertyPath' => 'book',
-                    'message' => 'This value is not a valid URL.',
+                '@context' => '/contexts/ConstraintViolationList',
+                '@type' => 'ConstraintViolationList',
+                'hydra:title' => 'An error occurred',
+                'violations' => [
+                    [
+                        'propertyPath' => 'book',
+                        'message' => 'This value is not a valid URL.',
+                    ],
                 ],
             ],
         ];
@@ -408,27 +446,22 @@ public function testAsAdminUserICannotUpdateAnInvalidBook(): void
     /**
      * @dataProvider getInvalidData
      */
-    public function testAsAdminUserICannotUpdateABookWithInvalidData(array $data, array $violations): void
+    public function testAsAdminUserICannotUpdateABookWithInvalidData(array $data, int $statusCode, array $expected): void
     {
-        BookFactory::createOne();
+        $book = BookFactory::createOne();
 
         $token = self::getContainer()->get(OidcTokenGenerator::class)->generate([
             'email' => UserFactory::createOneAdmin()->email,
         ]);
 
-        $this->client->request('PUT', '/admin/books/invalid', [
+        $this->client->request('PUT', '/admin/books/'.$book->getId(), [
             'auth_bearer' => $token,
             'json' => $data,
         ]);
 
-        self::assertResponseStatusCodeSame(Response::HTTP_UNPROCESSABLE_ENTITY);
+        self::assertResponseStatusCodeSame($statusCode);
         self::assertResponseHeaderSame('content-type', 'application/ld+json; charset=utf-8');
-        self::assertJsonContains([
-            '@context' => '/contexts/ConstraintViolationList',
-            '@type' => 'ConstraintViolationList',
-            'hydra:title' => 'An error occurred',
-            'violations' => $violations,
-        ]);
+        self::assertJsonContains($expected);
     }
 
     /**
@@ -449,6 +482,10 @@ public function testAsAdminUserICanUpdateABook(): void
         $this->client->request('PUT', '/admin/books/'.$book->getId(), [
             'auth_bearer' => $token,
             'json' => [
+                /* @see https://github.com/api-platform/core/blob/main/src/Serializer/ItemNormalizer.php */
+                'id' => '/books/'.$book->getId(),
+                // Must set all data because of standard PUT
+                'book' => 'https://openlibrary.org/books/OL28346544M.json',
                 'condition' => BookCondition::DamagedCondition->value,
             ],
         ]);

diff --git a/api/tests/Api/Admin/ReviewTest.php b/api/tests/Api/Admin/ReviewTest.php
@@ -249,7 +249,8 @@ public function testAsAdminUserICannotUpdateAnInvalidReview(): void
      */
     public function testAsAdminUserICanUpdateAReview(): void
     {
-        $review = ReviewFactory::createOne();
+        $book = BookFactory::createOne();
+        $review = ReviewFactory::createOne(['book' => $book]);
 
         $token = self::getContainer()->get(OidcTokenGenerator::class)->generate([
             'email' => UserFactory::createOneAdmin()->email,
@@ -258,6 +259,9 @@ public function testAsAdminUserICanUpdateAReview(): void
         $this->client->request('PUT', '/admin/reviews/'.$review->getId(), [
             'auth_bearer' => $token,
             'json' => [
+                // Must set all data because of standard PUT
+                'book' => '/admin/books/'.$book->getId(),
+                'letter' => null,
                 'body' => 'Very good book!',
                 'rating' => 5,
             ],

diff --git a/api/tests/Api/BookmarkTest.php b/api/tests/Api/BookmarkTest.php
@@ -15,6 +15,7 @@
 use App\Tests\Api\Trait\MercureTrait;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Mercure\Update;
+use Symfony\Component\Uid\Uuid;
 use Zenstruck\Foundry\Test\Factories;
 use Zenstruck\Foundry\Test\ResetDatabase;
 
@@ -97,25 +98,22 @@ public function testAsAUserICannotCreateABookmarkWithInvalidData(): void
             'email' => UserFactory::createOne()->email,
         ]);
 
+        $uuid = Uuid::v7()->__toString();
+
         $this->client->request('POST', '/bookmarks', [
             'json' => [
-                'book' => '/books/invalid',
+                'book' => '/books/'.$uuid,
             ],
             'auth_bearer' => $token,
         ]);
 
-        self::assertResponseStatusCodeSame(Response::HTTP_UNPROCESSABLE_ENTITY);
+        self::assertResponseStatusCodeSame(Response::HTTP_BAD_REQUEST);
         self::assertResponseHeaderSame('content-type', 'application/ld+json; charset=utf-8');
         self::assertJsonContains([
-            '@context' => '/contexts/ConstraintViolationList',
-            '@type' => 'ConstraintViolationList',
+            '@context' => '/contexts/Error',
+            '@type' => 'hydra:Error',
             'hydra:title' => 'An error occurred',
-            'violations' => [
-                [
-                    'propertyPath' => 'book',
-                    'message' => 'This value is not valid.',
-                ],
-            ],
+            'hydra:description' => 'Item not found for "/books/'.$uuid.'".',
         ]);
     }