Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
AurelienPillevesse wants to merge3 commits intosymfony:7.1
base:7.1
Choose a base branch
Loading
fromAurelienPillevesse:patch-1

Conversation

AurelienPillevesse
Copy link
Contributor

@AurelienPillevesseAurelienPillevesse commentedFeb 23, 2024
edited
Loading

Added a tip for using built-in types only withMapRequestPayload, to avoid revealing the internal implementation of your applications

Edit : 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)

qdequippe, pierreflaudias, LudoWeb, and Koc reacted with thumbs up emoji
@carsonbotcarsonbot added this to the7.1 milestoneFeb 23, 2024
@AurelienPillevesseAurelienPillevesse changed the titleAdd MapRequestPayload tips with built-in typedAdd MapRequestPayload tips with built-in typesFeb 23, 2024
@AurelienPillevesseAurelienPillevesse changed the base branch from7.1 to7.0February 26, 2024 12:57
@AurelienPillevesseAurelienPillevesse changed the base branch from7.0 to7.1February 26, 2024 13:07
@AurelienPillevesse
Copy link
ContributorAuthor

If you have any feedback@nicolas-grekas@fabpot@dunglas

@nicolas-grekas
Copy link
Member

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
Copy link
ContributorAuthor

AurelienPillevesse commentedFeb 27, 2024
edited
Loading

Currently, the solution with an Enum in a#[MapRequestPayload] class is to do like this :

#[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
Copy link
ContributorAuthor

AurelienPillevesse commentedFeb 27, 2024
edited
Loading

Is it better now ?

@alexandre-daubois
Copy link
Member

I 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
Copy link
ContributorAuthor

AurelienPillevesse commentedMar 12, 2024
edited
Loading

@alexandre-daubois seems good for me, I applied the changes
What do you think about releasing it to 6.4 or 7.0 to be visible on documentation when validated ?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees
No one assigned
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.

4 participants
@AurelienPillevesse@nicolas-grekas@alexandre-daubois@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp