refactor: ensure no meta recursion
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
This commit is contained in:
parent
a5e44c4c6e
commit
7ad6f7e2c6
7 changed files with 48 additions and 98 deletions
|
|
@ -550,8 +550,6 @@ class DocType(Document):
|
|||
self.sync_doctype_layouts()
|
||||
delete_notification_count_for(doctype=self.name)
|
||||
|
||||
self._clear_sort_cache_if_needed()
|
||||
|
||||
frappe.clear_cache(doctype=self.name)
|
||||
|
||||
# clear user cache so that on the next reload this doctype is included in boot
|
||||
|
|
@ -562,13 +560,6 @@ class DocType(Document):
|
|||
|
||||
clear_linked_doctype_cache()
|
||||
|
||||
def _clear_sort_cache_if_needed(self):
|
||||
"""Clear sort cache only if sort_field or sort_order changed."""
|
||||
if self.has_value_changed("sort_field") or self.has_value_changed("sort_order"):
|
||||
from frappe.model.meta import clear_doctype_sort_cache
|
||||
|
||||
clear_doctype_sort_cache(self.name)
|
||||
|
||||
@savepoint(catch=Exception)
|
||||
def sync_doctype_layouts(self):
|
||||
"""Sync Doctype Layout"""
|
||||
|
|
|
|||
|
|
@ -42,11 +42,9 @@ class PropertySetter(Document):
|
|||
if self.is_new():
|
||||
delete_property_setter(self.doc_type, self.property, self.field_name, self.row_name)
|
||||
|
||||
self._clear_sort_cache_if_needed()
|
||||
frappe.clear_cache(doctype=self.doc_type)
|
||||
|
||||
def on_trash(self):
|
||||
self._clear_sort_cache_if_needed()
|
||||
frappe.clear_cache(doctype=self.doc_type)
|
||||
|
||||
def validate_fieldtype_change(self):
|
||||
|
|
@ -62,13 +60,6 @@ class PropertySetter(Document):
|
|||
|
||||
validate_fields_for_doctype(self.doc_type)
|
||||
|
||||
def _clear_sort_cache_if_needed(self):
|
||||
"""Clear sort cache if this property setter modifies sort_field or sort_order."""
|
||||
if self.property in ("sort_field", "sort_order") and self.doctype_or_field == "DocType":
|
||||
from frappe.model.meta import clear_doctype_sort_cache
|
||||
|
||||
clear_doctype_sort_cache(self.doc_type)
|
||||
|
||||
def get_permission_log_options(self, event=None):
|
||||
if self.property in ("ignore_user_permissions", "permlevel"):
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -11,9 +11,9 @@ import warnings
|
|||
from collections.abc import Iterable, Sequence
|
||||
from contextlib import contextmanager, suppress
|
||||
from time import time
|
||||
from typing import TYPE_CHECKING, Any
|
||||
from typing import TYPE_CHECKING, Any, Literal
|
||||
|
||||
from pypika.queries import QueryBuilder
|
||||
from pypika.queries import QueryBuilder, Table
|
||||
|
||||
import frappe
|
||||
import frappe.defaults
|
||||
|
|
@ -583,11 +583,16 @@ class Database:
|
|||
# single field is requested, send it without wrapping in containers
|
||||
return row[0]
|
||||
|
||||
def _convert_default_order_by(self, doctype: str, order_by: str) -> str:
|
||||
def _convert_default_order_by(
|
||||
self, doctype: str | Table, order_by: str | Literal["KEEP_DEFAULT_ORDERING"]
|
||||
) -> str:
|
||||
"""Convert DefaultOrderBy sentinel to explicit order string to avoid meta loading."""
|
||||
if order_by != DefaultOrderBy:
|
||||
return order_by
|
||||
|
||||
if isinstance(doctype, Table):
|
||||
doctype = doctype.get_table_name()[3:]
|
||||
|
||||
sort_field, sort_order = get_doctype_sort_info(doctype)
|
||||
return f"{sort_field} {sort_order.lower()}"
|
||||
|
||||
|
|
@ -1333,12 +1338,12 @@ class Database:
|
|||
|
||||
from frappe.utils import now_datetime
|
||||
|
||||
Table = frappe.qb.DocType(doctype)
|
||||
dt = frappe.qb.DocType(doctype)
|
||||
|
||||
return (
|
||||
frappe.qb.from_(Table)
|
||||
.select(Count(Table.name))
|
||||
.where(Table.creation >= now_datetime() - relativedelta(minutes=minutes))
|
||||
frappe.qb.from_(dt)
|
||||
.select(Count(dt.name))
|
||||
.where(dt.creation >= now_datetime() - relativedelta(minutes=minutes))
|
||||
.run()[0][0]
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,12 @@ from frappe.database.utils import (
|
|||
get_doctype_sort_info,
|
||||
)
|
||||
from frappe.model import get_permitted_fields
|
||||
from frappe.model.base_document import DOCTYPES_FOR_DOCTYPE
|
||||
from frappe.query_builder import Criterion, Field, Order, functions
|
||||
|
||||
CORE_DOCTYPES = DOCTYPES_FOR_DOCTYPE | frozenset(
|
||||
("Custom Field", "Property Setter", "Module Def", "__Auth", "__global_search", "Singles")
|
||||
)
|
||||
from frappe.query_builder.utils import PseudoColumnMapper
|
||||
from frappe.utils.data import MARIADB_SPECIFIC_COMMENT
|
||||
|
||||
|
|
@ -83,43 +88,6 @@ OPERATOR_MAPPING = {
|
|||
}
|
||||
|
||||
|
||||
def _get_table_fields(doctype: str) -> list[dict]:
|
||||
"""Try to get table fields from cached meta, queries DB if not cached."""
|
||||
|
||||
# Don't use cache during install/migrate/tests
|
||||
if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test):
|
||||
if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"):
|
||||
return [
|
||||
{"fieldname": df.fieldname, "options": df.options}
|
||||
for df in meta.get_table_fields(include_computed=True)
|
||||
]
|
||||
|
||||
return frappe.db.sql(
|
||||
"""
|
||||
SELECT fieldname, options
|
||||
FROM tabDocField
|
||||
WHERE parent = %s AND fieldtype = 'Table'
|
||||
""",
|
||||
doctype,
|
||||
as_dict=True,
|
||||
)
|
||||
|
||||
|
||||
def _field_exists_in_doctype(doctype: str, fieldname: str) -> bool:
|
||||
"""Check if field exists in doctype, using cache if available."""
|
||||
# Don't query client cache during install/migrate/tests
|
||||
if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test):
|
||||
if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"):
|
||||
return meta.has_field(fieldname)
|
||||
|
||||
from frappe.model.meta import get_table_columns
|
||||
|
||||
try:
|
||||
return fieldname in get_table_columns(doctype)
|
||||
except frappe.db.TableMissingError:
|
||||
return False
|
||||
|
||||
|
||||
class Engine:
|
||||
def get_query(
|
||||
self,
|
||||
|
|
@ -673,23 +641,37 @@ class Engine:
|
|||
# If doctype wasn't specified, and the field isn't a standard field and doesn't exist in main doctype, check child tables
|
||||
from frappe.model import child_table_fields, default_fields, optional_fields
|
||||
|
||||
if self.doctype in CORE_DOCTYPES:
|
||||
meta = None
|
||||
else:
|
||||
try:
|
||||
meta = frappe.get_meta(self.doctype)
|
||||
except frappe.DoesNotExistError:
|
||||
meta = None
|
||||
|
||||
if (
|
||||
not doctype
|
||||
meta
|
||||
and not doctype
|
||||
and target_fieldname not in default_fields + optional_fields + child_table_fields
|
||||
and not _field_exists_in_doctype(self.doctype, target_fieldname)
|
||||
and not meta.has_field(target_fieldname)
|
||||
):
|
||||
for df in _get_table_fields(self.doctype):
|
||||
if _field_exists_in_doctype(df["options"], target_fieldname):
|
||||
for df in meta.get_table_fields(include_computed=True):
|
||||
try:
|
||||
child_meta = frappe.get_meta(df.options)
|
||||
except frappe.DoesNotExistError:
|
||||
continue
|
||||
|
||||
if child_meta.has_field(target_fieldname):
|
||||
# Found in child table, create handler for it
|
||||
child_field_handler = ChildTableField(
|
||||
doctype=df["options"],
|
||||
doctype=df.options,
|
||||
fieldname=target_fieldname,
|
||||
parent_doctype=self.doctype,
|
||||
parent_fieldname=df["fieldname"],
|
||||
parent_fieldname=df.fieldname,
|
||||
)
|
||||
parent_doctype_for_perm = self.doctype
|
||||
self._check_field_permission(
|
||||
df["options"], target_fieldname, parent_doctype_for_perm
|
||||
df.options, target_fieldname, parent_doctype_for_perm
|
||||
)
|
||||
self.query = child_field_handler.apply_join(self.query)
|
||||
return child_field_handler.field
|
||||
|
|
|
|||
|
|
@ -60,10 +60,7 @@ def get_doctype_name(table_name: str) -> str:
|
|||
|
||||
def get_doctype_sort_info(doctype: str) -> tuple[str, str]:
|
||||
"""
|
||||
Get sort_field and sort_order for a DocType from cache or database.
|
||||
|
||||
This is separate from regular meta to avoid recursive calls.
|
||||
Caches for a day since sort order won't change often (invalidated on doctype update).
|
||||
Get sort_field and sort_order for a DocType from meta.
|
||||
|
||||
Args:
|
||||
doctype: The DocType name
|
||||
|
|
@ -71,26 +68,16 @@ def get_doctype_sort_info(doctype: str) -> tuple[str, str]:
|
|||
Returns:
|
||||
Tuple of (sort_field, sort_order) with defaults ("creation", "DESC") if not found
|
||||
"""
|
||||
from frappe.database.query import CORE_DOCTYPES
|
||||
|
||||
cache_key = f"doctype_sort_info::{doctype}"
|
||||
if doctype in CORE_DOCTYPES:
|
||||
return "creation", "DESC"
|
||||
|
||||
if cached := frappe.cache.get_value(cache_key):
|
||||
sort_field, sort_order = cached
|
||||
else:
|
||||
sort_field, sort_order = None, None
|
||||
if result := frappe.db.sql(
|
||||
"SELECT sort_field, sort_order FROM tabDocType WHERE name = %s",
|
||||
(doctype,),
|
||||
):
|
||||
sort_field, sort_order = result[0]
|
||||
|
||||
if not sort_field:
|
||||
sort_field = "creation"
|
||||
if not sort_order:
|
||||
sort_order = "DESC"
|
||||
frappe.cache.set_value(cache_key, (sort_field, sort_order), expires_in_sec=86400)
|
||||
|
||||
return sort_field, sort_order
|
||||
try:
|
||||
meta = frappe.get_meta(doctype)
|
||||
return meta.sort_field or "creation", meta.sort_order or "DESC"
|
||||
except frappe.DoesNotExistError:
|
||||
return "creation", "DESC"
|
||||
|
||||
|
||||
class LazyString:
|
||||
|
|
|
|||
|
|
@ -424,11 +424,6 @@ class Meta(Document):
|
|||
self.extend("fields", custom_fields)
|
||||
|
||||
def apply_property_setters(self):
|
||||
"""
|
||||
Property Setters are set via Customize Form. They override standard properties
|
||||
of the doctype or its child properties like fields, links etc. This method
|
||||
applies the customized properties over the standard meta object
|
||||
"""
|
||||
if not frappe.db.table_exists("Property Setter"):
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -80,8 +80,7 @@ class TestPerformance(IntegrationTestCase):
|
|||
with self.assertQueryCount(1):
|
||||
frappe.db.set_value("User", "Administrator", "interest", "Nothing")
|
||||
|
||||
# TODO: get this back down to one after fixing query builder meta access
|
||||
with self.assertQueryCount(2):
|
||||
with self.assertQueryCount(1):
|
||||
frappe.db.set_value("User", {"user_type": "System User"}, "interest", "Nothing")
|
||||
|
||||
with self.assertQueryCount(1):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue