Second Level Cache One-to-One exception on PUT

stevro commented 4 years ago

Bug Report

Q	A
BC Break	no
Version	2.7.x-dev

Summary

Given 2 enties, User and Schedule with a One To One relation between them User can have one schedule, but it's not mandatory, so there can be users without schedule.

When updating the user, if you try to also add a schedule, it will try to save in the cache an unpersisted entity Schedule with id=null.

I mention that the error is triggered by the unique validator on User entity, when it does the select. Maybe SLC should ignore entities that don't have a valid identifier. I'll also report this to symfony validation component team.

/**
* [...]
* @ORM\Cache(usage="NONSTRICT_READ_WRITE")
*/
class User
[...]
    /**
     * @ORM\OneToOne(targetEntity="Schedule", mappedBy="user", cascade={"persist", "remove"})
     * @ORM\Cache(usage="NONSTRICT_READ_WRITE")
     */
    private $schedule;
[...]

/**
* [...]
* @ORM\Cache(usage="NONSTRICT_READ_WRITE")
*/
class Schedule
[...]
/**
     * @ORM\OneToOne(targetEntity="User", inversedBy="schedule")
     * @ORM\JoinColumn(nullable=false, onDelete="CASCADE")
     *
     * @ORM\Cache(usage="NONSTRICT_READ_WRITE")
     */
    private $user;
[...]

Current behavior

With env=dev you will get a Notice:

Undefined index: 000000000df78ebe00000000086513ba
On prod, you will get Uncaught Error: Argument 2 passed to Doctrine\ORM\Cache\EntityCacheKey::__construct() must be of the type array, null given, called in /home/ubuntu/[...].com/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php on line 359

"message": "Argument 2 passed to Doctrine\\ORM\\Cache\\EntityCacheKey::__construct() must be of the type array, null given, called in /home/ubuntu/[...]/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php on line 359",
    "code": 0,
    "file": "/home/ubuntu/[...]/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/EntityCacheKey.php:49",
    "trace": [
        "/home/ubuntu/[...]/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php:359",
        "/home/ubuntu/[...].com/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php:311",
        "/home/ubuntu/[...].com/shared/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/Persister/Entity/AbstractEntityPersister.php:426",
        "/home/ubuntu/[...].com/shared/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php:178",
        "/home/ubuntu/[...].com/shared/vendor/symfony/doctrine-bridge/Validator/Constraints/UniqueEntityValidator.php:135",
        "/home/ubuntu/[...].com/shared/vendor/symfony/validator/Validator/RecursiveContextualValidator.php:741",
        "/home/ubuntu/[...].com/shared/vendor/symfony/validator/Validator/RecursiveContextualValidator.php:489",
        "/home/ubuntu/[...].com/shared/vendor/symfony/validator/Validator/RecursiveContextualValidator.php:312",
        "/home/ubuntu/[...].com/shared/vendor/symfony/validator/Validator/RecursiveContextualValidator.php:135",
        "/home/ubuntu/[...].com/shared/vendor/symfony/form/Extension/Validator/Constraints/FormValidator.php:58",

How to reproduce

Create 2 entities with bidirectional OneToOne relation, User <-> Schedule Have a unique constraint on User (by email for ex.) Have a single form for creating both the User and Schedule. Firstly create only the User Then update the User and fill in the schedule.

lcobucci commented 4 years ago

@stevro thanks for submitting this issue.

On the first glance it seems that your mapping isn't correct, based on what you're describing. I'd say that User should be owning side of the association and not Schedule.

How the ORM behaves when you invert your mapping?

stevro commented 4 years ago

If I invert the mapping it's working. But both cases should be covered in my opinion. We can't always do the queries from the owning side.

I solved the problem by creating a custom repository method for the UniqueEntity validator where I fetch only the User entity without any join.

But maybe SLC could ignore entities that are not yet persisted or have the identifier = null.

lcobucci commented 4 years ago

@stevro unfortunately this is not the case for covering the two mappings...

According to the ORM semantics, the one you've posted is invalid and should have triggered a mapping validation error - I'm assuming that the schema validation command didn't trigger any error.

doctrine / orm