Merge pull request #160 from Kharhamel/nameConflicts

better bean method names conflicts resolution

Author: Kharhamel

src/Utils/AbstractBeanPropertyDescriptor.php

@@ -10,7 +10,7 @@
 /**
  * This class represent a property in a bean (a property has a getter, a setter, etc...).
  */
-abstract class AbstractBeanPropertyDescriptor
+abstract class AbstractBeanPropertyDescriptor implements MethodDescriptorInterface
 {
     /**
      * @var Table
@@ -80,6 +80,14 @@ public function getSetterName(): string
         return $this->namingStrategy->getSetterName($this);
     }
 
+    /**
+     * Alias of the method getGetterName(). Used to validate MethodDescriptorInterface
+     */
+    public function getName(): string
+    {
+        return $this->getGetterName();
+    }
+
     public function getGetterName(): string
     {
         return $this->namingStrategy->getGetterName($this);

src/Utils/BeanDescriptor.php

@@ -110,6 +110,18 @@ class BeanDescriptor implements BeanDescriptorInterface
      * @var BeanRegistry
      */
     private $registry;
+    /**
+     * @var MethodDescriptorInterface[][]
+     */
+    private $descriptorsByMethodName = [];
+    /**
+     * @var DirectForeignKeyMethodDescriptor[]|null
+     */
+    private $directForeignKeysDescriptors = null;
+    /**
+     * @var PivotTableMethodsDescriptor[]|null
+     */
+    private $pivotTableDescriptors = null;
 
     public function __construct(
         Table $table,
@@ -144,6 +156,11 @@ public function __construct(
     public function initBeanPropertyDescriptors(): void
     {
         $this->beanPropertyDescriptors = $this->getProperties($this->table);
+
+        //init the list of method names with regular properties names
+        foreach ($this->beanPropertyDescriptors as $beanPropertyDescriptor) {
+            $this->checkForDuplicate($beanPropertyDescriptor);
+        }
     }
 
     /**
@@ -373,68 +390,82 @@ private function generateBeanConstructor() : MethodGenerator
      */
     private function getDirectForeignKeysDescriptors(): array
     {
+        if ($this->directForeignKeysDescriptors !== null) {
+            return $this->directForeignKeysDescriptors;
+        }
         $fks = $this->tdbmSchemaAnalyzer->getIncomingForeignKeys($this->table->getName());
 
         $descriptors = [];
 
         foreach ($fks as $fk) {
-            $descriptors[] = new DirectForeignKeyMethodDescriptor($fk, $this->table, $this->namingStrategy, $this->annotationParser, $this->beanNamespace);
+            $desc = new DirectForeignKeyMethodDescriptor($fk, $this->table, $this->namingStrategy, $this->annotationParser, $this->beanNamespace);
+            $this->checkForDuplicate($desc);
+            $descriptors[] = $desc;
         }
 
-        return $descriptors;
+        $this->directForeignKeysDescriptors = $descriptors;
+        return $this->directForeignKeysDescriptors;
     }
 
     /**
      * @return PivotTableMethodsDescriptor[]
      */
     private function getPivotTableDescriptors(): array
     {
+        if ($this->pivotTableDescriptors !== null) {
+            return $this->pivotTableDescriptors;
+        }
         $descs = [];
         foreach ($this->schemaAnalyzer->detectJunctionTables(true) as $table) {
             // There are exactly 2 FKs since this is a pivot table.
             $fks = array_values($table->getForeignKeys());
 
             if ($fks[0]->getForeignTableName() === $this->table->getName()) {
                 list($localFk, $remoteFk) = $fks;
-                $descs[] = new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser);
+                $desc = new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser);
+                $this->checkForDuplicate($desc);
+                $descs[] = $desc;
             }
             if ($fks[1]->getForeignTableName() === $this->table->getName()) {
                 list($remoteFk, $localFk) = $fks;
-                $descs[] = new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser);
+                $desc = new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser);
+                $this->checkForDuplicate($desc);
+                $descs[] = $desc;
             }
         }
 
-        return $descs;
+        $this->pivotTableDescriptors = $descs;
+        return $this->pivotTableDescriptors;
+    }
+
+    /**
+     * Check the method name isn't already used and flag the associated descriptors to use their alternative names if it is the case
+     */
+    private function checkForDuplicate(MethodDescriptorInterface $descriptor): void
+    {
+        $name = $descriptor->getName();
+        if (!isset($this->descriptorsByMethodName[$name])) {
+            $this->descriptorsByMethodName[$name] = [];
+        }
+        $this->descriptorsByMethodName[$name][] = $descriptor;
+        if (count($this->descriptorsByMethodName[$name]) > 1) {
+            foreach ($this->descriptorsByMethodName[$name] as $duplicateDescriptor) {
+                $duplicateDescriptor->useAlternativeName();
+            }
+        }
     }
 
     /**
      * Returns the list of method descriptors (and applies the alternative name if needed).
      *
-     * @return MethodDescriptorInterface[]
+     * @return RelationshipMethodDescriptorInterface[]
      */
     public function getMethodDescriptors(): array
     {
         $directForeignKeyDescriptors = $this->getDirectForeignKeysDescriptors();
         $pivotTableDescriptors = $this->getPivotTableDescriptors();
 
-        $descriptors = array_merge($directForeignKeyDescriptors, $pivotTableDescriptors);
-
-        // Descriptors by method names
-        $descriptorsByMethodName = [];
-
-        foreach ($descriptors as $descriptor) {
-            $descriptorsByMethodName[$descriptor->getName()][] = $descriptor;
-        }
-
-        foreach ($descriptorsByMethodName as $descriptorsForMethodName) {
-            if (count($descriptorsForMethodName) > 1) {
-                foreach ($descriptorsForMethodName as $descriptor) {
-                    $descriptor->useAlternativeName();
-                }
-            }
-        }
-
-        return $descriptors;
+        return array_merge($directForeignKeyDescriptors, $pivotTableDescriptors);
     }
 
     public function generateJsonSerialize(): MethodGenerator

src/Utils/DirectForeignKeyMethodDescriptor.php

@@ -18,7 +18,7 @@
 /**
  * Represents a method to get a list of beans from a direct foreign key pointing to our bean.
  */
-class DirectForeignKeyMethodDescriptor implements MethodDescriptorInterface
+class DirectForeignKeyMethodDescriptor implements RelationshipMethodDescriptorInterface
 {
     use ForeignKeyAnalyzerTrait;
 

src/Utils/MethodDescriptorInterface.php

@@ -3,8 +3,6 @@
 
 namespace TheCodingMachine\TDBM\Utils;
 
-use Zend\Code\Generator\MethodGenerator;
-
 interface MethodDescriptorInterface
 {
     /**
@@ -14,36 +12,8 @@ interface MethodDescriptorInterface
      */
     public function getName() : string;
 
-    /**
-     * Returns the name of the class that will be returned by the getter (short name).
-     *
-     * @return string
-     */
-    public function getBeanClassName(): string;
-
     /**
      * Requests the use of an alternative name for this method.
      */
     public function useAlternativeName(): void;
-
-    /**
-     * Returns the code of the method.
-     *
-     * @return MethodGenerator[]
-     */
-    public function getCode() : array;
-
-    /**
-     * Returns an array of classes that needs a "use" for this method.
-     *
-     * @return string[]
-     */
-    public function getUsedClasses() : array;
-
-    /**
-     * Returns the code to past in jsonSerialize.
-     *
-     * @return string
-     */
-    public function getJsonSerializeCode() : string;
 }

src/Utils/PivotTableMethodsDescriptor.php

@@ -17,7 +17,7 @@
 use Zend\Code\Generator\ParameterGenerator;
 use function var_export;
 
-class PivotTableMethodsDescriptor implements MethodDescriptorInterface
+class PivotTableMethodsDescriptor implements RelationshipMethodDescriptorInterface
 {
     /**
      * @var Table

src/Utils/RelationshipMethodDescriptorInterface.php

@@ -0,0 +1,49 @@
+<?php
+declare(strict_types=1);
+
+namespace TheCodingMachine\TDBM\Utils;
+
+use Zend\Code\Generator\MethodGenerator;
+
+interface RelationshipMethodDescriptorInterface extends MethodDescriptorInterface
+{
+    /**
+     * Returns the name of the method to be generated.
+     *
+     * @return string
+     */
+    public function getName() : string;
+
+    /**
+     * Returns the name of the class that will be returned by the getter (short name).
+     *
+     * @return string
+     */
+    public function getBeanClassName(): string;
+
+    /**
+     * Requests the use of an alternative name for this method.
+     */
+    public function useAlternativeName(): void;
+
+    /**
+     * Returns the code of the method.
+     *
+     * @return MethodGenerator[]
+     */
+    public function getCode() : array;
+
+    /**
+     * Returns an array of classes that needs a "use" for this method.
+     *
+     * @return string[]
+     */
+    public function getUsedClasses() : array;
+
+    /**
+     * Returns the code to past in jsonSerialize.
+     *
+     * @return string
+     */
+    public function getJsonSerializeCode() : string;
+}

tests/TDBMAbstractServiceTest.php

@@ -416,6 +416,7 @@ private static function initSchema(Connection $connection): void
 
         $db->table('artists')
             ->column('id')->integer()->primaryKey()->autoIncrement()
+            ->column('children')->array()->null() //used to test conflicts with autopivot
             ->column('name')->string();
 
         $db->table('albums')
@@ -437,11 +438,15 @@ private static function initSchema(Connection $connection): void
             ->column('track_id')->references('tracks')
             ->column('artist_id')->references('artists')->comment('@JsonKey("feat") @JsonInclude');
 
-        $db->table('artists_relations')
+        $db->table('artists_relations') //used to test the auto pivot case
             ->column('id')->integer()->primaryKey()->autoIncrement()
             ->column('parent_id')->references('artists')
             ->column('child_id')->references('artists');
 
+        $db->table('children') //used to test conflicts with autopivot
+            ->column('id')->integer()->primaryKey()->autoIncrement()
+            ->column('artist_id')->references('artists');
+
         $db->junctionTable('person', 'boats');
 
         $db->table('base_objects')

tests/TDBMDaoGeneratorTest.php

@@ -2008,7 +2008,7 @@ public function testFloydHasOneChild(): void
     {
         $artists = new ArtistDao($this->tdbmService);
         $pinkFloyd = $artists->getById(1);
-        $children = $pinkFloyd->getChildren();
+        $children = $pinkFloyd->getChildrenByArtistsRelations();
 
         $this->assertEquals(1, count($children));
         $this->assertEquals(2, $children[0]->getId());
@@ -2160,4 +2160,13 @@ public function testLazyStopRecursionOnCompositeForeignKey(): void
         $this->assertEquals(1, $json['compositeFkTarget']['id1']);
         $this->assertEquals(1, $json['compositeFkTarget']['id2']);
     }
+
+    public function testMethodNameConflictsBetweenRegularAndAutoPivotProperties()
+    {
+        $artist = new ArtistBean('Super');
+        $artist->getChildren(); // regular property
+        $artist->getChildrenByArtistId(); // one-to-may relationship
+        $artist->getChildrenByArtistsRelations(); // auto-pivot relationship
+        $this->assertEquals(1, 1);
+    }
 }