Skip to content

Commit

Permalink
Extract discriminator column mapping into its own DTO (#10609)
Browse files Browse the repository at this point in the history
  • Loading branch information
greg0ire authored Apr 2, 2023
1 parent e4a7403 commit e22592f
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 30 deletions.
43 changes: 21 additions & 22 deletions lib/Doctrine/ORM/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,6 @@
* type: int,
* unique?: bool,
* }
* @psalm-type DiscriminatorColumnMapping = array{
* name: string,
* fieldName: string,
* type: string,
* length?: int,
* columnDefinition?: string|null,
* enumType?: class-string<BackedEnum>|null,
* options?: array<string, mixed>,
* }
* @psalm-type EmbeddedClassMapping = array{
* class: class-string,
* columnPrefix: string|null,
Expand Down Expand Up @@ -469,11 +460,8 @@ class ClassMetadata implements PersistenceClassMetadata, Stringable
/**
* READ-ONLY: The definition of the discriminator column used in JOINED and SINGLE_TABLE
* inheritance mappings.
*
* @var array<string, mixed>
* @psalm-var DiscriminatorColumnMapping|null
*/
public array|null $discriminatorColumn = null;
public DiscriminatorColumnMapping|null $discriminatorColumn = null;

/**
* READ-ONLY: The primary table definition. The definition is an array with the
Expand Down Expand Up @@ -2454,13 +2442,28 @@ public function addEntityListener(string $eventName, string $class, string $meth
*
* @see getDiscriminatorColumn()
*
* @param mixed[]|null $columnDef
* @psalm-param array{name: string|null, fieldName?: string, type?: string, length?: int, columnDefinition?: string|null, enumType?: class-string<BackedEnum>|null, options?: array<string, mixed>}|null $columnDef
* @param DiscriminatorColumnMapping|mixed[]|null $columnDef
* @psalm-param DiscriminatorColumnMapping|array{
* name: string|null,
* fieldName?: string,
* type?: string,
* length?: int,
* columnDefinition?: string|null,
* enumType?: class-string<BackedEnum>|null,
* options?:array<string,
* mixed>|null
* }|null $columnDef
*
* @throws MappingException
*/
public function setDiscriminatorColumn(array|null $columnDef): void
public function setDiscriminatorColumn(DiscriminatorColumnMapping|array|null $columnDef): void
{
if ($columnDef instanceof DiscriminatorColumnMapping) {
$this->discriminatorColumn = $columnDef;

return;
}

if ($columnDef !== null) {
if (! isset($columnDef['name'])) {
throw MappingException::nameIsMandatoryForDiscriminatorColumns($this->name);
Expand All @@ -2482,15 +2485,11 @@ public function setDiscriminatorColumn(array|null $columnDef): void
throw MappingException::invalidDiscriminatorColumnType($this->name, $columnDef['type']);
}

$this->discriminatorColumn = $columnDef;
$this->discriminatorColumn = DiscriminatorColumnMapping::fromMappingArray($columnDef);
}
}

/**
* @return array<string, mixed>
* @psalm-return DiscriminatorColumnMapping
*/
final public function getDiscriminatorColumn(): array
final public function getDiscriminatorColumn(): DiscriminatorColumnMapping
{
if ($this->discriminatorColumn === null) {
throw new LogicException('The discriminator column was not set.');
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected function doLoadMetadata(
): void {
if ($parent) {
$class->setInheritanceType($parent->inheritanceType);
$class->setDiscriminatorColumn($parent->discriminatorColumn);
$class->setDiscriminatorColumn($parent->discriminatorColumn === null ? null : clone $parent->discriminatorColumn);
$class->setIdGeneratorType($parent->generatorType);
$this->addInheritedFields($class, $parent);
$this->addInheritedRelations($class, $parent);
Expand Down
83 changes: 83 additions & 0 deletions lib/Doctrine/ORM/Mapping/DiscriminatorColumnMapping.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Mapping;

use ArrayAccess;
use BackedEnum;
use Exception;

use function in_array;
use function property_exists;

/** @template-implements ArrayAccess<string, mixed> */
final class DiscriminatorColumnMapping implements ArrayAccess
{
use ArrayAccessImplementation;

/** The database length of the column. Optional. Default value taken from the type. */
public int|null $length = null;

public string|null $columnDefinition = null;

/** @var class-string<BackedEnum>|null */
public string|null $enumType = null;

/** @var array<string, mixed> */
public array $options = [];

public function __construct(
public string $type,
public string $fieldName,
public string $name,
) {
}

/**
* @psalm-param array{
* type: string,
* fieldName: string,
* name: string,
* length?: int,
* columnDefinition?: string,
* enumType?: class-string<BackedEnum>,
* options?: array<string, mixed>,
* } $mappingArray
*/
public static function fromMappingArray(array $mappingArray): self
{
$mapping = new self(
$mappingArray['type'],
$mappingArray['fieldName'],
$mappingArray['name'],
);
foreach ($mappingArray as $key => $value) {
if (in_array($key, ['type', 'fieldName', 'name'])) {
continue;
}

if (property_exists($mapping, $key)) {
$mapping->$key = $value;
} else {
throw new Exception('Unknown property ' . $key . ' on class ' . static::class);
}
}

return $mapping;
}

/** @return list<string> */
public function __sleep(): array
{
$serialized = ['type', 'fieldName', 'name'];

foreach (['length', 'columnDefinition', 'enumType', 'options'] as $stringOrArrayKey) {
if ($this->$stringOrArrayKey !== null) {
$serialized[] = $stringOrArrayKey;
}
}

return $serialized;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ protected function getInsertColumnList(): array

// Add discriminator column if it is the topmost class.
if ($this->class->name === $this->class->rootEntityName) {
$columns[] = $this->class->getDiscriminatorColumn()['name'];
$columns[] = $this->class->getDiscriminatorColumn()->name;
}

return $columns;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected function getInsertColumnList(): array
$columns = parent::getInsertColumnList();

// Add discriminator column to the INSERT SQL
$columns[] = $this->class->getDiscriminatorColumn()['name'];
$columns[] = $this->class->getDiscriminatorColumn()->name;

return $columns;
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Doctrine/ORM/Tools/SchemaTool.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\DiscriminatorColumnMapping;
use Doctrine\ORM\Mapping\FieldMapping;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\ORM\Mapping\QuoteStrategy;
Expand Down Expand Up @@ -48,7 +49,6 @@
* @link www.doctrine-project.org
*
* @psalm-import-type AssociationMapping from ClassMetadata
* @psalm-import-type DiscriminatorColumnMapping from ClassMetadata
* @psalm-import-type JoinColumnData from ClassMetadata
*/
class SchemaTool
Expand Down Expand Up @@ -406,6 +406,7 @@ public function getSchemaFromMetadata(array $classes): Schema
private function addDiscriminatorColumnDefinition(ClassMetadata $class, Table $table): void
{
$discrColumn = $class->discriminatorColumn;
assert($discrColumn !== null);

if (
! isset($discrColumn['type']) ||
Expand Down Expand Up @@ -776,7 +777,7 @@ private function gatherRelationJoinColumns(
*
* @return mixed[]
*/
private function gatherColumnOptions(FieldMapping|array $mapping): array
private function gatherColumnOptions(FieldMapping|DiscriminatorColumnMapping|array $mapping): array
{
$mappingOptions = $mapping['options'] ?? [];

Expand Down
4 changes: 4 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,10 @@
<ArgumentTypeCoercion>
<code>$classes</code>
</ArgumentTypeCoercion>
<InvalidArrayOffset>
<code><![CDATA[$mapping['enumType']]]></code>
<code><![CDATA[$mapping['options']]]></code>
</InvalidArrayOffset>
<MissingClosureParamType>
<code>$asset</code>
</MissingClosureParamType>
Expand Down
11 changes: 10 additions & 1 deletion tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Doctrine\ORM\Mapping\Builder\EmbeddedBuilder;
use Doctrine\ORM\Mapping\Builder\FieldBuilder;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\DiscriminatorColumnMapping;
use Doctrine\ORM\Mapping\FieldMapping;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\Persistence\Mapping\RuntimeReflectionService;
Expand Down Expand Up @@ -183,7 +184,15 @@ public function testSetInheritanceSingleTable(): void
public function testSetDiscriminatorColumn(): void
{
$this->assertIsFluent($this->builder->setDiscriminatorColumn('discr', 'string', 124, null, null));
self::assertEquals(['fieldName' => 'discr', 'name' => 'discr', 'type' => 'string', 'length' => '124', 'columnDefinition' => null, 'enumType' => null, 'options' => []], $this->cm->discriminatorColumn);
self::assertEquals(DiscriminatorColumnMapping::fromMappingArray([
'fieldName' => 'discr',
'name' => 'discr',
'type' => 'string',
'length' => 124,
'columnDefinition' => null,
'enumType' => null,
'options' => [],
]), $this->cm->discriminatorColumn);
}

public function testAddDiscriminatorMapClass(): void
Expand Down
7 changes: 6 additions & 1 deletion tests/Doctrine/Tests/ORM/Mapping/ClassMetadataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
use Doctrine\ORM\Mapping\DefaultTypedFieldMapper;
use Doctrine\ORM\Mapping\DiscriminatorColumnMapping;
use Doctrine\ORM\Mapping\MappedSuperclass;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\ORM\Mapping\UnderscoreNamingStrategy;
Expand Down Expand Up @@ -102,7 +103,11 @@ public function testClassMetadataInstanceSerialization(): void
self::assertEquals([One::class, Two::class, Three::class], $cm->subClasses);
self::assertEquals(['UserParent'], $cm->parentClasses);
self::assertEquals(UserRepository::class, $cm->customRepositoryClassName);
self::assertEquals(['name' => 'disc', 'type' => 'integer', 'fieldName' => 'disc'], $cm->discriminatorColumn);
self::assertEquals(DiscriminatorColumnMapping::fromMappingArray([
'name' => 'disc',
'type' => 'integer',
'fieldName' => 'disc',
]), $cm->discriminatorColumn);
self::assertTrue($cm->associationMappings['phonenumbers']['type'] === ClassMetadata::ONE_TO_ONE);
self::assertEquals(1, count($cm->associationMappings));
$oneOneMapping = $cm->getAssociationMapping('phonenumbers');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Mapping;

use Doctrine\ORM\Mapping\DiscriminatorColumnMapping;
use PHPUnit\Framework\TestCase;

use function assert;
use function serialize;
use function unserialize;

final class DiscriminatorColumnMappingTest extends TestCase
{
public function testItSurvivesSerialization(): void
{
$mapping = new DiscriminatorColumnMapping(
type: 'string',
fieldName: 'discr',
name: 'discr',
);

$mapping->length = 255;
$mapping->columnDefinition = 'VARCHAR(255)';
$mapping->enumType = 'MyEnum';
$mapping->options = ['foo' => 'bar'];

$resurrectedMapping = unserialize(serialize($mapping));
assert($resurrectedMapping instanceof DiscriminatorColumnMapping);

self::assertSame($resurrectedMapping->length, 255);
self::assertSame($resurrectedMapping->columnDefinition, 'VARCHAR(255)');
self::assertSame($resurrectedMapping->enumType, 'MyEnum');
self::assertSame($resurrectedMapping->options, ['foo' => 'bar']);
}
}
10 changes: 9 additions & 1 deletion tests/Doctrine/Tests/ORM/Mapping/MappingDriverTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
use Doctrine\ORM\Mapping\DefaultTypedFieldMapper;
use Doctrine\ORM\Mapping\DiscriminatorColumn;
use Doctrine\ORM\Mapping\DiscriminatorColumnMapping;
use Doctrine\ORM\Mapping\DiscriminatorMap;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
Expand Down Expand Up @@ -457,7 +458,14 @@ public function testDiscriminatorColumnDefaults(): void
$class = $this->createClassMetadata(Animal::class);

self::assertEquals(
['name' => 'discr', 'type' => 'string', 'length' => 32, 'fieldName' => 'discr', 'columnDefinition' => null, 'enumType' => null],
DiscriminatorColumnMapping::fromMappingArray([
'name' => 'discr',
'type' => 'string',
'length' => 32,
'fieldName' => 'discr',
'columnDefinition' => null,
'enumType' => null,
]),
$class->discriminatorColumn,
);
}
Expand Down

0 comments on commit e22592f

Please sign in to comment.