From a154cedaab4be87218144dc8d507c7f9583d1c07 Mon Sep 17 00:00:00 2001 From: Christian Mehldau Date: Wed, 24 Nov 2021 08:50:53 +0100 Subject: [PATCH] Generated/Virtual Columns: Insertable / Updateable Refactoring reg. change request - Fetch not insertable / updateable columns after an insert / update and set them on the entity. #5728 --- .../ORM/Mapping/ClassMetadataInfo.php | 21 ++++++- .../Entity/BasicEntityPersister.php | 59 ++++++++++++------- .../Entity/JoinedSubclassPersister.php | 26 ++++---- .../BasicEntityPersisterUpsertableTest.php | 6 +- 4 files changed, 79 insertions(+), 33 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 868885daa53..1182e14703c 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -661,6 +661,14 @@ class ClassMetadataInfo implements ClassMetadata */ public $changeTrackingPolicy = self::CHANGETRACKING_DEFERRED_IMPLICIT; + /** + * READ-ONLY: A Flag indicating whether or not one or more columns of this class have to be reloaded + * after insert / update operations. + * + * @var bool + */ + public $requiresFetchAfterUpdate = false; + /** * READ-ONLY: A flag for whether or not instances of this class are to be versioned * with optimistic locking. @@ -2688,6 +2696,10 @@ public function mapField(array $mapping) $mapping = $this->validateAndCompleteFieldMapping($mapping); $this->assertFieldNotMapped($mapping['fieldName']); + if (isset($mapping['notInsertable']) || isset($mapping['notUpdateable'])) { + $this->requiresFetchAfterUpdate = true; + } + $this->fieldMappings[$mapping['fieldName']] = $mapping; } @@ -3418,8 +3430,9 @@ public function setSequenceGeneratorDefinition(array $definition) */ public function setVersionMapping(array &$mapping) { - $this->isVersioned = true; - $this->versionField = $mapping['fieldName']; + $this->isVersioned = true; + $this->versionField = $mapping['fieldName']; + $this->requiresFetchAfterUpdate = true; if (! isset($mapping['default'])) { if (in_array($mapping['type'], ['integer', 'bigint', 'smallint'], true)) { @@ -3442,6 +3455,10 @@ public function setVersionMapping(array &$mapping) public function setVersioned($bool) { $this->isVersioned = $bool; + + if ($bool) { + $this->requiresFetchAfterUpdate = true; + } } /** diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 9af968e956c..089bcff39b9 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -32,6 +32,7 @@ use Doctrine\ORM\UnitOfWork; use Doctrine\ORM\Utility\IdentifierFlattener; use Doctrine\ORM\Utility\PersisterHelper; +use LengthException; use function array_combine; use function array_map; @@ -39,6 +40,7 @@ use function array_search; use function array_unique; use function array_values; +use function array_keys; use function assert; use function count; use function get_class; @@ -287,8 +289,8 @@ public function executeInserts() $id = $this->class->getIdentifierValues($entity); } - if ($this->class->isVersioned) { - $this->assignDefaultVersionValue($entity, $id); + if ($this->class->requiresFetchAfterUpdate) { + $this->assignDefaultVersionAndUpsertableValues($entity, $id); } } @@ -300,50 +302,67 @@ public function executeInserts() /** * Retrieves the default version value which was created * by the preceding INSERT statement and assigns it back in to the - * entities version field. + * entities version field if the given entity is versioned. + * Also retrieves values of columns marked as 'non insertable' and / or + * 'not updatetable' and assigns them back to the entities corresponding fields. * * @param object $entity * @param mixed[] $id * * @return void */ - protected function assignDefaultVersionValue($entity, array $id) + protected function assignDefaultVersionAndUpsertableValues($entity, array $id) { - $value = $this->fetchVersionValue($this->class, $id); + $values = $this->fetchVersionAndNotUpsertableValues($this->class, $id); - $this->class->setFieldValue($entity, $this->class->versionField, $value); + foreach ($values as $field => $value) { + $value = Type::getType($this->class->fieldMappings[$field]['type'])->convertToPHPValue($value, $this->platform); + + $this->class->setFieldValue($entity, $field, $value); + } } /** - * Fetches the current version value of a versioned entity. + * Fetches the current version value of a versioned entity and / or the values of fields + * marked as 'not insertable' and / or 'not updateable'. * * @param ClassMetadata $versionedClass * @param mixed[] $id * * @return mixed */ - protected function fetchVersionValue($versionedClass, array $id) + protected function fetchVersionAndNotUpsertableValues($versionedClass, array $id) { - $versionField = $versionedClass->versionField; - $fieldMapping = $versionedClass->fieldMappings[$versionField]; - $tableName = $this->quoteStrategy->getTableName($versionedClass, $this->platform); - $identifier = $this->quoteStrategy->getIdentifierColumnNames($versionedClass, $this->platform); - $columnName = $this->quoteStrategy->getColumnName($versionField, $versionedClass, $this->platform); + $columnNames = []; + foreach ($this->class->fieldMappings as $key => $column) { + if (isset($column['notInsertable']) || isset($column['notUpdateable']) || ($this->class->isVersioned && $key === $versionedClass->versionField)) { + $columnNames[$key] = $this->quoteStrategy->getColumnName($key, $versionedClass, $this->platform); + } + } + + $tableName = $this->quoteStrategy->getTableName($versionedClass, $this->platform); + $identifier = $this->quoteStrategy->getIdentifierColumnNames($versionedClass, $this->platform); // FIXME: Order with composite keys might not be correct - $sql = 'SELECT ' . $columnName - . ' FROM ' . $tableName - . ' WHERE ' . implode(' = ? AND ', $identifier) . ' = ?'; + $sql = 'SELECT ' . implode(', ', $columnNames) + . ' FROM ' . $tableName + . ' WHERE ' . implode(' = ? AND ', $identifier) . ' = ?'; $flatId = $this->identifierFlattener->flattenIdentifier($versionedClass, $id); - $value = $this->conn->fetchOne( + $values = $this->conn->fetchNumeric( $sql, array_values($flatId), $this->extractIdentifierTypes($id, $versionedClass) ); - return Type::getType($fieldMapping['type'])->convertToPHPValue($value, $this->platform); + $values = array_combine(array_keys($columnNames), array_values($values)); + + if (! $values) { + throw new LengthException('Unexpected number of database columns.'); + } + + return $values; } /** @@ -386,10 +405,10 @@ public function update($entity) $this->updateTable($entity, $quotedTableName, $data, $isVersioned); - if ($isVersioned) { + if ($this->class->requiresFetchAfterUpdate) { $id = $this->class->getIdentifierValues($entity); - $this->assignDefaultVersionValue($entity, $id); + $this->assignDefaultVersionAndUpsertableValues($entity, $id); } } diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php index 5c59168eb75..77cec9ec4b6 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php @@ -6,6 +6,7 @@ use Doctrine\Common\Collections\Criteria; use Doctrine\DBAL\LockMode; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Internal\SQLResultCasing; use Doctrine\ORM\Mapping\ClassMetadata; @@ -168,8 +169,8 @@ public function executeInserts() $id = $this->em->getUnitOfWork()->getEntityIdentifier($entity); } - if ($this->class->isVersioned) { - $this->assignDefaultVersionValue($entity, $id); + if ($this->class->requiresFetchAfterUpdate) { + $this->assignDefaultVersionAndUpsertableValues($entity, $id); } // Execute inserts on subtables. @@ -225,10 +226,10 @@ public function update($entity) $this->updateTable($entity, $tableName, $data, $versioned); } - // Make sure the table with the version column is updated even if no columns on that - // table were affected. - if ($isVersioned) { - if (! isset($updateData[$versionedTable])) { + if ($this->class->requiresFetchAfterUpdate) { + // Make sure the table with the version column is updated even if no columns on that + // table were affected. + if ($isVersioned && ! isset($updateData[$versionedTable])) { $tableName = $this->quoteStrategy->getTableName($versionedClass, $this->platform); $this->updateTable($entity, $tableName, [], true); @@ -236,7 +237,7 @@ public function update($entity) $identifiers = $this->em->getUnitOfWork()->getEntityIdentifier($entity); - $this->assignDefaultVersionValue($entity, $identifiers); + $this->assignDefaultVersionAndUpsertableValues($entity, $identifiers); } } @@ -549,10 +550,15 @@ protected function getInsertColumnList() /** * {@inheritdoc} */ - protected function assignDefaultVersionValue($entity, array $id) + protected function assignDefaultVersionAndUpsertableValues($entity, array $id) { - $value = $this->fetchVersionValue($this->getVersionedClassMetadata(), $id); - $this->class->setFieldValue($entity, $this->class->versionField, $value); + $values = $this->fetchVersionAndNotUpsertableValues($this->getVersionedClassMetadata(), $id); + + foreach ($values as $field => $value) { + $value = Type::getType($this->class->fieldMappings[$field]['type'])->convertToPHPValue($value, $this->platform); + + $this->class->setFieldValue($entity, $field, $value); + } } private function getJoinSql(string $baseTableAlias): string diff --git a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterUpsertableTest.php b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterUpsertableTest.php index dd10b7053c6..2245ca01d4b 100644 --- a/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterUpsertableTest.php +++ b/tests/Doctrine/Tests/ORM/Persisters/BasicEntityPersisterUpsertableTest.php @@ -7,6 +7,7 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\Persisters\Entity\BasicEntityPersister; use Doctrine\ORM\Persisters\Exception\NonUpdateableField; +use Doctrine\Tests\Mocks\ArrayResultFactory; use Doctrine\Tests\Models\Upsertable\Insertable; use Doctrine\Tests\Models\Upsertable\Updateable; use Doctrine\Tests\OrmTestCase; @@ -36,7 +37,7 @@ public function testInsertSQLUsesInsertableColumns(): void self::assertEquals('INSERT INTO insertable_column (insertableContent, insertableContentDefault) VALUES (?, ?)', $method->invoke($persister)); } - public function testUpdateSQLUsesUpdateableColumns(): void + public function testUpdateSQLUsesUpdateableColumnsAndNonUpdateableColumnIsReloadedDueToDatabaseModification(): void { $persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Updateable::class)); @@ -52,11 +53,14 @@ public function testUpdateSQLUsesUpdateableColumns(): void $this->entityManager->getUnitOfWork()->propertyChanged($entity, 'updateableContent', 'default', 'persistable'); $this->entityManager->getUnitOfWork()->propertyChanged($entity, 'updateableContentDefault', 'default', 'persistable'); + $this->entityManager->getConnection()->setQueryResult(ArrayResultFactory::createFromArray([['modifed-by-trigger']])); + $persister->update($entity); $executeStatements = $this->entityManager->getConnection()->getExecuteStatements(); self::assertEquals('UPDATE updateable_column SET updateableContent = ?, updateableContentDefault = ? WHERE id = ?', $executeStatements[0]['sql']); + self::assertSame($entity->nonUpdateableContent, 'modifed-by-trigger'); } public function testExceptionIsThrownWhenTryingToChangeNonUpdateableColumn(): void