Skip to content

Commit

Permalink
Merge pull request #155 from plone/fix-plone-dx-siteroot-recursion
Browse files Browse the repository at this point in the history
Fix recursion in FTI lookup
  • Loading branch information
jensens authored Sep 27, 2021
2 parents 92d2511 + a679a8e commit b004e2f
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 25 deletions.
2 changes: 2 additions & 0 deletions news/155.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Catch maximum recursion error when lookup FTI
[petschki]
48 changes: 25 additions & 23 deletions plone/dexterity/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
from zope.security.interfaces import IPermission

import six
import warnings
import threading


Expand Down Expand Up @@ -152,32 +151,35 @@ def __get__(self, inst, cls=None):
# 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(portal_type),
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(portal_type)
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
4 changes: 3 additions & 1 deletion plone/dexterity/tests/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import unittest
import zope.component
import zope.component.testing

import zope.globalrequest

try:
from unittest.mock import Mock
Expand All @@ -24,6 +24,8 @@ class MockTestCase(unittest.TestCase):

def tearDown(self):
zope.component.testing.tearDown(self)
zope.globalrequest.setRequest(None)

if self._replaced_globals is not None:
for mock, orig in self._replaced_globals.items():
_global_replace(mock, orig)
Expand Down
11 changes: 11 additions & 0 deletions plone/dexterity/tests/test_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
from zope.component import provideAdapter
from zope.interface import alsoProvides
from zope.interface import Interface
from zope.globalrequest import setRequest
from zope.publisher.browser import TestRequest

import six
import zope.schema
Expand All @@ -42,6 +44,7 @@

class TestContent(MockTestCase):
def setUp(self):
setRequest(TestRequest())
SCHEMA_CACHE.clear()
provideAdapter(DefaultOrdering)
provideAdapter(AttributeAnnotations)
Expand Down Expand Up @@ -74,6 +77,7 @@ class IMarker(Interface):
fti_mock = Mock(wraps=DexterityFTI("testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

self.assertFalse(ISchema.implementedBy(Item))

Expand Down Expand Up @@ -129,6 +133,7 @@ class IMarker(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

self.assertFalse(ISchema.implementedBy(MyItem))

Expand Down Expand Up @@ -175,6 +180,7 @@ class IMarker(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

self.assertFalse(ISchema.implementedBy(MyItem))

Expand Down Expand Up @@ -358,6 +364,7 @@ class IMarker3(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

# start clean
SCHEMA_CACHE.invalidate("testtype")
Expand Down Expand Up @@ -411,6 +418,7 @@ class ISchema(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

SCHEMA_CACHE.invalidate("testtype")

Expand All @@ -433,6 +441,7 @@ class ISchema(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

SCHEMA_CACHE.invalidate("testtype")

Expand Down Expand Up @@ -462,6 +471,7 @@ class ISchema(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

SCHEMA_CACHE.invalidate("testtype")

Expand All @@ -487,6 +497,7 @@ class ISchema(Interface):
fti_mock = Mock(wraps=DexterityFTI(u"testtype"))
fti_mock.lookupSchema = Mock(return_value=ISchema)
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
alsoProvides(fti_mock, IDexterityFTI)

SCHEMA_CACHE.invalidate("testtype")

Expand Down
4 changes: 3 additions & 1 deletion plone/dexterity/tests/test_schema_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
from plone.dexterity.interfaces import IDexterityFTI
from plone.dexterity.schema import SCHEMA_CACHE
from zope.interface import Interface

from zope.globalrequest import setRequest
from zope.publisher.browser import TestRequest

try:
from unittest.mock import Mock
Expand All @@ -19,6 +20,7 @@

class TestSchemaCache(MockTestCase):
def setUp(self):
setRequest(TestRequest())
SCHEMA_CACHE.clear()

def test_repeated_get_lookup(self):
Expand Down
3 changes: 3 additions & 0 deletions plone/dexterity/tests/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from zope.interface import provider
from zope.security.interfaces import IPermission
from zope.security.permission import Permission
from zope.globalrequest import setRequest
from zope.publisher.browser import TestRequest

import zope.schema

Expand All @@ -22,6 +24,7 @@

class TestAttributeProtection(MockTestCase):
def setUp(self):
setRequest(TestRequest())
SCHEMA_CACHE.clear()

def test_item(self):
Expand Down

0 comments on commit b004e2f

Please sign in to comment.