move supported languages logic to service (#2701)

config/services.yaml

@@ -35,8 +35,8 @@ services:
     security.user.provider.chain:
         class: App\Security\KimaiUserProvider
 
-    App\EventSubscriber\RedirectToLocaleSubscriber:
+    App\Utils\LanguageService:
-        arguments: ['@router', '%app_locales%', '%locale%']
+        arguments: ['%app_locales%']
 
     App\Repository\WidgetRepository:
         arguments:
@@ -105,13 +105,6 @@ services:
         arguments:
             - !service { class: PDO, factory: ['@database_connection', 'getWrappedConnection'] }
 
-    # ================================================================================
-    # FORMS
-    # ================================================================================
-
-    App\Form\Type\LanguageType:
-        arguments: ['%app_locales%']
-
     # ================================================================================
     # THEME
     # ================================================================================
@@ -152,7 +145,7 @@ services:
     # ================================================================================
 
     App\Ldap\LdapAuthenticationProvider:
-        arguments: ['@App\Security\UserChecker', '', '', '', '@App\Configuration\LdapConfiguration', '%security.authentication.hide_user_not_found%']
+        arguments: ['@App\Security\UserChecker', '', '', '', '@App\Configuration\LdapConfiguration']
 
     # ================================================================================
     # REPOSITORIES

src/Controller/HomepageController.php

@@ -11,6 +11,7 @@ namespace App\Controller;
 
 use App\Entity\User;
 use App\Form\Type\InitialViewType;
+use App\Utils\LanguageService;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -27,7 +28,7 @@ class HomepageController extends AbstractController
     /**
      * @Route(path="", defaults={}, name="homepage", methods={"GET"})
      */
-    public function indexAction(Request $request): Response
+    public function indexAction(Request $request, LanguageService $service): Response
     {
         /** @var User $user */
         $user = $this->getUser();
@@ -43,6 +44,12 @@ class HomepageController extends AbstractController
             $userLanguage = $requestLanguage;
         }
 
+        // if a user somehow managed to get a wrong locale into hos account (eg. an imported user from Kimai 1)
+        // make sure that he will still see a beautiful page and not a 404
+        if (!$service->isKnownLanguage($userLanguage)) {
+            $userLanguage = $service->getDefaultLanguage();
+        }
+
         $routes = [
             [$userRoute, $userLanguage],
             [$userRoute, $requestLanguage],

src/EventSubscriber/RedirectToLocaleSubscriber.php

@@ -9,6 +9,7 @@
 
 namespace App\EventSubscriber;
 
+use App\Utils\LanguageService;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -25,48 +26,13 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
  */
 class RedirectToLocaleSubscriber implements EventSubscriberInterface
 {
-    /**
-     * @var UrlGeneratorInterface
-     */
     private $urlGenerator;
+    private $languageService;
 
-    /**
+    public function __construct(UrlGeneratorInterface $urlGenerator, LanguageService $languageService)
-     * List of supported locales.
-     *
-     * @var string[]
-     */
-    private $locales = [];
-
-    /**
-     * @var string
-     */
-    private $defaultLocale = '';
-
-    /**
-     * Constructor.
-     *
-     * @param UrlGeneratorInterface $urlGenerator
-     * @param string $locales Supported locales separated by '|'
-     * @param string|null $defaultLocale
-     */
-    public function __construct(UrlGeneratorInterface $urlGenerator, $locales, $defaultLocale = null)
     {
         $this->urlGenerator = $urlGenerator;
-
+        $this->languageService = $languageService;
-        $this->locales = explode('|', trim($locales));
-        $this->defaultLocale = $defaultLocale ?: $this->locales[0];
-
-        if (!\in_array($this->defaultLocale, $this->locales)) {
-            throw new \UnexpectedValueException(
-                sprintf('The default locale ("%s") must be one of "%s".', $this->defaultLocale, $locales)
-            );
-        }
-
-        // Add the default locale at the first position of the array,
-        // because Symfony\HttpFoundation\Request::getPreferredLanguage
-        // returns the first element when no an appropriate language is found
-        array_unshift($this->locales, $this->defaultLocale);
-        $this->locales = array_unique($this->locales);
     }
 
     public static function getSubscribedEvents(): array
@@ -84,13 +50,20 @@ class RedirectToLocaleSubscriber implements EventSubscriberInterface
         if ('/' !== $request->getPathInfo()) {
             return;
         }
+
         // Ignore requests from referrers with the same HTTP host in order to prevent
         // changing language for users who possibly already selected it for this application.
         if (0 === stripos($request->headers->get('referer'), $request->getSchemeAndHttpHost())) {
             return;
         }
 
-        $preferredLanguage = $request->getPreferredLanguage($this->locales);
+        $allLanguages = $this->languageService->getAllLanguages();
+
+        // Add the default locale at the first position of the array, because getPreferredLanguage()
+        // returns the first element when no appropriate language is found
+        array_unshift($allLanguages, $this->languageService->getDefaultLanguage());
+
+        $preferredLanguage = $request->getPreferredLanguage(array_unique($allLanguages));
 
         $response = new RedirectResponse($this->urlGenerator->generate('homepage', ['_locale' => $preferredLanguage]));
         $event->setResponse($response);

src/Form/Type/LanguageType.php

@@ -9,6 +9,7 @@
 
 namespace App\Form\Type;
 
+use App\Utils\LanguageService;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
 use Symfony\Component\Intl\Locales;
@@ -19,21 +20,11 @@ use Symfony\Component\OptionsResolver\OptionsResolver;
  */
 class LanguageType extends AbstractType
 {
-    /**
+    private $languageService;
-     * @var string[]
-     */
-    private $locales = [];
 
-    /**
+    public function __construct(LanguageService $languageService)
-     * @param array|string $locales
-     */
-    public function __construct($locales)
     {
-        if (!\is_array($locales)) {
+        $this->languageService = $languageService;
-            $locales = explode('|', $locales);
-        }
-
-        $this->locales = $locales;
     }
 
     /**
@@ -42,7 +33,7 @@ class LanguageType extends AbstractType
     public function configureOptions(OptionsResolver $resolver)
     {
         $choices = [];
-        foreach ($this->locales as $key) {
+        foreach ($this->languageService->getAllLanguages() as $key) {
             $name = ucfirst(Locales::getName($key, $key));
             $choices[$name] = $key;
         }

src/Utils/LanguageService.php

@@ -0,0 +1,49 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Utils;
+
+use App\Constants;
+
+final class LanguageService
+{
+    /**
+     * @var string[]|string
+     */
+    private $locales;
+
+    public function __construct(string $locales)
+    {
+        $this->locales = $locales;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getAllLanguages(): array
+    {
+        if (!\is_array($this->locales)) {
+            // no further checks, because the list of languages is hard coded and we can be sure that
+            // it is well formatted and contains the default langauge english
+            $this->locales = array_unique(explode('|', trim($this->locales)));
+        }
+
+        return $this->locales;
+    }
+
+    public function isKnownLanguage(string $language): bool
+    {
+        return \in_array($language, $this->getAllLanguages());
+    }
+
+    public function getDefaultLanguage(): string
+    {
+        return Constants::DEFAULT_LOCALE;
+    }
+}

tests/EventSubscriber/RedirectToLocaleSubscriberTest.php

@@ -10,6 +10,7 @@
 namespace App\Tests\EventSubscriber;
 
 use App\EventSubscriber\RedirectToLocaleSubscriber;
+use App\Utils\LanguageService;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -24,7 +25,7 @@ class RedirectToLocaleSubscriberTest extends TestCase
     public function testConstruct()
     {
         $urlGenerator = $this->createMock(UrlGeneratorInterface::class);
-        $sut = new RedirectToLocaleSubscriber($urlGenerator, 'de|en', 'en');
+        $sut = new RedirectToLocaleSubscriber($urlGenerator, new LanguageService('de|en'));
 
         self::assertEquals([KernelEvents::REQUEST => ['onKernelRequest']], RedirectToLocaleSubscriber::getSubscribedEvents());
 
@@ -37,13 +38,4 @@ class RedirectToLocaleSubscriberTest extends TestCase
 
         $sut->onKernelRequest($event);
     }
-
-    public function testConstructWithUnknownDefaultLocale()
-    {
-        $this->expectException(\UnexpectedValueException::class);
-        $this->expectExceptionMessage('The default locale ("en") must be one of "de|it".');
-
-        $urlGenerator = $this->createMock(UrlGeneratorInterface::class);
-        $sut = new RedirectToLocaleSubscriber($urlGenerator, 'de|it', 'en');
-    }
 }

tests/Utils/LanguageServiceTest.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Utils;
+
+use App\Utils\LanguageService;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \App\Utils\LanguageService
+ */
+class LanguageServiceTest extends TestCase
+{
+    public function testDefaults()
+    {
+        $sut = new LanguageService('en');
+        self::assertFalse($sut->isKnownLanguage('de'));
+        self::assertTrue($sut->isKnownLanguage('en'));
+        self::assertFalse($sut->isKnownLanguage('xx'));
+        self::assertEquals('en', $sut->getDefaultLanguage());
+        self::assertEquals(['en'], $sut->getAllLanguages());
+    }
+
+    public function testOneLanguage()
+    {
+        $sut = new LanguageService('de');
+        self::assertTrue($sut->isKnownLanguage('de'));
+        self::assertFalse($sut->isKnownLanguage('en'));
+        self::assertFalse($sut->isKnownLanguage('xx'));
+        self::assertEquals('en', $sut->getDefaultLanguage());
+        self::assertEquals(['de'], $sut->getAllLanguages());
+    }
+
+    public function testMultipleLanguages()
+    {
+        $sut = new LanguageService('de|it|fr|de_CH|ru|hu|en|zh_CN');
+        self::assertTrue($sut->isKnownLanguage('de'));
+        self::assertTrue($sut->isKnownLanguage('en'));
+        self::assertTrue($sut->isKnownLanguage('en'));
+        self::assertFalse($sut->isKnownLanguage('xx'));
+        self::assertEquals('en', $sut->getDefaultLanguage());
+        // casing is important for locales!
+        self::assertEquals(['de', 'it', 'fr', 'de_CH', 'ru', 'hu', 'en', 'zh_CN'], $sut->getAllLanguages());
+    }
+}