-
-
Notifications
You must be signed in to change notification settings - Fork 5.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 our terms of service and privacy 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?
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? |
Currently, the solution with an Enum in a #[Assert\Choice(callback: [YourEnum::class, 'values'])
public string $myInputAttribute; And in the enum : public static function values(): array
{
return array_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? |
Is it better now ? |
fcb2b79
to
0408c0a
Compare
I would rephrase this like this 🙂 Tell me what you think! diff --git a/controller.rst b/controller.rst
diff --git a/controller.rst b/controller.rst
index 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'; |
@alexandre-daubois seems good for me, I applied the changes |
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)