From 9224289c1a1ee86e671c21fa217b73c931bbdf86 Mon Sep 17 00:00:00 2001 From: Sherin Bloemendaal Date: Mon, 23 Jun 2025 12:22:34 +0200 Subject: [PATCH] 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 --- src/UnitOfWork.php | 12 ++ .../ORM/Functional/Ticket/GH12017Test.php | 187 ++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 tests/Tests/ORM/Functional/Ticket/GH12017Test.php diff --git a/src/UnitOfWork.php b/src/UnitOfWork.php index 4c55b728775..9e3a5beac1a 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -637,6 +637,10 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void foreach ($actualData as $propName => $actualValue) { if (! isset($class->associationMappings[$propName])) { + if (isset($class->fieldMappings[$propName]) && $class->fieldMappings[$propName]->notInsertable) { + continue; + } + $changeSet[$propName] = [null, $actualValue]; continue; @@ -664,6 +668,10 @@ public function computeChangeSet(ClassMetadata $class, object $entity): void $orgValue = $originalData[$propName]; + if (isset($class->fieldMappings[$propName]) && $class->fieldMappings[$propName]->notUpdatable) { + continue; + } + if (! empty($class->fieldMappings[$propName]->enumType)) { if (is_array($orgValue)) { foreach ($orgValue as $id => $val) { @@ -1019,6 +1027,10 @@ public function recomputeSingleEntityChangeSet(ClassMetadata $class, object $ent } if ($orgValue !== $actualValue) { + if (isset($class->fieldMappings[$propName]) && $class->fieldMappings[$propName]->notUpdatable) { + continue; + } + $changeSet[$propName] = [$orgValue, $actualValue]; } } diff --git a/tests/Tests/ORM/Functional/Ticket/GH12017Test.php b/tests/Tests/ORM/Functional/Ticket/GH12017Test.php new file mode 100644 index 00000000000..0de0dd5c5f5 --- /dev/null +++ b/tests/Tests/ORM/Functional/Ticket/GH12017Test.php @@ -0,0 +1,187 @@ +setUpEntitySchema([ + GH12017EntityWithGeneratedFields::class, + GH12017EntityWithMixedFlags::class, + ]); + } + + public function testGeneratedFieldsShouldNotBeDetectedAsChanges(): void + { + $entity = new GH12017EntityWithGeneratedFields(); + $entity->name = 'Test Entity'; + + $this->_em->persist($entity); + $this->_em->flush(); + + // Simulate database-generated values being fetched back via property accessors + // This mimics the behavior of assignDefaultVersionAndUpsertableValues() + $metadata = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class); + $metadata->setFieldValue($entity, 'generatedField', new DateTimeImmutable()); + $metadata->setFieldValue($entity, 'computedField', 'computed-value-from-db'); + + $uow = $this->_em->getUnitOfWork(); + $uow->computeChangeSets(); + + self::assertFalse( + $uow->isScheduledForUpdate($entity), + 'Entity with only generated field changes should not be scheduled for update', + ); + + $changeSet = $uow->getEntityChangeSet($entity); + self::assertEmpty($changeSet, 'Changeset should not include generated fields'); + } + + public function testRecomputeSingleEntityChangeSetWithGeneratedFields(): void + { + $entity = new GH12017EntityWithGeneratedFields(); + $entity->name = 'Test Entity'; + + $this->_em->persist($entity); + $this->_em->flush(); + + // Simulate database-generated values being fetched back via property accessors + $metadata = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class); + $metadata->setFieldValue($entity, 'generatedField', new DateTimeImmutable()); + $metadata->setFieldValue($entity, 'computedField', 'computed-value-from-db'); + + $uow = $this->_em->getUnitOfWork(); + $class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class); + $uow->recomputeSingleEntityChangeSet($class, $entity); + + self::assertFalse( + $uow->isScheduledForUpdate($entity), + 'Entity should not be scheduled for update after recomputeSingleEntityChangeSet', + ); + + $changeSet = $uow->getEntityChangeSet($entity); + self::assertEmpty($changeSet, 'Changeset should be empty after recomputeSingleEntityChangeSet'); + } + + public function testNotInsertableFieldsShouldNotBeInChangesetForNewEntities(): void + { + $entity = new GH12017EntityWithGeneratedFields(); + $entity->name = 'Test Entity'; + $entity->generatedField = new DateTimeImmutable(); + $entity->computedField = 'manually-set-value'; + + $this->_em->persist($entity); + + $uow = $this->_em->getUnitOfWork(); + $class = $this->_em->getClassMetadata(GH12017EntityWithGeneratedFields::class); + $uow->computeChangeSet($class, $entity); + + $changeSet = $uow->getEntityChangeSet($entity); + + self::assertArrayHasKey('name', $changeSet, 'Name should be in changeset'); + self::assertArrayNotHasKey('generatedField', $changeSet, 'Generated field should not be in changeset for new entity'); + self::assertArrayNotHasKey('computedField', $changeSet, 'Computed field should not be in changeset for new entity'); + } + + public function testMixedInsertableUpdatableFlags(): void + { + $entity = new GH12017EntityWithMixedFlags(); + $entity->name = 'Test Entity'; + $entity->notInsertableButUpdatable = new DateTimeImmutable('2024-01-01 10:00:00'); + $entity->insertableButNotUpdatable = new DateTimeImmutable('2024-01-01 11:00:00'); + + $this->_em->persist($entity); + + $uow = $this->_em->getUnitOfWork(); + $class = $this->_em->getClassMetadata(GH12017EntityWithMixedFlags::class); + $uow->computeChangeSet($class, $entity); + + $changeSet = $uow->getEntityChangeSet($entity); + + self::assertArrayNotHasKey('notInsertableButUpdatable', $changeSet, 'Field with insertable:false should not be in changeset for new entity'); + self::assertArrayHasKey('insertableButNotUpdatable', $changeSet, 'Field with insertable:true should be in changeset for new entity'); + + $this->_em->flush(); + + $entity->notInsertableButUpdatable = new DateTimeImmutable('2024-02-01 10:00:00'); + $entity->insertableButNotUpdatable = new DateTimeImmutable('2024-02-01 11:00:00'); + + $uow->computeChangeSets(); + $changeSet = $uow->getEntityChangeSet($entity); + + self::assertArrayHasKey('notInsertableButUpdatable', $changeSet, 'Field with updatable:true should be in changeset for managed entity'); + self::assertArrayNotHasKey('insertableButNotUpdatable', $changeSet, 'Field with updatable:false should not be in changeset for managed entity'); + } +} + +#[ORM\Entity] +#[ORM\Table(name: 'gh12017_entity_with_generated_fields')] +class GH12017EntityWithGeneratedFields +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column(type: 'integer')] + public int|null $id = null; + + #[ORM\Column(type: 'string')] + public string|null $name = null; + + #[ORM\Column( + name: 'generated_field', + type: 'datetime_immutable', + nullable: true, + insertable: false, + updatable: false, + generated: 'ALWAYS', + )] + public DateTimeImmutable|null $generatedField = null; + + #[ORM\Column( + name: 'computed_field', + type: 'string', + nullable: true, + insertable: false, + updatable: false, + generated: 'ALWAYS', + )] + public string|null $computedField = null; +} + +#[ORM\Entity] +#[ORM\Table(name: 'gh12017_entity_with_mixed_flags')] +class GH12017EntityWithMixedFlags +{ + #[ORM\Id] + #[ORM\GeneratedValue] + #[ORM\Column(type: 'integer')] + public int|null $id = null; + + #[ORM\Column(type: 'string')] + public string|null $name = null; + + #[ORM\Column( + name: 'not_insertable_but_updatable', + type: 'datetime_immutable', + nullable: true, + insertable: false, + updatable: true, + )] + public DateTimeImmutable|null $notInsertableButUpdatable = null; + + #[ORM\Column( + name: 'insertable_but_not_updatable', + type: 'datetime_immutable', + insertable: true, + updatable: false, + )] + public DateTimeImmutable|null $insertableButNotUpdatable = null; +}