write absolute namespaces to schema.xml

michbeck · May 15, 2021 · 5fe2f83 · 5fe2f83
1 parent 0692734
commit 5fe2f83
Show file tree

Hide file tree

Showing 13 changed files with 248 additions and 66 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
diff --git a/src/Propel/Generator/Builder/Om/AbstractOMBuilder.php b/src/Propel/Generator/Builder/Om/AbstractOMBuilder.php
@@ -361,21 +361,23 @@ public function declareClassNamespace($class, $namespace = '', $alias = false)
      * check if the current $class need an alias or if the class could be used with a shortname without conflict
      *
      * @param string $class
-     * @param string $namespace
+     * @param string $classNamespace
      *
      * @return bool
      */
-    protected function needAliasForClassName($class, $namespace)
+    protected function needAliasForClassName($class, $classNamespace)
     {
-        if ($namespace == $this->getNamespace()) {
+        $builderNamespace = trim($this->getNamespace(), '\\');
+
+        if ($classNamespace == $builderNamespace) {
             return false;
         }
 
-        if (str_replace('\\Base', '', $namespace) == str_replace('\\Base', '', $this->getNamespace())) {
+        if (str_replace('\\Base', '', $classNamespace) == str_replace('\\Base', '', $builderNamespace)) {
             return true;
         }
 
-        if (empty($namespace) && $this->getNamespace() === 'Base') {
+        if (empty($classNamespace) && $builderNamespace === 'Base') {
             if (str_replace(['Query'], '', $class) == str_replace(['Query'], '', $this->getUnqualifiedClassName())) {
                 return true;
             }
@@ -385,14 +387,14 @@ protected function needAliasForClassName($class, $namespace)
             }
 
             // force alias for model without namespace
-            if (array_search($class, $this->whiteListOfDeclaredClasses, true) === false) {
+            if (!in_array($class, $this->whiteListOfDeclaredClasses, true)) {
                 return true;
             }
         }
 
-        if ($namespace === 'Base' && $this->getNamespace() === '') {
+        if ($classNamespace === 'Base' && $builderNamespace === '') {
             // force alias for model without namespace
-            if (array_search($class, $this->whiteListOfDeclaredClasses, true) === false) {
+            if (!in_array($class, $this->whiteListOfDeclaredClasses, true)) {
                 return true;
             }
         }

diff --git a/src/Propel/Generator/Manager/ReverseManager.php b/src/Propel/Generator/Manager/ReverseManager.php
@@ -219,7 +219,9 @@ protected function buildModel()
         $database->setPlatform($config->getConfiguredPlatform($connection));
         $database->setDefaultIdMethod(IdMethod::NATIVE);
 
-        $this->getNamespace() && $database->setNamespace($this->getNamespace());
+        if ($this->getNamespace()) {
+            $database->setNamespace($this->getNamespace());
+        }
 
         $buildConnection = $config->getBuildConnection($databaseName);
         $this->log(sprintf('Reading database structure of database `%s` using dsn `%s`', $this->getDatabaseName(), $buildConnection['dsn']));

diff --git a/src/Propel/Generator/Model/Database.php b/src/Propel/Generator/Model/Database.php
@@ -261,7 +261,7 @@ public function getBaseQueryClass()
      */
     public function setBaseClass($class)
     {
-        $this->baseClass = $class;
+        $this->baseClass = $this->makeNamespaceAbsolute($class);
     }
 
     /**
@@ -274,7 +274,7 @@ public function setBaseClass($class)
      */
     public function setBaseQueryClass($class)
     {
-        $this->baseQueryClass = $class;
+        $this->baseQueryClass = $this->makeNamespaceAbsolute($class);
     }
 
     /**
@@ -590,7 +590,10 @@ public function addTable($table)
         $this->tablesByLowercaseName[strtolower($table->getName())] = $table;
         $this->tablesByPhpName[$table->getPhpName()] = $table;
 
-        $this->computeTableNamespace($table);
+        $newTableNamespace = $this->getCombinedNamespace($table);
+        if ($newTableNamespace !== null) {
+            $table->setNamespace($newTableNamespace);
+        }
 
         if ($table->getPackage() === null) {
             $table->setPackage($this->getPackage());
@@ -704,27 +707,29 @@ public function setSchema($schema)
      *
      * @param \Propel\Generator\Model\Table $table
      *
-     * @return string
+     * @return string|null
      */
-    private function computeTableNamespace(Table $table)
+    private function getCombinedNamespace(Table $table): ?string
     {
-        $namespace = $table->getNamespace();
-        if ($this->isAbsoluteNamespace($namespace)) {
-            $namespace = ltrim($namespace, '\\');
-            $table->setNamespace($namespace);
+        $tableNamespace = $table->getNamespace();
 
-            return $namespace;
+        if ($this->isAbsoluteNamespace($tableNamespace)) {
+            return ltrim($tableNamespace, '\\');
         }
 
-        if ($namespace = $this->getNamespace()) {
-            if ($table->getNamespace()) {
-                $namespace .= '\\' . $table->getNamespace();
-            }
+        $databaseNamespace = $this->getNamespace();
+        if ($this->isAbsoluteNamespace($databaseNamespace)) {
+            $databaseNamespace = ltrim($databaseNamespace, '\\');
+        }
 
-            $table->setNamespace($namespace);
+        if (empty($tableNamespace)) {
+            return $databaseNamespace;
+        }
+        if (!empty($databaseNamespace)) {
+            return "$databaseNamespace\\$tableNamespace";
         }
 
-        return $namespace;
+        return $tableNamespace;
     }
 
     /**

diff --git a/src/Propel/Generator/Model/ScopedMappingModel.php b/src/Propel/Generator/Model/ScopedMappingModel.php
@@ -76,10 +76,16 @@ protected function setupObject()
     /**
      * Returns the namespace.
      *
-     * @return string
+     * @param bool $getAbsoluteNamespace
+     *
+     * @return string|null
      */
-    public function getNamespace()
+    public function getNamespace(bool $getAbsoluteNamespace = false): ?string
     {
+        if ($getAbsoluteNamespace) {
+            return $this->makeNamespaceAbsolute($this->namespace);
+        }
+
         return $this->namespace;
     }
 
@@ -116,7 +122,23 @@ public function setNamespace($namespace)
      */
     public function isAbsoluteNamespace($namespace)
     {
-        return strpos($namespace, '\\') === 0;
+        return ($namespace && substr($namespace, 0, 1) === '\\');
+    }
+
+    /**
+     * Prepends a backslash to a namespace if there is none.
+     *
+     * A namespace with a backslash is considered absolute.
+     *
+     * @param string|null $namespace
+     *
+     * @return string|null
+     */
+    protected function makeNamespaceAbsolute(?string $namespace): ?string
+    {
+        $prependBackslash = ($namespace && !$this->isAbsoluteNamespace($namespace));
+
+        return ($prependBackslash) ?  "\\$namespace" : $namespace;
     }
 
     /**

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
@@ -608,7 +608,7 @@ public function getBaseQueryClass()
      */
     public function setBaseClass($class)
     {
-        $this->baseClass = $class;
+        $this->baseClass = $this->makeNamespaceAbsolute($class);
     }
 
     /**
@@ -620,7 +620,7 @@ public function setBaseClass($class)
      */
     public function setBaseQueryClass($class)
     {
-        $this->baseQueryClass = $class;
+        $this->baseQueryClass = $this->makeNamespaceAbsolute($class);
     }
 
     /**

diff --git a/src/Propel/Generator/Schema/Dumper/XmlDumper.php b/src/Propel/Generator/Schema/Dumper/XmlDumper.php
@@ -109,8 +109,9 @@ private function appendDatabaseNode(Database $database, DOMNode $parentNode)
             $databaseNode->setAttribute('schema', $schema);
         }
 
-        if ($namespace = $database->getNamespace()) {
-            $databaseNode->setAttribute('namespace', $namespace);
+        $absoluteNamespace = $database->getNamespace(true);
+        if ($absoluteNamespace) {
+            $databaseNode->setAttribute('namespace', $absoluteNamespace);
         }
 
         if ($baseClass = $database->getBaseClass()) {
@@ -225,8 +226,9 @@ private function appendTableNode(Table $table, DOMNode $parentNode)
             $tableNode->setAttribute('package', $package);
         }
 
-        if ($namespace = $table->getNamespace()) {
-            $tableNode->setAttribute('namespace', $namespace);
+        $absoluteNamespace = $table->getNamespace(true);
+        if ($absoluteNamespace && $absoluteNamespace !== $database->getNamespace(true)) {
+            $tableNode->setAttribute('namespace', $absoluteNamespace);
         }
 
         if ($table->isSkipSql()) {

diff --git a/tests/Propel/Tests/Generator/Builder/Om/AbstractOMBuilderNamespaceTest.php b/tests/Propel/Tests/Generator/Builder/Om/AbstractOMBuilderNamespaceTest.php
@@ -170,6 +170,41 @@ public function testDeclareClasses()
         ];
         $this->assertEquals($expected, $builder->getDeclaredClasses());
     }
+
+    /**
+     * @return array
+     */
+    public function namespaceDataProvider(): array
+    {
+        //[<table namespace>, <class namespace>, <message>]]
+        return [
+            ['\\My\\Namespace', '\\My\\Namespace', 'slashed namespace should work'],
+            ['My\\Namespace', 'My\\Namespace', 'non-slashed namespace should work'],
+            ['My\\Namespace', '\\My\\Namespace', 'slashes are stripped from class namespace anyway'],
+            ['\\My\\Namespace', 'My\\Namespace', 'slashes are stripped from table namespace anyway'],
+        ];
+    }
+
+    /**
+     * @dataProvider namespaceDataProvider
+     * @doesNotPerformAssertions
+     *
+     * @return void
+     */
+    public function testDeclareClassNamespaceIgnoresLeadingSlashInNamespace(string $tableNamespace, string $classNamespace, string $message): void
+    {
+        $table = new Table('Table1');
+        $table->setNamespace($tableNamespace);
+
+        $builder = new TestableOMBuilder2($table);
+
+        $builder->declareClassNamespace('MyTable1Class', $classNamespace . '\\Base');
+        try{
+            $builder->declareClassNamespace('MyTable1Class', $classNamespace);
+        } catch(LogicException $e) {
+            $this->fail($message);
+        }
+    }
 }
 
 class TestableOMBuilder2 extends AbstractOMBuilder

diff --git a/tests/Propel/Tests/Generator/Model/DatabaseTest.php b/tests/Propel/Tests/Generator/Model/DatabaseTest.php
@@ -18,7 +18,6 @@
 use Propel\Generator\Platform\MysqlPlatform;
 use Propel\Generator\Platform\PgsqlPlatform;
 use Propel\Generator\Util\VfsTrait;
-use Symfony\Component\Filesystem\Filesystem;
 
 /**
  * Unit test suite for Database model class.
@@ -459,25 +458,41 @@ public function testSetHeavyIndexing()
     }
 
     /**
+     * return array
+     */
+    public function baseClassDataProvider(): array
+    {
+        return [
+            // [<Class name>, <Expected class name>, <message>]]
+            ['\CustomBaseQueryObject', '\CustomBaseQueryObject', 'Setter should set base query class'],
+            ['CustomBaseQueryObject', '\CustomBaseQueryObject', 'Setter should set absolute namespace of base query class'],
+        ];
+    }
+
+    /**
+     * @dataProvider baseClassDataProvider
+     *
      * @return void
      */
-    public function testSetBaseClasses()
+    public function testSetBaseClass(string $className, string $expectedClassName, string $message)
     {
         $database = new Database();
-        $database->setBaseClass('CustomBaseObject');
+        $database->setBaseClass($className);
 
-        $this->assertSame('CustomBaseObject', $database->getBaseClass());
+        $this->assertSame($expectedClassName, $database->getBaseClass(), $message);
     }
 
     /**
+     * @dataProvider baseClassDataProvider
+     *
      * @return void
      */
-    public function testSetBaseQueryClasses()
+    public function testSetBaseQueryClass(string $className, string $expectedClassName, string $message)
     {
         $database = new Database();
-        $database->setBaseQueryClass('CustomBaseQueryObject');
+        $database->setBaseQueryClass($className);
 
-        $this->assertSame('CustomBaseQueryObject', $database->getBaseQueryClass());
+        $this->assertSame($expectedClassName, $database->getBaseQueryClass(), $message);
     }
 
     /**
@@ -555,4 +570,50 @@ classname: Propel\Runtime\Connection\DebugPDO
 
         $this->assertEquals($schema, $db->getNamespace());
     }
+
+    /**
+     * @return array
+     */
+    public function combinedNamespaceDataProvider(): array
+    {
+        // [<Database namespace>, <Table namespace>, <Combined namespace>, <Message>]
+        return [
+            [null, null, null, 'No namespaces should leave table namespace empty'],
+            ['Le\\Database', null, 'Le\\Database', 'No table namespace should use database namespace'],
+            [null, 'Il\\Table', 'Il\\Table', 'No database namespace should result in unchanged table namespace'],
+            ['Le\\Database', '\\Il\\Table', 'Il\\Table', 'Absolute table namespace should superseed database namespace'],
+            ['Le\\Database', 'Il\\Table', 'Le\\Database\\Il\\Table', 'Relative table namespace should be apended to database namespace'],
+            ['Le\\Database', 'Le\\Database', 'Le\\Database\\Le\\Database', 'Same relative namespace on database and table should be doubled'],
+            ['Le\\Database', '\\Le\\Database', 'Le\\Database', 'Same absolute namespace on database and table should be used only once (as all absolute namespaces)'],
+        ];
+    }
+
+    /**
+     * @dataProvider combinedNamespaceDataProvider
+     *
+     * @param string|null $databaseNamespace
+     * @param string|null $tableNamespace
+     * @param string|null $expectedNamespace
+     * @param string $message
+     *
+     * @return void
+     */
+    public function testCombineNamespace($databaseNamespace, $tableNamespace, $expectedNamespace, $message)
+    {
+        $database = new Database();
+
+        if ($databaseNamespace !== null) {
+            $database->setNamespace($databaseNamespace);
+        }
+
+        $table = new Table();
+        if ($tableNamespace !== null) {
+            $table->setNamespace($tableNamespace);
+        }
+
+        $database->addTable($table);
+        $combinedNamespace = $table->getNamespace();
+
+        $this->assertEquals($expectedNamespace, $combinedNamespace, $message);
+    }
 }