From ed20cf5b00305ec0ddf8ba29a7414f1a3ef4833c Mon Sep 17 00:00:00 2001 From: Massimiliano Arione Date: Mon, 30 Sep 2024 14:50:56 +0200 Subject: [PATCH] check readonly property via serialization --- src/Mapping/ReflectionReadonlyProperty.php | 3 +- .../Models/ReadonlyProperties/Library.php | 22 +++++ tests/Tests/Models/ValueObjects/Uuid.php | 16 ++++ .../ReflectionReadonlyPropertyTest.php | 82 +++++++++++++++---- 4 files changed, 106 insertions(+), 17 deletions(-) create mode 100644 tests/Tests/Models/ReadonlyProperties/Library.php create mode 100644 tests/Tests/Models/ValueObjects/Uuid.php diff --git a/src/Mapping/ReflectionReadonlyProperty.php b/src/Mapping/ReflectionReadonlyProperty.php index ed91cd6715d..9cc655ff955 100644 --- a/src/Mapping/ReflectionReadonlyProperty.php +++ b/src/Mapping/ReflectionReadonlyProperty.php @@ -12,6 +12,7 @@ use function func_get_args; use function func_num_args; use function is_object; +use function serialize; use function sprintf; /** @internal */ @@ -42,7 +43,7 @@ public function setValue(mixed $objectOrValue, mixed $value = null): void assert(is_object($objectOrValue)); - if (parent::getValue($objectOrValue) !== $value) { + if (serialize(parent::getValue($objectOrValue)) !== serialize($value)) { throw new LogicException(sprintf('Attempting to change readonly property %s::$%s.', $this->class, $this->name)); } } diff --git a/tests/Tests/Models/ReadonlyProperties/Library.php b/tests/Tests/Models/ReadonlyProperties/Library.php new file mode 100644 index 00000000000..a7697f8d8a5 --- /dev/null +++ b/tests/Tests/Models/ReadonlyProperties/Library.php @@ -0,0 +1,22 @@ +id = $id; + } +} diff --git a/tests/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php b/tests/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php index aa3e77444de..e4deb4283f8 100644 --- a/tests/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php +++ b/tests/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php @@ -7,6 +7,9 @@ use Doctrine\ORM\Mapping\ReflectionReadonlyProperty; use Doctrine\Tests\Models\CMS\CmsTag; use Doctrine\Tests\Models\ReadonlyProperties\Author; +use Doctrine\Tests\Models\ReadonlyProperties\Library; +use Doctrine\Tests\Models\ValueObjects\Uuid; +use Generator; use InvalidArgumentException; use LogicException; use PHPUnit\Framework\TestCase; @@ -17,36 +20,83 @@ */ class ReflectionReadonlyPropertyTest extends TestCase { - public function testSecondWriteWithSameValue(): void + /** + * @dataProvider sameValueProvider + */ + public function testSecondWriteWithSameValue(object $entity, string $property, mixed $value, mixed $sameValue): void { - $author = new Author(); - - $wrappedReflection = new ReflectionProperty($author, 'name'); + $wrappedReflection = new ReflectionProperty($entity, $property); $reflection = new ReflectionReadonlyProperty($wrappedReflection); - $reflection->setValue($author, 'John Doe'); + $reflection->setValue($entity, $value); - self::assertSame('John Doe', $wrappedReflection->getValue($author)); - self::assertSame('John Doe', $reflection->getValue($author)); + self::assertSame($value, $wrappedReflection->getValue($entity)); + self::assertSame($value, $reflection->getValue($entity)); - $reflection->setValue($author, 'John Doe'); + $reflection->setValue($entity, $sameValue); - self::assertSame('John Doe', $wrappedReflection->getValue($author)); - self::assertSame('John Doe', $reflection->getValue($author)); + /* + * Intentionally testing against the initial $value rather than the $sameValue that we just set above one in + * order to catch false positives when dealing with object types + */ + self::assertSame($value, $wrappedReflection->getValue($entity)); + self::assertSame($value, $reflection->getValue($entity)); } - public function testSecondWriteWithDifferentValue(): void + public function sameValueProvider(): Generator { - $author = new Author(); + yield 'string' => [ + 'entity' => new Author(), + 'property' => 'name', + 'value' => 'John Doe', + 'sameValue' => 'John Doe', + ]; + + yield 'uuid' => [ + 'entity' => new Library(), + 'property' => 'uuid', + 'value' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'), + 'sameValue' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'), + ]; + } - $wrappedReflection = new ReflectionProperty($author, 'name'); + /** + * @dataProvider differentValueProvider + */ + public function testSecondWriteWithDifferentValue( + object $entity, + string $property, + mixed $value, + mixed $differentValue, + string $expectedExceptionMessage + ): void { + $wrappedReflection = new ReflectionProperty($entity, $property); $reflection = new ReflectionReadonlyProperty($wrappedReflection); - $reflection->setValue($author, 'John Doe'); + $reflection->setValue($entity, $value); $this->expectException(LogicException::class); - $this->expectExceptionMessage('Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Author::$name.'); - $reflection->setValue($author, 'Jane Doe'); + $this->expectExceptionMessage($expectedExceptionMessage); + $reflection->setValue($entity, $differentValue); + } + + public function differentValueProvider(): Generator + { + yield 'string' => [ + 'entity' => new Author(), + 'property' => 'name', + 'value' => 'John Doe', + 'differentValue' => 'Jane Doe', + 'expectedExceptionMessage' => 'Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Author::$name.', + ]; + + yield 'uuid' => [ + 'entity' => new Library(), + 'property' => 'uuid', + 'value' => new Uuid('438d5dc3-36c9-410a-88db-7a184856ebb8'), + 'differentValue' => new Uuid('5d5049ee-01fd-4b66-9f82-9f637fff6a7d'), + 'expectedExceptionMessage' => 'Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Library::$uuid.', + ]; } public function testNonReadonlyPropertiesAreForbidden(): void