Add a$filters argument to theMapQueryString attribute to allow filtering parameters usingfilter_var function. It will be useful to cast "false", "0", "no" or "off" tofalse before denormalizing the request data.

EDIT

Add aAbstractNormalizer::FILTER_BOOL context option to cast to a boolean using thefilter_var function with the\FILTER_VALIDATE_BOOL filter (seehttps://www.php.net/manual/fr/filter.filters.validate.php).

MapQueryString will use this context option when denormalizing the query string.
MapPayload also but only with data coming from a form.

See the relative comment:#54153 (comment)

Example:

finalreadonlyclass SearchDto{publicfunction__construct(public ?bool$restricted =null)    {    }}finalclass MyController{    #[Route(name:'search', path:'/search')]publicfunction__invoke(#[MapQueryString]SearchDto$search):Response    {// /search?restricted=// "true", "1", "yes" and "on" will be cast to true// "false", "0", "no", "off" and "" will be cast to false// other will be cast to null    }}

carsonbot added Status: Needs Review Feature HttpKernel labels

Mar 4, 2024

carsonbot added this to the7.1 milestone

Mar 4, 2024

Copy link

Member

nicolas-grekas commentedMar 5, 2024

I'm not sure how useful this could be: the query string can be a tuple of many different type of values, while this works only of all properties of the DTO are of the same type, isn't it?

Copy link

ContributorAuthor

Jean-Beru commentedMar 5, 2024

I'm not sure how useful this could be: the query string can be a tuple of many different type of values, while this works only of all properties of the DTO are of the same type, isn't it?

The associative array allows to define which parameter is concerned.#[MapQueryString(filters: ['restricted' => \FILTER_VALIDATE_BOOL])] will only apply this filter on therestricted parameter.

TBH, I think that this mapping will mainly be used to cast string to boolean since other types (ex: string to int) are already handled by the Serializer.

Copy link

Member

nicolas-grekas commentedMar 5, 2024

Ah OK, so "restricted" is the name of the property. This wasn't obvious to me.

It's a bit of a shame that a type that is already clearly described on the DTO need to be specified again somewhere else.

This maps to the serializer component, which could have a mode to do the cast on its own, don't you think?

Copy link

ContributorAuthor

Jean-Beru commentedMar 5, 2024•
edited
Loading

It's a bit of a shame that a type that is already clearly described on the DTO need to be specified again somewhere else.

I totally agree.

This maps to the serializer component, which could have a mode to do the cast on its own, don't you think?

We could pass[FILTER_BOOL => value] in the context to apply this filter. Should this filter be applied by default when using#[MapQueryString] ? Current request parameters likeactive=false will be cast tofalse in place oftrue.

Copy link

Member

nicolas-grekas commentedMar 5, 2024•
edited
Loading

Yes, that'd be better IMHO. Dunno if bool is the only concerned type of if other ones could benefit from using filter_var to do the casting. Would need to be investigated.
Then yes, I would enable that flag for MapQueryString, but also for MapRequestPayloadwhen the payload is a form submission.

Jean-Beru requested a review fromdunglas as acode owner

March 6, 2024 09:34

Jean-Beru changed the title~~[HttpKernel] allow boolean argument support for MapQueryString~~[HttpKernel][Serializer] allow boolean argument support for MapQueryString

Mar 6, 2024

Copy link

ContributorAuthor

Jean-Beru commentedMar 6, 2024

I updated this PR and its description this way with a newAbstractNormalizer::FILTER_BOOL context option.

nicolas-grekas approved these changes

Mar 13, 2024

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I like that :)

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Mar 13, 2024

carsonbot changed the title~~[HttpKernel][Serializer] allow boolean argument support for MapQueryString~~[HttpKernel] allow boolean argument support for MapQueryString

Mar 13, 2024

fabpot approved these changes

Mar 17, 2024

View reviewed changes

fabpot requested changes

Mar 17, 2024

View reviewed changes

src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php Outdated


		returnfilter_var($value, \FILTER_VALIDATE_BOOL, \FILTER_NULL_ON_FAILURE) ??$value;
		return$value;
		}

Copy link

Member

fabpotMar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks wrong :)

carsonbot added Status: Needs Work and removed Status: Reviewed labels

Mar 17, 2024

[HttpKernel] allow boolean argument support for MapQueryString

5c20295

fabpot force-pushed thefix/53616-MapQueryString-with-bool branch frombe335e6 to5c20295Compare

March 17, 2024 14:48

Copy link

Member

fabpot commentedMar 17, 2024

Thank you@Jean-Beru.

fabpot merged commitd7d670e intosymfony:7.1

Mar 17, 2024

fabpot mentioned this pull request

Mar 17, 2024

[HttpKernel] allow boolean argument support for MapQueryStringsymfony/symfony-docs#19682

Closed

symfony-splitter pushed a commit to symfony/http-kernel that referenced this pull request

Mar 17, 2024

feature #54153 [HttpKernel] allow boolean argument support for MapQue…

7733558

…ryString (Jean-Beru)This PR was squashed before being merged into the 7.1 branch.Discussion----------[HttpKernel] allow boolean argument support for MapQueryString| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | Fix #53616| License       | MIT~~Add a `$filters` argument to the `MapQueryString` attribute to allow filtering parameters using `filter_var` function. It will be useful to cast "false", "0", "no" or "off" to `false` before denormalizing the request data.~~**EDIT**Add a `AbstractNormalizer::FILTER_BOOL` context option to cast to a boolean using the `filter_var` function with the `\FILTER_VALIDATE_BOOL` filter (seehttps://www.php.net/manual/fr/filter.filters.validate.php).`MapQueryString` will use this context option when denormalizing the query string.`MapPayload` also but only with data coming from a form.See the relative comment:symfony/symfony#54153 (comment)Example:```phpfinal readonly class SearchDto{    public function __construct(public ?bool $restricted = null)    {    }}final class MyController{    #[Route(name: 'search', path: '/search')]    public function __invoke(#[MapQueryString] SearchDto $search): Response    {        // /search?restricted=        // "true", "1", "yes" and "on" will be cast to true        // "false", "0", "no", "off" and "" will be cast to false        // other will be cast to null    }}```Commits-------5c20295662 [HttpKernel] allow boolean argument support for MapQueryString

symfony-splitter pushed a commit to symfony/serializer that referenced this pull request

Mar 17, 2024

feature #54153 [HttpKernel] allow boolean argument support for MapQue…

5b73e6b

…ryString (Jean-Beru)This PR was squashed before being merged into the 7.1 branch.Discussion----------[HttpKernel] allow boolean argument support for MapQueryString| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | Fix #53616| License       | MIT~~Add a `$filters` argument to the `MapQueryString` attribute to allow filtering parameters using `filter_var` function. It will be useful to cast "false", "0", "no" or "off" to `false` before denormalizing the request data.~~**EDIT**Add a `AbstractNormalizer::FILTER_BOOL` context option to cast to a boolean using the `filter_var` function with the `\FILTER_VALIDATE_BOOL` filter (seehttps://www.php.net/manual/fr/filter.filters.validate.php).`MapQueryString` will use this context option when denormalizing the query string.`MapPayload` also but only with data coming from a form.See the relative comment:symfony/symfony#54153 (comment)Example:```phpfinal readonly class SearchDto{    public function __construct(public ?bool $restricted = null)    {    }}final class MyController{    #[Route(name: 'search', path: '/search')]    public function __invoke(#[MapQueryString] SearchDto $search): Response    {        // /search?restricted=        // "true", "1", "yes" and "on" will be cast to true        // "false", "0", "no", "off" and "" will be cast to false        // other will be cast to null    }}```Commits-------5c20295662 [HttpKernel] allow boolean argument support for MapQueryString