Skip to content

Commit 8117e54

Browse files
gechetsprgechetspr
authored
Merge pull request #1963 from mringler/bugfix/avoid_cross_ref_relation_name_collisions
avoid name collision in cross ref relations
2 parents 4f36eea + 03ff801 commit 8117e54

File tree

4 files changed

+150
-22
lines changed

4 files changed

+150
-22
lines changed

‎src/Propel/Generator/Builder/Om/AbstractOMBuilder.php‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -686,34 +686,43 @@ public function getFKPhpNameAffix(ForeignKey $fk, bool $plural = false): string
686686
* @return string
687687
*/
688688
protected function getCrossFKsPhpNameAffix(CrossForeignKeys $crossFKs, bool $plural = true): string
689+
{
690+
$baseName = $this->buildCombineCrossFKsPhpNameAffix($crossFKs, false);
691+
692+
$existingTable = $this->getDatabase()->getTableByPhpName($baseName);
693+
$isNameCollision = $existingTable && $this->getTable()->isConnectedWithTable($existingTable);
694+
695+
return ($plural || $isNameCollision) ? $this->buildCombineCrossFKsPhpNameAffix($crossFKs, $plural, $isNameCollision) : $baseName;
696+
}
697+
698+
/**
699+
* @param \Propel\Generator\Model\CrossForeignKeys $crossFKs
700+
* @param bool $plural
701+
* @param bool $withPrefix
702+
*
703+
* @return string
704+
*/
705+
protected function buildCombineCrossFKsPhpNameAffix(CrossForeignKeys $crossFKs, bool $plural = true, bool $withPrefix = false): string
689706
{
690707
$names = [];
708+
if ($withPrefix) {
709+
$names[] = 'Cross';
710+
}
711+
$fks = $crossFKs->getCrossForeignKeys();
712+
$lastCrossFk = array_pop($fks);
713+
$unclassifiedPrimaryKeys = $crossFKs->getUnclassifiedPrimaryKeys();
714+
$lastIsPlural = $plural && !$unclassifiedPrimaryKeys;
691715

692-
if ($plural) {
693-
if ($crossFKs->getUnclassifiedPrimaryKeys()) {
694-
//we have a non fk as pk as well, so we need to make pluralisation on our own and can't
695-
//rely on getFKPhpNameAffix's pluralisation
696-
foreach ($crossFKs->getCrossForeignKeys() as $fk) {
697-
$names[] = $this->getFKPhpNameAffix($fk, false);
698-
}
699-
} else {
700-
//we have only fks, so give us names with plural and return those
701-
$lastIdx = count($crossFKs->getCrossForeignKeys()) - 1;
702-
foreach ($crossFKs->getCrossForeignKeys() as $idx => $fk) {
703-
$needPlural = $idx === $lastIdx; //only last fk should be plural
704-
$names[] = $this->getFKPhpNameAffix($fk, $needPlural);
705-
}
716+
foreach ($fks as $fk) {
717+
$names[] = $this->getFKPhpNameAffix($fk, false);
718+
}
719+
$names[] = $this->getFKPhpNameAffix($lastCrossFk, $lastIsPlural);
706720

707-
return implode('', $names);
708-
}
709-
} else {
710-
// no plural, so $plural=false
711-
foreach ($crossFKs->getCrossForeignKeys() as $fk) {
712-
$names[] = $this->getFKPhpNameAffix($fk, false);
713-
}
721+
if (!$unclassifiedPrimaryKeys) {
722+
return implode('', $names);
714723
}
715724

716-
foreach ($crossFKs->getUnclassifiedPrimaryKeys() as $pk) {
725+
foreach ($unclassifiedPrimaryKeys as $pk) {
717726
$names[] = $pk->getPhpName();
718727
}
719728

‎src/Propel/Generator/Model/Table.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,4 +2288,18 @@ public function getAdditionalModelClassImports(): ?array
22882288

22892289
return null;
22902290
}
2291+
2292+
/**
2293+
* Check if there is a FK rellation between the current table and the given
2294+
* table in either direction.
2295+
*
2296+
* @param \Propel\Generator\Model\Table $table
2297+
*
2298+
* @return bool
2299+
*/
2300+
public function isConnectedWithTable(Table $table): bool
2301+
{
2302+
return $this->getForeignKeysReferencingTable($table->getName()) ||
2303+
$table->getForeignKeysReferencingTable($this->getName());
2304+
}
22912305
}

‎tests/Propel/Tests/Issues/Issue941Test.php‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
<?php
2+
3+
/**
4+
* MIT License. This file is part of the Propel package.
5+
* For the full copyright and license information, please view the LICENSE
6+
* file that was distributed with this source code.
7+
*/
8+
9+
namespace Propel\Tests\Issues;
10+
11+
use Nature;
12+
use Propel\Generator\Util\QuickBuilder;
13+
use Propel\Runtime\Collection\ObjectCollection;
14+
use Propel\Tests\TestCase;
15+
use Recherche;
16+
use Category;
17+
use RechercheNature;
18+
19+
/**
20+
* This test proves the bug described in https://github.com/propelorm/Propel2/issues/941.
21+
*
22+
* @group database
23+
*/
24+
class Issue941Test extends TestCase
25+
{
26+
/**
27+
* @return void
28+
*/
29+
protected function setUp(): void
30+
{
31+
parent::setUp();
32+
if (!class_exists('\Nature')) {
33+
$schema = '
34+
<database>
35+
<table name="recherche" phpName="Recherche">
36+
<column name="id" type="integer" primaryKey="true" autoIncrement="true"/>
37+
</table>
38+
39+
<table name="category" phpName="Category">
40+
<column name="id" type="integer" primaryKey="true" autoIncrement="true"/>
41+
</table>
42+
43+
<table name="recherche_nature" phpName="RechercheNature" isCrossRef="true">
44+
<column name="recherche_id" type="integer" primaryKey="true"/>
45+
<column name="nature_id" type="integer" primaryKey="true"/>
46+
<column name="category_id" type="integer"/>
47+
48+
<foreign-key foreignTable="recherche" onDelete="cascade">
49+
<reference local="recherche_id" foreign="id"/>
50+
</foreign-key>
51+
<foreign-key foreignTable="nature">
52+
<reference local="nature_id" foreign="id"/>
53+
</foreign-key>
54+
55+
<foreign-key foreignTable="category">
56+
<reference local="category_id" foreign="id"/>
57+
</foreign-key>
58+
59+
</table>
60+
61+
<table name="nature" phpName="Nature">
62+
<column name="id" type="integer" primaryKey="true" autoIncrement="true"/>
63+
</table>
64+
65+
</database>
66+
';
67+
QuickBuilder::buildSchema($schema);
68+
}
69+
}
70+
71+
/**
72+
* @return void
73+
*/
74+
public function testIssue941()
75+
{
76+
$nature = new Nature();
77+
$nature->save();
78+
79+
$category = new Category();
80+
$category->save();
81+
82+
// RechercheNature
83+
$rechercheNature = new RechercheNature();
84+
$rechercheNature->setNatureId($nature->getId());
85+
$rechercheNature->setCategoryId($category->getId());
86+
87+
// Collection
88+
$collection = new ObjectCollection();
89+
$collection->setModel('\RechercheNature');
90+
$collection->setData([$rechercheNature]);
91+
92+
// Recherche
93+
$recherche = new Recherche();
94+
$recherche->setRechercheNatures($collection);
95+
96+
$countBeforeSave = $recherche->countRechercheNatures();
97+
98+
$recherche->save();
99+
100+
$countAfterSave = $recherche->countRechercheNatures();
101+
102+
$this->assertEquals($countBeforeSave, $countAfterSave);
103+
}
104+
}

‎tests/Propel/Tests/Issues/Issue989Test.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ protected function setUp(): void
3838
<table name="recherche_nature" phpName="RechercheNature" isCrossRef="true">
3939
<column name="recherche_id" type="integer" primaryKey="true"/>
4040
<column name="nature_id" type="integer" primaryKey="true"/>
41+
<column name="category_id" type="integer"/>
4142
<foreign-key foreignTable="recherche" onDelete="cascade">
4243
<reference local="recherche_id" foreign="id"/>
4344
</foreign-key>

0 commit comments

Comments
 (0)