CS and test fixes

AlexejKossmann · Apr 8, 2020 · 192fff1 · 192fff1
1 parent 95c7042
commit 192fff1
Show file tree

Hide file tree

Showing 7 changed files with 290 additions and 262 deletions.
diff --git a/core-bundle/src/EventListener/DataContainer/PageUrlListener.php b/core-bundle/src/EventListener/DataContainer/PageUrlListener.php
@@ -74,15 +74,11 @@ public function generateAlias(string $value, DataContainer $dc): string
         /** @var PageModel $page */
         $page = $this->getPageAdapter()->findWithDetails($dc->id);
 
-        if ($value !== '') {
+        if ('' !== $value) {
             try {
                 $this->aliasExists($value, (int) $page->id, $page, true);
             } catch (DuplicateAliasException $exception) {
-                throw new \RuntimeException(
-                    $this->translator->trans('ERR.pageUrlExists', [$exception->getUrl()], 'contao_default'),
-                    $exception->getCode(),
-                    $exception
-                );
+                throw new \RuntimeException($this->translator->trans('ERR.pageUrlExists', [$exception->getUrl()], 'contao_default'), $exception->getCode(), $exception);
             }
 
             return $value;
@@ -132,7 +128,7 @@ public function purgeSearchIndexOnAliasChange(string $value, DataContainer $dc):
      */
     public function validateLanguagePrefix(string $value, DataContainer $dc): string
     {
-        if ($dc->activeRecord->type !== 'root' || $dc->activeRecord->languagePrefix === $value) {
+        if ('root' !== $dc->activeRecord->type || $dc->activeRecord->languagePrefix === $value) {
             return $value;
         }
 
@@ -145,11 +141,7 @@ public function validateLanguagePrefix(string $value, DataContainer $dc): string
         try {
             $this->recursiveValidatePages((int) $rootPage->id, $rootPage);
         } catch (DuplicateAliasException $exception) {
-            throw new \RuntimeException(
-                $this->translator->trans('ERR.pageUrlPrefix', [$exception->getUrl()], 'contao_default'),
-                $exception->getCode(),
-                $exception
-            );
+            throw new \RuntimeException($this->translator->trans('ERR.pageUrlPrefix', [$exception->getUrl()], 'contao_default'), $exception->getCode(), $exception);
         }
 
         return $value;
@@ -160,7 +152,7 @@ public function validateLanguagePrefix(string $value, DataContainer $dc): string
      */
     public function validateUrlSuffix($value, DataContainer $dc)
     {
-        if ($dc->activeRecord->type !== 'root' || $dc->activeRecord->urlSuffix === $value) {
+        if ('root' !== $dc->activeRecord->type || $dc->activeRecord->urlSuffix === $value) {
             return $value;
         }
 
@@ -173,15 +165,13 @@ public function validateUrlSuffix($value, DataContainer $dc)
         try {
             $this->recursiveValidatePages((int) $rootPage->id, $rootPage);
         } catch (DuplicateAliasException $exception) {
-            throw new \RuntimeException(
-                $this->translator->trans('ERR.pageUrlSuffix', [$exception->getUrl()], 'contao_default')
-            );
+            throw new \RuntimeException($this->translator->trans('ERR.pageUrlSuffix', [$exception->getUrl()], 'contao_default'));
         }
 
         return $value;
     }
 
-    public function reset()
+    public function reset(): void
     {
         $this->prefixes = null;
         $this->suffixes = null;
@@ -237,7 +227,7 @@ private function aliasExists(string $currentAlias, int $currentId, PageModel $cu
             )->fetchAll(FetchMode::COLUMN)
         ;
 
-        if (0 === count($aliasIds)) {
+        if (0 === \count($aliasIds)) {
             return false;
         }
 
@@ -299,7 +289,7 @@ private function stripPrefixesAndSuffixes(string $alias, string $languagePrefix,
                 ->fetchAll()
             ;
 
-            if (0 === ($prefixLength = strlen($languagePrefix))) {
+            if (0 === ($prefixLength = \strlen($languagePrefix))) {
                 $this->prefixes = array_column($rows, 'languagePrefix');
             } else {
                 foreach (array_column($rows, 'languagePrefix') as $prefix) {
@@ -313,7 +303,7 @@ private function stripPrefixesAndSuffixes(string $alias, string $languagePrefix,
                 }
             }
 
-            if (0 === ($suffixLength = strlen($urlSuffix))) {
+            if (0 === ($suffixLength = \strlen($urlSuffix))) {
                 $this->suffixes = array_column($rows, 'urlSuffix');
             } else {
                 foreach (array_column($rows, 'urlSuffix') as $suffix) {
@@ -339,7 +329,7 @@ private function regexArray(array $data, string $delimiter = '/'): ?string
     {
         $data = array_filter(array_unique($data));
 
-        if (0 === count($data)) {
+        if (0 === \count($data)) {
             return null;
         }
 

diff --git a/core-bundle/src/Routing/Candidates/Candidates.php b/core-bundle/src/Routing/Candidates/Candidates.php
@@ -125,6 +125,7 @@ protected function addCandidatesFor(string $url, array &$candidates): void
     {
         if ('' === $url) {
             $candidates[] = 'index';
+
             return;
         }
 

diff --git a/core-bundle/tests/EventListener/DataContainer/LegacyRoutingListenerTest.php b/core-bundle/tests/EventListener/DataContainer/LegacyRoutingListenerTest.php
@@ -13,6 +13,7 @@
 namespace Contao\CoreBundle\Tests\EventListener;
 
 use Contao\CoreBundle\EventListener\DataContainer\LegacyRoutingListener;
+use Contao\CoreBundle\Framework\ContaoFramework;
 use Contao\CoreBundle\Tests\TestCase;
 use Contao\DataContainer;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -28,9 +29,12 @@ protected function setUp(): void
         $GLOBALS['TL_DCA'] = [];
     }
 
-    public function testDisabledThePageFieldsInLegacyMode()
+    public function testDoesNothingInNonLegacyMode(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), true);
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(false),
+            $this->createMock(TranslatorInterface::class)
+        );
 
         $GLOBALS['TL_DCA']['tl_page']['palettes'] = ['root' => '', 'rootfallback' => ''];
         $GLOBALS['TL_DCA']['tl_page']['fields'] = [
@@ -40,31 +44,16 @@ public function testDisabledThePageFieldsInLegacyMode()
 
         $listener->disableRoutingFields();
 
-        $this->assertTrue($GLOBALS['TL_DCA']['tl_page']['fields']['languagePrefix']['eval']['disabled']);
-        $this->assertTrue($GLOBALS['TL_DCA']['tl_page']['fields']['urlSuffix']['eval']['disabled']);
+        $this->assertEmpty($GLOBALS['TL_DCA']['tl_page']['fields']['languagePrefix']);
+        $this->assertEmpty($GLOBALS['TL_DCA']['tl_page']['fields']['urlSuffix']);
     }
 
-    public function testDisabledThePageFieldsIfGetPageIdFromUrlHookIsRegistered()
+    public function testDisabledThePageFieldsInLegacyMode(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), false);
-        $GLOBALS['TL_HOOKS']['getPageIdFromUrl'][] = ['class', 'method'];
-
-        $GLOBALS['TL_DCA']['tl_page']['palettes'] = ['root' => '', 'rootfallback' => ''];
-        $GLOBALS['TL_DCA']['tl_page']['fields'] = [
-            'languagePrefix' => [],
-            'urlSuffix' => [],
-        ];
-
-        $listener->disableRoutingFields();
-
-        $this->assertTrue($GLOBALS['TL_DCA']['tl_page']['fields']['languagePrefix']['eval']['disabled']);
-        $this->assertTrue($GLOBALS['TL_DCA']['tl_page']['fields']['urlSuffix']['eval']['disabled']);
-    }
-
-    public function testDisabledThePageFieldsIfGetRootPageFromUrlHookIsRegistered()
-    {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), false);
-        $GLOBALS['TL_HOOKS']['getRootPageFromUrl'][] = ['class', 'method'];
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(true),
+            $this->createMock(TranslatorInterface::class)
+        );
 
         $GLOBALS['TL_DCA']['tl_page']['palettes'] = ['root' => '', 'rootfallback' => ''];
         $GLOBALS['TL_DCA']['tl_page']['fields'] = [
@@ -78,16 +67,20 @@ public function testDisabledThePageFieldsIfGetRootPageFromUrlHookIsRegistered()
         $this->assertTrue($GLOBALS['TL_DCA']['tl_page']['fields']['urlSuffix']['eval']['disabled']);
     }
 
-    public function testAddsTheRoutingWarningInLegacyMode()
+    public function testAddsTheRoutingWarningInLegacyMode(): void
     {
         $translator = $this->createMock(TranslatorInterface::class);
         $translator
             ->expects($this->once())
             ->method('trans')
             ->with('tl_page.legacyRouting', [], 'contao_tl_page')
-            ->willReturn('warning');
+            ->willReturn('warning')
+        ;
 
-        $listener = new LegacyRoutingListener($translator, true);
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(true),
+            $translator
+        );
 
         $GLOBALS['TL_DCA']['tl_page']['palettes'] = ['root' => '', 'rootfallback' => ''];
         $GLOBALS['TL_DCA']['tl_page']['fields'] = [
@@ -100,44 +93,74 @@ public function testAddsTheRoutingWarningInLegacyMode()
         $this->assertIsCallable($GLOBALS['TL_DCA']['tl_page']['fields']['legacy_routing']['input_field_callback']);
         $this->assertSame(
             '<p class="tl_gerror">warning</p>',
-            call_user_func($GLOBALS['TL_DCA']['tl_page']['fields']['legacy_routing']['input_field_callback'])
+            \call_user_func($GLOBALS['TL_DCA']['tl_page']['fields']['legacy_routing']['input_field_callback'])
         );
     }
 
-    public function testOverridesTheLanguagePrefixWithPrependLocale()
+    public function testOverridesTheLanguagePrefixWithPrependLocale(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), true, true);
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(true),
+            $this->createMock(TranslatorInterface::class),
+            true
+        );
 
         /** @var DataContainer&MockObject $dc */
         $dc = $this->mockClassWithProperties(DataContainer::class, ['activeRecord' => (object) ['language' => 'en-US']]);
 
         $this->assertSame('en-US', $listener->overrideLanguagePrefix('foo', $dc));
     }
 
-    public function testOverridesTheLanguagePrefixWithoutPrependLocale()
+    public function testOverridesTheLanguagePrefixWithoutPrependLocale(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), true, false);
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(true),
+            $this->createMock(TranslatorInterface::class),
+            false
+        );
 
         /** @var DataContainer&MockObject $dc */
         $dc = $this->mockClassWithProperties(DataContainer::class, ['activeRecord' => (object) ['language' => 'en-US']]);
 
         $this->assertSame('', $listener->overrideLanguagePrefix('foo', $dc));
     }
 
-    public function testOverridesTheUrlSuffix()
+    public function testOverridesTheUrlSuffix(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), true, false, '.bar');
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(true),
+            $this->createMock(TranslatorInterface::class),
+            false,
+            '.bar'
+        );
 
         $this->assertSame('.bar', $listener->overrideUrlSuffix('foo'));
     }
 
-    public function testReturnsTheOriginalFieldValuesWithoutLegacyMode()
+    public function testReturnsTheOriginalFieldValuesWithoutLegacyMode(): void
     {
-        $listener = new LegacyRoutingListener($this->createMock(TranslatorInterface::class), false);
+        $listener = new LegacyRoutingListener(
+            $this->mockContaoFrameworkWithLegacyRouting(false),
+            $this->createMock(TranslatorInterface::class)
+        );
+
         $dc = $this->createMock(DataContainer::class);
         $dc->activeRecord = (object) ['language' => 'en-US'];
 
         $this->assertSame('foo', $listener->overrideLanguagePrefix('foo', $dc));
         $this->assertSame('foo', $listener->overrideUrlSuffix('foo'));
     }
+
+    private function mockContaoFrameworkWithLegacyRouting(bool $legacyRouting)
+    {
+        $framework = $this->createMock(ContaoFramework::class);
+
+        $framework
+            ->expects($this->atLeastOnce())
+            ->method('isLegacyRouting')
+            ->willReturn($legacyRouting)
+        ;
+
+        return $framework;
+    }
 }