Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Invalidate PageActionInvokerProvider cache when compilation cache exp…
Browse files Browse the repository at this point in the history
…ires

Fixes #6428
  • Loading branch information
pranavkm committed Jun 22, 2017
1 parent de64c84 commit 2de7fc8
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface IPageLoader
/// Produces a <see cref="CompiledPageActionDescriptor"/> given a <see cref="PageActionDescriptor"/>.
/// </summary>
/// <param name="actionDescriptor">The <see cref="PageActionDescriptor"/>.</param>
/// <returns>The <see cref="CompiledPageActionDescriptor"/>.</returns>
CompiledPageActionDescriptor Load(PageActionDescriptor actionDescriptor);
/// <returns>The <see cref="PageLoaderResult"/>.</returns>
PageLoaderResult Load(PageActionDescriptor actionDescriptor);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
{
public struct PageLoaderResult
{
public PageLoaderResult(
CompiledPageActionDescriptor actionDescriptor,
IList<IChangeToken> changeTokens)
{
ActionDescriptor = actionDescriptor;
ExpirationTokens = changeTokens;
}

public CompiledPageActionDescriptor ActionDescriptor { get; }

public IList<IChangeToken> ExpirationTokens { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ public DefaultPageLoader(IViewCompilerProvider viewCompilerProvider)

private IViewCompiler Compiler => _viewCompilerProvider.GetCompiler();

public CompiledPageActionDescriptor Load(PageActionDescriptor actionDescriptor)
public PageLoaderResult Load(PageActionDescriptor actionDescriptor)
{
var compileTask = Compiler.CompileAsync(actionDescriptor.RelativePath);
var viewDescriptor = compileTask.GetAwaiter().GetResult();
var pageAttribute = (RazorPageAttribute)viewDescriptor.ViewAttribute;

return CreateDescriptor(actionDescriptor, pageAttribute);
var descriptor = CreateDescriptor(actionDescriptor, pageAttribute);
return new PageLoaderResult(descriptor, viewDescriptor.ExpirationTokens);
}

// Internal for unit testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
Expand All @@ -17,6 +16,7 @@
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

Expand Down Expand Up @@ -97,15 +97,24 @@ public void OnProvidersExecuting(ActionInvokerProviderContext context)
}

var cache = CurrentCache;
PageActionInvokerCacheEntry cacheEntry;

IFilterMetadata[] filters;
if (!cache.Entries.TryGetValue(actionDescriptor, out cacheEntry))
if (!cache.Entries.TryGetValue(actionDescriptor, out PageActionInvokerCacheEntry cacheEntry))
{
var loaderResult = _loader.Load(actionDescriptor);
var compiledActionDescriptor = loaderResult.ActionDescriptor;

var filterFactoryResult = FilterFactory.GetAllFilters(_filterProviders, actionContext);
filters = filterFactoryResult.Filters;
cacheEntry = CreateCacheEntry(context, filterFactoryResult.CacheableFilters);
cacheEntry = cache.Entries.GetOrAdd(actionDescriptor, cacheEntry);
cacheEntry = CreateCacheEntry(context, compiledActionDescriptor, filterFactoryResult.CacheableFilters);

var cacheEntryOptions = new MemoryCacheEntryOptions();
for (var i = 0; i < loaderResult.ExpirationTokens.Count; i++)
{
cacheEntryOptions.ExpirationTokens.Add(loaderResult.ExpirationTokens[i]);
}

cacheEntry = cache.Entries.Set(actionDescriptor, cacheEntry, cacheEntryOptions);
}
else
{
Expand All @@ -120,7 +129,6 @@ public void OnProvidersExecuting(ActionInvokerProviderContext context)

public void OnProvidersExecuted(ActionInvokerProviderContext context)
{

}

private InnerCache CurrentCache
Expand Down Expand Up @@ -167,11 +175,9 @@ private PageActionInvoker CreateActionInvoker(

private PageActionInvokerCacheEntry CreateCacheEntry(
ActionInvokerProviderContext context,
CompiledPageActionDescriptor compiledActionDescriptor,
FilterItem[] cachedFilters)
{
var actionDescriptor = (PageActionDescriptor)context.ActionContext.ActionDescriptor;
var compiledActionDescriptor = _loader.Load(actionDescriptor);

var viewDataFactory = ViewDataDictionaryFactory.CreateFactory(compiledActionDescriptor.ModelTypeInfo);

var pageFactory = _pageFactoryProvider.CreatePageFactory(compiledActionDescriptor);
Expand Down Expand Up @@ -250,8 +256,7 @@ public InnerCache(int version)
Version = version;
}

public ConcurrentDictionary<ActionDescriptor, PageActionInvokerCacheEntry> Entries { get; } =
new ConcurrentDictionary<ActionDescriptor, PageActionInvokerCacheEntry>();
public MemoryCache Entries { get; } = new MemoryCache(new MemoryCacheOptions());

public int Version { get; }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Threading;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Filters;
Expand All @@ -17,7 +18,6 @@
using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure;
using Microsoft.AspNetCore.Mvc.Rendering;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Logging.Abstractions;
Expand Down Expand Up @@ -45,7 +45,7 @@ public void OnProvidersExecuting_WithEmptyModel_PopulatesCacheEntry()
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor));
.Returns(CreatePageLoaderResult(descriptor));

var pageFactoryProvider = new Mock<IPageFactoryProvider>();
pageFactoryProvider
Expand Down Expand Up @@ -100,7 +100,7 @@ public void OnProvidersExecuting_WithModel_PopulatesCacheEntry()
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor, pageType: typeof(PageWithModel)));
.Returns(CreatePageLoaderResult(descriptor, pageType: typeof(PageWithModel)));

var pageFactoryProvider = new Mock<IPageFactoryProvider>();
pageFactoryProvider
Expand Down Expand Up @@ -172,7 +172,7 @@ public void OnProvidersExecuting_CachesViewStartFactories()
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor, pageType: typeof(PageWithModel)));
.Returns(CreatePageLoaderResult(descriptor, pageType: typeof(PageWithModel)));

var razorPageFactoryProvider = new Mock<IRazorPageFactoryProvider>();

Expand Down Expand Up @@ -230,7 +230,7 @@ public void OnProvidersExecuting_CachesEntries()
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor));
.Returns(CreatePageLoaderResult(descriptor));

var invokerProvider = CreateInvokerProvider(
loader.Object,
Expand Down Expand Up @@ -261,6 +261,57 @@ public void OnProvidersExecuting_CachesEntries()
Assert.Same(entry1, entry2);
}

[Fact]
public void OnProvidersExecuting_InvalidatesCompiledActionDescriptor_IfCompilationExpires()
{
// Arrange
var descriptor = new PageActionDescriptor
{
RelativePath = "Path1",
FilterDescriptors = new FilterDescriptor[0],
};

var tokenSource = new CancellationTokenSource();
var changeToken = new CancellationChangeToken(tokenSource.Token);
var result1 = CreatePageLoaderResult(descriptor, expirationTokens: new[] { changeToken });
var result2 = CreatePageLoaderResult(descriptor, expirationTokens: new[] { changeToken });

var loader = new Mock<IPageLoader>();
loader
.SetupSequence(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(result1)
.Returns(result2);

var invokerProvider = CreateInvokerProvider(
loader.Object,
CreateActionDescriptorCollection(descriptor));

var context = new ActionInvokerProviderContext(new ActionContext()
{
ActionDescriptor = descriptor,
HttpContext = new DefaultHttpContext(),
RouteData = new RouteData(),
});

// Act - 1
invokerProvider.OnProvidersExecuting(context);

// Assert - 1
Assert.NotNull(context.Result);
var actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
var entry1 = actionInvoker.CacheEntry;

// Act - 2
tokenSource.Cancel();
invokerProvider.OnProvidersExecuting(context);

// Assert - 2
Assert.NotNull(context.Result);
actionInvoker = Assert.IsType<PageActionInvoker>(context.Result);
var entry2 = actionInvoker.CacheEntry;
Assert.NotSame(entry1, entry2);
}

[Fact]
public void OnProvidersExecuting_UpdatesEntriesWhenActionDescriptorProviderCollectionIsUpdated()
{
Expand All @@ -283,7 +334,7 @@ public void OnProvidersExecuting_UpdatesEntriesWhenActionDescriptorProviderColle
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor));
.Returns(CreatePageLoaderResult(descriptor));

var invokerProvider = CreateInvokerProvider(
loader.Object,
Expand Down Expand Up @@ -328,7 +379,7 @@ public void GetViewStartFactories_FindsFullHeirarchy()
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor, typeof(TestPageModel)));
.Returns(CreatePageLoaderResult(descriptor, typeof(TestPageModel)));

var fileProvider = new TestFileProvider();
fileProvider.AddFile("/View/Deeper/Not_ViewStart.cshtml", "page content");
Expand Down Expand Up @@ -364,10 +415,10 @@ public void GetViewStartFactories_FindsFullHeirarchy()
razorProject: razorProject,
razorPagesOptions: new RazorPagesOptions { RootDirectory = "/" });

var compiledDescriptor = CreateCompiledPageActionDescriptor(descriptor);
var loaderResult = CreatePageLoaderResult(descriptor);

// Act
var factories = invokerProvider.GetViewStartFactories(compiledDescriptor);
var factories = invokerProvider.GetViewStartFactories(loaderResult.ActionDescriptor);

// Assert
mock.Verify();
Expand All @@ -394,7 +445,7 @@ public void GetPageFactories_DoesNotFindViewStartsOutsideBaseDirectory(string ro
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(compiledPageDescriptor);
.Returns(new PageLoaderResult(compiledPageDescriptor, Array.Empty<IChangeToken>()));

var fileProvider = new TestFileProvider();
fileProvider.AddFile("/_ViewStart.cshtml", "page content");
Expand Down Expand Up @@ -451,7 +502,7 @@ public void GetViewStartFactories_ReturnsFactoriesForFilesThatDoNotExistInProjec
var loader = new Mock<IPageLoader>();
loader
.Setup(l => l.Load(It.IsAny<PageActionDescriptor>()))
.Returns(CreateCompiledPageActionDescriptor(descriptor, typeof(TestPageModel)));
.Returns(CreatePageLoaderResult(descriptor, typeof(TestPageModel)));

var pageFactory = new Mock<IRazorPageFactoryProvider>();
pageFactory
Expand All @@ -477,18 +528,19 @@ public void GetViewStartFactories_ReturnsFactoriesForFilesThatDoNotExistInProjec
razorProject: razorProject,
razorPagesOptions: new RazorPagesOptions { RootDirectory = "/" });

var compiledDescriptor = CreateCompiledPageActionDescriptor(descriptor);
var loaderResult = CreatePageLoaderResult(descriptor);

// Act
var factories = invokerProvider.GetViewStartFactories(compiledDescriptor).ToList();
var factories = invokerProvider.GetViewStartFactories(loaderResult.ActionDescriptor).ToList();

// Assert
Assert.Equal(2, factories.Count);
}

private static CompiledPageActionDescriptor CreateCompiledPageActionDescriptor(
private static PageLoaderResult CreatePageLoaderResult(
PageActionDescriptor descriptor,
Type pageType = null)
Type pageType = null,
IList<IChangeToken> expirationTokens = null)
{
pageType = pageType ?? typeof(object);
var pageTypeInfo = pageType.GetTypeInfo();
Expand All @@ -499,12 +551,14 @@ private static CompiledPageActionDescriptor CreateCompiledPageActionDescriptor(
modelTypeInfo = pageTypeInfo.GetProperty("Model")?.PropertyType.GetTypeInfo();
}

return new CompiledPageActionDescriptor(descriptor)
var compiledPageActionDescriptor = new CompiledPageActionDescriptor(descriptor)
{
HandlerTypeInfo = modelTypeInfo ?? pageTypeInfo,
ModelTypeInfo = modelTypeInfo ?? pageTypeInfo,
PageTypeInfo = pageTypeInfo,
};

return new PageLoaderResult(compiledPageActionDescriptor, expirationTokens ?? Array.Empty<IChangeToken>());
}

private static PageActionInvokerProvider CreateInvokerProvider(
Expand Down

0 comments on commit 2de7fc8

Please sign in to comment.