DDC2016User::__construct() - 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 06:56 UTC

DDC2016User::__construct() A

↳ Parent: DDC2016User

Complexity

Conditions	1
Paths	1

Size

Total Lines	4
Code Lines	2

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
c	0
b	0
f	0
dl	0
loc	4
rs	10
cc	1
eloc	2
nc	1
nop	1

<?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);
        $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)->find($user->id);
        $this->assertInstanceOf(DDC2016User::class, $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, $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);
        $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 dbValue not the ValueObject.
         */
        $this->assertInternalType('string', $changeSet['username'][1]);

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

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

        $username = $user->getUsername();
        $this->assertInstanceOf(DC2016UsernameVo::class, $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(__NAMESPACE__ . '\DDC2016User'),
            $this->_em->getClassMetadata(__NAMESPACE__ . '\DC2016UserWithVo'),
        ));
    }

    protected function tearDown()
    {
        $this->_schemaTool->dropSchema(array(
            $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2016User'),
            $this->_em->getClassMetadata(__NAMESPACE__ . '\DC2016UserWithVo'),
        ));

        parent::tearDown();
    }
}

/**
 * @Entity
 * @MappedSuperclass()
 */
class DDC2016User
{
    /** @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
{
    /**
     * @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;
    }
}

class DC2016UsernameVo extends DDC2016Username implements DoctrineValueObject
{
    public $test = 'valami';

    /**
     * @return mixed
     */
    public function toPhpValue()
    {
//        return $this->test;
        return $this->getUsername();
    }

    /**
     * @param mixed $value
     *
     * @return bool
     */
    public function equals($value)
    {
        if ($value instanceof DoctrineValueObject) {
if ($x instanceof DoesNotExist) {
    // Do something.
}
            return $this->getUsername() == $value->toPhpValue();
        }

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

/**
 * @Entity()
 */
class DC2016UserWithVo
{
    /** @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;
    }
}


Pull Request — master (#5841)

DDC2016User::__construct() A

Complexity

Size

Duplication

Importance

1. Missing dependencies

2. Missing use statement

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);
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)->find($user->id);
39			$this->assertInstanceOf(DDC2016User::class, $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, $username);
51
52			$uow->computeChangeSet($metadata, $user);
			0 ignored issues – show Bug introduced 2016-05-27 07:03 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor...class)->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:03 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor...class)->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);
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 dbValue not the ValueObject.
77			*/
78			$this->assertInternalType('string', $changeSet['username'][1]);
79
80			$this->_em->flush();
81			$this->_em->clear();
82
83			$user = $this->_em->getRepository(DC2016UserWithVo::class)->find($user->id);
84			$this->assertInstanceOf(DC2016UserWithVo::class, $user);
85			$this->assertEquals($txtUser, $user->getUsername()->toPhpValue());
86
87			$username = $user->getUsername();
88			$this->assertInstanceOf(DC2016UsernameVo::class, $username);
89
90			$uow->computeChangeSet($metadata, $user);
			0 ignored issues – show Bug introduced 2016-05-27 07:03 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor...class)->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 07:03 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->_em->getRepositor...class)->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(__NAMESPACE__ . '\DDC2016User'),
134			$this->_em->getClassMetadata(__NAMESPACE__ . '\DC2016UserWithVo'),
135			));
136			}
137
138			protected function tearDown()
139			{
140			$this->_schemaTool->dropSchema(array(
141			$this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2016User'),
142			$this->_em->getClassMetadata(__NAMESPACE__ . '\DC2016UserWithVo'),
143			));
144
145			parent::tearDown();
146			}
147			}
148
149			/**
150			* @Entity
151			* @MappedSuperclass()
152			*/
153			class DDC2016User
154			{
155			/** @Id @Column(type="integer") @GeneratedValue */
156			public $id;
157
158			/**
159			* @var DDC2016Username
160			*
161			* @Column(type="string", length=128, nullable=false)
162			*/
163			public $username;
164
165			/** Constructor
166			*
167			* @param DDC2016Username $username
168			*/
169			public function __construct(DDC2016Username $username)
170			{
171			$this->username = $username;
172			}
173
174			/**
175			* @return DDC2016Username
176			*/
177			public function getUsername()
178			{
179			return ($this->username instanceof DDC2016Username)
180			? $this->username
181			: $this->username = new DDC2016Username($this->username);
182			}
183
184			/**
185			* @param DDC2016Username $username
186			*
187			* @return $this
188			*/
189			public function setUsername(DDC2016Username $username)
190			{
191			$this->username = $username;
192
193			return $this;
194			}
195			}
196
197			/**
198			* Username ValueObject
199			*/
200			class DDC2016Username
201			{
202			/**
203			* @var string
204			*/
205			protected $username;
206
207			/**
208			* @param string $username
209			*
210			* @throws \InvalidArgumentException If $username is not a string.
211			* @throws \UnexpectedValueException If $username is not acceptable.
212			*/
213			public function __construct($username)
214			{
215			if ( ! is_string($username)) {
216			throw new \InvalidArgumentException(
217			sprintf('Username should be a string. [received: %s]', gettype($username))
218			);
219			}
220
221			if (preg_match('/.*-INVALID$/', $username)) {
222			throw new \UnexpectedValueException(
223			sprintf('Username not acceptable. [username: %s]', $username)
224			);
225			}
226
227			$this->username = $username;
228			}
229
230			/**
231			* @return string
232			*/
233			public function getUsername()
234			{
235			return $this->username;
236			}
237			}
238
239			class DC2016UsernameVo extends DDC2016Username implements DoctrineValueObject
240			{
241			public $test = 'valami';
242
243			/**
244			* @return mixed
245			*/
246			public function toPhpValue()
247			{
248			// return $this->test;
249			return $this->getUsername();
250			}
251
252			/**
253			* @param mixed $value
254			*
255			* @return bool
256			*/
257			public function equals($value)
258			{
259			if ($value instanceof DoctrineValueObject) {
			0 ignored issues – show Bug introduced 2016-05-27 07:03 UTC by Report Bug Copy Issue Report The class `Doctrine\ORM\DoctrineValueObject` does not exist. Did you forget a USE statement, or did you not list all dependencies? This error could be the result of: 1. Missing dependencies PHP Analyzer uses your `composer.json` file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects the `composer.json` to be in the root folder of your repository. Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the `require` or `require-dev` section? 2. Missing use statement PHP does not complain about undefined classes in `ìnstanceof` checks. For example, the following PHP code will work perfectly fine: if ($x instanceof DoesNotExist) { // Do something. } If you have not tested against this specific condition, such errors might go unnoticed. Loading history...
260			return $this->getUsername() == $value->toPhpValue();
261			}
262
263			return $this->getUsername() == $value;
264			}
265			}
266
267			/**
268			* @Entity()
269			*/
270			class DC2016UserWithVo
271			{
272			/** @Id @Column(type="integer") @GeneratedValue */
273			public $id;
274
275			/**
276			* @var DDC2016Username
277			*
278			* @Column(type="string", length=128, nullable=false)
279			*/
280			public $username;
281
282			/** Constructor
283			*
284			* @param DDC2016Username $username
285			*/
286			public function __construct(DDC2016Username $username)
287			{
288			$this->username = $username;
289			}
290
291			public function getUsername()
292			{
293			return ($this->username instanceof DC2016UsernameVo)
294			? $this->username
295			: $this->username = new DC2016UsernameVo($this->username);
296			}
297
298			/**
299			* @param DDC2016Username $username
300			*
301			* @return $this
302			*/
303			public function setUsername(DDC2016Username $username)
304			{
305			$this->username = $username;
306
307			return $this;
308			}
309			}
310

doctrine / orm

Pull Request — master (#5841)

DDC2016User::__construct() A

Complexity

Size

Duplication

Importance

1. Missing dependencies

2. Missing use statement

Duplication Side-by-Side

Filter issues like