diff --git a/CHANGELOG.md b/CHANGELOG.md index b546e73..543f1cd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 719ea67..944ba9f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -225,11 +225,6 @@ - - - - - diff --git a/src/Validator/Coercion/AbstractCoercer.php b/src/Validator/Coercion/AbstractCoercer.php index 5f6249e..8674d62 100644 --- a/src/Validator/Coercion/AbstractCoercer.php +++ b/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|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 $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|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; + } } diff --git a/src/Validator/Request/RequestBodyCoercer.php b/src/Validator/Request/RequestBodyCoercer.php index 064969a..f7f49e2 100644 --- a/src/Validator/Request/RequestBodyCoercer.php +++ b/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 $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); } } diff --git a/src/Validator/Request/TypeCoercer.php b/src/Validator/Request/TypeCoercer.php index cd46a9c..cda7954 100644 --- a/src/Validator/Request/TypeCoercer.php +++ b/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,23 +58,30 @@ 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 $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) { @@ -81,7 +89,7 @@ private function coerceUnionType(mixed $value, array $types, bool $strict): arra } 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|int|string|float|bool */ + $recurse = fn(mixed $nested, Schema $nestedSchema): array|int|string|float|bool|null + => $this->coerceBySchema($nested, $nestedSchema, $strict); + + /** @var array|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), }; } diff --git a/tests/Unit/Regression/R4/NestedParameterCoercionRegressionTest.php b/tests/Unit/Regression/R4/NestedParameterCoercionRegressionTest.php new file mode 100644 index 0000000..684b470 --- /dev/null +++ b/tests/Unit/Regression/R4/NestedParameterCoercionRegressionTest.php @@ -0,0 +1,121 @@ +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); + } +} diff --git a/tests/Unit/Validator/Request/TypeCoercerNestedTypesTest.php b/tests/Unit/Validator/Request/TypeCoercerNestedTypesTest.php new file mode 100644 index 0000000..1fd7964 --- /dev/null +++ b/tests/Unit/Validator/Request/TypeCoercerNestedTypesTest.php @@ -0,0 +1,288 @@ +coercer = new TypeCoercer(); + } + + #[Test] + public function coerce_object_property_to_integer(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, true); + + $this->assertSame(['limit' => 3], $result); + } + + #[Test] + public function coerce_object_property_to_boolean(): void + { + $param = new Parameter( + name: 'filter', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['enabled' => new Schema(type: 'boolean')], + ), + ); + + $result = $this->coercer->coerce(['enabled' => 'true'], $param, true); + + $this->assertSame(['enabled' => true], $result); + } + + #[Test] + public function coerce_object_property_to_number(): void + { + $param = new Parameter( + name: 'range', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['min' => new Schema(type: 'number')], + ), + ); + + $result = $this->coercer->coerce(['min' => '19.99'], $param, true); + + $this->assertSame(['min' => 19.99], $result); + } + + #[Test] + public function coerce_array_items_to_integer(): void + { + $param = new Parameter( + name: 'ids', + in: 'query', + schema: new Schema( + type: 'array', + items: new Schema(type: 'integer'), + ), + ); + + $result = $this->coercer->coerce(['1', '2', '3'], $param, true); + + $this->assertSame([1, 2, 3], $result); + } + + #[Test] + public function coerce_array_of_objects_recursively(): void + { + $param = new Parameter( + name: 'points', + in: 'query', + schema: new Schema( + type: 'array', + items: new Schema( + type: 'object', + properties: ['x' => new Schema(type: 'integer')], + ), + ), + ); + + $result = $this->coercer->coerce([['x' => '1'], ['x' => '2']], $param, true); + + $this->assertSame([['x' => 1], ['x' => 2]], $result); + } + + #[Test] + public function coerce_object_property_holding_array_of_integers(): void + { + $param = new Parameter( + name: 'filter', + in: 'query', + schema: new Schema( + type: 'object', + properties: [ + 'ids' => new Schema(type: 'array', items: new Schema(type: 'integer')), + ], + ), + ); + + $result = $this->coercer->coerce(['ids' => ['7', '8']], $param, true); + + $this->assertSame(['ids' => [7, 8]], $result); + } + + #[Test] + public function keep_object_properties_absent_from_value_untouched(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: [ + 'limit' => new Schema(type: 'integer'), + 'offset' => new Schema(type: 'integer'), + ], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, true); + + $this->assertSame(['limit' => 3], $result); + } + + #[Test] + public function coerce_declared_property_that_follows_an_absent_one(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: [ + 'offset' => new Schema(type: 'integer'), + 'limit' => new Schema(type: 'integer'), + ], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, true); + + $this->assertSame(['limit' => 3], $result); + } + + #[Test] + public function keep_undeclared_object_members_as_is(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3', 'cursor' => 'abc'], $param, true); + + $this->assertSame(['limit' => 3, 'cursor' => 'abc'], $result); + } + + #[Test] + public function throw_type_mismatch_error_for_invalid_object_property_in_strict_mode(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $this->expectException(TypeMismatchError::class); + + $this->coercer->coerce(['limit' => 'not-a-number'], $param, true, true); + } + + #[Test] + public function keep_invalid_object_property_as_is_in_non_strict_mode(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $result = $this->coercer->coerce(['limit' => 'not-a-number'], $param, true, false); + + $this->assertSame(['limit' => 'not-a-number'], $result); + } + + #[Test] + public function keep_nested_null_property_as_null_when_nullable(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer', nullable: true)], + ), + ); + + $result = $this->coercer->coerce(['limit' => null], $param, true); + + $this->assertSame(['limit' => null], $result); + } + + #[Test] + public function coerce_object_property_with_union_type_to_integer(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: ['integer', 'string'])], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, true); + + $this->assertSame(['limit' => 3], $result); + } + + #[Test] + public function coerce_object_properties_when_declared_by_union_type(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: ['object', 'string'], + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, true); + + $this->assertSame(['limit' => 3], $result); + } + + #[Test] + public function skip_coercion_of_nested_values_when_coercion_disabled(): void + { + $param = new Parameter( + name: 'page', + in: 'query', + schema: new Schema( + type: 'object', + properties: ['limit' => new Schema(type: 'integer')], + ), + ); + + $result = $this->coercer->coerce(['limit' => '3'], $param, false); + + $this->assertSame(['limit' => '3'], $result); + } +}