Skip to content

Fix the initialization of lazy-ghost proxies with postLoad listeners#11917

Merged
greg0ire merged 2 commits intodoctrine:2.20.xfrom
stof:lazy_ghost_postload
May 2, 2025
Merged

Fix the initialization of lazy-ghost proxies with postLoad listeners#11917
greg0ire merged 2 commits intodoctrine:2.20.xfrom
stof:lazy_ghost_postload

Conversation

@stof
Copy link
Copy Markdown
Member

@stof stof commented Apr 22, 2025

This is reopening #11544 (against the newer branch as some release has been done in the meantime), with the fixes for the testsuite that were missing in it.

PostLoad listeners might initialize values for transient properties, so the proxy should not skip initialization when using transient properties.

Closes #11524

The old proxy implementation of doctrine/common was triggered by public
methods rather than access to properties (making public properties
unsupported in entities), so tests could use public instance properties
to track the state of postLoad lifecycle callbacks without triggering
the proxy initialization when reading that state (which then changes the
state of triggering the postLoad callback).
As the new proxy implementation hooks into properties instead, the tests
now use a static method (ensuring it is reset properly before loading
the instance for which we care about the tracking) instead of an
instance property.
@stof stof force-pushed the lazy_ghost_postload branch from b77b4c8 to f368175 Compare April 22, 2025 15:39
@stof
Copy link
Copy Markdown
Member Author

stof commented Apr 22, 2025

The usage of static properties to track the calling state of those callbacks is the same solution than used in #11853 (for the same reason, as the new implementation based on lazy objects in 3.4.x had to solve the same issue in the testsuite)

@stof stof changed the title Lazy ghost postload Fix the initialization of lazy-ghost proxies with postLoad listeners Apr 22, 2025
@nicolas-grekas
Copy link
Copy Markdown
Member

nicolas-grekas commented Apr 23, 2025

Here’s where I’m at about the remaining failure:
The entity ends up in this intermediate buggy state because of a call to __setInitialized in the ORM.
This call disables lazy initialization without actually setting the values of the lazy properties.
One difference between the native implementation and that of var-exporter is precisely about default values: the native one initializes with default values, while var-exporter leaves them uninitialized.
I patched the implementation in lazyGhostTrait, but it causes another error when I run the tests.
Something to dig into. I’ll need to take a break — not sure if I’ll be able to pick it up again before I leave for holidays.
I’m wondering if the ORM shouldn't set the default values of all lazy properties on its own after a call to __setInitialized. That would make some sense. But I don’t yet understand the new error, so we should see if that gives any other ideas also.

Here is the patched method in LazyGhostTrait:
    #[Ignore]
    private function setLazyObjectAsInitialized(bool $initialized): void
    {
        if (!$state = $this->lazyObjectState ?? null) {
            return;
        }

        if (!$initialized) {
            $state->status = LazyObjectState::STATUS_UNINITIALIZED_FULL;

            return;
        }
        
        $initializer = $state->initializer;
        $state->initializer = static fn () => null;

        try {
            $state->initialize($this, '', null);
        } finally {
            $state->initializer = $initializer;
        }
    }

@nicolas-grekas
Copy link
Copy Markdown
Member

nicolas-grekas commented Apr 23, 2025

I figured it out:

The behavior of lazy-ghost-trait is desired on the side of Doctrine's ReflectionProperty since this is how doctrine/persistence can set a property without triggering lazy-initialization.

But on the UnitOfWork side, we do need to set all props to their default value.

Here is a patch that does so:

--- a/src/UnitOfWork.php
+++ b/src/UnitOfWork.php
@@ -49,6 +49,7 @@ use Doctrine\Persistence\PropertyChangedListener;
 use Exception;
 use InvalidArgumentException;
 use RuntimeException;
+use Symfony\Component\VarExporter\Hydrator;
 use UnexpectedValueException;
 
 use function array_chunk;
@@ -2944,6 +2945,7 @@ EXCEPTION
 
             if ($this->isUninitializedObject($entity)) {
                 $entity->__setInitialized(true);
+                Hydrator::hydrate($entity, (array) $class->reflClass->newInstanceWithoutConstructor());
             } else {
                 if (
                     ! isset($hints[Query::HINT_REFRESH])

@stof stof force-pushed the lazy_ghost_postload branch from f368175 to 249692b Compare May 2, 2025 13:12
PostLoad listeners might initialize values for transient properties, so
the proxy should not skip initialization when using transient
properties.

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>
@stof stof force-pushed the lazy_ghost_postload branch from 249692b to a2d510c Compare May 2, 2025 13:29
@greg0ire greg0ire added the Bug label May 2, 2025
@greg0ire greg0ire added this to the 2.20.3 milestone May 2, 2025
@greg0ire greg0ire merged commit 17d28b5 into doctrine:2.20.x May 2, 2025
74 checks passed
@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented May 2, 2025

Thanks @stof !

@stof stof deleted the lazy_ghost_postload branch May 3, 2025 19:27
@beberlei
Copy link
Copy Markdown
Member

beberlei commented May 7, 2025

Thanks @stof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants