Query parameters are not being checked by spec tests (#79)

hotmeteor · Dec 10, 2021 · edc2248 · edc2248
1 parent 7ba93c5
commit edc2248
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 20 deletions.
diff --git a/src/Middleware.php b/src/Middleware.php
@@ -2,8 +2,10 @@
 
 namespace Spectator;
 
+use cebe\openapi\exceptions\IOException;
 use cebe\openapi\exceptions\TypeErrorException;
 use cebe\openapi\exceptions\UnresolvableReferenceException;
+use cebe\openapi\json\InvalidJsonPointerSyntaxException;
 use cebe\openapi\spec\PathItem;
 use Closure;
 use Illuminate\Contracts\Debug\ExceptionHandler;
@@ -99,19 +101,33 @@ protected function formatResponse($exception, $code): JsonResponse
      * @return mixed
      *
      * @throws InvalidPathException
-     * @throws MissingSpecException|RequestValidationException|ResponseValidationException
+     * @throws MissingSpecException
+     * @throws RequestValidationException
+     * @throws TypeErrorException
+     * @throws UnresolvableReferenceException
+     * @throws IOException
+     * @throws InvalidJsonPointerSyntaxException
      */
     protected function validate(Request $request, Closure $next)
     {
         $request_path = $request->route()->uri();
 
         $pathItem = $this->pathItem($request_path, $request->method());
 
-        RequestValidator::validate($request, $pathItem, $request->method());
+        RequestValidator::validate(
+            $request,
+            $pathItem,
+            $request->method()
+        );
 
         $response = $next($request);
 
-        ResponseValidator::validate($request_path, $response, $pathItem->{strtolower($request->method())}, $this->version);
+        ResponseValidator::validate(
+            $request_path,
+            $response,
+            $pathItem->{strtolower($request->method())},
+            $this->version
+        );
 
         $this->spectator->reset();
 
@@ -127,8 +143,8 @@ protected function validate(Request $request, Closure $next)
      * @throws MissingSpecException
      * @throws TypeErrorException
      * @throws UnresolvableReferenceException
-     * @throws \cebe\openapi\exceptions\IOException
-     * @throws \cebe\openapi\json\InvalidJsonPointerSyntaxException
+     * @throws IOException
+     * @throws InvalidJsonPointerSyntaxException
      */
     protected function pathItem($request_path, $request_method): PathItem
     {

diff --git a/src/Validation/RequestValidator.php b/src/Validation/RequestValidator.php
@@ -8,6 +8,7 @@
 use Opis\JsonSchema\ValidationResult;
 use Opis\JsonSchema\Validator;
 use Spectator\Exceptions\RequestValidationException;
+use Spectator\Exceptions\SchemaValidationException;
 
 class RequestValidator extends AbstractValidator
 {
@@ -26,6 +27,11 @@ class RequestValidator extends AbstractValidator
      */
     protected string $method;
 
+    /**
+     * @var array
+     */
+    protected array $parameters;
+
     /**
      * RequestValidator constructor.
      *
@@ -45,11 +51,11 @@ public function __construct(Request $request, PathItem $pathItem, string $method
     /**
      * @param  Request  $request
      * @param  PathItem  $pathItem
-     * @param $method
+     * @param  string  $method
      *
      * @throws RequestValidationException
      */
-    public static function validate(Request $request, PathItem $pathItem, $method)
+    public static function validate(Request $request, PathItem $pathItem, string $method)
     {
         $instance = new self($request, $pathItem, $method);
 
@@ -69,13 +75,17 @@ protected function handle()
     }
 
     /**
-     * @throws RequestValidationException
+     * @throws RequestValidationException|SchemaValidationException
      */
     protected function validateParameters()
     {
         $route = $this->request->route();
 
-        $parameters = $this->pathItem->parameters;
+        $parameters = array_merge(
+            $this->pathItem->parameters,
+            $this->operation()->parameters
+        );
+
         $required_parameters = array_filter($parameters, fn ($parameter) => $parameter->required === true);
 
         foreach ($required_parameters as $parameter) {
@@ -89,12 +99,15 @@ protected function validateParameters()
             } elseif ($parameter->in === 'cookie' && ! $this->request->cookies->has($parameter->name)) {
                 throw new RequestValidationException("Missing required cookie [{$parameter->name}].");
             }
+        }
 
+        foreach ($parameters as $parameter) {
             // Validate schemas, if provided.
             if ($parameter->schema) {
                 $validator = new Validator();
                 $expected_parameter_schema = $parameter->schema->getSerializableData();
                 $result = null;
+                $actual_parameter = null;
 
                 // Get parameter, then validate it.
                 if ($parameter->in === 'path' && $route->hasParameter($parameter->name)) {
@@ -106,21 +119,24 @@ protected function validateParameters()
                 } elseif ($parameter->in === 'cookie' && $this->request->cookies->has($parameter->name)) {
                     $actual_parameter = $this->request->cookies->get($parameter->name);
                 }
-                $result = $validator->validate($actual_parameter, $expected_parameter_schema);
-
-                // If the result is not valid, then display failure reason.
-                if ($result instanceof ValidationResult && $result->isValid() === false) {
-                    $message = RequestValidationException::validationErrorMessage($expected_parameter_schema, $result->error());
-                    throw RequestValidationException::withError($message, $result->error());
-                } elseif ($result->isValid() === false) {
-                    throw RequestValidationException::withError("Parameter [{$parameter->name}] did not match provided JSON schema.", $result->error());
+
+                if ($actual_parameter) {
+                    $result = $validator->validate($actual_parameter, $expected_parameter_schema);
+
+                    // If the result is not valid, then display failure reason.
+                    if ($result instanceof ValidationResult && $result->isValid() === false) {
+                        $message = RequestValidationException::validationErrorMessage($expected_parameter_schema, $result->error());
+                        throw RequestValidationException::withError($message, $result->error());
+                    } elseif ($result->isValid() === false) {
+                        throw RequestValidationException::withError("Parameter [{$parameter->name}] did not match provided JSON schema.", $result->error());
+                    }
                 }
             }
         }
     }
 
     /**
-     * @throws RequestValidationException
+     * @throws RequestValidationException|SchemaValidationException
      */
     protected function validateBody(): void
     {

diff --git a/tests/Fixtures/Test.v1.json b/tests/Fixtures/Test.v1.json
@@ -61,15 +61,15 @@
         "operationId": "get-users",
         "parameters": [
           {
+            "in": "query",
+            "name": "order",
             "schema": {
               "type": "string",
               "enum": [
                 "name",
                 "email"
               ]
             },
-            "in": "query",
-            "name": "order",
             "allowEmptyValue": true
           }
         ]

diff --git a/tests/RequestValidatorTest.php b/tests/RequestValidatorTest.php
@@ -433,6 +433,25 @@ public function allOfSchemaProvider()
             ],
         ];
     }
+
+    public function test_handles_query_parameters()
+    {
+        Spectator::using('Test.v1.json');
+
+        // When testing query parameters, they are not found nor checked by RequestValidator->validateParameters().
+        Route::get('/users', function () {
+            return [];
+        })->middleware(Middleware::class);
+
+        $this->getJson('/users?order=invalid')
+            ->assertInvalidRequest()
+            ->assertErrorsContain([
+                'The data should match one item from enum',
+            ]);
+
+        $this->getJson('/users?order=name')
+            ->assertValidRequest();
+    }
 }
 
 class TestUser extends Model