symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.65k stars 9.43k forks source link

Bug in lazy-loading group property on validator component #45042

Closed houssemz closed 2 years ago

houssemz commented 2 years ago

Symfony version(s) affected

^4.4.34

Description

I am using Symfony v4.4.33 and Sonata. When I update Symfony to ^4.4.34, I encountered a bug in group property in the Constraint class in the validator component.

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 3.107.0 The missing Symfony Admin Generator sonata-project/article-bundle 1.6.1 Advanced article management sonata-project/block-bundle 3.23.3 Symfony SonataBlockBundle sonata-project/cache 2.0.1 Cache library sonata-project/cache-bundle 3.3.0 This bundle provides caching services sonata-project/classification-bundle 3.18.0 Symfony SonataClassificationBundle sonata-project/datagrid-bundle 3.4.2 Symfony SonataDatagridBundle sonata-project/doctrine-extensions 1.15.0 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 3.35.0 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/entity-audit-bundle 1.3.2 Audit for Doctrine Entities sonata-project/exporter 2.8.0 Lightweight Exporter library sonata-project/form-extensions 1.12.1 Symfony form extensions sonata-project/formatter-bundle 4.6.1 Symfony SonataFormatterBundle sonata-project/intl-bundle 2.11.1 Symfony SonataIntlBundle sonata-project/media-bundle 3.36.0 Symfony SonataMediaBundle sonata-project/notification-bundle 3.13.0 Symfony SonataNotificationBundle sonata-project/page-bundle 3.24.0 This bundle provides a Site and Page management through container and block services sonata-project/seo-bundle 2.15.0 Symfony SonataSeoBundle sonata-project/translation-bundle 2.9.1 SonataTranslationBundle sonata-project/twig-extensions 1.9.0 Sonata twig extensions sonata-project/user-bundle 4.13.0 Symfony SonataUserBundle ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/amazon-sqs-messenger v5.3.4 Symfony Amazon SQS extension Messenger Bridge symfony/contracts v1.1.10 A set of abstractions extracted out of the Symfony components symfony/deprecation-contracts v2.4.0 A generic function and convention to trigger deprecation notices symfony/monolog-bundle v3.7.0 Symfony MonologBundle symfony/phpunit-bridge v5.3.7 Provides utilities for PHPUnit, especially user deprecation notices management symfony/polyfill-ctype v1.23.0 Symfony polyfill for ctype functions symfony/polyfill-iconv v1.23.0 Symfony polyfill for the Iconv extension symfony/polyfill-intl-grapheme v1.23.1 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu v1.23.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn v1.23.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer v1.23.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.23.1 Symfony polyfill for the Mbstring extension symfony/polyfill-php72 v1.23.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions symfony/polyfill-php73 v1.23.0 Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions symfony/polyfill-php80 v1.23.1 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions symfony/polyfill-php81 v1.23.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions symfony/security-acl v3.2.0 Symfony Security Component - ACL (Access Control List) symfony/string v5.3.7 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way symfony/swiftmailer-bundle v3.5.2 Symfony SwiftmailerBundle symfony/symfony v4.4.36 The Symfony PHP framework ```

PHP version

$ php -v
PHP 7.4.25 (cli) (built: Oct 22 2021 18:03:01) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.25, Copyright (c), by Zend Technologies

How to reproduce

In this video, I tried to report haw to report this bug

https://user-images.githubusercontent.com/6786605/149780709-5b8e14ca-5e13-409c-991d-95aa5fc19cbe.mov

Update: @nicolas-grekas the how to reproduce is as bellow:

Here is my composer.json:

{
    "type": "project",
    "license": "proprietary",
    "require": {
        "php": ">=7.1.3",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "doctrine/annotations": "^1.0",
        "doctrine/doctrine-bundle": "^2.5",
        "doctrine/doctrine-migrations-bundle": "^3.2",
        "doctrine/orm": "^2.11",
        "phpdocumentor/reflection-docblock": "^5.3",
        "sensio/framework-extra-bundle": "^5.1",
        "sonata-project/admin-bundle": "^3.88",
        "sonata-project/block-bundle": "^3.21",
        "sonata-project/doctrine-orm-admin-bundle": "^3",
        "sonata-project/form-extensions": "^1.13",
        "symfony/asset": "4.4.*",
        "symfony/console": "4.4.*",
        "symfony/dotenv": "4.4.*",
        "symfony/expression-language": "4.4.*",
        "symfony/flex": "^1.3.1",
        "symfony/form": "4.4.*",
        "symfony/framework-bundle": "4.4.*",
        "symfony/http-client": "4.4.*",
        "symfony/intl": "4.4.*",
        "symfony/mailer": "4.4.*",
        "symfony/monolog-bundle": "^3.1",
        "symfony/process": "4.4.*",
        "symfony/property-access": "4.4.*",
        "symfony/property-info": "4.4.*",
        "symfony/proxy-manager-bridge": "4.4.*",
        "symfony/security-bundle": "4.4.*",
        "symfony/serializer": "4.4.*",
        "symfony/translation": "4.4.*",
        "symfony/twig-bundle": "4.4.*",
        "symfony/validator": "4.4.*",
        "symfony/web-link": "4.4.*",
        "symfony/yaml": "4.4.*",
        "twig/extra-bundle": "^2.12|^3.0",
        "twig/twig": "^2.12|^3.0"
    },
    "require-dev": {
        "phpunit/phpunit": "^9.5",
        "symfony/browser-kit": "4.4.*",
        "symfony/css-selector": "4.4.*",
        "symfony/debug-bundle": "4.4.*",
        "symfony/maker-bundle": "^1.0",
        "symfony/phpunit-bridge": "^6.0",
        "symfony/stopwatch": "4.4.*",
        "symfony/web-profiler-bundle": "4.4.*"
    },
    "config": {
        "allow-plugins": {
            "composer/package-versions-deprecated": true,
            "symfony/flex": true
        },
        "preferred-install": {
            "*": "dist"
        },
        "sort-packages": true
    },
    "autoload": {
        "psr-4": {
            "App\\": "src/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "App\\Tests\\": "tests/"
        }
    },
    "replace": {
        "paragonie/random_compat": "2.*",
        "symfony/polyfill-ctype": "*",
        "symfony/polyfill-iconv": "*",
        "symfony/polyfill-php71": "*",
        "symfony/polyfill-php70": "*",
        "symfony/polyfill-php56": "*"
    },
    "scripts": {
        "auto-scripts": {
            "cache:clear": "symfony-cmd",
            "assets:install %PUBLIC_DIR%": "symfony-cmd"
        },
        "post-install-cmd": [
            "@auto-scripts"
        ],
        "post-update-cmd": [
            "@auto-scripts"
        ]
    },
    "conflict": {
        "symfony/symfony": "*"
    },
    "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "4.4.*"
        }
    }
}

And my entity:

<?php

declare(strict_types=1);

use Doctrine\ORM\Mapping as ORM;
use Gedmo\Mapping\Annotation as Gedmo;
use Sonata\Form\Validator\Constraints\InlineConstraint;

/**
 * @ORM\Entity
 * @ORM\Table(name="page__bloc")
 * @ORM\HasLifecycleCallbacks
 * @Gedmo\TranslationEntity(class="CreativeMedia\ORM\Entity\BlockTranslation")
 * @InlineConstraint(service="sonata.block.manager", method="validate")
 */
class Block
{
    /**
     * @ORM\Id
     * @ORM\Column(name="id", type="integer")
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    protected int $id;

    /**
     * @ORM\Column(name="name", type="string", length=255, nullable=true)
     */
    protected ?string $name;

    /**
     * @ORM\Column(name="type", type="string", length=255)
     */
    protected ?string $type;

    /**
     * @ORM\Column(name="settings", type="json")
     * @Gedmo\Translatable(fallback=true)
     */
    protected array $settings;

    /**
     * @var bool|null
     * @ORM\Column(name="enabled", type="boolean", nullable=true)
     */
    protected ?bool $enabled;
}

Possible Solution

When I used the PHP style rather than the annotation style I bypassed the bug

use Sonata\Form\Validator\Constraints\InlineConstraint;
+use Symfony\Component\Validator\Mapping\ClassMetadata;

 /**
@@ -30,8 +31,6 @@ use Sonata\TranslationBundle\Model\Gedmo\TranslatableInterface;
  * @ORM\Table(name="page__bloc")
  * @ORM\HasLifecycleCallbacks
  *
- * @InlineConstraint(service="sonata.block.manager", method="validate")
  * @Gedmo\TranslationEntity(class="CreativeMedia\ORM\Entity\BlockTranslation")
@@ -204,4 +203,10 @@ class Block extends BaseBlock implements TranslatableInterface
     {
         $this->updatedAt = new \DateTime();
     }
+
+    public static function loadValidatorMetadata(ClassMetadata $metadata): void
+    {
+        $inlineConstraint = new InlineConstraint(['service' => 'sonata.block.manager', 'method' => 'validate']);
+        $metadata->addConstraint($inlineConstraint);
+    }
 }

Additional Context

No response

stof commented 2 years ago

Do you have some custom Constraint child classes that don't call the parent constructor from Symfony ?

houssemz commented 2 years ago

Do you have some custom Constraint child classes that don't call the parent constructor from Symfony ?

No, I don't have

nicolas-grekas commented 2 years ago

Can you please provide a reproducer (a small app we could clone locally), or better: a failing test case?

houssemz commented 2 years ago

@nicolas-grekas Thank you for your time.

In fact, I tried to give you a prototype but that was hard to escape some client data. Anyway, I did an upgrade to Symfony 4.4.37, and thanks to your commit, I changed the way that I validate my entity.

Here is some code:

use Sonata\Form\Validator\Constraints\InlineConstraint;
+use Symfony\Component\Validator\Mapping\ClassMetadata;

 /**
@@ -30,8 +31,6 @@ use Sonata\TranslationBundle\Model\Gedmo\TranslatableInterface;
  * @ORM\Table(name="page__bloc")
  * @ORM\HasLifecycleCallbacks
  *
- * @InlineConstraint(service="sonata.block.manager", method="validate")
  * @Gedmo\TranslationEntity(class="CreativeMedia\ORM\Entity\BlockTranslation")
@@ -204,4 +203,10 @@ class Block extends BaseBlock implements TranslatableInterface
     {
         $this->updatedAt = new \DateTime();
     }
+
+    public static function loadValidatorMetadata(ClassMetadata $metadata): void
+    {
+        $inlineConstraint = new InlineConstraint(['service' => 'sonata.block.manager', 'method' => 'validate']);
+        $metadata->addConstraint($inlineConstraint);
+    }
 }

Note that, before instantiating the constraint, I use the validation with annotation style.

Hope I am clear and convincing :)

Still, I have some questions. Since your commit, doesn't mean that we should favor the PHP style rather than the annotation style?

houssemz commented 2 years ago

@nicolas-grekas I updated this issue (please see how to reproduce and the possible solution sections)

nicolas-grekas commented 2 years ago

Thanks for the details. I didn't reproduce, but I have a guess that the issue is in sonata-project/form-extensions: their InlineConstraint class has a sleep() method that doesn't call `parent::sleep()`. Please report the issue there.

nicolas-grekas commented 2 years ago

call the parent or add these lines in the body of the InlineConstraint::__sleep method (you can maybe confirm the fix?)

// Initialize "groups" option if it is not set
$this->groups;
houssemz commented 2 years ago

@nicolas-grekas I confirm that is fixed. Thank you :)