Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support options like charset and collation on DiscriminatedColumn #10601

Merged
merged 1 commit into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/en/reference/attributes-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ Optional parameters:
- **length**: By default this is 255.
- **columnDefinition**: By default this is null the definition according to the type will be used. This option allows to override it.
- **enumType**: By default this is `null`. Allows to map discriminatorColumn value to PHP enum
- **options**: See "options" attribute on :ref:`#[Column] <attrref_column>`.

.. _attrref_discriminatormap:

Expand Down
2 changes: 1 addition & 1 deletion docs/en/reference/php-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ The API of the ClassMetadataBuilder has the following methods with a fluent inte
- ``addNamedQuery($name, $dqlQuery)``
- ``setJoinedTableInheritance()``
- ``setSingleTableInheritance()``
- ``setDiscriminatorColumn($name, $type = 'string', $length = 255, $columnDefinition = null, $enumType = null)``
- ``setDiscriminatorColumn($name, $type = 'string', $length = 255, $columnDefinition = null, $enumType = null, $options = [])``
- ``addDiscriminatorMapClass($name, $class)``
- ``setChangeTrackingPolicyDeferredExplicit()``
- ``setChangeTrackingPolicyNotify()``
Expand Down
1 change: 1 addition & 0 deletions doctrine-mapping.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@

<xs:complexType name="discriminator-column">
<xs:choice minOccurs="0" maxOccurs="unbounded">
<xs:element name="options" type="orm:options" minOccurs="0" />
<xs:any minOccurs="0" maxOccurs="unbounded" namespace="##other"/>
</xs:choice>
<xs:attribute name="name" type="xs:NMTOKEN" use="required" />
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,11 @@ public function setSingleTableInheritance()
* @param string $type
* @param int $length
* @psalm-param class-string<BackedEnum>|null $enumType
* @psalm-param array<string, mixed> $options
*
* @return $this
*/
public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?string $columnDefinition = null, ?string $enumType = null)
public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?string $columnDefinition = null, ?string $enumType = null, array $options = [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically a breaking change for extending classes, but we didn't frown upon a similar change in #10288 , so maybe we should just make the class as final on 3.0.x.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, that PR modified docs/en/reference/php-mapping.rst , and this one should too I think. You might want to look at that PR for other changes that need to be done.

{
$this->cm->setDiscriminatorColumn(
[
Expand All @@ -232,6 +233,7 @@ public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?
'length' => $length,
'columnDefinition' => $columnDefinition,
'enumType' => $enumType,
'options' => $options,
]
);

Expand Down
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
* length?: int,
* columnDefinition?: string|null,
* enumType?: class-string<BackedEnum>|null,
* options?: array<string, mixed>,
* }
* @psalm-type EmbeddedClassMapping = array{
* class: class-string,
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -3131,7 +3131,7 @@ public function addEntityListener($eventName, $class, $method)
* @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}|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
*
* @return void
*
Expand Down
15 changes: 13 additions & 2 deletions lib/Doctrine/ORM/Mapping/DiscriminatorColumn.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,29 @@ final class DiscriminatorColumn implements MappingAttribute
*/
public $enumType = null;

/** @param class-string<\BackedEnum>|null $enumType */
/**
* @var array<string, mixed>
* @readonly
*/
public $options = [];

/**
* @param class-string<\BackedEnum>|null $enumType
* @param array<string, mixed> $options
*/
public function __construct(
?string $name = null,
?string $type = null,
?int $length = null,
?string $columnDefinition = null,
?string $enumType = null
?string $enumType = null,
array $options = []
) {
$this->name = $name;
$this->type = $type;
$this->length = $length;
$this->columnDefinition = $columnDefinition;
$this->enumType = $enumType;
$this->options = $options;
}
}
22 changes: 13 additions & 9 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,19 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
$discrColumnAnnot = $classAnnotations[Mapping\DiscriminatorColumn::class];
assert($discrColumnAnnot instanceof Mapping\DiscriminatorColumn);

$metadata->setDiscriminatorColumn(
[
'name' => $discrColumnAnnot->name,
'type' => $discrColumnAnnot->type ?: 'string',
'length' => $discrColumnAnnot->length ?? 255,
'columnDefinition' => $discrColumnAnnot->columnDefinition,
'enumType' => $discrColumnAnnot->enumType,
]
);
$columnDef = [
'name' => $discrColumnAnnot->name,
'type' => $discrColumnAnnot->type ?: 'string',
'length' => $discrColumnAnnot->length ?? 255,
'columnDefinition' => $discrColumnAnnot->columnDefinition,
'enumType' => $discrColumnAnnot->enumType,
];

if ($discrColumnAnnot->options) {
$columnDef['options'] = $discrColumnAnnot->options;
}

$metadata->setDiscriminatorColumn($columnDef);
} else {
$metadata->setDiscriminatorColumn(['name' => 'dtype', 'type' => 'string', 'length' => 255]);
}
Expand Down
22 changes: 13 additions & 9 deletions lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,15 +265,19 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
if (isset($classAttributes[Mapping\DiscriminatorColumn::class])) {
$discrColumnAttribute = $classAttributes[Mapping\DiscriminatorColumn::class];

$metadata->setDiscriminatorColumn(
[
'name' => isset($discrColumnAttribute->name) ? (string) $discrColumnAttribute->name : null,
'type' => isset($discrColumnAttribute->type) ? (string) $discrColumnAttribute->type : 'string',
'length' => isset($discrColumnAttribute->length) ? (int) $discrColumnAttribute->length : 255,
'columnDefinition' => isset($discrColumnAttribute->columnDefinition) ? (string) $discrColumnAttribute->columnDefinition : null,
'enumType' => isset($discrColumnAttribute->enumType) ? (string) $discrColumnAttribute->enumType : null,
]
);
$columnDef = [
'name' => isset($discrColumnAttribute->name) ? (string) $discrColumnAttribute->name : null,
'type' => isset($discrColumnAttribute->type) ? (string) $discrColumnAttribute->type : 'string',
'length' => isset($discrColumnAttribute->length) ? (int) $discrColumnAttribute->length : 255,
'columnDefinition' => isset($discrColumnAttribute->columnDefinition) ? (string) $discrColumnAttribute->columnDefinition : null,
'enumType' => isset($discrColumnAttribute->enumType) ? (string) $discrColumnAttribute->enumType : null,
];

if ($discrColumnAttribute->options) {
$columnDef['options'] = (array) $discrColumnAttribute->options;
}

$metadata->setDiscriminatorColumn($columnDef);
} else {
$metadata->setDiscriminatorColumn(['name' => 'dtype', 'type' => 'string', 'length' => 255]);
}
Expand Down
22 changes: 13 additions & 9 deletions lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,19 @@ public function loadMetadataForClass($className, PersistenceClassMetadata $metad
// Evaluate <discriminator-column...>
if (isset($xmlRoot->{'discriminator-column'})) {
$discrColumn = $xmlRoot->{'discriminator-column'};
$metadata->setDiscriminatorColumn(
[
'name' => isset($discrColumn['name']) ? (string) $discrColumn['name'] : null,
'type' => isset($discrColumn['type']) ? (string) $discrColumn['type'] : 'string',
'length' => isset($discrColumn['length']) ? (int) $discrColumn['length'] : 255,
'columnDefinition' => isset($discrColumn['column-definition']) ? (string) $discrColumn['column-definition'] : null,
'enumType' => isset($discrColumn['enum-type']) ? (string) $discrColumn['enum-type'] : null,
]
);
$columnDef = [
'name' => isset($discrColumn['name']) ? (string) $discrColumn['name'] : null,
'type' => isset($discrColumn['type']) ? (string) $discrColumn['type'] : 'string',
'length' => isset($discrColumn['length']) ? (int) $discrColumn['length'] : 255,
'columnDefinition' => isset($discrColumn['column-definition']) ? (string) $discrColumn['column-definition'] : null,
'enumType' => isset($discrColumn['enum-type']) ? (string) $discrColumn['enum-type'] : null,
];

if (isset($discrColumn['options'])) {
$columnDef['options'] = $this->parseOptions($discrColumn['options']->children());
}

$metadata->setDiscriminatorColumn($columnDef);
} else {
$metadata->setDiscriminatorColumn(['name' => 'dtype', 'type' => 'string', 'length' => 255]);
}
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/ORM/Tools/SchemaTool.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
* @link www.doctrine-project.org
*
* @psalm-import-type AssociationMapping from ClassMetadata
* @psalm-import-type DiscriminatorColumnMapping from ClassMetadata
* @psalm-import-type FieldMapping from ClassMetadata
* @psalm-import-type JoinColumnData from ClassMetadata
*/
Expand Down Expand Up @@ -444,6 +445,7 @@ private function addDiscriminatorColumnDefinition(ClassMetadata $class, Table $t
$options['columnDefinition'] = $discrColumn['columnDefinition'];
}

$options = $this->gatherColumnOptions($discrColumn) + $options;
$table->addColumn($discrColumn['name'], $discrColumn['type'], $options);
}

Expand Down Expand Up @@ -792,7 +794,7 @@ private function gatherRelationJoinColumns(
}

/**
* @psalm-param JoinColumnData|FieldMapping $mapping
* @psalm-param JoinColumnData|FieldMapping|DiscriminatorColumnMapping $mapping
*
* @return mixed[]
*/
Expand Down
18 changes: 4 additions & 14 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -812,14 +812,10 @@
</UndefinedInterfaceMethod>
</file>
<file src="lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php">
<ArgumentTypeCoercion>
<code>$columnDef</code>
</ArgumentTypeCoercion>
<InvalidArgument>
<code><![CDATA[[
'name' => isset($discrColumnAttribute->name) ? (string) $discrColumnAttribute->name : null,
'type' => isset($discrColumnAttribute->type) ? (string) $discrColumnAttribute->type : 'string',
'length' => isset($discrColumnAttribute->length) ? (int) $discrColumnAttribute->length : 255,
'columnDefinition' => isset($discrColumnAttribute->columnDefinition) ? (string) $discrColumnAttribute->columnDefinition : null,
'enumType' => isset($discrColumnAttribute->enumType) ? (string) $discrColumnAttribute->enumType : null,
]]]></code>
<code><![CDATA[[
'sequenceName' => $seqGeneratorAttribute->sequenceName,
'allocationSize' => $seqGeneratorAttribute->allocationSize,
Expand Down Expand Up @@ -914,13 +910,7 @@
<code>addNamedQuery</code>
</DeprecatedMethod>
<InvalidArgument>
<code><![CDATA[[
'name' => isset($discrColumn['name']) ? (string) $discrColumn['name'] : null,
'type' => isset($discrColumn['type']) ? (string) $discrColumn['type'] : 'string',
'length' => isset($discrColumn['length']) ? (int) $discrColumn['length'] : 255,
'columnDefinition' => isset($discrColumn['column-definition']) ? (string) $discrColumn['column-definition'] : null,
'enumType' => isset($discrColumn['enum-type']) ? (string) $discrColumn['enum-type'] : null,
]]]></code>
<code>$columnDef</code>
</InvalidArgument>
<InvalidPropertyAssignmentValue>
<code><![CDATA[$metadata->table]]></code>
Expand Down
64 changes: 64 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH10462Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\MySQLPlatform;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\DiscriminatorColumn;
use Doctrine\ORM\Mapping\DiscriminatorMap;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\InheritanceType;
use Doctrine\ORM\Mapping\Table;
use Doctrine\Tests\OrmFunctionalTestCase;

use function method_exists;

class GH10462Test extends OrmFunctionalTestCase
{
public function testCharsetAndCollationOptionsOnDiscriminatedColumn(): void
{
if (! $this->_em->getConnection()->getDatabasePlatform() instanceof MySQLPlatform) {
self::markTestSkipped('This test is useful for all databases, but designed only for mysql.');
}

if (method_exists(AbstractPlatform::class, 'getGuidExpression')) {
self::markTestSkipped('Test valid for doctrine/dbal:3.x only.');
}

$this->createSchemaForModels(GH10462Person::class, GH10462Employee::class);
$schemaManager = $this->createSchemaManager();
$personOptions = $schemaManager->introspectTable('gh10462_person')->getColumn('discr')->toArray();
self::assertSame('ascii', $personOptions['charset']);
self::assertSame('ascii_general_ci', $personOptions['collation']);
}
}

/**
* @Entity
* @Table(name="gh10462_person")
* @InheritanceType("SINGLE_TABLE")
* @DiscriminatorColumn(name="discr", type="string", options={"charset"="ascii", "collation"="ascii_general_ci"})
* @DiscriminatorMap({"person"=GH10462Person::class, "employee"=GH10462Employee::class})
*/
class GH10462Person
{
/**
* @var int
* @Id
* @Column(type="integer")
* @GeneratedValue
*/
public $id;
}

/**
* @Entity
*/
class GH10462Employee extends GH10462Person
{
}
40 changes: 40 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/AttributeReaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,21 @@
namespace Doctrine\Tests\ORM\Mapping;

use Doctrine\ORM\Mapping as ORM;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\DiscriminatorColumn;
use Doctrine\ORM\Mapping\DiscriminatorMap;
use Doctrine\ORM\Mapping\Driver\AttributeReader;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\InheritanceType;
use Doctrine\ORM\Mapping\InverseJoinColumn;
use Doctrine\ORM\Mapping\JoinColumn;
use Doctrine\ORM\Mapping\JoinTable;
use Doctrine\ORM\Mapping\ManyToMany;
use LogicException;
use PHPUnit\Framework\TestCase;
use ReflectionClass;
use ReflectionProperty;

/**
Expand Down Expand Up @@ -72,6 +80,19 @@ public function testJoinColumnOptions(): void
'collation' => 'utf8mb4_bin',
], $inverseJoinColumns[0]->options);
}

public function testDiscriminatedColumnOptions(): void
{
$reader = new AttributeReader();
$class = new ReflectionClass(TestPerson::class);

$attributes = $reader->getClassAttributes($class);
self::assertArrayHasKey(DiscriminatorColumn::class, $attributes);
self::assertSame([
'charset' => 'ascii',
'collation' => 'ascii_general_ci',
], $attributes[DiscriminatorColumn::class]->options);
}
}

#[ORM\Entity]
Expand Down Expand Up @@ -101,3 +122,22 @@ class TestTag
#[ORM\GeneratedValue]
public $id;
}


#[Entity]
#[InheritanceType('SINGLE_TABLE')]
#[DiscriminatorColumn(name: 'discr', options: ['charset' => 'ascii', 'collation' => 'ascii_general_ci'])]
#[DiscriminatorMap(['person' => TestPerson::class, 'employee' => TestEmployee::class])]
class TestPerson
{
/** @var int */
#[Id]
#[Column(type: 'integer')]
#[GeneratedValue]
public $id;
}

#[Entity]
class TestEmployee extends TestPerson
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ 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], $this->cm->discriminatorColumn);
self::assertEquals(['fieldName' => 'discr', 'name' => 'discr', 'type' => 'string', 'length' => '124', 'columnDefinition' => null, 'enumType' => null, 'options' => []], $this->cm->discriminatorColumn);
}

public function testAddDiscriminatorMapClass(): void
Expand Down