Move joinTableColumns down

This field only makes sense for the owning side of many-to-many mappings. Moving it down allows us to make it non-nullable.
derrabus · Apr 17, 2023 · 49cc83d · 49cc83d
1 parent ada109b
commit 49cc83d
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 15 deletions.
diff --git a/lib/Doctrine/ORM/Mapping/AssociationMapping.php b/lib/Doctrine/ORM/Mapping/AssociationMapping.php
@@ -81,9 +81,6 @@ abstract class AssociationMapping implements ArrayAccess
     /** @var array<string, string>|null */
     public array|null $joinColumnFieldNames = null;
 
-    /** @var list<mixed>|null */
-    public array|null $joinTableColumns = null;
-
     /** @var class-string|null */
     public string|null $originalClass = null;
 
@@ -336,7 +333,6 @@ public function __sleep(): array
                 'declared',
                 'cache',
                 'joinColumnFieldNames',
-                'joinTableColumns',
                 'originalClass',
                 'originalField',
             ] as $stringOrArrayProperty

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadata.php b/lib/Doctrine/ORM/Mapping/ClassMetadata.php
@@ -1722,7 +1722,6 @@ public function setAssociationOverride(string $fieldName, array $overrideMapping
         }
 
         $mapping['joinColumnFieldNames'] = null;
-        $mapping['joinTableColumns']     = null;
 
         switch ($mapping['type']) {
             case self::ONE_TO_ONE:

diff --git a/lib/Doctrine/ORM/Mapping/ManyToManyOwningSideMapping.php b/lib/Doctrine/ORM/Mapping/ManyToManyOwningSideMapping.php
@@ -17,6 +17,9 @@ final class ManyToManyOwningSideMapping extends ManyToManyAssociationMapping imp
      */
     public JoinTableMapping $joinTable;
 
+    /** @var list<mixed> */
+    public array $joinTableColumns = [];
+
     /** @return array<string, mixed> */
     public function toArray(): array
     {
@@ -130,6 +133,7 @@ public function __sleep(): array
     {
         $serialized   = parent::__sleep();
         $serialized[] = 'joinTable';
+        $serialized[] = 'joinTableColumns';
 
         return $serialized;
     }

diff --git a/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
@@ -17,6 +17,7 @@
 
 use function array_fill;
 use function array_pop;
+use function assert;
 use function count;
 use function implode;
 use function in_array;
@@ -501,9 +502,10 @@ private function collectJoinTableColumnParameters(
         PersistentCollection $collection,
         object $element,
     ): array {
-        $params      = [];
-        $mapping     = $collection->getMapping();
-        $isComposite = count($mapping['joinTableColumns']) > 2;
+        $params  = [];
+        $mapping = $collection->getMapping();
+        assert($mapping->isManyToManyOwningSide());
+        $isComposite = count($mapping->joinTableColumns) > 2;
 
         $identifier1 = $this->uow->getEntityIdentifier($collection->getOwner());
         $identifier2 = $this->uow->getEntityIdentifier($element);
@@ -514,7 +516,7 @@ private function collectJoinTableColumnParameters(
             $class2 = $collection->getTypeClass();
         }
 
-        foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
+        foreach ($mapping->joinTableColumns as $joinTableColumn) {
             $isRelationToSource = isset($mapping['relationToSourceKeyColumns'][$joinTableColumn]);
 
             if (! $isComposite) {
@@ -593,7 +595,8 @@ private function getJoinTableRestrictionsWithKey(
             $types[]        = PersisterHelper::getTypeOfColumn($columnName, $targetClass, $this->em);
         }
 
-        foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
+        assert($mapping->isManyToManyOwningSide());
+        foreach ($mapping->joinTableColumns as $joinTableColumn) {
             if (isset($mapping[$sourceRelationMode][$joinTableColumn])) {
                 $column         = $mapping[$sourceRelationMode][$joinTableColumn];
                 $whereClauses[] = 't.' . $joinTableColumn . ' = ?';
@@ -659,7 +662,8 @@ private function getJoinTableRestrictions(
         $params          = [];
         $types           = [];
 
-        foreach ($mapping['joinTableColumns'] as $joinTableColumn) {
+        assert($mapping->isManyToManyOwningSide());
+        foreach ($mapping->joinTableColumns as $joinTableColumn) {
             $whereClauses[] = ($addFilters ? 't.' : '') . $joinTableColumn . ' = ?';
 
             if (isset($mapping['relationToTargetKeyColumns'][$joinTableColumn])) {

diff --git a/phpstan.neon b/phpstan.neon
@@ -29,6 +29,10 @@ parameters:
             message: "#^Access to an undefined property Doctrine\\\\ORM\\\\Mapping\\\\ToOneAssociationMapping\\:\\:\\$joinColumns\\.$#"
             path: lib/Doctrine/ORM/Mapping/ToOneAssociationMapping.php
 
+        -
+            message: "#^Access to an undefined property Doctrine\\\\ORM\\\\Mapping\\\\AssociationMapping\\:\\:\\$joinTableColumns\\.$#"
+            path: lib/Doctrine/ORM/Persisters/Collection/ManyToManyPersister.php
+
         -
             message: "#^Method Doctrine\\\\ORM\\\\Mapping\\\\ToOneAssociationMapping\\:\\:fromMappingArrayAndName\\(\\) should return Doctrine\\\\ORM\\\\Mapping\\\\ManyToOneAssociationMapping\\|Doctrine\\\\ORM\\\\Mapping\\\\OneToOneAssociationMapping but returns static\\(Doctrine\\\\ORM\\\\Mapping\\\\ToOneAssociationMapping\\)\\.$#"
             path: lib/Doctrine/ORM/Mapping/ToOneAssociationMapping.php

diff --git a/tests/Doctrine/Tests/ORM/Mapping/AssociationMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/AssociationMappingTest.php
@@ -33,7 +33,6 @@ public function testItSurvivesSerialization(): void
         $mapping->id                   = true;
         $mapping->isOnDeleteCascade    = true;
         $mapping->joinColumnFieldNames = ['foo' => 'bar'];
-        $mapping->joinTableColumns     = ['foo', 'bar'];
         $mapping->originalClass        = self::class;
         $mapping->originalField        = 'foo';
         $mapping->orphanRemoval        = true;
@@ -52,7 +51,6 @@ public function testItSurvivesSerialization(): void
         self::assertTrue($resurrectedMapping->id);
         self::assertTrue($resurrectedMapping->isOnDeleteCascade);
         self::assertSame(['foo' => 'bar'], $resurrectedMapping->joinColumnFieldNames);
-        self::assertSame(['foo', 'bar'], $resurrectedMapping->joinTableColumns);
         self::assertSame(self::class, $resurrectedMapping->originalClass);
         self::assertSame('foo', $resurrectedMapping->originalField);
         self::assertTrue($resurrectedMapping->orphanRemoval);

diff --git a/tests/Doctrine/Tests/ORM/Mapping/ManyToManyOwningSideMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/ManyToManyOwningSideMappingTest.php
@@ -22,12 +22,14 @@ public function testItSurvivesSerialization(): void
             targetEntity: self::class,
         );
 
-        $mapping->joinTable       = new JoinTableMapping();
-        $mapping->joinTable->name = 'bar';
+        $mapping->joinTable        = new JoinTableMapping();
+        $mapping->joinTable->name  = 'bar';
+        $mapping->joinTableColumns = ['foo', 'bar'];
 
         $resurrectedMapping = unserialize(serialize($mapping));
         assert($resurrectedMapping instanceof ManyToManyOwningSideMapping);
 
         self::assertSame($resurrectedMapping->joinTable->name, 'bar');
+        self::assertSame(['foo', 'bar'], $resurrectedMapping->joinTableColumns);
     }
 }