AdminType clones admin with subject still present

[7ochem]

Environment

Sonata 4 on Symfony 4.4 on PHP 8.1

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 4.19.0 sonata-project/block-bundle 4.18.0 sonata-project/cache 2.2.0 sonata-project/doctrine-extensions 1.18.1 sonata-project/doctrine-orm-admin-bundle 4.8.0 sonata-project/exporter 3.0.0 sonata-project/form-extensions 1.18.0 sonata-project/twig-extensions 1.12.0 ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/asset v4.4.46 symfony/browser-kit v4.4.44 symfony/cache v4.4.46 symfony/cache-contracts v2.5.2 symfony/config v4.4.44 symfony/console v4.4.45 symfony/css-selector v4.4.44 symfony/debug v4.4.44 symfony/debug-bundle v4.4.37 symfony/dependency-injection v4.4.44 symfony/deprecation-contracts v3.1.1 symfony/doctrine-bridge v4.4.46 symfony/dom-crawler v4.4.45 symfony/dotenv v4.4.37 symfony/error-handler v4.4.44 symfony/event-dispatcher v4.4.44 symfony/event-dispatcher-contracts v1.1.13 symfony/expression-language v4.4.44 symfony/filesystem v4.4.42 symfony/finder v4.4.44 symfony/flex v1.19.3 symfony/form v4.4.46 symfony/framework-bundle v4.4.46 symfony/http-client v4.4.46 symfony/http-client-contracts v2.5.2 symfony/http-foundation v4.4.46 symfony/http-kernel v4.4.46 symfony/inflector v4.4.44 symfony/intl v4.4.44 symfony/mime v4.4.46 symfony/monolog-bridge v4.4.43 symfony/monolog-bundle v3.8.0 symfony/options-resolver v4.4.44 symfony/phpunit-bridge v5.4.11 symfony/polyfill-intl-grapheme v1.26.0 symfony/polyfill-intl-icu v1.26.0 symfony/polyfill-intl-idn v1.26.0 symfony/polyfill-intl-normalizer v1.26.0 symfony/polyfill-mbstring v1.26.0 symfony/process v4.4.44 symfony/property-access v4.4.44 symfony/routing v4.4.44 symfony/security-acl v3.3.2 symfony/security-bundle v4.4.44 symfony/security-core v4.4.46 symfony/security-csrf v4.4.37 symfony/security-guard v4.4.46 symfony/security-http v4.4.44 symfony/serializer v4.4.45 symfony/service-contracts v2.5.2 symfony/stopwatch v4.4.46 symfony/string v6.1.5 symfony/swiftmailer-bundle v3.5.4 symfony/translation v4.4.45 symfony/translation-contracts v2.5.2 symfony/twig-bridge v4.4.45 symfony/twig-bundle v4.4.41 symfony/validator v4.4.46 symfony/var-dumper v4.4.46 symfony/var-exporter v4.4.43 symfony/web-link v4.4.37 symfony/web-profiler-bundle v4.4.44 symfony/yaml v4.4.45 ```

PHP version

$ php -v
PHP 8.1.11 (cli) (built: Sep 29 2022 22:28:23) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.11, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.11, Copyright (c), by Zend Technologies
    with Xdebug v3.1.5, Copyright (c) 2002-2022, by Derick Rethans

Subject

I have an admin class (let's say GenericAdmin) with multiple entities related to it (let's say A and B).

In the AdminType class a clone is being used. It then has a subject set. https://github.com/sonata-project/SonataAdminBundle/blob/53dc6ddaa403cdbecf8d9630ef32a8e6e2a0f3ef/src/Form/Type/AdminType.php#L34-L41

Later the data is set as subject, which is of a different entity class. And this results in an error:

https://github.com/sonata-project/SonataAdminBundle/blob/53dc6ddaa403cdbecf8d9630ef32a8e6e2a0f3ef/src/Form/Type/AdminType.php#L102

Admin "GenericAdmin" does not allow this subject: A, use the one register with this admin class B

This is because inside the setSubject() method a check is done if the subject matches getClass(). Inside getClass() is will check if there's a subject and will return the class of the subject present.

Minimal repository with the bug

-

Steps to reproduce

-

Expected results

No error.

Perhaps after the clone line, there should be a line $admin->setSubject(null);.

Actual results

Error:

Admin "GenericAdmin" does not allow this subject: A, use the one register with this admin class B

I have now added $this->setSubject(null); in a public function __clone() method of my GenericAdmin. Which works for now.

[VincentLanglet]

How do you configure the Admin ?

The issue is related to

final public function getClass(): string
    {
        if ($this->hasActiveSubClass()) {
            if ($this->hasParentFieldDescription()) {
                throw new \LogicException('Feature not implemented: an embedded admin cannot have subclass');
            }

            $subClass = $this->getRequest()->query->get('subclass');
            \assert(\is_string($subClass));

            if (!$this->hasSubClass($subClass)) {
                throw new \LogicException(sprintf('Subclass "%s" is not defined.', $subClass));
            }

            return $this->getSubClass($subClass);
        }

        // Do not use `$this->hasSubject()` and `$this->getSubject()` here to avoid infinite loop.
        // `getSubject` use `hasSubject()` which use `getObject()` which use `getClass()`.
        if (null !== $this->subject) {
            $modelManager = $this->getModelManager();
            /** @phpstan-var class-string<T> $class */
            $class = $modelManager instanceof ProxyResolverInterface
                ? $modelManager->getRealClass($this->subject)
                // NEXT_MAJOR: Change to `\get_class($this->subject)` instead
                : BCHelper::getClass($this->subject);

            return $class;
        }

        return $this->getModelClass();
    }

Seems like, the modelClass of your admin, is not the same than the class of the subject.

Maybe the check in the setSubject method is wrong and

    final public function setSubject(?object $subject): void
    {
        if (null !== $subject && !is_a($subject, $this->getClass(), true)) {
            throw new \LogicException(sprintf(
                'Admin "%s" does not allow this subject: %s, use the one register with this admin class %s',
                static::class,
                \get_class($subject),
                $this->getClass()
            ));
        }

        $this->subject = $subject;
    }

should be changed to

    final public function setSubject(?object $subject): void
    {
        if (null !== $subject && !is_a($subject, $this->getModelClass(), true)) {
            throw new \LogicException(sprintf(
                'Admin "%s" does not allow this subject: %s, use the one register with this admin class %s',
                static::class,
                \get_class($subject),
                $this->getModelClass()
            ));
        }

        $this->subject = $subject;
    }

[7ochem]

I've removed the __clone() method again and applied the changes you suggested locally to AdminType and that seems to work 👍🏻

The model class is set to be a parent class (of A and B I'm mentioning in my report).

[VincentLanglet]

    final public function setSubject(?object $subject): void
    {
        if (null !== $subject && !is_a($subject, $this->getModelClass(), true)) {
            throw new \LogicException(sprintf(
                'Admin "%s" does not allow this subject: %s, use the one register with this admin class %s',
                static::class,
                \get_class($subject),
                $this->getModelClass()
            ));
        }

        $this->subject = $subject;
    }

IMHO this is the right fix. Do you want to provide a pr with a unit test @7ochem ?

[7ochem]

Yes, I'll sure try 👍🏻. It'll be a nice job for the weekend 😄 Thanks for your fast response Vincent