Feature/handle path parameters (#76)

m8nky · joseph · Holger Hahn · web-flow · commit 2284946869e4 · 2020-08-10T17:26:37.000+03:00
* using the pathfinder to find the path instead of pure string matching - but only when required

* test to reproduce issue and fix

* reproduces issue

* adds exception, throws it

* phpcbf ran

* phpcbf ran

* realising that guzzle must have been a dev dependency so we need to ensure we have a UriFactory

* realising that we were requiring a whole URL interface when all we needed was the path which can safely just be a string

* cs fixes

* should only check for empty specs if we have at least one path with parameters

* DEL: Remove orphans.

* FIX: Naming for collecting operational and path level specs.

* CHG: Refactored path spec finder.

Co-authored-by: joseph &lt;joseph@joseph&gt;
Co-authored-by: Holger Hahn &lt;holger.hahn@c24.de&gt;
diff --git a/src/PSR7/PathFinder.php b/src/PSR7/PathFinder.php
@@ -5,8 +5,8 @@
 namespace League\OpenAPIValidation\PSR7;
 
 use cebe\openapi\spec\OpenApi;
+use cebe\openapi\spec\PathItem;
 use cebe\openapi\spec\Server;
-use Psr\Http\Message\UriInterface;
 use const PHP_URL_PATH;
 use function array_key_exists;
 use function count;
@@ -29,18 +29,39 @@ class PathFinder
 {
     /** @var OpenApi */
     protected $openApiSpec;
-    /** @var UriInterface */
-    protected $uri;
+    /** @var string */
+    protected $path;
     /** @var string $method like "get" */
     protected $method;
 
-    public function __construct(OpenApi $openApiSpec, UriInterface $uri, string $method)
+    public function __construct(OpenApi $openApiSpec, string $uri, string $method)
     {
         $this->openApiSpec = $openApiSpec;
-        $this->uri         = $uri;
+        $this->path        = (string) parse_url($uri, PHP_URL_PATH);
         $this->method      = strtolower($method);
     }
 
+    /**
+     * Determine matching paths.
+     *
+     * @return PathItem[]
+     */
+    public function getPathMatches() : array
+    {
+        // Determine if path matches exactly.
+        $match = $this->openApiSpec->paths->getPath($this->path);
+        if ($match !== null) {
+            return [$match];
+        }
+        // Probably path is parametrized or matches partially. Determine candidates and try to match path.
+        $matches = [];
+        foreach ($this->search() as $result) {
+            $matches[] = $this->openApiSpec->paths->getPath($result->path());
+        }
+
+        return $matches;
+    }
+
     /**
      * Make search
      *
@@ -72,8 +93,7 @@ public function search() : array
                 );
 
                 // 3.1 Compare this path against the real/given path
-                $searchPath = (string) parse_url((string) $this->uri, PHP_URL_PATH);
-                if (! OperationAddress::isPathMatchesSpec($candidatePath, $searchPath)) {
+                if (! OperationAddress::isPathMatchesSpec($candidatePath, $this->path)) {
                     continue;
                 }
 
@@ -105,7 +125,7 @@ private function searchForCandidates() : array
             // servers:     /v1
             $pattern = '#' . preg_replace('#{[^}]+}#', '[^/]+', $specPath) . '/?$#';
 
-            if (! (bool) preg_match($pattern, $this->uri->getPath())) {
+            if (! (bool) preg_match($pattern, $this->path)) {
                 continue;
             }
 
diff --git a/src/PSR7/RequestValidator.php b/src/PSR7/RequestValidator.php
@@ -98,7 +98,7 @@ public function validate(RequestInterface $request) : OperationAddress
      */
     private function findMatchingOperations(RequestInterface $request) : array
     {
-        $pathFinder = new PathFinder($this->openApi, $request->getUri(), $request->getMethod());
+        $pathFinder = new PathFinder($this->openApi, (string) $request->getUri(), $request->getMethod());
 
         return $pathFinder->search();
     }
diff --git a/src/PSR7/ServerRequestValidator.php b/src/PSR7/ServerRequestValidator.php
@@ -98,7 +98,7 @@ public function validate(ServerRequestInterface $serverRequest) : OperationAddre
      */
     private function findMatchingOperations(ServerRequestInterface $request) : array
     {
-        $pathFinder = new PathFinder($this->openApi, $request->getUri(), $request->getMethod());
+        $pathFinder = new PathFinder($this->openApi, (string) $request->getUri(), $request->getMethod());
 
         return $pathFinder->search();
     }
diff --git a/src/PSR7/SpecFinder.php b/src/PSR7/SpecFinder.php
@@ -37,6 +37,39 @@ public function __construct(OpenApi $openApi)
         $this->openApi = $openApi;
     }
 
+    /**
+     * @return Parameter[]
+     *
+     * @throws NoPath
+     */
+    public function findOperationAndPathLevelSpecs(OperationAddress $addr) : array
+    {
+        $spec = $this->findOperationSpec($addr);
+
+        // 1. Collect operation-level params
+        $pathSpecs = [];
+
+        foreach ($spec->parameters as $p) {
+            if ($p->in !== 'path') {
+                continue;
+            }
+
+            $pathSpecs[$p->name] = $p;
+        }
+
+        // 2. Collect path-level params
+        $pathSpec = $this->findPathSpec($addr);
+        foreach ($pathSpec->parameters as $p) {
+            if ($p->in !== 'path') {
+                continue;
+            }
+
+            $pathSpecs += [$p->name => $p]; // union won't override
+        }
+
+        return $pathSpecs;
+    }
+
     /**
      * Find a particular operation (path + method) in the spec
      *
@@ -65,46 +98,14 @@ public function findOperationSpec(OperationAddress $addr) : Operation
      */
     public function findPathSpec(OperationAddress $addr) : PathItem
     {
-        $pathSpec = $this->openApi->paths->getPath($addr->path());
+        $finder    = new PathFinder($this->openApi, $addr->path(), $addr->method());
+        $pathSpecs = $finder->getPathMatches();
 
-        if (! $pathSpec) {
+        if (empty($pathSpecs) === true) {
             throw NoPath::fromPath($addr->path());
         }
 
-        return $pathSpec;
-    }
-
-    /**
-     * @return Parameter[]
-     *
-     * @throws NoPath
-     */
-    public function findPathSpecs(OperationAddress $addr) : array
-    {
-        $spec = $this->findOperationSpec($addr);
-
-        // 1. Collect operation-level params
-        $pathSpecs = [];
-
-        foreach ($spec->parameters as $p) {
-            if ($p->in !== 'path') {
-                continue;
-            }
-
-            $pathSpecs[$p->name] = $p;
-        }
-
-        // 2. Collect path-level params
-        $pathSpec = $this->findPathSpec($addr);
-        foreach ($pathSpec->parameters as $p) {
-            if ($p->in !== 'path') {
-                continue;
-            }
-
-            $pathSpecs += [$p->name => $p]; // union won't override
-        }
-
-        return $pathSpecs;
+        return $pathSpecs[0];
     }
 
     /**
@@ -198,10 +199,13 @@ public function findBodySpec(OperationAddress $addr) : array
      */
     public function findResponseSpec($addr) : ResponseSpec
     {
-        Assert::isInstanceOfAny($addr, [
-            ResponseAddress::class,
-            CallbackResponseAddress::class,
-        ]);
+        Assert::isInstanceOfAny(
+            $addr,
+            [
+                ResponseAddress::class,
+                CallbackResponseAddress::class,
+            ]
+        );
 
         $operation = $this->findOperationSpec($addr);
 
@@ -237,7 +241,8 @@ public function findHeaderSpecs(OperationAddress $addr) : array
         $spec = $this->findOperationSpec($addr);
 
         // 1. Collect operation level headers from "parameters" keyword
-        // An API call may require that custom headers be sent with an HTTP request. OpenAPI lets you define custom request headers as in: header parameters.
+        // An API call may require that custom headers be sent with an HTTP request. OpenAPI lets you define custom
+        // request headers as in: header parameters.
         $headerSpecs = [];
         foreach ($spec->parameters as $p) {
             if ($p->in !== 'header') {
@@ -307,13 +312,23 @@ private function findCallbackInOperation(CallbackAddress $addr, Operation $opera
     {
         $callbacks = $operation->callbacks;
         if (! isset($callbacks[$addr->callbackName()])) {
-            throw NoCallback::fromCallbackPath($addr->path(), $addr->method(), $addr->callbackName(), $addr->callbackMethod());
+            throw NoCallback::fromCallbackPath(
+                $addr->path(),
+                $addr->method(),
+                $addr->callbackName(),
+                $addr->callbackMethod()
+            );
         }
 
         /** @var Callback $callback */
         $callback = $callbacks[$addr->callbackName()];
         if (! isset($callback->getRequest()->getOperations()[$addr->callbackMethod()])) {
-            throw NoCallback::fromCallbackPath($addr->path(), $addr->method(), $addr->callbackName(), $addr->callbackMethod());
+            throw NoCallback::fromCallbackPath(
+                $addr->path(),
+                $addr->method(),
+                $addr->callbackName(),
+                $addr->callbackMethod()
+            );
         }
 
         return $callback->getRequest()->getOperations()[$addr->callbackMethod()];
diff --git a/src/PSR7/Validators/PathValidator.php b/src/PSR7/Validators/PathValidator.php
@@ -43,7 +43,7 @@ public function validate(OperationAddress $addr, MessageInterface $message) : vo
      */
     private function validateRequest(OperationAddress $addr, RequestInterface $message) : void
     {
-        $validator        = new ArrayValidator($this->finder->findPathSpecs($addr));
+        $validator        = new ArrayValidator($this->finder->findOperationAndPathLevelSpecs($addr));
         $path             = $message->getUri()->getPath();
         $pathParsedParams = $addr->parseParams($path); // ['id'=>12]
 
diff --git a/tests/PSR7/PathFinderTest.php b/tests/PSR7/PathFinderTest.php
@@ -5,7 +5,6 @@
 namespace League\OpenAPIValidation\Tests\PSR7;
 
 use cebe\openapi\Reader;
-use GuzzleHttp\Psr7\Uri;
 use League\OpenAPIValidation\PSR7\PathFinder;
 use PHPUnit\Framework\TestCase;
 
@@ -30,7 +29,7 @@ public function testItFindsMatchingOperation() : void
       summary: Product Types
 SPEC;
 
-        $pathFinder = new PathFinder(Reader::readFromYaml($spec), new Uri('/v1/products/10'), 'get');
+        $pathFinder = new PathFinder(Reader::readFromYaml($spec), '/v1/products/10', 'get');
         $opAddrs    = $pathFinder->search();
 
         $this->assertCount(1, $opAddrs);
@@ -56,7 +55,7 @@ public function testItFindsMatchingOperationWithParametrizedServer() : void
       summary: Product Types
 SPEC;
 
-        $pathFinder = new PathFinder(Reader::readFromYaml($spec), new Uri('/v1/2019-05-07/products/20'), 'get');
+        $pathFinder = new PathFinder(Reader::readFromYaml($spec), '/v1/2019-05-07/products/20', 'get');
         $opAddrs    = $pathFinder->search();
 
         $this->assertCount(1, $opAddrs);
@@ -83,7 +82,7 @@ public function testItFindsMatchingOperationForFullUrl() : void
       summary: Product Types
 SPEC;
 
-        $pathFinder = new PathFinder(Reader::readFromYaml($spec), new Uri('https://localhost/v1/products/10'), 'get');
+        $pathFinder = new PathFinder(Reader::readFromYaml($spec), 'https://localhost/v1/products/10', 'get');
         $opAddrs    = $pathFinder->search();
 
         $this->assertCount(1, $opAddrs);
@@ -111,7 +110,7 @@ public function testItFindsMatchingOperationForMultipleServersWithSamePath() : v
       summary: Product Types
 SPEC;
 
-        $pathFinder = new PathFinder(Reader::readFromYaml($spec), new Uri('https://localhost/v1/products/10'), 'get');
+        $pathFinder = new PathFinder(Reader::readFromYaml($spec), 'https://localhost/v1/products/10', 'get');
         $opAddrs    = $pathFinder->search();
 
         $this->assertCount(1, $opAddrs);
diff --git a/tests/PSR7/SpecFinderTest.php b/tests/PSR7/SpecFinderTest.php
@@ -5,6 +5,7 @@
 namespace League\OpenAPIValidation\Tests\PSR7;
 
 use League\OpenAPIValidation\PSR7\CallbackAddress;
+use League\OpenAPIValidation\PSR7\OperationAddress;
 use League\OpenAPIValidation\PSR7\SpecFinder;
 use League\OpenAPIValidation\PSR7\ValidatorBuilder;
 use PHPUnit\Framework\TestCase;
@@ -65,7 +66,59 @@ public function testFindCallbackSpecs() : void
         $operation = $specFinder->findOperationSpec($address);
 
         // Some assertions to ensure we have the right operation
-        $this->assertEquals('boolean', $operation->requestBody->content['application/json']->schema->properties['success']->type);
+        $this->assertEquals(
+            'boolean',
+            $operation->requestBody->content['application/json']->schema->properties['success']->type
+        );
         $this->assertEquals(['200'], array_keys(iterator_to_array($operation->responses->getIterator())));
     }
+
+    public function testHandleParameters() : void
+    {
+        $json       = /** @lang JSON */
+            <<<'JSON'
+{
+    "openapi": "3.0.0",
+    "info": {
+        "title": "API",
+        "version": "1.0"
+    },
+    "paths": {
+        "/api/1.0/order/{orderId}": {
+            "get": {
+                "operationId": "get_order",
+                "parameters": [
+                    {
+                        "name": "orderId",
+                        "in": "path",
+                        "description": "The order ID",
+                        "required": true,
+                        "schema": {
+                            "type": "integer",
+                            "minimum": "1"
+                        }
+                    }
+                ],
+                "responses": {
+                    "200": {
+                        "description": "The order object",
+                        "content": {
+                            "application/json": {
+                                "properties": {
+                                  "value": true
+                                }
+                            }
+                        }
+                    }                    
+                }
+            }
+        }
+    }
+}
+JSON;
+        $schema     = (new ValidatorBuilder())->fromJson($json)->getServerRequestValidator()->getSchema();
+        $specFinder = new SpecFinder($schema);
+        $pathItem   = $specFinder->findPathSpec(new OperationAddress('/api/1.0/order/123', 'get'));
+        self::assertSame('The order object', $pathItem->get->responses[200]->description);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ public function validate(RequestInterface $request) : OperationAddress`
`98`	`98`	`*/`
`99`	`99`	`private function findMatchingOperations(RequestInterface $request) : array`
`100`	`100`	`{`
`101`		`- $pathFinder = new PathFinder($this->openApi, $request->getUri(), $request->getMethod());`
	`101`	`+ $pathFinder = new PathFinder($this->openApi, (string) $request->getUri(), $request->getMethod());`
`102`	`102`
`103`	`103`	`return $pathFinder->search();`
`104`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public function validate(OperationAddress $addr, MessageInterface $message) : vo`
`43`	`43`	`*/`
`44`	`44`	`private function validateRequest(OperationAddress $addr, RequestInterface $message) : void`
`45`	`45`	`{`
`46`		`- $validator = new ArrayValidator($this->finder->findPathSpecs($addr));`
	`46`	`+ $validator = new ArrayValidator($this->finder->findOperationAndPathLevelSpecs($addr));`
`47`	`47`	`$path = $message->getUri()->getPath();`
`48`	`48`	`$pathParsedParams = $addr->parseParams($path); // ['id'=>12]`
`49`	`49`