perf: short-circuit guest connection and basic perf tests (#17988) (#17991)

* perf: reorder condition to avoid redis call

* test: basic perf tests

(cherry picked from commit f5b8e5f015)

Co-authored-by: Ankush Menat <ankush@frappe.io>

frappe/app.py

@@ -46,7 +46,7 @@ class RequestContext:
 
 @local_manager.middleware
 @Request.application
-def application(request):
+def application(request: Request):
 	response = None
 
 	try:

frappe/auth.py

@@ -226,14 +226,16 @@ class LoginManager:
 
 	def clear_active_sessions(self):
 		"""Clear other sessions of the current user if `deny_multiple_sessions` is not set"""
+		if frappe.session.user == "Guest":
+			return
+
 		if not (
 			cint(frappe.conf.get("deny_multiple_sessions"))
 			or cint(frappe.db.get_system_setting("deny_multiple_sessions"))
 		):
 			return
 
-		if frappe.session.user != "Guest":
-			clear_sessions(frappe.session.user, keep_current=True)
+		clear_sessions(frappe.session.user, keep_current=True)
 
 	def authenticate(self, user: str = None, pwd: str = None):
 		from frappe.core.doctype.user.user import User

frappe/tests/test_perf.py

@@ -0,0 +1,64 @@
+"""
+This file contains multiple primitive tests for avoiding performance regressions.
+
+- Time bound tests: Benchmarks are done on GHA before adding numbers
+- Query count tests: More than expected # of queries for any action is frequent source of
+  performance issues. This guards against such problems.
+
+
+E.g. We know get_controller is supposed to be cached and hence shouldn't make query post first
+query. This test can be written like this.
+
+>>> def test_controller_caching(self):
+>>>
+>>> 	get_controller("User")  # <- "warm up code"
+>>> 	with self.assertQueryCount(0):
+>>> 		get_controller("User")
+
+"""
+import unittest
+
+import frappe
+from frappe.model.base_document import get_controller
+from frappe.tests.utils import FrappeTestCase
+from frappe.website.path_resolver import PathResolver
+
+
+class TestPerformance(FrappeTestCase):
+	def reset_request_specific_caches(self):
+		# To simulate close to request level of handling
+		frappe.destroy()  # releases everything on frappe.local
+		frappe.init(site=self.TEST_SITE)
+		frappe.connect()
+		frappe.clear_cache()
+
+	def setUp(self) -> None:
+		self.reset_request_specific_caches()
+
+	def test_meta_caching(self):
+		frappe.get_meta("User")
+
+		with self.assertQueryCount(0):
+			frappe.get_meta("User")
+
+	def test_controller_caching(self):
+
+		get_controller("User")
+		with self.assertQueryCount(0):
+			get_controller("User")
+
+	def test_db_value_cache(self):
+		"""Link validation if repeated should just use db.value_cache, hence no extra queries"""
+		doc = frappe.get_last_doc("User")
+		doc.get_invalid_links()
+
+		with self.assertQueryCount(0):
+			doc.get_invalid_links()
+
+	@unittest.skip("Not implemented")
+	def test_homepage_resolver(self):
+		paths = ["/", "/app"]
+		for path in paths:
+			PathResolver(path).resolve()
+			with self.assertQueryCount(1):
+				PathResolver(path).resolve()

frappe/tests/utils.py

@@ -19,10 +19,13 @@ class FrappeTestCase(unittest.TestCase):
 	otherwise this class will become ineffective.
 	"""
 
+	TEST_SITE = "test_site"
+
 	SHOW_TRANSACTION_COMMIT_WARNINGS = False
 
 	@classmethod
 	def setUpClass(cls) -> None:
+		cls.TEST_SITE = getattr(frappe.local, "site", None) or cls.TEST_SITE
 		# flush changes done so far to avoid flake
 		frappe.db.commit()
 		frappe.db.begin()
@@ -64,6 +67,21 @@ class FrappeTestCase(unittest.TestCase):
 		else:
 			self.assertEqual(expected, actual, msg=msg)
 
+	@contextmanager
+	def assertQueryCount(self, count):
+		def _sql_with_count(*args, **kwargs):
+			frappe.db.sql_query_count += 1
+			return orig_sql(*args, **kwargs)
+
+		try:
+			orig_sql = frappe.db.sql
+			frappe.db.sql_query_count = 0
+			frappe.db.sql = _sql_with_count
+			yield
+			self.assertLessEqual(frappe.db.sql_query_count, count)
+		finally:
+			frappe.db.sql = orig_sql
+
 
 def _commit_watcher():
 	import traceback