Skip to content

Commit 3a0fb22

Browse files
SherinBloemendaalSherinBloemendaal
committed
Fix false positive changes for generated columns
UnitOfWork was incorrectly detecting changes for database-generated columns with insertable: false and updatable: false, causing entities to be scheduled for update when only generated values changed. This adds checks to skip notInsertable fields for NEW entities and notUpdatable fields for MANAGED entities during change detection, aligning UnitOfWork behavior with BasicEntityPersister. Fixes #12017
1 parent 92e2f6d commit 3a0fb22

File tree

2 files changed

+188
-0
lines changed

2 files changed

+188
-0
lines changed

‎src/UnitOfWork.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,10 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void
636636

637637
foreach ($actualData as $propName => $actualValue) {
638638
if (! isset($class->associationMappings[$propName])) {
639+
if (isset($class->fieldMappings[$propName]->notInsertable)) {
640+
continue;
641+
}
642+
639643
$changeSet[$propName] = [null, $actualValue];
640644

641645
continue;
@@ -663,6 +667,10 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void
663667

664668
$orgValue = $originalData[$propName];
665669

670+
if (isset($class->fieldMappings[$propName]->notUpdatable)) {
671+
continue;
672+
}
673+
666674
if (! empty($class->fieldMappings[$propName]->enumType)) {
667675
if (is_array($orgValue)) {
668676
foreach ($orgValue as $id => $val) {
@@ -1016,6 +1024,10 @@ public function recomputeSingleEntityChangeSet(ClassMetadata $class, object $ent
10161024
}
10171025

10181026
if ($orgValue !== $actualValue) {
1027+
if (isset($class->fieldMappings[$propName]->notUpdatable)) {
1028+
continue;
1029+
}
1030+
10191031
$changeSet[$propName] = [$orgValue, $actualValue];
10201032
}
10211033
}

‎tests/Tests/ORM/Functional/Ticket/GH12017Test.php‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket;
6+
7+
use DateTimeImmutable;
8+
use Doctrine\ORM\Mapping as ORM;
9+
use Doctrine\Tests\OrmFunctionalTestCase;
10+
11+
class GH12017Test extends OrmFunctionalTestCase
12+
{
13+
protected function setUp(): void
14+
{
15+
parent::setUp();
16+
17+
$this->setUpEntitySchema([
18+
GH12017EntityWithGeneratedFields::class,
19+
GH12017EntityWithMixedFlags::class,
20+
]);
21+
}
22+
23+
public function testGeneratedFieldsShouldNotBeDetectedAsChanges(): void
24+
{
25+
$entity = new GH12017EntityWithGeneratedFields();
26+
$entity->name = 'Test Entity';
27+
28+
$this->_em->persist($entity);
29+
$this->_em->flush();
30+
31+
$uow = $this->_em->getUnitOfWork();
32+
$uow->computeChangeSets();
33+
34+
self::assertFalse(
35+
$uow->isScheduledForUpdate($entity),
36+
'Entity with only generated field changes should not be scheduled for update',
37+
);
38+
39+
$changeSet = $uow->getEntityChangeSet($entity);
40+
self::assertEmpty($changeSet, 'Changeset should not include generated fields');
41+
}
42+
43+
public function testRecomputeSingleEntityChangeSetWithGeneratedFields(): void
44+
{
45+
$entity = new GH12017EntityWithGeneratedFields();
46+
$entity->name = 'Test Entity';
47+
48+
$this->_em->persist($entity);
49+
$this->_em->flush();
50+
51+
$uow = $this->_em->getUnitOfWork();
52+
$class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
53+
$uow->recomputeSingleEntityChangeSet($class, $entity);
54+
55+
self::assertFalse(
56+
$uow->isScheduledForUpdate($entity),
57+
'Entity should not be scheduled for update after recomputeSingleEntityChangeSet',
58+
);
59+
60+
$changeSet = $uow->getEntityChangeSet($entity);
61+
self::assertEmpty($changeSet, 'Changeset should be empty after recomputeSingleEntityChangeSet');
62+
}
63+
64+
public function testNotInsertableFieldsShouldNotBeInChangesetForNewEntities(): void
65+
{
66+
$entity = new GH12017EntityWithGeneratedFields();
67+
$entity->name = 'Test Entity';
68+
$entity->generatedField = new DateTimeImmutable();
69+
$entity->computedField = 'manually-set-value';
70+
71+
$this->_em->persist($entity);
72+
73+
$uow = $this->_em->getUnitOfWork();
74+
$class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
75+
$uow->computeChangeSet($class, $entity);
76+
77+
$changeSet = $uow->getEntityChangeSet($entity);
78+
79+
self::assertArrayHasKey('name', $changeSet, 'Name should be in changeset');
80+
self::assertArrayNotHasKey('generatedField', $changeSet, 'Generated field should not be in changeset for new entity');
81+
self::assertArrayNotHasKey('computedField', $changeSet, 'Computed field should not be in changeset for new entity');
82+
}
83+
84+
public function testMixedInsertableUpdatableFlags(): void
85+
{
86+
$entity = new GH12017EntityWithMixedFlags();
87+
$entity->name = 'Test Entity';
88+
$entity->notInsertableButUpdatable = new DateTimeImmutable('2024-01-01 10:00:00');
89+
$entity->insertableButNotUpdatable = new DateTimeImmutable('2024-01-01 11:00:00');
90+
91+
$this->_em->persist($entity);
92+
93+
$uow = $this->_em->getUnitOfWork();
94+
$class = $this->_em->getClassMetadata(GH12017EntityWithMixedFlags::class);
95+
$uow->computeChangeSet($class, $entity);
96+
97+
$changeSet = $uow->getEntityChangeSet($entity);
98+
99+
self::assertArrayNotHasKey('notInsertableButUpdatable', $changeSet, 'Field with insertable:false should not be in changeset for new entity');
100+
self::assertArrayHasKey('insertableButNotUpdatable', $changeSet, 'Field with insertable:true should be in changeset for new entity');
101+
102+
$this->_em->flush();
103+
104+
$entity->notInsertableButUpdatable = new DateTimeImmutable('2024-02-01 10:00:00');
105+
$entity->insertableButNotUpdatable = new DateTimeImmutable('2024-02-01 11:00:00');
106+
107+
$uow->computeChangeSets();
108+
$changeSet = $uow->getEntityChangeSet($entity);
109+
110+
self::assertArrayHasKey('notInsertableButUpdatable', $changeSet, 'Field with updatable:true should be in changeset for managed entity');
111+
self::assertArrayNotHasKey('insertableButNotUpdatable', $changeSet, 'Field with updatable:false should not be in changeset for managed entity');
112+
}
113+
}
114+
115+
#[ORM\Entity]
116+
#[ORM\Table(name: 'gh12017_entity_with_generated_fields')]
117+
class GH12017EntityWithGeneratedFields
118+
{
119+
#[ORM\Id]
120+
#[ORM\GeneratedValue]
121+
#[ORM\Column(type: 'integer')]
122+
public int|null $id = null;
123+
124+
#[ORM\Column(type: 'string')]
125+
public string|null $name = null;
126+
127+
#[ORM\Column(
128+
name: 'generated_field',
129+
type: 'datetime_immutable',
130+
insertable: false,
131+
updatable: false,
132+
columnDefinition: 'TIMESTAMP DEFAULT CURRENT_TIMESTAMP',
133+
generated: 'ALWAYS',
134+
)]
135+
public DateTimeImmutable|null $generatedField = null;
136+
137+
#[ORM\Column(
138+
name: 'computed_field',
139+
type: 'string',
140+
insertable: false,
141+
updatable: false,
142+
columnDefinition: "VARCHAR(255) GENERATED ALWAYS AS (CONCAT('computed-', name)) STORED",
143+
generated: 'ALWAYS',
144+
)]
145+
public string|null $computedField = null;
146+
}
147+
148+
#[ORM\Entity]
149+
#[ORM\Table(name: 'gh12017_entity_with_mixed_flags')]
150+
class GH12017EntityWithMixedFlags
151+
{
152+
#[ORM\Id]
153+
#[ORM\GeneratedValue]
154+
#[ORM\Column(type: 'integer')]
155+
public int|null $id = null;
156+
157+
#[ORM\Column(type: 'string')]
158+
public string|null $name = null;
159+
160+
#[ORM\Column(
161+
name: 'not_insertable_but_updatable',
162+
type: 'datetime_immutable',
163+
nullable: true,
164+
insertable: false,
165+
updatable: true,
166+
)]
167+
public DateTimeImmutable|null $notInsertableButUpdatable = null;
168+
169+
#[ORM\Column(
170+
name: 'insertable_but_not_updatable',
171+
type: 'datetime_immutable',
172+
insertable: true,
173+
updatable: false,
174+
)]
175+
public DateTimeImmutable|null $insertableButNotUpdatable = null;
176+
}

0 commit comments

Comments
 (0)