-
-
Notifications
You must be signed in to change notification settings - Fork 921
tests: test parameters type detection #7240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: test parameters type detection #7240
Conversation
3f4932a
to
4f58677
Compare
...Validator/Metadata/Resource/Factory/ParameterValidationResourceMetadataCollectionFactory.php
Outdated
Show resolved
Hide resolved
|
||
// Allow null in case of optional parameter | ||
if (isset($schema['type']) && 'boolean' === $schema['type']) { | ||
$assertions[] = new Expression(expression: 'value in [null, 0, 1, "0", "1", false, true, "false", "true"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of using an expression, I'd prefer to use Assert\IsTrue
or Assert\IsFalse
or why not even Assert\Type
. This leads to the same issue as above that is that parameters are always strings. Suggestion: let's add a new castToNativeType
option on parameters, if its true
we can cast the string to the native type inside the ApiPlatform\State\Provider\ParameterProvider
(there's a function in type info for that IIRC). Then, if castToNativeType
is true we can safely add an Assert\Type
constraint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soyuka does the last commit suit your proposition?
I considered the following:
- if
nativeType: new BuiltinType(TypeIdentifier::BOOL)
is set, cast value to bool - if
schema: ['type' => 'boolean']
ANDcastToNativeType: true
, cast value to bool
bb82236
to
27dda8c
Compare
9035ac8
to
0652118
Compare
753ce17
to
7d4dc73
Compare
7d4dc73
to
c4e4bb7
Compare
Thanks @vincentchalamon ! |
No description provided.