Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/Controller/ArgumentResolver/CommandValueResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
namespace Stixx\OpenApiCommandBundle\Controller\ArgumentResolver;

use JsonException;
use ReflectionClass;
use ReflectionNamedType;
use Stixx\OpenApiCommandBundle\Attribute\CommandObject;
use Symfony\Component\HttpFoundation\HeaderUtils;
use Symfony\Component\HttpFoundation\Request;
Expand Down Expand Up @@ -55,6 +57,8 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable
$payload = array_replace($payload, $params);
}

$payload = $this->coerceScalarsAgainstConstructor($payload, $type);

yield $this->denormalizeToType($payload, $type);
}

Expand Down Expand Up @@ -149,6 +153,57 @@ private function decodeJsonBodyToArray(Request $request): array
return $decoded;
}

/**
* Coerces query/route string scalars to the declared `int`/`float`/`bool` constructor types.
* Invalid input stays as a string so the denormalizer surfaces its normal error.
*
* @param array<string, mixed> $payload
*
* @return array<string, mixed>
*/
private function coerceScalarsAgainstConstructor(array $payload, string $type): array
{
if (!class_exists($type)) {
return $payload;
}

$reflection = new ReflectionClass($type);
$constructor = $reflection->getConstructor();
if ($constructor === null) {
return $payload;
}

foreach ($constructor->getParameters() as $parameter) {
$name = $parameter->getName();
if (!array_key_exists($name, $payload)) {
continue;
}

$value = $payload[$name];
if (!is_string($value)) {
continue;
}

$parameterType = $parameter->getType();
if (!$parameterType instanceof ReflectionNamedType) {
continue;
}

$coerced = match ($parameterType->getName()) {
'int' => filter_var($value, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE),
'float' => filter_var($value, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE),
'bool' => filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE),
default => null,
};

if ($coerced !== null) {
$payload[$name] = $coerced;
}
}

return $payload;
}

/**
* @param array<string, mixed> $data
*/
Expand Down
150 changes: 150 additions & 0 deletions tests/Unit/Controller/ArgumentResolver/CommandValueResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,138 @@ public function testResolveThrowsOnDenormalizationFailure(): void
iterator_to_array($this->resolver->resolve($request, $argument));
}

/**
* @param array<string, mixed> $expectedPayload
*/
#[DataProvider('provideQueryParameterCoercionCases')]
public function testResolveCoercesQueryScalarsAgainstConstructorTypes(Request $request, array $expectedPayload): void
{
// Arrange — the resolver must coerce string query params (`"1"`, `"true"`, `"1.5"`)
// to the declared scalar constructor parameter types before handing off to the
// denormalizer. Without coercion, Symfony's ObjectNormalizer rejects strings for
// typed scalar properties, producing a 400 on every paginated list endpoint.
$argument = new ArgumentMetadata(
'command',
CommandValueResolverTestPaginatedCommand::class,
false,
false,
null,
false,
[new CommandObject()]
);

$this->serializer->expects($this->once())
->method('denormalize')
->with($expectedPayload, CommandValueResolverTestPaginatedCommand::class)
->willReturn(new stdClass());

// Act
iterator_to_array($this->resolver->resolve($request, $argument));

// Assert — expectation verified by mock `with()` matcher.
}

/**
* @return iterable<string, array{Request, array<string, mixed>}>
*/
public static function provideQueryParameterCoercionCases(): iterable
{
yield 'string "1" coerces to int 1 for typed int parameter' => [
new Request(query: ['page' => '1']),
['page' => 1],
];

yield 'string "1.5" coerces to float 1.5 for typed float parameter' => [
new Request(query: ['ratio' => '1.5']),
['ratio' => 1.5],
];

yield 'string "true" coerces to bool true for typed bool parameter' => [
new Request(query: ['archived' => 'true']),
['archived' => true],
];

yield 'string "false" coerces to bool false for typed bool parameter' => [
new Request(query: ['archived' => 'false']),
['archived' => false],
];

yield 'invalid int string stays as string so validator surfaces its error' => [
new Request(query: ['page' => 'not-a-number']),
['page' => 'not-a-number'],
];

yield 'string "1" stays as string for typed string parameter' => [
new Request(query: ['status' => '1']),
['status' => '1'],
];

yield 'union-typed parameter is skipped (int|string stays string)' => [
new Request(query: ['limit' => '25']),
['limit' => '25'],
];

yield 'missing constructor parameter key is left alone' => [
new Request(query: ['unknownParameter' => '1']),
['unknownParameter' => '1'],
];

yield 'route attribute string coerces to int the same as a query string' => [
new Request(attributes: ['page' => '7']),
['page' => 7],
];

yield 'zero coerces to int 0 (verifies the strict !== null guard rather than a truthy check)' => [
new Request(query: ['page' => '0']),
['page' => 0],
];

yield 'zero-point-zero coerces to float 0.0 (verifies the strict !== null guard)' => [
new Request(query: ['ratio' => '0.0']),
['ratio' => 0.0],
];

yield 'negative integer string coerces to a negative int' => [
new Request(query: ['page' => '-1']),
['page' => -1],
];

yield 'zero from a route attribute coerces to int 0 the same as from a query string' => [
new Request(attributes: ['page' => '0']),
['page' => 0],
];

yield 'negative integer from a route attribute coerces to a negative int' => [
new Request(attributes: ['page' => '-3']),
['page' => -3],
];
}

public function testResolveSkipsCoercionWhenTargetClassHasNoConstructor(): void
{
// Arrange
$request = new Request(query: ['page' => '1']);
$argument = new ArgumentMetadata(
'command',
stdClass::class,
false,
false,
null,
false,
[new CommandObject()]
);

$this->serializer->expects($this->once())
->method('denormalize')
->with(['page' => '1'], stdClass::class)
->willReturn(new stdClass());

// Act
iterator_to_array($this->resolver->resolve($request, $argument));

// Assert — expectation verified by mock.
}

/**
* @param array<string, mixed> $query
* @param array<string, mixed> $attributes
Expand All @@ -223,3 +355,21 @@ private static function createJsonRequest(string $content, array $query = [], ar
return $request;
}
}

/**
* Test-only command class exercising every parameter type variation the coercion
* logic needs to recognise. Kept in the same file to avoid scattering test fixtures.
*
* @internal
*/
final readonly class CommandValueResolverTestPaginatedCommand
{
public function __construct(
public int $page = 1,
public float $ratio = 0.0,
public bool $archived = false,
public string $status = '',
public int|string $limit = 20,
) {
}
}
Loading