Skip to content

Commit

Permalink
rethink recursion fix -> catch earlier instead of caching FTI lookup
Browse files Browse the repository at this point in the history
  • Loading branch information
petschki committed Sep 21, 2021
1 parent 4caa6a9 commit a679a8e
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 32 deletions.
2 changes: 1 addition & 1 deletion news/155.bugfix
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
Fix recursion error when lookup IDexterityFTI for `Plone Site`
Catch maximum recursion error when lookup FTI
[petschki]
56 changes: 25 additions & 31 deletions plone/dexterity/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from plone.dexterity.filerepresentation import DAVResourceMixin
from plone.dexterity.interfaces import IDexterityContainer
from plone.dexterity.interfaces import IDexterityContent
from plone.dexterity.interfaces import IDexterityFTI
from plone.dexterity.interfaces import IDexterityItem
from plone.dexterity.schema import SCHEMA_CACHE
from plone.dexterity.utils import all_merged_tagged_values_dict
Expand Down Expand Up @@ -62,7 +61,6 @@

# see comment in DexterityContent.__getattr__ method
ATTRIBUTE_NAMES_TO_IGNORE = (
"_v__cached_fti",
"_dav_writelocks",
"aq_inner",
"getCurrentSkinName",
Expand Down Expand Up @@ -150,42 +148,38 @@ def __get__(self, inst, cls=None):
if portal_type is None:
return spec

fti = getattr(inst, "_v__cached_fti", None)
if fti is None:
fti = queryUtility(IDexterityFTI, name=portal_type)
if fti is None:
return spec
setattr(inst, "_v__cached_fti", fti)

# Find the cached value. This calculation is expensive and called
# hundreds of times during each request, so we require a fast cache
cache = getattr(inst, "_v__providedBy__", None)

# See if we have a current cache. Reasons to do this include:
#
# - The FTI was modified.
# - The instance was modified and persisted since the cache was built.
# - The instance has a different direct specification.
updated = (
inst._p_mtime,
SCHEMA_CACHE.modified(fti),
SCHEMA_CACHE.invalidations,
hash(spec),
)
if cache is not None and cache[:-1] == updated:
if cache[-1] is not None:
return cache[-1]
return spec

main_schema = SCHEMA_CACHE.get(fti)
if main_schema:
dynamically_provided = [main_schema]
else:
dynamically_provided = []
updated = ()
dynamically_provided = []

# block recursion
setattr(_recursion_detection, "blocked", True)
try:
# See if we have a current cache. Reasons to do this include:
#
# - The FTI was modified.
# - The instance was modified and persisted since the cache was built.
# - The instance has a different direct specification.
updated = (
inst._p_mtime,
SCHEMA_CACHE.modified(portal_type),
SCHEMA_CACHE.invalidations,
hash(spec),
)
if cache is not None and cache[:-1] == updated:
setattr(_recursion_detection, "blocked", False)
if cache[-1] is not None:
return cache[-1]
return spec

main_schema = SCHEMA_CACHE.get(portal_type)
if main_schema:
dynamically_provided = [main_schema]
else:
dynamically_provided = []

assignable = get_assignable(inst)
if assignable is not None:
for behavior_registration in assignable.enumerateBehaviors():
Expand Down

0 comments on commit a679a8e

Please sign in to comment.