Skip to content

Commit

Permalink
Generated/Virtual Columns: Insertable / Updateable
Browse files Browse the repository at this point in the history
Refactoring reg. change request
- Using more ORM idomatic style.
- Inverting 'insertable' and 'updateable' reg. optimization of the ClassMetadata cache.
#5728
  • Loading branch information
mehldau committed Nov 9, 2021
1 parent 386a25c commit 19db8e4
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 57 deletions.
8 changes: 6 additions & 2 deletions lib/Doctrine/ORM/Mapping/Builder/FieldBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ public function precision($p)
*/
public function insertable($flag = true)
{
$this->mapping['insertable'] = (bool) $flag;
if (! (bool) $flag) {
$this->mapping['notInsertable'] = true;
}

return $this;
}
Expand All @@ -133,7 +135,9 @@ public function insertable($flag = true)
*/
public function updateable($flag = true)
{
$this->mapping['updateable'] = (bool) $flag;
if (! (bool) $flag) {
$this->mapping['notUpdateable'] = true;
}

return $this;
}
Expand Down
24 changes: 12 additions & 12 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,11 @@ class ClassMetadataInfo implements ClassMetadata
* - <b>nullable</b> (boolean, optional)
* Whether the column is nullable. Defaults to FALSE.
*
* - <b>'insertable'</b> (boolean, optional)
* Whether the column is insertable. Defaults to TRUE.
* - <b>'notInsertable'</b> (boolean, optional)
* Whether the column is not insertable. Optional. Is only set if value is TRUE.
*
* - <b>'updateable'</b> (boolean, optional)
* Whether the column is updateable. Defaults to TRUE.
* - <b>'notUpdateable'</b> (boolean, optional)
* Whether the column is updateable. Optional. Is only set if value is TRUE.
*
* - <b>columnDefinition</b> (string, optional, schema-only)
* The SQL fragment that is used when generating the DDL for the column.
Expand All @@ -439,8 +439,8 @@ class ClassMetadataInfo implements ClassMetadata
* length?: int,
* id?: bool,
* nullable?: bool,
* insertable?: bool,
* updateable?: bool,
* notInsertable?: bool,
* notUpdateable?: bool,
* columnDefinition?: string,
* precision?: int,
* scale?: int,
Expand Down Expand Up @@ -1268,27 +1268,27 @@ public function isNullable($fieldName)
*
* @param string $fieldName The field name.
*
* @return bool TRUE if the field is not null, FALSE otherwise.
* @return bool TRUE if the field is insertable, FALSE otherwise.
*/
public function isInsertable($fieldName)
{
$mapping = $this->getFieldMapping($fieldName);

return $mapping !== false && isset($mapping['insertable']) && $mapping['insertable'];
return ! isset($mapping['notInsertable']);
}

/**
* Checks if the field is updateable.
*
* @param string $fieldName The field name.
*
* @return bool TRUE if the field is not null, FALSE otherwise.
* @return bool TRUE if the field is updateable, FALSE otherwise.
*/
public function isUpdateable($fieldName)
{
$mapping = $this->getFieldMapping($fieldName);

return $mapping !== false && isset($mapping['updateable']) && $mapping['updateable'];
return ! isset($mapping['notUpdateable']);
}

/**
Expand Down Expand Up @@ -1318,8 +1318,8 @@ public function getColumnName($fieldName)
* columnName?: string,
* inherited?: class-string,
* nullable?: bool,
* insertable?: bool,
* updateable?: bool,
* notInsertable?: bool,
* notUpdateable?: bool,
* originalClass?: class-string,
* originalField?: string,
* scale?: int,
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/ORM/Mapping/Column.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ public function __construct(
?int $scale = null,
bool $unique = false,
bool $nullable = false,
?bool $insertable = true,
?bool $updateable = true,
bool $insertable = true,
bool $updateable = true,
array $options = [],
?string $columnDefinition = null
) {
Expand Down
12 changes: 6 additions & 6 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,8 @@ private function joinColumnToArray(Mapping\JoinColumn $joinColumn): array
* unique: bool,
* nullable: bool,
* precision: int,
* insertable?: bool,
* updateble?: bool,
* notInsertable?: bool,
* notUpdateble?: bool,
* options?: mixed[],
* columnName?: string,
* columnDefinition?: string
Expand All @@ -739,12 +739,12 @@ private function columnToArray(string $fieldName, Mapping\Column $column): array
'precision' => $column->precision,
];

if ($column->insertable) {
$mapping['insertable'] = $column->insertable;
if (! $column->insertable) {
$mapping['notInsertable'] = true;
}

if ($column->updateable) {
$mapping['updateable'] = $column->updateable;
if (! $column->updateable) {
$mapping['notUpdateable'] = true;
}

if ($column->options) {
Expand Down
12 changes: 6 additions & 6 deletions lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -800,8 +800,8 @@ private function joinColumnToArray(SimpleXMLElement $joinColumnElement): array
* scale?: int,
* unique?: bool,
* nullable?: bool,
* insertable?: bool,
* updateable?: bool,
* notInsertable?: bool,
* notUpdateable?: bool,
* version?: bool,
* columnDefinition?: string,
* options?: array
Expand Down Expand Up @@ -841,12 +841,12 @@ private function columnToArray(SimpleXMLElement $fieldMapping): array
$mapping['nullable'] = $this->evaluateBoolean($fieldMapping['nullable']);
}

if (isset($fieldMapping['insertable'])) {
$mapping['insertable'] = $this->evaluateBoolean($fieldMapping['insertable']);
if (isset($fieldMapping['insertable']) && ! $this->evaluateBoolean($fieldMapping['insertable'])) {
$mapping['notInsertable'] = true;
}

if (isset($fieldMapping['updateable'])) {
$mapping['updateable'] = $this->evaluateBoolean($fieldMapping['updateable']);
if (isset($fieldMapping['updateable']) && ! $this->evaluateBoolean($fieldMapping['updateable'])) {
$mapping['notUpdateable'] = true;
}

if (isset($fieldMapping['version']) && $fieldMapping['version']) {
Expand Down
12 changes: 6 additions & 6 deletions lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,8 @@ private function joinColumnToArray(array $joinColumnElement): array
* unique?: bool,
* options?: mixed,
* nullable?: mixed,
* insertable?: mixed,
* updateable?: mixed,
* notInsertable?: mixed,
* notUpdateable?: mixed,
* version?: mixed,
* columnDefinition?: mixed
* }
Expand Down Expand Up @@ -852,12 +852,12 @@ private function columnToArray(string $fieldName, ?array $column): array
$mapping['nullable'] = $column['nullable'];
}

if (isset($column['insertable'])) {
$mapping['insertable'] = $column['insertable'];
if (isset($column['insertable']) && ! (bool) $column['insertable']) {
$mapping['notInsertable'] = true;
}

if (isset($column['updateable'])) {
$mapping['updateable'] = $column['updateable'];
if (isset($column['updateable']) && ! (bool) $column['updateable']) {
$mapping['notUpdateable'] = true;
}

if (isset($column['version']) && $column['version']) {
Expand Down
12 changes: 6 additions & 6 deletions lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
Expand Up @@ -597,12 +597,13 @@ public function delete($entity)
* )
* </code>
*
* @param object $entity The entity for which to prepare the data.
* @param object $entity The entity for which to prepare the data.
* @param bool $isInsert Whether the data to be prepared refers to an insert statement.
*
* @return mixed[][] The prepared data.
* @psalm-return array<string, array<array-key, mixed|null>>
*/
protected function prepareUpdateData($entity)
protected function prepareUpdateData($entity, $isInsert = false)
{
$versionField = null;
$result = [];
Expand All @@ -628,16 +629,15 @@ protected function prepareUpdateData($entity)
$fieldMapping = $this->class->fieldMappings[$field];
$columnName = $fieldMapping['columnName'];

$isInsert = ! empty($this->queuedInserts[spl_object_id($entity)]);
if (! $isInsert && ! $this->class->isUpdateable($field)) {
if (! $isInsert && isset($fieldMapping['notUpdateable'])) {
if ($change[0] !== $change[1]) {
throw NonUpdateableField::byName($field);
}

continue;
}

if ($isInsert && ! $this->class->isInsertable($field)) {
if ($isInsert && isset($fieldMapping['notInsertable'])) {
continue;
}

Expand Down Expand Up @@ -708,7 +708,7 @@ protected function prepareUpdateData($entity)
*/
protected function prepareInsertData($entity)
{
return $this->prepareUpdateData($entity);
return $this->prepareUpdateData($entity, true);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/ORM/Tools/Export/Driver/XmlExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,12 @@ public function exportClassMetadata(ClassMetadataInfo $metadata)
$fieldXml->addAttribute('nullable', $field['nullable'] ? 'true' : 'false');
}

if (isset($field['insertable'])) {
$fieldXml->addAttribute('insertable', $field['insertable'] ? 'true' : 'false');
if (isset($field['notInsertable'])) {
$fieldXml->addAttribute('insertable', 'false');
}

if (isset($field['updateable'])) {
$fieldXml->addAttribute('updateable', $field['updateable'] ? 'true' : 'false');
if (isset($field['notUpdateable'])) {
$fieldXml->addAttribute('updateable', 'false');
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@ protected function setUp(): void

public function testInsertSQLUsesInsertableColumns(): void
{
$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Insertable::class));
$method = new ReflectionMethod($persister, 'getInsertSQL');
$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Insertable::class));
$method = new ReflectionMethod($persister, 'getInsertSQL');
$method->setAccessible(true);

self::assertEquals('INSERT INTO insertable_column (insertableContent, insertableContentDefault) VALUES (?, ?)', $method->invoke($persister));
}

public function testUpdateSQLUsesUpdateableColumns(): void
{
$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Updateable::class));
$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Updateable::class));

$entity = new Updateable();
$entity->id = 1;
$entity->nonUpdateableContent = 'non-persistable';
$entity->updateableContent = 'persistable';
$entity->updateableContentDefault = 'persistable';
$entity = new Updateable();
$entity->id = 1;
$entity->nonUpdateableContent = 'non-persistable';
$entity->updateableContent = 'persistable';
$entity->updateableContentDefault = 'persistable';

$this->entityManager->getUnitOfWork()->registerManaged($entity, ['id' => 1], ['nonUpdateableContent' => 'default', 'updateableContent' => 'default', 'updateableContentDefault' => 'default']);

Expand All @@ -63,12 +63,12 @@ public function testExceptionIsThrownWhenTryingToChangeNonUpdateableColumn(): vo
{
$this->expectException(NonUpdateableField::class);

$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Updateable::class));
$persister = new BasicEntityPersister($this->entityManager, $this->entityManager->getClassMetadata(Updateable::class));

$entity = new Updateable();
$entity->nonUpdateableContent = 'non-persistable';
$entity->updateableContent = 'persistable';
$entity->updateableContentDefault = 'persistable';
$entity = new Updateable();
$entity->nonUpdateableContent = 'non-persistable';
$entity->updateableContent = 'persistable';
$entity->updateableContentDefault = 'persistable';

$this->entityManager->getUnitOfWork()->registerManaged($entity, ['id' => 1], ['nonUpdateableContent' => 'default', 'updateableContent' => 'default', 'updateableContentDefault' => 'default']);

Expand Down

0 comments on commit 19db8e4

Please sign in to comment.