Comment 0 for bug 142568

Uploaded: Cache.py

I've refactored OFS/Cache.py to include a few featurettes I needed for a new CacheManager implementation and squashed a troup of bugs in the process. Here's the summary:

>Bugs squashed --
> Cacheable.ZCacheable_getURL unprotected
> could be called ttw
> CacheManager.manage_{afterAdd,beforeDelete} unprotected
> could be called by restricted code
> CacheManager.ZCacheManager_locate object hierarchy exposure
> under atypical conditions (Change cache settings
> permission given without View management screens
> given in kind, maybe a "lord of the cache"
> scenario? [obviously a very minor problem]) this
> method could be used to examine an object hierarchy
> which wouldn't otherwise be visible to a user

>Noteworthy improvements --
> removed all 4 bare except clauses in Cache.py
> ~100 lines less code than before
> uses declarative security model throughout

>Featurettes introduced --
> better exception interface
> support for weakref.proxy wrapped Cache instances in
> Cacheable mix-in

The case for a new exception interface:

The current Cache.py uses bare except clauses to ignore and log any exceptions raised by Cache.ZCache_{set,get,invalidate}. Apart from the more global problems caused by bare except statements, this implementation also dumped a full traceback into the logs everytime something innocuous happened (eg. when a RAM Cache Manager got handed unpicklable data) when the exception value alone would suffice in those conditions. Instead of swallowing exceptions, the refactored version defines a new CacheException class, which may be subclassed by Cache implementations, and only caches and logs exceptions if they are derived from that class. This gives the Cache author an API for logging non-terminal, but inappropriate cache activity. (eg. being handed data which is uncacheable for whatever reason)

The case for support weakref ProxyTypes:

When resetting the properties of an active RAM Cache the RAM Cache Manager simply makes some adjustments and lets the cache adjust itself naturally at the given cleanup interval. That works great for the RAM Cache product, but it doesn't work great for all cache products. There are times when resetting the properties of an active cache could mean invoking expensive thread locks to do a mass cleanup of the internal data structures. (Generally this will occur anytime you change a property which then means having to examine or change every object currently in the cache.) To avoid the overhead of a long-running cache scrubing its can be entirely acceptable to just throw the cache out entirely and start over, and weakref proxy objects give us just the mechanism to do this painlessly. The refactored Cache.py allows for CacheManager.ZCacheManager_getCache to return a weakref.proxy wrapped Cache object, or a direct reference to a Cache object (thus maintaining backwards compability). If a Cacheable object attempts a ZCacheable_* operation and it raises a ReferenceError it will simply invalidate its volatile reference to the current Cache so that the next operation will refresh the Cache reference using ZCacheable_getManager().
This implies the following behavior for the 3 affected methods:

ZCacheable_get -- invalidates its volatile reference and returns default, just like a normal cache miss
ZCacheable_set -- invalidates its volatile reference, this will incur another cache miss in the future at the time of the next ZCacheable_get()
ZCacheable_invalidate -- invalidates its volatile reference, returns a normal message saying the entry has been invalidated, (which is something of an understatement as the entire cache has evidently been destroyed <g>)

Risk factors, challenged assumptions, & API changes:

All in all the refactored code is almost totally backwards compatible, there are only a few minor API changes, in particular:

The Cache.py module variables ViewManagementScreensPermission, and ChangeCacheSettingsPermission have been changed to MANAGE_PERM and CHANGE_PERM respectively, because intollerably long variable names piss me off. Nowhere else in the entire Zope code base were these variables referenced, nor in any of the Cache products on Zope.org.

The managersExist() function was removed from the Cache.py module, it was only called in one spot from one another def in the same module so I just inlined it. Nowhere else in the entire Zope code base was this function referenced, nor in any of the Cache products on Zope.org.

Caches which depend on the greedy except nature of the old API will not behave as expected. I'd argue they were buggy anyway, but they can easily use the new CacheException base class to fix the issue. I'll include patches which do just this for RAMCacheManager.py. I examined the Cache products on Zope.org and found no behavior which relied upon the greedy nature of the old API.

There is some rather bizzare behavior which occurs when you've got nested cache managers, particularly of the same name. AFAICT this behavior was intentional, so I didn't change it, but if you add a cache manager with the same id as a manager above it in the containment hierarchy, documents which used to be assocated with the higher manager will be automatically placed into the new manager instead. Same story for deletion.

Appendix:
Cache products found on zope.org which I reference above were FSCacheManager and ZopeProxyCacheManager. There was a very old one called SimpleCache, but its no longer relevant. The listed bugs fixed are all security issues, but this bug isn't marked as security related because they are quite minor in the grand scheme of things. The refactored code has been tested extensively, I believe it to be sound.