* perf: reorder condition to avoid redis call
* test: basic perf tests
(cherry picked from commit f5b8e5f015
)
Co-authored-by: Ankush Menat <ankush@frappe.io>
version-14
@@ -46,7 +46,7 @@ class RequestContext: | |||||
@local_manager.middleware | @local_manager.middleware | ||||
@Request.application | @Request.application | ||||
def application(request): | |||||
def application(request: Request): | |||||
response = None | response = None | ||||
try: | try: | ||||
@@ -226,14 +226,16 @@ class LoginManager: | |||||
def clear_active_sessions(self): | def clear_active_sessions(self): | ||||
"""Clear other sessions of the current user if `deny_multiple_sessions` is not set""" | """Clear other sessions of the current user if `deny_multiple_sessions` is not set""" | ||||
if frappe.session.user == "Guest": | |||||
return | |||||
if not ( | if not ( | ||||
cint(frappe.conf.get("deny_multiple_sessions")) | cint(frappe.conf.get("deny_multiple_sessions")) | ||||
or cint(frappe.db.get_system_setting("deny_multiple_sessions")) | or cint(frappe.db.get_system_setting("deny_multiple_sessions")) | ||||
): | ): | ||||
return | 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): | def authenticate(self, user: str = None, pwd: str = None): | ||||
from frappe.core.doctype.user.user import User | from frappe.core.doctype.user.user import User | ||||
@@ -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() |
@@ -19,10 +19,13 @@ class FrappeTestCase(unittest.TestCase): | |||||
otherwise this class will become ineffective. | otherwise this class will become ineffective. | ||||
""" | """ | ||||
TEST_SITE = "test_site" | |||||
SHOW_TRANSACTION_COMMIT_WARNINGS = False | SHOW_TRANSACTION_COMMIT_WARNINGS = False | ||||
@classmethod | @classmethod | ||||
def setUpClass(cls) -> None: | def setUpClass(cls) -> None: | ||||
cls.TEST_SITE = getattr(frappe.local, "site", None) or cls.TEST_SITE | |||||
# flush changes done so far to avoid flake | # flush changes done so far to avoid flake | ||||
frappe.db.commit() | frappe.db.commit() | ||||
frappe.db.begin() | frappe.db.begin() | ||||
@@ -64,6 +67,21 @@ class FrappeTestCase(unittest.TestCase): | |||||
else: | else: | ||||
self.assertEqual(expected, actual, msg=msg) | 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(): | def _commit_watcher(): | ||||
import traceback | import traceback | ||||