Fix typed properties for default metadata (#7939) (#8589)

* Make Column::$type, Column::$nullable and JoinColumn::$nullable nullable by default * Add tests for mapped typed properties (type and nullable) * Fix Yaml driver tests and remove driver exceptions thrown too early * Fix PHP driver tests * Fix static PHP driver tests * Fix XML driver tests * Coding Standards * Deprecate unused MappingException method * Add manyToOne test and check nullable at the right place * Coding Standards * Bugfix: Temporarily change association join columns in CascadeRemoveOrderTest to circumvent new CommitOrderCalculator bug. * phpcs Co-authored-by: Benjamin Eberlei <[email protected]>
doctrine · Apr 18, 2021 · a223048 · a223048
1 parent a68aa58
commit a223048
Show file tree

Hide file tree

Showing 17 changed files with 285 additions and 25 deletions.
diff --git a/doctrine-mapping.xsd b/doctrine-mapping.xsd
@@ -541,7 +541,7 @@
       <xs:any minOccurs="0" maxOccurs="unbounded" namespace="##other"/>
     </xs:sequence>
     <xs:attribute name="field" type="xs:NMTOKEN" use="required" />
-    <xs:attribute name="target-entity" type="xs:string" use="required" />
+    <xs:attribute name="target-entity" type="xs:string" />
     <xs:attribute name="inversed-by" type="xs:NMTOKEN" />
     <xs:attribute name="fetch" type="orm:fetch-type" default="LAZY" />
     <xs:anyAttribute namespace="##other"/>
@@ -559,7 +559,7 @@
       <xs:any minOccurs="0" maxOccurs="unbounded" namespace="##other"/>
     </xs:sequence>
     <xs:attribute name="field" type="xs:NMTOKEN" use="required" />
-    <xs:attribute name="target-entity" type="xs:string" use="required" />
+    <xs:attribute name="target-entity" type="xs:string" />
     <xs:attribute name="mapped-by" type="xs:NMTOKEN" />
     <xs:attribute name="inversed-by" type="xs:NMTOKEN" />
     <xs:attribute name="fetch" type="orm:fetch-type" default="LAZY" />

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -1758,6 +1758,7 @@ protected function _validateAndCompleteOneToOneMapping(array $mapping)
                     [
                         'name' => $this->namingStrategy->joinColumnName($mapping['fieldName'], $this->name),
                         'referencedColumnName' => $this->namingStrategy->referenceColumnName(),
+                        'nullable' => true,
                     ],
                 ];
             }
@@ -1775,6 +1776,10 @@ protected function _validateAndCompleteOneToOneMapping(array $mapping)
                     }
                 }
 
+                if (! isset($joinColumn['nullable'])) {
+                    $joinColumn['nullable'] = true;
+                }
+
                 if (empty($joinColumn['name'])) {
                     $joinColumn['name'] = $this->namingStrategy->joinColumnName($mapping['fieldName'], $this->name);
                 }

diff --git a/lib/Doctrine/ORM/Mapping/Column.php b/lib/Doctrine/ORM/Mapping/Column.php
@@ -35,7 +35,7 @@ final class Column implements Annotation
     public $name;
 
     /** @var mixed */
-    public $type = 'string';
+    public $type;
 
     /** @var int */
     public $length;
@@ -57,8 +57,8 @@ final class Column implements Annotation
     /** @var bool */
     public $unique = false;
 
-    /** @var bool */
-    public $nullable = false;
+    /** @var bool|null */
+    public $nullable;
 
     /** @var array<string,mixed> */
     public $options = [];
@@ -71,12 +71,12 @@ final class Column implements Annotation
      */
     public function __construct(
         ?string $name = null,
-        string $type = 'string',
+        ?string $type = null,
         ?int $length = null,
         ?int $precision = null,
         ?int $scale = null,
         bool $unique = false,
-        bool $nullable = false,
+        ?bool $nullable = null,
         array $options = [],
         ?string $columnDefinition = null
     ) {

diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
@@ -370,10 +370,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             // @Column, @OneToOne, @OneToMany, @ManyToOne, @ManyToMany
             $columnAnnot = $this->reader->getPropertyAnnotation($property, Mapping\Column::class);
             if ($columnAnnot) {
-                if ($columnAnnot->type === null) {
-                    throw MappingException::propertyTypeIsRequired($className, $property->getName());
-                }
-
                 $mapping = $this->columnToArray($property->getName(), $columnAnnot);
 
                 $idAnnot = $this->reader->getPropertyAnnotation($property, Mapping\Id::class);

diff --git a/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php
@@ -258,10 +258,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata): void
             $embeddedAttribute   = $this->reader->getPropertyAnnotation($property, Mapping\Embedded::class);
 
             if ($columnAttribute !== null) {
-                if ($columnAttribute->type === null) {
-                    throw MappingException::propertyTypeIsRequired($className, $property->getName());
-                }
-
                 $mapping = $this->columnToArray($property->getName(), $columnAttribute);
 
                 if ($this->reader->getPropertyAnnotation($property, Mapping\Id::class)) {

diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
@@ -409,9 +409,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($xmlRoot->{'one-to-one'} as $oneToOneElement) {
                 $mapping = [
                     'fieldName' => (string) $oneToOneElement['field'],
-                    'targetEntity' => (string) $oneToOneElement['target-entity'],
                 ];
 
+                if (isset($oneToOneElement['target-entity'])) {
+                    $mapping['targetEntity'] = (string) $oneToOneElement['target-entity'];
+                }
+
                 if (isset($associationIds[$mapping['fieldName']])) {
                     $mapping['id'] = true;
                 }
@@ -462,10 +465,13 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($xmlRoot->{'one-to-many'} as $oneToManyElement) {
                 $mapping = [
                     'fieldName' => (string) $oneToManyElement['field'],
-                    'targetEntity' => (string) $oneToManyElement['target-entity'],
                     'mappedBy' => (string) $oneToManyElement['mapped-by'],
                 ];
 
+                if (isset($oneToManyElement['target-entity'])) {
+                    $mapping['targetEntity'] = (string) $oneToManyElement['target-entity'];
+                }
+
                 if (isset($oneToManyElement['fetch'])) {
                     $mapping['fetch'] = constant('Doctrine\ORM\Mapping\ClassMetadata::FETCH_' . (string) $oneToManyElement['fetch']);
                 }
@@ -509,9 +515,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($xmlRoot->{'many-to-one'} as $manyToOneElement) {
                 $mapping = [
                     'fieldName' => (string) $manyToOneElement['field'],
-                    'targetEntity' => (string) $manyToOneElement['target-entity'],
                 ];
 
+                if (isset($manyToOneElement['target-entity'])) {
+                    $mapping['targetEntity'] = (string) $manyToOneElement['target-entity'];
+                }
+
                 if (isset($associationIds[$mapping['fieldName']])) {
                     $mapping['id'] = true;
                 }
@@ -554,9 +563,12 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($xmlRoot->{'many-to-many'} as $manyToManyElement) {
                 $mapping = [
                     'fieldName' => (string) $manyToManyElement['field'],
-                    'targetEntity' => (string) $manyToManyElement['target-entity'],
                 ];
 
+                if (isset($manyToManyElement['target-entity'])) {
+                    $mapping['targetEntity'] = (string) $manyToManyElement['target-entity'];
+                }
+
                 if (isset($manyToManyElement['fetch'])) {
                     $mapping['fetch'] = constant('Doctrine\ORM\Mapping\ClassMetadata::FETCH_' . (string) $manyToManyElement['fetch']);
                 }

diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
@@ -420,7 +420,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($element['oneToOne'] as $name => $oneToOneElement) {
                 $mapping = [
                     'fieldName' => $name,
-                    'targetEntity' => $oneToOneElement['targetEntity'],
+                    'targetEntity' => $oneToOneElement['targetEntity'] ?? null,
                 ];
 
                 if (isset($associationIds[$mapping['fieldName']])) {
@@ -515,7 +515,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             foreach ($element['manyToOne'] as $name => $manyToOneElement) {
                 $mapping = [
                     'fieldName' => $name,
-                    'targetEntity' => $manyToOneElement['targetEntity'],
+                    'targetEntity' => $manyToOneElement['targetEntity'] ?? null,
                 ];
 
                 if (isset($associationIds[$mapping['fieldName']])) {

diff --git a/lib/Doctrine/ORM/Mapping/JoinColumn.php b/lib/Doctrine/ORM/Mapping/JoinColumn.php
@@ -40,8 +40,8 @@ final class JoinColumn implements Annotation
     /** @var bool */
     public $unique = false;
 
-    /** @var bool */
-    public $nullable = true;
+    /** @var bool|null */
+    public $nullable;
 
     /** @var mixed */
     public $onDelete;
@@ -60,7 +60,7 @@ public function __construct(
         ?string $name = null,
         string $referencedColumnName = 'id',
         bool $unique = false,
-        bool $nullable = true,
+        ?bool $nullable = null,
         $onDelete = null,
         ?string $columnDefinition = null,
         ?string $fieldName = null

diff --git a/lib/Doctrine/ORM/Mapping/ManyToOne.php b/lib/Doctrine/ORM/Mapping/ManyToOne.php
@@ -52,7 +52,7 @@ final class ManyToOne implements Annotation
      * @param array<string> $cascade
      */
     public function __construct(
-        string $targetEntity,
+        ?string $targetEntity = null,
         ?array $cascade = null,
         string $fetch = 'LAZY',
         ?string $inversedBy = null

diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -382,6 +382,8 @@ public static function classIsNotAValidEntityOrMappedSuperClass($className)
     }
 
     /**
+     * @deprecated 2.9 no longer in use
+     *
      * @param string $className
      * @param string $propertyName
      *

diff --git a/tests/Doctrine/Tests/Models/TypedProperties/UserTyped.php b/tests/Doctrine/Tests/Models/TypedProperties/UserTyped.php
@@ -7,46 +7,129 @@
 use DateInterval;
 use DateTime;
 use DateTimeImmutable;
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\ORM\Mapping\ClassMetadataInfo;
 use Doctrine\Tests\Models\CMS\CmsEmail;
 
 /**
  * @Entity
  * @Table(name="cms_users_typed")
  */
+#[ORM\Entity, ORM\Table(name: 'cms_users_typed')]
 class UserTyped
 {
     /**
      * @Id @Column
      * @GeneratedValue
      */
+    #[ORM\Id, ORM\Column, ORM\GeneratedValue]
     public int $id;
     /** @Column(length=50) */
+    #[ORM\Column(length: 50)]
     public ?string $status;
 
     /** @Column(length=255, unique=true) */
+    #[ORM\Column(length: 255, unique: true)]
     public string $username;
 
     /** @Column */
+    #[ORM\Column]
     public DateInterval $dateInterval;
 
     /** @Column */
+    #[ORM\Column]
     public DateTime $dateTime;
 
     /** @Column */
+    #[ORM\Column]
     public DateTimeImmutable $dateTimeImmutable;
 
     /** @Column */
+    #[ORM\Column]
     public array $array;
 
     /** @Column */
+    #[ORM\Column]
     public bool $boolean;
 
     /** @Column */
+    #[ORM\Column]
     public float $float;
 
     /**
      * @OneToOne(cascade={"persist"}, orphanRemoval=true)
      * @JoinColumn
      */
+    #[ORM\OneToOne(cascade: ['persist'], orphanRemoval: true), ORM\JoinColumn]
     public CmsEmail $email;
+
+    /** @ManyToOne */
+    #[ORM\ManyToOne]
+    public ?CmsEmail $mainEmail;
+
+    public static function loadMetadata(ClassMetadataInfo $metadata): void
+    {
+        $metadata->setInheritanceType(ClassMetadataInfo::INHERITANCE_TYPE_NONE);
+        $metadata->setPrimaryTable(
+            ['name' => 'cms_users_typed']
+        );
+
+        $metadata->mapField(
+            [
+                'id' => true,
+                'fieldName' => 'id',
+            ]
+        );
+        $metadata->setIdGeneratorType(ClassMetadataInfo::GENERATOR_TYPE_AUTO);
+
+        $metadata->mapField(
+            [
+                'fieldName' => 'status',
+                'length' => 50,
+            ]
+        );
+        $metadata->mapField(
+            [
+                'fieldName' => 'username',
+                'length' => 255,
+                'unique' => true,
+            ]
+        );
+        $metadata->mapField(
+            ['fieldName' => 'dateInterval']
+        );
+        $metadata->mapField(
+            ['fieldName' => 'dateTime']
+        );
+        $metadata->mapField(
+            ['fieldName' => 'dateTimeImmutable']
+        );
+        $metadata->mapField(
+            ['fieldName' => 'array']
+        );
+        $metadata->mapField(
+            ['fieldName' => 'boolean']
+        );
+        $metadata->mapField(
+            ['fieldName' => 'float']
+        );
+
+        $metadata->mapOneToOne(
+            [
+                'fieldName' => 'email',
+                'cascade' =>
+                    [0 => 'persist'],
+                'joinColumns' =>
+                    [
+                        0 =>
+                            [],
+                    ],
+                'orphanRemoval' => true,
+            ]
+        );
+
+        $metadata->mapManyToOne(
+            ['fieldName' => 'mainEmail']
+        );
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Functional/CascadeRemoveOrderTest.php b/tests/Doctrine/Tests/ORM/Functional/CascadeRemoveOrderTest.php
@@ -158,6 +158,7 @@ class CascadeRemoveOrderEntityG
      *     targetEntity="Doctrine\Tests\ORM\Functional\CascadeRemoveOrderEntityO",
      *     inversedBy="oneToMany"
      * )
+     * @JoinColumn(nullable=true, onDelete="SET NULL")
      */
     private $ownerO;
-Original file line number
+Diff line change
@@ Expand Up @@
         }
         /**
+         * @deprecated 2.9 no longer in use
+         *
          * @param string $className
          * @param string $propertyName
          *
@@ Expand Down @@