Merge pull request #14378 from cakephp/shared-belongstomany

4.1- Throw exception adding incompatible BelongsToMany associations

src/ORM/Association/BelongsToMany.php

@@ -253,6 +253,7 @@ class BelongsToMany extends Association
      *
      * @param string|\Cake\ORM\Table|null $table Name or instance for the join table
      * @return \Cake\ORM\Table
+     * @throws \InvalidArgumentException If the expected associations are incompatible with existing associations.
      */
     public function junction($table = null): Table
     {
@@ -389,6 +390,7 @@ class BelongsToMany extends Association
      * @param \Cake\ORM\Table $source The source table.
      * @param \Cake\ORM\Table $target The target table.
      * @return void
+     * @throws \InvalidArgumentException If the expected associations are incompatible with existing associations.
      */
     protected function _generateJunctionAssociations(Table $junction, Table $source, Table $target): void
     {
@@ -400,7 +402,19 @@ class BelongsToMany extends Association
                 'foreignKey' => $this->getTargetForeignKey(),
                 'targetTable' => $target,
             ]);
+        } else {
+            $belongsTo = $junction->getAssociation($tAlias);
+            if (
+                $this->getTargetForeignKey() !== $belongsTo->getForeignKey() ||
+                $target !== $belongsTo->getTarget()
+            ) {
+                throw new InvalidArgumentException(
+                    "The existing `{$tAlias}` association on `{$junction->getAlias()} " .
+                    "is incompatible with the `{$this->getName()}` association on `{$source->getAlias()}`"
+                );
+            }
         }
+
         if (!$junction->hasAssociation($sAlias)) {
             $junction->belongsTo($sAlias, [
                 'foreignKey' => $this->getForeignKey(),

tests/TestCase/ORM/Association/BelongsToManyTest.php

@@ -29,6 +29,7 @@ use Cake\ORM\Query;
 use Cake\ORM\RulesChecker;
 use Cake\ORM\Table;
 use Cake\TestSuite\TestCase;
+use InvalidArgumentException;
 
 /**
  * Tests BelongsToMany class
@@ -219,11 +220,13 @@ class BelongsToManyTest extends TestCase
     public function testJunctionCustomKeys()
     {
         $this->article->belongsToMany('Tags', [
+            'targetTable' => $this->tag,
             'joinTable' => 'articles_tags',
             'foreignKey' => 'article',
             'targetForeignKey' => 'tag',
         ]);
         $this->tag->belongsToMany('Articles', [
+            'targetTable' => $this->article,
             'joinTable' => 'articles_tags',
             'foreignKey' => 'tag',
             'targetForeignKey' => 'article',
@@ -255,6 +258,30 @@ class BelongsToManyTest extends TestCase
     }
 
     /**
+     * Test multiple associations with differerent keys fails
+     *
+     * @return void
+     */
+    public function testMultipleAssociationsSameJunction()
+    {
+        $assoc = new BelongsToMany('This', [
+            'sourceTable' => $this->article,
+            'targetTable' => $this->tag,
+            'targetForeignKey' => 'this_id',
+        ]);
+        $assoc->junction();
+
+        $assoc = new BelongsToMany('That', [
+            'sourceTable' => $this->article,
+            'targetTable' => $this->tag,
+            'targetForeignKey' => 'that_id',
+        ]);
+
+        $this->expectException(InvalidArgumentException::class);
+        $assoc->junction();
+    }
+
+    /**
      * Tests saveStrategy
      *
      * @return void

tests/TestCase/ORM/TableTest.php

@@ -4331,10 +4331,10 @@ class TableTest extends TestCase
      */
     public function testLinkBelongsToMany()
     {
-        $table = $this->getTableLocator()->get('articles');
-        $table->belongsToMany('tags');
-        $tagsTable = $this->getTableLocator()->get('tags');
-        $source = ['source' => 'tags'];
+        $table = $this->getTableLocator()->get('Articles');
+        $table->belongsToMany('Tags');
+        $tagsTable = $this->getTableLocator()->get('Tags');
+        $source = ['source' => 'Tags'];
         $options = ['markNew' => false];
 
         $article = new Entity([
@@ -4352,14 +4352,14 @@ class TableTest extends TestCase
         $tags[] = $newTag;
 
         $tagsTable->save($newTag);
-        $table->getAssociation('tags')->link($article, $tags);
+        $table->getAssociation('Tags')->link($article, $tags);
 
         $this->assertEquals($article->tags, $tags);
         foreach ($tags as $tag) {
             $this->assertFalse($tag->isNew());
         }
 
-        $article = $table->find('all')->where(['id' => 1])->contain(['tags'])->first();
+        $article = $table->find('all')->where(['id' => 1])->contain(['Tags'])->first();
         $this->assertEquals($article->tags[2]->id, $tags[0]->id);
         $this->assertEquals($article->tags[3], $tags[1]);
     }
@@ -4863,16 +4863,16 @@ class TableTest extends TestCase
      */
     public function testUnlinkBelongsToMany()
     {
-        $table = $this->getTableLocator()->get('articles');
-        $table->belongsToMany('tags');
-        $tagsTable = $this->getTableLocator()->get('tags');
+        $table = $this->getTableLocator()->get('Articles');
+        $table->belongsToMany('Tags');
+        $tagsTable = $this->getTableLocator()->get('Tags');
         $options = ['markNew' => false];
 
         $article = $table->find('all')
             ->where(['id' => 1])
             ->contain(['tags'])->first();
 
-        $table->getAssociation('tags')->unlink($article, [$article->tags[0]]);
+        $table->getAssociation('Tags')->unlink($article, [$article->tags[0]]);
         $this->assertCount(1, $article->tags);
         $this->assertEquals(2, $article->tags[0]->get('id'));
         $this->assertFalse($article->isDirty('tags'));
@@ -4885,17 +4885,17 @@ class TableTest extends TestCase
      */
     public function testUnlinkBelongsToManyMultiple()
     {
-        $table = $this->getTableLocator()->get('articles');
-        $table->belongsToMany('tags');
-        $tagsTable = $this->getTableLocator()->get('tags');
+        $table = $this->getTableLocator()->get('Articles');
+        $table->belongsToMany('Tags');
+        $tagsTable = $this->getTableLocator()->get('Tags');
         $options = ['markNew' => false];
 
         $article = new Entity(['id' => 1], $options);
         $tags[] = new \TestApp\Model\Entity\Tag(['id' => 1], $options);
         $tags[] = new \TestApp\Model\Entity\Tag(['id' => 2], $options);
 
-        $table->getAssociation('tags')->unlink($article, $tags);
-        $left = $table->find('all')->where(['id' => 1])->contain(['tags'])->first();
+        $table->getAssociation('Tags')->unlink($article, $tags);
+        $left = $table->find('all')->where(['id' => 1])->contain(['Tags'])->first();
         $this->assertEmpty($left->tags);
     }
 
@@ -4907,9 +4907,9 @@ class TableTest extends TestCase
      */
     public function testUnlinkBelongsToManyPassingJoint()
     {
-        $table = $this->getTableLocator()->get('articles');
-        $table->belongsToMany('tags');
-        $tagsTable = $this->getTableLocator()->get('tags');
+        $table = $this->getTableLocator()->get('Articles');
+        $table->belongsToMany('Tags');
+        $tagsTable = $this->getTableLocator()->get('Tags');
         $options = ['markNew' => false];
 
         $article = new Entity(['id' => 1], $options);
@@ -4921,8 +4921,8 @@ class TableTest extends TestCase
             'tag_id' => 2,
         ], $options);
 
-        $table->getAssociation('tags')->unlink($article, $tags);
-        $left = $table->find('all')->where(['id' => 1])->contain(['tags'])->first();
+        $table->getAssociation('Tags')->unlink($article, $tags);
+        $left = $table->find('all')->where(['id' => 1])->contain(['Tags'])->first();
         $this->assertEmpty($left->tags);
     }