doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.76k stars 224 forks source link

ReferenceRepository won't work with more than one EntityManager #431

Closed Pixelshaped closed 1 year ago

Pixelshaped commented 1 year ago

I'm working with two EntityManager, and two DBs, one called "default", the other called "lab".

When we're calling $this->addReference($referenceName, $entity); from a Fixture extending AbstractFixture it now calls $class = $this->getRealClass(get_class($object)); which is problematic for us.

It seems there is no way to indicate which EntityManager to create the reference on. As a result, all our fixtures running on the default EntityManager are running fine, but the first that tries to call addReference on the lab EntityManager crashes with the message:

The class 'App\Entity\Lab\StockOrder' was not found in the chain configured namespaces App\Entity\Main, Gedmo\Translatable\Entity

Which makes sense because it's exactly right. StockOrder is defined on another mapping.

This issue appeared since #409

Beforehand, ReferenceRepository was only some kind of dictionary, and calling addReference or setReference would only add an object to an array for further reference.

Now, the object class is gotten from classes existing in the current EntityManager:

ReferenceRepository::addReference():188
$class = $this->getRealClass(get_class($object));

See: https://github.com/doctrine/data-fixtures/blame/1.6.x/src/ReferenceRepository.php

Our load method:

public function load(ObjectManager $manager): void
{
        $this->entityManager = $this->managerRegistry->getManager($this->getTargetEntityManagerName());

        $this->loadData(); // Creates entities and persists them and calls addReference() 

        $this->entityManager->flush();
}

(getTargetEntityManagerName() is a custom method that our fixtures can override and it returns default by default and another EntityManager name when needed.)

@VincentLanglet

Pixelshaped commented 1 year ago

On a side note, I quickly tried to hack that behavior with a static array that would have been a persistent dictionary of ReferenceRepository instances depending on the desired EntityManager

public function load(ObjectManager $manager): void
{
    $targetEntityManagerName = $this->getTargetEntityManagerName(); // Occasionally returns 'lab'

    $this->entityManager = $this->managerRegistry->getManager();

    if($this->entityManager !== $this->referenceRepository->getManager()) {
        if(!isset(self::$additionalReferenceRepositories[targetEntityManagerName])) {
            self::$additionalReferenceRepositories[targetEntityManagerName] = new ReferenceRepository($this->entityManager);
        }
        $this->setReferenceRepository(self::$additionalReferenceRepositories[targetEntityManagerName]);
    }

    $this->loadData();

    $this->entityManager->flush();
}

But then I crash elsewhere. In my StockProduct fixture (the one that crashes because it's on the lab EntityManager), I indeed get a reference to a Product entity living on the default EntityManager. Which now fails because I explicitly created a new ReferenceRepository.

So I kinda needed this non-blocking dictionary behavior ReferenceRepository had.

stof commented 1 year ago

Before the mentioned PR, this restriction still existed, but only in ProxyReferenceRepository

VincentLanglet commented 1 year ago

I would say this wasn't mean to be supported at all. There was already multiple call to $this->manager in the code, so it works with one manager, not many.

Can't you just declare to ReferenceRepository service, one by entityManager, and call setReferenceRepository with the right one in your Fixture class ?

Pixelshaped commented 1 year ago

I would say this wasn't mean to be supported at all.

You're certainly right. ORMExecutor even wraps all fixtures load in a transaction. The ones using the second EntityManager evade this but it's in undefined behavior territory.

Can't you just declare to ReferenceRepository service, one by entityManager, and call setReferenceRepository with the right one in your Fixture class ?

I think it's what I'm doing in the second post - or I may have misunderstood the remark. But I can't do that because I cross reference entities from the two DBs. So if I create a second ReferenceRepository for the lab mapping it won't have access to all the entities created for the default mapping.

We're not using that a lot so I think the best course of action in the short term for me would be to drop cross references and use the command as it was intended: one EM at a time (--em=EM)

Thanks