Merge pull request #10747 from wtfzdotnet/feature/fix-one-to-many-custom-id-orphan-removal

Author: greg0ire

lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php

@@ -16,6 +16,7 @@
 use function array_values;
 use function assert;
 use function implode;
+use function is_int;
 use function is_string;
 
 /**
@@ -174,16 +175,22 @@ private function deleteEntityCollection(PersistentCollection $collection): int
         $targetClass = $this->em->getClassMetadata($mapping['targetEntity']);
         $columns     = [];
         $parameters  = [];
+        $types       = [];
 
         foreach ($targetClass->associationMappings[$mapping['mappedBy']]['joinColumns'] as $joinColumn) {
             $columns[]    = $this->quoteStrategy->getJoinColumnName($joinColumn, $targetClass, $this->platform);
             $parameters[] = $identifier[$sourceClass->getFieldForColumn($joinColumn['referencedColumnName'])];
+            $types[]      = PersisterHelper::getTypeOfColumn($joinColumn['referencedColumnName'], $sourceClass, $this->em);
         }
 
         $statement = 'DELETE FROM ' . $this->quoteStrategy->getTableName($targetClass, $this->platform)
             . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';
 
-        return $this->conn->executeStatement($statement, $parameters);
+        $numAffected = $this->conn->executeStatement($statement, $parameters, $types);
+
+        assert(is_int($numAffected));
+
+        return $numAffected;
     }
 
     /**
@@ -247,6 +254,8 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):
 
         $this->conn->executeStatement($statement);
 
+        assert(is_int($numDeleted));
+
         return $numDeleted;
     }
 }

psalm-baseline.xml

@@ -1244,14 +1244,6 @@
                 $mapping['indexBy']  => $index,
             ]]]></code>
     </InvalidArrayOffset>
-    <InvalidReturnStatement>
-      <code>$numDeleted</code>
-      <code><![CDATA[$this->conn->executeStatement($statement, $parameters)]]></code>
-    </InvalidReturnStatement>
-    <InvalidReturnType>
-      <code>int</code>
-      <code>int</code>
-    </InvalidReturnType>
     <PossiblyNullArgument>
       <code><![CDATA[$collection->getOwner()]]></code>
       <code><![CDATA[$collection->getOwner()]]></code>

tests/Doctrine/Tests/ORM/Functional/GH10747Test.php

@@ -0,0 +1,195 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Types\Type as DBALType;
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\ORM\Mapping\Column;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\Id;
+use Doctrine\ORM\Mapping\Table;
+use Doctrine\Tests\DbalTypes\CustomIdObject;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+use function method_exists;
+use function str_replace;
+
+/**
+ * Functional tests for asserting that orphaned children in a OneToMany relationship get removed with a custom identifier
+ *
+ * @group GH10747
+ */
+final class GH10747Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if (! DBALType::hasType(GH10747CustomIdObjectHashType::class)) {
+            DBALType::addType(GH10747CustomIdObjectHashType::class, GH10747CustomIdObjectHashType::class);
+        }
+
+        $this->setUpEntitySchema([GH10747Article::class, GH10747Credit::class]);
+    }
+
+    public function testOrphanedOneToManyDeletesCollection(): void
+    {
+        $object = new GH10747Article(
+            new CustomIdObject('article')
+        );
+
+        $creditOne = new GH10747Credit(
+            $object,
+            'credit1'
+        );
+
+        $creditTwo = new GH10747Credit(
+            $object,
+            'credit2'
+        );
+
+        $object->setCredits(new ArrayCollection([$creditOne, $creditTwo]));
+
+        $this->_em->persist($object);
+        $this->_em->persist($creditOne);
+        $this->_em->persist($creditTwo);
+        $this->_em->flush();
+
+        $id = $object->id;
+
+        $object2 = $this->_em->find(GH10747Article::class, $id);
+
+        $creditThree = new GH10747Credit(
+            $object2,
+            'credit3'
+        );
+
+        $object2->setCredits(new ArrayCollection([$creditThree]));
+
+        $this->_em->persist($object2);
+        $this->_em->persist($creditThree);
+        $this->_em->flush();
+
+        $currentDatabaseCredits = $this->_em->createQueryBuilder()
+            ->select('c.id')
+            ->from(GH10747Credit::class, 'c')
+            ->getQuery()
+            ->execute();
+
+        self::assertCount(1, $currentDatabaseCredits);
+    }
+}
+
+/**
+ * @Entity
+ * @Table
+ */
+class GH10747Article
+{
+    /**
+     * @Id
+     * @Column(type="Doctrine\Tests\ORM\Functional\GH10747CustomIdObjectHashType")
+     * @var CustomIdObject
+     */
+    public $id;
+
+    /**
+     * @ORM\OneToMany(targetEntity="GH10747Credit", mappedBy="article", orphanRemoval=true)
+     *
+     * @var Collection<int, GH10747Credit>
+     */
+    public $credits;
+
+    public function __construct(CustomIdObject $id)
+    {
+        $this->id      = $id;
+        $this->credits = new ArrayCollection();
+    }
+
+    public function setCredits(Collection $credits): void
+    {
+        $this->credits = $credits;
+    }
+
+    /** @return Collection<int, GH10747Credit> */
+    public function getCredits(): Collection
+    {
+        return $this->credits;
+    }
+}
+
+/**
+ * @Entity
+ * @Table
+ */
+class GH10747Credit
+{
+    /**
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue()
+     *
+     * @Id()
+     * @var int|null
+     */
+    public $id = null;
+
+    /** @var string */
+    public $name;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="GH10747Article", inversedBy="credits")
+     *
+     * @var GH10747Article
+     */
+    public $article;
+
+    public function __construct(GH10747Article $article, string $name)
+    {
+        $this->article = $article;
+        $this->name    = $name;
+    }
+}
+
+class GH10747CustomIdObjectHashType 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)
+    {
+        if (method_exists($platform, 'getStringTypeDeclarationSQL')) {
+            return $platform->getStringTypeDeclarationSQL($fieldDeclaration);
+        }
+
+        return $platform->getVarcharTypeDeclarationSQL($fieldDeclaration);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function getName()
+    {
+        return self::class;
+    }
+}