liip / LiipTestFixturesBundle

This bundles enables efficient loading of Doctrine fixtures in functional test-cases for Symfony applications
https://liip.ch
MIT License
167 stars 45 forks source link

[Bug]: Multiple entity managers per connection OR entity manager name does not match connection name #225

Open olifrieda opened 1 year ago

olifrieda commented 1 year ago

Preconditions

If the entity manager does have a name not matching the connection name or if having multiple entity managers per connection, it is not possible to load fixtures.

The reason is because of this:

    public function setObjectManagerName(string $omName = null): void
    {
        $this->omName = $omName;
        $this->om = $this->registry->getManager($omName);
        $this->connection = $this->registry->getConnection($omName);  <<<<< Connection name may not the same as OM name
    }

See: https://github.com/liip/LiipTestFixturesBundle/blob/9ec9d52903df0c723b8d6489842c5b205a08bf8d/src/Services/DatabaseTools/AbstractDatabaseTool.php#L107-L112

Versions: Bundle 2.6.0 PHP 8.1

Steps to reproduce

Configure doctrine like this:

doctrine:
    dbal:
        default_connection: core
        connections:
            core:
                ...
    orm:
        entity_managers:
            core:
                ...
            a_second_entity_manager:
                ...    

or this:

doctrine:
    dbal:
        default_connection: core
        connections:
            default:
                ...
    orm:
        entity_managers:
            a_name_not_matching_connection_name:
                ...    

Then inside your test load a fixture like this:

static::getContainer()
    ->get(DatabaseToolCollection::class)
    ->get('a_second_entity_manager')
    ->loadFixtures([YourFixtures::class]);

or this:

static::getContainer()
    ->get(DatabaseToolCollection::class)
    ->get('a_name_not_matching_connection_name')
    ->loadFixtures([YourFixtures::class]);

Expected result

Successfully loading fixtures.

Actual result

InvalidArgumentException : Doctrine ORM Connection named "a_second_entity_manager" does not exist.

or:

InvalidArgumentException : Doctrine ORM Connection named "a_name_not_matching_connection_name" does not exist.
olifrieda commented 1 year ago

A dirty fix:

$this->connection = $this->om instanceof EntityManagerInterface ? $this->om->getConnection() : $this->registry->getConnection($omName);

The problem is, that the $this->registry->getManager($omName) does not return the EntityManagerInterface but the ObjectManager.

alexislefebvre commented 1 year ago

Thanks for the report!

Could you please open a PR with a fix? First we need to add a new Kernel with a new config that use more than entity manager, we only have one at the moment: https://github.com/liip/LiipTestFixturesBundle/blob/2.x/tests/AppConfigMysql/config.yml Then add tests that call the different entity managers.

Shoud we update the setObjectManagerName method like this?

-public function setObjectManagerName(string $omName = null): void
+public function setObjectManagerName(string $omName = null, ?string $connectionName = null): void
{
      $this->omName = $omName;
      $this->om = $this->registry->getManager($omName);
-     $this->connection = $this->registry->getConnection($omName);
+     $this->connection = $this->registry->getConnection($connectionName ?? $omName);
}

Then

static::getContainer()
    ->get(DatabaseToolCollection::class)
    ->get('a_name_not_matching_connection_name', 'default')
    ->loadFixtures([YourFixtures::class]);

may works.

olifrieda commented 1 year ago

@alexislefebvre It looks like, it is not that easy.

Loading fixtures like this:

static::getContainer()
            ->get(DatabaseToolCollection::class)
            ->get('client')
            ->loadFixtures([RunActionFixtures::class]);

the DatabaseToolCollection should handle the list of database tools. The index of this list depends on registry name and driver name. No dependency on connection or entity manager (no sure, if both is necessary). Each time, you pull out the database tool from the collection, the object manager (and connection) is set. This overwrites previous settings. This means, the last call of the get method will win.

See: https://github.com/liip/LiipTestFixturesBundle/blob/9ec9d52903df0c723b8d6489842c5b205a08bf8d/src/Services/DatabaseToolCollection.php#L61

I have no quick fix for this. So I found a workaround. I changed the doctrine configuration for the test environment, so that the number of connections is equal to the number of entity managers and the entity managers names do now match the connection name. E. g. like this:

doctrine.yaml:

when@test:
    doctrine:
        dbal:
            connections:
                a_name_not_matching_connection_name:
                    url: '%env(resolve:DATABASE_TEST_URL)%'
        orm:
            entity_managers:
                a_name_not_matching_connection_name:
                    connection: a_name_not_matching_connection_name

If there is no one else out there having the same problem, it is ok, to handle this as an edge case without fixing it.

alexislefebvre commented 1 year ago

I tried to add a test with 2 connections and 2 entity managers and it added way more complexity than I expected: it means 2 different configurations for the entities, their schema and fixtures. I ended up with that classes that can't be found and I gave up.

I'm sorry but I can't help with this topic.

olifrieda commented 1 year ago

I also had a closer look, and - yes - it is more complex than it looked in the beginning. I do not have a really good idea right now, but I think, it is possible.

I think most projects do not have to deal with more than one connection or entity manager. If I found a good solution, I'll let you know. Thanks for your support.