Merge pull request #492 from Khaled-Farhat/fix-normalizing-url-params

Fixed a bug in normalizing URL parameters

camel/Extraction/ExtractedEndpointData.php

@@ -2,6 +2,7 @@
 
 namespace Knuckles\Camel\Extraction;
 
+use Illuminate\Database\Eloquent\Model;
 use Illuminate\Routing\Route;
 use Illuminate\Support\Str;
 use Knuckles\Camel\BaseDTO;
@@ -81,11 +82,12 @@ class ExtractedEndpointData extends BaseDTO
 
     public function __construct(array $parameters = [])
     {
-        $parameters['uri'] = $this->normalizeResourceParamName($parameters['uri'], $parameters['route']);
         $parameters['metadata'] = $parameters['metadata'] ?? new Metadata([]);
         $parameters['responses'] = $parameters['responses'] ?? new ResponseCollection([]);
 
         parent::__construct($parameters);
+
+        $this->uri = $this->normalizeResourceParamName($this->uri, $this->route, $this->getTypeHintedArguments());
     }
 
     public static function fromRoute(Route $route, array $extras = []): self
@@ -131,7 +133,7 @@ class ExtractedEndpointData extends BaseDTO
         return $this->httpMethods[0] . str_replace(['/', '?', '{', '}', ':', '\\', '+', '|'], '-', $this->uri);
     }
 
-    public function normalizeResourceParamName(string $uri, Route $route): string
+    public function normalizeResourceParamName(string $uri, Route $route, array $typeHintedArguments): string
     {
         $params = [];
         preg_match_all('#\{(\w+?)}#', $uri, $params);
@@ -157,9 +159,11 @@ class ExtractedEndpointData extends BaseDTO
                     "{$pluralResource}/{{$singularResource}?}"
                 ];
 
-                // We'll replace with {id} by default, but if the user is using a different key,
-                // like /users/{user:uuid}, use that instead
-                $binding = static::getFieldBindingForUrlParam($route, $singularResource, 'id');
+                // If there is an inline binding in the route, like /users/{user:uuid}, use that key,
+                // Else, search for a type-hinted variable in the action, whose name matches the route segment name,
+                // If there is such variable (like User $user), call getRouteKeyName() on the model,
+                // Otherwise, use the id
+                $binding = static::getFieldBindingForUrlParam($route, $singularResource, $typeHintedArguments, 'id');
 
                 if (!$foundResourceParam) {
                     // Only the last resource param should be {id}
@@ -179,8 +183,8 @@ class ExtractedEndpointData extends BaseDTO
         }
 
         foreach ($params[1] as $param) {
-            // For non-resource parameters, if there's a field binding, replace that too:
-            if ($binding = static::getFieldBindingForUrlParam($route, $param)) {
+            // For non-resource parameters, if there's a field binding/type-hinted variable, replace that too:
+            if ($binding = static::getFieldBindingForUrlParam($route, $param, $typeHintedArguments)) {
                 $search = ["{{$param}}", "{{$param}?}"];
                 $replace = ["{{$param}_{$binding}}", "{{$param}_{$binding}?}"];
                 $uri = str_replace($search, $replace, $uri);
@@ -206,7 +210,8 @@ class ExtractedEndpointData extends BaseDTO
         return $copy;
     }
 
-    public static function getFieldBindingForUrlParam(Route $route, string $paramName, string $default = null): ?string
+    public static function getFieldBindingForUrlParam(Route $route, string $paramName, array $typeHintedArguments = [],
+                                                      string $default = null): ?string
     {
         $binding = null;
         // Was added in Laravel 7.x
@@ -214,6 +219,48 @@ class ExtractedEndpointData extends BaseDTO
             $binding = $route->bindingFieldFor($paramName);
         }
 
+        // Search for a type-hinted variable whose name matches the route segment name
+        if (is_null($binding) && array_key_exists($paramName, $typeHintedArguments)) {
+            $argumentType = $typeHintedArguments[$paramName]->getType();
+            $argumentClassName = $argumentType->getName();
+            $argumentInstance = new $argumentClassName;
+            $binding = $argumentInstance->getRouteKeyName();
+        }
+
         return $binding ?: $default;
     }
+
+    /**
+     * Return the type-hinted method arguments in the action that have a Model type,
+     * The arguments will be returned as an array of the form: $arguments[<variable_name>] = $argument
+     */
+    protected function getTypeHintedArguments(): array
+    {
+        $arguments = [];
+        if ($this->method) {
+            foreach ($this->method->getParameters() as $argument) {
+                if ($this->argumentHasModelType($argument)) {
+                    $arguments[$argument->getName()] = $argument;
+                }
+            }
+        }
+
+        return $arguments;
+    }
+
+    /**
+     * Determine whether the argument has a Model type
+     */
+    protected function argumentHasModelType(\ReflectionParameter $argument): bool
+    {
+        $argumentType = $argument->getType();
+        if (!$argumentType) {
+            // The argument does not have a type-hint
+            return false;
+        } else {
+            $argumentClassName = $argumentType->getName();
+            $argumentInstance = new $argumentClassName;
+            return ($argumentInstance instanceof Model);
+        }
+    }
 }

tests/Fixtures/TestPost.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Knuckles\Scribe\Tests\Fixtures;
+
+use Illuminate\Database\Eloquent\Model;
+
+class TestPost extends Model
+{
+    public function getRouteKeyName()
+    {
+        return 'slug';
+    }
+}

tests/Fixtures/TestPostController.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Knuckles\Scribe\Tests\Fixtures;
+
+class TestPostController
+{
+    public function update(TestPost $post)
+    {
+    }
+}

tests/Fixtures/TestPostUserController.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace Knuckles\Scribe\Tests\Fixtures;
+
+class TestPostUserController
+{
+    public function update(TestPost $post, TestUser $user)
+    {
+    }
+}

tests/GenerateDocumentationTest.php

@@ -8,6 +8,9 @@ use Knuckles\Scribe\Tests\Fixtures\TestController;
 use Knuckles\Scribe\Tests\Fixtures\TestGroupController;
 use Knuckles\Scribe\Tests\Fixtures\TestIgnoreThisController;
 use Knuckles\Scribe\Tests\Fixtures\TestPartialResourceController;
+use Knuckles\Scribe\Tests\Fixtures\TestPost;
+use Knuckles\Scribe\Tests\Fixtures\TestPostController;
+use Knuckles\Scribe\Tests\Fixtures\TestPostUserController;
 use Knuckles\Scribe\Tests\Fixtures\TestResourceController;
 use Knuckles\Scribe\Tests\Fixtures\TestUser;
 use Knuckles\Scribe\Tools\Utils;
@@ -414,6 +417,36 @@ class GenerateDocumentationTest extends BaseLaravelTest
         $this->assertEquals('providers/{provider_slug}/users/{user_id}/addresses/{uuid}', $groupB['endpoints'][0]['uri']);
     }
 
+    /** @test */
+    public function generates_correct_url_params_from_resource_routes_and_model_binding()
+    {
+        RouteFacade::resource('posts', TestPostController::class)->only('update');
+        RouteFacade::resource('posts.users', TestPostUserController::class)->only('update');
+
+        config(['scribe.routes.0.match.prefixes' => ['*']]);
+        config(['scribe.routes.0.apply.response_calls.methods' => []]);
+
+        $this->artisan('scribe:generate');
+
+        $group = Yaml::parseFile('.scribe/endpoints/00.yaml');
+        $this->assertEquals('posts/{slug}', $group['endpoints'][0]['uri']);
+        $this->assertEquals('posts/{post_slug}/users/{id}', $group['endpoints'][1]['uri']);
+    }
+
+    /** @test */
+    public function generates_correct_url_params_from_non_resource_routes_and_model_binding()
+    {
+        RouteFacade::get('posts/{post}/users', function(TestPost $post) {});
+
+        config(['scribe.routes.0.match.prefixes' => ['*']]);
+        config(['scribe.routes.0.apply.response_calls.methods' => []]);
+
+        $this->artisan('scribe:generate');
+
+        $group = Yaml::parseFile('.scribe/endpoints/00.yaml');
+        $this->assertEquals('posts/{post_slug}/users', $group['endpoints'][0]['uri']);
+    }
+
     /** @test */
     public function will_generate_without_extracting_if_noExtraction_flag_is_set()
     {