Merge pull request #4968 from usu/chore/performance-period-endpoint-2

chore(performance): use subresources & uriTemplate property
ecamp · May 15, 2024 · 439746e · 439746e
2 parents 08ee4aa + ed8433d
commit 439746e
Show file tree

Hide file tree

Showing 19 changed files with 699 additions and 164 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
@@ -3,6 +3,7 @@ api_platform:
     version: 1.0.0
     show_webby: false
     use_symfony_listeners: true
+    enable_link_security: true
     mapping:
         paths:
             - '%kernel.project_dir%/src/Entity'

diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php
@@ -8,6 +8,7 @@
 use ApiPlatform\Metadata\ApiResource;
 use ApiPlatform\Metadata\Get;
 use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\Link;
 use App\Repository\DayRepository;
 use App\Serializer\Normalizer\RelatedCollectionLink;
 use Doctrine\Common\Collections\ArrayCollection;
@@ -33,6 +34,18 @@
             normalizationContext: self::COLLECTION_NORMALIZATION_CONTEXT,
             security: 'is_authenticated()'
         ),
+        new GetCollection(
+            uriTemplate: self::PERIOD_SUBRESOURCE_URI_TEMPLATE,
+            uriVariables: [
+                'periodId' => new Link(
+                    toProperty: 'period',
+                    fromClass: Period::class,
+                    security: 'is_granted("CAMP_COLLABORATOR", period) or is_granted("CAMP_IS_PROTOTYPE", period)'
+                ),
+            ],
+            normalizationContext: self::COLLECTION_NORMALIZATION_CONTEXT,
+            security: 'is_fully_authenticated()',
+        ),
     ],
     denormalizationContext: ['groups' => ['write']],
     normalizationContext: ['groups' => ['read']],
@@ -43,6 +56,8 @@
 #[ORM\Entity(repositoryClass: DayRepository::class)]
 #[ORM\UniqueConstraint(name: 'offset_period_idx', columns: ['periodId', 'dayOffset'])]
 class Day extends BaseEntity implements BelongsToCampInterface {
+    public const PERIOD_SUBRESOURCE_URI_TEMPLATE = '/periods/{periodId}/days{._format}';
+
     public const ITEM_NORMALIZATION_CONTEXT = [
         'groups' => [
             'read',
@@ -61,7 +76,11 @@ class Day extends BaseEntity implements BelongsToCampInterface {
     /**
      * The list of people who have a whole-day responsibility on this day.
      */
-    #[ApiProperty(writable: false, example: '["/day_responsibles/1a2b3c4d"]')]
+    #[ApiProperty(
+        writable: false,
+        uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
+        example: '/days/1a2b3c4d/day_responsibles'
+    )]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: DayResponsible::class, mappedBy: 'day', orphanRemoval: true)]
     public Collection $dayResponsibles;
@@ -161,7 +180,10 @@ public function getEnd(): ?\DateTime {
     /**
      * @return DayResponsible[]
      */
-    #[ApiProperty(readableLink: true)]
+    #[ApiProperty(
+        readableLink: true,
+        uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
+    )]
     #[SerializedName('dayResponsibles')]
     #[Groups(['Day:DayResponsibles'])]
     public function getEmbeddedDayResponsibles(): array {

diff --git a/api/src/Entity/DayResponsible.php b/api/src/Entity/DayResponsible.php
@@ -9,6 +9,7 @@
 use ApiPlatform\Metadata\Delete;
 use ApiPlatform\Metadata\Get;
 use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\Link;
 use ApiPlatform\Metadata\Post;
 use App\Repository\DayResponsibleRepository;
 use App\Validator\AssertBelongsToSameCamp;
@@ -31,6 +32,16 @@
         new GetCollection(
             security: 'is_authenticated()'
         ),
+        new GetCollection(
+            uriTemplate: self::DAY_SUBRESOURCE_URI_TEMPLATE,
+            uriVariables: [
+                'dayId' => new Link(
+                    toProperty: 'day',
+                    fromClass: Day::class,
+                    security: 'is_granted("CAMP_COLLABORATOR", day) or is_granted("CAMP_IS_PROTOTYPE", day)'
+                ),
+            ],
+        ),
         new Post(
             securityPostDenormalize: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object)'
         ),
@@ -46,6 +57,8 @@
 #[ORM\Entity(repositoryClass: DayResponsibleRepository::class)]
 #[ORM\UniqueConstraint(name: 'day_campCollaboration_unique', columns: ['dayId', 'campCollaborationId'])]
 class DayResponsible extends BaseEntity implements BelongsToCampInterface {
+    public const DAY_SUBRESOURCE_URI_TEMPLATE = '/days/{dayId}/day_responsibles{._format}';
+
     /**
      * The day on which the person is responsible.
      */

diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php
@@ -68,7 +68,11 @@ class Period extends BaseEntity implements BelongsToCampInterface {
     /**
      * The days in this time period. These are generated automatically.
      */
-    #[ApiProperty(writable: false, example: '["/days?period=/periods/1a2b3c4d"]')]
+    #[ApiProperty(
+        writable: false,
+        uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
+        example: '/periods/1a2b3c4d/days'
+    )]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: Day::class, mappedBy: 'period', orphanRemoval: true, cascade: ['persist'])]
     #[ORM\OrderBy(['dayOffset' => 'ASC'])]
@@ -80,7 +84,11 @@ class Period extends BaseEntity implements BelongsToCampInterface {
      *
      * @var Collection<int, ScheduleEntry>
      */
-    #[ApiProperty(writable: false, example: '["/schedule_entries/1a2b3c4d"]')]
+    #[ApiProperty(
+        writable: false,
+        uriTemplate: ScheduleEntry::PERIOD_SUBRESOURCE_URI_TEMPLATE,
+        example: '/periods/1a2b3c4d/schedule_entries'
+    )]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: ScheduleEntry::class, mappedBy: 'period')]
     #[ORM\OrderBy(['startOffset' => 'ASC', 'left' => 'ASC', 'endOffset' => 'DESC', 'id' => 'ASC'])]
@@ -178,7 +186,10 @@ public function __construct() {
     /**
      * @return Day[]
      */
-    #[ApiProperty(readableLink: true)]
+    #[ApiProperty(
+        readableLink: true,
+        uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
+    )]
     #[SerializedName('days')]
     #[Groups('Period:Days')]
     public function getEmbeddedDays(): array {

diff --git a/api/src/Entity/ScheduleEntry.php b/api/src/Entity/ScheduleEntry.php
@@ -9,6 +9,7 @@
 use ApiPlatform\Metadata\Delete;
 use ApiPlatform\Metadata\Get;
 use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Metadata\Link;
 use ApiPlatform\Metadata\Patch;
 use ApiPlatform\Metadata\Post;
 use App\Doctrine\Filter\ExpressionDateTimeFilter;
@@ -42,6 +43,17 @@
         new GetCollection(
             security: 'is_authenticated()'
         ),
+        new GetCollection(
+            uriTemplate: self::PERIOD_SUBRESOURCE_URI_TEMPLATE,
+            uriVariables: [
+                'periodId' => new Link(
+                    toProperty: 'period',
+                    fromClass: Period::class,
+                    security: 'is_granted("CAMP_COLLABORATOR", period) or is_granted("CAMP_IS_PROTOTYPE", period)'
+                ),
+            ],
+            security: 'is_fully_authenticated()',
+        ),
         new Post(
             denormalizationContext: ['groups' => ['write', 'create']],
             normalizationContext: self::ITEM_NORMALIZATION_CONTEXT,
@@ -61,6 +73,8 @@
 #[ORM\Index(columns: ['startOffset'])]
 #[ORM\Index(columns: ['endOffset'])]
 class ScheduleEntry extends BaseEntity implements BelongsToCampInterface {
+    public const PERIOD_SUBRESOURCE_URI_TEMPLATE = '/periods/{periodId}/schedule_entries{._format}';
+
     public const ITEM_NORMALIZATION_CONTEXT = [
         'groups' => ['read', 'ScheduleEntry:Activity'],
         'swagger_definition_name' => 'read',

diff --git a/api/src/Metadata/Resource/Factory/UriTemplateFactory.php b/api/src/Metadata/Resource/Factory/UriTemplateFactory.php
@@ -68,9 +68,11 @@ public function createFromResourceClass(string $resourceClass): array {
         $getCollectionOperation = OperationHelper::findOneByType($resourceMetadataCollection, GetCollection::class);
 
         $baseUri = $this->iriConverter->getIriFromResource($resourceClass, UrlGeneratorInterface::ABS_PATH, $getCollectionOperation);
+        $relativeUri = $this->iriConverter->getIriFromResource($resourceClass, UrlGeneratorInterface::REL_PATH, $getCollectionOperation);
+
         $idParameter = $this->getIdParameter($resourceMetadataCollection);
         $queryParameters = $this->getQueryParameters($resourceClass, $resourceMetadataCollection);
-        $additionalPathParameter = $this->allowsActionParameter($resourceMetadataCollection) ? '{/action}' : '';
+        $additionalPathParameter = $this->allowsActionParameter($relativeUri, $resourceMetadataCollection) ? '{/action}' : '';
 
         return [
             $baseUri.$idParameter.$additionalPathParameter.$queryParameters,
@@ -150,18 +152,19 @@ protected function getPaginationParameters(ResourceMetadataCollection $resourceM
         return $parameters;
     }
 
-    protected function allowsActionParameter(ResourceMetadataCollection $resourceMetadataCollection): bool {
+    protected function allowsActionParameter(string $uriTemplateBase, ResourceMetadataCollection $resourceMetadataCollection): bool {
         foreach ($resourceMetadataCollection->getIterator()->current()->getOperations() as $operation) {
             /*
              * Matches:
-             * {/inviteKey}/find
-             * users{/id}/activate
+             * - invitations/{/inviteKey}/find
+             * - /users{/id}/activate
              *
              * Does not match:
-             * profiles{/id}
+             * - profiles{/id}
+             * - any uriTemplate, which doesn't start with the same $uriTemplateBase
              */
             if ($operation instanceof HttpOperation) {
-                if (preg_match('/^.*\\/?{.*}\\/.+$/', $operation->getUriTemplate() ?? '')) {
+                if (preg_match('/^\/?'.preg_quote($uriTemplateBase, '/').'.*\\/?{.*}\\/.+$/', $operation->getUriTemplate() ?? '')) {
                     return true;
                 }
             }

diff --git a/api/tests/Api/Camps/ReadCampTest.php b/api/tests/Api/Camps/ReadCampTest.php
@@ -45,7 +45,7 @@ public function testGetSingleCampIsAllowedForGuest() {
         /** @var Camp $camp */
         $camp = static::getFixture('camp1');
         $user = static::getFixture('user3guest');
-        static::createClientWithCredentials(['email' => $user->getEmail()])->request('GET', '/camps/'.$camp->getId());
+        $response = static::createClientWithCredentials(['email' => $user->getEmail()])->request('GET', '/camps/'.$camp->getId());
         $this->assertResponseStatusCodeSame(200);
         $this->assertJsonContains([
             'id' => $camp->getId(),
@@ -67,6 +67,12 @@ public function testGetSingleCampIsAllowedForGuest() {
                 'categories' => ['href' => '/categories?camp=%2Fcamps%2F'.$camp->getId()],
             ],
         ]);
+
+        $responseArray = $response->toArray();
+        $period1 = static::getFixture('period1');
+        $period2 = static::getFixture('period2');
+        $this->assertEquals("/periods/{$period2->getId()}/days", $responseArray['_embedded']['periods'][0]['_links']['days']['href']);
+        $this->assertEquals("/periods/{$period1->getId()}/days", $responseArray['_embedded']['periods'][1]['_links']['days']['href']);
     }
 
     public function testGetSingleCampIsAllowedForMember() {

diff --git a/api/tests/Api/DayResponsibles/ListDayResponsiblesTest.php b/api/tests/Api/DayResponsibles/ListDayResponsiblesTest.php
@@ -101,4 +101,31 @@ public function testListDayResponsiblesFilteredByDayInCampPrototypeIsAllowedForU
             ['href' => $this->getIriFor('dayResponsible1day1period1campPrototype')],
         ], $response->toArray()['_links']['items']);
     }
+
+    public function testListDayResponsiblesAsDaySubresourceIsAllowedForCollaborator() {
+        $day = static::getFixture('day1period1');
+        $response = static::createClientWithCredentials()->request('GET', '/days/'.$day->getId().'/day_responsibles');
+        $this->assertResponseStatusCodeSame(200);
+        $this->assertJsonContains([
+            'totalItems' => 1,
+            '_links' => [
+                'items' => [],
+            ],
+            '_embedded' => [
+                'items' => [],
+            ],
+        ]);
+        $this->assertEqualsCanonicalizing([
+            ['href' => $this->getIriFor('dayResponsible1')],
+        ], $response->toArray()['_links']['items']);
+    }
+
+    public function testListDayResponsiblesAsDaySubresourceIsDeniedForUnrelatedUser() {
+        $day = static::getFixture('day1period1');
+        $response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
+            ->request('GET', '/days/'.$day->getId().'/day_responsibles')
+        ;
+
+        $this->assertResponseStatusCodeSame(404);
+    }
 }
diff --git a/api/tests/Api/Days/ListDaysTest.php b/api/tests/Api/Days/ListDaysTest.php
@@ -122,4 +122,33 @@ public function testListDaysFilteredByPeriodInCampPrototypeIsAllowedForCollabora
             ['href' => $this->getIriFor('day1period1campPrototype')],
         ], $response->toArray()['_links']['items']);
     }
+
+    public function testListDaysAsPeriodSubresourceIsAllowedForCollaborator() {
+        $period = static::getFixture('period1');
+        $response = static::createClientWithCredentials()->request('GET', '/periods/'.$period->getId().'/days');
+        $this->assertResponseStatusCodeSame(200);
+        $this->assertJsonContains([
+            'totalItems' => 3,
+            '_links' => [
+                'items' => [],
+            ],
+            '_embedded' => [
+                'items' => [],
+            ],
+        ]);
+        $this->assertEqualsCanonicalizing([
+            ['href' => $this->getIriFor('day1period1')],
+            ['href' => $this->getIriFor('day2period1')],
+            ['href' => $this->getIriFor('day3period1')],
+        ], $response->toArray()['_links']['items']);
+    }
+
+    public function testListDaysAsPeriodSubresourceIsDeniedForUnrelatedUser() {
+        $period = static::getFixture('period1');
+        $response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
+            ->request('GET', '/periods/'.$period->getId().'/days')
+        ;
+
+        $this->assertResponseStatusCodeSame(404);
+    }
 }
diff --git a/api/tests/Api/Days/ReadDayTest.php b/api/tests/Api/Days/ReadDayTest.php
@@ -62,7 +62,7 @@ public function testGetSingleDayIsAllowedForGuest() {
             '_links' => [
                 'period' => ['href' => $this->getIriFor('period1')],
                 'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
-                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
+                'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
             ],
         ]);
     }
@@ -83,7 +83,7 @@ public function testGetSingleDayIsAllowedForMember() {
             '_links' => [
                 'period' => ['href' => $this->getIriFor('period1')],
                 'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
-                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
+                'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
             ],
         ]);
     }
@@ -102,7 +102,7 @@ public function testGetSingleDayIsAllowedForManager() {
             '_links' => [
                 'period' => ['href' => $this->getIriFor('period1')],
                 'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
-                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
+                'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
             ],
         ]);
     }
@@ -121,7 +121,7 @@ public function testGetSingleDayFromCampPrototypeIsAllowedForUnrelatedUser() {
             '_links' => [
                 'period' => ['href' => $this->getIriFor('period1campPrototype')],
                 'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$day->period->getId().'&start%5Bstrictly_before%5D='.urlencode($end).'&end%5Bafter%5D='.urlencode($start)],
-                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
+                'dayResponsibles' => ['href' => '/days/'.$day->getId().'/day_responsibles'],
             ],
         ]);
     }

diff --git a/api/tests/Api/Periods/ReadPeriodTest.php b/api/tests/Api/Periods/ReadPeriodTest.php
@@ -62,8 +62,8 @@ public function testGetSinglePeriodIsAllowedForGuest() {
             '_links' => [
                 'camp' => ['href' => $this->getIriFor('camp1')],
                 'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
-                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
-                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
+                'days' => ['href' => '/periods/'.$period->getId().'/days'],
+                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
                 'contentNodes' => ['href' => '/content_nodes?period=%2Fperiods%2F'.$period->getId()],
                 'dayResponsibles' => ['href' => '/day_responsibles?day.period=%2Fperiods%2F'.$period->getId()],
             ],
@@ -86,8 +86,8 @@ public function testGetSinglePeriodIsAllowedForMember() {
             '_links' => [
                 'camp' => ['href' => $this->getIriFor('camp1')],
                 'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
-                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
-                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
+                'days' => ['href' => '/periods/'.$period->getId().'/days'],
+                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
             ],
         ]);
     }
@@ -105,8 +105,8 @@ public function testGetSinglePeriodIsAllowedForManager() {
             '_links' => [
                 'camp' => ['href' => $this->getIriFor('camp1')],
                 'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
-                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
-                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
+                'days' => ['href' => '/periods/'.$period->getId().'/days'],
+                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
             ],
         ]);
     }