Removing one side of BiDi 1-1 relation throws

[duzenko]

Bug Report

Q	A
BC Break	no
Version	2.19.6

Summary

I have added an inverseBy field to an existing entity class so as to use it in the BiDi 1-1 fashion. The DB structure did not change. The setup is similar to the example given at https://www.doctrine-project.org/projects/doctrine-orm/en/2.19/reference/association-mapping.html#one-to-one-bidirectional. After this, under certain scenarios, deleting an entity throws an Exception. Specifically, the entities are loaded via a DQL query:

        $dq = 'SELECT e, ec FROM ' . \E5\Entity\EventConfirmation::class . ' ec JOIN ec.event e WHERE ec.user = :user AND e.type = :type';
        $query = $this->getEntityManager()->createQuery( $dq )
                ->setParameter( 'type', $type )
                ->setParameter( 'user', $user );

        return $query->getOneOrNullResult();

This returns a nested object structure as expected:

After removing the $confirmation entity the structure becomes instable:

        $em->remove( $confirmation );
        $em->flush();

When I try commit the transaction after this, I get an exception:

Doctrine\ORM\ORMInvalidArgumentException: A new entity was found through the relationship 'E5\Entity\Event#confirmation' that was not configured to cascade persist operatio
ns for entity: E5\Entity\EventConfirmation@282. To solve this issue: Either explicitly call EntityManager#persist() on this unknown entity or configure cascade persist this
 association in the mapping for example @ManyToOne(..,cascade={"persist"}). If you cannot find out which entity causes the problem implement 'E5\Entity\EventConfirmation#__toString()' to get a clue. in /media/sf_Remote/main-web-application/vendor/doctrine/orm/src/ORMInvalidArgumentException.php:103
Stack trace:
#0 /media/sf_Remote/main-web-application/vendor/doctrine/orm/src/UnitOfWork.php(3856): Doctrine\ORM\ORMInvalidArgumentException::newEntitiesFoundThroughRelationships()     
#1 /media/sf_Remote/main-web-application/vendor/doctrine/orm/src/UnitOfWork.php(417): Doctrine\ORM\UnitOfWork->assertThatThereAreNoUnintentionallyNonPersistedAssociations()#2 /media/sf_Remote/main-web-application/vendor/doctrine/orm/src/EntityManager.php(403): Doctrine\ORM\UnitOfWork->commit()
#3 /media/sf_Remote/main-web-application/E4/C/Admin/Casher.php(209): Doctrine\ORM\EntityManager->flush()
#4 /media/sf_Remote/main-web-application/E4/C/Admin/Casher.php(190): C_Admin_Casher::eventIdentified()
#5 /media/sf_Remote/main-web-application/E8/feature/user/UserService.php(287): C_Admin_Casher::emailIdentifiedSend()
#6 /media/sf_Remote/main-web-application/E8/feature/onboarding/Service.php(84): E8\feature\user\UserService->sendIdentifiedMail()
#7 /media/sf_Remote/main-web-application/scripts/lib/dev.php(22): E8\feature\onboarding\Service->approveIdentificationDocument()
#8 /media/sf_Remote/main-web-application/scripts/cli.php(27): include_once('...')
#9 {main}

Which means that Doctrine is trying to persist the confirmation entity back to the DB. This is the opposite to what the code did before successfully (before the additing of the inversedBy confirmation field in the Event entity.

Current behavior

Throws

How to reproduce

Delete the owner side of a BiDi 1-1 relation and commit.

Expected behavior

Delete successful. The inversed side changes to null.

[duzenko]

Relevant field definitions:

class EventConfirmation {
...
    /**
     * @ORM\OneToOne(targetEntity="Event", inversedBy="confirmation")
     * @JoinColumn(name="event", referencedColumnName="id")
     */
    private ?Event $event;

This was added (along with inversedBy above) before the bug manifested:

class Event {
...
    /**
     * @OneToOne(targetEntity="EventConfirmation", mappedBy="event")
     */
    private ?EventConfirmation $confirmation;

[duzenko]

It seems that changing SELECT e, ec FROM to SELECT ec FROM helps in this particular case as it changes the type of event field from Entity to Proxy. However in general this kind of breakage should not occur at all.

[duzenko]

It would be great to get some feedback regarding this The bug also manifests with proxy objects after they get initialized

        $em = phpVirtualMachine()->getEntityManager();

        $eventConfirmations = $em->getRepository( \E5\Entity\EventConfirmation::class )->findBy( [ 'user' => $user ] );

        foreach( $eventConfirmations as $eventConfirmation ) {
            if( $eventConfirmation->getEvent()->getType() != $type ) {
                continue;
            }

            // now delete eventConfirmation will throw
        }

[greg0ire]

Which means that Doctrine is trying to persist the confirmation entity back to the DB.

I wouldn't be so sure.

Looking at the stack trace, we can see that we are here:

https://github.com/doctrine/orm/blob/cfc0655a1c9b5ffd5506172cb7642b6090aa5f7e/src/UnitOfWork.php#L353-L369

So, directly inside commit(). I think you were mislead into thinking it has to do with persistence because of the error message.

I think you should debug further as to why this entity is considered new, but before that, have you validated your schema? I find it suspicious that such a big bug with what appears to be a pretty common situation happens at all.

You could also try reproducing it with a test.

[duzenko]

@greg0ire Do you have a test already that deletes an entity on the owner side of a BiDi relation? I'd think no, which is why it was not noticed And I don't think that deleting a BiDi owner (with a second flush after that) is pretty common There is a chance that I'm doing something wrong, but I'd like to hear from you first Note that commit is not necessary. It can be a second flush as well.

[duzenko]

@greg0ire This reproduced the error reliably for me

<?php
// bootstrap.php
use Doctrine\DBAL\DriverManager;
use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\JoinColumn;
use Doctrine\ORM\Mapping\OneToOne;
use Doctrine\ORM\ORMSetup;
use Doctrine\ORM\Mapping as ORM;

require_once "vendor/autoload.php";

#[Entity]
class Customer {
    #[ORM\Id]
    #[ORM\Column( type: 'integer' )]
    #[ORM\GeneratedValue]
    private int|null $id = null;

    /** One Customer has One Cart. */
    #[OneToOne( targetEntity: Cart::class, mappedBy: 'customer' )]
    public Cart|null $cart = null;

    #[ORM\Column( type: 'string' )]
    private string $name = 'qwe';

    public function getName(): string {
        return $this->name;
    }

    public function setName( string $name ): void {
        $this->name = $name;
    }
}

#[Entity]
class Cart {
    #[ORM\Id]
    #[ORM\Column( type: 'integer' )]
    #[ORM\GeneratedValue]
    public int|null $id = null;

    /** One Cart has One Customer. */
    #[OneToOne( targetEntity: Customer::class, inversedBy: 'cart' )]
    #[JoinColumn( name: 'customer_id', referencedColumnName: 'id' )]
    public Customer|null $customer = null;

}

unlink( 'db.sqlite' );
$database = new SQLite3( 'db.sqlite' );
$database->query( 'CREATE TABLE if not exists Cart (
    id integer PRIMARY KEY,
    customer_id INT DEFAULT NULL
);
CREATE TABLE if not exists Customer (
    id integer PRIMARY KEY,
    name varchar DEFAULT NULL
);' );

// Create a simple "default" Doctrine ORM configuration for Attributes
$config = ORMSetup::createAttributeMetadataConfiguration(
    paths: [],
    isDevMode: true,
);

// configuring the database connection
$connection = DriverManager::getConnection( [
    'driver' => 'pdo_sqlite',
    'path' => __DIR__ . '/db.sqlite',
], $config );

// obtaining the entity manager
$entityManager = new EntityManager( $connection, $config );

//$schemaTool = new \Doctrine\ORM\Tools\SchemaTool($entityManager);
//$classes = $entityManager->getMetadataFactory()->getAllMetadata();
//$schemaTool->createSchema($classes);

$cart = new Cart();
$cart->customer = new Customer();
$entityManager->persist( $cart->customer );
$entityManager->persist( $cart );
$entityManager->flush();
$entityManager->clear(); // simulate separate run

$cart = $entityManager->find( Cart::class, $cart->id );
$cart->customer->getName(); // initialize proxy
$entityManager->remove( $cart );
$entityManager->flush();

$cusomer = $entityManager->getRepository( Customer::class )->findAll()[ 0 ];
$cusomer->setName( 'asd' );
$entityManager->flush(); // throws here

echo 'OK', PHP_EOL;

Output

"[sshConfig://a@192.168.52.3:22 key]:/usr/bin/php" -dxdebug.mode=debug -dxdebug.client_port=9003 -dxdebug.client_host=192.168.52.1 /media/sf_Remote/test/bootstrap.php
PHP Fatal error:  Uncaught Doctrine\ORM\ORMInvalidArgumentException: A new entity was found through the relationship 'Customer#cart' that was not configured to cascade persist operations for entity: Cart@99. To solve this issue: Either explicitly call EntityManager#persist() on this unknown entity or configure cascade persist this association in the mapping for example @ManyToOne(..,cascade={"persist"}). If you cannot find out which entity causes the problem implement 'Cart#__toString()' to get a clue. in /media/sf_Remote/test/vendor/doctrine/orm/src/ORMInvalidArgumentException.php:103
Stack trace:
#0 /media/sf_Remote/test/vendor/doctrine/orm/src/UnitOfWork.php(3856): Doctrine\ORM\ORMInvalidArgumentException::newEntitiesFoundThroughRelationships()
#1 /media/sf_Remote/test/vendor/doctrine/orm/src/UnitOfWork.php(417): Doctrine\ORM\UnitOfWork->assertThatThereAreNoUnintentionallyNonPersistedAssociations()
#2 /media/sf_Remote/test/vendor/doctrine/orm/src/EntityManager.php(403): Doctrine\ORM\UnitOfWork->commit()
#3 /media/sf_Remote/test/bootstrap.php(94): Doctrine\ORM\EntityManager->flush()
#4 {main}
  thrown in /media/sf_Remote/test/vendor/doctrine/orm/src/ORMInvalidArgumentException.php on line 103

Process finished with exit code 255

[greg0ire]

Is it a minimal reproducer? For instance, is the JoinColumn attribute necessary to reproduce the issue? Also, when I wrote "You could also try reproducing it with a test." I meant a PHPUnit test that you would add to the doctrine/orm test suite.

[duzenko]

Is it a minimal reproducer? For instance, is the JoinColumn attribute necessary to reproduce the issue?

How would I know if it is necessary? I took this code from your official docs at https://www.doctrine-project.org/projects/doctrine-orm/en/2.19/reference/association-mapping.html.

Also, when I wrote "You could also try reproducing it with a test." I meant a PHPUnit test that you would add to the doctrine/orm test suite.

Sorry, does it mean the a PHP file that you can run from CLI or IDE directly is not acepted as reproduction case? I'd think that you could convert it to a PHPUnit test in less than 5 minutes if you were interested in this bug, without the lengthy procedure of me creating a PR just for it to chill in the waiting list with another 298 PRs you already have. My originial post had less code for which you advised to "debug further" on my side implying that I'm not using Doctrine right. I don't think I should be getting any more of these "try harder" responses at this point

[greg0ire]

Let me focus on the 298 PRs first (more actually if you can't other repos), and I will get back to you.