Ensure proxy-manager is not used when native lazy objects are enabled (#2884)

Fix initialization of proxies when hydrating collection with ProxyManager (bug fix).
Fix unsetting native lazy object config to not enable symfony lazy ghost objects (bug fix)
Add a CI job without optional dependencies to ensure they are not required.
Create test with read-only properties with lazy-objects. Not supported by ProxyManager

Author: GromNaN

.github/workflows/continuous-integration.yml

@@ -92,6 +92,15 @@ jobs:
             dependencies: "highest"
             symfony-version: "stable"
             proxy: "lazy-ghost"
+          # Test removing optional dependencies
+          - topology: "server"
+            php-version: "8.4"
+            mongodb-version: "8.0"
+            driver-version: "stable"
+            dependencies: "highest"
+            symfony-version: "stable"
+            proxy: "native"
+            remove-optional-dependencies: true
           # Test with a sharded cluster
           # Currently disabled due to a bug where MongoDB reports "sharding status unknown"
 #          - topology: "sharded_cluster"
@@ -148,6 +157,13 @@ jobs:
           composer require --no-update symfony/var-dumper:^${{ matrix.symfony-version }}
           composer require --no-update --dev symfony/cache:^${{ matrix.symfony-version }}
 
+      - name: "Remove optional dependencies"
+        if: "${{ matrix.remove-optional-dependencies }}"
+        run: |
+          composer remove --no-update friendsofphp/proxy-manager-lts symfony/var-exporter
+          composer remove --no-update --dev symfony/cache doctrine/orm doctrine/annotations
+          composer remove --no-update --dev doctrine/coding-standard phpstan/phpstan phpstan/phpstan-deprecation-rule phpstan/phpstan-phpunit
+
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v3"
         with:

src/Configuration.php

@@ -708,7 +708,8 @@ public function setUseLazyGhostObject(bool $flag): void
 
     public function isLazyGhostObjectEnabled(): bool
     {
-        return $this->lazyGhostObject;
+        // Always false if native lazy objects are enabled
+        return $this->lazyGhostObject && ! $this->nativeLazyObject;
     }
 
     public function setUseNativeLazyObject(bool $nativeLazyObject): void
@@ -718,7 +719,6 @@ public function setUseNativeLazyObject(bool $nativeLazyObject): void
         }
 
         $this->nativeLazyObject = $nativeLazyObject;
-        $this->lazyGhostObject  = ! $nativeLazyObject || $this->lazyGhostObject;
     }
 
     public function isNativeLazyObjectEnabled(): bool

src/DocumentManager.php

@@ -154,9 +154,23 @@ protected function __construct(?Client $client = null, ?Configuration $config =
             $this->config->getDriverOptions(),
         );
 
-        $this->classNameResolver = $this->config->isLazyGhostObjectEnabled()
-            ? new CachingClassNameResolver(new LazyGhostProxyClassNameResolver())
-            : new CachingClassNameResolver(new ProxyManagerClassNameResolver($this->config));
+        if ($this->config->isNativeLazyObjectEnabled()) {
+            $this->classNameResolver = new class implements ClassNameResolver, ProxyClassNameResolver {
+                public function getRealClass(string $class): string
+                {
+                    return $class;
+                }
+
+                public function resolveClassName(string $className): string
+                {
+                    return $className;
+                }
+            };
+        } elseif ($this->config->isLazyGhostObjectEnabled()) {
+            $this->classNameResolver = new CachingClassNameResolver(new LazyGhostProxyClassNameResolver());
+        } else {
+            $this->classNameResolver = new CachingClassNameResolver(new ProxyManagerClassNameResolver($this->config));
+        }
 
         $metadataFactoryClassName = $this->config->getClassMetadataFactoryName();
         $this->metadataFactory    = new $metadataFactoryClassName();

src/Hydrator/HydratorFactory.php

@@ -451,29 +451,18 @@ public function hydrate(object $document, array $data, array $hints = []): array
             }
         }
 
+        // Skip initialization to not load any object data
         if (PHP_VERSION_ID >= 80400) {
             $metadata->reflClass->markLazyObjectAsInitialized($document);
         }
 
         if ($document instanceof InternalProxy) {
-            // Skip initialization to not load any object data
             $document->__setInitialized(true);
         }
 
         // Support for legacy proxy-manager-lts
-        if ($document instanceof GhostObjectInterface && $document->getProxyInitializer() !== null) {
-            // Inject an empty initialiser to not load any object data
-            $document->setProxyInitializer(static function (
-                GhostObjectInterface $ghostObject,
-                string $method, // we don't care
-                array $parameters, // we don't care
-                &$initializer,
-                array $properties, // we currently do not use this
-            ): bool {
-                $initializer = null;
-
-                return true;
-            });
+        if ($document instanceof GhostObjectInterface) {
+            $document->setProxyInitializer(null);
         }
 
         $data = $this->getHydratorFor($metadata->name)->hydrate($document, $data, $hints);

src/Mapping/PropertyAccessors/ObjectCastPropertyAccessor.php

@@ -52,7 +52,7 @@ public function setValue(object $object, mixed $value): void
             $object->__setInitialized(false);
         } elseif ($object instanceof GhostObjectInterface && ! $object->isProxyInitialized()) {
             $initializer = $object->getProxyInitializer();
-            $object->setProxyInitializer();
+            $object->setProxyInitializer(null);
             $this->reflectionProperty->setValue($object, $value);
             $object->setProxyInitializer($initializer);
         } else {

src/Proxy/Factory/StaticProxyFactory.php

@@ -146,7 +146,7 @@ private function skippedFieldsFqns(ClassMetadata $metadata): array
         $skippedFieldsFqns = [];
 
         foreach ($metadata->getIdentifierFieldNames() as $idField) {
-            $skippedFieldsFqns[] = $this->propertyFqcn($metadata->getReflectionProperty($idField));
+            $skippedFieldsFqns[] = $this->propertyFqcn($metadata->getPropertyAccessor($idField)->getUnderlyingReflector());
         }
 
         foreach ($metadata->getReflectionClass()->getProperties() as $property) {

tests/Documents/Tag.php

@@ -14,7 +14,7 @@ class Tag
     public ?string $id;
 
     #[ODM\Field]
-    public readonly string $name;
+    public string $name;
 
     /** @var Collection<int, BlogPost> */
     #[ODM\ReferenceMany(targetDocument: BlogPost::class, mappedBy: 'tags')]

tests/Tests/BaseTestCase.php

@@ -105,8 +105,13 @@ protected static function getConfiguration(): Configuration
         $config->setPersistentCollectionNamespace('PersistentCollections');
         $config->setDefaultDB(DOCTRINE_MONGODB_DATABASE);
         $config->setMetadataDriverImpl(static::createMetadataDriverImpl());
-        $config->setUseLazyGhostObject((bool) $_ENV['USE_LAZY_GHOST_OBJECT']);
-        $config->setUseNativeLazyObject((bool) $_ENV['USE_NATIVE_LAZY_OBJECT']);
+        if ($_ENV['USE_LAZY_GHOST_OBJECT']) {
+            $config->setUseLazyGhostObject(true);
+        }
+
+        if ($_ENV['USE_NATIVE_LAZY_OBJECT']) {
+            $config->setUseNativeLazyObject(true);
+        }
 
         if ($config->isNativeLazyObjectEnabled()) {
             NativeLazyObjectFactory::enableTracking();

tests/Tests/ConfigurationTest.php

@@ -13,9 +13,11 @@
 use PHPUnit\Framework\Attributes\RequiresPhp;
 use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\TestCase;
+use ProxyManager\Configuration as ProxyManagerConfiguration;
 use stdClass;
 
 use function base64_encode;
+use function class_exists;
 use function str_repeat;
 
 class ConfigurationTest extends TestCase
@@ -38,6 +40,12 @@ public function testUseLazyGhostObject(): void
         self::assertFalse($c->isLazyGhostObjectEnabled());
         $c->setUseLazyGhostObject(true);
         self::assertTrue($c->isLazyGhostObjectEnabled());
+
+        if (! class_exists(ProxyManagerConfiguration::class)) {
+            $this->expectException(LogicException::class);
+            $this->expectExceptionMessage('Package "friendsofphp/proxy-manager-lts" is required to disable LazyGhostObject.');
+        }
+
         $c->setUseLazyGhostObject(false);
         self::assertFalse($c->isLazyGhostObjectEnabled());
     }

tests/Tests/Functional/ReadOnlyPropertiesTest.php

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Functional;
+
+use Doctrine\ODM\MongoDB\Mapping\Annotations\Document;
+use Doctrine\ODM\MongoDB\Mapping\Annotations\Field;
+use Doctrine\ODM\MongoDB\Mapping\Annotations\Id;
+use Doctrine\ODM\MongoDB\Mapping\Annotations\ReferenceOne;
+use Doctrine\ODM\MongoDB\Tests\BaseTestCase;
+
+class ReadOnlyPropertiesTest extends BaseTestCase
+{
+    public function testReadOnlyDocument(): void
+    {
+        $configuration = $this->dm->getConfiguration();
+        if (! $configuration->isNativeLazyObjectEnabled() && ! $configuration->isLazyGhostObjectEnabled()) {
+            $this->markTestSkipped('Read-only properties are not supported by the legacy Proxy Manager. https://github.com/FriendsOfPHP/proxy-manager-lts/issues/26');
+        }
+
+        $document           = new ReadOnlyProperties('Test Name');
+        $document->onlyRead = new ReadOnlyProperties('Nested Name');
+        $this->dm->persist($document);
+        $this->dm->persist($document->onlyRead);
+        $this->dm->flush();
+        $this->dm->clear();
+
+        $document = $this->dm->getRepository(ReadOnlyProperties::class)->find($document->id);
+        $this->assertEquals('Test Name', $document->name);
+        $this->assertEquals('Nested Name', $document->onlyRead->name);
+    }
+}
+
+#[Document]
+class ReadOnlyProperties
+{
+    #[Id]
+    public readonly string $id; // @phpstan-ignore property.uninitializedReadonly (initialized by reflection)
+
+    #[Field]
+    public readonly string $name;
+
+    #[ReferenceOne(targetDocument: self::class)]
+    public ?self $onlyRead;
+
+    public function __construct(string $name)
+    {
+        $this->name = $name;
+    }
+}