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

Use lazy ghosts unconditionally #10969

Merged
merged 3 commits into from
Oct 8, 2023
Merged
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
8 changes: 0 additions & 8 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ jobs:
- "default"
extension:
- "pdo_sqlite"
proxy:
- "common"
deps:
- "highest"
include:
Expand All @@ -53,10 +51,6 @@ jobs:
- php-version: "8.2"
dbal-version: "default"
extension: "sqlite3"
- php-version: "8.1"
dbal-version: "default"
proxy: "lazy-ghost"
extension: "pdo_sqlite"
- php-version: "8.1"
dbal-version: "default"
deps: "lowest"
Expand Down Expand Up @@ -90,13 +84,11 @@ jobs:
run: "vendor/bin/phpunit -c ci/github/phpunit/${{ matrix.extension }}.xml --coverage-clover=coverage-no-cache.xml"
env:
ENABLE_SECOND_LEVEL_CACHE: 0
ORM_PROXY_IMPLEMENTATION: "${{ matrix.proxy }}"

- name: "Run PHPUnit with Second Level Cache"
run: "vendor/bin/phpunit -c ci/github/phpunit/${{ matrix.extension }}.xml --exclude-group performance,non-cacheable,locking_functional --coverage-clover=coverage-cache.xml"
env:
ENABLE_SECOND_LEVEL_CACHE: 1
ORM_PROXY_IMPLEMENTATION: "${{ matrix.proxy }}"

- name: "Upload coverage file"
uses: "actions/upload-artifact@v3"
Expand Down
14 changes: 14 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Upgrade to 3.0

## BC BREAK: `Doctrine\ORM\Proxy\ProxyFactory` no longer extends abstract factory from `doctrine/common`

It is no longer possible to call methods, constants or properties inherited
from that class on a `ProxyFactory` instance.

`Doctrine\ORM\Proxy\ProxyFactory::createProxyDefinition()` and
`Doctrine\ORM\Proxy\ProxyFactory::resetUninitializedProxy()` are removed as well.

## BC BREAK: lazy ghosts are enabled unconditionally

`Doctrine\ORM\Configuration::setLazyGhostObjectEnabled()` and
`Doctrine\ORM\Configuration::isLazyGhostObjectEnabled()` are now no-ops and
will be deprecated in 3.1.0

## BC BREAK: collisions in identity map are unconditionally rejected

`Doctrine\ORM\Configuration::setRejectIdCollisionInIdentityMap()` and
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"doctrine/lexer": "^2.1 || ^3",
"doctrine/persistence": "^3.1.1",
"psr/cache": "^1 || ^2 || ^3",
"symfony/console": "^5.4 || ^6.0 || ^7.0"
"symfony/console": "^5.4 || ^6.0 || ^7.0",
"symfony/var-exporter": "~6.2.13 || ^6.3.2"
},
"require-dev": {
"doctrine/coding-standard": "^12.0",
Expand All @@ -43,7 +44,6 @@
"psr/log": "^1 || ^2 || ^3",
"squizlabs/php_codesniffer": "3.7.2",
"symfony/cache": "^5.4 || ^6.2",
"symfony/var-exporter": "^6.2.13",
"vimeo/psalm": "5.15.0"
},
"suggest": {
Expand Down
30 changes: 12 additions & 18 deletions lib/Doctrine/ORM/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@
use Doctrine\ORM\Repository\DefaultRepositoryFactory;
use Doctrine\ORM\Repository\RepositoryFactory;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\Persistence\Reflection\RuntimeReflectionProperty;
use LogicException;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\VarExporter\LazyGhostTrait;

use function class_exists;
use function is_a;
use function strtolower;
use function trait_exists;

/**
* Configuration container for all configuration options of Doctrine.
Expand Down Expand Up @@ -581,28 +578,25 @@
$this->attributes['schemaIgnoreClasses'] = $schemaIgnoreClasses;
}

/**
* To be deprecated in 3.1.0
*
* @return true
*/
public function isLazyGhostObjectEnabled(): bool
{
return $this->attributes['isLazyGhostObjectEnabled'] ?? false;
return true;

Check warning on line 588 in lib/Doctrine/ORM/Configuration.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Configuration.php#L588

Added line #L588 was not covered by tests
}

/** To be deprecated in 3.1.0 */
public function setLazyGhostObjectEnabled(bool $flag): void
derrabus marked this conversation as resolved.
Show resolved Hide resolved
{
if ($flag && ! trait_exists(LazyGhostTrait::class)) {
throw new LogicException(
'Lazy ghost objects cannot be enabled because the "symfony/var-exporter" library'
. ' version 6.2 or higher is not installed. Please run "composer require symfony/var-exporter:^6.2".',
);
}

if ($flag && ! class_exists(RuntimeReflectionProperty::class)) {
throw new LogicException(
'Lazy ghost objects cannot be enabled because the "doctrine/persistence" library'
. ' version 3.1 or higher is not installed. Please run "composer update doctrine/persistence".',
);
if (! $flag) {
throw new LogicException(<<<'EXCEPTION'

Check warning on line 595 in lib/Doctrine/ORM/Configuration.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Configuration.php#L594-L595

Added lines #L594 - L595 were not covered by tests
The lazy ghost object feature cannot be disabled anymore.
Please remove the call to setLazyGhostObjectEnabled(false).
EXCEPTION);

Check warning on line 598 in lib/Doctrine/ORM/Configuration.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Configuration.php#L598

Added line #L598 was not covered by tests
}

$this->attributes['isLazyGhostObjectEnabled'] = $flag;
}

/** To be deprecated in 3.1.0 */
Expand Down
3 changes: 1 addition & 2 deletions lib/Doctrine/ORM/Proxy/InternalProxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
*
* @template T of object
* @template-extends Proxy<T>
*
* @method void __setInitialized(bool $initialized)
*/
interface InternalProxy extends Proxy
{
public function __setInitialized(bool $initialized): void;
}
181 changes: 7 additions & 174 deletions lib/Doctrine/ORM/Proxy/ProxyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@
namespace Doctrine\ORM\Proxy;

use Closure;
use Doctrine\Common\Proxy\AbstractProxyFactory;
use Doctrine\Common\Proxy\Proxy as CommonProxy;
use Doctrine\Common\Proxy\ProxyDefinition;
use Doctrine\Common\Proxy\ProxyGenerator;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\EntityNotFoundException;
use Doctrine\ORM\ORMInvalidArgumentException;
Expand All @@ -20,7 +15,6 @@
use Doctrine\Persistence\Proxy;
use ReflectionProperty;
use Symfony\Component\VarExporter\ProxyHelper;
use Throwable;

use function array_combine;
use function array_flip;
Expand Down Expand Up @@ -51,12 +45,11 @@
use function ucfirst;

use const DIRECTORY_SEPARATOR;
use const PHP_VERSION_ID;

/**
* This factory is used to create proxy objects for entities at runtime.
*/
class ProxyFactory extends AbstractProxyFactory
class ProxyFactory
{
/**
* Never autogenerate a proxy and rely that it was generated by some
Expand Down Expand Up @@ -137,9 +130,6 @@ public function __serialize(): array
/** @var array<class-string, Closure> */
private $proxyFactories = [];

/** @var bool */
private $isLazyGhostObjectEnabled = true;

/**
* Initializes a new instance of the <tt>ProxyFactory</tt> class that is
* connected to the given <tt>EntityManager</tt>.
Expand All @@ -155,23 +145,6 @@ public function __construct(
private readonly string $proxyNs,
bool|int $autoGenerate = self::AUTOGENERATE_NEVER,
) {
if (! $em->getConfiguration()->isLazyGhostObjectEnabled()) {
if (PHP_VERSION_ID >= 80100) {
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/pull/10837/',
'Not enabling lazy ghost objects is deprecated and will not be supported in Doctrine ORM 3.0. Ensure Doctrine\ORM\Configuration::setLazyGhostObjectEnabled(true) is called to enable them.',
);
}

$this->isLazyGhostObjectEnabled = false;

$proxyGenerator = new ProxyGenerator($proxyDir, $proxyNs);
$proxyGenerator->setPlaceholder('baseProxyInterface', CommonProxy::class . ', \\' . InternalProxy::class);

parent::__construct($proxyGenerator, $em->getMetadataFactory(), $autoGenerate);
}

if (! $proxyDir) {
throw ORMInvalidArgumentException::proxyDirectoryRequired();
}
Expand All @@ -191,14 +164,13 @@ public function __construct(
}

/**
* {@inheritDoc}
* @param class-string $className
* @param array<mixed> $identifier
*
* @return InternalProxy
Copy link
Member Author

@greg0ire greg0ire Oct 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a native return type here will involve modify some tests that mock calls to getProxy and make them return actual entities.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix those tests, but that could be done in a follow-up.

*/
public function getProxy($className, array $identifier)
public function getProxy(string $className, array $identifier)
{
if (! $this->isLazyGhostObjectEnabled) {
return parent::getProxy($className, $identifier);
}

$proxyFactory = $this->proxyFactories[$className] ?? $this->getProxyFactory($className);

return $proxyFactory($identifier);
Expand All @@ -214,12 +186,8 @@ public function getProxy($className, array $identifier)
*
* @return int Number of generated proxies.
*/
public function generateProxyClasses(array $classes, $proxyDir = null)
public function generateProxyClasses(array $classes, $proxyDir = null): int
{
if (! $this->isLazyGhostObjectEnabled) {
return parent::generateProxyClasses($classes, $proxyDir);
}

$generated = 0;

foreach ($classes as $class) {
Expand All @@ -238,108 +206,13 @@ public function generateProxyClasses(array $classes, $proxyDir = null)
return $generated;
}

/**
* {@inheritDoc}
*
* @deprecated ProxyFactory::resetUninitializedProxy() is deprecated and will be removed in version 3.0 of doctrine/orm.
*/
public function resetUninitializedProxy(CommonProxy $proxy)
{
return parent::resetUninitializedProxy($proxy);
}

protected function skipClass(ClassMetadata $metadata): bool
{
return $metadata->isMappedSuperclass
|| $metadata->isEmbeddedClass
|| $metadata->getReflectionClass()->isAbstract();
}

/**
* {@inheritDoc}
*
* @deprecated ProxyFactory::createProxyDefinition() is deprecated and will be removed in version 3.0 of doctrine/orm.
*/
protected function createProxyDefinition($className): ProxyDefinition
{
$classMetadata = $this->em->getClassMetadata($className);
$entityPersister = $this->uow->getEntityPersister($className);

$initializer = $this->createInitializer($classMetadata, $entityPersister);
$cloner = $this->createCloner($classMetadata, $entityPersister);

return new ProxyDefinition(
self::generateProxyClassName($className, $this->proxyNs),
$classMetadata->getIdentifierFieldNames(),
$classMetadata->getReflectionProperties(),
$initializer,
$cloner,
);
}

/**
* Creates a closure capable of initializing a proxy
*
* @deprecated ProxyFactory::createInitializer() is deprecated and will be removed in version 3.0 of doctrine/orm.
*
* @psalm-return Closure(CommonProxy):void
*
* @throws EntityNotFoundException
*/
private function createInitializer(ClassMetadata $classMetadata, EntityPersister $entityPersister): Closure
{
$wakeupProxy = $classMetadata->getReflectionClass()->hasMethod('__wakeup');

return function (CommonProxy $proxy) use ($entityPersister, $classMetadata, $wakeupProxy): void {
$initializer = $proxy->__getInitializer();
$cloner = $proxy->__getCloner();

$proxy->__setInitializer(null);
$proxy->__setCloner(null);

if ($proxy->__isInitialized()) {
return;
}

$properties = $proxy->__getLazyProperties();

foreach ($properties as $propertyName => $property) {
if (! isset($proxy->$propertyName)) {
$proxy->$propertyName = $properties[$propertyName];
}
}

$proxy->__setInitialized(true);

if ($wakeupProxy) {
$proxy->__wakeup();
}

$identifier = $classMetadata->getIdentifierValues($proxy);

try {
$entity = $entityPersister->loadById($identifier, $proxy);
} catch (Throwable $exception) {
$proxy->__setInitializer($initializer);
$proxy->__setCloner($cloner);
$proxy->__setInitialized(false);

throw $exception;
}

if ($entity === null) {
$proxy->__setInitializer($initializer);
$proxy->__setCloner($cloner);
$proxy->__setInitialized(false);

throw EntityNotFoundException::fromClassNameAndIdentifier(
$classMetadata->getName(),
$this->identifierFlattener->flattenIdentifier($classMetadata, $identifier),
);
}
};
}

/**
* Creates a closure capable of initializing a proxy
*
Expand Down Expand Up @@ -376,46 +249,6 @@ private function createLazyInitializer(ClassMetadata $classMetadata, EntityPersi
};
}

/**
* Creates a closure capable of finalizing state a cloned proxy
*
* @deprecated ProxyFactory::createCloner() is deprecated and will be removed in version 3.0 of doctrine/orm.
*
* @psalm-return Closure(CommonProxy):void
*
* @throws EntityNotFoundException
*/
private function createCloner(ClassMetadata $classMetadata, EntityPersister $entityPersister): Closure
{
return function (CommonProxy $proxy) use ($entityPersister, $classMetadata): void {
if ($proxy->__isInitialized()) {
return;
}

$proxy->__setInitialized(true);
$proxy->__setInitializer(null);

$class = $entityPersister->getClassMetadata();
$identifier = $classMetadata->getIdentifierValues($proxy);
$original = $entityPersister->loadById($identifier);

if ($original === null) {
throw EntityNotFoundException::fromClassNameAndIdentifier(
$classMetadata->getName(),
$this->identifierFlattener->flattenIdentifier($classMetadata, $identifier),
);
}

foreach ($class->getReflectionProperties() as $property) {
if (! $class->hasField($property->name) && ! $class->hasAssociation($property->name)) {
continue;
}

$property->setValue($proxy, $property->getValue($original));
}
};
}

private function getProxyFileName(string $className, string $baseDirectory): string
{
$baseDirectory = $baseDirectory ?: $this->proxyDir;
Expand Down
Loading