vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

perf: Lazily fetch shared documents

Browse source 
We eagerly fetch shared documents for ANY `get_list` query, even when
user has full read acess doctype, where it's moot to consider adding
shared document as separately.

This eliminates one entire db call from get_list and in most cases
get_list will translate to single DB call, hence probably worth the
additional complexity.
This commit is contained in:
Ankush Menat 2023-07-22 17:44:53 +05:30
parent eed90a871b
commit 2c99583247
2 changed files with 29 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
frappe/model/db_query.py
View file

@ -69,6 +69,8 @@ class DatabaseQuery:
self.flags = frappe._dict()
self.reference_doctype = None
self.permission_map = {}
self.shared = []
self._fetch_shared_documents = False
@property
def doctype_meta(self):
@ -895,8 +897,6 @@ class DatabaseQuery:
self.extract_tables()
role_permissions = frappe.permissions.get_role_permissions(self.doctype_meta, user=self.user)
self.shared = frappe.share.get_shared(self.doctype, self.user)
if (
not self.doctype_meta.istable
and not (role_permissions.get("select") or role_permissions.get("read"))
@ -904,6 +904,7 @@ class DatabaseQuery:
and not has_any_user_permission_for_doctype(self.doctype, self.user, self.reference_doctype)
):
only_if_shared = True
self.shared = frappe.share.get_shared(self.doctype, self.user)
if not self.shared:
frappe.throw(_("No permission to read {0}").format(_(self.doctype)), frappe.PermissionError)
else:
@ -912,6 +913,7 @@ class DatabaseQuery:
else:
# skip user perm check if owner constraint is required
if requires_owner_constraint(role_permissions):
self._fetch_shared_documents = True
self.match_conditions.append(
f"`tab{self.doctype}`.`owner` = {frappe.db.escape(self.user, percent=False)}"
)
@ -922,6 +924,14 @@ class DatabaseQuery:
user_permissions = frappe.permissions.get_user_permissions(self.user)
self.add_user_permissions(user_permissions)
# Only when full read access is not present fetch shared docuemnts.
# This is done to avoid extra query.
# Only following cases can require explicit addition of shared documents.
# 1. DocType has if_owner constraint and hence can't see shared documents
# 2. DocType has user permissions and hence can't see shared documents
if self._fetch_shared_documents:
self.shared = frappe.share.get_shared(self.doctype, self.user)
if as_condition:
conditions = ""
if self.match_conditions:
@ -1000,9 +1010,11 @@ class DatabaseQuery:
match_filters[df.get("options")] = docs
if match_conditions:
self._fetch_shared_documents = True
self.match_conditions.append(" and ".join(match_conditions))
if match_filters:
self._fetch_shared_documents = True
self.match_filters.append(match_filters)
def get_permission_query_conditions(self):

16
frappe/tests/test_perf.py
View file

@ -17,7 +17,6 @@ query. This test can be written like this.
"""
import time
import unittest
from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
@ -30,6 +29,8 @@ from frappe.tests.utils import FrappeTestCase
from frappe.utils import cint
from frappe.website.path_resolver import PathResolver
TEST_USER = "test@example.com"
@run_only_if(db_type_is.MARIADB)
class TestPerformance(FrappeTestCase):
@ -144,3 +145,16 @@ class TestPerformance(FrappeTestCase):
from frappe.utils import get_build_version
self.assertEqual(get_build_version(), get_build_version())
def test_get_list_single_query(self):
"""get_list should only perform single query."""
user = frappe.get_doc("User", TEST_USER)
frappe.set_user(TEST_USER)
# Give full read access, no share/user perm check should be done.
user.add_roles("System Manager")
frappe.get_list("User")
with self.assertQueryCount(1):
frappe.get_list("User")