Merge pull request #7322 from dennisenderink/fix/joinedsubclasspersis…

…ter-pass-identifier-types-on-delete JoinedSubclassPersister pass identifier types on delete
doctrine · Sep 20, 2019 · b52ef5a · b52ef5a
2 parents d0e1da8 + ef783f7
commit b52ef5a
Show file tree

Hide file tree

Showing 3 changed files with 143 additions and 23 deletions.
diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php
@@ -37,7 +37,9 @@
 use Doctrine\ORM\UnitOfWork;
 use Doctrine\ORM\Utility\IdentifierFlattener;
 use Doctrine\ORM\Utility\PersisterHelper;
+use function array_map;
 use function array_merge;
+use function assert;
 use function reset;
 
 /**
@@ -569,29 +571,12 @@ protected function deleteJoinTableRecords($identifier)
      */
     public function delete($entity)
     {
-        $self       = $this;
         $class      = $this->class;
         $identifier = $this->em->getUnitOfWork()->getEntityIdentifier($entity);
         $tableName  = $this->quoteStrategy->getTableName($class, $this->platform);
         $idColumns  = $this->quoteStrategy->getIdentifierColumnNames($class, $this->platform);
         $id         = array_combine($idColumns, $identifier);
-        $types      = array_map(function ($identifier) use ($class, $self) {
-            if (isset($class->fieldMappings[$identifier])) {
-                return $class->fieldMappings[$identifier]['type'];
-            }
-
-            $targetMapping = $self->em->getClassMetadata($class->associationMappings[$identifier]['targetEntity']);
-
-            if (isset($targetMapping->fieldMappings[$targetMapping->identifier[0]])) {
-                return $targetMapping->fieldMappings[$targetMapping->identifier[0]]['type'];
-            }
-
-            if (isset($targetMapping->associationMappings[$targetMapping->identifier[0]])) {
-                return $targetMapping->associationMappings[$targetMapping->identifier[0]]['type'];
-            }
-
-            throw ORMException::unrecognizedField($targetMapping->identifier[0]);
-        }, $class->identifier);
+        $types      = $this->getClassIdentifiersTypes($class);
 
         $this->deleteJoinTableRecords($identifier);
 
@@ -2089,4 +2074,22 @@ protected function switchPersisterContext($offset, $limit)
 
         $this->currentPersisterContext = $this->limitsHandlingContext;
     }
+
+    /**
+     * @return string[]
+     */
+    protected function getClassIdentifiersTypes(ClassMetadata $class) : array
+    {
+        $entityManager = $this->em;
+
+        return array_map(
+            static function ($fieldName) use ($class, $entityManager) : string {
+                $types = PersisterHelper::getTypeOfField($fieldName, $class, $entityManager);
+                assert(isset($types[0]));
+
+                return $types[0];
+            },
+            $class->identifier
+        );
+    }
 }
diff --git a/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/Entity/JoinedSubclassPersister.php
@@ -278,22 +278,25 @@ public function delete($entity)
         // If the database platform supports FKs, just
         // delete the row from the root table. Cascades do the rest.
         if ($this->platform->supportsForeignKeyConstraints()) {
-            $rootClass  = $this->em->getClassMetadata($this->class->rootEntityName);
-            $rootTable  = $this->quoteStrategy->getTableName($rootClass, $this->platform);
+            $rootClass = $this->em->getClassMetadata($this->class->rootEntityName);
+            $rootTable = $this->quoteStrategy->getTableName($rootClass, $this->platform);
+            $rootTypes = $this->getClassIdentifiersTypes($rootClass);
 
-            return (bool) $this->conn->delete($rootTable, $id);
+            return (bool) $this->conn->delete($rootTable, $id, $rootTypes);
         }
 
         // Delete from all tables individually, starting from this class' table up to the root table.
         $rootTable = $this->quoteStrategy->getTableName($this->class, $this->platform);
+        $rootTypes = $this->getClassIdentifiersTypes($this->class);
 
-        $affectedRows = $this->conn->delete($rootTable, $id);
+        $affectedRows = $this->conn->delete($rootTable, $id, $rootTypes);
 
         foreach ($this->class->parentClasses as $parentClass) {
             $parentMetadata = $this->em->getClassMetadata($parentClass);
             $parentTable    = $this->quoteStrategy->getTableName($parentMetadata, $this->platform);
+            $parentTypes    = $this->getClassIdentifiersTypes($parentMetadata);
 
-            $this->conn->delete($parentTable, $id);
+            $this->conn->delete($parentTable, $id, $parentTypes);
         }
 
         return (bool) $affectedRows;

diff --git a/tests/Doctrine/Tests/ORM/Functional/GH5988Test.php b/tests/Doctrine/Tests/ORM/Functional/GH5988Test.php
@@ -0,0 +1,114 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Types\Type as DBALType;
+use Doctrine\Tests\DbalTypes\CustomIdObject;
+use Doctrine\Tests\OrmFunctionalTestCase;
+use function str_replace;
+
+/**
+ * Functional tests for the Class Table Inheritance mapping strategy with custom id object types.
+ *
+ * @group GH5988
+ */
+final class GH5988Test extends OrmFunctionalTestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+
+        if (! DBALType::hasType(GH5988CustomIdObjectHashType::class)) {
+            DBALType::addType(GH5988CustomIdObjectHashType::class, GH5988CustomIdObjectHashType::class);
+        }
+
+        $this->setUpEntitySchema([GH5988CustomIdObjectTypeParent::class, GH5988CustomIdObjectTypeChild::class]);
+    }
+
+    public function testDelete()
+    {
+        $object = new GH5988CustomIdObjectTypeChild(new CustomIdObject('foo'), 'Test');
+
+        $this->_em->persist($object);
+        $this->_em->flush();
+
+        $id = $object->id;
+
+        $object2 = $this->_em->find(GH5988CustomIdObjectTypeChild::class, $id);
+
+        $this->_em->remove($object2);
+        $this->_em->flush();
+
+        self::assertNull($this->_em->find(GH5988CustomIdObjectTypeChild::class, $id));
+    }
+}
+
+
+class GH5988CustomIdObjectHashType extends DBALType
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function convertToDatabaseValue($value, AbstractPlatform $platform)
+    {
+        return $value->id . '_test';
+    }
+    /**
+     * {@inheritdoc}
+     */
+    public function convertToPHPValue($value, AbstractPlatform $platform)
+    {
+        return new CustomIdObject(str_replace('_test', '', $value));
+    }
+    /**
+     * {@inheritdoc}
+     */
+    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
+    {
+        return $platform->getVarcharTypeDeclarationSQL($fieldDeclaration);
+    }
+    /**
+     * {@inheritdoc}
+     */
+    public function getName()
+    {
+        return self::class;
+    }
+}
+
+/**
+ * @Entity
+ * @Table
+ * @InheritanceType("JOINED")
+ * @DiscriminatorColumn(name="type", type="string")
+ * @DiscriminatorMap({"child" = GH5988CustomIdObjectTypeChild::class})
+ */
+abstract class GH5988CustomIdObjectTypeParent
+{
+    /**
+     * @Id
+     * @Column(type="Doctrine\Tests\ORM\Functional\GH5988CustomIdObjectHashType")
+     * @var CustomIdObject
+     */
+    public $id;
+}
+
+
+/**
+ * @Entity
+ * @Table
+ */
+class GH5988CustomIdObjectTypeChild extends GH5988CustomIdObjectTypeParent
+{
+    /** @var string */
+    public $name;
+
+    public function __construct(CustomIdObject $id, string $name)
+    {
+        $this->id   = $id;
+        $this->name = $name;
+    }
+}