From 9eb2d6139efe8b8d1ae77d5b47c1e38ae109bc95 Mon Sep 17 00:00:00 2001 From: Emiel Nijpels Date: Thu, 24 Sep 2015 10:26:10 +0200 Subject: [PATCH 1/5] DDC-3146 remove event listener from event listener in abstract hydrator in cleanup function --- .../Internal/Hydration/AbstractHydrator.php | 3 + .../ORM/Functional/Ticket/DDC3146Test.php | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index ee9385c81..d462421ac 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -211,6 +211,9 @@ abstract class AbstractHydrator $this->_rsm = null; $this->_cache = []; $this->_metadataCache = []; + + $evm = $this->_em->getEventManager(); + $evm->removeEventListener(array(Events::onClear), $this); } /** diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php new file mode 100644 index 000000000..7e09642c2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php @@ -0,0 +1,65 @@ + + */ +class DDC3146Test extends \Doctrine\Tests\OrmFunctionalTestCase +{ + /** + * Verify that the number of added events to the event listener from the abstract hydrator class is equal to the number of removed events + */ + public function testEventListeners() + { + // Create mock connection to be returned from the entity manager interface + $mockConnection = $this->getMockBuilder('Doctrine\DBAL\Connection')->disableOriginalConstructor()->getMock(); + $mockEntityManagerInterface = $this->getMockBuilder('Doctrine\ORM\EntityManagerInterface')->disableOriginalConstructor()->getMock(); + $mockEntityManagerInterface->expects($this->any())->method('getConnection')->will($this->returnValue($mockConnection)); + + // Create mock event manager to be returned from the entity manager interface + $mockEventManager = $this->getMockBuilder('Doctrine\Common\EventManager')->disableOriginalConstructor()->getMock(); + $mockEntityManagerInterface->expects($this->any())->method('getEventManager')->will($this->returnValue($mockEventManager)); + + // Create mock statement and result mapping + $mockStatement = $this->getMockBuilder('Doctrine\DBAL\Driver\Statement')->disableOriginalConstructor()->getMock(); + $mockStatement->expects($this->once())->method('fetch')->will($this->returnValue(false)); + $mockResultMapping = $this->getMockBuilder('Doctrine\ORM\Query\ResultSetMapping')->disableOriginalConstructor()->getMock(); + + // Create mock abstract hydrator + $mockAbstractHydrator = $this->getMockBuilder('Doctrine\ORM\Internal\Hydration\AbstractHydrator') + ->setConstructorArgs(array($mockEntityManagerInterface)) + ->setMethods(array('hydrateAllData')) + ->getMock(); + + // Increase counter every time the event listener is added and decrease the counter every time the event listener is removed + $eventCounter = 0; + $mockEventManager->expects($this->any()) + ->method('addEventListener') + ->will( + $this->returnCallback( + function () use (&$eventCounter) { + $eventCounter++; + } + ) + ); + + $mockEventManager->expects($this->any()) + ->method('removeEventListener') + ->will( + $this->returnCallback( + function () use (&$eventCounter) { + $eventCounter--; + } + ) + ); + + // Create iterable result + $iterableResult = $mockAbstractHydrator->iterate($mockStatement, $mockResultMapping, array()); + $iterableResult->next(); + + // Number of added events listeners should be equal or less than the number of removed events + $this->assertLessThanOrEqual(0, $eventCounter, 'More events added to the event listener than removed; this can create a memory leak when references are not cleaned up'); + } +} From aba486ea2d5a189ac1673aed4b2174a7938bc554 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jun 2017 03:27:12 +0200 Subject: [PATCH 2/5] #1515 removing redundant assignment, short array notation --- lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php index d462421ac..6a7061285 100644 --- a/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php +++ b/lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php @@ -212,8 +212,10 @@ abstract class AbstractHydrator $this->_cache = []; $this->_metadataCache = []; - $evm = $this->_em->getEventManager(); - $evm->removeEventListener(array(Events::onClear), $this); + $this + ->_em + ->getEventManager() + ->removeEventListener([Events::onClear], $this); } /** From 067e01e0d7fd550d726fc5e448188e2c02962406 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jun 2017 03:32:58 +0200 Subject: [PATCH 3/5] #1515 cleaning up test case, since the PHPUnit 5.4+ API is much nicer --- .../ORM/Functional/Ticket/DDC3146Test.php | 61 +++++++++---------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php index 7e09642c2..cdf7b1234 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php @@ -2,64 +2,59 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; +use Doctrine\DBAL\Connection; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Common\EventManager; +use Doctrine\DBAL\Driver\Statement; +use Doctrine\ORM\Query\ResultSetMapping; +use Doctrine\ORM\Internal\Hydration\AbstractHydrator; +use Doctrine\Tests\OrmFunctionalTestCase; + /** * @group DDC-3146 * @author Emiel Nijpels */ -class DDC3146Test extends \Doctrine\Tests\OrmFunctionalTestCase +class DDC3146Test extends OrmFunctionalTestCase { /** * Verify that the number of added events to the event listener from the abstract hydrator class is equal to the number of removed events */ public function testEventListeners() { - // Create mock connection to be returned from the entity manager interface - $mockConnection = $this->getMockBuilder('Doctrine\DBAL\Connection')->disableOriginalConstructor()->getMock(); - $mockEntityManagerInterface = $this->getMockBuilder('Doctrine\ORM\EntityManagerInterface')->disableOriginalConstructor()->getMock(); - $mockEntityManagerInterface->expects($this->any())->method('getConnection')->will($this->returnValue($mockConnection)); + $mockConnection = $this->createMock(Connection::class); + $mockEntityManagerInterface = $this->createMock(EntityManagerInterface::class); + $mockEventManager = $this->createMock(EventManager::class); + $mockStatement = $this->createMock(Statement::class); + $mockResultMapping = $this->getMockBuilder(ResultSetMapping::class); - // Create mock event manager to be returned from the entity manager interface - $mockEventManager = $this->getMockBuilder('Doctrine\Common\EventManager')->disableOriginalConstructor()->getMock(); - $mockEntityManagerInterface->expects($this->any())->method('getEventManager')->will($this->returnValue($mockEventManager)); + $mockEntityManagerInterface->expects(self::any())->method('getEventManager')->willReturn($mockEventManager); + $mockEntityManagerInterface->expects(self::any())->method('getConnection')->willReturn($mockConnection); + $mockStatement->expects(self::once())->method('fetch')->willReturn(false); - // Create mock statement and result mapping - $mockStatement = $this->getMockBuilder('Doctrine\DBAL\Driver\Statement')->disableOriginalConstructor()->getMock(); - $mockStatement->expects($this->once())->method('fetch')->will($this->returnValue(false)); - $mockResultMapping = $this->getMockBuilder('Doctrine\ORM\Query\ResultSetMapping')->disableOriginalConstructor()->getMock(); - - // Create mock abstract hydrator - $mockAbstractHydrator = $this->getMockBuilder('Doctrine\ORM\Internal\Hydration\AbstractHydrator') + $mockAbstractHydrator = $this->getMockBuilder(AbstractHydrator::class) ->setConstructorArgs(array($mockEntityManagerInterface)) - ->setMethods(array('hydrateAllData')) + ->setMethods(['hydrateAllData']) ->getMock(); // Increase counter every time the event listener is added and decrease the counter every time the event listener is removed $eventCounter = 0; - $mockEventManager->expects($this->any()) + $mockEventManager->expects(self::atLeastOnce()) ->method('addEventListener') - ->will( - $this->returnCallback( - function () use (&$eventCounter) { - $eventCounter++; - } - ) - ); + ->willReturnCallback(function () use (&$eventCounter) { + $eventCounter++; + }); - $mockEventManager->expects($this->any()) + $mockEventManager->expects(self::atLeastOnce()) ->method('removeEventListener') - ->will( - $this->returnCallback( - function () use (&$eventCounter) { - $eventCounter--; - } - ) - ); + ->willReturnCallback(function () use (&$eventCounter) { + $eventCounter--; + }); // Create iterable result $iterableResult = $mockAbstractHydrator->iterate($mockStatement, $mockResultMapping, array()); $iterableResult->next(); // Number of added events listeners should be equal or less than the number of removed events - $this->assertLessThanOrEqual(0, $eventCounter, 'More events added to the event listener than removed; this can create a memory leak when references are not cleaned up'); + self::assertSame(0, $eventCounter, 'More events added to the event listener than removed; this can create a memory leak when references are not cleaned up'); } } From 0b5d877d5f61aee70c2cc80cdd500a0881f7ea5a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jun 2017 03:40:12 +0200 Subject: [PATCH 4/5] Migrating #1515 tests to `AbstractHydratorTest` --- .../ORM/Hydration/AbstractHydratorTest.php | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php diff --git a/tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php b/tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php new file mode 100644 index 000000000..1cd49d5ca --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Hydration/AbstractHydratorTest.php @@ -0,0 +1,57 @@ +createMock(Connection::class); + $mockEntityManagerInterface = $this->createMock(EntityManagerInterface::class); + $mockEventManager = $this->createMock(EventManager::class); + $mockStatement = $this->createMock(Statement::class); + $mockResultMapping = $this->getMockBuilder(ResultSetMapping::class); + + $mockEntityManagerInterface->expects(self::any())->method('getEventManager')->willReturn($mockEventManager); + $mockEntityManagerInterface->expects(self::any())->method('getConnection')->willReturn($mockConnection); + $mockStatement->expects(self::once())->method('fetch')->willReturn(false); + + /* @var $mockAbstractHydrator AbstractHydrator */ + $mockAbstractHydrator = $this + ->getMockBuilder(AbstractHydrator::class) + ->setConstructorArgs([$mockEntityManagerInterface]) + ->setMethods(['hydrateAllData']) + ->getMock(); + + $mockEventManager + ->expects(self::at(0)) + ->method('addEventListener') + ->with([Events::onClear], $mockAbstractHydrator); + + $mockEventManager + ->expects(self::at(1)) + ->method('removeEventListener') + ->with([Events::onClear], $mockAbstractHydrator); + + iterator_to_array($mockAbstractHydrator->iterate($mockStatement, $mockResultMapping)); + } +} From 995054d884e68cbc15d9f85e126dc1b6682aa2bf Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 24 Jun 2017 03:40:46 +0200 Subject: [PATCH 5/5] #1515 dropping DDC-3146 test, which was moved to the hydration tests --- .../ORM/Functional/Ticket/DDC3146Test.php | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php deleted file mode 100644 index cdf7b1234..000000000 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC3146Test.php +++ /dev/null @@ -1,60 +0,0 @@ - - */ -class DDC3146Test extends OrmFunctionalTestCase -{ - /** - * Verify that the number of added events to the event listener from the abstract hydrator class is equal to the number of removed events - */ - public function testEventListeners() - { - $mockConnection = $this->createMock(Connection::class); - $mockEntityManagerInterface = $this->createMock(EntityManagerInterface::class); - $mockEventManager = $this->createMock(EventManager::class); - $mockStatement = $this->createMock(Statement::class); - $mockResultMapping = $this->getMockBuilder(ResultSetMapping::class); - - $mockEntityManagerInterface->expects(self::any())->method('getEventManager')->willReturn($mockEventManager); - $mockEntityManagerInterface->expects(self::any())->method('getConnection')->willReturn($mockConnection); - $mockStatement->expects(self::once())->method('fetch')->willReturn(false); - - $mockAbstractHydrator = $this->getMockBuilder(AbstractHydrator::class) - ->setConstructorArgs(array($mockEntityManagerInterface)) - ->setMethods(['hydrateAllData']) - ->getMock(); - - // Increase counter every time the event listener is added and decrease the counter every time the event listener is removed - $eventCounter = 0; - $mockEventManager->expects(self::atLeastOnce()) - ->method('addEventListener') - ->willReturnCallback(function () use (&$eventCounter) { - $eventCounter++; - }); - - $mockEventManager->expects(self::atLeastOnce()) - ->method('removeEventListener') - ->willReturnCallback(function () use (&$eventCounter) { - $eventCounter--; - }); - - // Create iterable result - $iterableResult = $mockAbstractHydrator->iterate($mockStatement, $mockResultMapping, array()); - $iterableResult->next(); - - // Number of added events listeners should be equal or less than the number of removed events - self::assertSame(0, $eventCounter, 'More events added to the event listener than removed; this can create a memory leak when references are not cleaned up'); - } -}