[Validator] Dump Valid constraints on debug command #48840

macintoshplus · 2022-12-31T20:33:07Z

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #46544
License	MIT

The command debug:validator doesn't dump the Valid constraints with the default group.

The Valid constraints with default group change the property CascadingStrategy and the TraversalStrategy on property metadata.

This patch adds the Valid constraint on debug output if the CascadingStrategy is the same as CASCADE.

carsonbot · 2022-12-31T20:33:12Z

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (see https://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
Features and deprecations must be submitted against the 6.3 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

carsonbot · 2023-01-01T16:36:58Z

Hey!

I think @loic425 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

WebMamba · 2023-01-01T22:05:46Z

src/Symfony/Component/Validator/Command/DebugCommand.php

+            if (CascadingStrategy::CASCADE === $metadata->getCascadingStrategy()) {
+                $constraint = new Valid([], [], null, TraversalStrategy::IMPLICIT === $metadata->getTraversalStrategy());
+                $data[] = [
+                    'class' => Valid::class,
+                    'groups' => [$constraint::DEFAULT_GROUP],
+                    'options' => $this->getConstraintOptions($constraint),
+                ];


Hey, @macintoshplus thanks for your this PR! It's working great! But are we sure only Valid constraints have the CascadingStrategy::CASCADE? For example, if you create custom constraints that extend the Valid constraint you gonna have this CascadingStrategy::CASCADE, but you don't want to add here the Valid constraint but the custom one.

It's true, if you extend Valid annotation without changing anything, the behavior is the same as using the Valid constraint. And, your custom constraint is changed to Valid on debug command. The debug command cannot know the original constraint class name because Symfony has removed it.

See the code https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Validator/Mapping/GenericMetadata.php#L145-L155

But if you add some options or groups, the original constraint is kept by Symfony and can be displayed on the dump command output.

Symfony changes property mapping configuration with EnableAutoMapping and DisableAutoMapping (I cannot add the constraint in debug command output because the getAutoMappingStrategy function is not defined in MetadataInterface).

@macintoshplus Maybe you can check if the metadata is an instance of ClassMetadata and then getAutoMappingStragegy

It is the same for Cascade and ClassMetadata https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Validator/Mapping/ClassMetadata.php#L210

@loic425 Yes, I can.

During the investigation (thank @HeahDude), I found some other constraints with a particular behavior.

Without trying to retrieve the original constraint to display, it's possible to display class or property metadata properties differently? How?

alexislefebvre · 2023-01-03T19:56:59Z

Nitpicking: the title of this PR mentions “constaints” instead of “constraints”.

nicolas-grekas · 2023-04-18T13:40:09Z

What can we do here to unlock the PR?

macintoshplus · 2023-04-18T14:12:56Z

What can we do here to unlock the PR?

Hi Nicolas,

Thank you for your interest in this PR.

What way to display the validation constraint that changes parameters and has been removed by Symfony?

I have tried to display Valid assert in the dump command but, some others constraint is in the same situation.

IMHO two choices:

display parameters on top
display the Symfony native constraints used to change the parameters (my first choice in this PR)

I'm available on Symfony Slack (Jean-Baptiste N.) or Twitter (@jbnahan69) to discuss this in French if you want.

macintoshplus · 2023-07-11T08:14:20Z

Hi @nicolas-grekas, @loic425,

I have changed the property options displayed on debug output.

The 3 options (cascadeStrategy, autoMappingStrategy, traversalStrategy) it's now visible before property validators.

Symfony\Component\Validator\Tests\Dummy\DummyClassOne
-----------------------------------------------------

+---------------+----------------------------------------------------+---------+------------------------------------------------------------+
| Property      | Name                                               | Groups  | Options                                                    |
+---------------+----------------------------------------------------+---------+------------------------------------------------------------+
| -             | Symfony\Component\Validator\Constraints\Expression | Default | [                                                          |
|               |                                                    |         |   "expression" => "1 + 1 = 2",                             |
|               |                                                    |         |   "message" => "This value is not valid.",                 |
|               |                                                    |         |   "payload" => null,                                       |
|               |                                                    |         |   "values" => []                                           |
|               |                                                    |         | ]                                                          |
| firstArgument | property options                                   |         | [                                                          |
|               |                                                    |         |   "cascadeStrategy" => "Cascade",                          |
|               |                                                    |         |   "autoMappingStrategy" => "Not supported",                |
|               |                                                    |         |   "traversalStrategy" => "Implicit"                        |
|               |                                                    |         | ]                                                          |
| firstArgument | Symfony\Component\Validator\Constraints\NotBlank   | Default | [                                                          |
|               |                                                    |         |   "allowNull" => false,                                    |
|               |                                                    |         |   "message" => "This value should not be blank.",          |
|               |                                                    |         |   "normalizer" => null,                                    |
|               |                                                    |         |   "payload" => null                                        |
|               |                                                    |         | ]                                                          |
| firstArgument | Symfony\Component\Validator\Constraints\Email      | Default | [                                                          |
|               |                                                    |         |   "message" => "This value is not a valid email address.", |
|               |                                                    |         |   "mode" => null,                                          |
|               |                                                    |         |   "normalizer" => null,                                    |
|               |                                                    |         |   "payload" => null                                        |
|               |                                                    |         | ]                                                          |
+---------------+----------------------------------------------------+---------+------------------------------------------------------------+

loic425 · 2023-07-11T12:54:37Z

@nicolas-grekas I think we need this PR first to be sure of the behaviour.

…d tests (loic425) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Validator] Do not mock metadata factory on debug command tests | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | not really, but it will be usefull to fix that command | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR |  Before #48840 I think this one could improve the tests. Commits ------- de6d15f [Validator] Do not mock metadata factory on debug command tests

macintoshplus · 2023-07-16T18:38:49Z

@loic425, @nicolas-grekas I have rebased this PR and update tests.

nicolas-grekas · 2023-08-14T13:06:22Z

Thank you @macintoshplus.

carsonbot added Status: Needs Review Bug labels Dec 31, 2022

carsonbot added this to the 5.4 milestone Dec 31, 2022

macintoshplus mentioned this pull request Dec 31, 2022

incomplete result of debug:validator command #46544

Closed

OskarStark added the Validator label Jan 1, 2023

carsonbot changed the title ~~Dump Valid constaints on debug command #46544~~ [Validator] Dump Valid constaints on debug command #46544 Jan 1, 2023

OskarStark changed the title ~~[Validator] Dump Valid constaints on debug command #46544~~ [Validator] Dump Valid constaints on debug command Jan 1, 2023

WebMamba reviewed Jan 1, 2023

View reviewed changes

macintoshplus changed the title ~~[Validator] Dump Valid constaints on debug command~~ [Validator] Dump Valid constraints on debug command Jan 3, 2023

macintoshplus requested review from lyrixx, yceruto, wouterj, chalasr, dunglas, OskarStark and xabbuh as code owners July 11, 2023 08:16

loic425 mentioned this pull request Jul 11, 2023

[Validator] Do not mock metadata factory on debug command tests #50935

Merged

Dump Valid constaints on debug command #46544

a99b707

nicolas-grekas merged commit 3bff6fe into symfony:5.4 Aug 14, 2023

macintoshplus deleted the patch-1 branch August 14, 2023 22:18

This was referenced Aug 26, 2023

Release v5.4.28 #51495

Merged

Release v6.3.4 #51496

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] Dump Valid constraints on debug command #48840

[Validator] Dump Valid constraints on debug command #48840

macintoshplus commented Dec 31, 2022 •

edited by OskarStark

Loading

carsonbot commented Dec 31, 2022

carsonbot commented Jan 1, 2023

WebMamba Jan 1, 2023

macintoshplus Jan 2, 2023 •

edited

Loading

loic425 Jan 3, 2023

HeahDude Jan 3, 2023

macintoshplus Jan 3, 2023

alexislefebvre commented Jan 3, 2023

nicolas-grekas commented Apr 18, 2023

macintoshplus commented Apr 18, 2023

macintoshplus commented Jul 11, 2023

loic425 commented Jul 11, 2023 •

edited

Loading

macintoshplus commented Jul 16, 2023

nicolas-grekas commented Aug 14, 2023

[Validator] Dump Valid constraints on debug command #48840

[Validator] Dump Valid constraints on debug command #48840

Conversation

macintoshplus commented Dec 31, 2022 • edited by OskarStark Loading

carsonbot commented Dec 31, 2022

carsonbot commented Jan 1, 2023

WebMamba Jan 1, 2023

Choose a reason for hiding this comment

macintoshplus Jan 2, 2023 • edited Loading

Choose a reason for hiding this comment

loic425 Jan 3, 2023

Choose a reason for hiding this comment

HeahDude Jan 3, 2023

Choose a reason for hiding this comment

macintoshplus Jan 3, 2023

Choose a reason for hiding this comment

alexislefebvre commented Jan 3, 2023

nicolas-grekas commented Apr 18, 2023

macintoshplus commented Apr 18, 2023

macintoshplus commented Jul 11, 2023

loic425 commented Jul 11, 2023 • edited Loading

macintoshplus commented Jul 16, 2023

nicolas-grekas commented Aug 14, 2023

macintoshplus commented Dec 31, 2022 •

edited by OskarStark

Loading

macintoshplus Jan 2, 2023 •

edited

Loading

loic425 commented Jul 11, 2023 •

edited

Loading