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

[Validator] IntroduceBackedEnumValue constraint#54226

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 merge5 commits intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromAurelienPillevesse:introduce-enum-constraint

Conversation

AurelienPillevesse
Copy link
Contributor

@AurelienPillevesseAurelienPillevesse commentedMar 10, 2024•
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
Issues-
LicenseMIT

My use case: I receive data in my DTOs that we only want if this value exists in an Enum and reject the whole thing at validation otherwise.

Koc, mdeboer, welcoMattic, dderenko, deguif, floristics, alexandre-le-borgne, sfmok, ChristopheCavanna, yceruto, and 3 more reacted with thumbs up emoji
@carsonbotcarsonbot added Status: Needs Review Feature RFCRFC = Request For Comments (proposals about features that you want to be discussed) Validator labelsMar 10, 2024
@carsonbotcarsonbot added this to the7.1 milestoneMar 10, 2024
@carsonbotcarsonbot changed the title[RFC][Validator] Introduce Enum constraint[Validator] Introduce Enum constraintMar 10, 2024
@AurelienPillevesseAurelienPillevesse changed the title[Validator] Introduce Enum constraint[Validator] IntroduceEnum constraintMar 10, 2024
@derrabus
Copy link
Member

Did you read the discussion on#43047?

@AurelienPillevesse
Copy link
ContributorAuthor

Didn't see it before, my bad

@AurelienPillevesse
Copy link
ContributorAuthor

It's a dedicated constraint so why not but I understand if your point of view is to not accept it and adding the code below (as mentioned in the other discussion) if people want to accept an Enum value with#[MapRequestPayload] for example :

publicstaticfunctionvalues():array{returnarray_column(self::cases(),'value');}

Because you are speaking about this topic, can you maybe give me feedback about this PR :symfony/symfony-docs#19590 (if it's added in the documentation, we will probably less try to find a way to achieve it)

@derrabus
Copy link
Member

I understand if your point of view is to not accept it

I didn't say that. It's just that there has been a PR on that topic a while ago, and if we revisit the topic, we should take that prior discussion into account.

@AurelienPillevesseAurelienPillevesse changed the title[Validator] IntroduceEnum constraint[Validator] IntroduceBackedEnumValue constraintMar 11, 2024
@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@AurelienPillevesseAurelienPillevesseforce-pushed theintroduce-enum-constraint branch 4 times, most recently frome48670d toedf0705CompareDecember 10, 2024 19:04
@AurelienPillevesse
Copy link
ContributorAuthor

AurelienPillevesse commentedDec 12, 2024•
edited
Loading

@fabpot@nicolas-grekas
When I add my line in the changelog, it's telling me that I've a conflict.
If I'm fixing the conflict with github UI, the conflict is solved but it merge the branch 7.3 and fabbot is not happy

Can you tell me how to handle it properly?

@stof
Copy link
Member

@AurelienPillevesse you should rebase your branch on top of the latest upstream 7.3 branch instead of merging. Seehttps://symfony.com/doc/current/contributing/code/pull_requests.html#rebase-your-pull-request for the detailed explanation.

@AurelienPillevesse
Copy link
ContributorAuthor

Thanks@stof

parent::__construct([], $groups, $payload);

if (!is_a($type, \BackedEnum::class, true)) {
throw new ConstraintDefinitionException(\sprintf('The "type" must be a \BackedEnum, got "%s".', get_debug_type($type)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
thrownewConstraintDefinitionException(\sprintf('The "type" must be a \BackedEnum, got "%s".',get_debug_type($type)));
thrownewConstraintDefinitionException(\sprintf('The "type" must be a"\BackedEnum", got "%s".',get_debug_type($type)));

) {
parent::__construct([], $groups, $payload);

if (!is_a($type, \BackedEnum::class, true)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!is_a($type, \BackedEnum::class,true)) {
if (!is_subclass_of($type, \BackedEnum::class,true)) {

Copy link
Member

Choose a reason for hiding this comment

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

is_a() looks okay to me?

];

/**
* @param class-string<\BackedEnum> $type the type of the enum
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param class-string<\BackedEnum>$type thetype of theenum
* @param class-string<\BackedEnum>$className theenum class name

Copy link
Member

Choose a reason for hiding this comment

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

first letter should be uppercase also

public string $type,
public array $except = [],
public string $message = 'The value you selected is not a valid choice.',
public string $typeMessage = 'This value should be of type {{ type }}.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public string$typeMessage ='This value should be of type {{ type }}.',
public string$invalidValueTypeMessage ='The value should be of type {{ type }}.',

public function __construct(
public string $type,
public array $except = [],
public string $message = 'The value you selected is not a valid choice.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public string$message ='The valueyou selectedis not avalid choice.',
public string$invalidValueMessage ='The value is not abacking value of {{ className }}.',

Comment on lines +48 to +49
->setParameter('{{ value }}', $this->formatValue($value))
->setParameter('{{ choices }}', $this->formatValidCases($constraint))
Copy link
Contributor

Choose a reason for hiding this comment

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

@derrabus is right, you only need to define what is in the constraint message

Copy link
ContributorAuthor

@AurelienPillevesseAurelienPillevesseDec 24, 2024•
edited
Loading

Choose a reason for hiding this comment

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

It's build in a similar way as Choice constraint.

Message :

publicstring$message ='The value you selected is not a valid choice.';

Validator :

$this->context->buildViolation($constraint->message)
->setParameter('{{ value }}',$this->formatValue($value))
->setParameter('{{ choices }}',$this->formatValues($choices))
->setCode(Choice::NO_SUCH_CHOICE_ERROR)
->addViolation();

Thevalue orchoice values are not used by default. If you override the default message and use{{ value }} or{{ choice }} on it, it will be replaced in your new message.

}

try {
$enumTypeValue = $constraint->type::tryFrom($value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$enumTypeValue =$constraint->type::tryFrom($value);
$enumValue =$constraint->type::tryFrom($value);

Comment on lines +67 to +73
return $this->formatValues(array_map(
static fn (\BackedEnum $case) => $case->value,
array_filter(
$constraint->type::cases(),
static fn (\BackedEnum $currentValue) => !\in_array($currentValue, $constraint->except, true),
)
));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return$this->formatValues(array_map(
staticfn (\BackedEnum$case) =>$case->value,
array_filter(
$constraint->type::cases(),
staticfn (\BackedEnum$currentValue) => !\in_array($currentValue,$constraint->except,true),
)
));
return$this->formatValues(array_column(
array_filter(
$constraint->type::cases(),
staticfn (\BackedEnum$currentValue) => !\in_array($currentValue,$constraint->except,true),
),
'value',
));

@sfmok
Copy link
Contributor

I think this change requires settingALLOW_INVALID_VALUES to true by default in

if ($context[self::ALLOW_INVALID_VALUES] ??false) {
Otherwise, a typical use of this validator in APIs will throw aNotNormalizableValueException unlessALLOW_INVALID_VALUES is explicitly set to true in the context, which does not make sense IMO.

@stof
Copy link
Member

@sfmok this PR is totally unrelated to that normalizer. The constraint added here is meant to be used in case your DTO expects a scalar value that must match the value associated with a case of the backed enum. TheBackedEnumNormalizer is triggered in case the DTO expects a BackedEnum instance.

sfmok and derrabus reacted with thumbs up emoji

@thePanz
Copy link
Contributor

Thanks for pointing that out@stof and@sfmok : we should make it clean from the docs that the case is the following:

class MyDto {    #[Assert\BackedEnumValue(MyStringBackedEnum::class)]    public string $value = null;}
sfmok reacted with thumbs up emoji

@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@zexazexazexa left review comments

@ro0NLro0NLro0NL left review comments

@mdeboermdeboermdeboer left review comments

@mtarldmtarldmtarld left review comments

@ERubanERubanERuban left review comments

@dderenkodderenkodderenko approved these changes

@derrabusderrabusAwaiting requested review from derrabus

@stofstofAwaiting requested review from stof

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@xabbuhxabbuhAwaiting requested review from xabbuh

@chalasrchalasrAwaiting requested review from chalasr

Assignees
No one assigned
Labels
FeatureRFCRFC = Request For Comments (proposals about features that you want to be discussed)Status: Needs ReviewValidator
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

16 participants
@AurelienPillevesse@derrabus@mdeboer@zexa@stof@sfmok@thePanz@OskarStark@ro0NL@xabbuh@mtarld@chalasr@ERuban@dderenko@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp