change methods that instantiate abstract classes

FXdSP · May 20, 2021 · 3295188 · 3295188
1 parent 0692734
commit 3295188
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 21 deletions.
diff --git a/src/Propel/Generator/Builder/Om/ExtensionQueryBuilder.php b/src/Propel/Generator/Builder/Om/ExtensionQueryBuilder.php
@@ -40,6 +40,7 @@ protected function addClassOpen(&$script)
         $table = $this->getTable();
         $tableName = $table->getName();
         $tableDesc = $table->getDescription();
+        $className = $this->getUnqualifiedClassName();
         $baseClassName = $this->getClassNameFromBuilder($this->getQueryBuilder());
 
         if ($this->getBuildProperty('generator.objectModel.addClassLevelComment')) {
@@ -64,7 +65,7 @@ protected function addClassOpen(&$script)
  */";
         }
         $script .= "
-class " . $this->getUnqualifiedClassName() . " extends $baseClassName
+class $className extends $baseClassName
 {
 ";
     }

diff --git a/src/Propel/Generator/Builder/Om/QueryBuilder.php b/src/Propel/Generator/Builder/Om/QueryBuilder.php
@@ -67,8 +67,16 @@ public function getUnprefixedClassName()
     public function getParentClass()
     {
         $parentClass = $this->getBehaviorContent('parentClass');
+        if ($parentClass) {
+            return $parentClass;
+        }
+
+        $baseQueryClass = $this->getTable()->getBaseQueryClass();
+        if ($baseQueryClass) {
+            return $baseQueryClass;
+        }
 
-        return $parentClass === null ? ($this->getTable()->getBaseQueryClass() != '' ? $this->getTable()->getBaseQueryClass() : 'ModelCriteria') : $parentClass;
+        return 'ModelCriteria';
     }
 
     /**
@@ -627,10 +635,9 @@ public function findPk(\$key, ConnectionInterface \$con = null)
             return null;
         }";
         if ($table->hasCompositePrimaryKey()) {
-            $pks = [];
-            foreach ($table->getPrimaryKey() as $index => $column) {
-                $pks[] = "\$key[$index]";
-            }
+            $numberOfPks = count($table->getPrimaryKey());
+            $pkIndexes = range(0, $numberOfPks - 1);
+            $pks = preg_filter('/(\d+)/', '$key[${1}]', $pkIndexes); // put ids into "$key[]"
         } else {
             $pks = '$key';
         }
@@ -675,6 +682,19 @@ protected function addFindPkSimple(&$script)
             return;
         }
 
+        $usesConcreteInheritance = $table->usesConcreteInheritance();
+        if ($table->isAbstract() && !$usesConcreteInheritance) {
+            $tableName = $table->getPhpName();
+            $script .= "
+    protected function findPkSimple(\$key, ConnectionInterface \$con)
+    {
+        throw new PropelException('$tableName is declared abstract, you cannot query it.');
+    }
+";
+
+            return;
+        }
+
         $platform = $this->getPlatform();
         $tableMapClassName = $this->getTableMapClassName();
         $ARClassName = $this->getObjectClassName();
@@ -741,7 +761,7 @@ protected function findPkSimple(\$key, ConnectionInterface \$con)
         \$obj = null;
         if (\$row = \$stmt->fetch(\PDO::FETCH_NUM)) {";
 
-        if ($table->getChildrenColumn()) {
+        if ($usesConcreteInheritance) {
             $script .= "
             \$cls = {$tableMapClassName}::getOMClass(\$row, 0, false);
             /** @var $ARClassName \$obj */
@@ -1224,7 +1244,6 @@ public function filterBy$colPhpName(\$$variableName = null, \$comparison = null)
      */
     protected function addFilterByArrayCol(&$script, Column $col)
     {
-        $colPhpName = $col->getPhpName();
         $singularPhpName = $col->getPhpSingularName();
         $colName = $col->getName();
         $variableName = $col->getCamelCaseName();
@@ -1306,7 +1325,6 @@ protected function addFilterByFk(&$script, $fk)
             '\Propel\Runtime\Collection\ObjectCollection',
             '\Propel\Runtime\Exception\PropelException'
         );
-        $table = $this->getTable();
         $queryClass = $this->getQueryClassName();
         $fkTable = $fk->getForeignTable();
         $fkStubObjectBuilder = $this->getNewStubObjectBuilder($fkTable);

diff --git a/src/Propel/Generator/Builder/Om/TableMapBuilder.php b/src/Propel/Generator/Builder/Om/TableMapBuilder.php
@@ -958,7 +958,6 @@ public static function getPrimaryKeyFromRow(\$row, \$offset = 0, \$indexType = T
             \$pks = [];
             ";
 
-            $pks = [];
             foreach ($table->getColumns() as $col) {
                 if (!$col->isLazyLoad()) {
                     if ($col->isPrimaryKey()) {
@@ -1125,16 +1124,21 @@ public static function getOMClass(\$withPrefix = true)
      */
     protected function addGetOMClass_NoInheritance_Abstract(&$script)
     {
+        $objectClassName = $this->getObjectClassName();
+
         $script .= "
     /**
      * The class that the tableMap will make instances of.
      *
      * This method must be overridden by the stub subclass, because
-     * " . $this->getObjectClassName() . " is declared abstract in the schema.
+     * $objectClassName is declared abstract in the schema.
      *
      * @param boolean \$withPrefix
      */
-    abstract public static function getOMClass(\$withPrefix = true);
+    public static function getOMClass(\$withPrefix = true)
+    {
+        throw new PropelException('$objectClassName is declared abstract, it cannot be instantiated.');
+    }
 ";
     }
 

diff --git a/src/Propel/Generator/Model/Table.php b/src/Propel/Generator/Model/Table.php
@@ -545,7 +545,7 @@ protected function collectIndexedColumns($indexName, $columns, &$collectedIndexe
     /**
      * Returns a delimiter-delimited string list of column names.
      *
-     * @see Platform::getColumnList() if quoting is required
+     * @see \Propel\Generator\Platform\PlatformInterface::getColumnListDDL() if quoting is required
      *
      * @param array $columns
      * @param string $delimiter
@@ -801,6 +801,16 @@ public function getChildrenColumn()
         return $this->inheritanceColumn;
     }
 
+    /**
+     * Checks whether the table uses concrete inheritance
+     *
+     * @return bool
+     */
+    public function usesConcreteInheritance(): bool
+    {
+        return ($this->inheritanceColumn !== null);
+    }
+
     /**
      * Returns the subclasses that can be created from this table.
      *

diff --git a/tests/Propel/Tests/Generator/Builder/Om/QueryBuilderTest.php b/tests/Propel/Tests/Generator/Builder/Om/QueryBuilderTest.php
@@ -9,6 +9,7 @@
 namespace Propel\Tests\Generator\Builder\Om;
 
 use Propel\Runtime\ActiveQuery\Criteria;
+use Propel\Runtime\ActiveQuery\Criterion\ExistsCriterion;
 use Propel\Runtime\ActiveQuery\ModelCriteria;
 use Propel\Runtime\ActiveQuery\ModelJoin;
 use Propel\Runtime\Exception\PropelException;
@@ -40,7 +41,6 @@
 use Propel\Tests\Helpers\Bookstore\BookstoreDataPopulator;
 use Propel\Tests\Helpers\Bookstore\BookstoreTestBase;
 use ReflectionMethod;
-use Propel\Runtime\ActiveQuery\Criterion\ExistsCriterion;
 
 /**
  * Test class for QueryBuilder.
@@ -58,6 +58,7 @@ protected function setUp(): void
     {
         parent::setUp();
         include_once(__DIR__ . '/QueryBuilderTestClasses.php');
+        include_once(__DIR__ . '/TestableQueryBuilder.php');
     }
 
     /**
@@ -1287,7 +1288,7 @@ public function testUseFkQueryNoAliasThenWith()
         $q2 = $con->getLastExecutedQuery();
         $this->assertEquals($q1, $q2, 'with() can be used after a call to useFkQuery() with no alias');
     }
-    
+
     /**
      * @return void
      */
@@ -1301,10 +1302,10 @@ public function testUseRelationExistsQuery()
         ->useAuthorExistsQuery()
         ->filterByFirstName('Leo')
         ->endUse();
-        
+
         $this->assertEquals($expected, $actual, 'useExistsQuery() is available and calls correct parent method');
     }
-    
+
     /**
      * @return void
      */
@@ -1318,10 +1319,10 @@ public function testUseRelationNotExistsQuery()
         ->useAuthorNotExistsQuery()
         ->filterByFirstName('Leo')
         ->endUse();
-        
+
         $this->assertEquals($expected, $actual, 'useNotExistsQuery() is available and calls correct parent method');
     }
-    
+
     /**
      * @return void
      */
@@ -1330,7 +1331,7 @@ public function testUseRelationExistsQueryWithCustomQueryClass()
         $query = BookQuery::create()->useAuthorExistsQuery(null, BookClubListQuery::class, false);
         $this->assertInstanceOf(BookClubListQuery::class, $query, 'useExistsQuery() passes on given query class');
     }
-    
+
     /**
      * @return void
      */
@@ -1339,7 +1340,7 @@ public function testUseRelationNotExistsQueryWithCustomQueryClass()
         $query = BookQuery::create()->useAuthorNotExistsQuery(null, BookClubListQuery::class);
         $this->assertInstanceOf(BookClubListQuery::class, $query, 'useNotExistsQuery() passes on given query class');
     }
-    
+
     /**
      * @return void
      */
@@ -1389,4 +1390,44 @@ public function testPruneCompositeKey()
         $nbBookListRel = BookListRelQuery::create()->prune($testBookListRel)->count();
         $this->assertEquals(1, $nbBookListRel, 'prune() removes an object from the result');
     }
+
+    /**
+     * @return void
+     */
+    public function testFindPkSimpleThrowsExceptionWhenTableIsAbstract(): void
+    {
+        $databaseXml = '
+<database>
+    <table name="my_table" abstract="true">
+        <column name="id" type="integer" primaryKey="true"/>
+    </table>
+</database>
+';
+        $script = TestableQueryBuilder::forTableFromXml($databaseXml, 'my_table')->buildScript('addFindPkSimple');
+        $throwStatement = 'throw new PropelException(\'MyTable is declared abstract, you cannot query it.\');';
+        $msg = 'Query class for abstract table should throw exception when calling findPkSimple()';
+        $this->assertStringContainsString($throwStatement, $script, $msg);
+    }
+
+    /**
+     * @return void
+     */
+    public function testFindPkSimpleThrowsNoExceptionWhenTableIsAbstractWithInheritance(): void
+    {
+        $databaseXml = '
+<database>
+    <table name="my_table" abstract="true">
+        <column name="id" type="integer" primaryKey="true"/>
+        <column name="class_key" type="integer" inheritance="single">
+            <inheritance key="1" class="class1"/>
+            <inheritance key="2" class="class2" extends="my_table"/>
+        </column>
+    </table>
+</database>
+';
+        $script = TestableQueryBuilder::forTableFromXml($databaseXml, 'my_table')->buildScript('addFindPkSimple');
+        $throwStatement = 'throw new PropelException(\'MyTable is declared abstract, you cannot query it.\');';
+        $msg = 'Query class for abstract table should not have abstract findPkSimple() method if table uses inheritance';
+        $this->assertStringNotContainsString($throwStatement, $script, $msg);
+    }
 }
diff --git a/tests/Propel/Tests/Generator/Builder/Om/TestableQueryBuilder.php b/tests/Propel/Tests/Generator/Builder/Om/TestableQueryBuilder.php
@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * MIT License. This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Propel\Tests\Generator\Builder\Om;
+
+use Propel\Generator\Builder\Om\QueryBuilder;
+use Propel\Generator\Builder\Util\SchemaReader;
+use Propel\Generator\Config\QuickGeneratorConfig;
+use Propel\Generator\Platform\DefaultPlatform;
+
+/**
+ * Utility class for QueryBuilder.
+ */
+class TestableQueryBuilder extends QueryBuilder
+{
+    /**
+     * Build a TestableQueryBuilder for a table given in schema xml format.
+     *
+     * @param string $schemaXml
+     * @param string $tableName
+     *
+     * @return self
+     */
+    public static function forTableFromXml(string $schemaXml, string $tableName): self
+    {
+        $reader = new SchemaReader();
+        $schema = $reader->parseString($schemaXml);
+        $table = $schema->getDatabase()->getTable($tableName);
+        $builder = new static($table);
+        $builder->setGeneratorConfig(new QuickGeneratorConfig());
+        $builder->setPlatform(new DefaultPlatform());
+
+        return $builder;
+    }
+
+    /**
+     * Call a (usually protected) script builder function by name and return the result.
+     *
+     * @param string $scriptBuilderFunctionName
+     *
+     * @return string
+     */
+    public function buildScript(string $scriptBuilderFunctionName): string
+    {
+        $script = '';
+        call_user_func_array([$this, $scriptBuilderFunctionName], [&$script]);
+
+        return $script;
+    }
+}