-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Conversation
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.
b77b4c8
to
f368175
Compare
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) |
Here’s where I’m at about the remaining failure: 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;
}
} |
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]) |
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>
Thanks @stof ! |
Thanks @stof |
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