From dfddc4e8ff7dceceafd202ffdd9f5b046424e2df Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 8 Feb 2019 09:40:24 -0800 Subject: [PATCH] [MVC] Remove obsolete APIs - Removed ViewsFeatureProvider - Removed PageArgumentBinder and its internal implementation DefaultPageArgumentBinder. - Removed corresponding test classes/methods for all the above. - Reacted to class/member changes in dependencies. #7326 --- .../RazorCompiledItemFeatureProvider.cs | 2 +- .../Compilation/ViewsFeatureProvider.cs | 111 ---------- .../MvcRazorMvcCoreBuilderExtensions.cs | 7 - .../MvcRazorPagesMvcCoreBuilderExtensions.cs | 5 - .../DefaultPageArgumentBinder.cs | 50 ----- .../Infrastructure/PageArgumentBinder.cs | 44 ---- .../Infrastructure/RazorPageAdapter.cs | 11 - .../RazorCompiledItemFeatureProviderTest.cs | 2 +- .../Compilation/ViewsFeatureProviderTest.cs | 197 ------------------ .../MvcServiceCollectionExtensionsTest.cs | 5 +- 10 files changed, 3 insertions(+), 431 deletions(-) delete mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/Compilation/ViewsFeatureProvider.cs delete mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageArgumentBinder.cs delete mode 100644 src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs delete mode 100644 src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/Compilation/ViewsFeatureProviderTest.cs diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/ApplicationParts/RazorCompiledItemFeatureProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/ApplicationParts/RazorCompiledItemFeatureProvider.cs index ccfc49127d..eece1503ab 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/ApplicationParts/RazorCompiledItemFeatureProvider.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/ApplicationParts/RazorCompiledItemFeatureProvider.cs @@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationParts foreach (var item in provider.CompiledItems) { - var descriptor = new CompiledViewDescriptor(item, attribute: null); + var descriptor = new CompiledViewDescriptor(item); feature.ViewDescriptors.Add(descriptor); } } diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/Compilation/ViewsFeatureProvider.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/Compilation/ViewsFeatureProvider.cs deleted file mode 100644 index 911b010261..0000000000 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/Compilation/ViewsFeatureProvider.cs +++ /dev/null @@ -1,111 +0,0 @@ -// 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; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Reflection; -using Microsoft.AspNetCore.Mvc.ApplicationParts; -using Microsoft.Extensions.Primitives; - -namespace Microsoft.AspNetCore.Mvc.Razor.Compilation -{ - /// - /// An for . - /// - [Obsolete("This type is obsolete and will be removed in a future version. See " + nameof(IRazorCompiledItemProvider) + " for alternatives.")] - public class ViewsFeatureProvider : IApplicationFeatureProvider - { - public static readonly string PrecompiledViewsAssemblySuffix = ".PrecompiledViews"; - - /// - public void PopulateFeature(IEnumerable parts, ViewsFeature feature) - { - foreach (var assemblyPart in parts.OfType()) - { - var viewAttributes = GetViewAttributes(assemblyPart) - .Select(attribute => (Attribute: attribute, RelativePath: ViewPath.NormalizePath(attribute.Path))); - - var duplicates = viewAttributes.GroupBy(a => a.RelativePath, StringComparer.OrdinalIgnoreCase) - .FirstOrDefault(g => g.Count() > 1); - - if (duplicates != null) - { - // Ensure parts do not specify views with differing cases. This is not supported - // at runtime and we should flag at as such for precompiled views. - var viewsDifferingInCase = string.Join(Environment.NewLine, duplicates.Select(d => d.RelativePath)); - - var message = string.Join( - Environment.NewLine, - Resources.RazorViewCompiler_ViewPathsDifferOnlyInCase, - viewsDifferingInCase); - throw new InvalidOperationException(message); - } - - foreach (var (attribute, relativePath) in viewAttributes) - { - var viewDescriptor = new CompiledViewDescriptor - { - ExpirationTokens = Array.Empty(), - RelativePath = relativePath, - ViewAttribute = attribute, - }; - - feature.ViewDescriptors.Add(viewDescriptor); - } - } - } - - /// - /// Gets the sequence of instances associated with the specified . - /// - /// The . - /// The sequence of instances. - protected virtual IEnumerable GetViewAttributes(AssemblyPart assemblyPart) - { - if (assemblyPart == null) - { - throw new ArgumentNullException(nameof(assemblyPart)); - } - - var featureAssembly = GetFeatureAssembly(assemblyPart); - if (featureAssembly != null) - { - return featureAssembly.GetCustomAttributes(); - } - - return Enumerable.Empty(); - } - - private static Assembly GetFeatureAssembly(AssemblyPart assemblyPart) - { - if (assemblyPart.Assembly.IsDynamic || string.IsNullOrEmpty((string)assemblyPart.Assembly.Location)) - { - return null; - } - - var precompiledAssemblyFileName = assemblyPart.Assembly.GetName().Name - + PrecompiledViewsAssemblySuffix - + ".dll"; - - var precompiledAssemblyFilePath = Path.Combine( - Path.GetDirectoryName(assemblyPart.Assembly.Location), - precompiledAssemblyFileName); - - if (File.Exists(precompiledAssemblyFilePath)) - { - try - { - return Assembly.LoadFile(precompiledAssemblyFilePath); - } - catch (FileLoadException) - { - // Don't throw if assembly cannot be loaded. This can happen if the file is not a managed assembly. - } - } - - return null; - } - } -} \ No newline at end of file diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/DependencyInjection/MvcRazorMvcCoreBuilderExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/DependencyInjection/MvcRazorMvcCoreBuilderExtensions.cs index 7b3ee93a3a..fdd018eca7 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/DependencyInjection/MvcRazorMvcCoreBuilderExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/DependencyInjection/MvcRazorMvcCoreBuilderExtensions.cs @@ -72,13 +72,6 @@ namespace Microsoft.Extensions.DependencyInjection { builder.PartManager.FeatureProviders.Add(new RazorCompiledItemFeatureProvider()); } - -#pragma warning disable CS0618 // Type or member is obsolete - if (!builder.PartManager.FeatureProviders.OfType().Any()) - { - builder.PartManager.FeatureProviders.Add(new ViewsFeatureProvider()); - } -#pragma warning restore CS0618 // Type or member is obsolete } /// diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs index dbe98e5fad..b58349c3db 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs @@ -114,11 +114,6 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddSingleton(); - // Page model binding -#pragma warning disable CS0618 // Type or member is obsolete - services.TryAddSingleton(); -#pragma warning restore CS0618 // Type or member is obsolete - // Action executors services.TryAddSingleton(); diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageArgumentBinder.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageArgumentBinder.cs deleted file mode 100644 index acc6b2f001..0000000000 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/DefaultPageArgumentBinder.cs +++ /dev/null @@ -1,50 +0,0 @@ -// 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; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.ModelBinding; - -namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure -{ -#pragma warning disable CS0618 // Type or member is obsolete - internal class DefaultPageArgumentBinder : PageArgumentBinder -#pragma warning restore CS0618 // Type or member is obsolete - { - private readonly ParameterBinder _parameterBinder; - - public DefaultPageArgumentBinder(ParameterBinder binder) - { - _parameterBinder = binder; - } - - protected override async Task BindAsync(PageContext pageContext, object value, string name, Type type) - { - var valueProvider = await GetCompositeValueProvider(pageContext); - var parameterDescriptor = new ParameterDescriptor - { - BindingInfo = null, - Name = name, - ParameterType = type, - }; - -#pragma warning disable CS0618 // Type or member is obsolete - return await _parameterBinder.BindModelAsync(pageContext, valueProvider, parameterDescriptor, value); -#pragma warning restore CS0618 // Type or member is obsolete - } - - private static async Task GetCompositeValueProvider(PageContext pageContext) - { - var factories = pageContext.ValueProviderFactories; - var valueProviderFactoryContext = new ValueProviderFactoryContext(pageContext); - for (var i = 0; i < factories.Count; i++) - { - var factory = factories[i]; - await factory.CreateValueProviderAsync(valueProviderFactoryContext); - } - - return new CompositeValueProvider(valueProviderFactoryContext.ValueProviders); - } - } -} diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs deleted file mode 100644 index a4d8f5cba6..0000000000 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs +++ /dev/null @@ -1,44 +0,0 @@ -// 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; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Mvc.ModelBinding; - -namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure -{ - [Obsolete("This type is obsolete and will be removed in a future version.")] - public abstract class PageArgumentBinder - { - public async Task BindModelAsync(PageContext context, Type type, object @default, string name) - { - var result = await BindAsync(context, null, name, type); - return result.IsModelSet ? result.Model : @default; - } - - public Task BindModelAsync(PageContext context, string name) - { - return BindModelAsync(context, default(TModel), name); - } - - public async Task BindModelAsync(PageContext context, TModel @default, string name) - { - var result = await BindAsync(context, null, name, typeof(TModel)); - return result.IsModelSet ? (TModel)result.Model : @default; - } - - public async Task TryUpdateModelAsync(PageContext context, TModel value) - { - var result = await BindAsync(context, value, string.Empty, typeof(TModel)); - return result.IsModelSet && context.ModelState.IsValid; - } - - public async Task TryUpdateModelAsync(PageContext context, TModel value, string name) - { - var result = await BindAsync(context, value, name, typeof(TModel)); - return result.IsModelSet && context.ModelState.IsValid; - } - - protected abstract Task BindAsync(PageContext context, object value, string name, Type type); - } -} diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/RazorPageAdapter.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/RazorPageAdapter.cs index 52c319f22a..116a67f679 100644 --- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/RazorPageAdapter.cs +++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/RazorPageAdapter.cs @@ -19,17 +19,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure private readonly RazorPageBase _page; private readonly Type _modelType; - [Obsolete("This constructor is obsolete and will be removed in a future version.")] - public RazorPageAdapter(RazorPageBase page) - { - if (page == null) - { - throw new ArgumentNullException(nameof(page)); - } - - _page = page; - } - public RazorPageAdapter(RazorPageBase page, Type modelType) { _page = page ?? throw new ArgumentNullException(nameof(page)); diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/ApplicationParts/RazorCompiledItemFeatureProviderTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/ApplicationParts/RazorCompiledItemFeatureProviderTest.cs index de3b31d0b3..6559c77b1b 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/ApplicationParts/RazorCompiledItemFeatureProviderTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/ApplicationParts/RazorCompiledItemFeatureProviderTest.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationParts } [Fact] - public void PopulateFeature_PopulatesRazorViewAttributeFromTypeAssembly() + public void PopulateFeature_PopulatesRazorCompiledItemsFromTypeAssembly() { // Arrange var item1 = Mock.Of(i => i.Identifier == "Item1" && i.Type == typeof(TestView)); diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/Compilation/ViewsFeatureProviderTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/Compilation/ViewsFeatureProviderTest.cs deleted file mode 100644 index 4b76f0c8d9..0000000000 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Razor.Test/Compilation/ViewsFeatureProviderTest.cs +++ /dev/null @@ -1,197 +0,0 @@ -// 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; -using System.Collections.Generic; -using System.Linq; -using System.Reflection; -using System.Reflection.Emit; -using Microsoft.AspNetCore.Mvc.ApplicationParts; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Razor.Compilation -{ -#pragma warning disable CS0618 // Type or member is obsolete - public class ViewsFeatureProviderTest - { - [Fact] - public void PopulateFeature_ReturnsEmptySequenceIfNoAssemblyPartHasViewAssembly() - { - // Arrange - var applicationPartManager = new ApplicationPartManager(); - applicationPartManager.ApplicationParts.Add( - new AssemblyPart(typeof(ViewsFeatureProviderTest).GetTypeInfo().Assembly)); - applicationPartManager.FeatureProviders.Add(new ViewsFeatureProvider()); - var feature = new ViewsFeature(); - - // Act - applicationPartManager.PopulateFeature(feature); - - // Assert - Assert.Empty(feature.ViewDescriptors); - } - - [Fact] - public void PopulateFeature_ReturnsViewsFromAllAvailableApplicationParts() - { - // Arrange - var part1 = new AssemblyPart(typeof(object).GetTypeInfo().Assembly); - var part2 = new AssemblyPart(GetType().GetTypeInfo().Assembly); - var featureProvider = new TestableViewsFeatureProvider(new Dictionary> - { - { - part1, - new[] - { - new RazorViewAttribute("/Views/test/Index.cshtml", typeof(object)), - } - }, - { - part2, - new[] - { - new RazorViewAttribute("/Areas/Admin/Views/Index.cshtml", typeof(string)), - new RazorViewAttribute("/Areas/Admin/Views/About.cshtml", typeof(int)), - } - }, - }); - - var applicationPartManager = new ApplicationPartManager(); - applicationPartManager.ApplicationParts.Add(part1); - applicationPartManager.ApplicationParts.Add(part2); - applicationPartManager.FeatureProviders.Add(featureProvider); - var feature = new ViewsFeature(); - - // Act - applicationPartManager.PopulateFeature(feature); - - // Assert - Assert.Collection(feature.ViewDescriptors.OrderBy(f => f.RelativePath, StringComparer.Ordinal), - view => - { - Assert.Equal("/Areas/Admin/Views/About.cshtml", view.RelativePath); - Assert.Equal(typeof(int), view.ViewAttribute.ViewType); - }, - view => - { - Assert.Equal("/Areas/Admin/Views/Index.cshtml", view.RelativePath); - Assert.Equal(typeof(string), view.ViewAttribute.ViewType); - }, - view => - { - Assert.Equal("/Views/test/Index.cshtml", view.RelativePath); - Assert.Equal(typeof(object), view.ViewAttribute.ViewType); - }); - } - - [Fact] - public void PopulateFeature_ThrowsIfSingleAssemblyContainsMultipleAttributesWithTheSamePath() - { - // Arrange - var path1 = "/Views/test/Index.cshtml"; - var path2 = "/views/test/index.cshtml"; - var expected = string.Join( - Environment.NewLine, - "The following precompiled view paths differ only in case, which is not supported:", - path1, - path2); - var part = new AssemblyPart(typeof(object).GetTypeInfo().Assembly); - var featureProvider = new TestableViewsFeatureProvider(new Dictionary> - { - { - part, - new[] - { - new RazorViewAttribute(path1, typeof(object)), - new RazorViewAttribute(path2, typeof(object)), - } - }, - }); - - var applicationPartManager = new ApplicationPartManager(); - applicationPartManager.ApplicationParts.Add(part); - applicationPartManager.FeatureProviders.Add(featureProvider); - var feature = new ViewsFeature(); - - // Act & Assert - var ex = Assert.Throws(() => applicationPartManager.PopulateFeature(feature)); - Assert.Equal(expected, ex.Message); - } - - [Fact] - public void PopulateFeature_ReturnsEmptySequenceIfNoDynamicAssemblyPartHasViewAssembly() - { - // Arrange - var name = new AssemblyName($"DynamicAssembly-{Guid.NewGuid()}"); - var assembly = AssemblyBuilder.DefineDynamicAssembly(name, - AssemblyBuilderAccess.RunAndCollect); - - var applicationPartManager = new ApplicationPartManager(); - applicationPartManager.ApplicationParts.Add(new AssemblyPart(assembly)); - applicationPartManager.FeatureProviders.Add(new ViewsFeatureProvider()); - var feature = new ViewsFeature(); - - // Act - applicationPartManager.PopulateFeature(feature); - - // Assert - Assert.Empty(feature.ViewDescriptors); - } - - [Fact] - public void PopulateFeature_DoesNotFail_IfAssemblyHasEmptyLocation() - { - // Arrange - var assembly = new AssemblyWithEmptyLocation(); - var applicationPartManager = new ApplicationPartManager(); - applicationPartManager.ApplicationParts.Add(new AssemblyPart(assembly)); - applicationPartManager.FeatureProviders.Add(new ViewsFeatureProvider()); - var feature = new ViewsFeature(); - - // Act - applicationPartManager.PopulateFeature(feature); - - // Assert - Assert.Empty(feature.ViewDescriptors); - } - - private class TestableViewsFeatureProvider : ViewsFeatureProvider - { - private readonly Dictionary> _attributeLookup; - - public TestableViewsFeatureProvider(Dictionary> attributeLookup) - { - _attributeLookup = attributeLookup; - } - - protected override IEnumerable GetViewAttributes(AssemblyPart assemblyPart) - { - return _attributeLookup[assemblyPart]; - } - } - - private class AssemblyWithEmptyLocation : Assembly - { - public override string Location => string.Empty; - - public override string FullName => typeof(ViewsFeatureProviderTest).GetTypeInfo().Assembly.FullName; - - public override IEnumerable DefinedTypes - { - get - { - throw new NotImplementedException(); - } - } - - public override IEnumerable Modules - { - get - { - throw new NotImplementedException(); - } - } - } - } -#pragma warning restore CS0618 // Type or member is obsolete -} diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs index 259db589b9..0c9ed9c640 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs @@ -209,10 +209,7 @@ namespace Microsoft.AspNetCore.Mvc feature => Assert.IsType(feature), feature => Assert.IsType(feature), feature => Assert.IsType(feature), - feature => Assert.IsType(feature), -#pragma warning disable CS0618 // Type or member is obsolete - feature => Assert.IsType(feature)); -#pragma warning restore CS0618 // Type or member is obsolete + feature => Assert.IsType(feature)); } [Fact]