From 860a64c6f290baea95cfe0f6e4f1083245cc0cbc 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 949bf20dd47..3a80a3f3bb8 100644 --- a/src/UnitOfWork.php +++ b/src/UnitOfWork.php @@ -636,6 +636,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; @@ -663,6 +667,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) { @@ -1016,6 +1024,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; +}