From 9a77c2bc1e1e58cba2941a7c3690938a0edb641b Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 24 Oct 2014 13:04:49 -0700 Subject: [PATCH] Fix for #1448 - ViewComponents must be public top-level classes ViewComponents and Controllers now follow the same rules exactly for what types of classes they can be. Also corrected a bug in a test for controllers. Closed-generic types can be controllers, the test was wrong. --- .../ViewComponentConventions.cs | 1 + .../DefaultControllerModelBuilderTest.cs | 4 +- .../ViewComponentConventionsTest.cs | 105 ++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/ViewComponentConventionsTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs index bb6ca96a67..1fa23c8cfe 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNet.Mvc public static bool IsComponent([NotNull] TypeInfo typeInfo) { if (!typeInfo.IsClass || + !typeInfo.IsPublic || typeInfo.IsAbstract || typeInfo.ContainsGenericParameters) { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs index 23fd344f36..e02d2431c6 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs @@ -118,7 +118,7 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels var isController = builder.IsController(typeInfo); // Assert - Assert.False(isController); + Assert.True(isController); } [Fact] @@ -201,7 +201,7 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels.DefaultControllerModelBuilderTe { } - public class OpenGenericController + public class OpenGenericController : Mvc.Controller { } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/ViewComponentConventionsTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/ViewComponentConventionsTest.cs new file mode 100644 index 0000000000..360e087d70 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/ViewComponentConventionsTest.cs @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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.Reflection; +using Microsoft.AspNet.Mvc.ViewComponentConventionsTestClasses; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class ViewComponentConventionsTest + { + [Theory] + + // Only public top-level classes can be view components. + [InlineData(typeof(PublicClass), true)] + [InlineData(typeof(InternalClass), false)] + [InlineData(typeof(PublicNestedClass), false)] + [InlineData(typeof(PrivateNestedClass), false)] + + // Abstract classes, interfaces, and open generics don't work either. + [InlineData(typeof(AbstractClass), false)] + [InlineData(typeof(IAmAnInterfaceViewComponent), false)] + [InlineData(typeof(GenericViewComponent<>), false)] + [InlineData(typeof(GenericViewComponent), true)] + + // You need the attribute, or a naming convention + [InlineData(typeof(Nada), false)] + + // Naming convention doesn't apply to derived classes that don't follow it. + [InlineData(typeof(NamingConventionViewComponent), true)] + [InlineData(typeof(CaseInsensitiveNamingConventionVIEWCOMPONENT), true)] + [InlineData(typeof(DerivedNamingConvention), false)] + + // The Attribute does apply to derived classes. + [InlineData(typeof(WithAttribute), true)] + [InlineData(typeof(DerivedWithAttribute), true)] + public void IsComponent(Type type, bool expected) + { + // Arrange & Act + var result = ViewComponentConventions.IsComponent(type.GetTypeInfo()); + + // Assert + Assert.Equal(expected, result); + } + + public class PublicNestedClass : ViewComponent + { + } + + private class PrivateNestedClass : ViewComponent + { + } + } +} + +// These types need to be public/non-nested for validity of the test +namespace Microsoft.AspNet.Mvc.ViewComponentConventionsTestClasses +{ + public class PublicClass : ViewComponent + { + } + + internal class InternalClass : ViewComponent + { + } + + public abstract class AbstractClass : ViewComponent + { + } + + public class GenericViewComponent : ViewComponent + { + } + + public interface IAmAnInterfaceViewComponent + { + } + + public class Nada + { + } + + public class NamingConventionViewComponent + { + } + + public class DerivedNamingConvention : NamingConventionViewComponent + { + } + + public class CaseInsensitiveNamingConventionVIEWCOMPONENT + { + } + + + [ViewComponent] + public class WithAttribute + { + } + + public class DerivedWithAttribute : WithAttribute + { + } +} \ No newline at end of file