From 7afa37ab15b7f19f076266b96d0fee90c7d5f018 Mon Sep 17 00:00:00 2001 From: kharhamel Date: Tue, 13 Aug 2019 10:39:24 +0200 Subject: [PATCH 1/4] added a test for a conflict case --- tests/TDBMAbstractServiceTest.php | 1 + tests/TDBMDaoGeneratorTest.php | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php index ba69640a..b3391739 100644 --- a/tests/TDBMAbstractServiceTest.php +++ b/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() ->column('name')->string(); $db->table('albums') diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php index c037d832..15383cb4 100644 --- a/tests/TDBMDaoGeneratorTest.php +++ b/tests/TDBMDaoGeneratorTest.php @@ -2160,4 +2160,12 @@ 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(); + $artist->getChildrenByArtistsRelations(); + $this->assertEquals(1, 1); + } } From 97cfa5b4e4485e10fe617e55f6c5118a2c9c71d3 Mon Sep 17 00:00:00 2001 From: kharhamel Date: Tue, 13 Aug 2019 11:19:44 +0200 Subject: [PATCH 2/4] improved dulicate names detection in bean descriptor --- src/Utils/BeanDescriptor.php | 83 ++++++++++++++++++++++++---------- tests/TDBMDaoGeneratorTest.php | 2 +- 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/Utils/BeanDescriptor.php b/src/Utils/BeanDescriptor.php index ac9e0230..1ab1d8d8 100644 --- a/src/Utils/BeanDescriptor.php +++ b/src/Utils/BeanDescriptor.php @@ -110,6 +110,18 @@ class BeanDescriptor implements BeanDescriptorInterface * @var BeanRegistry */ private $registry; + /** + * @var int[] + */ + private $descriptorsByMethodName = []; + /** + * @var DirectForeignKeyMethodDescriptor[]|null + */ + private $directForeignKeysDescriptors = null; + /** + * @var PivotTableMethodsDescriptor[]|null + */ + private $pivotTableDescriptors = null; public function __construct( Table $table, @@ -144,6 +156,18 @@ 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) { + $name = $beanPropertyDescriptor->getGetterName(); + if (!isset($this->descriptorsByMethodName[$name])) { + $this->descriptorsByMethodName[$name] = 0; + } + $this->descriptorsByMethodName[$name] ++; + if ($this->descriptorsByMethodName[$name] > 1) { + $beanPropertyDescriptor->useAlternativeName(); + } + } } /** @@ -373,15 +397,21 @@ 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); + /** @var DirectForeignKeyMethodDescriptor $desc */ + $desc = $this->checkForDuplicate(new DirectForeignKeyMethodDescriptor($fk, $this->table, $this->namingStrategy, $this->annotationParser, $this->beanNamespace)); + $descriptors[] = $desc; } - return $descriptors; + $this->directForeignKeysDescriptors = $descriptors; + return $this->directForeignKeysDescriptors; } /** @@ -389,6 +419,9 @@ private function getDirectForeignKeysDescriptors(): array */ 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. @@ -396,15 +429,36 @@ private function getPivotTableDescriptors(): array if ($fks[0]->getForeignTableName() === $this->table->getName()) { list($localFk, $remoteFk) = $fks; - $descs[] = new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser); + /** @var PivotTableMethodsDescriptor $desc */ + $desc = $this->checkForDuplicate(new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser)); + $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); + /** @var PivotTableMethodsDescriptor $desc */ + $desc = $this->checkForDuplicate(new PivotTableMethodsDescriptor($table, $localFk, $remoteFk, $this->namingStrategy, $this->beanNamespace, $this->annotationParser)); + $descs[] = $desc; } } - return $descs; + $this->pivotTableDescriptors = $descs; + return $this->pivotTableDescriptors; + } + + /** + * Check the method name isn't already used and flag the $descriptor to use its alternative name if it is the case + */ + public function checkForDuplicate(MethodDescriptorInterface $descriptor): MethodDescriptorInterface + { + $name = $descriptor->getName(); + if (!isset($this->descriptorsByMethodName[$name])) { + $this->descriptorsByMethodName[$name] = 0; + } + $this->descriptorsByMethodName[$name] ++; + if ($this->descriptorsByMethodName[$name] > 1) { + $descriptor->useAlternativeName(); + } + return $descriptor; } /** @@ -417,24 +471,7 @@ 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 diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php index 15383cb4..b2e39fcf 100644 --- a/tests/TDBMDaoGeneratorTest.php +++ b/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()); From 58bdc11e2ced3db4127f837352e6f12dbb462baa Mon Sep 17 00:00:00 2001 From: kharhamel Date: Tue, 13 Aug 2019 15:17:52 +0200 Subject: [PATCH 3/4] DAN feedback --- src/Utils/AbstractBeanPropertyDescriptor.php | 10 +++- src/Utils/BeanDescriptor.php | 40 +++++++-------- .../DirectForeignKeyMethodDescriptor.php | 2 +- src/Utils/MethodDescriptorInterface.php | 30 ------------ src/Utils/PivotTableMethodsDescriptor.php | 2 +- .../RelationshipMethodDescriptorInterface.php | 49 +++++++++++++++++++ 6 files changed, 77 insertions(+), 56 deletions(-) create mode 100644 src/Utils/RelationshipMethodDescriptorInterface.php diff --git a/src/Utils/AbstractBeanPropertyDescriptor.php b/src/Utils/AbstractBeanPropertyDescriptor.php index a4fedae6..cd8e7c01 100644 --- a/src/Utils/AbstractBeanPropertyDescriptor.php +++ b/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); diff --git a/src/Utils/BeanDescriptor.php b/src/Utils/BeanDescriptor.php index 1ab1d8d8..e908259c 100644 --- a/src/Utils/BeanDescriptor.php +++ b/src/Utils/BeanDescriptor.php @@ -111,7 +111,7 @@ class BeanDescriptor implements BeanDescriptorInterface */ private $registry; /** - * @var int[] + * @var MethodDescriptorInterface[][] */ private $descriptorsByMethodName = []; /** @@ -159,14 +159,7 @@ public function initBeanPropertyDescriptors(): void //init the list of method names with regular properties names foreach ($this->beanPropertyDescriptors as $beanPropertyDescriptor) { - $name = $beanPropertyDescriptor->getGetterName(); - if (!isset($this->descriptorsByMethodName[$name])) { - $this->descriptorsByMethodName[$name] = 0; - } - $this->descriptorsByMethodName[$name] ++; - if ($this->descriptorsByMethodName[$name] > 1) { - $beanPropertyDescriptor->useAlternativeName(); - } + $this->checkForDuplicate($beanPropertyDescriptor); } } @@ -405,8 +398,8 @@ private function getDirectForeignKeysDescriptors(): array $descriptors = []; foreach ($fks as $fk) { - /** @var DirectForeignKeyMethodDescriptor $desc */ - $desc = $this->checkForDuplicate(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; } @@ -429,14 +422,14 @@ private function getPivotTableDescriptors(): array if ($fks[0]->getForeignTableName() === $this->table->getName()) { list($localFk, $remoteFk) = $fks; - /** @var PivotTableMethodsDescriptor $desc */ - $desc = $this->checkForDuplicate(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; - /** @var PivotTableMethodsDescriptor $desc */ - $desc = $this->checkForDuplicate(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; } } @@ -446,25 +439,26 @@ private function getPivotTableDescriptors(): array } /** - * Check the method name isn't already used and flag the $descriptor to use its alternative name if it is the case + * Check the method name isn't already used and flag the associated descriptors to use their alternative names if it is the case */ - public function checkForDuplicate(MethodDescriptorInterface $descriptor): MethodDescriptorInterface + private function checkForDuplicate(MethodDescriptorInterface $descriptor): void { $name = $descriptor->getName(); if (!isset($this->descriptorsByMethodName[$name])) { - $this->descriptorsByMethodName[$name] = 0; + $this->descriptorsByMethodName[$name] = []; } - $this->descriptorsByMethodName[$name] ++; - if ($this->descriptorsByMethodName[$name] > 1) { - $descriptor->useAlternativeName(); + $this->descriptorsByMethodName[$name][] = $descriptor; + if (count($this->descriptorsByMethodName[$name]) > 1) { + foreach ($this->descriptorsByMethodName[$name] as $duplicateDescriptor) { + $duplicateDescriptor->useAlternativeName(); + } } - return $descriptor; } /** * Returns the list of method descriptors (and applies the alternative name if needed). * - * @return MethodDescriptorInterface[] + * @return RelationshipMethodDescriptorInterface[] */ public function getMethodDescriptors(): array { diff --git a/src/Utils/DirectForeignKeyMethodDescriptor.php b/src/Utils/DirectForeignKeyMethodDescriptor.php index f9b97e0c..97c88778 100644 --- a/src/Utils/DirectForeignKeyMethodDescriptor.php +++ b/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; diff --git a/src/Utils/MethodDescriptorInterface.php b/src/Utils/MethodDescriptorInterface.php index f0879e45..6646db41 100644 --- a/src/Utils/MethodDescriptorInterface.php +++ b/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; } diff --git a/src/Utils/PivotTableMethodsDescriptor.php b/src/Utils/PivotTableMethodsDescriptor.php index 372a5648..36dee569 100644 --- a/src/Utils/PivotTableMethodsDescriptor.php +++ b/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 diff --git a/src/Utils/RelationshipMethodDescriptorInterface.php b/src/Utils/RelationshipMethodDescriptorInterface.php new file mode 100644 index 00000000..3f0ab119 --- /dev/null +++ b/src/Utils/RelationshipMethodDescriptorInterface.php @@ -0,0 +1,49 @@ + Date: Tue, 13 Aug 2019 15:37:18 +0200 Subject: [PATCH 4/4] better conflict test --- tests/TDBMAbstractServiceTest.php | 8 ++++++-- tests/TDBMDaoGeneratorTest.php | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php index b3391739..5a204397 100644 --- a/tests/TDBMAbstractServiceTest.php +++ b/tests/TDBMAbstractServiceTest.php @@ -416,7 +416,7 @@ private static function initSchema(Connection $connection): void $db->table('artists') ->column('id')->integer()->primaryKey()->autoIncrement() - ->column('children')->array()->null() + ->column('children')->array()->null() //used to test conflicts with autopivot ->column('name')->string(); $db->table('albums') @@ -438,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') diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php index b2e39fcf..b3eb31b0 100644 --- a/tests/TDBMDaoGeneratorTest.php +++ b/tests/TDBMDaoGeneratorTest.php @@ -2164,8 +2164,9 @@ public function testLazyStopRecursionOnCompositeForeignKey(): void public function testMethodNameConflictsBetweenRegularAndAutoPivotProperties() { $artist = new ArtistBean('Super'); - $artist->getChildren(); - $artist->getChildrenByArtistsRelations(); + $artist->getChildren(); // regular property + $artist->getChildrenByArtistId(); // one-to-may relationship + $artist->getChildrenByArtistsRelations(); // auto-pivot relationship $this->assertEquals(1, 1); } }