Change db.cache.virtual_libraries_for_books() to raise an exception when recursion is detected instead of returning an empty list. Using an exception gives a better chance of telling the user that the VL expression is wrong.

This commit is contained in:
Charles Haley 2022-01-03 11:47:38 +00:00
parent 6a3309175a
commit f30a190b36
2 changed files with 22 additions and 11 deletions

View File

@ -147,6 +147,7 @@ class Cache:
self.formatter_template_cache = {} self.formatter_template_cache = {}
self.dirtied_cache = {} self.dirtied_cache = {}
self.vls_for_books_cache = None self.vls_for_books_cache = None
self.vls_for_books_lib_in_process = None
self.vls_cache_lock = Lock() self.vls_cache_lock = Lock()
self.dirtied_sequence = 0 self.dirtied_sequence = 0
self.cover_caches = set() self.cover_caches = set()
@ -257,6 +258,7 @@ class Cache:
self.clear_search_cache_count += 1 self.clear_search_cache_count += 1
self._search_api.update_or_clear(self, book_ids) self._search_api.update_or_clear(self, book_ids)
self.vls_for_books_cache = None self.vls_for_books_cache = None
self.vls_for_books_lib_in_process = None
@read_api @read_api
def last_modified(self): def last_modified(self):
@ -2276,18 +2278,24 @@ class Cache:
c = defaultdict(list) c = defaultdict(list)
if not got_lock: if not got_lock:
# We get here if resolving the books in a VL triggers another VL # We get here if resolving the books in a VL triggers another VL
# calculation. This can be 'real' recursion, in which case the # cache calculation. This can be 'real' recursion, for example a
# eventual answer will be wrong. It can also be a search using # VL expression using a template that calls virtual_libraries(),
# a location of 'all' that causes evaluation of a composite that # or a search using a location of 'all' that causes evaluation
# references virtual_libraries(). If the composite isn't used in a # of a composite that uses virtual_libraries(). The first case
# VL then the eventual answer will be correct because get_metadata # is an error and the exception message should appear somewhere.
# will clear the caches. # However, the error can seem nondeterministic. It might not be
return c # raised if the use is via a composite and that composite is
# evaluated before it is used in the search. The second case is
# also an error but if the composite isn't used in a VL then the
# eventual answer will be correct because get_metadata() will
# clear the caches.
raise ValueError(_('Recursion detected while processing virtual library "%s"')
% self.vls_for_books_lib_in_process)
if self.vls_for_books_cache is None: if self.vls_for_books_cache is None:
self.vls_for_books_cache_is_loading = True
libraries = self._pref('virtual_libraries', {}) libraries = self._pref('virtual_libraries', {})
for lib, expr in libraries.items(): for lib, expr in libraries.items():
book = None book = None
self.vls_for_books_lib_in_process = lib
try: try:
for book in self._search(expr, virtual_fields=virtual_fields): for book in self._search(expr, virtual_fields=virtual_fields):
c[book].append(lib) c[book].append(lib)

View File

@ -1756,9 +1756,12 @@ class BuiltinVirtualLibraries(BuiltinFormatterFunction):
def evaluate(self, formatter, kwargs, mi, locals_): def evaluate(self, formatter, kwargs, mi, locals_):
with suppress(Exception): with suppress(Exception):
from calibre.gui2.ui import get_gui try:
a = get_gui().current_db.data.get_virtual_libraries_for_books((mi.id,)) from calibre.gui2.ui import get_gui
return ', '.join(a[mi.id]) a = get_gui().current_db.data.get_virtual_libraries_for_books((mi.id,))
return ', '.join(a[mi.id])
except ValueError as v:
return str(v)
return _('This function can be used only in the GUI') return _('This function can be used only in the GUI')