Skip to content

Commit 860a64c

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 860a64c

File tree

2 files changed

+199
-0
lines changed

2 files changed

+199
-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]) && $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]) && $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]) && $class->fieldMappings[$propName]->notUpdatable) {
1028+
continue;
1029+
}
1030+
10191031
$changeSet[$propName] = [$orgValue, $actualValue];
10201032
}
10211033
}

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

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
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+
// Simulate database-generated values being fetched back via property accessors
32+
// This mimics the behavior of assignDefaultVersionAndUpsertableValues()
33+
$metadata = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
34+
$metadata->setFieldValue($entity, 'generatedField', new DateTimeImmutable());
35+
$metadata->setFieldValue($entity, 'computedField', 'computed-value-from-db');
36+
37+
$uow = $this->_em->getUnitOfWork();
38+
$uow->computeChangeSets();
39+
40+
self::assertFalse(
41+
$uow->isScheduledForUpdate($entity),
42+
'Entity with only generated field changes should not be scheduled for update',
43+
);
44+
45+
$changeSet = $uow->getEntityChangeSet($entity);
46+
self::assertEmpty($changeSet, 'Changeset should not include generated fields');
47+
}
48+
49+
public function testRecomputeSingleEntityChangeSetWithGeneratedFields(): void
50+
{
51+
$entity = new GH12017EntityWithGeneratedFields();
52+
$entity->name = 'Test Entity';
53+
54+
$this->_em->persist($entity);
55+
$this->_em->flush();
56+
57+
// Simulate database-generated values being fetched back via property accessors
58+
$metadata = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
59+
$metadata->setFieldValue($entity, 'generatedField', new DateTimeImmutable());
60+
$metadata->setFieldValue($entity, 'computedField', 'computed-value-from-db');
61+
62+
$uow = $this->_em->getUnitOfWork();
63+
$class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
64+
$uow->recomputeSingleEntityChangeSet($class, $entity);
65+
66+
self::assertFalse(
67+
$uow->isScheduledForUpdate($entity),
68+
'Entity should not be scheduled for update after recomputeSingleEntityChangeSet',
69+
);
70+
71+
$changeSet = $uow->getEntityChangeSet($entity);
72+
self::assertEmpty($changeSet, 'Changeset should be empty after recomputeSingleEntityChangeSet');
73+
}
74+
75+
public function testNotInsertableFieldsShouldNotBeInChangesetForNewEntities(): void
76+
{
77+
$entity = new GH12017EntityWithGeneratedFields();
78+
$entity->name = 'Test Entity';
79+
$entity->generatedField = new DateTimeImmutable();
80+
$entity->computedField = 'manually-set-value';
81+
82+
$this->_em->persist($entity);
83+
84+
$uow = $this->_em->getUnitOfWork();
85+
$class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class);
86+
$uow->computeChangeSet($class, $entity);
87+
88+
$changeSet = $uow->getEntityChangeSet($entity);
89+
90+
self::assertArrayHasKey('name', $changeSet, 'Name should be in changeset');
91+
self::assertArrayNotHasKey('generatedField', $changeSet, 'Generated field should not be in changeset for new entity');
92+
self::assertArrayNotHasKey('computedField', $changeSet, 'Computed field should not be in changeset for new entity');
93+
}
94+
95+
public function testMixedInsertableUpdatableFlags(): void
96+
{
97+
$entity = new GH12017EntityWithMixedFlags();
98+
$entity->name = 'Test Entity';
99+
$entity->notInsertableButUpdatable = new DateTimeImmutable('2024-01-01 10:00:00');
100+
$entity->insertableButNotUpdatable = new DateTimeImmutable('2024-01-01 11:00:00');
101+
102+
$this->_em->persist($entity);
103+
104+
$uow = $this->_em->getUnitOfWork();
105+
$class = $this->_em->getClassMetadata(GH12017EntityWithMixedFlags::class);
106+
$uow->computeChangeSet($class, $entity);
107+
108+
$changeSet = $uow->getEntityChangeSet($entity);
109+
110+
self::assertArrayNotHasKey('notInsertableButUpdatable', $changeSet, 'Field with insertable:false should not be in changeset for new entity');
111+
self::assertArrayHasKey('insertableButNotUpdatable', $changeSet, 'Field with insertable:true should be in changeset for new entity');
112+
113+
$this->_em->flush();
114+
115+
$entity->notInsertableButUpdatable = new DateTimeImmutable('2024-02-01 10:00:00');
116+
$entity->insertableButNotUpdatable = new DateTimeImmutable('2024-02-01 11:00:00');
117+
118+
$uow->computeChangeSets();
119+
$changeSet = $uow->getEntityChangeSet($entity);
120+
121+
self::assertArrayHasKey('notInsertableButUpdatable', $changeSet, 'Field with updatable:true should be in changeset for managed entity');
122+
self::assertArrayNotHasKey('insertableButNotUpdatable', $changeSet, 'Field with updatable:false should not be in changeset for managed entity');
123+
}
124+
}
125+
126+
#[ORM\Entity]
127+
#[ORM\Table(name: 'gh12017_entity_with_generated_fields')]
128+
class GH12017EntityWithGeneratedFields
129+
{
130+
#[ORM\Id]
131+
#[ORM\GeneratedValue]
132+
#[ORM\Column(type: 'integer')]
133+
public int|null $id = null;
134+
135+
#[ORM\Column(type: 'string')]
136+
public string|null $name = null;
137+
138+
#[ORM\Column(
139+
name: 'generated_field',
140+
type: 'datetime_immutable',
141+
nullable: true,
142+
insertable: false,
143+
updatable: false,
144+
generated: 'ALWAYS',
145+
)]
146+
public DateTimeImmutable|null $generatedField = null;
147+
148+
#[ORM\Column(
149+
name: 'computed_field',
150+
type: 'string',
151+
nullable: true,
152+
insertable: false,
153+
updatable: false,
154+
generated: 'ALWAYS',
155+
)]
156+
public string|null $computedField = null;
157+
}
158+
159+
#[ORM\Entity]
160+
#[ORM\Table(name: 'gh12017_entity_with_mixed_flags')]
161+
class GH12017EntityWithMixedFlags
162+
{
163+
#[ORM\Id]
164+
#[ORM\GeneratedValue]
165+
#[ORM\Column(type: 'integer')]
166+
public int|null $id = null;
167+
168+
#[ORM\Column(type: 'string')]
169+
public string|null $name = null;
170+
171+
#[ORM\Column(
172+
name: 'not_insertable_but_updatable',
173+
type: 'datetime_immutable',
174+
nullable: true,
175+
insertable: false,
176+
updatable: true,
177+
)]
178+
public DateTimeImmutable|null $notInsertableButUpdatable = null;
179+
180+
#[ORM\Column(
181+
name: 'insertable_but_not_updatable',
182+
type: 'datetime_immutable',
183+
insertable: true,
184+
updatable: false,
185+
)]
186+
public DateTimeImmutable|null $insertableButNotUpdatable = null;
187+
}

0 commit comments

Comments
 (0)