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

3 files changed, 160 insertions, 33 deletions

diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php
index 74f759a4e0d..91233a0c4a7 100644
--- a/lib/private/L10N/Factory.php
+++ b/lib/private/L10N/Factory.php
@@ -1,6 +1,7 @@
 <?php
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @copyright 2016 Roeland Jago Douma <roeland@famdouma.nl>
  *
  * @author Bart Visscher <bartv@thisnet.nl>
  * @author Joas Schilling <coding@schilljs.com>
@@ -8,6 +9,7 @@
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Robin Appelman <robin@icewind.nl>
  * @author Robin McCorkell <robin@mccorkell.me.uk>
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
  *
  * @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;
-		}
-
-		$lang = $this->setLanguageFromRequest($app);
-		if ($userId !== null && $app === null && !$userLang) {
-			$this->config->setUserValue($userId, 'core', 'lang', $lang);
+		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;
+			}
 		}
 
-		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;
 					}
 				}
 			}
 		}
 
+		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';
+		}
+
 		if ($app === null && !$this->requestLanguage) {
-			$this->requestLanguage = 'en';
+			$this->requestLanguage = $requestLanguage;
 		}
-		return 'en'; // Last try: English
+		return $requestLanguage;
 	}
 
 	/**
diff --git a/lib/private/L10N/LanguageNotFoundException.php b/lib/private/L10N/LanguageNotFoundException.php
new file mode 100644
index 00000000000..175f6ab3483
--- /dev/null
+++ b/lib/private/L10N/LanguageNotFoundException.php
@@ -0,0 +1,26 @@
+<?php
+/**
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OC\L10N;
+
+class LanguageNotFoundException extends \Exception {
+
+}
diff --git a/tests/lib/L10N/FactoryTest.php b/tests/lib/L10N/FactoryTest.php
index bb72d84941c..5e422758cb4 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);
+
+	}
 }