From 12a66f08c12dde61a387efe26f3bec3164947910 Mon Sep 17 00:00:00 2001 From: Khaled Farhat Date: Thu, 7 Jul 2022 00:20:50 +0300 Subject: [PATCH] Fixed a bug in normalizing URL parameters --- camel/Extraction/ExtractedEndpointData.php | 63 +++++++++++++++++++--- tests/Fixtures/TestPost.php | 13 +++++ tests/Fixtures/TestPostController.php | 10 ++++ tests/Fixtures/TestPostUserController.php | 10 ++++ tests/GenerateDocumentationTest.php | 33 ++++++++++++ 5 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 tests/Fixtures/TestPost.php create mode 100644 tests/Fixtures/TestPostController.php create mode 100644 tests/Fixtures/TestPostUserController.php diff --git a/camel/Extraction/ExtractedEndpointData.php b/camel/Extraction/ExtractedEndpointData.php index 4938fd85..cc5fce16 100644 --- a/camel/Extraction/ExtractedEndpointData.php +++ b/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 @@ public function endpointId() 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 @@ public function normalizeResourceParamName(string $uri, Route $route): string "{$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 @@ public function normalizeResourceParamName(string $uri, Route $route): string } 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 @@ public function forSerialisation() 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 @@ public static function getFieldBindingForUrlParam(Route $route, string $paramNam $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[] = $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); + } + } } diff --git a/tests/Fixtures/TestPost.php b/tests/Fixtures/TestPost.php new file mode 100644 index 00000000..5d75c3ed --- /dev/null +++ b/tests/Fixtures/TestPost.php @@ -0,0 +1,13 @@ +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() {