Merge remote-tracking branch 'headnet/schema-cache-fix'
This commit is contained in:
commit
a5bb50a94a
|
@ -19,6 +19,24 @@ Changelog
|
|||
expected effect.
|
||||
[gaudenzius]
|
||||
|
||||
- Fixed schema caching. Previously, a non-persistent counter would be
|
||||
used as part of the cache key, and changes made to this counter in
|
||||
one process would obviously not propagate to other processes.
|
||||
|
||||
Instead, the cache key now includes the schema and subtypes which
|
||||
are both retrieved from a FTI-specific volatile cache that uses the
|
||||
modification time as its cache key.
|
||||
[malthe]
|
||||
|
||||
- Fixed schema caching. Previously, a non-persistent counter would be
|
||||
used as part of the cache key, and changes made to this counter in
|
||||
one process would obviously not propagate to other processes.
|
||||
|
||||
Instead, the cache key now includes the schema and subtypes which
|
||||
are both retrieved from a FTI-specific volatile cache that uses the
|
||||
modification time as its cache key.
|
||||
[malthe]
|
||||
|
||||
- The default attribute accessor now also looks through subtypes
|
||||
(behaviors) to find a field default.
|
||||
[malthe]
|
||||
|
|
|
@ -45,6 +45,7 @@ from plone.autoform.interfaces import READ_PERMISSIONS_KEY
|
|||
from plone.supermodel.utils import mergedTaggedValueDict
|
||||
|
||||
from plone.dexterity.filerepresentation import DAVResourceMixin, DAVCollectionMixin
|
||||
from plone.dexterity.interfaces import IDexterityFTI
|
||||
|
||||
_marker = object()
|
||||
|
||||
|
@ -58,69 +59,57 @@ class FTIAwareSpecification(ObjectSpecificationDescriptor):
|
|||
if inst is None:
|
||||
return getObjectSpecification(cls)
|
||||
|
||||
# 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)
|
||||
|
||||
# Find the data we need to know if our cache needs to be invalidated
|
||||
|
||||
direct_spec = getattr(inst, '__provides__', None)
|
||||
portal_type = getattr(inst, 'portal_type', None)
|
||||
|
||||
fti_counter = -1
|
||||
if portal_type is not None:
|
||||
fti_counter = SCHEMA_CACHE.counter(portal_type)
|
||||
|
||||
# See if we have a valid cache. Reasons to do this include:
|
||||
#
|
||||
# - We don't have a portal_type yet, so we can't have found the schema
|
||||
# - The FTI was modified, and the schema cache invalidated globally.
|
||||
# The fti_counter will have advanced.
|
||||
# - The instance was modified and persisted since the cache was built.
|
||||
# - The instance now has a different __provides__, which means that someone
|
||||
# called directlyProvides/alsoProvides on it.
|
||||
|
||||
if cache is not None and portal_type is not None:
|
||||
cached_mtime, cached_fti_counter, cached_direct_spec, cached_spec = cache
|
||||
|
||||
if inst._p_mtime == cached_mtime and \
|
||||
fti_counter == cached_fti_counter and \
|
||||
direct_spec is cached_direct_spec:
|
||||
return cached_spec
|
||||
|
||||
# We don't have a cache, so we need to build a new spec and maybe cache it
|
||||
|
||||
spec = direct_spec
|
||||
|
||||
|
||||
# If the instance doesn't have a __provides__ attribute, get the
|
||||
# interfaces implied by the class as a starting point.
|
||||
if spec is None:
|
||||
spec = implementedBy(cls)
|
||||
|
||||
# Add the schema from the FTI and behavior subtypes
|
||||
|
||||
dynamically_provided = []
|
||||
|
||||
if portal_type is not None:
|
||||
schema = SCHEMA_CACHE.get(portal_type)
|
||||
if schema is not None:
|
||||
dynamically_provided.append(schema)
|
||||
|
||||
subtypes = SCHEMA_CACHE.subtypes(portal_type)
|
||||
if subtypes:
|
||||
dynamically_provided.extend(subtypes)
|
||||
|
||||
# If we have any dynamically provided interface, prepend them to the spec
|
||||
# and cache. We can't cache until we have at least the schema, because
|
||||
# it's possible that we were called before traversal and so could not
|
||||
# find the schema yet.
|
||||
|
||||
if dynamically_provided:
|
||||
dynamically_provided.append(spec)
|
||||
spec = Implements(*dynamically_provided)
|
||||
|
||||
inst._v__providedBy__ = inst._p_mtime, SCHEMA_CACHE.counter(portal_type), direct_spec, spec
|
||||
|
||||
|
||||
# If the instance has no portal type, then we're done.
|
||||
if portal_type is None:
|
||||
return spec
|
||||
|
||||
fti = queryUtility(IDexterityFTI, name=portal_type)
|
||||
if fti is None:
|
||||
return spec
|
||||
|
||||
schema = SCHEMA_CACHE.get(portal_type)
|
||||
subtypes = SCHEMA_CACHE.subtypes(portal_type)
|
||||
|
||||
# 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)
|
||||
updated = inst._p_mtime, schema, subtypes, direct_spec
|
||||
|
||||
# See if we have a valid cache. Reasons to do this include:
|
||||
#
|
||||
# - The schema was modified.
|
||||
# - The subtypes were modified.
|
||||
# - The instance was modified and persisted since the cache was built.
|
||||
# - The instance has a different direct specification.
|
||||
if cache is not None:
|
||||
cached_mtime, cached_schema, cached_subtypes, \
|
||||
cached_direct_spec, cached_spec = cache
|
||||
|
||||
if cache[:-1] == updated:
|
||||
return cached_spec
|
||||
|
||||
dynamically_provided = [] if schema is None else [schema]
|
||||
dynamically_provided.extend(subtypes)
|
||||
|
||||
# If we have neither a schema, nor a subtype, then we're also done.
|
||||
if not dynamically_provided:
|
||||
return spec
|
||||
|
||||
dynamically_provided.append(spec)
|
||||
spec = Implements(*dynamically_provided)
|
||||
inst._v__providedBy__ = updated + (spec, )
|
||||
|
||||
return spec
|
||||
|
||||
|
||||
|
|
|
@ -374,11 +374,11 @@ def unregister(fti, old_name=None):
|
|||
site_manager = getSiteManager(site)
|
||||
|
||||
portal_type = old_name or fti.getId()
|
||||
|
||||
notify(SchemaInvalidatedEvent(portal_type))
|
||||
|
||||
site_manager.unregisterUtility(provided=IDexterityFTI, name=portal_type)
|
||||
unregister_factory(fti.factory, site_manager)
|
||||
|
||||
notify(SchemaInvalidatedEvent(portal_type))
|
||||
unregister_factory(fti.factory, site_manager)
|
||||
|
||||
def unregister_factory(factory_name, site_manager):
|
||||
"""Helper method to unregister factories when unused by any dexterity
|
||||
|
@ -472,4 +472,4 @@ def ftiModified(object, event):
|
|||
model = fti.lookupModel()
|
||||
syncSchema(model.schema, schema, overwrite=True)
|
||||
|
||||
notify(SchemaInvalidatedEvent(portal_type))
|
||||
notify(SchemaInvalidatedEvent(portal_type))
|
||||
|
|
|
@ -1,4 +1,5 @@
|
|||
import new
|
||||
import functools
|
||||
|
||||
from threading import RLock
|
||||
from plone.synchronize import synchronized
|
||||
|
@ -7,7 +8,7 @@ from zope.interface import implements, alsoProvides
|
|||
from zope.interface.interface import InterfaceClass
|
||||
|
||||
from zope.component import adapter
|
||||
from zope.component import queryUtility
|
||||
from zope.component import queryUtility, getAllUtilitiesRegisteredFor
|
||||
|
||||
from plone.behavior.interfaces import IBehavior
|
||||
|
||||
|
@ -28,7 +29,32 @@ from plone.alterego import dynamic
|
|||
generated = dynamic.create('plone.dexterity.schema.generated')
|
||||
transient = new.module("transient")
|
||||
|
||||
# Schema cache
|
||||
def invalidate_cache(fti):
|
||||
fti._p_activate()
|
||||
fti.__dict__.pop('_v_schema_get', None)
|
||||
fti.__dict__.pop('_v_schema_subtypes', None)
|
||||
|
||||
|
||||
def volatile(func):
|
||||
@functools.wraps(func)
|
||||
def decorator(self, portal_type):
|
||||
fti = queryUtility(IDexterityFTI, name=portal_type)
|
||||
if fti is not None and self.cache_enabled:
|
||||
key = '_v_schema_%s' % func.__name__
|
||||
cache = getattr(fti, key, None)
|
||||
if cache is not None:
|
||||
mtime, value = cache
|
||||
if fti._p_mtime == mtime:
|
||||
return value
|
||||
|
||||
value = func(self, fti)
|
||||
|
||||
if fti is not None and value is not None and self.cache_enabled:
|
||||
setattr(fti, key, (fti._p_mtime, value))
|
||||
|
||||
return value
|
||||
return decorator
|
||||
|
||||
|
||||
class SchemaCache(object):
|
||||
"""Simple schema cache.
|
||||
|
@ -50,62 +76,48 @@ class SchemaCache(object):
|
|||
>>> from plone.dexterity.schema import SCHEMA_CACHE
|
||||
>>> my_schema = SCHEMA_CACHE.get(portal_type)
|
||||
|
||||
Invalidate the cache by calling invalidate() (for one portal_type) or
|
||||
clear() (for all cached values), or simply raise a SchemaInvalidatedEvent.
|
||||
The cache uses the FTI's modification time as its invariant.
|
||||
"""
|
||||
|
||||
lock = RLock()
|
||||
cache = {}
|
||||
subtypes_cache = {}
|
||||
counter_values = {}
|
||||
|
||||
def __init__(self, cache_enabled=True):
|
||||
self.cache_enabled = cache_enabled
|
||||
|
||||
@synchronized(lock)
|
||||
def get(self, portal_type):
|
||||
cached = self.cache.get(portal_type, None)
|
||||
if cached is None:
|
||||
fti = queryUtility(IDexterityFTI, name=portal_type)
|
||||
if fti is not None:
|
||||
try:
|
||||
cached = self.cache[portal_type] = fti.lookupSchema()
|
||||
except (AttributeError, ValueError):
|
||||
pass
|
||||
return cached
|
||||
|
||||
@volatile
|
||||
def get(self, fti):
|
||||
if fti is None:
|
||||
return
|
||||
try:
|
||||
return fti.lookupSchema()
|
||||
except (AttributeError, ValueError):
|
||||
pass
|
||||
|
||||
@synchronized(lock)
|
||||
def subtypes(self, portal_type):
|
||||
cached = self.subtypes_cache.get(portal_type, None)
|
||||
if cached is None:
|
||||
subtypes = []
|
||||
fti = queryUtility(IDexterityFTI, name=portal_type)
|
||||
if fti is None:
|
||||
return ()
|
||||
for behavior_name in fti.behaviors:
|
||||
behavior = queryUtility(IBehavior, name=behavior_name)
|
||||
if behavior is not None and behavior.marker is not None:
|
||||
subtypes.append(behavior.marker)
|
||||
cached = self.subtypes_cache[portal_type] = tuple(subtypes)
|
||||
return cached
|
||||
|
||||
@synchronized(lock)
|
||||
def counter(self, portal_type):
|
||||
counter = self.counter_values.get(portal_type, None)
|
||||
if counter is None:
|
||||
counter = self.counter_values[portal_type] = 0
|
||||
return counter
|
||||
|
||||
@synchronized(lock)
|
||||
def invalidate(self, portal_type):
|
||||
self.cache[portal_type] = None
|
||||
self.subtypes_cache[portal_type] = None
|
||||
if portal_type in self.counter_values:
|
||||
self.counter_values[portal_type] += 1
|
||||
else:
|
||||
self.counter_values[portal_type] = 0
|
||||
@volatile
|
||||
def subtypes(self, fti):
|
||||
if fti is None:
|
||||
return ()
|
||||
subtypes = []
|
||||
for behavior_name in fti.behaviors:
|
||||
behavior = queryUtility(IBehavior, name=behavior_name)
|
||||
if behavior is not None and behavior.marker is not None:
|
||||
subtypes.append(behavior.marker)
|
||||
return tuple(subtypes)
|
||||
|
||||
@synchronized(lock)
|
||||
def clear(self):
|
||||
self.cache.clear()
|
||||
self.subtypes_cache.clear()
|
||||
for fti in getAllUtilitiesRegisteredFor(IDexterityFTI):
|
||||
invalidate_cache(fti)
|
||||
|
||||
|
||||
@synchronized(lock)
|
||||
def invalidate(self, portal_type):
|
||||
fti = queryUtility(IDexterityFTI, name=portal_type)
|
||||
if fti is not None:
|
||||
invalidate_cache(fti)
|
||||
|
||||
|
||||
SCHEMA_CACHE = SchemaCache()
|
||||
|
||||
|
|
|
@ -16,11 +16,15 @@ from plone.dexterity.fti import DexterityFTI
|
|||
|
||||
from plone.autoform.interfaces import READ_PERMISSIONS_KEY
|
||||
|
||||
class TestAttributeProtection(MockTestCase):
|
||||
|
||||
class TestAttributeProtection(MockTestCase):
|
||||
def setUp(self):
|
||||
SCHEMA_CACHE.clear()
|
||||
|
||||
SCHEMA_CACHE.cache_enabled = False
|
||||
|
||||
def tearDown(self):
|
||||
SCHEMA_CACHE.cache_enabled = True
|
||||
|
||||
def test_item(self):
|
||||
|
||||
# Mock schema model
|
||||
|
@ -31,6 +35,8 @@ class TestAttributeProtection(MockTestCase):
|
|||
# Mock FTI
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, u'testtype')
|
||||
|
||||
|
@ -70,6 +76,8 @@ class TestAttributeProtection(MockTestCase):
|
|||
# Mock FTI
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, u'testtype')
|
||||
|
||||
|
@ -111,6 +119,8 @@ class TestAttributeProtection(MockTestCase):
|
|||
# Mock FTI
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, u'testtype')
|
||||
|
||||
|
@ -148,6 +158,8 @@ class TestAttributeProtection(MockTestCase):
|
|||
# Mock FTI
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, u'testtype')
|
||||
|
||||
|
@ -176,6 +188,8 @@ class TestAttributeProtection(MockTestCase):
|
|||
# Mock FTI
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITestSchema)
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, u'testtype')
|
||||
|
||||
|
|
|
@ -920,8 +920,6 @@ class TestFileRepresentation(MockTestCase):
|
|||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
SCHEMA_CACHE.clear()
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
|
||||
|
@ -943,8 +941,6 @@ class TestFileRepresentation(MockTestCase):
|
|||
SCHEMA_CACHE.clear()
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
|
||||
|
@ -968,8 +964,6 @@ class TestFileRepresentation(MockTestCase):
|
|||
SCHEMA_CACHE.clear()
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
self.expect(fti_mock.behaviors).result([])
|
||||
|
||||
self.mock_utility(fti_mock, IDexterityFTI, name=u"testtype")
|
||||
|
@ -1030,7 +1024,6 @@ class TestFileRepresentation(MockTestCase):
|
|||
SCHEMA_CACHE.clear()
|
||||
fti_mock = self.mocker.mock(DexterityFTI)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
self.expect(fti_mock.lookupSchema()).result(ITest)
|
||||
|
||||
self.mock_adapter(MockBehaviorAssignable, IBehaviorAssignable,
|
||||
(Item, ))
|
||||
|
|
Reference in New Issue