diff --git a/docs/HISTORY.txt b/docs/HISTORY.txt index c51b411..1555236 100644 --- a/docs/HISTORY.txt +++ b/docs/HISTORY.txt @@ -4,6 +4,15 @@ Changelog 2.0.1 (unreleased) ------------------ +- 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] diff --git a/plone/dexterity/content.py b/plone/dexterity/content.py index 2c6912b..e922541 100644 --- a/plone/dexterity/content.py +++ b/plone/dexterity/content.py @@ -44,6 +44,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() @@ -57,69 +58,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 diff --git a/plone/dexterity/schema.py b/plone/dexterity/schema.py index eee4e29..cd56ab7 100644 --- a/plone/dexterity/schema.py +++ b/plone/dexterity/schema.py @@ -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: + 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: + setattr(fti, key, (fti._p_mtime, value)) + + return value + return decorator + class SchemaCache(object): """Simple schema cache. @@ -50,62 +76,44 @@ 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 = {} @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) + invalidate_cache(fti) + SCHEMA_CACHE = SchemaCache()