Provide migration for folderUrl setting (see contao#1987)

Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | -
| Docs PR or issue | -

When upgrading a Contao 4.9 instance to 4.10 I noticed that the `folderUrl` setting wasn't migrated to the `tl_page` settings.

_Note:_ I need some help with the unit tests, as I am not sure how to properly mock the `Config` singleton, which also has static methods.

Commits
-------

5638df5 provide migration for folderUrl setting
69c840f fix code style and namespace
fb2566f use existing RoutingMigration
4e3f302 Merge branch '4.10' into migrate-folderUrl-config
4c9ce4e update functional test
c873d39 fix code style

core-bundle/src/Migration/Version410/RoutingMigration.php

@@ -12,6 +12,8 @@
 
 namespace Contao\CoreBundle\Migration\Version410;
 
+use Contao\Config;
+use Contao\CoreBundle\Framework\ContaoFramework;
 use Contao\CoreBundle\Migration\AbstractMigration;
 use Contao\CoreBundle\Migration\MigrationResult;
 use Doctrine\DBAL\Connection;
@@ -29,6 +31,11 @@ class RoutingMigration extends AbstractMigration
      */
     private $connection;
 
+    /**
+     * @var ContaoFramework
+     */
+    private $framework;
+
     /**
      * @var string
      */
@@ -39,9 +46,10 @@ class RoutingMigration extends AbstractMigration
      */
     private $prependLocale;
 
-    public function __construct(Connection $connection, string $urlSuffix = '.html', bool $prependLocale = false)
+    public function __construct(Connection $connection, ContaoFramework $framework, string $urlSuffix = '.html', bool $prependLocale = false)
     {
         $this->connection = $connection;
+        $this->framework = $framework;
         $this->urlSuffix = $urlSuffix;
         $this->prependLocale = $prependLocale;
     }
@@ -56,7 +64,7 @@ public function shouldRun(): bool
 
         $columns = $schemaManager->listTableColumns('tl_page');
 
-        return !isset($columns['urlprefix']) && !isset($columns['urlsuffix']);
+        return !isset($columns['urlprefix']) && !isset($columns['urlsuffix']) && !isset($columns['usefolderurl']);
     }
 
     public function run(): MigrationResult
@@ -67,7 +75,10 @@ public function run(): MigrationResult
         $urlSuffix = new Column('urlSuffix', new StringType());
         $urlSuffix->setColumnDefinition("varchar(16) NOT NULL default '.html'");
 
-        $diff = new TableDiff('tl_page', [$urlPrefix, $urlSuffix]);
+        $useFolderUrl = new Column('useFolderUrl', new StringType());
+        $useFolderUrl->setColumnDefinition("char(1) NOT NULL default ''");
+
+        $diff = new TableDiff('tl_page', [$urlPrefix, $urlSuffix, $useFolderUrl]);
         $sql = $this->connection->getDatabasePlatform()->getAlterTableSQL($diff);
 
         foreach ($sql as $statement) {
@@ -81,6 +92,15 @@ public function run(): MigrationResult
             ->execute(['suffix' => $this->urlSuffix])
         ;
 
+        $this->framework->initialize();
+
+        /** @var Config $config */
+        $config = $this->framework->getAdapter(Config::class);
+
+        if ($config->get('folderUrl')) {
+            $this->connection->update('tl_page', ['useFolderUrl' => '1'], ['type' => 'root']);
+        }
+
         return $this->createResult(true);
     }
 }

core-bundle/src/Resources/config/migrations.yml

@@ -60,3 +60,10 @@ services:
     Contao\CoreBundle\Migration\Version410\OrderFieldMigration:
         arguments:
             - '@database_connection'
+
+    Contao\CoreBundle\Migration\Version410\RoutingMigration:
+        arguments:
+            - '@database_connection'
+            - '@contao.framework'
+            - '%contao.url_suffix%'
+            - '%contao.prepend_locale%'

core-bundle/tests/DependencyInjection/ContaoCoreExtensionTest.php

@@ -97,6 +97,7 @@
 use Contao\CoreBundle\Menu\BackendMenuBuilder;
 use Contao\CoreBundle\Migration\MigrationCollection;
 use Contao\CoreBundle\Migration\Version409\CeAccessMigration;
+use Contao\CoreBundle\Migration\Version410\RoutingMigration;
 use Contao\CoreBundle\Monolog\ContaoTableHandler;
 use Contao\CoreBundle\Monolog\ContaoTableProcessor;
 use Contao\CoreBundle\OptIn\OptIn;
@@ -3843,6 +3844,27 @@ public function testRegistersTheVersion409CeAccessMigration(): void
         );
     }
 
+    public function testRegistersTheVersion410RoutingMigration(): void
+    {
+        $container = $this->getContainerBuilder();
+
+        $this->assertTrue($container->has(RoutingMigration::class));
+
+        $definition = $container->getDefinition(RoutingMigration::class);
+
+        $this->assertTrue($definition->isPrivate());
+
+        $this->assertEquals(
+            [
+                new Reference('database_connection'),
+                new Reference('contao.framework'),
+                '%contao.url_suffix%',
+                '%contao.prepend_locale%',
+            ],
+            $definition->getArguments()
+        );
+    }
+
     public function testRegistersThePredefinedImageSizes(): void
     {
         $container = $this->getContainerBuilder();

core-bundle/tests/Functional/Migration/RoutingMigrationTest.php

@@ -12,6 +12,8 @@
 
 namespace Contao\CoreBundle\Tests\Functional\Migration;
 
+use Contao\Config;
+use Contao\CoreBundle\Framework\ContaoFramework;
 use Contao\CoreBundle\Migration\Version410\RoutingMigration;
 use Contao\TestCase\FunctionalTestCase;
 use Doctrine\DBAL\Connection;
@@ -33,30 +35,38 @@ public function testShouldRun(array $dropFields, bool $expected): void
             $connection->exec('ALTER TABLE tl_page DROP '.$field);
         }
 
-        $migration = new RoutingMigration($connection);
+        /** @var ContaoFramework $framework */
+        $framework = static::$container->get('contao.framework');
+
+        $migration = new RoutingMigration($connection, $framework);
 
         $this->assertSame($expected, $migration->shouldRun());
     }
 
     public function shouldRunProvider(): \Generator
     {
-        yield 'should not run if both fields exist' => [
+        yield 'should not run if all fields exist' => [
             [],
             false,
         ];
 
         yield 'should not run if urlSuffix exist' => [
-            ['urlPrefix'],
+            ['urlPrefix', 'useFolderUrl'],
             false,
         ];
 
         yield 'should not run if urlPrefix exist' => [
-            ['urlSuffix'],
+            ['urlSuffix', 'useFolderUrl'],
             false,
         ];
 
-        yield 'should run if both fields do not exist' => [
+        yield 'should not run if useFolderUrl exist' => [
             ['urlPrefix', 'urlSuffix'],
+            false,
+        ];
+
+        yield 'should run if all fields do not exist' => [
+            ['urlPrefix', 'urlSuffix', 'useFolderUrl'],
             true,
         ];
     }
@@ -68,14 +78,18 @@ public function testMigratesSchema(): void
 
         /** @var Connection $connection */
         $connection = static::$container->get('database_connection');
-        $connection->exec('ALTER TABLE tl_page DROP urlPrefix, DROP urlSuffix');
+        $connection->exec('ALTER TABLE tl_page DROP urlPrefix, DROP urlSuffix, DROP useFolderUrl');
 
         $columns = $connection->getSchemaManager()->listTableColumns('tl_page');
 
         $this->assertFalse(isset($columns['urlPrefix']));
         $this->assertFalse(isset($columns['urlsuffix']));
+        $this->assertFalse(isset($columns['usefolderurl']));
 
-        $migration = new RoutingMigration($connection);
+        /** @var ContaoFramework $framework */
+        $framework = static::$container->get('contao.framework');
+
+        $migration = new RoutingMigration($connection, $framework);
         $result = $migration->run();
 
         $this->assertTrue($result->isSuccessful());
@@ -84,35 +98,45 @@ public function testMigratesSchema(): void
 
         $this->assertTrue(isset($columns['urlprefix']));
         $this->assertTrue(isset($columns['urlsuffix']));
+        $this->assertTrue(isset($columns['usefolderurl']));
     }
 
     /**
      * @dataProvider migrationDataProvider
      */
-    public function testMigratesData(bool $prependLocale, string $urlSuffix): void
+    public function testMigratesData(bool $prependLocale, string $urlSuffix, bool $folderUrl): void
     {
         static::bootKernel();
         static::resetDatabaseSchema();
         static::loadFixtures([__DIR__.'/../../Fixtures/Functional/Migration/routing.yml'], false);
 
         /** @var Connection $connection */
         $connection = static::$container->get('database_connection');
-        $connection->exec('ALTER TABLE tl_page DROP urlPrefix, DROP urlSuffix');
+        $connection->exec('ALTER TABLE tl_page DROP urlPrefix, DROP urlSuffix, DROP useFolderUrl');
+
+        /** @var ContaoFramework $framework */
+        $framework = static::$container->get('contao.framework');
 
-        $migration = new RoutingMigration($connection, $urlSuffix, $prependLocale);
+        /** @var Config $config */
+        $config = $framework->getAdapter(Config::class);
+        $config->set('folderUrl', $folderUrl);
+
+        $migration = new RoutingMigration($connection, $framework, $urlSuffix, $prependLocale);
         $migration->run();
 
-        $rows = $connection->fetchAll('SELECT type, language, urlPrefix, urlSuffix FROM tl_page');
+        $rows = $connection->fetchAll('SELECT type, language, urlPrefix, urlSuffix, useFolderUrl FROM tl_page');
 
         foreach ($rows as $row) {
             if ('root' !== $row['type']) {
                 $this->assertSame('', $row['urlPrefix']);
                 $this->assertSame('.html', $row['urlSuffix']);
+                $this->assertSame('', $row['useFolderUrl']);
                 continue;
             }
 
             $this->assertSame($prependLocale ? $row['language'] : '', $row['urlPrefix']);
             $this->assertSame($urlSuffix, $row['urlSuffix']);
+            $this->assertSame($folderUrl ? '1' : '', $row['useFolderUrl']);
         }
     }
 
@@ -121,41 +145,97 @@ public function migrationDataProvider(): \Generator
         yield [
             false,
             '.html',
+            true,
         ];
 
         yield [
             true,
             '.html',
+            true,
         ];
 
         yield [
             false,
             '.json',
+            true,
         ];
 
         yield [
             true,
             '.json',
+            true,
         ];
 
         yield [
             false,
             '/',
+            true,
         ];
 
         yield [
             true,
             '/',
+            true,
         ];
 
         yield [
             false,
             '',
+            true,
         ];
 
         yield [
             true,
             '',
+            true,
+        ];
+
+        yield [
+            false,
+            '.html',
+            true,
+        ];
+
+        yield [
+            true,
+            '.html',
+            false,
+        ];
+
+        yield [
+            false,
+            '.json',
+            false,
+        ];
+
+        yield [
+            true,
+            '.json',
+            false,
+        ];
+
+        yield [
+            false,
+            '/',
+            false,
+        ];
+
+        yield [
+            true,
+            '/',
+            false,
+        ];
+
+        yield [
+            false,
+            '',
+            false,
+        ];
+
+        yield [
+            true,
+            '',
+            false,
         ];
     }
 }