Unable to update belongs-to relationship when loaded through query from the parent (incomplete hydration)

[matt-stuart]

Bug Report

Q	A
BC Break	no
Version	2.6.3

Summary

Symptom: Unable to update the parent of an entity when that entity was loaded via a query from the parent including the children through a join and inclusion in the select list.

Deeper: Hydration of child entities in the UnitOfWork::$originalEntityData during a query that includes the children as part of it does not fully hydrate parent relationships. Specifically, it doesn't hydrate the parent relationship that the child was loaded through.

This causes issues where computing the change set of an entity who was loaded in this way and their parent was changed causes this change to be missed.

From the UnitOfWork.php lines 2686:

            if (isset($hints['fetchAlias']) && isset($hints['fetched'][$hints['fetchAlias']][$field])) {
                continue;
            }

Appears to skip hydrating the parent relationship because it's in the hint list

Current behavior

The parent property cannot be changed on the child entities loaded this way as:

The UnitOfWork::$originalEntityData contains a property parent_id = # as the relationship is not hydrated to the proper field parent. When the parent property is changed using the entity and persisted no change will be detected as the compute change set won't find the property in the copy coming from $originalEntityData

How to reproduce

entity Parent {
    Id
    has-many children of Child
}
entity Child {
    Id
    belongs-to parent of Parent
}

// Assume ::getRepository() is equiv to getEntityManager()->getRepository(Parent)
$parentEntity = Parent::getRepository()->createQueryBuilder('p')
            ->leftJoin('p.children', 'c', 'WITH')
            ->addSelect(['c'])
            ->where('p.id = :parent_id')
            ->setParameter('parent_id', X)
            ->getQuery()->getResult();

// Assume ::load is a pass through to ->find(ID)
$targetParent = Parent::load(Y);
foreach ($parentEntity->children as $child) {
    $child->parent = $targetParent;
    $child->persist();
}
flush()

Expected behavior

The parent of the child entity should have changed from the original X to the target Y

[lcobucci]

@matt-stuart thanks for submitting the bug report. I'm afraid we need more information than that, for example: which side is the owner of the association? Are you using a different tracking policy (other than the default one)? Have you tried fixing the Parent#children property of both parent objects in combination with updating the Children#parent (as in removing from collection A and appending to collection B)?

To clarify everything, could you please send us PR with a failing functional test that fully reproduces the problem?

[matt-stuart]

Will do

Update: Tried removing it from the collection, adding it to the new collection, and adjusting the parent pointer. Same behaviour. Adding test.

[matt-stuart]

Thought i'd give a quick code sample that's literally what I'm using:

<?php
namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
/**
 * @ORM\MappedSuperclass
 */
abstract class EntityAbstract
{
    /**
     * Magic method to retrieve properties easily
     *
     * @param string $property The property name
     * @return mixed
     * @throws @see \Petrosight\Exception
     */
    public function __get($property) ...

    /**
     * Magic setter to assist with setting properties for the entity
     *
     * @param string $property The property to set
     * @param mixed $value The value to set to
     */
    public function __set($property, $value) ...
}

/**
 * @ORM\Table(name="daily_cost", indexes={
 *     @ORM\Index(columns={"active"})
 * })
 * @ORM\Entity
 * @ORM\ChangeTrackingPolicy("DEFERRED_EXPLICIT")
 *
 *
 * @property $id
 * @property $items
 */
class DailyCost extends EntityAbstract
{
    /**
     * @var integer $id
     * @ORM\Column(name="id", type="integer", nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    protected $id;

    /**
     * @var \Doctrine\Common\Collections\ArrayCollection
     * @ORM\OneToMany(targetEntity="DailiyCostItem", mappedBy="dailyCost", cascade={"remove"}, orphanRemoval=true, indexBy="id")
     */
    protected $items;
    
    /**
     * @var bool
     * @ORM\Column(type="boolean", nullable=false)
     */
    protected $active = TRUE;

    /**
     * Initialize a few values
     */
    public function __construct()
    {
        parent::__construct();
        $this->items = new \Doctrine\Common\Collections\ArrayCollection();
    }
}

/**
 * @ORM\Table(name="daily_cost_item", indexes={@ORM\Index(columns={"_discr"})})
 * @ORM\Entity
 * @ORM\ChangeTrackingPolicy("DEFERRED_EXPLICIT")
 * @ORM\InheritanceType("SINGLE_TABLE")
 * @ORM\DiscriminatorColumn(name="_discr", type="string", length=20)
 * @ORM\DiscriminatorMap({
 *  "vendor" = "DailyCostItem_Vendor",
 *  "purchase-order" = "DailyCostItem_PurchaseOrder",
 *  "rental" = "DailyCostItem_Rental",
 *  "mud" = "DailyCostItem_Mud",
 * })
 *
 * @property $id
 * @property $dailyCost
 */
abstract class DailyCostItem extends EntityAbstract
{
    /**
     * @var integer $id
     * @ORM\Column(name="id", type="integer",nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    protected $id;

    /**
     * @var \App\Entity\DailyCost
     * @ORM\ManyToOne(targetEntity="DailyCost", inversedBy="items")
     * @ORM\JoinColumns({
     *      @ORM\JoinColumn(name="daily_cost_id", referencedColumnName="id")
     * })
     */
    protected $dailyCost;

    public function __construct()
    {
        parent::__construct();
        $this->invoices = new ArrayCollection();
    }
}

/**
 * @ORM\Entity
 *
 * @property $vendor
 */
class DailyCostItem_Rental extends DailyCostItem
{
    /**
     * @var \App\Entity\Vendor
     * @ORM\ManyToOne(targetEntity="\App\Entity\Vendor")
     * @ORM\JoinColumns({
     *      @ORM\JoinColumn(name="vendor_id", referencedColumnName="id")
     * })
     */
    protected $vendor;
}


$dailyCost = $em->getRepository(\App\Entity\DailyCost::class)->createQueryBuilder('dc')
            ->leftJoin('dc.items', 'dci', 'WITH')
            ->addSelect(['dci'])
            ->where('dc.id = :daily_cost')
            ->setParameter('daily_cost', 1)
            ->getQuery()->getResult();

$targetDailyCost = $em->find('\App\Entity\DailyCost', 2);

// Original
foreach ($dailyCost->items as $item) {
    $item->dailyCost = $targetDailyCost;
    $em->persist($item);
}
$em->flush();
// The `daily_cost_id` is not changed for any of the items

// 2nd Version
foreach ($dailyCost->items as $item) {
    $originalDailyCost = $item->dailyCost;
    $dailyCost->items->removeElement($item);
    
    $item->dailyCost = $targetDailyCost;
    $targetDailyCost->items->add($item);

    $em->persist($item);
}
$em->flush();
// The `daily_cost_id` is not changed for any of the items

[lcobucci]

@matt-stuart since you're changing $dailyCost and $targetDailyCost and use the DEFERRED_EXPLICIT you must persist them as well (it can be after the loop).

[matt-stuart]

As a note, through an EventSubscriber ultimately all of the entities get persisted. It's a version tracking thing that updates the treeVersion value on all of the parent entities to ensure tracking tree changes. Either through my ES or through explicit persists in the code above the change to the parent isn't picked up.

When loading these entities through just the relationships instead of through a combined query like above:

$dailyCost = $em->getRepository(\App\Entity\DailyCost::class)->createQueryBuilder('dc')
           ->where('dc.id = :daily_cost')
           ->setParameter('daily_cost', 1)
           ->getQuery()->getResult();

the $items get properly updated as they're hydrated fully and the UOW#computeChangeSet can track the change as the internal original record is accurate, this only occurs when hydrating from a join query.

So, that brings the question of who's responsibility of ensuring the internal tracking array is built? UOW has logic that does build out the association properties but are skipped in this case due to the hints provided by the ObjectHydrator class. The ObjectHydrator ALSO has logic that set the UOW#originalEntityData properties when it comes to associations in some cases but it appears not all (see ObjectHydrator#hydrateRowData@~432).
So before I start writing tests, who's responsibility should this be to ensure that the association properties are properly represented in the internal tracking?

[lcobucci]

@matt-stuart please send us the PR with the failing test. It will be much simpler to find the problem

[matt-stuart]

I'll send one along I'm just wondering what component should be the one considered to be failing. Is it the ObjectHydrator that should be responsible or the UnitOfWork. Both appear to make changes to the UOW::$originalEntityData in some way or another. I'm reviewing the code to determine, what I feel, is the most logical place for this hydration to take place.