Merge pull request #13703 from gavindsouza/lang-resolution

refactor: Request Language Resolution
2021-07-23 19:42:37 +05:30 · 2021-07-23 19:42:37 +05:30 · 2e71f53b3d
commit 2e71f53b3d
parent e3ea5cd05a 42b3c17800
10 changed files with 277 additions and 156 deletions
--- a/.github/semantic.yml
+++ b/.github/semantic.yml
@ -11,3 +11,20 @@ allowRevertCommits: true

 # For allowed PR types: https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json
 # Tool Reference: https://github.com/zeke/semantic-pull-requests
+
+# By default types specified in commitizen/conventional-commit-types is used.
+# See: https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json
+# You can override the valid types
+types:
+  - BREAKING CHANGE
+  - feat
+  - fix
+  - docs
+  - style
+  - refactor
+  - perf
+  - test
+  - build
+  - ci
+  - chore
+  - revert
--- a/frappe/auth.py
+++ b/frappe/auth.py
@ -1,31 +1,58 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# MIT License. See license.txt
-import datetime
-from frappe import _
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See LICENSE
+from urllib.parse import quote
+
 import frappe
 import frappe.database
 import frappe.utils
-from frappe.utils import cint, flt, get_datetime, datetime, date_diff, today
 import frappe.utils.user
-from frappe import conf
-from frappe.sessions import Session, clear_sessions, delete_session
-from frappe.modules.patch_handler import check_session_stopped
-from frappe.translate import get_lang_code
-from frappe.utils.password import check_password, delete_login_failed_cache
+from frappe import _, conf
 from frappe.core.doctype.activity_log.activity_log import add_authentication_log
-from frappe.twofactor import (should_run_2fa, authenticate_for_2factor,
-	confirm_otp_token, get_cached_user_pass)
+from frappe.modules.patch_handler import check_session_stopped
+from frappe.sessions import Session, clear_sessions, delete_session
+from frappe.translate import get_language
+from frappe.twofactor import authenticate_for_2factor, confirm_otp_token, get_cached_user_pass, should_run_2fa
+from frappe.utils import cint, date_diff, datetime, get_datetime, today
+from frappe.utils.password import check_password
 from frappe.website.utils import get_home_page
-from urllib.parse import quote


 class HTTPRequest:
 	def __init__(self):
-		# Get Environment variables
-		self.domain = frappe.request.host
-		if self.domain and self.domain.startswith('www.'):
-			self.domain = self.domain[4:]
+		# set frappe.local.request_ip
+		self.set_request_ip()

+		# load cookies
+		self.set_cookies()
+
+		# set frappe.local.db
+		self.connect()
+
+		# login and start/resume user session
+		self.set_session()
+
+		# set request language
+		self.set_lang()
+
+		# match csrf token from current session
+		self.validate_csrf_token()
+
+		# write out latest cookies
+		frappe.local.cookie_manager.init_cookies()
+
+		# check session status
+		check_session_stopped()
+
+	@property
+	def domain(self):
+		if not getattr(self, "_domain", None):
+			self._domain = frappe.request.host
+			if self._domain and self._domain.startswith('www.'):
+				self._domain = self._domain[4:]
+
+		return self._domain
+
+	def set_request_ip(self):
 		if frappe.get_request_header('X-Forwarded-For'):
 			frappe.local.request_ip = (frappe.get_request_header('X-Forwarded-For').split(",")[0]).strip()

@ -35,37 +62,21 @@ class HTTPRequest:
 		else:
 			frappe.local.request_ip = '127.0.0.1'

-		# language
-		self.set_lang()
-
-		# load cookies
+	def set_cookies(self):
 		frappe.local.cookie_manager = CookieManager()

-		# set db
-		self.connect()
-
-		# login
+	def set_session(self):
 		frappe.local.login_manager = LoginManager()

-		if frappe.form_dict._lang:
-			lang = get_lang_code(frappe.form_dict._lang)
-			if lang:
-				frappe.local.lang = lang
-
-		self.validate_csrf_token()
-
-		# write out latest cookies
-		frappe.local.cookie_manager.init_cookies()
-
-		# check status
-		check_session_stopped()
-
 	def validate_csrf_token(self):
 		if frappe.local.request and frappe.local.request.method in ("POST", "PUT", "DELETE"):
-			if not frappe.local.session: return
-			if not frappe.local.session.data.csrf_token \
-				or frappe.local.session.data.device=="mobile" \
-				or frappe.conf.get('ignore_csrf', None):
+			if not frappe.local.session:
+				return
+			if (
+				not frappe.local.session.data.csrf_token
+				or frappe.local.session.data.device == "mobile"
+				or frappe.conf.get('ignore_csrf', None)
+			):
 				# not via boot
 				return

@ -79,17 +90,18 @@ class HTTPRequest:
 				frappe.throw(_("Invalid Request"), frappe.CSRFTokenError)

 	def set_lang(self):
-		from frappe.translate import guess_language
-		frappe.local.lang = guess_language()
+		frappe.local.lang = get_language()

 	def get_db_name(self):
 		"""get database name from conf"""
 		return conf.db_name

-	def connect(self, ac_name = None):
+	def connect(self):
 		"""connect to db, from ac_name or db_name"""
-		frappe.local.db = frappe.database.get_db(user = self.get_db_name(), \
-			password = getattr(conf, 'db_password', ''))
+		frappe.local.db = frappe.database.get_db(
+			user=self.get_db_name(),
+			password=getattr(conf, 'db_password', '')
+		)

 class LoginManager:
 	def __init__(self):
@ -142,8 +154,9 @@ class LoginManager:
 		self.make_session()
 		self.setup_boot_cache()
 		self.set_user_info()
+		self.clear_preferred_language()

-	def get_user_info(self, resume=False):
+	def get_user_info(self):
 		self.info = frappe.db.get_value("User", self.user,
 			["user_type", "first_name", "last_name", "user_image"], as_dict=1)

@ -181,11 +194,13 @@ class LoginManager:
 			frappe.local.response["redirect_to"] = redirect_to
 			frappe.cache().hdel('redirect_after_login', self.user)

-
 		frappe.local.cookie_manager.set_cookie("full_name", self.full_name)
 		frappe.local.cookie_manager.set_cookie("user_id", self.user)
 		frappe.local.cookie_manager.set_cookie("user_image", self.info.user_image or "")

+	def clear_preferred_language(self):
+		frappe.local.cookie_manager.delete_cookie("preferred_language")
+
 	def make_session(self, resume=False):
 		# start session
 		frappe.local.session_obj = Session(user=self.user, resume=resume,
--- a/frappe/commands/utils.py
+++ b/frappe/commands/utils.py
@ -551,32 +551,16 @@ def run_tests(context, app=None, module=None, doctype=None, test=(), profile=Fal

 	if coverage:
 		from coverage import Coverage
+		from frappe.coverage import STANDARD_INCLUSIONS, STANDARD_EXCLUSIONS, FRAPPE_EXCLUSIONS

 		# Generate coverage report only for app that is being tested
 		source_path = os.path.join(get_bench_path(), 'apps', app or 'frappe')
-		incl = [
-			'*.py',
-		]
-		omit = [
-			'*.js',
-			'*.xml',
-			'*.pyc',
-			'*.css',
-			'*.less',
-			'*.scss',
-			'*.vue',
-			'*.html',
-			'*/test_*',
-			'*/node_modules/*',
-			'*/doctype/*/*_dashboard.py',
-			'*/patches/*',
-		]
+		omit = STANDARD_EXCLUSIONS[:]

 		if not app or app == 'frappe':
-			omit.append('*/tests/*')
-			omit.append('*/commands/*')
+			omit.extend(FRAPPE_EXCLUSIONS)

-		cov = Coverage(source=[source_path], omit=omit, include=incl)
+		cov = Coverage(source=[source_path], omit=omit, include=STANDARD_INCLUSIONS)
 		cov.start()

 	ret = frappe.test_runner.main(app, module, doctype, context.verbose, tests=tests,
@ -786,7 +770,7 @@ def get_version(output):
 		"table": lambda: render_table(
 			[["App", "Version", "Branch", "Commit"]] +
 			[
-				[app_info.app, app_info.version, app_info.branch, app_info.commit] 
+				[app_info.app, app_info.version, app_info.branch, app_info.commit]
 				for app_info in data
 			]
 		),
--- a/frappe/coverage.py
+++ b/frappe/coverage.py
@ -0,0 +1,35 @@
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
+# MIT License. See LICENSE
+"""
+	frappe.coverage
+	~~~~~~~~~~~~~~~~
+
+	Coverage settings for frappe
+"""
+
+STANDARD_INCLUSIONS = ["*.py"]
+
+STANDARD_EXCLUSIONS = [
+	'*.js',
+	'*.xml',
+	'*.pyc',
+	'*.css',
+	'*.less',
+	'*.scss',
+	'*.vue',
+	'*.html',
+	'*/test_*',
+	'*/node_modules/*',
+	'*/doctype/*/*_dashboard.py',
+	'*/patches/*',
+]
+
+FRAPPE_EXCLUSIONS = [
+	"*/tests/*",
+	"*/commands/*",
+	"*/frappe/change_log/*",
+	"*/frappe/exceptions*",
+	"*frappe/setup.py",
+	"*/doctype/*/*_dashboard.py",
+	"*/patches/*",
+]
--- a/frappe/parallel_test_runner.py
+++ b/frappe/parallel_test_runner.py
@ -111,33 +111,16 @@ class ParallelTestRunner():
 		if self.with_coverage:
 			from coverage import Coverage
 			from frappe.utils import get_bench_path
+			from frappe.coverage import STANDARD_INCLUSIONS, STANDARD_EXCLUSIONS, FRAPPE_EXCLUSIONS

 			# Generate coverage report only for app that is being tested
 			source_path = os.path.join(get_bench_path(), 'apps', self.app)
-			incl = [
-				'*.py',
-			]
-			omit = [
-				'*.js',
-				'*.xml',
-				'*.pyc',
-				'*.css',
-				'*.less',
-				'*.scss',
-				'*.vue',
-				'*.pyc',
-				'*.html',
-				'*/test_*',
-				'*/node_modules/*',
-				'*/doctype/*/*_dashboard.py',
-				'*/patches/*',
-			]
+			omit = STANDARD_EXCLUSIONS[:]

 			if self.app == 'frappe':
-				omit.append('*/tests/*')
-				omit.append('*/commands/*')
+				omit.extend(FRAPPE_EXCLUSIONS)

-			self.coverage = Coverage(source=[source_path], omit=omit, include=incl)
+			self.coverage = Coverage(source=[source_path], omit=omit, include=STANDARD_INCLUSIONS)
 			self.coverage.start()

 	def save_coverage(self):
--- a/frappe/pythonrc.py
+++ b/frappe/pythonrc.py
@ -1,8 +0,0 @@
-#!/usr/bin/env python2.7
-
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
-# MIT License. See license.txt
-
-import os
-import frappe
-frappe.connect(site=os.environ.get("site"))
--- a/frappe/sessions.py
+++ b/frappe/sessions.py
@ -1,4 +1,4 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See license.txt
 """
 Boot session from cache or build
@ -249,7 +249,6 @@ class Session:
 		data = self.get_session_record()

 		if data:
-			# set language
 			self.data.update({'data': data, 'user':data.user, 'sid': self.sid})
 			self.user = data.user
 			validate_ip_address(self.user)
--- a/frappe/tests/test_auth.py
+++ b/frappe/tests/test_auth.py
@ -4,8 +4,9 @@ import time
 import unittest

 import frappe
-from frappe.auth import LoginAttemptTracker
+from frappe.auth import HTTPRequest, LoginAttemptTracker
 from frappe.frappeclient import FrappeClient, AuthError
+from frappe.utils import set_request

 class TestAuth(unittest.TestCase):
 	def __init__(self, *args, **kwargs):
@ -124,3 +125,20 @@ class TestLoginAttemptTracker(unittest.TestCase):

 		tracker.add_failure_attempt()
 		self.assertTrue(tracker.is_user_allowed())
+
+class TestFrappeHTTPRequest(unittest.TestCase):
+	# test frappe.auth.HTTPRequest
+	def test_set_language(self):
+		"""Check if language is set on object initialization
+
+		This is a test to ensure that language has changed. To test correctness
+		of frappe.local.lang, check out the tests of frappe.translate.get_language
+		"""
+		lang_before_request = frappe.local.lang
+		random_lang = frappe.get_all("Language", limit=1, pluck="name")[0]
+		set_request(method='POST', path='/')
+		frappe.form_dict._lang = random_lang
+		HTTPRequest()
+		self.assertTrue(hasattr(frappe.local, "lang"))
+		self.assertIsInstance(frappe.local.lang, str)
+		self.assertNotEqual(lang_before_request, frappe.local.lang)
--- a/frappe/tests/test_translate.py
+++ b/frappe/tests/test_translate.py
@ -1,13 +1,26 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See license.txt
-import frappe, unittest, os
+import os
+import unittest
+from random import choices
+from unittest.mock import patch
+
+import frappe
 import frappe.translate
 from frappe import _
+from frappe.translate import get_language, get_parent_language
+from frappe.utils import set_request

 dirname = os.path.dirname(__file__)
 translation_string_file = os.path.join(dirname, 'translation_test_file.txt')
+first_lang, second_lang, third_lang, fourth_lang, fifth_lang = choices(
+	frappe.get_all("Language", pluck="name"), k=5
+)

 class TestTranslate(unittest.TestCase):
+	def tearDown(self):
+		frappe.form_dict.pop("_lang", None)
+
 	def test_extract_message_from_file(self):
 		data = frappe.translate.get_messages_from_file(translation_string_file)
 		self.assertListEqual(data, expected_output)
@ -20,6 +33,41 @@ class TestTranslate(unittest.TestCase):
 		finally:
 			frappe.local.lang = 'en'

+	def test_request_language_resolution_with_form_dict(self):
+		"""Test for frappe.translate.get_language
+
+		Case 1: frappe.form_dict._lang is set
+		"""
+
+		frappe.form_dict._lang = first_lang
+
+		with patch.object(frappe.translate, "get_preferred_language_cookie", return_value=second_lang):
+			return_val = get_language()
+
+		self.assertIn(return_val, [first_lang, get_parent_language(first_lang)])
+
+	def test_request_language_resolution_with_cookie(self):
+		"""Test for frappe.translate.get_language
+
+		Case 2: frappe.form_dict._lang is not set, but preferred_language cookie is
+		"""
+
+		with patch.object(frappe.translate, "get_preferred_language_cookie", return_value=second_lang):
+			set_request(method="POST", path="/", headers=[("Accept-Language", third_lang)])
+			return_val = get_language()
+
+		self.assertIn(return_val, [second_lang, get_parent_language(second_lang)])
+
+	def test_request_language_resolution_with_request_header(self):
+		"""Test for frappe.translate.get_language
+
+		Case 3: frappe.form_dict._lang & preferred_language cookie is not set, but Accept-Language header is
+		"""
+		set_request(method="POST", path="/", headers=[("Accept-Language", third_lang)])
+		return_val = get_language()
+		self.assertIn(return_val, [third_lang, get_parent_language(third_lang)])
+
+
 expected_output = [
 	('apps/frappe/frappe/tests/translation_test_file.txt', 'Warning: Unable to find {0} in any table related to {1}', 'This is some context', 2),
 	('apps/frappe/frappe/tests/translation_test_file.txt', 'Warning: Unable to find {0} in any table related to {1}', None, 4),
--- a/frappe/translate.py
+++ b/frappe/translate.py
@ -1,4 +1,4 @@
-# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See license.txt
 """
 	frappe.translate
@ -11,71 +11,100 @@ import io
 import itertools
 import json
 import operator
+import functools
 import os
 import re
 from csv import reader
+from typing import List, Union, Tuple

 import frappe
 from frappe.model.utils import InvalidIncludePath, render_include
-from frappe.utils import is_html, strip, strip_html_tags
+from frappe.utils import get_bench_path, is_html, strip, strip_html_tags


-def guess_language(lang_list=None):
-	"""Set `frappe.local.lang` from HTTP headers at beginning of request"""
-	user_preferred_language = frappe.request.cookies.get('preferred_language')
-	is_guest_user = not frappe.session.user or frappe.session.user == 'Guest'
-	if is_guest_user and user_preferred_language:
-		return user_preferred_language
+def get_language(lang_list: List = None) -> str:
+	"""Set `frappe.local.lang` from HTTP headers at beginning of request

-	lang_codes = frappe.request.accept_languages.values()
-	if not lang_codes:
-		return frappe.local.lang
+	Order of priority for setting language:
+	1. Form Dict => _lang
+	2. Cookie => preferred_language
+	3. Request Header => Accept-Language
+	4. User document => language
+	5. System Settings => language
+	"""

-	guess = None
-	if not lang_list:
-		lang_list = get_all_languages() or []
+	# fetch language from form_dict
+	if frappe.form_dict._lang:
+		language = get_lang_code(
+			frappe.form_dict._lang or get_parent_language(frappe.form_dict._lang)
+		)
+		if language:
+			return language

-	for l in lang_codes:
-		code = l.strip()
-		if not isinstance(code, str):
-			code = str(code, 'utf-8')
-		if code in lang_list or code == "en":
-			guess = code
-			break
+	lang_set = set(lang_list or get_all_languages() or [])

-		# check if parent language (pt) is setup, if variant (pt-BR)
-		if "-" in code:
-			code = code.split("-")[0]
-			if code in lang_list:
-				guess = code
-				break
+	# fetch language from cookie
+	preferred_language_cookie = get_preferred_language_cookie()

-	return guess or frappe.local.lang
+	if preferred_language_cookie:
+		if preferred_language_cookie in lang_set:
+			return preferred_language_cookie

-def get_user_lang(user=None):
+		parent_language = get_parent_language(language)
+		if parent_language in lang_set:
+			return parent_language
+
+	# fetch language from request headers
+	accept_language = list(frappe.request.accept_languages.values())
+
+	for language in accept_language:
+		if language in lang_set:
+			return language
+
+		parent_language = get_parent_language(language)
+		if parent_language in lang_set:
+			return parent_language
+
+	# fallback to language set in User or System Settings
+	return frappe.local.lang
+
+
+@functools.lru_cache()
+def get_parent_language(lang: str) -> str:
+	"""If the passed language is a variant, return its parent
+
+	Eg:
+		1. zh-TW -> zh
+		2. sr-BA -> sr
+	"""
+	is_language_variant = "-" in lang
+	if is_language_variant:
+		return lang[:lang.index("-")]
+
+
+def get_user_lang(user: str = None) -> str:
 	"""Set frappe.local.lang from user preferences on session beginning or resumption"""
-	if not user:
-		user = frappe.session.user
-
-	# via cache
+	user = user or frappe.session.user
 	lang = frappe.cache().hget("lang", user)

 	if not lang:
-
-		# if defined in user profile
-		lang = frappe.db.get_value("User", user, "language")
-		if not lang:
-			lang = frappe.db.get_default("lang")
-
-		if not lang:
-			lang = frappe.local.lang or 'en'
+		# User.language => Session Defaults => frappe.local.lang => 'en'
+		lang = (
+			frappe.db.get_value("User", user, "language")
+			or frappe.db.get_default("lang")
+			or frappe.local.lang
+			or "en"
+		)

 		frappe.cache().hset("lang", user, lang)

 	return lang

-def get_lang_code(lang):
-	return frappe.db.get_value('Language', {'language_name': lang}) or lang
+def get_lang_code(lang: str) -> Union[str, None]:
+	return (
+		frappe.db.get_value("Language", {"name": lang})
+		or frappe.db.get_value("Language", {"language_name": lang})
+	)

 def set_default_language(lang):
 	"""Set Global default language"""
@ -529,7 +558,7 @@ def get_all_messages_from_js_files(app_name=None):

 	return messages

-def get_messages_from_file(path):
+def get_messages_from_file(path: str) -> List[Tuple[str, str, str, str]]:
 	"""Returns a list of transatable strings from a code file

 	:param path: path of the code file
@ -542,7 +571,7 @@ def get_messages_from_file(path):

 	frappe.flags.scanned_files.append(path)

-	apps_path = get_bench_dir()
+	bench_path = get_bench_path()
 	if os.path.exists(path):
 		with open(path, 'r') as sourcefile:
 			try:
@ -550,11 +579,12 @@ def get_messages_from_file(path):
 			except Exception:
 				print("Could not scan file for translation: {0}".format(path))
 				return []
-			data = [(os.path.relpath(path, apps_path), message, context, line) \
-				for line, message, context in extract_messages_from_code(file_contents)]
-			return data
+
+			return [
+				(os.path.relpath(path, bench_path), message, context, line)
+				for (line, message, context) in extract_messages_from_code(file_contents)
+			]
 	else:
-		# print "Translate: {0} missing".format(os.path.abspath(path))
 		return []

 def extract_messages_from_code(code):
@ -768,9 +798,6 @@ def deduplicate_messages(messages):
 		ret.append(next(g))
 	return ret

-def get_bench_dir():
-	return os.path.join(frappe.__file__, '..', '..', '..', '..')
-
 def rename_language(old_name, new_name):
 	if not frappe.db.exists('Language', new_name):
 		return
@ -879,3 +906,6 @@ def get_all_languages(with_language_name=False):
@frappe.whitelist(allow_guest=True)
 def set_preferred_language_cookie(preferred_language):
 	frappe.local.cookie_manager.set_cookie("preferred_language", preferred_language)
+
+def get_preferred_language_cookie():
+	return frappe.request.cookies.get("preferred_language")