From 9b187b28d12fccfc9cefbc97f5c7a4899bd6b65f Mon Sep 17 00:00:00 2001
From: Pavel Batanov <pavel@batanov.me>
Date: Wed, 9 Jun 2021 17:25:32 +0300
Subject: [PATCH] Prioritize full match paths in PathFinder

---
 src/PSR7/OperationAddress.php                 |   9 +-
 src/PSR7/PathFinder.php                       |  42 +++++-
 .../FromCommunity/RouteCollisionIssueTest.php | 136 ++++++++++++++++++
 3 files changed, 179 insertions(+), 8 deletions(-)
 create mode 100644 tests/FromCommunity/RouteCollisionIssueTest.php

diff --git a/src/PSR7/OperationAddress.php b/src/PSR7/OperationAddress.php
index 152893df..0ec96e37 100644
--- a/src/PSR7/OperationAddress.php
+++ b/src/PSR7/OperationAddress.php
@@ -19,6 +19,8 @@
 
 class OperationAddress
 {
+    private const PATH_PLACEHOLDER = '#{[^}]+}#';
+
     /** @var string */
     protected $method;
     /** @var string */
@@ -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);
     }
@@ -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.
      *
diff --git a/src/PSR7/PathFinder.php b/src/PSR7/PathFinder.php
index c63af4eb..00ab1929 100644
--- a/src/PSR7/PathFinder.php
+++ b/src/PSR7/PathFinder.php
@@ -16,6 +16,7 @@
 use function rtrim;
 use function sprintf;
 use function strtolower;
+use function usort;
 
 use const PHP_URL_PATH;
 
@@ -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),
             ];
         }
@@ -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)) {
@@ -117,7 +114,7 @@ private function doSearch(): array
             }
         }
 
-        return $paths;
+        return $this->prioritizeStaticPaths($paths);
     }
 
     /**
@@ -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;
+    }
 }
diff --git a/tests/FromCommunity/RouteCollisionIssueTest.php b/tests/FromCommunity/RouteCollisionIssueTest.php
new file mode 100644
index 00000000..2358c21a
--- /dev/null
+++ b/tests/FromCommunity/RouteCollisionIssueTest.php
@@ -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);
+    }
+}