Merge branch 'fix/#6303-#6304-check-for-null-discriminator-before-hydrating-inheritance-rows'

Close #6303
Close #6304
ge aborts

lib/Doctrine/ORM/Internal/Hydration/AbstractHydrator.php

@@ -294,6 +294,17 @@ abstract class AbstractHydrator
                     $dqlAlias = $cacheKeyInfo['dqlAlias'];
                     $type     = $cacheKeyInfo['type'];
 
+                    // If there are field name collisions in the child class, then we need
+                    // to only hydrate if we are looking at the correct discriminator value
+                    if(
+                        isset($cacheKeyInfo['discriminatorColumn']) && 
+                        isset($data[$cacheKeyInfo['discriminatorColumn']]) &&
+                        // Note: loose comparison required. See https://github.com/doctrine/doctrine2/pull/6304#issuecomment-323294442
+                        $data[$cacheKeyInfo['discriminatorColumn']] != $cacheKeyInfo['discriminatorValue']
+                    ) {
+                        break;
+                    }
+
                     // in an inheritance hierarchy the same field could be defined several times.
                     // We overwrite this value so long we don't have a non-null value, that value we keep.
                     // Per definition it cannot be that a field is defined several times and has several values.
@@ -375,14 +386,28 @@ abstract class AbstractHydrator
                 $classMetadata = $this->getClassMetadata($this->_rsm->declaringClasses[$key]);
                 $fieldName     = $this->_rsm->fieldMappings[$key];
                 $fieldMapping  = $classMetadata->fieldMappings[$fieldName];
-
+                $ownerMap      = $this->_rsm->columnOwnerMap[$key];
-                return $this->_cache[$key] = [
+                $columnInfo    = [
-                    'isIdentifier' => in_array($fieldName, $classMetadata->identifier),
+                    'isIdentifier' => \in_array($fieldName, $classMetadata->identifier, true),
                     'fieldName'    => $fieldName,
                     'type'         => Type::getType($fieldMapping['type']),
-                    'dqlAlias'     => $this->_rsm->columnOwnerMap[$key],
+                    'dqlAlias'     => $ownerMap,
                 ];
 
+                // the current discriminator value must be saved in order to disambiguate fields hydration,
+                // should there be field name collisions
+                if ($classMetadata->parentClasses && isset($this->_rsm->discriminatorColumns[$ownerMap])) {
+                    return $this->_cache[$key] = \array_merge(
+                        $columnInfo,
+                        [
+                            'discriminatorColumn' => $this->_rsm->discriminatorColumns[$ownerMap],
+                            'discriminatorValue'  => $classMetadata->discriminatorValue
+                        ]
+                    );
+                }
+
+                return $this->_cache[$key] = $columnInfo;
+
             case (isset($this->_rsm->newObjectMappings[$key])):
                 // WARNING: A NEW object is also a scalar, so it must be declared before!
                 $mapping = $this->_rsm->newObjectMappings[$key];

tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6303Test.php

@@ -0,0 +1,121 @@
+<?php
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\ORM\Tools\ToolsException;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group #6303
+ */
+class DDC6303Test extends OrmFunctionalTestCase
+{
+    public function setUp() : void
+    {
+        parent::setUp();
+
+        try {
+            $this->_schemaTool->createSchema([
+                $this->_em->getClassMetadata(DDC6303BaseClass::class),
+                $this->_em->getClassMetadata(DDC6303ChildA::class),
+                $this->_em->getClassMetadata(DDC6303ChildB::class),
+            ]);
+        } catch (ToolsException $ignored) {
+        }
+    }
+
+    public function testMixedTypeHydratedCorrectlyInJoinedInheritance() : void
+    {
+        // DDC6303ChildA and DDC6303ChildB have an inheritance from DDC6303BaseClass,
+        // but one has a string originalData and the second has an array, since the fields
+        // are mapped differently
+        $this->assertHydratedEntitiesSameToPersistedOnes([
+            'a' => new DDC6303ChildA('a', 'authorized'),
+            'b' => new DDC6303ChildB('b', ['accepted', 'authorized']),
+        ]);
+
+    }
+
+    public function testEmptyValuesInJoinedInheritance() : void
+    {
+        $this->assertHydratedEntitiesSameToPersistedOnes([
+            'stringEmpty' => new DDC6303ChildA('stringEmpty', ''),
+            'stringZero'  => new DDC6303ChildA('stringZero', 0),
+            'arrayEmpty'  => new DDC6303ChildB('arrayEmpty', []),
+        ]);
+    }
+
+    /**
+     * @param DDC6303BaseClass[] $persistedEntities indexed by identifier
+     *
+     * @throws \Doctrine\Common\Persistence\Mapping\MappingException
+     * @throws \Doctrine\ORM\ORMException
+     * @throws \Doctrine\ORM\OptimisticLockException
+     */
+    private function assertHydratedEntitiesSameToPersistedOnes(array $persistedEntities) : void
+    {
+        array_walk($persistedEntities, [$this->_em, 'persist']);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        /* @var $entities DDC6303BaseClass[] */
+        $entities = $this
+            ->_em
+            ->getRepository(DDC6303BaseClass::class)
+            ->createQueryBuilder('p')
+            ->where('p.id IN(:ids)')
+            ->setParameter('ids', array_keys($persistedEntities))
+            ->getQuery()->getResult();
+
+        self::assertCount(count($persistedEntities), $entities);
+
+        foreach ($entities as $entity) {
+            self::assertEquals($entity, $persistedEntities[$entity->id]);
+        }
+    }
+}
+
+/**
+ * @Entity
+ * @Table
+ * @InheritanceType("JOINED")
+ * @DiscriminatorColumn(name="discr", type="string")
+ * @DiscriminatorMap({
+ *      DDC6303ChildB::class = DDC6303ChildB::class,
+ *      DDC6303ChildA::class = DDC6303ChildA::class,
+ * })
+ *
+ * Note: discriminator map order *IS IMPORTANT* for this test
+ */
+abstract class DDC6303BaseClass
+{
+    /** @Id @Column(type="string") @GeneratedValue(strategy="NONE") */
+    public $id;
+}
+
+/** @Entity @Table */
+class DDC6303ChildA extends DDC6303BaseClass
+{
+    /** @Column(type="string") */
+    private $originalData;
+
+    public function __construct(string $id, $originalData)
+    {
+        $this->id           = $id;
+        $this->originalData = $originalData;
+    }
+}
+
+/** @Entity @Table */
+class DDC6303ChildB extends DDC6303BaseClass
+{
+    /** @Column(type="simple_array", nullable=true) */
+    private $originalData;
+
+    public function __construct(string $id, array $originalData)
+    {
+        $this->id           = $id;
+        $this->originalData = $originalData;
+    }
+}