diff --git a/camel/Extraction/ExtractedEndpointData.php b/camel/Extraction/ExtractedEndpointData.php index adf6507a..b8652888 100644 --- a/camel/Extraction/ExtractedEndpointData.php +++ b/camel/Extraction/ExtractedEndpointData.php @@ -139,25 +139,44 @@ public function normalizeResourceParamName(string $uri, Route $route): string $params = []; preg_match_all('#\{(\w+?)}#', $uri, $params); - $foundResourceParam = false; - foreach ($params[1] as $param) { - $pluralParam = Str::plural($param); - $resourceRouteNames = ["$pluralParam.show", "$pluralParam.update", "$pluralParam.destroy"]; - - if (Str::contains($route->action['as'] ?? '', $resourceRouteNames)) { - $search = sprintf("%s/{%s}", $pluralParam, $param); + $resourceRouteNames = [ + ".index", ".show", ".update", ".destroy", + ]; + + if (Str::endsWith($route->action['as'] ?? '', $resourceRouteNames)) { + // Note that resource routes can be nested eg users.posts.show + $pluralResources = explode('.', $route->action['as']); + array_pop($pluralResources); + + $foundResourceParam = false; + foreach (array_reverse($pluralResources) as $pluralResource) { + $singularResource = Str::singular($pluralResource); + $search = ["{$pluralResource}/{{$singularResource}}", "{$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 (!$foundResourceParam) { - // Only the first resource param should be {id} - $replace = "$pluralParam/{id}"; + // Only the last resource param should be {id} + $replace = ["$pluralResource/{{$binding}}", "$pluralResource/{{$binding}?}"]; $foundResourceParam = true; } else { - // Subsequent ones should be {_id} - $replace = sprintf("%s/{%s}", $pluralParam, $param.'_id'); + // Earlier ones should be {_id} + $replace = ["{$pluralResource}/{{$singularResource}_{$binding}}", "{$pluralResource}/{{$singularResource}_{$binding}?}"]; } $uri = str_replace($search, $replace, $uri); } } + foreach ($params[1] as $param) { + // For non-resource parameters, if there's a field binding, replace that too: + if ($binding = static::getFieldBindingForUrlParam($route, $param)) { + $search = ["{{$param}}", "{{$param}?}"]; + $replace = ["{{$param}_{$binding}}", "{{$param}_{$binding}?}"]; + $uri = str_replace($search, $replace, $uri); + } + } + return $uri; } @@ -174,4 +193,14 @@ public function forSerialisation() 'route', 'controller', 'method', 'auth', ); } + + public static function getFieldBindingForUrlParam(Route $route, string $paramName, string $default = null): ?string + { + $binding = null; + // Was added in Laravel 7.x + if (method_exists($route, 'bindingFieldFor')) { + $binding = $route->bindingFieldFor($paramName); + } + return $binding ?: $default; + } } \ No newline at end of file diff --git a/src/Extracting/Strategies/UrlParameters/GetFromLaravelAPI.php b/src/Extracting/Strategies/UrlParameters/GetFromLaravelAPI.php index 28182e6a..0667966a 100644 --- a/src/Extracting/Strategies/UrlParameters/GetFromLaravelAPI.php +++ b/src/Extracting/Strategies/UrlParameters/GetFromLaravelAPI.php @@ -28,12 +28,8 @@ public function __invoke(ExtractedEndpointData $endpointData, array $routeRules) $optional = Str::endsWith($match, '?'); $name = rtrim($match, '?'); - // In case of /users/{user:id}, make the param {user}, but - $binding = null; - // Was added in Laravel 7.x - if (method_exists($endpointData->route, 'bindingFieldFor')) { - $binding = $endpointData->route->bindingFieldFor($name); - } + // In case of /users/{user:id}, make the param {user_id} + $binding = ExtractedEndpointData::getFieldBindingForUrlParam($endpointData->route, $name); $parameters[$name] = [ 'name' => $name, 'description' => $this->inferUrlParamDescription($endpointData->uri, $binding ?: $name, $binding ? $name : null), diff --git a/tests/GenerateDocumentationTest.php b/tests/GenerateDocumentationTest.php index 6991a674..3cd2ea69 100644 --- a/tests/GenerateDocumentationTest.php +++ b/tests/GenerateDocumentationTest.php @@ -174,7 +174,7 @@ public function supports_partial_resource_controller() { RouteFacade::resource('/api/users', TestPartialResourceController::class); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); $output = $this->artisan('scribe:generate'); @@ -273,7 +273,7 @@ public function can_parse_utf8_response() { RouteFacade::get('/api/utf8', [TestController::class, 'withUtf8ResponseTag']); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); $this->artisan('scribe:generate'); $generatedHtml = file_get_contents('public/docs/index.html'); @@ -288,7 +288,7 @@ public function sorts_group_naturally() RouteFacade::get('/api/action2', TestGroupController::class . '@action2'); RouteFacade::get('/api/action10', TestGroupController::class . '@action10'); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); $this->artisan('scribe:generate'); $this->assertFileExists(__DIR__ . '/../.scribe/endpoints/0.yaml'); @@ -304,7 +304,7 @@ public function can_customise_static_output_path() { RouteFacade::get('/api/action1', TestGroupController::class . '@action1'); - config(['scribe.routes.0.prefixes' => ['*']]); + config(['scribe.routes.0.match.prefixes' => ['*']]); config(['scribe.static.output_path' => 'static/docs']); $this->artisan('scribe:generate'); @@ -318,7 +318,7 @@ public function will_not_overwrite_manually_modified_content_unless_force_flag_i { RouteFacade::get('/api/action1', [TestGroupController::class, 'action1']); RouteFacade::get('/api/action1b', [TestGroupController::class, 'action1b']); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); $this->artisan('scribe:generate'); @@ -357,10 +357,32 @@ public function will_not_overwrite_manually_modified_content_unless_force_flag_i $this->assertStringNotContainsString('Some other useful stuff.', file_get_contents($authFilePath)); } + /** @test */ + public function generates_correct_url_params_from_resource_routes_and_field_bindings() + { + RouteFacade::prefix('providers/{provider:slug}')->group(function () { + RouteFacade::resource('users.addresses', TestPartialResourceController::class)->parameters([ + 'addresses' => 'address:uuid', + ]); + }); + config(['scribe.routes.0.match.prefixes' => ['*']]); + config(['scribe.openapi.enabled' => false]); + config(['scribe.postman.enabled' => false]); + + $this->artisan('scribe:generate'); + + $groupA = Yaml::parseFile('.scribe/endpoints/0.yaml'); + $this->assertEquals('providers/{provider_slug}/users/{user_id}/addresses', $groupA['endpoints'][0]['uri']); + $groupB = Yaml::parseFile('.scribe/endpoints/1.yaml'); + $this->assertEquals('providers/{provider_slug}/users/{user_id}/addresses/{uuid}', $groupB['endpoints'][0]['uri']); + } + /** @test */ public function will_not_extract_if_noExtraction_flag_is_set() { config(['scribe.routes.0.exclude' => ['*']]); + config(['scribe.openapi.enabled' => false]); + config(['scribe.postman.enabled' => false]); Utils::copyDirectory(__DIR__.'/Fixtures/.scribe', '.scribe'); $output = $this->artisan('scribe:generate', ['--no-extraction' => true]); @@ -383,7 +405,9 @@ public function merges_user_defined_endpoints() { RouteFacade::get('/api/action1', [TestGroupController::class, 'action1']); RouteFacade::get('/api/action2', [TestGroupController::class, 'action2']); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); + config(['scribe.openapi.enabled' => false]); + config(['scribe.postman.enabled' => false]); if (!is_dir('.scribe/endpoints')) mkdir('.scribe/endpoints', 0777, true); copy(__DIR__ . '/Fixtures/custom.0.yaml', '.scribe/endpoints/custom.0.yaml'); @@ -412,7 +436,9 @@ public function respects_endpoints_and_group_sort_order() RouteFacade::get('/api/action1', [TestGroupController::class, 'action1']); RouteFacade::get('/api/action1b', [TestGroupController::class, 'action1b']); RouteFacade::get('/api/action2', [TestGroupController::class, 'action2']); - config(['scribe.routes.0.prefixes' => ['api/*']]); + config(['scribe.routes.0.match.prefixes' => ['api/*']]); + config(['scribe.openapi.enabled' => false]); + config(['scribe.postman.enabled' => false]); $this->artisan('scribe:generate');