From 67e205b36acdc73097b79802b097389a491216b9 Mon Sep 17 00:00:00 2001 From: Ed Hartwell Goose Date: Mon, 11 Apr 2016 17:01:55 +0100 Subject: [PATCH 1/2] Fixes #5755, uses '->getReflectionProperties()' instead of '->getReflectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour --- lib/Doctrine/ORM/Proxy/ProxyFactory.php | 2 +- .../Tests/ORM/Proxy/ProxyFactoryTest.php | 50 +++++++++++++++++-- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Proxy/ProxyFactory.php b/lib/Doctrine/ORM/Proxy/ProxyFactory.php index 72f161eab..58dfc0932 100644 --- a/lib/Doctrine/ORM/Proxy/ProxyFactory.php +++ b/lib/Doctrine/ORM/Proxy/ProxyFactory.php @@ -196,7 +196,7 @@ class ProxyFactory extends AbstractProxyFactory ); } - foreach ($class->getReflectionClass()->getProperties() as $property) { + foreach ($class->getReflectionProperties() as $property) { if ( ! $class->hasField($property->name) && ! $class->hasAssociation($property->name)) { continue; } diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php index dd5b730f6..1d21207f6 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -3,16 +3,17 @@ namespace Doctrine\Tests\ORM\Proxy; use Doctrine\Common\Persistence\Mapping\RuntimeReflectionService; +use Doctrine\Common\Proxy\AbstractProxyFactory; use Doctrine\ORM\EntityNotFoundException; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Persisters\Entity\BasicEntityPersister; use Doctrine\ORM\Proxy\ProxyFactory; use Doctrine\Tests\Mocks\ConnectionMock; +use Doctrine\Tests\Mocks\DriverMock; use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Mocks\UnitOfWorkMock; -use Doctrine\Tests\Mocks\DriverMock; -use Doctrine\Common\Proxy\AbstractProxyFactory; use Doctrine\Tests\OrmTestCase; +use Doctrine\Tests\Models\Company\CompanyEmployee; /** * Test the proxy generator. Its work is generating on-the-fly subclasses of a given model, which implement the Proxy pattern. @@ -65,9 +66,9 @@ class ProxyFactoryTest extends OrmTestCase $persister ->expects($this->atLeastOnce()) - ->method('load') - ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) - ->will($this->returnValue(new \stdClass())); + ->method('load') + ->with($this->equalTo($identifier), $this->isInstanceOf($proxyClass)) + ->will($this->returnValue(new \stdClass())); $proxy->getDescription(); } @@ -139,6 +140,45 @@ class ProxyFactoryTest extends OrmTestCase $this->assertInstanceOf('Closure', $proxy->__getInitializer(), 'The initializer wasn\'t removed'); $this->assertInstanceOf('Closure', $proxy->__getCloner(), 'The cloner wasn\'t removed'); } + + public function testProxyClonesParentFields() + { + $companyEmployee = new CompanyEmployee(); + $companyEmployee->setSalary(1000); // A property on the CompanyEmployee + $companyEmployee->setName("Bob"); // A property on the parent class, CompanyPerson + + // Set the id of the CompanyEmployee (which is in the parent CompanyPerson) + $class = new \ReflectionClass('Doctrine\Tests\Models\Company\CompanyPerson'); + + $property = $class->getProperty('id'); + $property->setAccessible(true); + + $property->setValue($companyEmployee, 42); + + $classMetaData = $this->emMock->getClassMetadata('Doctrine\Tests\Models\Company\CompanyEmployee'); + + $persister = $this->getMock('Doctrine\ORM\Persisters\Entity\BasicEntityPersister', array('load', 'getClassMetadata'), array(), '', false); + $this->uowMock->setEntityPersister('Doctrine\Tests\Models\Company\CompanyEmployee', $persister); + + /* @var $proxy \Doctrine\Common\Proxy\Proxy */ + $proxy = $this->proxyFactory->getProxy('Doctrine\Tests\Models\Company\CompanyEmployee', array('id' => 42)); + + $persister + ->expects($this->atLeastOnce()) + ->method('load') + ->will($this->returnValue($companyEmployee)); + + $persister + ->expects($this->atLeastOnce()) + ->method('getClassMetadata') + ->will($this->returnValue($classMetaData)); + + $cloned = clone $proxy; + + $this->assertEquals(42, $cloned->getId(), "Expected the Id to be cloned"); + $this->assertEquals(1000, $cloned->getSalary(), "Expect properties on the CompanyEmployee class to be cloned"); + $this->assertEquals("Bob", $cloned->getName(), "Expect properties on the CompanyPerson class to be cloned"); + } } abstract class AbstractClass From 3fca33bdc42ddb70d2b284970e8e6533f11b06ba Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 10 Sep 2016 20:45:01 +0200 Subject: [PATCH 2/2] #5768 #5755 cleaned up test scenario, using new mocking logic as per PHPUnit 5.4+ --- .../Tests/ORM/Proxy/ProxyFactoryTest.php | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php index 1d21207f6..174daf3c0 100644 --- a/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Proxy/ProxyFactoryTest.php @@ -12,6 +12,7 @@ use Doctrine\Tests\Mocks\ConnectionMock; use Doctrine\Tests\Mocks\DriverMock; use Doctrine\Tests\Mocks\EntityManagerMock; use Doctrine\Tests\Mocks\UnitOfWorkMock; +use Doctrine\Tests\Models\Company\CompanyPerson; use Doctrine\Tests\OrmTestCase; use Doctrine\Tests\Models\Company\CompanyEmployee; @@ -145,39 +146,42 @@ class ProxyFactoryTest extends OrmTestCase { $companyEmployee = new CompanyEmployee(); $companyEmployee->setSalary(1000); // A property on the CompanyEmployee - $companyEmployee->setName("Bob"); // A property on the parent class, CompanyPerson + $companyEmployee->setName('Bob'); // A property on the parent class, CompanyPerson // Set the id of the CompanyEmployee (which is in the parent CompanyPerson) - $class = new \ReflectionClass('Doctrine\Tests\Models\Company\CompanyPerson'); + $property = new \ReflectionProperty(CompanyPerson::class, 'id'); - $property = $class->getProperty('id'); $property->setAccessible(true); - $property->setValue($companyEmployee, 42); - $classMetaData = $this->emMock->getClassMetadata('Doctrine\Tests\Models\Company\CompanyEmployee'); + $classMetaData = $this->emMock->getClassMetadata(CompanyEmployee::class); - $persister = $this->getMock('Doctrine\ORM\Persisters\Entity\BasicEntityPersister', array('load', 'getClassMetadata'), array(), '', false); - $this->uowMock->setEntityPersister('Doctrine\Tests\Models\Company\CompanyEmployee', $persister); + $persister = $this + ->getMockBuilder(BasicEntityPersister::class) + ->setMethods(['load', 'getClassMetadata']) + ->disableOriginalConstructor() + ->getMock(); + $this->uowMock->setEntityPersister(CompanyEmployee::class, $persister); /* @var $proxy \Doctrine\Common\Proxy\Proxy */ - $proxy = $this->proxyFactory->getProxy('Doctrine\Tests\Models\Company\CompanyEmployee', array('id' => 42)); + $proxy = $this->proxyFactory->getProxy(CompanyEmployee::class, ['id' => 42]); $persister - ->expects($this->atLeastOnce()) + ->expects(self::atLeastOnce()) ->method('load') - ->will($this->returnValue($companyEmployee)); + ->willReturn($companyEmployee); $persister - ->expects($this->atLeastOnce()) + ->expects(self::atLeastOnce()) ->method('getClassMetadata') - ->will($this->returnValue($classMetaData)); + ->willReturn($classMetaData); + /* @var $cloned CompanyEmployee */ $cloned = clone $proxy; - $this->assertEquals(42, $cloned->getId(), "Expected the Id to be cloned"); - $this->assertEquals(1000, $cloned->getSalary(), "Expect properties on the CompanyEmployee class to be cloned"); - $this->assertEquals("Bob", $cloned->getName(), "Expect properties on the CompanyPerson class to be cloned"); + self::assertSame(42, $cloned->getId(), 'Expected the Id to be cloned'); + self::assertSame(1000, $cloned->getSalary(), 'Expect properties on the CompanyEmployee class to be cloned'); + self::assertSame('Bob', $cloned->getName(), 'Expect properties on the CompanyPerson class to be cloned'); } }