From f37890c31f579477c5947d39ed0469b585c09ce4 Mon Sep 17 00:00:00 2001 From: Ayaan Ahmad Date: Wed, 14 Jan 2026 22:31:22 +0530 Subject: [PATCH] fix(model): address PR feedback for bulk link validation - Replace **kwargs with explicit link_value_cache parameter - Check db.value_cache before get_all for cross-document caching - Populate db.value_cache after fetching for subsequent documents - Add sorted() for deterministic cache key matching - Add isinstance guard for integer docnames (MariaDB) - Fix linting issues (RUF005, slice spacing) --- frappe/model/base_document.py | 6 ++--- frappe/model/document.py | 41 +++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 03ec83a6b8..2aebeb126e 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -962,15 +962,13 @@ class BaseDocument: return missing - def get_invalid_links(self, is_submittable=False, **kwargs): + def get_invalid_links(self, is_submittable=False, link_value_cache=None): """Return list of invalid links and also update fetch values if not set. Args: is_submittable: Whether the parent document is submittable - **kwargs: Additional arguments (link_value_cache for bulk optimization) + link_value_cache: Cache of prefetched link values for bulk optimization """ - link_value_cache = kwargs.get("link_value_cache") - is_submittable = is_submittable or self.meta.is_submittable def get_msg(df, docname): diff --git a/frappe/model/document.py b/frappe/model/document.py index aa049e0b18..6843028796 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1144,10 +1144,10 @@ class Document(BaseDocument): """Split iterable into chunks of given size.""" lst = list(iterable) for i in range(0, len(lst), size): - yield lst[i:i + size] + yield lst[i : i + size] self._link_value_cache = {} - docs_to_validate = [self] + self.get_all_children() + docs_to_validate = [self, *self.get_all_children()] # Collect: {doctype: {'names': set(), 'fields': set()}} prefetch_map = defaultdict(lambda: {"names": set(), "fields": {"name"}}) @@ -1196,7 +1196,7 @@ class Document(BaseDocument): for doctype, data in prefetch_map.items(): meta = frappe.get_meta(doctype) names = list(data["names"]) - fields = list(data["fields"]) + fields = sorted(data["fields"]) # Sorted for deterministic cache key matching # Skip if no names to fetch for this doctype if not names: @@ -1221,7 +1221,20 @@ class Document(BaseDocument): else: # Regular doctypes: bulk fetch with chunking result_dict = {} - for name_chunk in _chunk(names, 1000): + field_tuple = tuple(fields) + + # Check db.value_cache first for cross-document caching + names_to_query = [] + for name in names: + cached = frappe.db.value_cache.get(doctype, {}).get(name, {}).get(field_tuple) + if cached is not None: + # Use cached value from previous document in this transaction + self._link_value_cache.setdefault(doctype, {})[name] = cached[0] if cached else None + else: + names_to_query.append(name) + + # Only query for names not found in cache + for name_chunk in _chunk(names_to_query, 1000) if names_to_query else []: results = frappe.db.get_all( doctype, filters={"name": ("in", name_chunk)}, @@ -1229,18 +1242,24 @@ class Document(BaseDocument): ) for row in results: result_dict[row.name] = row - # Case-insensitive key for MariaDB compatibility - if frappe.db.db_type == "mariadb": + # Case-insensitive key for MariaDB compatibility (strings only) + if frappe.db.db_type == "mariadb" and isinstance(row.name, str): result_dict[row.name.casefold()] = row - # Store results (including None for missing names) + # Store results in both caches for name in names: if frappe.db.db_type == "mariadb" and isinstance(name, str): - self._link_value_cache.setdefault(doctype, {})[name] = ( - result_dict.get(name) or result_dict.get(name.casefold()) - ) + cached_value = result_dict.get(name) or result_dict.get(name.casefold()) else: - self._link_value_cache.setdefault(doctype, {})[name] = result_dict.get(name) + cached_value = result_dict.get(name) + + # Store in local document cache + self._link_value_cache.setdefault(doctype, {})[name] = cached_value + + # Also populate global db.value_cache for cross-document caching + # Only for string names (matching get_values behavior at line 632) + if cached_value is not None and isinstance(name, str): + frappe.db.value_cache[doctype][name][field_tuple] = [cached_value] def _validate_links(self): if self.flags.ignore_links or self._action == "cancel":