From 3a7dba437c9ccbbcda5bca5ebd4705a628c03ecc Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sat, 7 Aug 2021 01:11:42 +0200 Subject: [PATCH] move supported languages logic to service (#2701) --- config/services.yaml | 13 ++--- src/Controller/HomepageController.php | 9 +++- .../RedirectToLocaleSubscriber.php | 51 +++++-------------- src/Form/Type/LanguageType.php | 19 ++----- src/Utils/LanguageService.php | 49 ++++++++++++++++++ .../RedirectToLocaleSubscriberTest.php | 12 +---- tests/Utils/LanguageServiceTest.php | 51 +++++++++++++++++++ 7 files changed, 130 insertions(+), 74 deletions(-) create mode 100644 src/Utils/LanguageService.php create mode 100644 tests/Utils/LanguageServiceTest.php diff --git a/config/services.yaml b/config/services.yaml index 09d38b05..2aab3c2b 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -35,8 +35,8 @@ services: security.user.provider.chain: class: App\Security\KimaiUserProvider - App\EventSubscriber\RedirectToLocaleSubscriber: - arguments: ['@router', '%app_locales%', '%locale%'] + App\Utils\LanguageService: + 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 diff --git a/src/Controller/HomepageController.php b/src/Controller/HomepageController.php index 07a1e218..f7d18a15 100644 --- a/src/Controller/HomepageController.php +++ b/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], diff --git a/src/EventSubscriber/RedirectToLocaleSubscriber.php b/src/EventSubscriber/RedirectToLocaleSubscriber.php index b8bf1289..8a6f0bf5 100644 --- a/src/EventSubscriber/RedirectToLocaleSubscriber.php +++ b/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; - /** - * 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) + public function __construct(UrlGeneratorInterface $urlGenerator, LanguageService $languageService) { $this->urlGenerator = $urlGenerator; - - $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); + $this->languageService = $languageService; } 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); diff --git a/src/Form/Type/LanguageType.php b/src/Form/Type/LanguageType.php index fc8f1ac2..f636cfd6 100644 --- a/src/Form/Type/LanguageType.php +++ b/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 { - /** - * @var string[] - */ - private $locales = []; + private $languageService; - /** - * @param array|string $locales - */ - public function __construct($locales) + public function __construct(LanguageService $languageService) { - if (!\is_array($locales)) { - $locales = explode('|', $locales); - } - - $this->locales = $locales; + $this->languageService = $languageService; } /** @@ -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; } diff --git a/src/Utils/LanguageService.php b/src/Utils/LanguageService.php new file mode 100644 index 00000000..e31bed0e --- /dev/null +++ b/src/Utils/LanguageService.php @@ -0,0 +1,49 @@ +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; + } +} diff --git a/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php b/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php index da6e8a6a..998270d6 100644 --- a/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php +++ b/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'); - } } diff --git a/tests/Utils/LanguageServiceTest.php b/tests/Utils/LanguageServiceTest.php new file mode 100644 index 00000000..8383ab68 --- /dev/null +++ b/tests/Utils/LanguageServiceTest.php @@ -0,0 +1,51 @@ +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()); + } +}