Merge pull request #10601 from JanTvrdik/discriminated-column-options

doctrine · Mar 30, 2023 · 7ed28db · 7ed28db
2 parents ffbfbfc + f215515
commit 7ed28db
Show file tree

Hide file tree

Showing 15 changed files with 172 additions and 48 deletions.
diff --git a/docs/en/reference/attributes-reference.rst b/docs/en/reference/attributes-reference.rst
@@ -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:
 

diff --git a/docs/en/reference/php-mapping.rst b/docs/en/reference/php-mapping.rst
@@ -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()``

diff --git a/doctrine-mapping.xsd b/doctrine-mapping.xsd
@@ -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" />

diff --git a/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php b/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php
@@ -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 = [])
     {
         $this->cm->setDiscriminatorColumn(
             [
@@ -232,6 +233,7 @@ public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?
                 'length' => $length,
                 'columnDefinition' => $columnDefinition,
                 'enumType' => $enumType,
+                'options' => $options,
             ]
         );
 

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadata.php b/lib/Doctrine/ORM/Mapping/ClassMetadata.php
@@ -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,

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -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
      *

diff --git a/lib/Doctrine/ORM/Mapping/DiscriminatorColumn.php b/lib/Doctrine/ORM/Mapping/DiscriminatorColumn.php
@@ -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;
     }
 }
diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
@@ -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]);
                 }

diff --git a/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php
@@ -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]);
                 }

diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
@@ -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]);
                 }

diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php
@@ -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
  */
@@ -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);
     }
 
@@ -792,7 +794,7 @@ private function gatherRelationJoinColumns(
     }
 
     /**
-     * @psalm-param JoinColumnData|FieldMapping $mapping
+     * @psalm-param JoinColumnData|FieldMapping|DiscriminatorColumnMapping $mapping
      *
      * @return mixed[]
      */

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -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,
@@ -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>

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10462Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10462Test.php
@@ -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
+{
+}
diff --git a/tests/Doctrine/Tests/ORM/Mapping/AttributeReaderTest.php b/tests/Doctrine/Tests/ORM/Mapping/AttributeReaderTest.php
@@ -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;
 
 /**
@@ -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]
@@ -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
+{
+}
diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataBuilderTest.php
@@ -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