Fixing the FileSystemOutputCache feature

- Caching keys for filenames to prevent too long paths
- Separating metadata from content storage to optimize some scenarios
- Fixing retry logic

src/Orchard.Specs/MultiTenancy.feature

@@ -25,7 +25,6 @@ Scenario: A new tenant is created
         And I fill in 
             | name | value |
             | Name | Scott |
-            | RequestUrlPrefix | scott |
         And I hit "Save"
         And I am redirected
     Then I should see "<h3>Scott\s*</h3>"
@@ -38,7 +37,6 @@ Scenario: A new tenant is created with uninitialized state
         And I fill in 
             | name | value |
             | Name | Scott |
-            | RequestUrlPrefix | scott |
         And I hit "Save"
         And I am redirected
     Then I should see "<li class="tenant Uninitialized">"
@@ -201,4 +199,4 @@ Scenario: Listing tenants from command line
         And I have tenant "Alpha" on "example.org" as "New-site-name"
     When I execute >tenant list
     Then I should see "Name: Alpha"
-        And I should see "Request Url Host: example.org"
+        And I should see "Request Url Host: example.org"

src/Orchard.Specs/MultiTenancy.feature.cs

@@ -129,9 +129,6 @@ this.ScenarioSetup(scenarioInfo);
             table1.AddRow(new string[] {
                         "Name",
                         "Scott"});
-            table1.AddRow(new string[] {
-                        "RequestUrlPrefix",
-                        "scott"});
 #line 25
         testRunner.And("I fill in", ((string)(null)), table1, "And ");
 #line 28
@@ -166,9 +163,6 @@ this.ScenarioSetup(scenarioInfo);
             table2.AddRow(new string[] {
                         "Name",
                         "Scott"});
-            table2.AddRow(new string[] {
-                        "RequestUrlPrefix",
-                        "scott"});
 #line 37
         testRunner.And("I fill in", ((string)(null)), table2, "And ");
 #line 40

src/Orchard.Web/Modules/Orchard.Lists/Shapes.cs

@@ -24,8 +24,6 @@ namespace Orchard.Lists {
             builder.Describe("ListNavigation").OnDisplaying(context => {
                 var containable = (ContainablePart) context.Shape.ContainablePart;
                 var container = _containerService.Value.GetContainer(containable, VersionOptions.Latest);
-                if (container == null) return;
-
                 var previous = _containerService.Value.Previous(container.Id, containable);
                 var next = _containerService.Value.Next(container.Id, containable);
 
@@ -34,4 +32,4 @@ namespace Orchard.Lists {
             });
         }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.Lists/Views/Admin/List.cshtml

@@ -3,7 +3,7 @@
 @using Orchard.ContentManagement;
 @{
     Style.Require("jQueryColorBox");
-    Style.Include("nprogress.css");
+    Style.Include("nprogress.css", "nprogress.min.css");
     Style.Include("common-admin.css", "common-admin.min.css");
     Style.Include("list-admin.css", "list-admin.min.css");
     Script.Require("ContentPicker").AtFoot();
@@ -39,4 +39,4 @@
 </div>
 @if (Model.ListNavigation != null) {
     @Display(Model.ListNavigation)
-}
+}

src/Orchard.Web/Modules/Orchard.OutputCache/Filters/OutputCacheFilter.cs

@@ -5,7 +5,7 @@ using System.Globalization;
 using System.Linq;
 using System.Net;
 using System.Text;
-using System.Threading.Tasks;
+using System.Threading;
 using System.Web;
 using System.Web.Mvc;
 using System.Web.Routing;
@@ -39,13 +39,10 @@ namespace Orchard.OutputCache.Filters {
         private readonly ICacheService _cacheService;
         private readonly ISignals _signals;
         private readonly ShellSettings _shellSettings;
-        private readonly IOutputCacheFilterState _state;
-
         private bool _isDisposed = false;
 
         public ILogger Logger { get; set; }
 
-
         public OutputCacheFilter(
             ICacheManager cacheManager,
             IOutputCacheStorageProvider cacheStorageProvider,
@@ -56,9 +53,8 @@ namespace Orchard.OutputCache.Filters {
             IClock clock,
             ICacheService cacheService,
             ISignals signals,
-            ShellSettings shellSettings,
-            IOutputCacheFilterState state) {
-            _state = state;
+            ShellSettings shellSettings) {
+
             _cacheManager = cacheManager;
             _cacheStorageProvider = cacheStorageProvider;
             _tagCache = tagCache;
@@ -75,7 +71,6 @@ namespace Orchard.OutputCache.Filters {
 
         // State.
         private CacheSettings _cacheSettings;
-        private CacheRouteConfig _cacheRouteConfig;
         private DateTime _now;
         private WorkContext _workContext;
         private string _cacheKey;
@@ -97,18 +92,11 @@ namespace Orchard.OutputCache.Filters {
             _now = _clock.UtcNow;
             _workContext = _workContextAccessor.GetContext();
 
-            var configurations = _cacheService.GetRouteConfigs();
-            if (configurations.Any()) {
-                var route = filterContext.Controller.ControllerContext.RouteData.Route;
-                var key = _cacheService.GetRouteDescriptorKey(filterContext.HttpContext, route);
-                _cacheRouteConfig = configurations.FirstOrDefault(c => c.RouteKey == key);
-            }
-
             if (!RequestIsCacheable(filterContext))
                 return;
 
             // Computing the cache key after we know that the request is cacheable means that we are only performing this calculation on requests that require it
-            _cacheKey = ComputeCacheKey(filterContext, GetCacheKeyParameters(filterContext));
+            _cacheKey = String.Intern(ComputeCacheKey(filterContext, GetCacheKeyParameters(filterContext)));
             _invariantCacheKey = ComputeCacheKey(filterContext, null);
 
             Logger.Debug("Cache key '{0}' was created.", _cacheKey);
@@ -121,44 +109,53 @@ namespace Orchard.OutputCache.Filters {
                 if (allowServeFromCache && cacheItem != null) {
 
                     Logger.Debug("Item '{0}' was found in cache.", _cacheKey);
-                    
-                    if (cacheItem.IsValid(_now)) {
-                        Logger.Debug("Cached item is valid: {0}", _cacheKey);
-                        ServeCachedItem(filterContext, cacheItem);
-                        return;
-                    }
-                    else {
-                        Logger.Debug("Cached item is invalid: {0}", _cacheKey);
 
-                        // If the cached item is in its grace period 
-                        // and there is no current renderer, this request renders the new content.
-                        if (cacheItem.IsInGracePeriod(_now) &&
-                            !_state.Renderers.ContainsKey(_cacheKey)) {
+                    // Is the cached item in its grace period?
+                    if (cacheItem.IsInGracePeriod(_now)) {
+
+                        // Render the content unless another request is already doing so.
+                        if (Monitor.TryEnter(_cacheKey)) {
                             Logger.Debug("Item '{0}' is in grace period and not currently being rendered; rendering item...", _cacheKey);
-                            BeginRenderItem(filterContext, _cacheKey);
+                            BeginRenderItem(filterContext);
                             return;
                         }
-                        else {
-                            // Cached item not is in its grace period
-                            // or is already being rendered by another request; serve it from cache.
-                            Logger.Debug("Cache item is invalid: {0}", _cacheKey);
+                    }
+
+                    // Cached item is not yet in its grace period, or is already being
+                    // rendered by another request; serve it from cache.
+                    Logger.Debug("Serving item '{0}' from cache.", _cacheKey);
+                    ServeCachedItem(filterContext, cacheItem);
+                    return;
+                }
+
+                // No cached item found, or client doesn't want it; acquire the cache key
+                // lock to render the item.
+                Logger.Debug("Item '{0}' was not found in cache or client refuses it. Acquiring cache key lock...", _cacheKey);
+                if (Monitor.TryEnter(_cacheKey)) {
+                    Logger.Debug("Cache key lock for item '{0}' was acquired.", _cacheKey);
+
+                    // Item might now have been rendered and cached by another request; if so serve it from cache.
+                    if (allowServeFromCache) {
+                        cacheItem = GetCacheItem(_cacheKey);
+                        if (cacheItem != null) {
+                            Logger.Debug("Item '{0}' was now found; releasing cache key lock and serving from cache.", _cacheKey);
+                            Monitor.Exit(_cacheKey);
                             ServeCachedItem(filterContext, cacheItem);
                             return;
                         }
-                    }                    
+                    }
                 }
 
                 // Either we acquired the cache key lock and the item was still not in cache, or
                 // the lock acquisition timed out. In either case render the item.
                 Logger.Debug("Rendering item '{0}'...", _cacheKey);
-                BeginRenderItem(filterContext, _cacheKey);
+                BeginRenderItem(filterContext);
+
             }
             catch {
                 // Remember to release the cache key lock in the event of an exception!
                 Logger.Debug("Exception occurred for item '{0}'; releasing any acquired lock.", _cacheKey);
-
-                ReleaseWorkers();
-
+                ReleaseCacheKeyLock();
                 throw;
             }
         }
@@ -172,91 +169,96 @@ namespace Orchard.OutputCache.Filters {
 
         public void OnResultExecuted(ResultExecutedContext filterContext) {
 
-            // This filter is not reentrant (multiple executions within the same request are
-            // not supported) so child actions are ignored completely.
-            if (filterContext.IsChildAction || !_isCachingRequest)
+            var captureHandlerIsAttached = false;
+
+            try {
+                
+                // This filter is not reentrant (multiple executions within the same request are
+                // not supported) so child actions are ignored completely.
+                if (filterContext.IsChildAction || !_isCachingRequest)
                 return;
 
-            Logger.Debug("Item '{0}' was rendered.", _cacheKey);
+                Logger.Debug("Item '{0}' was rendered.", _cacheKey);
 
-
-            if (!ResponseIsCacheable(filterContext)) {
-                filterContext.HttpContext.Response.Cache.SetCacheability(HttpCacheability.NoCache);
-                filterContext.HttpContext.Response.Cache.SetNoStore();
-                filterContext.HttpContext.Response.Cache.SetMaxAge(new TimeSpan(0));
-                return;
-            }
-
-            // Determine duration and grace time.
-            var cacheDuration = _cacheRouteConfig != null && _cacheRouteConfig.Duration.HasValue ? _cacheRouteConfig.Duration.Value : CacheSettings.DefaultCacheDuration;
-            var cacheGraceTime = _cacheRouteConfig != null && _cacheRouteConfig.GraceTime.HasValue ? _cacheRouteConfig.GraceTime.Value : CacheSettings.DefaultCacheGraceTime;
-
-            // Include each content item ID as tags for the cache entry.
-            var contentItemIds = _displayedContentItemHandler.GetDisplayed().Select(x => x.ToString(CultureInfo.InvariantCulture)).ToArray();
-
-            // Capture the response output using a custom filter stream.
-            var response = filterContext.HttpContext.Response;
-            var captureStream = new CaptureStream(response.Filter);
-            response.Filter = captureStream;
-
-            // Add ETag header for the newly created item
-            var etag = Guid.NewGuid().ToString("n");
-            if (HttpRuntime.UsingIntegratedPipeline) {
-                if (response.Headers.Get("ETag") == null) {
-                    response.Headers["ETag"] = etag;
+                // Obtain individual route configuration, if any.
+                CacheRouteConfig configuration = null;
+                var configurations = _cacheService.GetRouteConfigs();
+                if (configurations.Any()) {
+                    var route = filterContext.Controller.ControllerContext.RouteData.Route;
+                    var key = _cacheService.GetRouteDescriptorKey(filterContext.HttpContext, route);
+                    configuration = configurations.FirstOrDefault(c => c.RouteKey == key);
                 }
-            }
 
-            captureStream.Captured += (output) => {
-                try {
-                    // Since this is a callback any call to injected dependencies can result in an Autofac exception: "Instances 
-                    // cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it has already been disposed."
-                    // To prevent access to the original lifetime scope a new work context scope should be created here and dependencies
-                    // should be resolved from it.
+                if (!ResponseIsCacheable(filterContext, configuration)) {
+                    filterContext.HttpContext.Response.Cache.SetCacheability(HttpCacheability.NoCache);
+                    filterContext.HttpContext.Response.Cache.SetNoStore();
+                    filterContext.HttpContext.Response.Cache.SetMaxAge(new TimeSpan(0));
+                    return;
+                }
 
-                    using (var scope = _workContextAccessor.CreateWorkContextScope()) {
-                        var cacheItem = new CacheItem() {
-                            CachedOnUtc = _now,
-                            Duration = cacheDuration,
-                            GraceTime = cacheGraceTime,
-                            Output = output,
-                            ContentType = response.ContentType,
-                            QueryString = filterContext.HttpContext.Request.Url.Query,
-                            CacheKey = _cacheKey,
-                            InvariantCacheKey = _invariantCacheKey,
-                            Url = filterContext.HttpContext.Request.Url.AbsolutePath,
-                            Tenant = scope.Resolve<ShellSettings>().Name,
-                            StatusCode = response.StatusCode,
-                            Tags = new[] { _invariantCacheKey }.Union(contentItemIds).ToArray(),
-                            ETag = etag
-                        };
+                // Determine duration and grace time.
+                var cacheDuration = configuration != null && configuration.Duration.HasValue ? configuration.Duration.Value : CacheSettings.DefaultCacheDuration;
+                var cacheGraceTime = configuration != null && configuration.GraceTime.HasValue ? configuration.GraceTime.Value : CacheSettings.DefaultCacheGraceTime;
 
-                        // Notifying any awaiting thread that the content is ready
-                        // for this page
-                        TaskCompletionSource<CacheItem> tcs;
-                        if (_state.Renderers.TryRemove(_cacheKey, out tcs)) {
-                            Logger.Debug("Page rendered, notifying awaiters: {0}", _cacheKey);
-                            tcs.SetResult(cacheItem);
-                        }
+                // Include each content item ID as tags for the cache entry.
+                var contentItemIds = _displayedContentItemHandler.GetDisplayed().Select(x => x.ToString(CultureInfo.InvariantCulture)).ToArray();
 
-                        // Write the rendered item to the cache.
-                        var cacheStorageProvider = scope.Resolve<IOutputCacheStorageProvider>();
-                        cacheStorageProvider.Set(_cacheKey, cacheItem);
+                // Capture the response output using a custom filter stream.
+                var response = filterContext.HttpContext.Response;
+                var captureStream = new CaptureStream(response.Filter);
+                response.Filter = captureStream;
+                captureStream.Captured += (output) => {
+                    try {
+                        // Since this is a callback any call to injected dependencies can result in an Autofac exception: "Instances 
+                        // cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it has already been disposed."
+                        // To prevent access to the original lifetime scope a new work context scope should be created here and dependencies
+                        // should be resolved from it.
 
-                        Logger.Debug("Item '{0}' was written to cache.", _cacheKey);
+                        using (var scope = _workContextAccessor.CreateWorkContextScope()) {
+                            var cacheItem = new CacheItem() {
+                                CachedOnUtc = _now,
+                                Duration = cacheDuration,
+                                GraceTime = cacheGraceTime,
+                                Output = output,
+                                ContentType = response.ContentType,
+                                QueryString = filterContext.HttpContext.Request.Url.Query,
+                                CacheKey = _cacheKey,
+                                InvariantCacheKey = _invariantCacheKey,
+                                Url = filterContext.HttpContext.Request.Url.AbsolutePath,
+                                Tenant = scope.Resolve<ShellSettings>().Name,
+                                StatusCode = response.StatusCode,
+                                Tags = new[] { _invariantCacheKey }.Union(contentItemIds).ToArray()
+                            };
 
-                        // Also add the item tags to the tag cache.
-                        var tagCache = scope.Resolve<ITagCache>();
-                        foreach (var tag in cacheItem.Tags) {
-                            tagCache.Tag(tag, _cacheKey);
+                            // Write the rendered item to the cache.
+                            var cacheStorageProvider = scope.Resolve<IOutputCacheStorageProvider>();
+                            cacheStorageProvider.Remove(_cacheKey);
+                            cacheStorageProvider.Set(_cacheKey, cacheItem);
+
+                            Logger.Debug("Item '{0}' was written to cache.", _cacheKey);
+
+                            // Also add the item tags to the tag cache.
+                            var tagCache = scope.Resolve<ITagCache>();
+                            foreach (var tag in cacheItem.Tags) {
+                                tagCache.Tag(tag, _cacheKey);
+                            }
                         }
                     }
-                }
-                catch {
-                    // Release the renderer task in case something happened
-                    ReleaseWorkers();
-                }
-            };
+                    finally {
+                        // Always release the cache key lock when the request ends.
+                        ReleaseCacheKeyLock();
+                    }
+                };
+
+                captureHandlerIsAttached = true;
+            }
+            finally {
+                // If the response filter stream capture handler was attached then we'll trust
+                // it to release the cache key lock at some point in the future when the stream
+                // is flushed; otherwise we'll make sure we'll release it here.
+                if (!captureHandlerIsAttached)
+                    ReleaseCacheKeyLock();
+            }
         }
 
         protected virtual bool RequestIsCacheable(ActionExecutingContext filterContext) {
@@ -310,12 +312,6 @@ namespace Orchard.OutputCache.Filters {
                 return false;
             }
 
-            // Don't cache if individual route configuration says no.
-            if (_cacheRouteConfig != null && _cacheRouteConfig.Duration == 0) {
-                Logger.Debug("Request for item '{0}' ignored because route is configured to not be cached.", itemDescriptor);
-                return false;
-            }
-
             // Ignore requests with the refresh key on the query string.
             foreach (var key in filterContext.RequestContext.HttpContext.Request.QueryString.AllKeys) {
                 if (String.Equals(_refreshKey, key, StringComparison.OrdinalIgnoreCase)) {
@@ -327,7 +323,7 @@ namespace Orchard.OutputCache.Filters {
             return true;
         }
 
-        protected virtual bool ResponseIsCacheable(ResultExecutedContext filterContext) {
+        protected virtual bool ResponseIsCacheable(ResultExecutedContext filterContext, CacheRouteConfig configuration) {
 
             if (filterContext.HttpContext.Request.Url == null) {
                 return false;
@@ -339,6 +335,12 @@ namespace Orchard.OutputCache.Filters {
                 return false;
             }
 
+            // Don't cache in individual route configuration says no.
+            if (configuration != null && configuration.Duration == 0) {
+                Logger.Debug("Response for item '{0}' will not be cached because route is configured to not be cached.", _cacheKey);
+                return false;
+            }
+
             // Don't cache if request created notifications.
             var hasNotifications = !String.IsNullOrEmpty(Convert.ToString(filterContext.Controller.TempData["messages"]));
             if (hasNotifications) {
@@ -454,7 +456,7 @@ namespace Orchard.OutputCache.Filters {
 
         private CacheSettings CacheSettings {
             get {
-                return _cacheSettings ?? (_cacheSettings = _cacheManager.Get(CacheSettings.CacheKey, context => {
+                return _cacheSettings ?? (_cacheSettings = _cacheManager.Get(CacheSettings.CacheKey, true, context => {
                     context.Monitor(_signals.When(CacheSettings.CacheKey));
                     return new CacheSettings(_workContext.CurrentSite.As<CacheSettingsPart>());
                 }));
@@ -463,7 +465,6 @@ namespace Orchard.OutputCache.Filters {
 
         private void ServeCachedItem(ActionExecutingContext filterContext, CacheItem cacheItem) {
             var response = filterContext.HttpContext.Response;
-            var request = filterContext.HttpContext.Request;
 
             // Fix for missing charset in response headers
             response.Charset = response.Charset;
@@ -473,68 +474,23 @@ namespace Orchard.OutputCache.Filters {
                 response.AddHeader("X-Cached-On", cacheItem.CachedOnUtc.ToString("r"));
                 response.AddHeader("X-Cached-Until", cacheItem.ValidUntilUtc.ToString("r"));
             }
-            
+
             // Shorcut action execution.
             filterContext.Result = new FileContentResult(cacheItem.Output, cacheItem.ContentType);
+
             response.StatusCode = cacheItem.StatusCode;
 
-            // Add ETag header
-            if (HttpRuntime.UsingIntegratedPipeline && response.Headers.Get("ETag") == null) {
-                response.Headers["ETag"] = cacheItem.ETag;
-            }
-
-            // Check ETag in request
-            // https://www.w3.org/2005/MWI/BPWG/techs/CachingWithETag.html
-            var etag = request.Headers["If-None-Match"];
-            if (!String.IsNullOrEmpty(etag)) {
-                if (String.Equals(etag, cacheItem.ETag, StringComparison.Ordinal)) {
-                    // ETag matches the cached item, we return a 304
-                    filterContext.Result = new HttpStatusCodeResult(HttpStatusCode.NotModified);
-                    return;
-                }
-            }
-
             ApplyCacheControl(response);
         }
 
-        private void BeginRenderItem(ActionExecutingContext filterContext, string key) {
+        private void BeginRenderItem(ActionExecutingContext filterContext) {
+
             var response = filterContext.HttpContext.Response;
-            CacheItem cacheItem = null;
 
-            // Tries to acquire a lock to render an item, or wait for the 
-            // actual render to finish an get the CachedItem directly
-            TaskCompletionSource<CacheItem> tcs;
-            if (!_state.Renderers.TryGetValue(key, out tcs)) {
-                Logger.Debug("Acquired processing role: {0}", _cacheKey);
+            ApplyCacheControl(response);
 
-                tcs = new TaskCompletionSource<CacheItem>();
-
-                // If the key could not be added it means another thread got there first,
-                // and in this case they will both try to render the page which is fine.
-                // The chances are very low however as even at it needs to happen between two 
-                // statements, even at a high RPS it would be uncommon.
-                _state.Renderers.TryAdd(key, tcs);
-            }
-            else {
-                Logger.Debug("Waiting for processed page: {0}", _cacheKey);
-
-                // We got a task completion source that will have the CacheItem once finished
-                cacheItem = tcs.Task.Result;
-            }
-
-            // the cache item can be null if an error occured while rendering
-            if (cacheItem != null) {
-                Logger.Debug("Serving result: {0}", _cacheKey);
-                ServeCachedItem(filterContext, cacheItem);
-            }
-            else {
-                Logger.Debug("Processing page: {0}", _cacheKey);
-
-                ApplyCacheControl(response);
-
-                // Remember that we should intercept the rendered response output.
-                _isCachingRequest = true;
-            }
+            // Remember that we should intercept the rendered response output.
+            _isCachingRequest = true;
         }
 
         private void ApplyCacheControl(HttpResponseBase response) {
@@ -553,6 +509,15 @@ namespace Orchard.OutputCache.Filters {
             // response.DisableKernelCache();
             // response.Cache.SetOmitVaryStar(true);
 
+            // An ETag is a string that uniquely identifies a specific version of a component.
+            // We use the cache item to detect if it's a new one.
+            if (HttpRuntime.UsingIntegratedPipeline) {
+                if (response.Headers.Get("ETag") == null) {
+                    // What is the point of GetHashCode() of a newly generated item? /DanielStolt
+                    response.Cache.SetETag(new CacheItem().GetHashCode().ToString(CultureInfo.InvariantCulture));
+                }
+            }
+
             if (CacheSettings.VaryByQueryStringParameters == null) {
                 response.Cache.VaryByParams["*"] = true;
             }
@@ -566,7 +531,15 @@ namespace Orchard.OutputCache.Filters {
                 response.Cache.VaryByHeaders[varyRequestHeader] = true;
             }
         }
-        
+
+        private void ReleaseCacheKeyLock() {
+            if (_cacheKey != null && Monitor.IsEntered(_cacheKey)) {
+                Logger.Debug("Releasing cache key lock for item '{0}'.", _cacheKey);
+                Monitor.Exit(_cacheKey);
+                _cacheKey = null;
+            }
+        }
+
         protected virtual bool IsIgnoredUrl(string url, IEnumerable<string> ignoredUrls) {
             if (ignoredUrls == null || !ignoredUrls.Any()) {
                 return false;
@@ -613,53 +586,15 @@ namespace Orchard.OutputCache.Filters {
         }
 
         protected virtual CacheItem GetCacheItem(string key) {
-            TaskCompletionSource<CacheItem> tcs;
-            if (!_state.Cachers.TryGetValue(key, out tcs)) {
-                tcs = new TaskCompletionSource<CacheItem>();
-
-                // If the key could not be added it means another thread got there first,
-                // and in this case they will both try to get the cache which is fine.
-                // The chances are very low however as even at it needs to happen between two 
-                // statements, even at a high RPS it would be uncommon.
-                _state.Cachers.TryAdd(key, tcs);
-
-                Logger.Debug("Fetching cache item: {0}", _cacheKey);
-
-                try {
-                    var cacheItem = _cacheStorageProvider.GetCacheItem(key);
-                    if (_state.Cachers.TryRemove(key, out tcs)) {
-                        tcs.SetResult(cacheItem);
-                    }
-                    return cacheItem;
-                }
-                catch (Exception e) {
-                    Logger.Error(e, "An unexpected error occured while reading a cache entry");
-                    if (_state.Cachers.TryRemove(key, out tcs)) {
-                        tcs.SetResult(null);
-                    }
-                    return null;
-                }
+            try {
+                var cacheItem = _cacheStorageProvider.GetCacheItem(key);
+                return cacheItem;
             }
-            else {
-                Logger.Debug("Awaiting cache worker: {0}", _cacheKey);
-
-                // We got a task completion source that will have the CacheItem once finished
-                return tcs.Task.Result;
+            catch (Exception e) {
+                Logger.Error(e, "An unexpected error occured while reading a cache entry");
             }
-        }
 
-        private void ReleaseWorkers() {
-            if (_cacheKey != null) {
-                TaskCompletionSource<CacheItem> tcs;
-
-                if (_state.Renderers.TryRemove(_cacheKey, out tcs)) {
-                    tcs.SetResult(null);
-                }
-
-                if (_state.Cachers.TryRemove(_cacheKey, out tcs)) {
-                    tcs.SetResult(null);
-                }
-            }
+            return null;
         }
 
         public void Dispose() {
@@ -673,7 +608,9 @@ namespace Orchard.OutputCache.Filters {
                     // Free other state (managed objects).
                 }
 
-                ReleaseWorkers();
+                if (_cacheKey != null && Monitor.IsEntered(_cacheKey)) {
+                    Monitor.Exit(_cacheKey);
+                }
 
                 _isDisposed = true;
             }
@@ -689,4 +626,4 @@ namespace Orchard.OutputCache.Filters {
     public class ViewDataContainer : IViewDataContainer {
         public ViewDataDictionary ViewData { get; set; }
     }
-}
+}

src/Orchard.Web/Modules/Orchard.OutputCache/Models/CacheItem.cs

@@ -4,7 +4,7 @@ namespace Orchard.OutputCache.Models {
     [Serializable]
     public class CacheItem {
         // used for serialization compatibility
-        public static readonly string Version = "2";
+        public static readonly string Version = "1";
 
         public DateTime CachedOnUtc { get; set; }
         public int Duration { get; set; }
@@ -18,7 +18,6 @@ namespace Orchard.OutputCache.Models {
         public string Tenant { get; set; }
         public int StatusCode { get; set; }
         public string[] Tags { get; set; }
-        public string ETag { get; set; }
 
         public int ValidFor {
             get { return Duration; }

src/Orchard.Web/Modules/Orchard.OutputCache/Orchard.OutputCache.csproj

@@ -115,13 +115,11 @@
     <Compile Include="Services\DefaultTagCache.cs" />
     <Compile Include="Services\ICacheControlStrategy.cs" />
     <Compile Include="Services\IDisplayedContentItemHandler.cs" />
-    <Compile Include="Services\IOutputCacheFilterState.cs" />
     <Compile Include="Services\ITagCache.cs" />
     <Compile Include="Services\DefaultCacheStorageProvider.cs" />
     <Compile Include="Services\ICacheService.cs" />
     <Compile Include="Services\IOutputCacheStorageProvider.cs" />
     <Compile Include="Models\CacheSettings.cs" />
-    <Compile Include="Services\OutputCacheFilterState.cs" />
     <Compile Include="ViewModels\StatisticsViewModel.cs" />
     <Compile Include="ViewModels\IndexViewModel.cs" />
     <Compile Include="Models\CacheRouteConfig.cs" />

src/Orchard.Web/Modules/Orchard.OutputCache/Services/CacheService.cs

@@ -99,7 +99,7 @@ namespace Orchard.OutputCache.Services {
         }
 
         public IEnumerable<CacheRouteConfig> GetRouteConfigs() {
-            return _cacheManager.Get(RouteConfigsCacheKey,
+            return _cacheManager.Get(RouteConfigsCacheKey, true,
                 ctx => {
                     ctx.Monitor(_signals.When(RouteConfigsCacheKey));
                     return _repository.Fetch(c => true).Select(c => new CacheRouteConfig { RouteKey = c.RouteKey, Duration = c.Duration, GraceTime = c.GraceTime }).ToReadOnlyCollection();

src/Orchard.Web/Modules/Orchard.OutputCache/Services/DefaultCacheStorageProvider.cs

@@ -4,7 +4,6 @@ using System.Collections.Generic;
 using System.Linq;
 using Orchard.OutputCache.Models;
 using Orchard.Environment.Configuration;
-using Orchard.Logging;
 
 namespace Orchard.OutputCache.Services {
     public class DefaultCacheStorageProvider : IOutputCacheStorageProvider {
@@ -16,12 +15,7 @@ namespace Orchard.OutputCache.Services {
             _tenantName = shellSettings.Name;
         }
 
-        public ILogger Logger { get; set; }
-         
         public void Set(string key, CacheItem cacheItem) {
-            Logger.Debug("Set {0}", key);
-
-            _workContext.HttpContext.Cache.Remove(key);
             _workContext.HttpContext.Cache.Add(
                 key,
                 cacheItem,
@@ -33,14 +27,10 @@ namespace Orchard.OutputCache.Services {
         }
 
         public void Remove(string key) {
-            Logger.Debug("Remove {0}", key);
-
             _workContext.HttpContext.Cache.Remove(key);
         }
 
         public void RemoveAll() {
-            Logger.Debug("RemoveAll");
-
             var items = GetCacheItems(0, 100).ToList();
             while (items.Any()) {
                 foreach (var item in items) {
@@ -51,14 +41,10 @@ namespace Orchard.OutputCache.Services {
         }
 
         public CacheItem GetCacheItem(string key) {
-            Logger.Debug("GetCacheItem {0}", key);
-
             return _workContext.HttpContext.Cache.Get(key) as CacheItem;
         }
 
         public IEnumerable<CacheItem> GetCacheItems(int skip, int count) {
-            Logger.Debug("GetCacheItems");
-
             // the ASP.NET cache can also contain other types of items
             return _workContext.HttpContext.Cache.AsParallel()
                         .Cast<DictionaryEntry>()
@@ -70,8 +56,6 @@ namespace Orchard.OutputCache.Services {
         }
 
         public int GetCacheItemsCount() {
-            Logger.Debug("GetCacheItemsCount");
-
             return _workContext.HttpContext.Cache.AsParallel()
                         .Cast<DictionaryEntry>()
                         .Select(x => x.Value)

src/Orchard.Web/Modules/Orchard.OutputCache/Services/FileSystemOutputCacheBackgroundTask.cs

@@ -1,4 +1,5 @@
-using System.Linq;
+using System.IO;
+using System.Linq;
 using Orchard.Caching;
 using Orchard.Environment.Configuration;
 using Orchard.Environment.Extensions;
@@ -18,7 +19,8 @@ namespace Orchard.OutputCache.Services {
         private readonly IClock _clock;
         private readonly ISignals _signals;
 
-        private string _root;
+        private string _content;
+        private string _metadata;
 
         public FileSystemOutputCacheBackgroundTask(
             IAppDataFolder appDataFolder, 
@@ -32,22 +34,26 @@ namespace Orchard.OutputCache.Services {
             _clock = clock;
             _signals = signals;
 
-            _root = _appDataFolder.Combine("OutputCache", _shellSettings.Name);
+            _metadata = FileSystemOutputCacheStorageProvider.GetMetadataPath(appDataFolder, _shellSettings.Name);
+            _content = FileSystemOutputCacheStorageProvider.GetContentPath(appDataFolder, _shellSettings.Name);
         }
 
         public void Sweep() {
-            foreach(var filename in _appDataFolder.ListFiles(_root).ToArray()) {
-                var validUntilUtc = _cacheManager.Get(filename, context => {
-                    _signals.When(filename);
+            foreach(var filename in _appDataFolder.ListFiles(_metadata).ToArray()) {
+                var hash = Path.GetFileName(filename);
+
+                var validUntilUtc = _cacheManager.Get(hash, context => {
+                    _signals.When(hash);
 
                     using (var stream = _appDataFolder.OpenFile(filename)) {
-                        var cacheItem = FileSystemOutputCacheStorageProvider.Deserialize(stream);
+                        var cacheItem = FileSystemOutputCacheStorageProvider.DeserializeMetadata(stream);
                         return cacheItem.ValidUntilUtc;
                     }
                 });
 
                 if (_clock.UtcNow > validUntilUtc) {
-                    _appDataFolder.DeleteFile(filename);
+                    _appDataFolder.DeleteFile(_appDataFolder.Combine(_metadata, hash));
+                    _appDataFolder.DeleteFile(_appDataFolder.Combine(_content, hash));
                     _signals.Trigger(filename);
                 }
             }

src/Orchard.Web/Modules/Orchard.OutputCache/Services/FileSystemOutputCacheProvider.cs

@@ -1,45 +1,51 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using Orchard.OutputCache.Models;
-using Orchard.Environment.Extensions;
-using Orchard.Logging;
-using Orchard.Services;
-using Orchard.FileSystems.AppData;
-using Orchard.Environment.Configuration;
-using System.Web;
 using System.IO;
+using System.Linq;
 using System.Runtime.Serialization.Formatters.Binary;
+using System.Security.Cryptography;
+using System.Text;
+using Orchard.Environment.Configuration;
+using Orchard.Environment.Extensions;
+using Orchard.FileSystems.AppData;
+using Orchard.Logging;
+using Orchard.OutputCache.Models;
+using Orchard.Services;
 
 namespace Orchard.OutputCache.Services {
     [OrchardFeature("Orchard.OutputCache.FileSystem")]
     [OrchardSuppressDependency("Orchard.OutputCache.Services.DefaultCacheStorageProvider")]
     /// <summary>
     /// This class provides an implementation of <see cref="IOutputCacheStorageProvider"/>
-    /// based on the local App_Data folder, inside <c>OuputCache/{tenant}</c>. It is not 
-    /// recommended when used in a server farm.
+    /// based on the local App_Data folder, inside <c>FileCache/{tenant}</c>. It is not 
+    /// recommended when used in a server farm, unless the file system is share (Azure App Services).
     /// The <see cref="CacheItem"/> instances are binary serialized.
     /// </summary>
     /// <remarks>
-    /// This provider doesn't implement quotas support yet.
+    /// This provider doesn't support quotas yet.
     /// </remarks>
     public class FileSystemOutputCacheStorageProvider : IOutputCacheStorageProvider {
         private readonly IClock _clock;
         private readonly IAppDataFolder _appDataFolder;
         private readonly ShellSettings _shellSettings;
-        private readonly string _root;
+        private readonly string _metadata;
+        private readonly string _content;
+
+        public static char[] InvalidPathChars = { '/', '\\', ':', '*', '?', '>', '<', '|' };
 
         public FileSystemOutputCacheStorageProvider(IClock clock, IAppDataFolder appDataFolder, ShellSettings shellSettings) {
             _appDataFolder = appDataFolder;
             _clock = clock;
             _shellSettings = shellSettings;
-            _root = _appDataFolder.Combine("OutputCache", _shellSettings.Name);
+
+            _metadata = GetMetadataPath(appDataFolder, _shellSettings.Name);
+            _content = GetContentPath(appDataFolder, _shellSettings.Name);
 
             Logger = NullLogger.Instance;
         }
 
         public ILogger Logger { get; set; }
- 
+
         public void Set(string key, CacheItem cacheItem) {
             Retry(() => {
                 if (cacheItem == null) {
@@ -50,82 +56,170 @@ namespace Orchard.OutputCache.Services {
                     return;
                 }
 
-                var filename = GetCacheItemFilename(key);
+                var hash = GetCacheItemFileHash(key);
 
-                using (var stream = Serialize(cacheItem)) {
-                    using (var fileStream = _appDataFolder.CreateFile(filename)) {
-                        stream.CopyTo(fileStream);
+                lock (String.Intern(hash)) {
+                    using (var stream = SerializeContent(cacheItem)) {
+                        var filename = _appDataFolder.Combine(_content, hash);
+                        using (var fileStream = _appDataFolder.CreateFile(filename)) {
+                            stream.CopyTo(fileStream);
+                        }
+                    }
+
+                    using (var stream = SerializeMetadata(cacheItem)) {
+                        var filename = _appDataFolder.Combine(_metadata, hash);
+                        using (var fileStream = _appDataFolder.CreateFile(filename)) {
+                            stream.CopyTo(fileStream);
+                        }
                     }
                 }
             });
         }
 
         public void Remove(string key) {
-            Retry(() => {
-                var filename = GetCacheItemFilename(key);
-                if (_appDataFolder.FileExists(filename)) {
-                    _appDataFolder.DeleteFile(filename);
-                }
-            });
+            var hash = GetCacheItemFileHash(key);
+            lock (String.Intern(hash)) {
+                Retry(() => {
+                    var filename = _appDataFolder.Combine(_metadata, hash);
+                    if (_appDataFolder.FileExists(filename)) {
+                        _appDataFolder.DeleteFile(filename);
+                    }
+                });
+
+                Retry(() => {
+                    var filename = _appDataFolder.Combine(_content, hash);
+                    if (_appDataFolder.FileExists(filename)) {
+                        _appDataFolder.DeleteFile(filename);
+                    }
+                });
+            }
         }
 
         public void RemoveAll() {
-            foreach(var filename in _appDataFolder.ListFiles(_root)) {
-                if(_appDataFolder.FileExists(filename)) {
-                    _appDataFolder.DeleteFile(filename);
+            foreach (var folder in new[] { _metadata, _content }) {
+                foreach (var filename in _appDataFolder.ListFiles(folder)) {
+                    var hash = Path.GetFileName(filename);
+                    lock (String.Intern(hash)) {
+                        try {
+                            if (_appDataFolder.FileExists(filename)) {
+                                _appDataFolder.DeleteFile(filename);
+                            }
+                        }
+                        catch (Exception e) {
+                            Logger.Warning(e, "An error occured while deleting the file: {0}", filename);
+                        }
+                    }
                 }
             }
         }
 
         public CacheItem GetCacheItem(string key) {
             return Retry(() => {
-                var filename = GetCacheItemFilename(key);
+                var hash = GetCacheItemFileHash(key);
+                lock (String.Intern(hash)) {
+                    var filename = _appDataFolder.Combine(_metadata, hash);
 
-                if (!_appDataFolder.FileExists(filename)) {
-                    return null;
-                }
-
-                using (var stream = _appDataFolder.OpenFile(filename)) {
-
-                    if (stream == null) {
+                    if (!_appDataFolder.FileExists(filename)) {
                         return null;
                     }
 
-                    return Deserialize(stream);
+                    CacheItem cacheItem = null;
+
+                    using (var stream = _appDataFolder.OpenFile(filename)) {
+                        if (stream == null) {
+                            return null;
+                        }
+
+                        cacheItem = DeserializeMetadata(stream);
+
+                        // We compare the requested key and the one stored in the metadata
+                        // as there could be key collisions with the hashed filenames.
+                        if (!cacheItem.CacheKey.Equals(key)) {
+                            return null;
+                        }
+                    }
+
+                    filename = _appDataFolder.Combine(_content, hash);
+                    using (var stream = _appDataFolder.OpenFile(filename)) {
+                        if (stream == null) {
+                            return null;
+                        }
+
+                        var content = DeserializeContent(stream);
+                        cacheItem.Output = content;
+                    }
+
+                    return cacheItem;
                 }
             });
         }
 
         public IEnumerable<CacheItem> GetCacheItems(int skip, int count) {
-            return _appDataFolder.ListFiles(_root)
+            return _appDataFolder.ListFiles(_metadata)
                 .OrderBy(x => x)
                 .Skip(skip)
                 .Take(count)
                 .Select(filename => {
                     using (var stream = _appDataFolder.OpenFile(filename)) {
-                        return Deserialize(stream);
+                        return DeserializeMetadata(stream);
                     }
                 })
                 .ToList();
         }
 
         public int GetCacheItemsCount() {
-            return _appDataFolder.ListFiles(_root).Count();
-        }
-        
-        private string GetCacheItemFilename(string key) {
-            return _appDataFolder.Combine(_root, HttpUtility.UrlEncode(key));
+            return _appDataFolder.ListFiles(_metadata).Count();
         }
 
-        internal static MemoryStream Serialize(CacheItem item) {
+        public static string GetMetadataPath(IAppDataFolder appDataFolder, string tenant) {
+            return appDataFolder.Combine("FileCache", tenant, "metadata");
+        }
+
+        public static string GetContentPath(IAppDataFolder appDataFolder, string tenant) {
+            return appDataFolder.Combine("FileCache", tenant, "content");
+        }
+
+        private string GetCacheItemFileHash(string key) {
+            // The key is typically too long to be useful, so we use a hash
+            using (var md5 = MD5.Create()) {
+                var keyBytes = Encoding.UTF8.GetBytes(key);
+                var hashedBytes = md5.ComputeHash(keyBytes);
+                var b64 = Convert.ToBase64String(hashedBytes);
+                return String.Join("-", b64.Split(InvalidPathChars, StringSplitOptions.RemoveEmptyEntries));
+            }
+        }
+
+        internal static MemoryStream SerializeContent(CacheItem item) {
             BinaryFormatter binaryFormatter = new BinaryFormatter();
             var memoryStream = new MemoryStream();
-            binaryFormatter.Serialize(memoryStream, item);
+            binaryFormatter.Serialize(memoryStream, item.Output);
             memoryStream.Seek(0, SeekOrigin.Begin);
             return memoryStream;
         }
 
-        internal static CacheItem Deserialize(Stream stream) {
+        internal static byte[] DeserializeContent(Stream stream) {
+            BinaryFormatter binaryFormatter = new BinaryFormatter();
+            var result = (byte[])binaryFormatter.Deserialize(stream);
+            return result;
+        }
+
+        internal static MemoryStream SerializeMetadata(CacheItem item) {
+            var output = item.Output;
+            item.Output = new byte[0];
+
+            try {
+                BinaryFormatter binaryFormatter = new BinaryFormatter();
+                var memoryStream = new MemoryStream();
+                binaryFormatter.Serialize(memoryStream, item);
+                memoryStream.Seek(0, SeekOrigin.Begin);
+                return memoryStream;
+            }
+            finally {
+                item.Output = output;
+            }            
+        }
+
+        internal static CacheItem DeserializeMetadata(Stream stream) {
             BinaryFormatter binaryFormatter = new BinaryFormatter();
             var result = (CacheItem)binaryFormatter.Deserialize(stream);
             return result;
@@ -138,7 +232,9 @@ namespace Orchard.OutputCache.Services {
                     var t = action();
                     return t;
                 }
-                catch {
+                catch(Exception e) {
+                    Logger.Warning("An unexpected error occured, attempt # {0}, i", e);
+
                     if (i == retries) {
                         throw;
                     }
@@ -153,9 +249,12 @@ namespace Orchard.OutputCache.Services {
             for(int i=1; i <= retries; i++) {
                 try {
                     action();
+                    return;
                 }
-                catch {
-                    if(i == retries) {
+                catch(Exception e) {
+                    Logger.Warning("An unexpected error occured, attempt # {0}, i", e);
+
+                    if (i == retries) {
                         throw;
                     }
                 }

src/Orchard.Web/Modules/Orchard.OutputCache/Services/IOutputCacheFilterState.cs

@@ -1,19 +0,0 @@
-using System.Collections.Concurrent;
-using System.Threading.Tasks;
-using Orchard.OutputCache.Models;
-
-namespace Orchard.OutputCache.Services {
-    public interface IOutputCacheFilterState : ISingletonDependency {
-        
-        /// <summary>
-        /// Tasks that are currently rendering pages
-        /// </summary>
-        ConcurrentDictionary<string, TaskCompletionSource<CacheItem>> Renderers { get; }
-
-        /// <summary>
-        /// Tasks that are currently retrieving cached items
-        /// </summary>
-        ConcurrentDictionary<string, TaskCompletionSource<CacheItem>> Cachers { get; }
-
-    }
-}

src/Orchard.Web/Modules/Orchard.OutputCache/Services/OutputCacheFilterState.cs

@@ -1,16 +0,0 @@
-using System.Collections.Concurrent;
-using System.Threading.Tasks;
-using Orchard.OutputCache.Models;
-
-namespace Orchard.OutputCache.Services {
-    public class OutputCacheFilterState : IOutputCacheFilterState {
-        public OutputCacheFilterState() {
-            Cachers = new ConcurrentDictionary<string, TaskCompletionSource<CacheItem>>();
-            Renderers = new ConcurrentDictionary<string, TaskCompletionSource<CacheItem>>();
-        }
-
-        public ConcurrentDictionary<string, TaskCompletionSource<CacheItem>> Cachers { get; private set; }
-
-        public ConcurrentDictionary<string, TaskCompletionSource<CacheItem>> Renderers { get; private set; }
-    }
-}