Switch proxies to `LazyGhostTrait` #2700

GromNaN · 2024-11-23T21:26:07Z

Q	A
Type	improvement
BC Break	no
Fixed issues	Replace #2692

Summary

Use the LazyGhostTrait from symfony/var-exporter to generate lazy proxy classes.

Add LazyGhostProxyFactory that implements the ProxyFactory interface and use it by default
Add configuration to switch back to Proxy Manager, and run the test suite with this it
Deprecate using proxy manager, to be removed in 3.0
Add documentation (including Autoloading Proxies)
Review implementation to ensure property hooks and native lazy ghost will not require any breaking change. Using the ORM PRs as reference implementation Add support for ReflectionClass::newLazyGhost of PHP 8.4 orm#11652 and Necessary refactorings for Property hooks orm#11659
Fix job names for the new matrix variable lazy-ghost-objects

GromNaN · 2024-11-23T22:01:39Z

lib/Doctrine/ODM/MongoDB/Proxy/Resolver/LazyGhostProxyClassNameResolver.php

+use function substr;
+
+/** @internal */
+class LazyGhostProxyClassNameResolver implements ClassNameResolver, ProxyClassNameResolver


This class is copied from Doctrine\ORM\Proxy\DefaultProxyClassNameResolver.

GromNaN · 2024-11-23T22:02:08Z

lib/Doctrine/ODM/MongoDB/Proxy/InternalProxy.php

+ * @template T of object
+ * @template-extends Proxy<T>
+ */
+interface InternalProxy extends Proxy


This interface is copied from Doctrine\ORM\Proxy\InternalProxy.

GromNaN · 2024-11-23T22:03:33Z

lib/Doctrine/ODM/MongoDB/Configuration.php

+        $proxyManagerConfiguration = new ProxyManagerConfiguration();
+        $proxyManagerConfiguration->setProxiesTargetDir($this->getProxyDir());
+        $proxyManagerConfiguration->setProxiesNamespace($this->getProxyNamespace());
+
+        switch ($this->getAutoGenerateProxyClasses()) {
+            case self::AUTOGENERATE_FILE_NOT_EXISTS:
+                $proxyManagerConfiguration->setGeneratorStrategy(new FileWriterGeneratorStrategy(
+                    new FileLocator($proxyManagerConfiguration->getProxiesTargetDir()),
+                ));
+
+                break;
+            case self::AUTOGENERATE_EVAL:
+                $proxyManagerConfiguration->setGeneratorStrategy(new EvaluatingGeneratorStrategy());
+
+                break;
+            default:
+                throw new InvalidArgumentException('Invalid proxy generation strategy given - only AUTOGENERATE_FILE_NOT_EXISTS and AUTOGENERATE_EVAL are supported.');
+        }
+
+        return $proxyManagerConfiguration;


Creating ProxyManagerConfiguration on-demand as the class is not required when LaryGhostObjects are used.

GromNaN · 2024-11-23T22:04:48Z

lib/Doctrine/ODM/MongoDB/Proxy/Autoloader.php

+ *
+ * @internal
+ */
+final class Autoloader


This class is copied from Doctrine\ORM\Proxy\Autoloader.

GromNaN · 2024-11-23T22:06:14Z

lib/Doctrine/ODM/MongoDB/Proxy/Factory/LazyGhostProxyFactory.php

+        return static function (InternalProxy $proxy, mixed $identifier) use ($persister, $classMetadata, $factory): void {
+            $original = $persister->load([$classMetadata->identifier => $identifier], $proxy);
+
+            if (! $original && ! $factory->lifecycleEventManager->documentNotFound($proxy, $identifier)) {
+                throw DocumentNotFoundException::documentNotFound($classMetadata->getName(), $identifier);
+            }
+
+            // phpcs:ignore SlevomatCodingStandard.ControlStructures.EarlyExit.EarlyExitNotUsed
+            if ($proxy instanceof NotifyPropertyChanged) {
+                $proxy->addPropertyChangedListener($factory->uow);
+            }
+        };


Initialization code copied from StaticProxyFactory.

GromNaN · 2024-11-23T22:08:19Z

lib/Doctrine/ODM/MongoDB/Proxy/Factory/ProxyFactory.php

     *
     * @template T of object
     */
-    public function getProxy(ClassMetadata $metadata, $identifier): GhostObjectInterface;
+    public function getProxy(ClassMetadata $metadata, $identifier): object;


The interface is modified, but child classes can be more restrictive. This is not a BC break for implementers of the interface, but it can be for users of the interface.

GromNaN · 2024-11-23T22:24:22Z

tests/Doctrine/ODM/MongoDB/Tests/BaseTestCase.php

+    public static function isLazyObject(object $document): bool
+    {
+        return $document instanceof InternalProxy || $document instanceof LazyLoadingInterface;
+    }


Most test changes are related to check interfaces to assert if the class is a lazy proxy object. I wonder if this method should be in UOW.

Later, it will use ReflectionClass::getLazyInitializer() to detect native lazy object.

GromNaN · 2024-11-23T22:32:00Z

lib/Doctrine/ODM/MongoDB/Configuration.php

+
+    /**
+     * Generate proxy classes using Symfony VarExporter's LazyGhostTrait if true.
+     * Otherwise, use ProxyManager's LazyLoadingGhostFactory (deprecated)
+     */
+    public function setUseLazyGhostObject(bool $flag): void
+    {
+        if ($flag === false) {
+            if (! class_exists(ProxyManagerConfiguration::class)) {
+                throw new LogicException('Package "friendsofphp/proxy-manager-lts" is required to disable LazyGhostObject.');
+            }
+
+            trigger_deprecation(
+                'doctrine/mongodb-odm',
+                '2.6',
+                'Using "friendsofphp/proxy-manager-lts" is deprecated. Use "symfony/var-exporter" LazyGhostObjects instead.',
+            );
+        }
+
+        $this->useLazyGhostObject = $flag;
+    }
+
+    public function isLazyGhostObjectEnabled(): bool
+    {
+        return $this->useLazyGhostObject ?? true;
+    }


Should we use the same method names as the ORM: isLazyGhostObjectEnabled and setLazyGhostObjectEnabled?

GromNaN · 2024-11-26T12:09:33Z

lib/Doctrine/ODM/MongoDB/Proxy/Factory/LazyGhostProxyFactory.php

+        switch ($this->autoGenerate) {
+            case Configuration::AUTOGENERATE_FILE_NOT_EXISTS_OR_CHANGED:
+                if (file_exists($fileName) && filemtime($fileName) >= filemtime($class->getReflectionClass()->getFileName())) {
+                    break;
+                }
+            // no break
+            case Configuration::AUTOGENERATE_FILE_NOT_EXISTS:
+                if (file_exists($fileName)) {
+                    break;
+                }
+            // no break
+            case Configuration::AUTOGENERATE_ALWAYS:
+                $this->generateProxyClass($class, $fileName, $proxyClassName);
+                break;
+        }


It would be more intuitive this way:

Suggested change

switch ($this->autoGenerate) {

case Configuration::AUTOGENERATE_FILE_NOT_EXISTS_OR_CHANGED:

if (file_exists($fileName) && filemtime($fileName) >= filemtime($class->getReflectionClass()->getFileName())) {

break;

}

// no break

case Configuration::AUTOGENERATE_FILE_NOT_EXISTS:

if (file_exists($fileName)) {

break;

}

// no break

case Configuration::AUTOGENERATE_ALWAYS:

$this->generateProxyClass($class, $fileName, $proxyClassName);

break;

}

if (

match ($this->autoGenerate) {

Configuration::AUTOGENERATE_FILE_NOT_EXISTS_OR_CHANGED => ! file_exists($fileName) || filemtime($fileName) < filemtime($class->getReflectionClass()->getFileName()),

Configuration::AUTOGENERATE_FILE_NOT_EXISTS => ! file_exists($fileName),

Configuration::AUTOGENERATE_ALWAYS => true,

Configuration::AUTOGENERATE_NEVER => false,

}

) {

$this->generateProxyClass($class, $fileName, $proxyClassName);

}

GromNaN · 2024-11-26T12:14:06Z

lib/Doctrine/ODM/MongoDB/Configuration.php

+
+            trigger_deprecation(
+                'doctrine/mongodb-odm',
+                '2.6',


Suggested change

'2.6',

'2.10',

GromNaN added 3 commits November 23, 2024 15:43

Generate proxy classes using symfony/var-exporter

2e48d18

Leverage UOW::initializeObject in tests

1ef6225

Trigger lifecycleEventManager from lazy ghost object

8742c51

GromNaN mentioned this pull request Nov 23, 2024

Switch proxies back to Doctrine Persistence and LazyGhostTrait #2692

Closed

GromNaN force-pushed the proxy-var-exporter-2 branch 3 times, most recently from 6a946e0 to 9bdb39c Compare November 23, 2024 21:35

Run the full test suite with proxy manager

a81c08c

GromNaN force-pushed the proxy-var-exporter-2 branch from 9bdb39c to a81c08c Compare November 23, 2024 21:36

GromNaN changed the title ~~Switch proxies back to LazyGhostTrait~~ Switch proxies to LazyGhostTrait Nov 23, 2024

Fix assert not lazy object

e69cd31

GromNaN commented Nov 23, 2024

View reviewed changes

GromNaN requested review from alcaeus and malarzm November 23, 2024 22:32

GromNaN commented Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch proxies to `LazyGhostTrait` #2700

Switch proxies to `LazyGhostTrait` #2700

GromNaN commented Nov 23, 2024 •

edited

Loading

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 23, 2024

GromNaN Nov 26, 2024

GromNaN Nov 26, 2024

Switch proxies to LazyGhostTrait #2700

Are you sure you want to change the base?

Switch proxies to LazyGhostTrait #2700

Conversation

GromNaN commented Nov 23, 2024 • edited Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Switch proxies to `LazyGhostTrait` #2700

Switch proxies to `LazyGhostTrait` #2700

GromNaN commented Nov 23, 2024 •

edited

Loading