Merge pull request #4963 from ecamp/revert-4940-chore/performance-per…

…iod-endpoint Revert "chore(performance): use subresources & uriTemplate property to avoid …"
ecamp · Apr 14, 2024 · 55be38b · 55be38b
2 parents 615d130 + 8d64f4e
commit 55be38b
Show file tree

Hide file tree

Showing 13 changed files with 230 additions and 1,013 deletions.
diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php
@@ -8,7 +8,6 @@
 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;
@@ -34,13 +33,6 @@
             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_fully_authenticated()',
-        ),
     ],
     denormalizationContext: ['groups' => ['write']],
     normalizationContext: ['groups' => ['read']],
@@ -51,8 +43,6 @@
 #[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',
@@ -71,11 +61,7 @@ class Day extends BaseEntity implements BelongsToCampInterface {
     /**
      * The list of people who have a whole-day responsibility on this day.
      */
-    #[ApiProperty(
-        writable: false,
-        uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
-        example: '/days/1a2b3c4d/day_responsibles'
-    )]
+    #[ApiProperty(writable: false, example: '["/day_responsibles/1a2b3c4d"]')]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: DayResponsible::class, mappedBy: 'day', orphanRemoval: true)]
     public Collection $dayResponsibles;
@@ -175,10 +161,7 @@ public function getEnd(): ?\DateTime {
     /**
      * @return DayResponsible[]
      */
-    #[ApiProperty(
-        readableLink: true,
-        uriTemplate: DayResponsible::DAY_SUBRESOURCE_URI_TEMPLATE,
-    )]
+    #[ApiProperty(readableLink: true)]
     #[SerializedName('dayResponsibles')]
     #[Groups(['Day:DayResponsibles'])]
     public function getEmbeddedDayResponsibles(): array {

diff --git a/api/src/Entity/DayResponsible.php b/api/src/Entity/DayResponsible.php
@@ -9,7 +9,6 @@
 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;
@@ -32,13 +31,6 @@
         new GetCollection(
             security: 'is_authenticated()'
         ),
-        new GetCollection(
-            uriTemplate: self::DAY_SUBRESOURCE_URI_TEMPLATE,
-            uriVariables: [
-                'dayId' => new Link(toProperty: 'day', fromClass: Day::class),
-            ],
-            security: 'is_fully_authenticated()',
-        ),
         new Post(
             securityPostDenormalize: 'is_granted("CAMP_MEMBER", object) or is_granted("CAMP_MANAGER", object)'
         ),
@@ -54,8 +46,6 @@
 #[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,11 +68,7 @@ class Period extends BaseEntity implements BelongsToCampInterface {
     /**
      * The days in this time period. These are generated automatically.
      */
-    #[ApiProperty(
-        writable: false,
-        uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
-        example: '/periods/1a2b3c4d/days'
-    )]
+    #[ApiProperty(writable: false, example: '["/days?period=/periods/1a2b3c4d"]')]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: Day::class, mappedBy: 'period', orphanRemoval: true, cascade: ['persist'])]
     #[ORM\OrderBy(['dayOffset' => 'ASC'])]
@@ -84,11 +80,7 @@ class Period extends BaseEntity implements BelongsToCampInterface {
      *
      * @var Collection<int, ScheduleEntry>
      */
-    #[ApiProperty(
-        writable: false,
-        uriTemplate: ScheduleEntry::PERIOD_SUBRESOURCE_URI_TEMPLATE,
-        example: '/periods/1a2b3c4d/schedule_entries'
-    )]
+    #[ApiProperty(writable: false, example: '["/schedule_entries/1a2b3c4d"]')]
     #[Groups(['read'])]
     #[ORM\OneToMany(targetEntity: ScheduleEntry::class, mappedBy: 'period')]
     #[ORM\OrderBy(['startOffset' => 'ASC', 'left' => 'ASC', 'endOffset' => 'DESC', 'id' => 'ASC'])]
@@ -186,10 +178,7 @@ public function __construct() {
     /**
      * @return Day[]
      */
-    #[ApiProperty(
-        readableLink: true,
-        uriTemplate: Day::PERIOD_SUBRESOURCE_URI_TEMPLATE,
-    )]
+    #[ApiProperty(readableLink: true)]
     #[SerializedName('days')]
     #[Groups('Period:Days')]
     public function getEmbeddedDays(): array {

diff --git a/api/src/Entity/ScheduleEntry.php b/api/src/Entity/ScheduleEntry.php
@@ -9,7 +9,6 @@
 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;
@@ -43,13 +42,6 @@
         new GetCollection(
             security: 'is_authenticated()'
         ),
-        new GetCollection(
-            uriTemplate: self::PERIOD_SUBRESOURCE_URI_TEMPLATE,
-            uriVariables: [
-                'periodId' => new Link(toProperty: 'period', fromClass: Period::class),
-            ],
-            security: 'is_fully_authenticated()',
-        ),
         new Post(
             denormalizationContext: ['groups' => ['write', 'create']],
             normalizationContext: self::ITEM_NORMALIZATION_CONTEXT,
@@ -69,8 +61,6 @@
 #[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/tests/Api/DayResponsibles/ListDayResponsiblesTest.php b/api/tests/Api/DayResponsibles/ListDayResponsiblesTest.php
@@ -101,34 +101,4 @@ 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(200);
-
-        $this->assertJsonContains(['totalItems' => 0]);
-        $this->assertArrayNotHasKey('items', $response->toArray()['_links']);
-    }
 }
diff --git a/api/tests/Api/Days/ListDaysTest.php b/api/tests/Api/Days/ListDaysTest.php
@@ -122,36 +122,4 @@ 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(200);
-
-        $this->assertJsonContains(['totalItems' => 0]);
-        $this->assertArrayNotHasKey('items', $response->toArray()['_links']);
-    }
 }
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' => '/days/'.$day->getId().'/day_responsibles'],
+                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
             ],
         ]);
     }
@@ -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' => '/days/'.$day->getId().'/day_responsibles'],
+                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
             ],
         ]);
     }
@@ -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' => '/days/'.$day->getId().'/day_responsibles'],
+                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
             ],
         ]);
     }
@@ -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' => '/days/'.$day->getId().'/day_responsibles'],
+                'dayResponsibles' => ['href' => '/day_responsibles?day=%2Fdays%2F'.$day->getId()],
             ],
         ]);
     }

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' => '/periods/'.$period->getId().'/days'],
-                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
+                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
+                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
                 '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' => '/periods/'.$period->getId().'/days'],
-                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
+                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
+                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
             ],
         ]);
     }
@@ -105,8 +105,8 @@ public function testGetSinglePeriodIsAllowedForManager() {
             '_links' => [
                 'camp' => ['href' => $this->getIriFor('camp1')],
                 'materialItems' => ['href' => '/material_items?period=%2Fperiods%2F'.$period->getId()],
-                'days' => ['href' => '/periods/'.$period->getId().'/days'],
-                'scheduleEntries' => ['href' => '/periods/'.$period->getId().'/schedule_entries'],
+                'days' => ['href' => '/days?period=%2Fperiods%2F'.$period->getId()],
+                'scheduleEntries' => ['href' => '/schedule_entries?period=%2Fperiods%2F'.$period->getId()],
             ],
         ]);
     }

diff --git a/api/tests/Api/ScheduleEntries/ListScheduleEntriesTest.php b/api/tests/Api/ScheduleEntries/ListScheduleEntriesTest.php
@@ -375,36 +375,4 @@ public function testListScheduleEntriesFilteredByInvalidEndDoesntFilter() {
             ['href' => $this->getIriFor('scheduleEntry2period1campPrototype')],
         ], $response->toArray()['_links']['items']);
     }
-
-    public function testListScheduleEntriesAsPeriodSubresourceIsAllowedForCollaborator() {
-        $period = static::getFixture('period1');
-        $response = static::createClientWithCredentials()->request('GET', '/periods/'.$period->getId().'/schedule_entries');
-        $this->assertResponseStatusCodeSame(200);
-        $this->assertJsonContains([
-            'totalItems' => 3,
-            '_links' => [
-                'items' => [],
-            ],
-            '_embedded' => [
-                'items' => [],
-            ],
-        ]);
-        $this->assertEqualsCanonicalizing([
-            ['href' => $this->getIriFor('scheduleEntry1')],
-            ['href' => $this->getIriFor('scheduleEntry2')],
-            ['href' => $this->getIriFor('scheduleEntry1period1camp1')],
-        ], $response->toArray()['_links']['items']);
-    }
-
-    public function testListScheduleEntriesAsPeriodSubresourceIsDeniedForUnrelatedUser() {
-        $period = static::getFixture('period1');
-        $response = static::createClientWithCredentials(['email' => static::$fixtures['user4unrelated']->getEmail()])
-            ->request('GET', '/periods/'.$period->getId().'/schedule_entries')
-        ;
-
-        $this->assertResponseStatusCodeSame(200);
-
-        $this->assertJsonContains(['totalItems' => 0]);
-        $this->assertArrayNotHasKey('items', $response->toArray()['_links']);
-    }
 }