From 580b9196e65adaacc05b9f7a50654739ad995597 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sun, 9 Jan 2022 20:15:56 +0100 Subject: [PATCH] Support readonly properties for read operations (#9316) * Provide failing test for readonly properties * Skip writing readonly properties if the value did not change --- .../ORM/Mapping/ClassMetadataInfo.php | 31 +++-- .../Mapping/ReflectionReadonlyProperty.php | 51 ++++++++ psalm-baseline.xml | 2 +- psalm.xml | 6 + .../Models/ReadonlyProperties/Author.php | 31 +++++ .../Tests/Models/ReadonlyProperties/Book.php | 51 ++++++++ .../Models/ReadonlyProperties/SimpleBook.php | 41 +++++++ .../ORM/Functional/ReadonlyPropertiesTest.php | 111 ++++++++++++++++++ .../ReflectionReadonlyPropertyTest.php | 59 ++++++++++ 9 files changed, 374 insertions(+), 9 deletions(-) create mode 100644 lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php create mode 100644 tests/Doctrine/Tests/Models/ReadonlyProperties/Author.php create mode 100644 tests/Doctrine/Tests/Models/ReadonlyProperties/Book.php create mode 100644 tests/Doctrine/Tests/Models/ReadonlyProperties/SimpleBook.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/ReadonlyPropertiesTest.php create mode 100644 tests/Doctrine/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 0359c2731da..c95461030bd 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -705,7 +705,7 @@ class ClassMetadataInfo implements ClassMetadata /** * The ReflectionProperty instances of the mapped class. * - * @var ReflectionProperty[]|null[] + * @var array */ public $reflFields = []; @@ -993,7 +993,8 @@ public function wakeupReflection($reflService) foreach ($this->embeddedClasses as $property => $embeddedClass) { if (isset($embeddedClass['declaredField'])) { - $childProperty = $reflService->getAccessibleProperty( + $childProperty = $this->getAccessibleProperty( + $reflService, $this->embeddedClasses[$embeddedClass['declaredField']]['class'], $embeddedClass['originalField'] ); @@ -1007,7 +1008,8 @@ public function wakeupReflection($reflService) continue; } - $fieldRefl = $reflService->getAccessibleProperty( + $fieldRefl = $this->getAccessibleProperty( + $reflService, $embeddedClass['declared'] ?? $this->name, $property ); @@ -1020,15 +1022,15 @@ public function wakeupReflection($reflService) if (isset($mapping['declaredField']) && isset($parentReflFields[$mapping['declaredField']])) { $this->reflFields[$field] = new ReflectionEmbeddedProperty( $parentReflFields[$mapping['declaredField']], - $reflService->getAccessibleProperty($mapping['originalClass'], $mapping['originalField']), + $this->getAccessibleProperty($reflService, $mapping['originalClass'], $mapping['originalField']), $mapping['originalClass'] ); continue; } $this->reflFields[$field] = isset($mapping['declared']) - ? $reflService->getAccessibleProperty($mapping['declared'], $field) - : $reflService->getAccessibleProperty($this->name, $field); + ? $this->getAccessibleProperty($reflService, $mapping['declared'], $field) + : $this->getAccessibleProperty($reflService, $this->name, $field); if (isset($mapping['enumType']) && $this->reflFields[$field] !== null) { $this->reflFields[$field] = new ReflectionEnumProperty( @@ -1040,8 +1042,8 @@ public function wakeupReflection($reflService) foreach ($this->associationMappings as $field => $mapping) { $this->reflFields[$field] = isset($mapping['declared']) - ? $reflService->getAccessibleProperty($mapping['declared'], $field) - : $reflService->getAccessibleProperty($this->name, $field); + ? $this->getAccessibleProperty($reflService, $mapping['declared'], $field) + : $this->getAccessibleProperty($reflService, $this->name, $field); } } @@ -3779,4 +3781,17 @@ private function assertMappingOrderBy(array $mapping): void throw new InvalidArgumentException("'orderBy' is expected to be an array, not " . gettype($mapping['orderBy'])); } } + + /** + * @psalm-param class-string $class + */ + private function getAccessibleProperty(ReflectionService $reflService, string $class, string $field): ?ReflectionProperty + { + $reflectionProperty = $reflService->getAccessibleProperty($class, $field); + if ($reflectionProperty !== null && PHP_VERSION_ID >= 80100 && $reflectionProperty->isReadOnly()) { + $reflectionProperty = new ReflectionReadonlyProperty($reflectionProperty); + } + + return $reflectionProperty; + } } diff --git a/lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php b/lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php new file mode 100644 index 00000000000..fba314be12e --- /dev/null +++ b/lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php @@ -0,0 +1,51 @@ +isReadOnly()) { + throw new InvalidArgumentException('Given property is not readonly.'); + } + + parent::__construct($wrappedProperty->class, $wrappedProperty->name); + } + + public function getValue(?object $object = null): mixed + { + return $this->wrappedProperty->getValue(...func_get_args()); + } + + public function setValue(mixed $objectOrValue, mixed $value = null): void + { + if (func_num_args() < 2 || $objectOrValue === null || ! $this->isInitialized($objectOrValue)) { + $this->wrappedProperty->setValue(...func_get_args()); + + return; + } + + assert(is_object($objectOrValue)); + + if (parent::getValue($objectOrValue) !== $value) { + throw new LogicException(sprintf('Attempting to change readonly property %s::$%s.', $this->class, $this->name)); + } + } +} diff --git a/psalm-baseline.xml b/psalm-baseline.xml index d5aacf2cf8d..2951933684e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -820,7 +820,7 @@ $parentReflFields[$embeddedClass['declaredField']] $parentReflFields[$mapping['declaredField']] $queryMapping['resultClass'] - $reflService->getAccessibleProperty($mapping['originalClass'], $mapping['originalField']) + $this->getAccessibleProperty($reflService, $mapping['originalClass'], $mapping['originalField']) $embeddable->reflClass->name diff --git a/psalm.xml b/psalm.xml index 57dabceddeb..e36df95e5be 100644 --- a/psalm.xml +++ b/psalm.xml @@ -75,6 +75,12 @@ + + + + + + diff --git a/tests/Doctrine/Tests/Models/ReadonlyProperties/Author.php b/tests/Doctrine/Tests/Models/ReadonlyProperties/Author.php new file mode 100644 index 00000000000..901b9ed3005 --- /dev/null +++ b/tests/Doctrine/Tests/Models/ReadonlyProperties/Author.php @@ -0,0 +1,31 @@ +id; + } + + public function getName(): string + { + return $this->name; + } +} diff --git a/tests/Doctrine/Tests/Models/ReadonlyProperties/Book.php b/tests/Doctrine/Tests/Models/ReadonlyProperties/Book.php new file mode 100644 index 00000000000..31e1b5b6be6 --- /dev/null +++ b/tests/Doctrine/Tests/Models/ReadonlyProperties/Book.php @@ -0,0 +1,51 @@ +authors = new ArrayCollection(); + } + + public function getId(): int + { + return $this->id; + } + + public function getTitle(): string + { + return $this->title; + } + + /** + * @return list + */ + public function getAuthors(): array + { + return $this->authors->getValues(); + } +} diff --git a/tests/Doctrine/Tests/Models/ReadonlyProperties/SimpleBook.php b/tests/Doctrine/Tests/Models/ReadonlyProperties/SimpleBook.php new file mode 100644 index 00000000000..d978d045839 --- /dev/null +++ b/tests/Doctrine/Tests/Models/ReadonlyProperties/SimpleBook.php @@ -0,0 +1,41 @@ +id; + } + + public function getTitle(): string + { + return $this->title; + } + + public function getAuthor(): Author + { + return $this->author; + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/ReadonlyPropertiesTest.php b/tests/Doctrine/Tests/ORM/Functional/ReadonlyPropertiesTest.php new file mode 100644 index 00000000000..17aa2306df6 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/ReadonlyPropertiesTest.php @@ -0,0 +1,111 @@ +_em = $this->getEntityManager(null, new AttributeDriver( + [dirname(__DIR__, 2) . '/Models/ReadonlyProperties'] + )); + $this->_schemaTool = new SchemaTool($this->_em); + + parent::setUp(); + + $this->setUpEntitySchema([Author::class, Book::class, SimpleBook::class]); + } + + public function testSimpleEntity(): void + { + $connection = $this->_em->getConnection(); + + $connection->insert('author', ['name' => 'Jane Austen']); + $authorId = $connection->lastInsertId(); + + $author = $this->_em->find(Author::class, $authorId); + + self::assertSame('Jane Austen', $author->getName()); + self::assertEquals($authorId, $author->getId()); + } + + public function testEntityWithLazyManyToOne(): void + { + $connection = $this->_em->getConnection(); + + $connection->insert('author', ['name' => 'Jane Austen']); + $authorId = $connection->lastInsertId(); + + $connection->insert('simple_book', ['title' => 'Pride and Prejudice', 'author_id' => $authorId]); + $bookId = $connection->lastInsertId(); + + $book = $this->_em->find(SimpleBook::class, $bookId); + + self::assertSame('Pride and Prejudice', $book->getTitle()); + self::assertEquals($bookId, $book->getId()); + self::assertSame('Jane Austen', $book->getAuthor()->getName()); + } + + public function testEntityWithEagerManyToOne(): void + { + $connection = $this->_em->getConnection(); + + $connection->insert('author', ['name' => 'Jane Austen']); + $authorId = $connection->lastInsertId(); + + $connection->insert('simple_book', ['title' => 'Pride and Prejudice', 'author_id' => $authorId]); + $bookId = $connection->lastInsertId(); + + [$book] = $this->_em->createQueryBuilder() + ->from(SimpleBook::class, 'b') + ->join('b.author', 'a') + ->select(['b', 'a']) + ->where('b.id = :id') + ->setParameter('id', $bookId) + ->getQuery() + ->execute(); + + self::assertInstanceOf(SimpleBook::class, $book); + self::assertSame('Pride and Prejudice', $book->getTitle()); + self::assertEquals($bookId, $book->getId()); + self::assertSame('Jane Austen', $book->getAuthor()->getName()); + } + + public function testEntityWithManyToMany(): void + { + $connection = $this->_em->getConnection(); + + $connection->insert('author', ['name' => 'Jane Austen']); + $authorId = $connection->lastInsertId(); + + $connection->insert('book', ['title' => 'Pride and Prejudice']); + $bookId = $connection->lastInsertId(); + + $connection->insert('book_author', ['book_id' => $bookId, 'author_id' => $authorId]); + + $book = $this->_em->find(Book::class, $bookId); + + self::assertSame('Pride and Prejudice', $book->getTitle()); + self::assertEquals($bookId, $book->getId()); + self::assertSame('Jane Austen', $book->getAuthors()[0]->getName()); + } +} diff --git a/tests/Doctrine/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php b/tests/Doctrine/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php new file mode 100644 index 00000000000..aa3e77444de --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/ReflectionReadonlyPropertyTest.php @@ -0,0 +1,59 @@ +setValue($author, 'John Doe'); + + self::assertSame('John Doe', $wrappedReflection->getValue($author)); + self::assertSame('John Doe', $reflection->getValue($author)); + + $reflection->setValue($author, 'John Doe'); + + self::assertSame('John Doe', $wrappedReflection->getValue($author)); + self::assertSame('John Doe', $reflection->getValue($author)); + } + + public function testSecondWriteWithDifferentValue(): void + { + $author = new Author(); + + $wrappedReflection = new ReflectionProperty($author, 'name'); + $reflection = new ReflectionReadonlyProperty($wrappedReflection); + + $reflection->setValue($author, 'John Doe'); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Attempting to change readonly property Doctrine\Tests\Models\ReadonlyProperties\Author::$name.'); + $reflection->setValue($author, 'Jane Doe'); + } + + public function testNonReadonlyPropertiesAreForbidden(): void + { + $reflection = new ReflectionProperty(CmsTag::class, 'name'); + + $this->expectException(InvalidArgumentException::class); + new ReflectionReadonlyProperty($reflection); + } +}