Skip to content

Commit

Permalink
bug #220 Duplicate initialisation of repositories (Justus Krapp)
Browse files Browse the repository at this point in the history
This PR was merged into the 1.7-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | fixes #217
| License         | MIT

Accessing a repository via `EntityManger::getReposiory` and direct injection via the dot notation of the repository will yield different instances of the same repository. This is caused by:
* Repositories are not injected into the ServiceLocator of [ContainerRepositoryFactory](https://github.com/doctrine/DoctrineBundle/blob/1.12.x/Repository/ContainerRepositoryFactory.php) as the required tag `doctrine.repository_service` is missing for our generated the repositories
* Even when injected (after adding that `doctrine.repository_service`) the [ContainerRepositoryFactory::getRepository](https://github.com/doctrine/DoctrineBundle/blob/1.12.x/Repository/ContainerRepositoryFactory.php#L35) method cant resolve the repository as its injected with the dot notation and doctrine looks for the repository by class name
* Problem also exists for default repositories but simply tagging them does not work



Solution/Changes;
For the solid repositories:
* **Tag the solid repositories** so they get injected
* **Switch to service id to class name**
* **Create a public alias** to maintain old functionality

For generic default repositories (cant be tagged as all use the same class)
* **Init default repositories via doctrine factory** so its using `ContainerRepositoryFactory` and by that prevent duplicate initialisation
* Decorated `ContainerRepositoryFactory` to be table to return the sylius `EntityRepository` instead of the default doctrine one




Commits
-------

2baf357 Preventing the duplicate initialisation of repositories when access via entity manager and dependency injection
117fb44 Preventing the duplicate initialisation of repositories when access via entity manager and dependency injection
3ceb1ba removes possible bc break
  • Loading branch information
pamil authored Nov 23, 2020
2 parents d6673ce + 3ceb1ba commit 45494f6
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace Sylius\Bundle\ResourceBundle\DependencyInjection\Driver\Doctrine;

use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\ServiceRepositoryCompilerPass;
use Doctrine\Common\Persistence\ObjectManager as DeprecatedObjectManager;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
Expand Down Expand Up @@ -44,14 +45,33 @@ protected function addRepository(ContainerBuilder $container, MetadataInterface
$repositoryClass = $metadata->getClass('repository');
}

$serviceId = $metadata->getServiceId('repository');
$repositoryFactoryDef = $container->getDefinition('sylius.doctrine.orm.container_repository_factory');
$managerReference = new Reference($metadata->getServiceId('manager'));
$definition = new Definition($repositoryClass);
$definition->setArguments([
new Reference($metadata->getServiceId('manager')),
$this->getClassMetadataDefinition($metadata),
]);
$definition->setPublic(true);

$container->setDefinition($metadata->getServiceId('repository'), $definition);
if ($repositoryClass === EntityRepository::class) {
$entityClass = $metadata->getClass('model');

$definition->setFactory([$managerReference, 'getRepository']);
$definition->setArguments([$entityClass]);

$container->setDefinition($serviceId, $definition);

$repositoryFactoryDef->addMethodCall('addGenericEntity', [$entityClass]);
} else {
$definition->setArguments([$managerReference, $this->getClassMetadataDefinition($metadata)]);

$container->setDefinition($serviceId, $definition);

$doctrineDefinition = new Definition($repositoryClass);
$doctrineDefinition->addTag(ServiceRepositoryCompilerPass::REPOSITORY_SERVICE_TAG);
$doctrineDefinition->setFactory([new Reference('service_container'), 'get']);
$doctrineDefinition->setArguments([$serviceId]);

$container->setDefinition($repositoryClass, $doctrineDefinition);
}

/** @psalm-suppress RedundantCondition Backward compatibility with Symfony */
if (method_exists($container, 'registerAliasForArgument')) {
Expand Down
70 changes: 70 additions & 0 deletions src/Bundle/Doctrine/ContainerRepositoryFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Sylius\Bundle\ResourceBundle\Doctrine;

use Doctrine\Bundle\DoctrineBundle\Repository\ContainerRepositoryFactory as DoctrineContainerRepositoryFactory;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Repository\RepositoryFactory;
use Doctrine\Persistence\ObjectRepository;
use Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository;

final class ContainerRepositoryFactory implements RepositoryFactory
{
/** @var ObjectRepository[] */
private $managedRepositories = [];

/** @var string[] */
private $genericEntities = [];

/** @var DoctrineContainerRepositoryFactory */
private $doctrineFactory;

public function __construct(DoctrineContainerRepositoryFactory $doctrineFactory)
{
$this->doctrineFactory = $doctrineFactory;
}

public function addGenericEntity(string $entityName): void
{
$this->genericEntities[$entityName] = $entityName;
}

/**
* {@inheritdoc}
*/
public function getRepository(EntityManagerInterface $entityManager, $entityName): ObjectRepository
{
if (isset($this->genericEntities[$entityName])) {
$metadata = $entityManager->getClassMetadata($entityName);

return $this->getOrCreateRepository($entityManager, $metadata);
}

return $this->doctrineFactory->getRepository($entityManager, $entityName);
}

private function getOrCreateRepository(
EntityManagerInterface $entityManager,
ClassMetadata $metadata
): ObjectRepository {
$repositoryHash = $metadata->getName() . spl_object_hash($entityManager);

if (isset($this->managedRepositories[$repositoryHash])) {
return $this->managedRepositories[$repositoryHash];
}

return $this->managedRepositories[$repositoryHash] = new EntityRepository($entityManager, $metadata);
}
}
4 changes: 4 additions & 0 deletions src/Bundle/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,9 @@
<argument>Sylius\Bundle\ResourceBundle\Form\Builder\DefaultFormBuilderInterface</argument>
<argument>form builder</argument>
</service>

<service id="sylius.doctrine.orm.container_repository_factory" decorates="doctrine.orm.container_repository_factory" class="Sylius\Bundle\ResourceBundle\Doctrine\ContainerRepositoryFactory" public="false">
<argument type="service" id="sylius.doctrine.orm.container_repository_factory.inner" />
</service>
</services>
</container>
37 changes: 37 additions & 0 deletions src/Bundle/Tests/Resource/ResourceServicesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

namespace Sylius\Bundle\ResourceBundle\Tests\Resource;

use AppBundle\Entity\Book;
use AppBundle\Entity\ComicBook;
use AppBundle\Repository\BookRepository;
use Doctrine\ORM\EntityManager;
use Sylius\Bundle\ResourceBundle\Controller\ResourceController;
use Sylius\Component\Resource\Factory\FactoryInterface;
Expand Down Expand Up @@ -40,4 +43,38 @@ public function it_allows_to_access_resource_services_from_container(): void
$productRepository = $client->getContainer()->get('app.factory.book');
$this->assertTrue($productRepository instanceof FactoryInterface);
}

/**
* @test
*/
public function it_will_return_the_same_repository_instance()
{
$client = parent::createClient();
$repository = self::$container->get(BookRepository::class);

$repositoryAlias = $client->getContainer()->get('app.repository.book');
$this->assertSame($repository, $repositoryAlias);

$em = $client->getContainer()->get('app.manager.book');
$repositoryAlias = $em->getRepository(Book::class);
$this->assertSame($repository, $repositoryAlias);

$em = $client->getContainer()->get('doctrine.orm.entity_manager');
$repositoryAlias = $em->getRepository(Book::class);
$this->assertSame($repository, $repositoryAlias);
}

/**
* @test
*/
public function it_will_return_the_same_repository_instance_for_default_repositories()
{
$client = parent::createClient();
$em = $client->getContainer()->get('doctrine.orm.entity_manager');
$repository = $client->getContainer()->get('app.repository.comic_book');
$repositoryAlias = $em->getRepository(ComicBook::class);

$this->assertInstanceOf(RepositoryInterface::class, $repository);
$this->assertSame($repository, $repositoryAlias);
}
}

0 comments on commit 45494f6

Please sign in to comment.