fix: Recurse into object properties and array items when coercing parameters (#60)

CHANGELOG.md

@@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+
+- A non-string type nested inside a parameter schema is no longer
+  unsatisfiable. `TypeCoercer` coerced only the top-level schema type,
+  so `object` and `array` parameters handed their un-coerced members to
+  the schema validator: `?page[limit]=3` against `page: {type: object,
+  properties: {limit: {type: integer}}}` could not pass for any value a
+  client could send, since query, path, header and cookie values arrive
+  as strings and coercion is the only path to a non-string leaf. Type
+  dispatch is now `Schema`-oriented and recurses through `properties`
+  and `items`, so `?page[limit]=3`, `?filter[enabled]=true` and
+  `?ids=1,2,3` coerce to `['limit' => 3]`, `['enabled' => true]` and
+  `[1, 2, 3]`. The traversal is shared with `RequestBodyCoercer` — which
+  already recursed for request bodies — through
+  `AbstractCoercer::coerceDeclaredProperties()` and
+  `AbstractCoercer::coerceDeclaredItems()` rather than duplicated. (#60)
+
 ## [0.7.0]
 
 Preparation for the 1.0.0 stable release. This section tracks work that

psalm-baseline.xml

@@ -225,11 +225,6 @@
       <code><![CDATA[$data]]></code>
     </MixedArgumentTypeCoercion>
   </file>
-  <file src="src/Validator/Request/RequestBodyCoercer.php">
-    <MixedAssignment>
-      <code><![CDATA[$item]]></code>
-    </MixedAssignment>
-  </file>
   <file src="src/Validator/Response/ResponseTypeCoercer.php">
     <MixedAssignment>
       <code><![CDATA[$item]]></code>

src/Validator/Coercion/AbstractCoercer.php

@@ -4,11 +4,15 @@
 
 namespace Duyler\OpenApi\Validator\Coercion;
 
+use Duyler\OpenApi\Schema\Model\Schema;
 use Duyler\OpenApi\Validator\Coercion\Internal\BooleanCoercer;
 use Duyler\OpenApi\Validator\Coercion\Internal\IntegerCoercer;
 use Duyler\OpenApi\Validator\Coercion\Internal\NumberCoercer;
 use Duyler\OpenApi\Validator\Coercion\Internal\StringCoercer;
 
+use function array_key_exists;
+use function is_array;
+
 abstract readonly class AbstractCoercer
 {
     public function __construct(
@@ -57,4 +61,58 @@ protected function coerceToString(mixed $value): string|int|float|bool|array|nul
     {
         return $this->stringCoercer->coerce($value);
     }
+
+    /**
+     * @param callable(mixed, Schema): (array<array-key, mixed>|int|string|float|bool|null) $recurse
+     */
+    protected function coerceDeclaredProperties(mixed $value, Schema $schema, callable $recurse): mixed
+    {
+        if (false === is_array($value)) {
+            return $value;
+        }
+
+        $properties = $schema->properties;
+
+        if (null === $properties) {
+            return $value;
+        }
+
+        /** @var array<array-key, mixed> $coerced */
+        $coerced = $value;
+
+        foreach ($properties as $name => $propertySchema) {
+            if (false === array_key_exists($name, $value)) {
+                continue;
+            }
+
+            $coerced[$name] = $recurse($value[$name], $propertySchema);
+        }
+
+        return $coerced;
+    }
+
+    /**
+     * @param callable(mixed, Schema): (array<array-key, mixed>|int|string|float|bool|null) $recurse
+     */
+    protected function coerceDeclaredItems(mixed $value, Schema $schema, callable $recurse): mixed
+    {
+        if (false === is_array($value)) {
+            return $value;
+        }
+
+        $itemsSchema = $schema->items instanceof Schema ? $schema->items : null;
+
+        if (null === $itemsSchema) {
+            return $value;
+        }
+
+        $coerced = [];
+
+        /** @var mixed $item */
+        foreach ($value as $item) {
+            $coerced[] = $recurse($item, $itemsSchema);
+        }
+
+        return $coerced;
+    }
 }

src/Validator/Request/RequestBodyCoercer.php

@@ -9,7 +9,6 @@
 use Duyler\OpenApi\Validator\Dto\CoercionContext;
 use Duyler\OpenApi\Validator\Exception\TypeMismatchError;
 
-use function array_key_exists;
 use function is_array;
 use function is_string;
 
@@ -91,50 +90,22 @@ private function coerceToType(mixed $value, string $type, Schema $schema, bool $
 
     private function coerceToObject(mixed $value, Schema $schema, bool $strict, bool $nullableAsType): array|int|string|float|bool|null
     {
-        if (false === is_array($value)) {
-            /** @var array|int|string|float|bool|null $value */
-            return $value;
-        }
-
-        $properties = $schema->properties ?? null;
-
-        if (null === $properties) {
-            return $value;
-        }
-
-        /** @var array<string, mixed> $coerced */
-        $coerced = $value;
-
-        foreach ($properties as $name => $propertySchema) {
-            if (false === array_key_exists($name, $value)) {
-                continue;
-            }
-
-            $coerced[$name] = $this->coerceInternal($value[$name], $propertySchema, $strict, $nullableAsType);
-        }
-
-        return $coerced;
+        /** @var array|int|string|float|bool|null */
+        return $this->coerceDeclaredProperties($value, $schema, $this->recursion($strict, $nullableAsType));
     }
 
     private function coerceToArray(mixed $value, Schema $schema, bool $strict, bool $nullableAsType): array|int|string|float|bool|null
     {
-        if (false === is_array($value)) {
-            /** @var array|int|string|float|bool|null $value */
-            return $value;
-        }
-
-        $itemsSchema = $schema->items instanceof Schema ? $schema->items : null;
-
-        if (null === $itemsSchema) {
-            return $value;
-        }
-
-        $coerced = [];
-
-        foreach ($value as $item) {
-            $coerced[] = $this->coerceInternal($item, $itemsSchema, $strict, $nullableAsType);
-        }
+        /** @var array|int|string|float|bool|null */
+        return $this->coerceDeclaredItems($value, $schema, $this->recursion($strict, $nullableAsType));
+    }
 
-        return $coerced;
+    /**
+     * @return callable(mixed, Schema): (array|int|string|float|bool|null)
+     */
+    private function recursion(bool $strict, bool $nullableAsType): callable
+    {
+        return fn(mixed $value, Schema $schema): array|int|string|float|bool|null
+            => $this->coerceInternal($value, $schema, $strict, $nullableAsType);
     }
 }

src/Validator/Request/TypeCoercer.php

@@ -5,6 +5,7 @@
 namespace Duyler\OpenApi\Validator\Request;
 
 use Duyler\OpenApi\Schema\Model\Parameter;
+use Duyler\OpenApi\Schema\Model\Schema;
 use Duyler\OpenApi\Validator\Coercion\AbstractCoercer;
 use Duyler\OpenApi\Validator\Exception\TypeMismatchError;
 
@@ -57,31 +58,38 @@ public function coerce(
             return $this->normalizeValue($value);
         }
 
-        $schema = $param->schema;
+        return $this->coerceBySchema($value, $param->schema, $strict);
+    }
+
+    private function coerceBySchema(mixed $value, Schema $schema, bool $strict): array|int|string|float|bool|null
+    {
+        if (null === $value) {
+            return null;
+        }
 
         if (null === $schema->type) {
             return $this->normalizeValue($value);
         }
 
         if (is_array($schema->type)) {
-            return $this->coerceUnionType($value, $schema->type, $strict);
+            return $this->coerceUnionType($value, $schema->type, $schema, $strict);
         }
 
-        return $this->coerceToType($value, $schema->type, $strict);
+        return $this->coerceToType($value, $schema->type, $schema, $strict);
     }
 
     /**
      * @param array<int, string> $types
      */
-    private function coerceUnionType(mixed $value, array $types, bool $strict): array|int|string|float|bool
+    private function coerceUnionType(mixed $value, array $types, Schema $schema, bool $strict): array|int|string|float|bool|null
     {
         foreach ($types as $type) {
             if ('null' === $type) {
                 continue;
             }
 
             try {
-                $coerced = $this->coerceToType($value, $type, $strict);
+                $coerced = $this->coerceToType($value, $type, $schema, $strict);
             } catch (TypeMismatchError) {
                 continue;
             }
@@ -94,18 +102,23 @@ private function coerceUnionType(mixed $value, array $types, bool $strict): arra
         return $this->normalizeValue($value);
     }
 
-    private function coerceToType(mixed $value, string $type, bool $strict): array|int|string|float|bool
+    private function coerceToType(mixed $value, string $type, Schema $schema, bool $strict): array|int|string|float|bool|null
     {
         if (false === is_scalar($value) && false === is_array($value)) {
             return $this->normalizeValue($value);
         }
 
-        /** @var array<array-key, mixed>|int|string|float|bool */
+        $recurse = fn(mixed $nested, Schema $nestedSchema): array|int|string|float|bool|null
+            => $this->coerceBySchema($nested, $nestedSchema, $strict);
+
+        /** @var array<array-key, mixed>|int|string|float|bool|null */
         return match ($type) {
             'integer' => $strict ? $this->coerceToIntegerStrict($value) : $this->coerceToInteger($value),
             'number' => $strict ? $this->coerceToNumberStrict($value) : $this->coerceToNumber($value),
             'boolean' => $strict ? $this->coerceToBooleanStrict($value) : $this->coerceToBoolean($value),
             'string' => $this->coerceToString($value),
+            'object' => $this->coerceDeclaredProperties($value, $schema, $recurse),
+            'array' => $this->coerceDeclaredItems($value, $schema, $recurse),
             default => $this->normalizeValue($value),
         };
     }

tests/Unit/Regression/R4/NestedParameterCoercionRegressionTest.php

@@ -0,0 +1,121 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Duyler\OpenApi\Test\Unit\Regression\R4;
+
+use Duyler\OpenApi\Builder\OpenApiValidatorBuilder;
+use Duyler\OpenApi\Builder\OpenApiValidatorInterface;
+use Duyler\OpenApi\Validator\Request\TypeCoercer;
+use Nyholm\Psr7\Factory\Psr17Factory;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ServerRequestInterface;
+
+use function parse_str;
+use function parse_url;
+
+use const PHP_URL_QUERY;
+
+/**
+ * Regression suite for issue #60: parameter coercion did not recurse into
+ * `object` properties or `array` items, so a nested `integer` / `number` /
+ * `boolean` was unsatisfiable — query, path, header and cookie values arrive
+ * as strings, and coercion was the only path to a non-string leaf.
+ *
+ * Anti-test: removing the `object` / `array` arms from
+ * {@see TypeCoercer::coerceToType()} makes every nested case below fail again.
+ *
+ * @internal
+ */
+final class NestedParameterCoercionRegressionTest extends TestCase
+{
+    private const string SPEC = <<<'YAML'
+openapi: 3.0.0
+info: { title: Coercion API, version: '1' }
+paths:
+  /things:
+    get:
+      operationId: listThings
+      parameters:
+        - name: limit
+          in: query
+          schema:
+            type: integer
+        - name: page
+          in: query
+          style: deepObject
+          schema:
+            type: object
+            properties:
+              limit:
+                type: integer
+        - name: filter
+          in: query
+          style: deepObject
+          schema:
+            type: object
+            properties:
+              enabled:
+                type: boolean
+        - name: ids
+          in: query
+          style: form
+          explode: false
+          schema:
+            type: array
+            items:
+              type: integer
+      responses:
+        '200': { description: ok }
+YAML;
+
+    #[Test]
+    public function top_level_integer_query_parameter_coerces(): void
+    {
+        $operation = $this->build()->validateRequest($this->request('/things?limit=10'));
+
+        self::assertSame('/things', $operation->path);
+    }
+
+    #[Test]
+    public function object_property_integer_query_parameter_coerces(): void
+    {
+        $operation = $this->build()->validateRequest($this->request('/things?page[limit]=3'));
+
+        self::assertSame('/things', $operation->path);
+    }
+
+    #[Test]
+    public function object_property_boolean_query_parameter_coerces(): void
+    {
+        $operation = $this->build()->validateRequest($this->request('/things?filter[enabled]=true'));
+
+        self::assertSame('/things', $operation->path);
+    }
+
+    #[Test]
+    public function array_items_integer_query_parameter_coerces(): void
+    {
+        $operation = $this->build()->validateRequest($this->request('/things?ids=1,2,3'));
+
+        self::assertSame('/things', $operation->path);
+    }
+
+    private function build(): OpenApiValidatorInterface
+    {
+        return OpenApiValidatorBuilder::create()
+            ->fromYamlString(self::SPEC)
+            ->enableCoercion()
+            ->build();
+    }
+
+    private function request(string $uri): ServerRequestInterface
+    {
+        parse_str((string) parse_url($uri, PHP_URL_QUERY), $query);
+
+        return new Psr17Factory()
+            ->createServerRequest('GET', 'http://localhost' . $uri)
+            ->withQueryParams($query);
+    }
+}