Skip to content

Commit

Permalink
Add enum description to route parameter if possible. (#2062)
Browse files Browse the repository at this point in the history
* Add enum description to route parameter if possible.

* code style

* Use strpos instead of str_contains

* Ignore dots for enum, add tests

* Update docstring

Co-authored-by: Guilhem Niot <guilhem@gniot.fr>
  • Loading branch information
lhausammann and GuilhemN authored Jan 4, 2023
1 parent 700b52e commit 1aa2cbd
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 2 deletions.
41 changes: 39 additions & 2 deletions RouteDescriber/RouteMetadataDescriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ final class RouteMetadataDescriber implements RouteDescriberInterface
{
use RouteDescriberTrait;

private const ALPHANUM_EXPANDED_REGEX = '/^[-a-zA-Z0-9_]*$/';

public function describe(OA\OpenApi $api, Route $route, \ReflectionMethod $reflectionMethod)
{
foreach ($this->getOperations($api, $route) as $operation) {
Expand Down Expand Up @@ -57,8 +59,17 @@ public function describe(OA\OpenApi $api, Route $route, \ReflectionMethod $refle
$parameter->schema->type = 'string';
}

if (isset($requirements[$pathVariable]) && Generator::UNDEFINED === $parameter->schema->pattern) {
$parameter->schema->pattern = $requirements[$pathVariable];
// handle pattern and possible enum values
if (isset($requirements[$pathVariable])) {
$req = $requirements[$pathVariable];
$enumValues = $this->getPossibleEnumValues($req);
if ($enumValues && Generator::UNDEFINED === $parameter->schema->pattern) {
$parameter->schema->enum = $enumValues;
}
// add the pattern anyway
if (Generator::UNDEFINED === $parameter->schema->pattern) {
$parameter->schema->pattern = $requirements[$pathVariable];
}
}
}
}
Expand Down Expand Up @@ -99,4 +110,30 @@ private function getRefParams(OA\OpenApi $api, OA\Operation $operation): array

return $existingParams;
}

/**
* returns array of separated alphanumeric (including '-', '_') strings from a simple OR regex requirement pattern.
* (routing parameters containing enums have already been resolved to that format at this time).
*
* @param string $reqPattern a requirement pattern to match, e.g. 'a.html|b.html'
*
* @return array<string>
*/
private function getPossibleEnumValues(string $reqPattern): array
{
$requirements = [];
if (false !== strpos($reqPattern, '|')) {
$parts = explode('|', $reqPattern);
foreach ($parts as $part) {
if ('' === $part || 0 === preg_match(self::ALPHANUM_EXPANDED_REGEX, $part)) {
// we check a complex regex expression containing | - abort in that case
return [];
} else {
$requirements[] = $part;
}
}
}

return $requirements;
}
}
86 changes: 86 additions & 0 deletions Tests/RouteDescriber/RouteMetadataDescriberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Nelmio\ApiDocBundle\RouteDescriber\RouteMetadataDescriber;
use OpenApi\Annotations\OpenApi;
use OpenApi\Context;
use OpenApi\Generator;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Routing\Route;

Expand All @@ -25,4 +26,89 @@ public function testUndefinedCheck()

$this->assertNull($routeDescriber->describe(new OpenApi(['_context' => new Context()]), new Route('foo'), new \ReflectionMethod(__CLASS__, 'testUndefinedCheck')));
}

public function testRouteRequirementsWithPattern()
{
$api = new OpenApi([]);
$routeDescriber = new RouteMetadataDescriber();
$route = new Route('/index/{bar}/{foo}.html', [], ['foo' => '[0-9]|[a-z]'], [], 'localhost', 'https', ['GET']);
$routeDescriber->describe(
$api,
$route,
new \ReflectionMethod(__CLASS__, 'testRouteRequirementsWithPattern')
);

$this->assertEquals('/index/{bar}/{foo}.html', $api->paths[0]->path);
$getPathParameter = $api->paths[0]->get->parameters[1];
if ('foo' === $getPathParameter->name) {
$this->assertEquals('path', $getPathParameter->in);
$this->assertEquals('foo', $getPathParameter->name);
$this->assertEquals('string', $getPathParameter->schema->type);
$this->assertEquals('[0-9]|[a-z]', $getPathParameter->schema->pattern);
}
}

/**
* @dataProvider provideEnumPattern
*/
public function testSimpleOrRequirementsAreHandledAsEnums($req)
{
$api = new OpenApi([]);
$routeDescriber = new RouteMetadataDescriber();
$route = new Route('/index/{bar}/{foo}.html', [], ['foo' => $req], [], 'localhost', 'https', ['GET']);
$routeDescriber->describe(
$api,
$route,
new \ReflectionMethod(__CLASS__, 'testSimpleOrRequirementsAreHandledAsEnums')
);

$this->assertEquals('/index/{bar}/{foo}.html', $api->paths[0]->path);
$getPathParameter = $api->paths[0]->get->parameters[1];
$this->assertEquals('path', $getPathParameter->in);
$this->assertEquals('foo', $getPathParameter->name);
$this->assertEquals('string', $getPathParameter->schema->type);
$this->assertEquals(explode('|', $req), $getPathParameter->schema->enum);
$this->assertEquals($req, $getPathParameter->schema->pattern);
}

/**
* @dataProvider provideInvalidEnumPattern
*/
public function testNonEnumPatterns($pattern)
{
$api = new OpenApi([]);
$routeDescriber = new RouteMetadataDescriber();
$route = new Route('/index/{foo}.html', [], ['foo' => $pattern], [], 'localhost', 'https', ['GET']);
$routeDescriber->describe(
$api,
$route,
new \ReflectionMethod(__CLASS__, 'testNonEnumPatterns')
);

$getPathParameter = $api->paths[0]->get->parameters[0];
$this->assertEquals($pattern, $getPathParameter->schema->pattern);
$this->assertEquals(Generator::UNDEFINED, $getPathParameter->schema->enum);
}

public function provideEnumPattern()
{
yield ['1|2|3'];
yield ['srf|rtr|rsi'];
yield ['srf-1|srf-2'];
yield ['srf-1|srf-2'];
}

public function provideInvalidEnumPattern()
{
yield ['|'];
yield ['|a'];
yield ['srf|*|rtr'];
yield ['srf||rtr'];
yield ['1|2|'];
yield ['/1|2/'];
yield ['\d|a'];
// dots have special meaning and should be skipped
yield ['a_1\.html|b-2\.html'];
yield ['a_1.html|b-2.html'];
}
}

0 comments on commit 1aa2cbd

Please sign in to comment.