Skip to content

Commit efe8d49

Browse files
committed
Removing an entity that is contained in a m2m association fails when flush() is called twice
Here is a failing test for #10483. When an entity is removed that is part of a many-to-many collection, that collection will be updated during the `flush()` operation to no longer contain the entity. However, afterwards the collection will be in a dirty state and the internal snapshot still contains the removed entity. That causes the collection to be processed again when `flush()` is called another time. This time, the ORM assumes the entity has been removed from the collection. It will try to build a `DELETE` operation but fails since the entity (from the snapshot) is no longer in the identity map.
1 parent d6c0031 commit efe8d49

File tree

2 files changed

+209
-0
lines changed

2 files changed

+209
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket;
6+
7+
use DateTimeImmutable;
8+
use DateTimeInterface;
9+
use Doctrine\Common\Collections\Criteria;
10+
use Doctrine\ORM\Mapping\Column;
11+
use Doctrine\ORM\Mapping\Entity;
12+
use Doctrine\ORM\Mapping\GeneratedValue;
13+
use Doctrine\ORM\Mapping\Id;
14+
use Doctrine\ORM\Mapping\Table;
15+
use Doctrine\ORM\QueryBuilder;
16+
use Doctrine\Tests\OrmFunctionalTestCase;
17+
18+
use function assert;
19+
20+
class DDC8702Test extends OrmFunctionalTestCase
21+
{
22+
protected function setUp(): void
23+
{
24+
parent::setUp();
25+
26+
$this->createSchemaForModels(DDC8702Article::class);
27+
}
28+
29+
public function testIssue(): void
30+
{
31+
//setup
32+
$article1 = new DDC8702Article();
33+
$article2 = new DDC8702Article();
34+
$article3 = new DDC8702Article();
35+
$article4 = new DDC8702Article();
36+
37+
$this->_em->persist($article1);
38+
$this->_em->persist($article2);
39+
$this->_em->persist($article3);
40+
$this->_em->persist($article4);
41+
42+
$this->_em->flush();
43+
$this->_em->clear();
44+
//end setup
45+
46+
$qb = $this->_em->getRepository(DDC8702Article::class)->createQueryBuilder('a');
47+
48+
assert($qb instanceof QueryBuilder);
49+
50+
$qb->addCriteria(Criteria::create()->andWhere(Criteria::expr()->lte('created', new DateTimeImmutable('tomorrow'))));
51+
$qb->addCriteria(Criteria::create()->andWhere(Criteria::expr()->lte('created', new DateTimeImmutable('-2 hours'))));
52+
$qb->addCriteria(Criteria::create()->andWhere(Criteria::expr()->gte('created', new DateTimeImmutable('yesterday'))));
53+
54+
$result = $qb->getQuery()->toIterable();
55+
self::assertCount(4, $result, 'invalid number of articles');
56+
57+
$this->_em->clear();
58+
}
59+
}
60+
61+
/**
62+
* @Entity
63+
* @Table(name="article")
64+
*/
65+
class DDC8702Article
66+
{
67+
/**
68+
* @var int
69+
* @Id
70+
* @Column(type="integer")
71+
* @GeneratedValue(strategy="AUTO")
72+
*/
73+
private $id;
74+
75+
/**
76+
* @var DateTimeInterface
77+
* @Column(type="datetime_immutable")
78+
*/
79+
private $created;
80+
81+
public function __construct()
82+
{
83+
$this->created = new DateTimeImmutable('now');
84+
}
85+
86+
public function getId(): int
87+
{
88+
return $this->id;
89+
}
90+
91+
public function getCreated(): DateTimeImmutable
92+
{
93+
return $this->created;
94+
}
95+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket;
6+
7+
use Doctrine\Common\Collections\ArrayCollection;
8+
use Doctrine\Common\Collections\Collection;
9+
use Doctrine\ORM\Mapping as ORM;
10+
use Doctrine\Tests\OrmFunctionalTestCase;
11+
12+
class GH10483Test extends OrmFunctionalTestCase
13+
{
14+
protected function setUp(): void
15+
{
16+
parent::setUp();
17+
18+
$this->createSchemaForModels(
19+
GH10483Role::class,
20+
GH10483User::class
21+
);
22+
}
23+
24+
public function testFlushChangedUserAfterRoleHasBeenDeleted(): void
25+
{
26+
$em = $this->_em;
27+
28+
$role = new GH10483Role();
29+
$role->name = 'test';
30+
$em->persist($role);
31+
32+
$user = new GH10483User();
33+
$user->name = 'test';
34+
$user->roles->add($role);
35+
$em->persist($user);
36+
37+
$em->flush();
38+
39+
self::assertFalse($user->roles->isDirty());
40+
41+
$em->remove($role);
42+
$em->flush();
43+
44+
// UnitOfWork::computeAssociationChanges(), lines 968 ff. will remove the removed entity from the collection:
45+
self::assertEmpty($user->roles);
46+
47+
// The UoW left the collection in a dirty state, is that correct?
48+
self::assertTrue($user->roles->isDirty()); // <-- might need to assert "false" (?)
49+
50+
// The collection's snapshot still contains the removed $role entity, is that correct?
51+
self::assertSame([$role], $user->roles->getSnapshot()); // <-- might need to assert snapshot being empty (?)
52+
53+
// Since the collection is dirty and/or has a snapshot that differs from the state,
54+
// this flush will try to remove the $role from the collection, and fails when looking for
55+
// it in the identity map
56+
$em->flush();
57+
}
58+
}
59+
60+
/**
61+
* @ORM\Entity
62+
*/
63+
class GH10483Role
64+
{
65+
/**
66+
* @ORM\Id
67+
* @ORM\GeneratedValue(strategy="AUTO")
68+
* @ORM\Column(type="integer")
69+
*
70+
* @var int
71+
*/
72+
public $id;
73+
74+
/**
75+
* @ORM\Column
76+
*
77+
* @var string
78+
*/
79+
public $name;
80+
}
81+
82+
/**
83+
* @ORM\Entity
84+
*/
85+
class GH10483User
86+
{
87+
/**
88+
* @ORM\Id
89+
* @ORM\GeneratedValue(strategy="AUTO")
90+
* @ORM\Column(type="integer")
91+
*
92+
* @var int
93+
*/
94+
public $id;
95+
96+
/**
97+
* @ORM\Column
98+
*
99+
* @var string
100+
*/
101+
public $name;
102+
103+
/**
104+
* @ORM\ManyToMany(targetEntity="GH10483Role")
105+
*
106+
* @var Collection
107+
*/
108+
public $roles;
109+
110+
public function __construct()
111+
{
112+
$this->roles = new ArrayCollection();
113+
}
114+
}

0 commit comments

Comments
 (0)