1
Vote

ICacheService.Clear() with DefaultCacheStorageProvider removes clears all cache entries, not just tenant-specific ones

description

In Orchard.Caching:

Use the default ICacheStorageProvider implementation that stores items in HttpRuntime.Cache (DefaultCacheStorageProvider).
Call ICacheService.Clear().
Expected: all cache entries for the current tenant will be removed.
Actual result: all items from HttpRuntime.Cache will be removed, also causing the application to be restarted. This makes such a call extremely dangerous (and totally misleading).

Potential fix: ICacheStorageProvider.Clear() should accept a prefix parameter and DefaultCacheStorageProvider should use a LINQ Where clause to only list items with matching key. Downside is that this would be inefficient with many cache items. However this is pretty much what DefaultCacheStorageProvider in Orchard.OutputCache uses.

Probably handling tenant-awareness is something that ICacheStorageProvider implementation should handle anyway (and not the ICacheService implementation that currently builds prefixed cache keys).

Also since HttpRuntime.Cache can be used by anything DefaultCacheService could use a more unique key pattern, e.g. Orchard.Caching.TenantName_Key.

BTW probably it would be better to refactor OutputCache to use ICacheService under the hood, so we'd only have to implement one interface for a new (e.g. distributed) provider, see: https://orchard.codeplex.com/workitem/20853

comments

Piedone wrote Aug 4 at 6:21 PM

Quick fix for DefaultCacheStorageProvider:
        public void Clear() {
            var all = HttpRuntime.Cache
                .AsParallel()
                .Cast<DictionaryEntry>()
                .Select(x => x.Key.ToString())
                .Where(key => key.StartsWith(_prefix + "_")) // Workaround for https://orchard.codeplex.com/workitem/20850
                .ToList();

            foreach (var key in all) {
                Remove(key);
            }
        }