Skip to content

Commit a6e3f5c

Browse files
garakgarak
committed
loose compare readonly properties
1 parent ee70178 commit a6e3f5c

File tree

5 files changed

+107
-18
lines changed

5 files changed

+107
-18
lines changed

‎src/Mapping/PropertyAccessors/ReadonlyAccessor.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ public function setValue(object $object, mixed $value): void
3232
return;
3333
}
3434

35-
if ($this->parent->getValue($object) !== $value) {
35+
// WARNING: do not use strict comparison here, see https://github.com/doctrine/orm/issues/9505
36+
// phpcs:ignore SlevomatCodingStandard.Operators.DisallowEqualOperators.DisallowedNotEqualOperator
37+
if ($this->parent->getValue($object) != $value) {
3638
throw new LogicException(sprintf(
3739
'Attempting to change readonly property %s::$%s.',
3840
$this->reflectionProperty->getDeclaringClass()->getName(),

‎src/Mapping/ReflectionReadonlyProperty.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ public function setValue(mixed $objectOrValue, mixed $value = null): void
4242

4343
assert(is_object($objectOrValue));
4444

45-
if (parent::getValue($objectOrValue) !== $value) {
45+
// WARNING: do not use strict comparison here, see https://github.com/doctrine/orm/issues/9505
46+
// phpcs:ignore SlevomatCodingStandard.Operators.DisallowEqualOperators.DisallowedNotEqualOperator
47+
if (parent::getValue($objectOrValue) != $value) {
4648
throw new LogicException(sprintf('Attempting to change readonly property %s::$%s.', $this->class, $this->name));
4749
}
4850
}

‎tests/Tests/Models/ReadonlyProperties/Library.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\Models\ReadonlyProperties;
6+
7+
use Doctrine\ORM\Mapping\Column;
8+
use Doctrine\ORM\Mapping\Entity;
9+
use Doctrine\ORM\Mapping\GeneratedValue;
10+
use Doctrine\ORM\Mapping\Id;
11+
use Doctrine\Tests\Models\ValueObjects\Uuid;
12+
13+
#[Entity]
14+
class Library
15+
{
16+
#[Column]
17+
#[Id]
18+
#[GeneratedValue(strategy: 'NONE')]
19+
public readonly Uuid $uuid;
20+
}

‎tests/Tests/Models/ValueObjects/Uuid.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\Models\ValueObjects;
6+
7+
class Uuid
8+
{
9+
/** @var string */
10+
public $id;
11+
12+
public function __construct(string $id)
13+
{
14+
$this->id = $id;
15+
}
16+
}

‎tests/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php‎

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,43 +7,92 @@
77
use Doctrine\ORM\Mapping\ReflectionReadonlyProperty;
88
use Doctrine\Tests\Models\CMS\CmsTag;
99
use Doctrine\Tests\Models\ReadonlyProperties\Author;
10+
use Doctrine\Tests\Models\ReadonlyProperties\Library;
11+
use Doctrine\Tests\Models\ValueObjects\Uuid;
12+
use Generator;
1013
use InvalidArgumentException;
1114
use LogicException;
15+
use PHPUnit\Framework\Attributes\DataProvider;
1216
use PHPUnit\Framework\TestCase;
1317
use ReflectionProperty;
1418

1519
class ReflectionReadonlyPropertyTest extends TestCase
1620
{
17-
public function testSecondWriteWithSameValue(): void
21+
#[DataProvider('sameValueProvider')]
22+
public function testSecondWriteWithSameValue(object $entity, string $property, mixed $value, mixed $sameValue): void
1823
{
19-
$author = new Author();
20-
21-
$wrappedReflection = new ReflectionProperty($author, 'name');
24+
$wrappedReflection = new ReflectionProperty($entity, $property);
2225
$reflection = new ReflectionReadonlyProperty($wrappedReflection);
2326

24-
$reflection->setValue($author, 'John Doe');
27+
$reflection->setValue($entity, $value);
2528

26-
self::assertSame('John Doe', $wrappedReflection->getValue($author));
27-
self::assertSame('John Doe', $reflection->getValue($author));
29+
self::assertSame($value, $wrappedReflection->getValue($entity));
30+
self::assertSame($value, $reflection->getValue($entity));
2831

29-
$reflection->setValue($author, 'John Doe');
32+
$reflection->setValue($entity, $sameValue);
3033

31-
self::assertSame('John Doe', $wrappedReflection->getValue($author));
32-
self::assertSame('John Doe', $reflection->getValue($author));
34+
/*
35+
* Intentionally testing against the initial $value rather than the $sameValue that we just set above one in
36+
* order to catch false positives when dealing with object types
37+
*/
38+
self::assertSame($value, $wrappedReflection->getValue($entity));
39+
self::assertSame($value, $reflection->getValue($entity));
3340
}
3441

35-
public function testSecondWriteWithDifferentValue(): void
42+
/** @return Generator<string, array{entity: object, property: string, value: string|object, sameValue: string|object}> */
43+
public static function sameValueProvider(): Generator
3644
{
37-
$author = new Author();
45+
yield 'string' => [
46+
'entity' => new Author(),
47+
'property' => 'name',
48+
'value' => 'John Doe',
49+
'sameValue' => 'John Doe',
50+
];
51+
52+
yield 'uuid' => [
53+
'entity' => new Library(),
54+
'property' => 'uuid',
55+
'value' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'),
56+
'sameValue' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'),
57+
];
58+
}
3859

39-
$wrappedReflection = new ReflectionProperty($author, 'name');
60+
#[DataProvider('differentValueProvider')]
61+
public function testSecondWriteWithDifferentValue(
62+
object $entity,
63+
string $property,
64+
mixed $value,
65+
mixed $differentValue,
66+
string $expectedExceptionMessage,
67+
): void {
68+
$wrappedReflection = new ReflectionProperty($entity, $property);
4069
$reflection = new ReflectionReadonlyProperty($wrappedReflection);
4170

42-
$reflection->setValue($author, 'John Doe');
71+
$reflection->setValue($entity, $value);
4372

4473
$this->expectException(LogicException::class);
45-
$this->expectExceptionMessage('Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Author::$name.');
46-
$reflection->setValue($author, 'Jane Doe');
74+
$this->expectExceptionMessage($expectedExceptionMessage);
75+
$reflection->setValue($entity, $differentValue);
76+
}
77+
78+
/** @return Generator<string, array{entity: object, property: string, value: string|object, sameValue: string|object, expectedExceptionMessage: string}> */
79+
public static function differentValueProvider(): Generator
80+
{
81+
yield 'string' => [
82+
'entity' => new Author(),
83+
'property' => 'name',
84+
'value' => 'John Doe',
85+
'differentValue' => 'Jane Doe',
86+
'expectedExceptionMessage' => 'Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Author::$name.',
87+
];
88+
89+
yield 'uuid' => [
90+
'entity' => new Library(),
91+
'property' => 'uuid',
92+
'value' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'),
93+
'differentValue' => new Uuid('5d5049ee-01fd-4b66-9f82-9f637fff6a7d'),
94+
'expectedExceptionMessage' => 'Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Library::$uuid.',
95+
];
4796
}
4897

4998
public function testNonReadonlyPropertiesAreForbidden(): void

0 commit comments

Comments
 (0)