8000 Regression in validator · Issue #50780 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Regression in validator #50780

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.

Sign up for GitHub

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

Jump to bottom
Closed
garak opened this issue Jun 26, 2023 · 5 comments
Closed

Regression in validator #50780

garak opened this issue Jun 26, 2023 · 5 comments
Labels
Bug Status: Needs Review Validator

Comments

@garak
Copy link
Contributor
garak commented Jun 26, 2023 •
edited
Loading

Symfony version(s) affected

5.4.25, 6.2.12, 6.3.1

Description

Upgrading symfony/validator from 5.4.34 to 5.4.35 breaks validation in a project of mine.
Some properties that have a NotBlank constraint applied are not detected as invalid when submitted without a value.
It seems that the regression was introduced in this change 6a3a4d7#diff-50c23a0a9ba92c8fb19fa501720ad061da508e3ccfd98fbcdfff2b7c8c6e6499 but unfortunately I cannot understand how.

How to reproduce

I'm sorry but it seems not so easy to provide a reproducer. In my project, I use XML validation and the problem appears on a property which is not directly defined in the validated object (it's defined in a parent object)

Possible Solution

Sorry, no clue about this

Additional Context

I isolated the problem during an upgrade, by requiring in my composer.json "symfony/validator": "^5.4,<5.4.25". With this requirement, all my tests pass.
If I replace it with the simple "symfony/validator": "^5.4", the tests break
@garak garak added the Bug label Jun 26, 2023
@rmikalkenas
Copy link
Contributor

Same with the upgrade of symfony/validator v6.2.11 => v6.2.12

@nicolas-grekas looks like it's related with yours changes: symfony/validator@81772c1
once I restore Mapping/ClassMetadata.php to previous changes - everything works as expected

@nicolas-grekas
8000 Copy link
Member

Could anyone come up with a test case to prevent a regression?

@rmikalkenas
Copy link
Contributor

@garak do you have a reproducer? I can try to extract from my project, but it will take some time

@garak
Copy link
Contributor Author
garak commented Jun 26, 2023 •
edited
Loading

Meanwhile I can confirm the same problem with validator 6.3.1 (while 6.3.0 works).
I found a simpler case, which maybe can be used as reproducer.

In the XML validation, I have:

<?xml version="1.0" encoding="UTF-8" ?>
<constraint-mapping xmlns="http://symfony.com/schema/dic/constraint-mapping"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://symfony.com/schema/dic/constraint-mapping https://symfony.com/schema/dic/constraint-mapping/constraint-mapping-1.0.xsd"
>
    <class name="Application\Command\User\AbstractCommand">
        <property name="password">
            <constraint name="Length">
                <option name="min">8</option>
                <option name="max">255</option>
            </constraint>
        </property>
    </class
    <class name="Application\Command\User\CreateCommand">
        <property name="password">
            <constraint name="NotBlank"/>
        </property>
    </class>
</constraint-mapping>

So, when I validate CreateCommand, which extends AbstractCommand, the NotBlank constraint is not applied (while the original Length constraint is)

@rmikalkenas rmikalkenas mentioned this issue Jun 27, 2023
Merged
@TheMilek TheMilek mentioned this issue Jun 27, 2023
Merged
Rafikooo added a commit to Sylius/Sylius that referenced this issue Jun 27, 2023
@Rafikooo
minor #15130 Add conflict to symfony/validator 5.4.25, 6.2.12 and 6.3…
eca88f8 
….1 (TheMilek)

This PR was merged into the 1.12 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.12  <!-- see the comment below -->                  |
| Bug fix?        | yes                                                     |
| New feature?    | no                                                      |
| BC breaks?      | no                                                   |
| Related tickets |    symfony/symfony#50780                   |
| License         | MIT                                                          |

<!--
 - Bug fixes must be submitted against the 1.12 branch
 - Features and deprecations must be submitted against the 1.13 branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->
It's probably related to [this](symfony/validator@81772c1) change

Commits
-------
  Add conflict to symfony/validator 5.4.25, 6.2.12 and 6.3.1 versions
  Update conflicts file
@toastedghost
Copy link

We have noticed the same behaviour in our project.

We use the FOSUserBundle and have extended the user entity.
Validation rules are defined in a "validation.yml" in our project and the FOSUserBundle itself provides validation rules in a "validation.xml". With version 5.4.25 of symfony/validator, only the rules from our file are taken into account for which there are no "property" definitions in the file from the FOSUserBundle. With version 5.4.24, on the other hand, it works correctly, so that the rules from our definition apply as well as those from the FOSUserBundle.

@usu usu mentioned this issue Jul 15, 2023
Merged
1 task
@michaelehrlein michaelehrlein mentioned this issue Jul 20, 2023
Closed
nicolas-grekas added a commit that referenced this issue Jul 20, 2023
@nicolas-grekas
bug #50788 [Validator] Fix regression with class metadatada on parent…
5f08512 
… classes (rmikalkenas)

This PR was merged into the 5.4 branch.

Discussion
----------

[Validator] Fix regression with class metadatada on parent classes

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
|
5EBD
 Tickets       | Fix #50780
| License       | MIT

Commits
-------

e98f5f2 [Validator] Fix regression with class metadatada on parent classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review Validator
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@garak @nicolas-grekas @toastedghost @xabbuh @carsonbot @rmikalkenas
0