Skip to content
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

Fix initializing lazy objects and get rid of "Typed property must not be accessed before initialization" errors #10385

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php
Original file line number Diff line number Diff line change
@@ -165,6 +165,10 @@ protected function hydrateRowData(array $row, array &$result)
}
}

if (isset($this->_hints[Query::HINT_REFRESH_ENTITY])) {
$this->registerManaged($this->class, $this->_hints[Query::HINT_REFRESH_ENTITY], $data);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines were removed in #10187 as pointed out in #10336 (comment)

$uow = $this->_em->getUnitOfWork();
$entity = $uow->createEntity($entityName, $data, $this->_hints);

22 changes: 15 additions & 7 deletions lib/Doctrine/ORM/Proxy/ProxyFactory.php
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@
use Doctrine\ORM\Utility\IdentifierFlattener;
use Doctrine\Persistence\Mapping\ClassMetadata;
use Doctrine\Persistence\Proxy;
use ReflectionProperty;
use Symfony\Component\VarExporter\ProxyHelper;
use Symfony\Component\VarExporter\VarExporter;

@@ -313,17 +314,24 @@ private function generateSkippedProperties(ClassMetadata $class): string
{
$skippedProperties = ['__isCloning' => true];
$identifiers = array_flip($class->getIdentifierFieldNames());
$filter = ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PRIVATE;
$reflector = $class->getReflectionClass();

foreach ($class->getReflectionClass()->getProperties() as $property) {
$name = $property->getName();
while ($reflector) {
foreach ($reflector->getProperties($filter) as $property) {
$name = $property->getName();

if ($property->isStatic() || (($class->hasField($name) || $class->hasAssociation($name)) && ! isset($identifiers[$name]))) {
continue;
}
if ($property->isStatic() || (($class->hasField($name) || $class->hasAssociation($name)) && ! isset($identifiers[$name]))) {
continue;
}

$prefix = $property->isPrivate() ? "\0" . $property->getDeclaringClass()->getName() . "\0" : ($property->isProtected() ? "\0*\0" : '');
$prefix = $property->isPrivate() ? "\0" . $property->getDeclaringClass()->getName() . "\0" : ($property->isProtected() ? "\0*\0" : '');

$skippedProperties[$prefix . $name] = true;
}

$skippedProperties[$prefix . $name] = true;
$filter = ReflectionProperty::IS_PRIVATE;
$reflector = $reflector->getParentClass();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to do the same as before, but for does it also for ancestors in the class hierarchy, retrieving only private properties for those (I guess public and protected are already retrieved at the first iteration because they are inherited).

}

uksort($skippedProperties, 'strnatcmp');
27 changes: 27 additions & 0 deletions tests/Doctrine/Tests/Models/GH10336/GH10336Entity.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\GH10336;

use Doctrine\ORM\Mapping as ORM;

/**
* @ORM\Entity
* @ORM\Table(name="gh10336_entities")
*/
class GH10336Entity
{
/**
* @ORM\Id
* @ORM\Column(type="integer")
* @ORM\GeneratedValue
*/
public ?int $id = null;

/**
* @ORM\ManyToOne(targetEntity="GH10336Relation")
* @ORM\JoinColumn(name="relation_id", referencedColumnName="id", nullable=true)
*/
public ?GH10336Relation $relation = null;
}
26 changes: 26 additions & 0 deletions tests/Doctrine/Tests/Models/GH10336/GH10336Relation.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\Models\GH10336;

use Doctrine\ORM\Mapping as ORM;

/**
* @ORM\Entity
* @ORM\Table(name="gh10336_relations")
*/
class GH10336Relation
{
/**
* @ORM\Id
* @ORM\Column(type="integer")
* @ORM\GeneratedValue
*/
public ?int $id = null;

/**
* @ORM\Column(type="string")
*/
public string $value;
}
44 changes: 44 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH10336Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\Tests\Models\GH10336\GH10336Entity;
use Doctrine\Tests\Models\GH10336\GH10336Relation;
use Doctrine\Tests\OrmFunctionalTestCase;

/**
* @requires PHP 7.4
*/
final class GH10336Test extends OrmFunctionalTestCase
{
public function setUp(): void
{
parent::setUp();

$this->createSchemaForModels(
GH10336Entity::class,
GH10336Relation::class
);
}

public function testCanAccessRelationPropertyAfterClear(): void
{
$relation = new GH10336Relation();
$relation->value = 'foo';
$entity = new GH10336Entity();
$entity->relation = $relation;

$this->_em->persist($entity);
$this->_em->persist($relation);
$this->_em->flush();
$this->_em->clear();

$entity = $this->_em->find(GH10336Entity::class, 1);

$this->_em->clear();

$this->assertSame('foo', $entity->relation->value);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this test 100% covers this change, the while loop only ever does one iteration. Yet when I use git restore --source=origin/2.14.x lib/Doctrine/ORM/Proxy/ProxyFactory.php, a preexisting test fails Doctrine\Tests\ORM\Functional\MappedSuperclassTest::testCRUD, which means the scenario where there is more than 1 iteration is already covered 👍

It gets broken by the changes in SimpleObjectHydrator, and fixed by the changes in ProxyFactory

}