Skip to content

Commit

Permalink
Put actual value instead of index inside $originalEntityData. (#9244)
Browse files Browse the repository at this point in the history
This fixes a bug with redundant UPDATE queries, that are executed when some entity uses foreign index of other entity as a primary key. This happens when after inserting related entities with $em->flush() call, you do the second $em->flush() without changing any data inside entities.
Fixes GH8217.

Co-authored-by: ivan <ivan.strygin@managinglife.com>
  • Loading branch information
Feolius and IvanStrygin authored Dec 23, 2021
1 parent f6e1dd4 commit aead77d
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 3 deletions.
8 changes: 5 additions & 3 deletions lib/Doctrine/ORM/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -1166,14 +1166,16 @@ private function addToEntityIdentifiersAndEntityMap(
$identifier = [];

foreach ($class->getIdentifierFieldNames() as $idField) {
$value = $class->getFieldValue($entity, $idField);
$origValue = $class->getFieldValue($entity, $idField);

$value = null;
if (isset($class->associationMappings[$idField])) {
// NOTE: Single Columns as associated identifiers only allowed - this constraint it is enforced.
$value = $this->getSingleIdentifierValue($value);
$value = $this->getSingleIdentifierValue($origValue);
}

$identifier[$idField] = $this->originalEntityData[$oid][$idField] = $value;
$identifier[$idField] = $value ?? $origValue;
$this->originalEntityData[$oid][$idField] = $origValue;
}

$this->entityStates[$oid] = self::STATE_MANAGED;
Expand Down
108 changes: 108 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH8217Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\JoinColumn;
use Doctrine\ORM\Mapping\ManyToOne;
use Doctrine\ORM\Mapping\OneToMany;
use Doctrine\Tests\OrmFunctionalTestCase;

use function count;

final class GH8217Test extends OrmFunctionalTestCase
{
protected function setUp(): void
{
parent::setUp();
$this->setUpEntitySchema(
[
GH8217Collection::class,
GH8217CollectionItem::class,
]
);
}

/**
* @group GH-8217
*/
public function testNoQueriesAfterSecondFlush(): void
{
$collection = new GH8217Collection();
$collection->addItem(new GH8217CollectionItem($collection, 0));
$collection->addItem(new GH8217CollectionItem($collection, 1));
$this->_em->persist($collection);
$this->_em->flush();

$logger = $this->_sqlLoggerStack;
$queriesNumberBeforeSecondFlush = count($logger->queries);
$this->_em->flush();
$queriesNumberAfterSecondFlush = count($logger->queries);
self::assertEquals($queriesNumberBeforeSecondFlush, $queriesNumberAfterSecondFlush);
}
}

/**
* @Entity
*/
class GH8217Collection
{
/**
* @var int
* @Id
* @Column(type="integer")
* @GeneratedValue
*/
public $id;

/**
* @psalm-var Collection<int, GH8217CollectionItem>
* @OneToMany(targetEntity="GH8217CollectionItem", mappedBy="collection",
* cascade={"persist", "remove"}, orphanRemoval=true)
*/
public $items;

public function __construct()
{
$this->items = new ArrayCollection();
}

public function addItem(GH8217CollectionItem $item): void
{
$this->items->add($item);
}
}

/**
* @Entity
*/
class GH8217CollectionItem
{
/**
* @var GH8217Collection
* @Id
* @ManyToOne(targetEntity="GH8217Collection", inversedBy="items")
* @JoinColumn(name="id", referencedColumnName="id")
*/
public $collection;

/**
* @var int
* @Id
* @Column(type="integer", options={"unsigned": true})
*/
public $collectionIndex;

public function __construct(GH8217Collection $collection, int $collectionIndex)
{
$this->collection = $collection;
$this->collectionIndex = $collectionIndex;
}
}

0 comments on commit aead77d

Please sign in to comment.