DDC2016Test::testDoctrineNotMarkAsChangedIfVOImplementsDoctrineValueObjectInterface() - Code Metrics - Inspection of "Embeddable value object" - doctrine/orm - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#5841)

by Peter

created 2016-05-27 07:51 UTC

testDoctrineNotMarkAsChangedIfVOImplementsDoctrineValueObjectInterface() B

↳ Parent: DDC2016Test

Complexity

Conditions	1
Paths	1

Size

Total Lines	37
Code Lines	20

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
c	0
b	0
f	0
dl	0
loc	37
rs	8.8571
cc	1
eloc	20
nc	1
nop	0

<?php

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\ORM\DoctrineValueObject;
use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\MappedSuperclass;

/**
 * @group DDC-2016
 */
class DDC2016Test extends \Doctrine\Tests\OrmFunctionalTestCase
{
    /**
     * Verifies that when eager loading is triggered, proxies are kept managed.
     *
     * The problem resides in the refresh hint passed to {@see \Doctrine\ORM\UnitOfWork::createEntity},
     * which, as of DDC-1734, causes the proxy to be marked as un-managed.
     * The check against the identity map only uses the identifier hash and the passed in class name, and
     * does not take into account the fact that the set refresh hint may be for an entity of a different
     * type from the one passed to {@see \Doctrine\ORM\UnitOfWork::createEntity}
     *
     * As a result, a refresh requested for an entity `Foo` with identifier `123` may cause a proxy
     * of type `Bar` with identifier `123` to be marked as un-managed.
     */
    public function testIssue()
    {
        $metadata = $this->_em->getClassMetadata(DDC2016User::CLASS_NAME);
        $uow      = $this->_em->getUnitOfWork();

        $username = new DDC2016Username('validUser');
        $user     = new DDC2016User($username);

        $this->_em->persist($user);
        $this->_em->flush();
        $this->_em->clear();

        $user = $this->_em->getRepository(DDC2016User::CLASS_NAME)->find($user->id);
        $this->assertInstanceOf(DDC2016User::CLASS_NAME, $user);

        /*
         * Call the getter which validate the DB value with Value Object and set the doctrine property to
         * avoid duplicate validation and continuously re-creating the Value Object.
         *
         * Issue:
         *
         * Because we set the property, computeChangeSet will detect as a change and will update the entity.
         */
        $username = $user->getUsername();
        $this->assertInstanceOf(DDC2016Username::CLASS_NAME, $username);

        $uow->computeChangeSet($metadata, $user);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
        $changeset = $uow->getEntityChangeSet($user);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

        /*
         * User not changed, just called the getter, which create and validate! the data from db.
         * Unfortunately, doctrine detect as a change and will mark property as changed.
         */
        $this->assertNotEmpty($changeset, 'Changeset not empty, but should!');
    }

    public function testDoctrineNotMarkAsChangedIfVOImplementsDoctrineValueObjectInterface()
    {
        $metadata = $this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME);
        $uow      = $this->_em->getUnitOfWork();

        $txtUser  = 'validUser';
        $username = new DC2016UsernameVo($txtUser);
        $user     = new DC2016UserWithVo($username);
        $this->_em->persist($user);

        $uow->computeChangeSet($metadata, $user);
        $changeSet = $uow->getEntityChangeSet($user);

        /*
         * Changeset should hold the ValueObject.
         */
        $this->assertInstanceOf(DC2016UsernameVo::CLASS_NAME, $changeSet['username'][1]);

        $this->_em->flush();
        $this->_em->clear();

        $user = $this->_em->getRepository(DC2016UserWithVo::CLASS_NAME)->find($user->id);
        $this->assertInstanceOf(DC2016UserWithVo::CLASS_NAME, $user);
        $this->assertEquals($txtUser, (string)$user->getUsername());

        $username = $user->getUsername();
        $this->assertInstanceOf(DC2016UsernameVo::CLASS_NAME, $username);

        $uow->computeChangeSet($metadata, $user);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
        $changeSet = $uow->getEntityChangeSet($user);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

        /*
         * We called the getter which set the Doctrine property to VO. Because of VO implements DoctrineValueObject,
         * $changeSet should be an empty array.
         */
        $this->assertEmpty($changeSet, 'Changeset should empty now, because we implement DoctrineValueObject!');
    }

    public function testUsernameGetUsernameReturnUsername()
    {
        $usernameString = 'validUserName';

        $username = new DDC2016Username($usernameString);

        $this->assertEquals($usernameString, $username->getUsername());
    }

    /**
     * @expectedException \InvalidArgumentException
     */
    public function testUsernameThrowExceptionOnNonString()
    {
        new DDC2016Username(123);
    }

    /**
     * @expectedException \UnexpectedValueException
     */
    public function testUsernameThrowExceptionOnInvalidUsername()
    {
        new DDC2016Username('invalidUser-INVALID');
    }

    /**
     * {@inheritDoc}
     */
    protected function setUp()
    {
        parent::setUp();

        $this->_schemaTool->createSchema(array(
            $this->_em->getClassMetadata(DDC2016User::CLASS_NAME),
            $this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME),
        ));
    }

    protected function tearDown()
    {
        $this->_schemaTool->dropSchema(array(
            $this->_em->getClassMetadata(DDC2016User::CLASS_NAME),
            $this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME),
        ));

        parent::tearDown();
    }
}

/**
 * @Entity
 * @MappedSuperclass()
 */
class DDC2016User
{
    const CLASS_NAME = __CLASS__;

    /** @Id @Column(type="integer") @GeneratedValue */
    public $id;

    /**
     * @var DDC2016Username
     *
     * @Column(type="string", length=128, nullable=false)
     */
    public $username;

    /** Constructor
     *
     * @param DDC2016Username $username
     */
    public function __construct(DDC2016Username $username)
    {
        $this->username = $username;
    }

    /**
     * @return DDC2016Username
     */
    public function getUsername()
    {
        return ($this->username instanceof DDC2016Username)
            ? $this->username
            : $this->username = new DDC2016Username($this->username);
    }

    /**
     * @param DDC2016Username $username
     *
     * @return $this
     */
    public function setUsername(DDC2016Username $username)
    {
        $this->username = $username;

        return $this;
    }
}

/**
 * Username ValueObject
 */
class DDC2016Username
{
    const CLASS_NAME = __CLASS__;

    /**
     * @var string
     */
    protected $username;

    /**
     * @param string $username
     *
     * @throws \InvalidArgumentException If $username is not a string.
     * @throws \UnexpectedValueException If $username is not acceptable.
     */
    public function __construct($username)
    {
        if ( ! is_string($username)) {
            throw new \InvalidArgumentException(
                sprintf('Username should be a string. [received: %s]', gettype($username))
            );
        }

        if (preg_match('/.*-INVALID$/', $username)) {
            throw new \UnexpectedValueException(
                sprintf('Username not acceptable. [username: %s]', $username)
            );
        }

        $this->username = $username;
    }

    /**
     * @return string
     */
    public function getUsername()
    {
        return $this->username;
    }

    public function __toString()
    {
        return $this->getUsername();
    }
}

class DC2016UsernameVo extends DDC2016Username implements DoctrineValueObject
{
    /**
     * @return string
     */
    public function __toString()
    {
        return $this->getUsername();
    }

    /**
     * @param mixed $value
     *
     * @return bool
     */
    public function equals($value)
    {
        if ($value instanceof DoctrineValueObject) {
            return $this->getUsername() == (string)$value;
        }

        return $this->getUsername() == $value;
    }
}

/**
 * @Entity()
 */
class DC2016UserWithVo
{
    const CLASS_NAME =  __CLASS__;

    /** @Id @Column(type="integer") @GeneratedValue */
    public $id;

    /**
     * @var DDC2016Username
     *
     * @Column(type="string", length=128, nullable=false)
     */
    public $username;

    /** Constructor
     *
     * @param DDC2016Username $username
     */
    public function __construct(DDC2016Username $username)
    {
        $this->username = $username;
    }

    public function getUsername()
    {
        return ($this->username instanceof DC2016UsernameVo)
            ? $this->username
            : $this->username = new DC2016UsernameVo($this->username);
    }

    /**
     * @param DDC2016Username $username
     *
     * @return $this
     */
    public function setUsername(DDC2016Username $username)
    {
        $this->username = $username;

        return $this;
    }
}


1			<?php
2
3			namespace Doctrine\Tests\ORM\Functional\Ticket;
4
5			use Doctrine\ORM\DoctrineValueObject;
6			use Doctrine\ORM\Mapping as ORM;
7			use Doctrine\ORM\Mapping\MappedSuperclass;
8
9			/**
10			* @group DDC-2016
11			*/
12			class DDC2016Test extends \Doctrine\Tests\OrmFunctionalTestCase
13			{
14			/**
15			* Verifies that when eager loading is triggered, proxies are kept managed.
16			*
17			* The problem resides in the refresh hint passed to {@see \Doctrine\ORM\UnitOfWork::createEntity},
18			* which, as of DDC-1734, causes the proxy to be marked as un-managed.
19			* The check against the identity map only uses the identifier hash and the passed in class name, and
20			* does not take into account the fact that the set refresh hint may be for an entity of a different
21			* type from the one passed to {@see \Doctrine\ORM\UnitOfWork::createEntity}
22			*
23			* As a result, a refresh requested for an entity `Foo` with identifier `123` may cause a proxy
24			* of type `Bar` with identifier `123` to be marked as un-managed.
25			*/
26			public function testIssue()
27			{
28			$metadata = $this->_em->getClassMetadata(DDC2016User::CLASS_NAME);
29			$uow = $this->_em->getUnitOfWork();
30
31			$username = new DDC2016Username('validUser');
32			$user = new DDC2016User($username);
33
34			$this->_em->persist($user);
35			$this->_em->flush();
36			$this->_em->clear();
37
38			$user = $this->_em->getRepository(DDC2016User::CLASS_NAME)->find($user->id);
39			$this->assertInstanceOf(DDC2016User::CLASS_NAME, $user);
40
41			/*
42			* Call the getter which validate the DB value with Value Object and set the doctrine property to
43			* avoid duplicate validation and continuously re-creating the Value Object.
44			*
45			* Issue:
46			*
47			* Because we set the property, computeChangeSet will detect as a change and will update the entity.
48			*/
49			$username = $user->getUsername();
50			$this->assertInstanceOf(DDC2016Username::CLASS_NAME, $username);
51
52			$uow->computeChangeSet($metadata, $user);
			0 ignored issues – show Bug introduced 2016-05-27 07:44 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor..._NAME)->find($user->id)` on line `38` can also be of type `null`; however, `Doctrine\ORM\UnitOfWork::computeChangeSet()` does only seem to accept `object`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
53			$changeset = $uow->getEntityChangeSet($user);
			0 ignored issues – show Bug introduced 2016-05-27 07:44 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor..._NAME)->find($user->id)` on line `38` can also be of type `null`; however, `Doctrine\ORM\UnitOfWork::getEntityChangeSet()` does only seem to accept `object`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
54
55			/*
56			* User not changed, just called the getter, which create and validate! the data from db.
57			* Unfortunately, doctrine detect as a change and will mark property as changed.
58			*/
59			$this->assertNotEmpty($changeset, 'Changeset not empty, but should!');
60			}
61
62			public function testDoctrineNotMarkAsChangedIfVOImplementsDoctrineValueObjectInterface()
63			{
64			$metadata = $this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME);
65			$uow = $this->_em->getUnitOfWork();
66
67			$txtUser = 'validUser';
68			$username = new DC2016UsernameVo($txtUser);
69			$user = new DC2016UserWithVo($username);
70			$this->_em->persist($user);
71
72			$uow->computeChangeSet($metadata, $user);
73			$changeSet = $uow->getEntityChangeSet($user);
74
75			/*
76			* Changeset should hold the ValueObject.
77			*/
78			$this->assertInstanceOf(DC2016UsernameVo::CLASS_NAME, $changeSet['username'][1]);
79
80			$this->_em->flush();
81			$this->_em->clear();
82
83			$user = $this->_em->getRepository(DC2016UserWithVo::CLASS_NAME)->find($user->id);
84			$this->assertInstanceOf(DC2016UserWithVo::CLASS_NAME, $user);
85			$this->assertEquals($txtUser, (string)$user->getUsername());
86
87			$username = $user->getUsername();
88			$this->assertInstanceOf(DC2016UsernameVo::CLASS_NAME, $username);
89
90			$uow->computeChangeSet($metadata, $user);
			0 ignored issues – show Bug introduced 2016-05-27 08:01 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor..._NAME)->find($user->id)` on line `83` can also be of type `null`; however, `Doctrine\ORM\UnitOfWork::computeChangeSet()` does only seem to accept `object`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
91			$changeSet = $uow->getEntityChangeSet($user);
			0 ignored issues – show Bug introduced 2016-05-27 08:01 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor..._NAME)->find($user->id)` on line `83` can also be of type `null`; however, `Doctrine\ORM\UnitOfWork::getEntityChangeSet()` does only seem to accept `object`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
92
93			/*
94			* We called the getter which set the Doctrine property to VO. Because of VO implements DoctrineValueObject,
95			* $changeSet should be an empty array.
96			*/
97			$this->assertEmpty($changeSet, 'Changeset should empty now, because we implement DoctrineValueObject!');
98			}
99
100			public function testUsernameGetUsernameReturnUsername()
101			{
102			$usernameString = 'validUserName';
103
104			$username = new DDC2016Username($usernameString);
105
106			$this->assertEquals($usernameString, $username->getUsername());
107			}
108
109			/**
110			* @expectedException \InvalidArgumentException
111			*/
112			public function testUsernameThrowExceptionOnNonString()
113			{
114			new DDC2016Username(123);
115			}
116
117			/**
118			* @expectedException \UnexpectedValueException
119			*/
120			public function testUsernameThrowExceptionOnInvalidUsername()
121			{
122			new DDC2016Username('invalidUser-INVALID');
123			}
124
125			/**
126			* {@inheritDoc}
127			*/
128			protected function setUp()
129			{
130			parent::setUp();
131
132			$this->_schemaTool->createSchema(array(
133			$this->_em->getClassMetadata(DDC2016User::CLASS_NAME),
134			$this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME),
135			));
136			}
137
138			protected function tearDown()
139			{
140			$this->_schemaTool->dropSchema(array(
141			$this->_em->getClassMetadata(DDC2016User::CLASS_NAME),
142			$this->_em->getClassMetadata(DC2016UserWithVo::CLASS_NAME),
143			));
144
145			parent::tearDown();
146			}
147			}
148
149			/**
150			* @Entity
151			* @MappedSuperclass()
152			*/
153			class DDC2016User
154			{
155			const CLASS_NAME = __CLASS__;
156
157			/** @Id @Column(type="integer") @GeneratedValue */
158			public $id;
159
160			/**
161			* @var DDC2016Username
162			*
163			* @Column(type="string", length=128, nullable=false)
164			*/
165			public $username;
166
167			/** Constructor
168			*
169			* @param DDC2016Username $username
170			*/
171			public function __construct(DDC2016Username $username)
172			{
173			$this->username = $username;
174			}
175
176			/**
177			* @return DDC2016Username
178			*/
179			public function getUsername()
180			{
181			return ($this->username instanceof DDC2016Username)
182			? $this->username
183			: $this->username = new DDC2016Username($this->username);
184			}
185
186			/**
187			* @param DDC2016Username $username
188			*
189			* @return $this
190			*/
191			public function setUsername(DDC2016Username $username)
192			{
193			$this->username = $username;
194
195			return $this;
196			}
197			}
198
199			/**
200			* Username ValueObject
201			*/
202			class DDC2016Username
203			{
204			const CLASS_NAME = __CLASS__;
205
206			/**
207			* @var string
208			*/
209			protected $username;
210
211			/**
212			* @param string $username
213			*
214			* @throws \InvalidArgumentException If $username is not a string.
215			* @throws \UnexpectedValueException If $username is not acceptable.
216			*/
217			public function __construct($username)
218			{
219			if ( ! is_string($username)) {
220			throw new \InvalidArgumentException(
221			sprintf('Username should be a string. [received: %s]', gettype($username))
222			);
223			}
224
225			if (preg_match('/.*-INVALID$/', $username)) {
226			throw new \UnexpectedValueException(
227			sprintf('Username not acceptable. [username: %s]', $username)
228			);
229			}
230
231			$this->username = $username;
232			}
233
234			/**
235			* @return string
236			*/
237			public function getUsername()
238			{
239			return $this->username;
240			}
241
242			public function __toString()
243			{
244			return $this->getUsername();
245			}
246			}
247
248			class DC2016UsernameVo extends DDC2016Username implements DoctrineValueObject
249			{
250			/**
251			* @return string
252			*/
253			public function __toString()
254			{
255			return $this->getUsername();
256			}
257
258			/**
259			* @param mixed $value
260			*
261			* @return bool
262			*/
263			public function equals($value)
264			{
265			if ($value instanceof DoctrineValueObject) {
266			return $this->getUsername() == (string)$value;
267			}
268
269			return $this->getUsername() == $value;
270			}
271			}
272
273			/**
274			* @Entity()
275			*/
276			class DC2016UserWithVo
277			{
278			const CLASS_NAME = __CLASS__;
279
280			/** @Id @Column(type="integer") @GeneratedValue */
281			public $id;
282
283			/**
284			* @var DDC2016Username
285			*
286			* @Column(type="string", length=128, nullable=false)
287			*/
288			public $username;
289
290			/** Constructor
291			*
292			* @param DDC2016Username $username
293			*/
294			public function __construct(DDC2016Username $username)
295			{
296			$this->username = $username;
297			}
298
299			public function getUsername()
300			{
301			return ($this->username instanceof DC2016UsernameVo)
302			? $this->username
303			: $this->username = new DC2016UsernameVo($this->username);
304			}
305
306			/**
307			* @param DDC2016Username $username
308			*
309			* @return $this
310			*/
311			public function setUsername(DDC2016Username $username)
312			{
313			$this->username = $username;
314
315			return $this;
316			}
317			}
318

doctrine / orm

Pull Request — master (#5841)

testDoctrineNotMarkAsChangedIfVOImplementsDoctrineValueObjectInterface() B

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like