8000 Fix the initialization of lazy-ghost proxies with postLoad listeners by stof · Pull Request #11917 · doctrine/orm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

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

Merged
merged 2 commits into from
May 2, 2025

Conversation

stof
Copy link
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
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
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
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
Member
greg0ire commented May 2, 2025

Thanks @stof !

@stof stof deleted the lazy_ghost_postload branch May 3, 2025 19:27
@beberlei
Copy link
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
0