Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Add MapRequestPayload tips with built-in types#19590
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.1
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
If you have any feedback@nicolas-grekas@fabpot@dunglas |
Shouldn't we not expose internal info instead? This recommendation looks scary without really providing the solution nor the why so I don't think it's how we want to deal with this. Maybe worth opening an issue on symfony/symfony? Or a PR if you have a proposal on the topic? |
AurelienPillevesse commentedFeb 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Currently, the solution with an Enum in a #[Assert\Choice(callback: [YourEnum::class,'values'])public string$myInputAttribute; And in the enum : publicstaticfunctionvalues():array{returnarray_column(self::cases(),'value');} This solution seems OK for me as we want to map a string from JSON (for example) to a string in our input. This string in our input can have all values from our Enum. Would you like me to add this example to the "tips" in the case of an enum in a #[MapRequestPayload] class? |
AurelienPillevesse commentedFeb 27, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Is it better now ? |
fcb2b79
to0408c0a
CompareI would rephrase this like this 🙂 Tell me what you think! diff --git a/controller.rst b/controller.rstdiff --git a/controller.rst b/controller.rstindex 392ed827b..255ec6246 100644--- a/controller.rst+++ b/controller.rst@@ -539,10 +539,18 @@ if you want to map a nested array of specific DTOs:: ) {} }-.. tip::+.. caution::++ If you're using typed properties with ``MapRequestPayload```, it is+ recommended to use built-in types like ``int``, ``bool`` or ``string`` for+ mapping. Using custom types could expose your application implementation in+ errors during denormalization. For example, validating an enum when using+ ``#[MapRequestPayload]`` could look like this::- If using typed properties with `MapRequestPayload`, it's recommanded to use built-in types (like `int`, `bool`, `string`...) for mapping. Using custom types can expose your application implementation outside with errors during denormalization.- Validating an Enum in your `#[MapRequestPayload]` class should look like this::+ // src/Controller/LuckyController.php+ use App\Model\MyInput;+ use Symfony\Component\HttpFoundation\Response;+ use Symfony\Component\HttpKernel\Attribute\MapRequestPayload; class LuckyController {@@ -553,12 +561,14 @@ if you want to map a nested array of specific DTOs:: } }+ // src/Model/MyInput.php class MyInput { #[Assert\Choice(callback: [MyEnum::class, 'values'])] public string $myInputAttribute; }+ // src/Model/MyEnum.php enum MyEnum: string { case FIRST_CASE = 'first_case'; |
AurelienPillevesse commentedMar 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@alexandre-daubois seems good for me, I applied the changes |
Uh oh!
There was an error while loading.Please reload this page.
Added a tip for using built-in types only with
MapRequestPayload
, to avoid revealing the internal implementation of your applicationsEdit : I tried to target to 7.0 because feature is already available but lot of commits involved. Currently stay for 7.1, let you decide (or I can make a new PR to target 6.4 if you want)