From 19fbcdf5a8345f14dfdf12868c6d05e66e5f3efe Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 7 Oct 2014 18:23:14 -0700 Subject: [PATCH] Fix for #1271 - Add copy constructors for ApplicationModel types. --- .../ApplicationModel/ActionModel.cs | 27 ++++- .../ApplicationModel/AttributeRouteModel.cs | 8 ++ .../ApplicationModel/ControllerModel.cs | 24 ++++- .../ApplicationModel/ParameterModel.cs | 9 ++ .../ApplicationModel/ActionModelTest.cs | 97 +++++++++++++++++ .../AttributeRouteModelTests.cs | 43 ++++++++ .../ApplicationModel/ControllerModelTest.cs | 102 ++++++++++++++++++ .../ApplicationModel/ParameterModelTest.cs | 64 +++++++++++ 8 files changed, 371 insertions(+), 3 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ParameterModelTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ActionModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ActionModel.cs index 2b6e629f32..3f1c4062bd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ActionModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ActionModel.cs @@ -4,8 +4,6 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; -using Microsoft.AspNet.Mvc.Description; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc.ApplicationModel { @@ -22,6 +20,31 @@ namespace Microsoft.AspNet.Mvc.ApplicationModel Parameters = new List(); } + public ActionModel([NotNull] ActionModel other) + { + ActionMethod = other.ActionMethod; + ActionName = other.ActionName; + ApiExplorerGroupName = other.ApiExplorerGroupName; + ApiExplorerIsVisible = other.ApiExplorerIsVisible; + IsActionNameMatchRequired = other.IsActionNameMatchRequired; + + // Not making a deep copy of the controller, this action still belongs to the same controller. + Controller = other.Controller; + + // These are just metadata, safe to create new collections + ActionConstraints = new List(other.ActionConstraints); + Attributes = new List(other.Attributes); + Filters = new List(other.Filters); + HttpMethods = new List(other.HttpMethods); + + // Make a deep copy of other 'model' types. + Parameters = new List(other.Parameters.Select(p => new ParameterModel(p))); + + if (other.AttributeRouteModel != null) + { + AttributeRouteModel = new AttributeRouteModel(other.AttributeRouteModel); + } + } public List ActionConstraints { get; private set; } public MethodInfo ActionMethod { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/AttributeRouteModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/AttributeRouteModel.cs index 85dcb78c51..7b941d0ef5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/AttributeRouteModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/AttributeRouteModel.cs @@ -25,6 +25,14 @@ namespace Microsoft.AspNet.Mvc.ApplicationModel Name = templateProvider.Name; } + public AttributeRouteModel([NotNull] AttributeRouteModel other) + { + Attribute = other.Attribute; + Name = other.Name; + Order = other.Order; + Template = other.Template; + } + public IRouteTemplateProvider Attribute { get; private set; } public string Template { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ControllerModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ControllerModel.cs index 72fab6008f..2c3d45b7cd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ControllerModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ControllerModel.cs @@ -1,8 +1,8 @@ // 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.Collections.Generic; +using System.Linq; using System.Reflection; namespace Microsoft.AspNet.Mvc.ApplicationModel @@ -21,6 +21,28 @@ namespace Microsoft.AspNet.Mvc.ApplicationModel RouteConstraints = new List(); } + public ControllerModel([NotNull] ControllerModel other) + { + ApiExplorerGroupName = other.ApiExplorerGroupName; + ApiExplorerIsVisible = other.ApiExplorerIsVisible; + ControllerName = other.ControllerName; + ControllerType = other.ControllerType; + + // Still part of the same application + Application = other.Application; + + // These are just metadata, safe to create new collections + ActionConstraints = new List(other.ActionConstraints); + Attributes = new List(other.Attributes); + Filters = new List(other.Filters); + RouteConstraints = new List(other.RouteConstraints); + + // Make a deep copy of other 'model' types. + Actions = new List(other.Actions.Select(a => new ActionModel(a))); + AttributeRoutes = new List( + other.AttributeRoutes.Select(a => new AttributeRouteModel(a))); + } + public List ActionConstraints { get; private set; } public List Actions { get; private set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ParameterModel.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ParameterModel.cs index a7a5344f03..0e807c176d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ParameterModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModel/ParameterModel.cs @@ -15,6 +15,15 @@ namespace Microsoft.AspNet.Mvc.ApplicationModel Attributes = new List(); } + public ParameterModel([NotNull] ParameterModel other) + { + Action = other.Action; + Attributes = new List(other.Attributes); + IsOptional = other.IsOptional; + ParameterInfo = other.ParameterInfo; + ParameterName = other.ParameterName; + } + public ActionModel Action { get; set; } public List Attributes { get; private set; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs new file mode 100644 index 0000000000..6ee8171405 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs @@ -0,0 +1,97 @@ +// 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.Collections.Generic; +using System.Reflection; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ApplicationModel +{ + public class ActionModelTest + { + [Fact] + public void CopyConstructor_DoesDeepCopyOfOtherModels() + { + // Arrange + var action = new ActionModel(typeof(TestController).GetMethod("Edit")); + + var parameter = new ParameterModel(action.ActionMethod.GetParameters()[0]); + parameter.Action = action; + action.Parameters.Add(parameter); + + var route = new AttributeRouteModel(new HttpGetAttribute("api/Products")); + action.AttributeRouteModel = route; + + // Act + var action2 = new ActionModel(action); + + // Assert + Assert.NotSame(action, action2.Parameters[0]); + Assert.NotSame(route, action2.AttributeRouteModel); + } + + [Fact] + public void CopyConstructor_CopiesAllProperties() + { + // Arrange + var action = new ActionModel(typeof(TestController).GetMethod("Edit")); + + action.ActionConstraints.Add(new HttpMethodConstraint(new string[] { "GET" })); + action.ActionName = "Edit"; + action.ApiExplorerGroupName = "group"; + action.ApiExplorerIsVisible = true; + action.Attributes.Add(new HttpGetAttribute()); + action.Controller = new ControllerModel(typeof(TestController).GetTypeInfo()); + action.Filters.Add(new AuthorizeAttribute()); + action.HttpMethods.Add("GET"); + action.IsActionNameMatchRequired = true; + + // Act + var action2 = new ActionModel(action); + + // Assert + foreach (var property in typeof(ActionModel).GetProperties()) + { + if (property.Name.Equals("Parameters") || property.Name.Equals("AttributeRouteModel")) + { + // This test excludes other ApplicationModel objects on purpose because we deep copy them. + continue; + } + + var value1 = property.GetValue(action); + var value2 = property.GetValue(action2); + + if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)) + { + Assert.Equal((IEnumerable)value1, (IEnumerable)value2); + + // Ensure non-default value + Assert.NotEmpty((IEnumerable)value1); + } + else if (property.PropertyType.IsValueType || + Nullable.GetUnderlyingType(property.PropertyType) != null) + { + Assert.Equal(value1, value2); + + // Ensure non-default value + Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); + } + else + { + Assert.Same(value1, value2); + + // Ensure non-default value + Assert.NotNull(value1); + } + } + } + + private class TestController + { + public void Edit(int id) + { + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs index 043cb20aac..b6647d7775 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs @@ -10,6 +10,49 @@ namespace Microsoft.AspNet.Mvc.ApplicationModel { public class AttributeRouteModelTests { + [Fact] + public void CopyConstructor_CopiesAllProperties() + { + // Arrange + var route = new AttributeRouteModel(new HttpGetAttribute("/api/Products")); + + route.Name = "products"; + route.Order = 5; + + // Act + var route2 = new AttributeRouteModel(route); + + // Assert + foreach (var property in typeof(AttributeRouteModel).GetProperties()) + { + var value1 = property.GetValue(route); + var value2 = property.GetValue(route2); + + if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)) + { + Assert.Equal((IEnumerable)value1, (IEnumerable)value2); + + // Ensure non-default value + Assert.NotEmpty((IEnumerable)value1); + } + else if (property.PropertyType.IsValueType || + Nullable.GetUnderlyingType(property.PropertyType) != null) + { + Assert.Equal(value1, value2); + + // Ensure non-default value + Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); + } + else + { + Assert.Same(value1, value2); + + // Ensure non-default value + Assert.NotNull(value1); + } + } + } + [Theory] [InlineData(null, null, null)] [InlineData("", null, "")] diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs new file mode 100644 index 0000000000..0f94d27794 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs @@ -0,0 +1,102 @@ +// 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.Collections.Generic; +using System.Reflection; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ApplicationModel +{ + public class ControllerModelTest + { + [Fact] + public void CopyConstructor_DoesDeepCopyOfOtherModels() + { + // Arrange + var controller = new ControllerModel(typeof(TestController).GetTypeInfo()); + + var action = new ActionModel(typeof(TestController).GetMethod("Edit")); + controller.Actions.Add(action); + action.Controller = controller; + + var route = new AttributeRouteModel(new HttpGetAttribute("api/Products")); + controller.AttributeRoutes.Add(route); + + // Act + var controller2 = new ControllerModel(controller); + + // Assert + Assert.NotSame(action, controller2.Actions[0]); + Assert.NotSame(route, controller2.AttributeRoutes[0]); + + Assert.NotSame(controller.ActionConstraints, controller2.ActionConstraints); + Assert.NotSame(controller.Actions, controller2.Actions); + Assert.NotSame(controller.Attributes, controller2.Attributes); + Assert.NotSame(controller.Filters, controller2.Filters); + Assert.NotSame(controller.RouteConstraints, controller2.RouteConstraints); + } + + [Fact] + public void CopyConstructor_CopiesAllProperties() + { + // Arrange + var controller = new ControllerModel(typeof(TestController).GetTypeInfo()); + + controller.ActionConstraints.Add(new HttpMethodConstraint(new string[] { "GET" })); + controller.ApiExplorerGroupName = "group"; + controller.ApiExplorerIsVisible = true; + controller.Application = new GlobalModel(); + controller.Attributes.Add(new HttpGetAttribute()); + controller.ControllerName = "cool"; + controller.Filters.Add(new AuthorizeAttribute()); + controller.RouteConstraints.Add(new AreaAttribute("Admin")); + + // Act + var controller2 = new ControllerModel(controller); + + // Assert + foreach (var property in typeof(ControllerModel).GetProperties()) + { + if (property.Name.Equals("Actions") || property.Name.Equals("AttributeRoutes")) + { + // This test excludes other ApplicationModel objects on purpose because we deep copy them. + continue; + } + + var value1 = property.GetValue(controller); + var value2 = property.GetValue(controller2); + + if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)) + { + Assert.Equal((IEnumerable)value1, (IEnumerable)value2); + + // Ensure non-default value + Assert.NotEmpty((IEnumerable)value1); + } + else if (property.PropertyType.IsValueType || + Nullable.GetUnderlyingType(property.PropertyType) != null) + { + Assert.Equal(value1, value2); + + // Ensure non-default value + Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); + } + else + { + Assert.Same(value1, value2); + + // Ensure non-default value + Assert.NotNull(value1); + } + } + } + + private class TestController + { + public void Edit() + { + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ParameterModelTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ParameterModelTest.cs new file mode 100644 index 0000000000..8540182f88 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/ParameterModelTest.cs @@ -0,0 +1,64 @@ +// 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.Collections.Generic; +using Xunit; + +namespace Microsoft.AspNet.Mvc.ApplicationModel +{ + public class ParameterModelTest + { + [Fact] + public void CopyConstructor_CopiesAllProperties() + { + // Arrange + var parameter = new ParameterModel(typeof(TestController).GetMethod("Edit").GetParameters()[0]); + + parameter.Action = new ActionModel(typeof(TestController).GetMethod("Edit")); + parameter.Attributes.Add(new FromBodyAttribute()); + parameter.IsOptional = true; + parameter.ParameterName = "id"; + + // Act + var parameter2 = new ParameterModel(parameter); + + // Assert + foreach (var property in typeof(ParameterModel).GetProperties()) + { + var value1 = property.GetValue(parameter); + var value2 = property.GetValue(parameter2); + + if (typeof(IEnumerable).IsAssignableFrom(property.PropertyType)) + { + Assert.Equal((IEnumerable)value1, (IEnumerable)value2); + + // Ensure non-default value + Assert.NotEmpty((IEnumerable)value1); + } + else if (property.PropertyType.IsValueType || + Nullable.GetUnderlyingType(property.PropertyType) != null) + { + Assert.Equal(value1, value2); + + // Ensure non-default value + Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); + } + else + { + Assert.Same(value1, value2); + + // Ensure non-default value + Assert.NotNull(value1); + } + } + } + + private class TestController + { + public void Edit(int id) + { + } + } + } +} \ No newline at end of file