search

sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Persist entity with CollectionType returns error after release 4.10.0 #7778

Closed willemverspyck closed 2 years ago

willemverspyck commented 2 years ago

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 4.10.0 4.10.0 The missing Symfony Admin Generator sonata-project/block-bundle 4.11.0 4.11.0 Symfony SonataBlockBundle sonata-project/cache 2.2.0 2.2.0 Cache library Package sonata-project/cache is abandoned, you should avoid using it. No replacement was suggested. sonata-project/doctrine-extensions 1.16.0 1.16.0 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 4.2.7 4.2.7 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/exporter 2.11.0 2.11.0 Lightweight Exporter library sonata-project/form-extensions 1.13.1 1.13.1 Symfony form extensions sonata-project/twig-extensions 1.9.1 1.9.1 Sonata twig extensions ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/amqp-messenger v6.0.5 v6.0.5 Symfony AMQP extension Messenger Bridge symfony/apache-pack v1.0.1 v1.0.1 A pack for Apache support in Symfony symfony/asset v6.0.3 v6.0.3 Manages URL generation and versioning of web assets such as CSS stylesheets, JavaScript files and image files symfony/cache v6.0.6 v6.0.6 Provides an extended PSR-6, PSR-16 (and tags) implementation symfony/cache-contracts v2.5.0 v3.0.0 Generic abstractions related to caching symfony/config v6.0.3 v6.0.3 Helps you find, load, combine, autofill and validate configuration values of any kind symfony/console v6.0.5 v6.0.5 Eases the creation of beautiful and testable command line interfaces symfony/css-selector v6.0.3 v6.0.3 Converts CSS selectors to XPath expressions symfony/debug-bundle v6.0.3 v6.0.3 Provides a tight integration of the Symfony VarDumper component and the ServerLogCommand from MonologBridge in... symfony/dependency-injection v6.0.6 v6.0.6 Allows you to standardize and centralize the way objects are constructed in your application symfony/deprecation-contracts v3.0.0 v3.0.0 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge v6.0.6 v6.0.6 Provides integration for Doctrine with various Symfony components symfony/doctrine-messenger v6.0.3 v6.0.3 Symfony Doctrine Messenger Bridge symfony/dom-crawler v6.0.6 v6.0.6 Eases DOM navigation for HTML and XML documents symfony/dotenv v6.0.5 v6.0.5 Registers environment variables from a .env file symfony/error-handler v6.0.3 v6.0.3 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher v6.0.3 v6.0.3 Provides tools that allow your application components to communicate with each other by dispatching events and... symfony/event-dispatcher-contracts v3.0.0 v3.0.0 Generic abstractions related to dispatching event symfony/expression-language v6.0.3 v6.0.3 Provides an engine that can compile and evaluate expressions symfony/filesystem v6.0.6 v6.0.6 Provides basic utilities for the filesystem symfony/finder v6.0.3 v6.0.3 Finds files and directories via an intuitive fluent interface symfony/flex v2.1.6 v2.1.6 Composer plugin for Symfony symfony/form v6.0.5 v6.0.5 Allows to easily create, process and reuse HTML forms symfony/framework-bundle v6.0.6 v6.0.6 Provides a tight integration between Symfony components and the Symfony full-stack framework symfony/http-client v6.0.5 v6.0.5 Provides powerful methods to fetch HTTP resources synchronously or asynchronously symfony/http-client-contracts v3.0.0 v3.0.0 Generic abstractions related to HTTP clients symfony/http-foundation v6.0.6 v6.0.6 Defines an object-oriented layer for the HTTP specification symfony/http-kernel v6.0.6 v6.0.6 Provides a structured process for converting a Request into a Response symfony/intl v6.0.5 v6.0.5 Provides a PHP replacement layer for the C intl extension that includes additional data from the ICU library symfony/lock v6.0.5 v6.0.5 Creates and manages locks, a mechanism to provide exclusive access to a shared resource symfony/mailer v6.0.5 v6.0.5 Helps sending emails symfony/maker-bundle v1.38.0 v1.38.0 Symfony Maker helps you create empty commands, controllers, form classes, tests and more so you can forget abo... symfony/messenger v6.0.3 v6.0.3 Helps applications send and receive messages to/from other applications or via message queues symfony/mime v6.0.3 v6.0.3 Allows manipulating MIME messages symfony/monolog-bridge v6.0.3 v6.0.3 Provides integration for Monolog with various Symfony components symfony/monolog-bundle v3.7.1 v3.7.1 Symfony MonologBundle symfony/options-resolver v6.0.3 v6.0.3 Provides an improved replacement for the array_replace PHP function symfony/password-hasher v6.0.3 v6.0.3 Provides password hashing utilities symfony/phpunit-bridge v6.0.3 v6.0.3 Provides utilities for PHPUnit, especially user deprecation notices management symfony/polyfill-ctype v1.25.0 v1.25.0 Symfony polyfill for ctype functions symfony/polyfill-intl-grapheme v1.25.0 v1.25.0 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu v1.25.0 v1.25.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn v1.25.0 v1.25.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer v1.25.0 v1.25.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.25.0 v1.25.0 Symfony polyfill for the Mbstring extension symfony/polyfill-php72 v1.25.0 v1.25.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions symfony/polyfill-php80 v1.25.0 v1.25.0 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions symfony/polyfill-php81 v1.25.0 v1.25.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions symfony/process v6.0.5 v6.0.5 Executes commands in sub-processes symfony/property-access v6.0.5 v6.0.5 Provides functions to read and write from/to an object or array using a simple string notation symfony/property-info v6.0.3 v6.0.3 Extracts information about PHP class' properties using metadata of popular sources symfony/proxy-manager-bridge v6.0.6 v6.0.6 Provides integration for ProxyManager with various Symfony components symfony/psr-http-message-bridge v2.1.2 v2.1.2 PSR HTTP message bridge symfony/rate-limiter v6.0.3 v6.0.3 Provides a Token Bucket implementation to rate limit input and output in your application symfony/routing v6.0.5 v6.0.5 Maps an HTTP request to a set of configuration variables symfony/runtime v6.0.5 v6.0.5 Enables decoupling PHP applications from global state symfony/security-acl v3.3.1 v3.3.1 Symfony Security Component - ACL (Access Control List) symfony/security-bundle v6.0.5 v6.0.5 Provides a tight integration of the Security component into the Symfony full-stack framework symfony/security-core v6.0.5 v6.0.5 Symfony Security Component - Core Library symfony/security-csrf v6.0.3 v6.0.3 Symfony Security Component - CSRF Library symfony/security-http v6.0.5 v6.0.5 Symfony Security Component - HTTP Integration symfony/serializer v6.0.6 v6.0.6 Handles serializing and deserializing data structures, including object graphs, into array structures or other... symfony/service-contracts v3.0.0 v3.0.0 Generic abstractions related to writing services symfony/stopwatch v6.0.5 v6.0.5 Provides a way to profile code symfony/string v6.0.3 v6.0.3 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a ... symfony/templating v6.0.3 v6.0.3 Provides all the tools needed to build any kind of template system symfony/translation v6.0.6 v6.0.6 Provides tools to internationalize your application symfony/translation-contracts v3.0.0 v3.0.0 Generic abstractions related to translation symfony/twig-bridge v6.0.5 v6.0.5 Provides integration for Twig with various Symfony components symfony/twig-bundle v6.0.3 v6.0.3 Provides a tight integration of Twig into the Symfony full-stack framework symfony/validator v6.0.6 v6.0.6 Provides tools to validate values symfony/var-dumper v6.0.6 v6.0.6 Provides mechanisms for walking through any arbitrary PHP variable symfony/var-exporter v6.0.6 v6.0.6 Allows exporting any serializable PHP data structure to plain PHP code symfony/web-link v6.0.3 v6.0.3 Manages links between resources symfony/web-profiler-bundle v6.0.6 v6.0.6 Provides a development tool that gives detailed information about the execution of any request symfony/yaml v6.0.3 v6.0.3 Loads and dumps YAML files ```

PHP version

$ php -v
PHP 8.0.15 (cli) (built: Jan 29 2022 07:24:52) ( NTS )

Subject

Minimal repository with the bug

I have an CompanyAdmin

declare(strict_types=1);

namespace App\Admin;

use App\Entity\Company;
use App\Translator\AdminTranslator;
use Sonata\AdminBundle\Datagrid\ListMapper;
use Sonata\AdminBundle\Datagrid\DatagridMapper;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\DoctrineORMAdminBundle\Filter\CallbackFilter;
use Sonata\Form\Type\CollectionType;
use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;

#[AutoconfigureTag('sonata.admin', [
    'manager_type' => 'orm',
    'model_class' => Company::class,
    'label' => 'Company',
    'label_translator_strategy' => AdminTranslator::class,
])]
final class CompanyAdmin extends AbstractAdmin
{
    /**
     * @var array
     */
    protected array $removeRoutes = ['delete'];

    /**
     * @inheritDoc
     */
    protected function configureFormFields(FormMapper $formMapper): void
    {
        $formMapper
            ->tab('tab1')
                ->with('tab1_block1')
                    ->add('name', null, [
                        'required' => true,
                    ])
                    ->add('contacts', CollectionType::class, [
                        'required' => false,
                    ], ['edit' => 'inline', 'inline' => 'table'])
                ->end()
            ->end();
    }

    /**
     * @inheritDoc
     */
    protected function configureDatagridFilters(DatagridMapper $datagridMapper): void
    {
        $datagridMapper
            ->add('name', CallbackFilter::class, [
                'callback' => [$this, 'getCallbackSearch'],
                'show_filter' => true,
            ]);
    }

    /**
     * @inheritDoc
     */
    protected function configureListFields(ListMapper $listMapper): void
    {
        $listMapper
            ->add('name')
            ->add(ListMapper::NAME_ACTIONS, null, [
                'actions' => [
                    'show' => [],
                    'edit' => [],
                    'delete' => [],
                ],
            ]);
    }
}

with a CollectionType with OneToMany relation with contacts:

declare(strict_types=1);

namespace App\Admin;

use App\Entity\Contact;
use App\Translator\AdminTranslator;
use Sonata\AdminBundle\Form\FormMapper;
use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;

#[AutoconfigureTag('sonata.admin', [
    'manager_type' => 'orm',
    'model_class' => Contact::class,
    'label_translator_strategy' => AdminTranslator::class,
])]
final class ContactAdmin extends AbstractAdmin
{
    /**
     * @var array
     */
    protected array $removeRoutes = ['delete', 'show'];

    /**
     * @inheritDoc
     */
    protected function configureFormFields(FormMapper $formMapper): void
    {
        $formMapper
            ->add('name', null, [
                'required' => true,
            ])
            ->add('email')
            ->add('telephone')
            ->add('roles', ChoiceType::class, [
                'choices' => Contact::getContactRoles(true),
                'multiple' => true,
                'required' => false,
            ]);
    }
}

When I persist the company with 1 or more contact I get the error:

Failed to create object: App\Entity\Company

NotNullConstraintViolationException

An exception occurred while executing a query: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'company_id' cannot be null

When I save the Company without contacts and then edit the new company, then I can add contacts to it without problems. In the Company entity I'm setting the company after adding a new contact:

public function addContact(Contact $contact): self
{
    $contact->setCompany($this);

    $this->contacts->add($contact);

    return $this;
}

In 4.9.0 it worked, with 4.10.0 it didn't work anymore.

VincentLanglet commented 2 years ago

Might be related to https://github.com/sonata-project/SonataAdminBundle/pull/7764

We stoped calling the add method manually to rely on the PropertyAccessor instead. But our tests were still working...

Code changed from 

return self::callAdder($object, $instance, $fieldName);

to

$collection = $propertyAccessor->getValue($object, $fieldName);
$collection[] = $instance;
$propertyAccessor->setValue($object, $fieldName, $collection);

return $instance;

Could you debug a little bit in your vendor to understand what's the difference ? Does the add method is still called ?

VincentLanglet commented 2 years ago 
public function addContact(Contact $contact): self
{
     $contact->setCompany($this);

    $this->contacts->add($contact);

    return $this;
}

@willemverspyck Do you have also a setter setContacts in your entity ?

In the PropertyAccessor, there is the following code

            if (PropertyWriteInfo::TYPE_METHOD === $type) {
                $object->{$mutator->getName()}($value);
            } elseif (PropertyWriteInfo::TYPE_PROPERTY === $type) {
                $object->{$mutator->getName()} = $value;
            } elseif (PropertyWriteInfo::TYPE_ADDER_AND_REMOVER === $type) {
                $this->writeCollection($zval, $property, $value, $mutator->getAdderInfo(), $mutator->getRemoverInfo());
            }

so I wonder if it could be the reason you have a different behavior

willemverspyck commented 2 years ago

There is no setContacts, I only have addContact, getContacts, clearContacts and removeContact.

VincentLanglet commented 2 years ago

Seems like you're not passing by_reference => false to your collection

->add('contacts', CollectionType::class, ['required' => false], ['edit' => 'inline', 'inline' => 'table']);

This reminded me this issue https://github.com/sonata-project/SonataAdminBundle/issues/6426. Can you confirm that adding by_reference => false solve the issue ? https://symfony.com/doc/current/reference/forms/types/collection.html#by-reference

Then we would need to find why the setCompany method is not called anymore. Is the addContact method still called ? @willemverspyck

The AdminType is doing

if (true === $options['collection_by_reference']) {
     $subject = ObjectManipulator::addInstance($parentSubject, $subject, $parentFieldDescription);
} else {
    $subject = ObjectManipulator::setObject($subject, $parentSubject, $parentFieldDescription);
}

You're supposed to pass in the addInstance call. It could be interesting to dump the values before and after the ObjectManipulator call.

As I said, code changed from

return self::callAdder($object, $instance, $fieldName); // which was calling addContact manually

to

$collection = $propertyAccessor->getValue($object, $fieldName);
$collection[] = $instance;
$propertyAccessor->setValue($object, $fieldName, $collection);

return $instance;

According to the property accessor

if (PropertyWriteInfo::TYPE_METHOD === $type) {
    $object->{$mutator->getName()}($value);
} elseif (PropertyWriteInfo::TYPE_PROPERTY === $type) {
    $object->{$mutator->getName()} = $value;
} elseif (PropertyWriteInfo::TYPE_ADDER_AND_REMOVER === $type) {
    $this->writeCollection($zval, $property, $value, $mutator->getAdderInfo(), $mutator->getRemoverInfo());
}

It would be interesting to know in which case you're.

A repository with a reproducer would be really helpful to debug this @willemverspyck

VincentLanglet commented 2 years ago

I might have found a solution. Can you try https://github.com/sonata-project/SonataAdminBundle/pull/7781 ?

willemverspyck commented 2 years ago

Hi @VincentLanglet,

With release 4.10.0 and by_reference => false it works.

Your solution https://github.com/sonata-project/SonataAdminBundle/pull/7781 works with and without by_reference => false

The dumps are (when the error occured) in before method in ObjectManipulator:

[App\Entity\Company] {[#454 ▼]()
  -id: null
  -country: null
  -type: null
  -code: null
  -contacts: [Doctrine\Common\Collections\ArrayCollection] {[#453 ▼]()
    -elements: []
  }
  #dateUpdated: null
}

[App\Entity\Contact] {[#2268 ▼]()
  -id: null
  -email: null
  -telephone: null
  #dateUpdated: null
}

And after:

[App\Entity\Company] {[#454 ▼]()
  -id: null
  -country: null
  -type: null
  -code: null
  -contacts: [Doctrine\Common\Collections\ArrayCollection] {[#453 ▼]()
    -elements: array:1 [[▼]()
      0 => [App\Entity\Contact] {[#2268 ▼]()
        -id: null
        -email: null
        -telephone: null
        #dateUpdated: null
      }
    ]
  }
  #dateUpdated: null
}

[App\Entity\Contact] {[#2268 ▼]()
  -id: null
  -email: null
  -telephone: null
  #dateUpdated: null
}
willemverspyck commented 2 years ago

Thanks @VincentLanglet for the quick response and fix 👍