From 0228bc6e66cbcb2848eacb41f1de6e7f63ebcb65 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 4 Sep 2016 13:15:46 +0200 Subject: [PATCH 1/2] ACCEPT_LANGUAGE goes before default_langauge See https://github.com/nextcloud/server/issues/970 Before we had 1. Users settings in personal settings 2. Admins default language settings 3. Accept-Language settings of the browser However this is not in line with https://www.w3.org/International/questions/qa-lang-priorities So this changes the order to 1. Users settings in personal settings 3. Accept-Language settings of the browser 2. Admins default language settings --- lib/private/L10N/Factory.php | 59 ++++++---- .../L10N/LanguageNotFoundException.php | 26 +++++ tests/lib/L10N/FactoryTest.php | 110 +++++++++++++++--- 3 files changed, 161 insertions(+), 34 deletions(-) create mode 100644 lib/private/L10N/LanguageNotFoundException.php diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php index 74f759a4e0..91233a0c4a 100644 --- a/lib/private/L10N/Factory.php +++ b/lib/private/L10N/Factory.php @@ -1,6 +1,7 @@ * * @author Bart Visscher * @author Joas Schilling @@ -8,6 +9,7 @@ * @author Morris Jobke * @author Robin Appelman * @author Robin McCorkell + * @author Roeland Jago Douma * * @license AGPL-3.0 * @@ -147,18 +149,23 @@ class Factory implements IFactory { } } - $defaultLanguage = $this->config->getSystemValue('default_language', false); - - if ($defaultLanguage !== false && $this->languageExists($app, $defaultLanguage)) { - return $defaultLanguage; + try { + // Try to get the language from the Request + $lang = $this->getLanguageFromRequest($app); + if ($userId !== null && $app === null && !$userLang) { + $this->config->setUserValue($userId, 'core', 'lang', $lang); + } + return $lang; + } catch (LanguageNotFoundException $e) { + // Finding language from request failed fall back to default language + $defaultLanguage = $this->config->getSystemValue('default_language', false); + if ($defaultLanguage !== false && $this->languageExists($app, $defaultLanguage)) { + return $defaultLanguage; + } } - $lang = $this->setLanguageFromRequest($app); - if ($userId !== null && $app === null && !$userLang) { - $this->config->setUserValue($userId, 'core', 'lang', $lang); - } - - return $lang; + // We could not find any language so fall back to english + return 'en'; } /** @@ -227,10 +234,11 @@ class Factory implements IFactory { } /** - * @param string|null $app App id or null for core + * @param string|null $app * @return string + * @throws LanguageNotFoundException */ - public function setLanguageFromRequest($app = null) { + private function getLanguageFromRequest($app) { $header = $this->request->getHeader('ACCEPT_LANGUAGE'); if ($header) { $available = $this->findAvailableLanguages($app); @@ -245,9 +253,6 @@ class Factory implements IFactory { foreach ($available as $available_language) { if ($preferred_language === strtolower($available_language)) { - if ($app === null && !$this->requestLanguage) { - $this->requestLanguage = $available_language; - } return $available_language; } } @@ -255,19 +260,31 @@ class Factory implements IFactory { // Fallback from de_De to de foreach ($available as $available_language) { if (substr($preferred_language, 0, 2) === $available_language) { - if ($app === null && !$this->requestLanguage) { - $this->requestLanguage = $available_language; - } return $available_language; } } } } - if ($app === null && !$this->requestLanguage) { - $this->requestLanguage = 'en'; + throw new LanguageNotFoundException(); + } + + /** + * @param string|null $app App id or null for core + * @return string + */ + public function setLanguageFromRequest($app = null) { + + try { + $requestLanguage = $this->getLanguageFromRequest($app); + } catch (LanguageNotFoundException $e) { + $requestLanguage = 'en'; } - return 'en'; // Last try: English + + if ($app === null && !$this->requestLanguage) { + $this->requestLanguage = $requestLanguage; + } + return $requestLanguage; } /** diff --git a/lib/private/L10N/LanguageNotFoundException.php b/lib/private/L10N/LanguageNotFoundException.php new file mode 100644 index 0000000000..175f6ab348 --- /dev/null +++ b/lib/private/L10N/LanguageNotFoundException.php @@ -0,0 +1,26 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OC\L10N; + +class LanguageNotFoundException extends \Exception { + +} diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php index bb72d84941..5e422758cb 100644 --- a/tests/lib/L10N/FactoryTest.php +++ b/tests/lib/L10N/FactoryTest.php @@ -9,6 +9,10 @@ namespace Test\L10N; use OC\L10N\Factory; +use OCP\IConfig; +use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; use Test\TestCase; /** @@ -18,13 +22,13 @@ use Test\TestCase; */ class FactoryTest extends TestCase { - /** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ protected $config; - /** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ protected $request; - /** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ protected $userSession; /** @var string */ @@ -33,17 +37,15 @@ class FactoryTest extends TestCase { public function setUp() { parent::setUp(); - /** @var \OCP\IConfig $request */ - $this->config = $this->getMockBuilder('OCP\IConfig') + $this->config = $this->getMockBuilder(IConfig::class) ->disableOriginalConstructor() ->getMock(); - /** @var \OCP\IRequest $request */ - $this->request = $this->getMockBuilder('OCP\IRequest') + $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor() ->getMock(); - $this->userSession = $this->getMockBuilder('\OCP\IUserSession') + $this->userSession = $this->getMockBuilder(IUserSession::class) ->disableOriginalConstructor() ->getMock(); @@ -56,7 +58,7 @@ class FactoryTest extends TestCase { */ protected function getFactory(array $methods = []) { if (!empty($methods)) { - return $this->getMockBuilder('OC\L10N\Factory') + return $this->getMockBuilder(Factory::class) ->setConstructorArgs([ $this->config, $this->request, @@ -111,7 +113,7 @@ class FactoryTest extends TestCase { ->method('getSystemValue') ->with('installed', false) ->willReturn(true); - $user = $this->getMockBuilder('\OCP\IUser') + $user = $this->getMockBuilder(IUser::class) ->getMock(); $user->expects($this->once()) ->method('getUID') @@ -145,7 +147,7 @@ class FactoryTest extends TestCase { ->method('getSystemValue') ->with('installed', false) ->willReturn(true); - $user = $this->getMockBuilder('\OCP\IUser') + $user = $this->getMockBuilder(IUser::class) ->getMock(); $user->expects($this->once()) ->method('getUID') @@ -188,7 +190,7 @@ class FactoryTest extends TestCase { ->method('getSystemValue') ->with('installed', false) ->willReturn(true); - $user = $this->getMockBuilder('\OCP\IUser') + $user = $this->getMockBuilder(IUser::class) ->getMock(); $user->expects($this->once()) ->method('getUID') @@ -234,7 +236,7 @@ class FactoryTest extends TestCase { ->method('getSystemValue') ->with('installed', false) ->willReturn(true); - $user = $this->getMockBuilder('\OCP\IUser') + $user = $this->getMockBuilder(IUser::class) ->getMock(); $user->expects($this->once()) ->method('getUID') @@ -460,4 +462,86 @@ class FactoryTest extends TestCase { $fn = $factory->createPluralFunction($function); $this->assertEquals($expected, $fn($count)); } + + public function dataFindLanguage() { + return [ + // Not logged in + [false, [], 'en'], + [false, ['fr'], 'fr'], + [false, ['de', 'fr'], 'de'], + [false, ['nl', 'de', 'fr'], 'de'], + + [true, [], 'en'], + [true, ['fr'], 'fr'], + [true, ['de', 'fr'], 'de'], + [true, ['nl', 'de', 'fr'], 'nl'], + ]; + } + + /** + * @dataProvider dataFindLanguage + * + * @param bool $loggedIn + * @param array $availableLang + * @param string $expected + */ + public function testFindLanguage($loggedIn, $availableLang, $expected) { + $userLang = 'nl'; + $browserLang = 'de'; + $defaultLang = 'fr'; + + $this->config->expects($this->any()) + ->method('getSystemValue') + ->will($this->returnCallback(function($var, $default) use ($defaultLang) { + if ($var === 'installed') { + return true; + } else if ($var === 'default_language') { + return $defaultLang; + } else { + return $default; + } + })); + + if ($loggedIn) { + $user = $this->getMockBuilder(IUser::class) + ->getMock(); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('MyUserUid'); + $this->userSession + ->expects($this->any()) + ->method('getUser') + ->willReturn($user); + $this->config->expects($this->any()) + ->method('getUserValue') + ->with('MyUserUid', 'core', 'lang', null) + ->willReturn($userLang); + } else { + $this->userSession + ->expects($this->any()) + ->method('getUser') + ->willReturn(null); + } + + $this->request->expects($this->any()) + ->method('getHeader') + ->with($this->equalTo('ACCEPT_LANGUAGE')) + ->willReturn($browserLang); + + $factory = $this->getFactory(['languageExists', 'findAvailableLanguages']); + $factory->expects($this->any()) + ->method('languageExists') + ->will($this->returnCallback(function ($app, $lang) use ($availableLang) { + return in_array($lang, $availableLang); + })); + $factory->expects($this->any()) + ->method('findAvailableLanguages') + ->will($this->returnCallback(function ($app) use ($availableLang) { + return $availableLang; + })); + + $lang = $factory->findLanguage(null); + $this->assertSame($expected, $lang); + + } } From 933c892a62c120c93502f283ce7d7842e2a30692 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sun, 4 Sep 2016 13:39:22 +0200 Subject: [PATCH 2/2] Autoloader update --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1363d96349..2700c87ec0 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -550,6 +550,7 @@ return array( 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => $baseDir . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\L10N\\Factory' => $baseDir . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => $baseDir . '/lib/private/L10N/L10N.php', + 'OC\\L10N\\LanguageNotFoundException' => $baseDir . '/lib/private/L10N/LanguageNotFoundException.php', 'OC\\LargeFileHelper' => $baseDir . '/lib/private/LargeFileHelper.php', 'OC\\Lock\\AbstractLockingProvider' => $baseDir . '/lib/private/Lock/AbstractLockingProvider.php', 'OC\\Lock\\DBLockingProvider' => $baseDir . '/lib/private/Lock/DBLockingProvider.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 42b6fc8321..3797711f91 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -580,6 +580,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\IntegrityCheck\\Iterator\\ExcludeFoldersByPathFilterIterator' => __DIR__ . '/../../..' . '/lib/private/IntegrityCheck/Iterator/ExcludeFoldersByPathFilterIterator.php', 'OC\\L10N\\Factory' => __DIR__ . '/../../..' . '/lib/private/L10N/Factory.php', 'OC\\L10N\\L10N' => __DIR__ . '/../../..' . '/lib/private/L10N/L10N.php', + 'OC\\L10N\\LanguageNotFoundException' => __DIR__ . '/../../..' . '/lib/private/L10N/LanguageNotFoundException.php', 'OC\\LargeFileHelper' => __DIR__ . '/../../..' . '/lib/private/LargeFileHelper.php', 'OC\\Lock\\AbstractLockingProvider' => __DIR__ . '/../../..' . '/lib/private/Lock/AbstractLockingProvider.php', 'OC\\Lock\\DBLockingProvider' => __DIR__ . '/../../..' . '/lib/private/Lock/DBLockingProvider.php',