Skip to content

Commit

Permalink
Support readonly properties for read operations (#9316)
Browse files Browse the repository at this point in the history
* Provide failing test for readonly properties

* Skip writing readonly properties if the value did not change
  • Loading branch information
derrabus authored Jan 9, 2022
1 parent 0d911b9 commit 580b919
Show file tree
Hide file tree
Showing 9 changed files with 374 additions and 9 deletions.
31 changes: 23 additions & 8 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ class ClassMetadataInfo implements ClassMetadata
/**
* The ReflectionProperty instances of the mapped class.
*
* @var ReflectionProperty[]|null[]
* @var array<string, ReflectionProperty|null>
*/
public $reflFields = [];

Expand Down Expand Up @@ -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']
);
Expand All @@ -1007,7 +1008,8 @@ public function wakeupReflection($reflService)
continue;
}

$fieldRefl = $reflService->getAccessibleProperty(
$fieldRefl = $this->getAccessibleProperty(
$reflService,
$embeddedClass['declared'] ?? $this->name,
$property
);
Expand All @@ -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(
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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;
}
}
51 changes: 51 additions & 0 deletions lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Mapping;

use InvalidArgumentException;
use LogicException;
use ReflectionProperty;

use function assert;
use function func_get_args;
use function func_num_args;
use function is_object;
use function sprintf;

/**
* @internal
*/
final class ReflectionReadonlyProperty extends ReflectionProperty
{
public function __construct(
private ReflectionProperty $wrappedProperty
) {
if (! $wrappedProperty->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));
}
}
}
2 changes: 1 addition & 1 deletion psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@
<code>$parentReflFields[$embeddedClass['declaredField']]</code>
<code>$parentReflFields[$mapping['declaredField']]</code>
<code>$queryMapping['resultClass']</code>
<code>$reflService-&gt;getAccessibleProperty($mapping['originalClass'], $mapping['originalField'])</code>
<code>$this-&gt;getAccessibleProperty($reflService, $mapping['originalClass'], $mapping['originalField'])</code>
</PossiblyNullArgument>
<PossiblyNullPropertyFetch occurrences="2">
<code>$embeddable-&gt;reflClass-&gt;name</code>
Expand Down
6 changes: 6 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@
<referencedClass name="Doctrine\DBAL\Platforms\PostgreSQLPlatform" />
</errorLevel>
</InvalidClass>
<MethodSignatureMismatch>
<errorLevel type="suppress">
<!-- See https://github.com/vimeo/psalm/issues/7357 -->
<file name="lib/Doctrine/ORM/Mapping/ReflectionReadonlyProperty.php"/>
</errorLevel>
</MethodSignatureMismatch>
<MissingDependency>
<errorLevel type="suppress">
<!-- DBAL 3.2 forward compatibility -->
Expand Down
31 changes: 31 additions & 0 deletions tests/Doctrine/Tests/Models/ReadonlyProperties/Author.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\ReadonlyProperties;

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\Table;

#[Entity, Table(name: 'author')]
class Author
{
#[Column, Id, GeneratedValue]
private readonly int $id;

#[Column]
private readonly string $name;

public function getId(): int
{
return $this->id;
}

public function getName(): string
{
return $this->name;
}
}
51 changes: 51 additions & 0 deletions tests/Doctrine/Tests/Models/ReadonlyProperties/Book.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\ReadonlyProperties;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\JoinTable;
use Doctrine\ORM\Mapping\ManyToMany;
use Doctrine\ORM\Mapping\Table;

#[Entity, Table(name: 'book')]
class Book
{
#[Column, Id, GeneratedValue]
private readonly int $id;

#[Column]
private readonly string $title;

#[ManyToMany(targetEntity: Author::class), JoinTable(name: 'book_author')]
private readonly Collection $authors;

public function __construct()
{
$this->authors = new ArrayCollection();
}

public function getId(): int
{
return $this->id;
}

public function getTitle(): string
{
return $this->title;
}

/**
* @return list<Author>
*/
public function getAuthors(): array
{
return $this->authors->getValues();
}
}
41 changes: 41 additions & 0 deletions tests/Doctrine/Tests/Models/ReadonlyProperties/SimpleBook.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\ReadonlyProperties;

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\JoinColumn;
use Doctrine\ORM\Mapping\ManyToOne;
use Doctrine\ORM\Mapping\Table;

#[Entity, Table(name: 'simple_book')]
class SimpleBook
{
#[Column, Id, GeneratedValue]
private readonly int $id;

#[Column]
private readonly string $title;

#[ManyToOne, JoinColumn(nullable: false)]
private readonly Author $author;

public function getId(): int
{
return $this->id;
}

public function getTitle(): string
{
return $this->title;
}

public function getAuthor(): Author
{
return $this->author;
}
}
111 changes: 111 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/ReadonlyPropertiesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional;

use Doctrine\ORM\Mapping\Driver\AttributeDriver;
use Doctrine\ORM\Tools\SchemaTool;
use Doctrine\Tests\Models\ReadonlyProperties\Author;
use Doctrine\Tests\Models\ReadonlyProperties\Book;
use Doctrine\Tests\Models\ReadonlyProperties\SimpleBook;
use Doctrine\Tests\OrmFunctionalTestCase;
use Doctrine\Tests\TestUtil;

use function dirname;

/**
* @requires PHP 8.1
*/
class ReadonlyPropertiesTest extends OrmFunctionalTestCase
{
protected function setUp(): void
{
if (! isset(static::$sharedConn)) {
static::$sharedConn = TestUtil::getConnection();
}

$this->_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());
}
}
Loading

0 comments on commit 580b919

Please sign in to comment.