Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't Generate Set or Unset on Proxy of Readonly Public Properties #957

Open
wants to merge 5 commits into
base: 3.2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions lib/Doctrine/Common/Proxy/ProxyGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ public function __construct(?\Closure $initializer = null, ?\Closure $cloner = n

$toUnset = array_map(static function (string $name): string {
return '$this->' . $name;
}, $this->getLazyLoadedPublicPropertiesNames($class));
}, $this->getWriteableLazyLoadedPublicPropertiesNames($class));

return $constructorImpl . ($toUnset === [] ? '' : ' unset(' . implode(', ', $toUnset) . ");\n")
. <<<'EOT'
Expand Down Expand Up @@ -591,7 +591,7 @@ public function {$returnReference}__get($parametersString)$returnTypeHint
*/
private function generateMagicSet(ClassMetadata $class)
{
$lazyPublicProperties = $this->getLazyLoadedPublicPropertiesNames($class);
$lazyPublicProperties = $this->getWriteableLazyLoadedPublicPropertiesNames($class);
$reflectionClass = $class->getReflectionClass();
$hasParentSet = false;
$inheritDoc = '';
Expand Down Expand Up @@ -808,7 +808,7 @@ private function generateWakeupImpl(ClassMetadata $class)
$hasParentWakeup = $reflectionClass->hasMethod('__wakeup');

$unsetPublicProperties = [];
foreach ($this->getLazyLoadedPublicPropertiesNames($class) as $lazyPublicProperty) {
foreach ($this->getWriteableLazyLoadedPublicPropertiesNames($class) as $lazyPublicProperty) {
$unsetPublicProperties[] = '$this->' . $lazyPublicProperty;
}

Expand Down Expand Up @@ -1005,6 +1005,32 @@ private function isShortIdentifierGetter($method, ClassMetadata $class)
return false;
}

/**
* Generates the list of public properties to be lazy loaded, that are writable.
*
* @return list<string>
*/
public function getWriteableLazyLoadedPublicPropertiesNames(ClassMetadata $class): array
{
$properties = [];

foreach ($class->getReflectionClass()->getProperties(ReflectionProperty::IS_PUBLIC) as $property) {
$name = $property->getName();

if (
(! $class->hasField($name) && ! $class->hasAssociation($name))
|| $class->isIdentifier($name)
|| (method_exists($property, 'isReadOnly') && $property->isReadOnly())
) {
continue;
}

$properties[] = $name;
}

return $properties;
}

/**
* Generates the list of public properties to be lazy loaded.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace Doctrine\Tests\Common\Proxy;

class Php81ReadonlyPublicPropertyType
{
public readonly string $readable;
public string $writeable = 'default';

public function __construct(
public readonly string $id,
string $readable = 'readable-default'
) {
$this->readable = $readable;
}
}
62 changes: 62 additions & 0 deletions tests/Doctrine/Tests/Common/Proxy/ProxyGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Doctrine\Common\Proxy\Exception\InvalidArgumentException;
use Doctrine\Common\Proxy\Exception\UnexpectedValueException;
use Doctrine\Common\Proxy\Proxy;
use Doctrine\Common\Proxy\ProxyGenerator;
use Doctrine\Persistence\Mapping\ClassMetadata;
use PHPUnit\Framework\MockObject\MockObject;
Expand Down Expand Up @@ -525,6 +526,67 @@ public function testPhp81NeverType()
);
}

/**
* @requires PHP >= 8.1.0
*/
public function testPhp81ReadonlyPublicProperties()
{
$className = Php81ReadonlyPublicPropertyType::class;
$proxyClassName = 'Doctrine\Tests\Common\ProxyProxy\__CG__\\' . $className;
$initializationData = [
'id' => 'c0b5cb93-f01b-43f8-bc66-bc943b1ebcfd',
'readable' => 'This field is read-only.',
'writeable' => 'This field is writeable.',
];

if ( ! class_exists($proxyClassName, false)) {
$metadata = $this->createClassMetadata($className, ['id']);

$metadata
->method('hasField')
->will($this->returnCallback(static function (string $fieldName) use ($initializationData): bool {
return in_array($fieldName, array_keys($initializationData));
}));

$proxyGenerator = new ProxyGenerator(__DIR__ . '/generated', __NAMESPACE__ . 'Proxy');
$this->generateAndRequire($proxyGenerator, $metadata);
}

// Readonly properties are removed from unset.
self::assertStringContainsString(
'unset($this->writeable);',
file_get_contents(__DIR__ . '/generated/__CG__DoctrineTestsCommonProxyPhp81ReadonlyPublicPropertyType.php')
);

$proxy = new $proxyClassName(static function (Proxy $proxy, $method, $params) use (&$counter, $initializationData) {
if (!in_array($params[0], array_keys($initializationData))) {
throw new InvalidArgumentException(
sprintf('Should not be initialized when checking isset("%s")', $params[0])
);
}
$initializer = $proxy->__getInitializer();
$proxy->__setInitializer(null);
isset($this->{$params[0]}) || $this->{$params[0]} = $initializationData[$params[0]];
$counter += 1;
$proxy->__setInitializer($initializer);
});

self::assertTrue(isset($proxy->id));
self::assertTrue(isset($proxy->readable));
self::assertTrue(isset($proxy->writeable));
self::assertFalse(isset($proxy->nonExisting));

self::assertSame('c0b5cb93-f01b-43f8-bc66-bc943b1ebcfd', $proxy->id);
self::assertSame('This field is writeable.', $proxy->writeable);
$proxy->writeable = 'Updated string contents.';
self::assertSame('Updated string contents.', $proxy->writeable);

self::assertSame(3, $counter);

self::expectError();
$proxy->readable = 'Invalid';
}

/**
* @requires PHP >= 8.1.0
*/
Expand Down