fix UriTemplateFactory: ignore uriTemplates with different base

usu · May 5, 2024 · ed8433d · ed8433d
1 parent 7e97257
commit ed8433d
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 20 deletions.
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/...SnapshotTests/__snapshots__/ResponseSnapshotTest__testRootEndpointMatchesSnapshot__1.json b/...SnapshotTests/__snapshots__/ResponseSnapshotTest__testRootEndpointMatchesSnapshot__1.json
@@ -37,11 +37,11 @@
             "templated": true
         },
         "dayResponsibles": {
-            "href": "\/day_responsibles{\/id}{\/action}{?day,day[],day.period,day.period[]}",
+            "href": "\/day_responsibles{\/id}{?day,day[],day.period,day.period[]}",
             "templated": true
         },
         "days": {
-            "href": "\/days{\/id}{\/action}{?period,period[],period.camp,period.camp[]}",
+            "href": "\/days{\/id}{?period,period[],period.camp,period.camp[]}",
             "templated": true
         },
         "invitations": {
@@ -104,7 +104,7 @@
             "templated": true
         },
         "scheduleEntries": {
-            "href": "\/schedule_entries{\/id}{\/action}{?period,period[],activity,activity[],start[before],start[strictly_before],start[after],start[strictly_after],end[before],end[strictly_before],end[after],end[strictly_after]}",
+            "href": "\/schedule_entries{\/id}{?period,period[],activity,activity[],start[before],start[strictly_before],start[after],start[strictly_after],end[before],end[strictly_before],end[after],end[strictly_after]}",
             "templated": true
         },
         "self": {

diff --git a/api/tests/Metadata/Resource/Factory/UriTemplateFactoryTest.php b/api/tests/Metadata/Resource/Factory/UriTemplateFactoryTest.php
@@ -3,7 +3,6 @@
 namespace App\Tests\Metadata\Resource\Factory;
 
 use ApiPlatform\Api\FilterInterface;
-use ApiPlatform\Exception\ResourceClassNotFoundException;
 use ApiPlatform\Metadata\ApiResource;
 use ApiPlatform\Metadata\Get;
 use ApiPlatform\Metadata\GetCollection;
@@ -183,11 +182,11 @@ public function testCreatesTemplatedUriWithActionPathParameters() {
             ),
             new Get(
                 name: '_api_/dummys/{id}/find{._format}_get',
-                uriTemplate: '/{inviteKey}/find{._format}',
+                uriTemplate: '/dummys/{inviteKey}/find{._format}',
             ),
             new Patch(
                 name: '_api_/dummys/{id}/accept{._format}_get',
-                uriTemplate: '/{inviteKey}/accept{._format}',
+                uriTemplate: '/dummys/{inviteKey}/accept{._format}',
             ),
         ])));
         $this->createFactory();
@@ -208,7 +207,7 @@ public function testIgnoresRoutesWithNoSlashAtEnd() {
                 name: '_api_/dummys/{id}{._format}_get'
             ),
             new Get(
-                uriTemplate: '/profiles{inviteKey}{._format}',
+                uriTemplate: '/dummys{inviteKey}{._format}',
             ),
         ])));
         $this->createFactory();
@@ -221,12 +220,7 @@ public function testIgnoresRoutesWithNoSlashAtEnd() {
         self::assertThat($templated, self::isTrue());
     }
 
-    /**
-     * This behaviour was not yet implemented, because we don't have the use case yet.
-     *
-     * @throws ResourceClassNotFoundException
-     */
-    public function testDoesNotYetIgnoreActionPathsOfOtherRouteStarts() {
+    public function testIgnoreActionPathsOfOtherRouteStarts() {
         // given
         $resource = 'Dummy';
         $this->resourceMetadataCollection->append((new ApiResource())->withShortName('Dummy')->withOperations(new Operations([
@@ -243,7 +237,7 @@ public function testDoesNotYetIgnoreActionPathsOfOtherRouteStarts() {
         [$uri, $templated] = $this->uriTemplateFactory->createFromShortname($resource);
 
         // then
-        self::assertThat($uri, self::equalTo('/dummys{/id}{/action}'));
+        self::assertThat($uri, self::equalTo('/dummys{/id}'));
         self::assertThat($templated, self::isTrue());
     }