Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritize full match paths in PathFinder #131

Merged
merged 1 commit into from
Jun 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/PSR7/OperationAddress.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

class OperationAddress
{
private const PATH_PLACEHOLDER = '#{[^}]+}#';

/** @var string */
protected $method;
/** @var string */
Expand All @@ -38,7 +40,7 @@ public function __construct(string $path, string $method)
*/
public static function isPathMatchesSpec(string $specPath, string $path): bool
{
$pattern = '#^' . preg_replace('#{[^}]+}#', '[^/]+', $specPath) . '/?$#';
$pattern = '#^' . preg_replace(self::PATH_PLACEHOLDER, '[^/]+', $specPath) . '/?$#';

return (bool) preg_match($pattern, $path);
}
Expand All @@ -58,6 +60,11 @@ public function path(): string
return $this->path;
}

public function hasPlaceholders(): bool
{
return preg_match(self::PATH_PLACEHOLDER, $this->path()) === 1;
}

/**
* Parses given URL and returns params according to the pattern.
*
Expand Down
42 changes: 35 additions & 7 deletions src/PSR7/PathFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use function rtrim;
use function sprintf;
use function strtolower;
use function usort;

use const PHP_URL_PATH;

Expand Down Expand Up @@ -91,7 +92,7 @@ private function doSearch(): array
// 2. for each operation, find suitable "servers" (with respect to overriding)
foreach ($opCandidates as $i => $opAddress) {
$opCandidates[$i] = [
'addr' => $opAddress,
'addr' => $opAddress,
'servers' => $this->findServersForOperation($opAddress),
];
}
Expand All @@ -100,11 +101,7 @@ private function doSearch(): array
foreach ($opCandidates as $opCandidate) {
/** @var Server $server */
foreach ($opCandidate['servers'] as $server) {
$candidatePath = sprintf(
'%s/%s',
rtrim((string) parse_url($server->url, PHP_URL_PATH), '/'),
ltrim($opCandidate['addr']->path(), '/')
);
$candidatePath = $this->composeFullOperationPath($server, $opCandidate['addr']);

// 3.1 Compare this path against the real/given path
if (! OperationAddress::isPathMatchesSpec($candidatePath, $this->path)) {
Expand All @@ -117,7 +114,7 @@ private function doSearch(): array
}
}

return $paths;
return $this->prioritizeStaticPaths($paths);
}

/**
Expand Down Expand Up @@ -180,4 +177,35 @@ private function findServersForOperation(OperationAddress $opAddress): array
// 3. Fallback with servers on root level
return $this->openApiSpec->servers;
}

private function composeFullOperationPath(Server $server, OperationAddress $addr): string
{
return sprintf(
'%s/%s',
rtrim((string) parse_url($server->url, PHP_URL_PATH), '/'),
ltrim($addr->path(), '/')
);
}

/**
* @param OperationAddress[] $paths
*
* @return OperationAddress[]
*/
private function prioritizeStaticPaths(array $paths): array
{
usort($paths, static function (OperationAddress $a, OperationAddress $b): int {
if ($a->hasPlaceholders() && ! $b->hasPlaceholders()) {
return 1;
}

if ($b->hasPlaceholders() && ! $a->hasPlaceholders()) {
return -1;
}

return 0;
});

return $paths;
}
}
136 changes: 136 additions & 0 deletions tests/FromCommunity/RouteCollisionIssueTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php

declare(strict_types=1);

namespace League\OpenAPIValidation\Tests\FromCommunity;

use GuzzleHttp\Psr7\Response;
use League\OpenAPIValidation\PSR7\OperationAddress;
use League\OpenAPIValidation\PSR7\ValidatorBuilder;
use PHPUnit\Framework\TestCase;

final class RouteCollisionIssueTest extends TestCase
{
public function testRouteCollisionIsHandlerProperly(): void
{
$schema = /** @lang JSON */
'
{
"openapi": "3.0.0",
"info": {
"description": "",
"version": "1.0.0",
"title": "Apples"
},
"servers": [
{
"url": "/api"
}
],
"paths": {
"/apples/{apple_id}": {
"get": {
"summary": "",
"description": "",
"operationId": "appleGet",
"parameters": [
{
"in": "path",
"name": "apple_id",
"schema": {
"type": "integer"
},
"required": true
}
],
"responses": {
"200": {
"description": "Success",
"content": {
"application/json": {
"schema": {
"type": "object",
"required": [
"apple"
],
"properties": {
"apple": {
"$ref": "#/components/schemas/apple"
}
}
}
}
}
}
}
}
},
"/apples/ready-to-eat": {
"get": {
"summary": "",
"description": "",
"operationId": "applesReadyGet",
"responses": {
"200": {
"description": "Success",
"content": {
"application/json": {
"schema": {
"properties": {
"apples": {
"type": "array",
"items": {
"$ref": "#/components/schemas/apple"
}
}
},
"required": [
"apples"
]
}
}
}
}
}
}
}
},
"components": {
"schemas": {
"apple": {
"type": "object",
"required": [
"id"
],
"properties": {
"id": {
"type": "integer"
}
}
}
}
}
}
';

$validator = (new ValidatorBuilder())->fromJson($schema)->getResponseValidator();
$operation = new OperationAddress('/api/apples/ready-to-eat', 'get');

$responseContent = /** @lang JSON */
'
{
"apples": [
{
"id": 0
}
]
}
';

$response = new Response(200, ['Content-Type' => 'application/json'], $responseContent);

$validator->validate($operation, $response);

$this->addToAssertionCount(1);
}
}