Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed entity manager resetting #7

Merged
merged 1 commit into from
May 9, 2016
Merged

Fixed entity manager resetting #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions src/Doctrine/ManagerRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ class ManagerRegistry implements ManagerRegistryInterface
/**
* @var EntityManager[]
*/
protected $managers;
protected $originalManagers;

/**
* @var EntityManager[]
*/
protected $resetManagers = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ghola

Thank you for your pull request. And the passion.

The only thing that i do not understand why you need originalManagers and resetManagers. I looks more complex to me as it used to.

Regards @dominikzogg

Copy link
Contributor Author

@ghola ghola Apr 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there,

The previous $mangers member was not an array as the docblock suggests but a container (because $this->managers = $this->container['orm.ems']; and the DoctrineOrmServiceProvider defines 'orm.ems' as being $ems = new Container(); ... return $ems). Since container services are frozen once they get instantiated once, it means that I was not able to replace the old service with a new one when needing to reset the service.

Making the previous $manages member be an actual array instead of a container was not a solution because that would have instantiated all the EMs from the 'orm.ems' container which defeats the purpose of a container.

So my solution was to leave the old $managers member as a container and rename it to $originalManagers and add the $resetManagers and when serving an EM I would pull from these giving priority to the $resetManagers member.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ghola

Makes sense, does it work resetting the same manager multiple times? I can't see a test for this, thats why i ask.

Regards @dominikzogg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dominikzogg

Yes it works. I just added tests for this as proof.

Thanks,
Alex


/**
* @var string
Expand Down Expand Up @@ -138,7 +143,7 @@ public function getManager($name = null)
$this->loadManagers();
$name = $this->validateManagerName($name);

return $this->managers[$name];
return isset($this->resetManagers[$name]) ? $this->resetManagers[$name] : $this->originalManagers[$name];
}

/**
Expand All @@ -148,7 +153,7 @@ public function getManager($name = null)
protected function validateManagerName($name)
{
return $this->validateName(
$this->managers,
$this->originalManagers,
$name,
$this->getDefaultManagerName())
;
Expand Down Expand Up @@ -181,15 +186,15 @@ public function getManagers()
{
$this->loadManagers();

if ($this->managers instanceof Container) {
if ($this->originalManagers instanceof Container) {
$managers = array();
foreach ($this->getManagerNames() as $name) {
$managers[$name] = $this->managers[$name];
$managers[$name] = $this->originalManagers[$name];
}
$this->managers = $managers;
$this->originalManagers = $managers;
}

return $this->managers;
return array_replace($this->originalManagers, $this->resetManagers);
}

/**
Expand All @@ -199,30 +204,29 @@ public function getManagerNames()
{
$this->loadManagers();

if ($this->managers instanceof Container) {
return $this->managers->keys();
if ($this->originalManagers instanceof Container) {
return $this->originalManagers->keys();
} else {
return array_keys($this->managers);
return array_keys($this->originalManagers);
}
}

/**
* @param string|null $name
* @return void
* @throws \InvalidArgumentException
*/
public function resetManager($name = null)
{
$this->loadManagers();
$name = $this->validateManagerName($name);

$this->managers[$name] = null;
$this->resetManagers[$name] = $this->container['orm.ems.factory'][$name]();
}

protected function loadManagers()
{
if (is_null($this->managers)) {
$this->managers = $this->container['orm.ems'];
if (is_null($this->originalManagers)) {
$this->originalManagers = $this->container['orm.ems'];
$this->defaultManagerName = $this->container['orm.ems.default'];
}
}
Expand Down
27 changes: 27 additions & 0 deletions src/Provider/DoctrineOrmManagerRegistryProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Saxulum\DoctrineOrmManagerRegistry\Provider;

use Doctrine\ORM\EntityManager;
use Pimple\Container;
use Pimple\ServiceProviderInterface;
use Saxulum\DoctrineOrmManagerRegistry\Doctrine\ManagerRegistry;
Expand Down Expand Up @@ -33,6 +34,32 @@ public function register(Container $container)
return new ManagerRegistry($container);
};

if (!isset($container['orm.ems.factory'])) {
$container['orm.ems.factory'] = function (Container $container) {
$container['orm.ems.options.initializer']();
$factory = new Container();
foreach ($container['orm.ems.options'] as $name => $options) {
if ($container['orm.ems.default'] === $name) {
// we use shortcuts here in case the default has been overridden
$config = $container['orm.em.config'];
} else {
$config = $container['orm.ems.config'][$name];
}
$factory[$name] = $factory->protect(
function () use ($container, $options, $config) {
return EntityManager::create(
$container['dbs'][$options['connection']],
$config,
$container['dbs.event_manager'][$options['connection']]
);
}
);
}
return $factory;
};
}


if (isset($container['form.extensions']) && class_exists('Symfony\\Bridge\\Doctrine\\Form\\DoctrineOrmExtension')) {
$container['form.extensions'] = $container->extend('form.extensions', function ($extensions, $container) {
$extensions[] = new DoctrineOrmExtension($container['doctrine']);
Expand Down
25 changes: 24 additions & 1 deletion tests/Doctrine/ManagerRegistryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class ManagerRegistryTest extends \PHPUnit_Framework_TestCase
{
/**
* @return array
* @return Container
*/
protected function createMockDefaultAppAndDeps()
{
Expand Down Expand Up @@ -67,6 +67,17 @@ protected function createMockDefaultAppAndDeps()
'default' => $entityManager,
));

$container['orm.ems.factory'] = new Container();
$container['orm.ems.factory']['default'] = $container['orm.ems.factory']->protect(
function () {
return $this
->getMockBuilder('Doctrine\ORM\EntityManager')
->disableOriginalConstructor()
->getMock()
;
}
);

$container['orm.ems.default'] = 'default';

return $container;
Expand All @@ -90,5 +101,17 @@ public function testRegisterDefaultImplementations()
$this->assertEquals($container['doctrine']->getAliasNamespace('Test'), 'Saxulum\DoctrineOrmManagerRegistry\Doctrine\ManagerRegistry');
$this->assertInstanceOf('Doctrine\Common\Persistence\ObjectRepository', $container['doctrine']->getRepository('Saxulum\DoctrineOrmManagerRegistry\Doctrine\ManagerRegistry'));
$this->assertInstanceOf('Doctrine\ORM\EntityManager', $container['doctrine']->getManagerForClass('Saxulum\DoctrineOrmManagerRegistry\Doctrine\ManagerRegistry'));

$initialManager = $container['doctrine']->getManager();
$container['doctrine']->resetManager();
$resetManager = $container['doctrine']->getManager();
$this->assertNotSame($resetManager, $initialManager);
$this->assertSame($resetManager, $container['doctrine']->getManagers()['default']);

$container['doctrine']->resetManager();
$reResetManager = $container['doctrine']->getManager();
$this->assertNotSame($reResetManager, $resetManager);
$this->assertNotSame($reResetManager, $initialManager);
$this->assertSame($reResetManager, $container['doctrine']->getManagers()['default']);
}
}
39 changes: 39 additions & 0 deletions tests/Provider/DoctrineOrmManagerRegistryProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,45 @@ public function testSchema()
$this->dropSchema($schemaTool, $metadatas);
}

public function testManagerFactory()
{
$app = $this->createApplication();

/** @var EntityManager $initialEm */
$initialEm = $app['doctrine']->getManager();

$schemaTool = $this->getSchemaTool($initialEm);
$metadatas = $this->getMetadatas($initialEm);

$this->createSchema($schemaTool, $metadatas);
$this->dropSchema($schemaTool, $metadatas);

$app['doctrine']->resetManager();
/** @var EntityManager $resetEm */
$resetEm = $app['doctrine']->getManager();

$schemaTool = $this->getSchemaTool($resetEm);
$metadatas = $this->getMetadatas($resetEm);

$this->createSchema($schemaTool, $metadatas);
$this->dropSchema($schemaTool, $metadatas);

$this->assertNotSame($resetEm, $initialEm);

$app['doctrine']->resetManager();
/** @var EntityManager $reResetEm */
$reResetEm = $app['doctrine']->getManager();

$schemaTool = $this->getSchemaTool($reResetEm);
$metadatas = $this->getMetadatas($reResetEm);

$this->createSchema($schemaTool, $metadatas);
$this->dropSchema($schemaTool, $metadatas);

$this->assertNotSame($reResetEm, $resetEm);
$this->assertNotSame($reResetEm, $initialEm);
}

public function testValidator()
{
$app = $this->createApplication();
Expand Down