From 1596cd9422ff05955fc1b26680682c2ab6805a39 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 9 Jul 2015 12:42:08 -0700 Subject: [PATCH] Fix #2527 - Remove FormCollection use This removes the dependency on Microsoft.AspNet.Http from the Mvc.Core code. Added the reference back to tests where needed (DefaultHttpContext). --- .../AuthorizationFilterAttribute.cs | 1 - .../ModelBinding/FormCollectionModelBinder.cs | 100 +++++++++++++++--- src/Microsoft.AspNet.Mvc.Core/project.json | 1 - .../project.json | 37 ++++--- .../FormCollectionModelBinderTest.cs | 33 +++--- ...rmCollectionModelBindingIntegrationTest.cs | 12 +-- .../project.json | 37 +++---- .../project.json | 25 ++--- .../project.json | 19 ++-- .../Controllers/FormCollectionController.cs | 3 +- 10 files changed, 172 insertions(+), 96 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/AuthorizationFilterAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/AuthorizationFilterAttribute.cs index 7e437cfe87..7eb7168423 100644 --- a/src/Microsoft.AspNet.Mvc.Core/AuthorizationFilterAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/AuthorizationFilterAttribute.cs @@ -4,7 +4,6 @@ using System; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNet.WebUtilities; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs index de3cbf4061..274a081df3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs @@ -1,12 +1,12 @@ // 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; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Http.Internal; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding @@ -19,8 +19,7 @@ public class FormCollectionModelBinder : IModelBinder /// public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { - if (bindingContext.ModelType != typeof(IFormCollection) && - bindingContext.ModelType != typeof(FormCollection)) + if (bindingContext.ModelType != typeof(IFormCollection)) { return null; } @@ -30,19 +29,11 @@ public async Task BindModelAsync([NotNull] ModelBindingConte if (request.HasFormContentType) { var form = await request.ReadFormAsync(); - if (bindingContext.ModelType.GetTypeInfo().IsAssignableFrom(form.GetType().GetTypeInfo())) - { - model = form; - } - else - { - var formValuesLookup = form.ToDictionary(p => p.Key, p => p.Value); - model = new FormCollection(formValuesLookup, form.Files); - } + model = form; } else { - model = new FormCollection(new Dictionary()); + model = new EmptyFormCollection(); } var validationNode = @@ -57,5 +48,86 @@ public async Task BindModelAsync([NotNull] ModelBindingConte isModelSet: true, validationNode: validationNode); } + + private class EmptyFormCollection : IFormCollection + { + public string this[string key] + { + get + { + return null; + } + } + + public int Count + { + get + { + return 0; + } + } + + public IFormFileCollection Files + { + get + { + return new EmptyFormFileCollection(); + } + } + + public ICollection Keys + { + get + { + return new List(); + } + } + + public bool ContainsKey(string key) + { + return false; + } + + public string Get(string key) + { + return null; + } + + public IEnumerator> GetEnumerator() + { + return Enumerable.Empty>().GetEnumerator(); + } + + public IList GetValues(string key) + { + return null; + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + } + + private class EmptyFormFileCollection : List, IFormFileCollection + { + public IFormFile this[string name] + { + get + { + return null; + } + } + + public IFormFile GetFile(string name) + { + return null; + } + + IReadOnlyList IFormFileCollection.GetFiles(string name) + { + throw new NotImplementedException(); + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/project.json b/src/Microsoft.AspNet.Mvc.Core/project.json index 0751834532..553822de15 100644 --- a/src/Microsoft.AspNet.Mvc.Core/project.json +++ b/src/Microsoft.AspNet.Mvc.Core/project.json @@ -12,7 +12,6 @@ "Microsoft.AspNet.Authorization": "1.0.0-*", "Microsoft.AspNet.FileProviders.Abstractions": "1.0.0-*", "Microsoft.AspNet.Hosting.Abstractions": "1.0.0-*", - "Microsoft.AspNet.Http": "1.0.0-*", "Microsoft.AspNet.Mvc.Abstractions": "6.0.0-*", "Microsoft.Dnx.Runtime.Abstractions": "1.0.0-*", "Microsoft.Framework.ClosedGenericMatcher.Sources": { "version": "1.0.0-*", "type": "build" }, diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/project.json b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/project.json index fe2a371f28..b3e32d71a9 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/project.json +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/project.json @@ -1,20 +1,27 @@ { - "description": "Provides compatibility in ASP.NET MVC with ASP.NET Web API 2 to simplify migration of existing Web API implementations.", - "version": "6.0.0-*", - "repository": { - "type": "git", - "url": "git://github.com/aspnet/mvc" - }, - "compilationOptions": { - "warningsAsErrors": false - }, - "dependencies": { - "Microsoft.AspNet.Mvc.Core": "6.0.0-*", - "Microsoft.AspNet.Mvc.Formatters.Json": "6.0.0-*", - "Microsoft.AspNet.WebApi.Client": "5.2.2", - "Microsoft.Framework.PropertyHelper.Sources": { "version": "1.0.0-*", "type": "build" }, - "Microsoft.Framework.NotNullAttribute.Sources": { "version": "1.0.0-*", "type": "build" } + "description": "Provides compatibility in ASP.NET MVC with ASP.NET Web API 2 to simplify migration of existing Web API implementations.", + "version": "6.0.0-*", + "repository": { + "type": "git", + "url": "git://github.com/aspnet/mvc" + }, + "compilationOptions": { + "warningsAsErrors": false + }, + "dependencies": { + "Microsoft.AspNet.WebUtilities": "1.0.0-*", + "Microsoft.AspNet.Mvc.Core": "6.0.0-*", + "Microsoft.AspNet.Mvc.Formatters.Json": "6.0.0-*", + "Microsoft.AspNet.WebApi.Client": "5.2.2", + "Microsoft.Framework.PropertyHelper.Sources": { + "version": "1.0.0-*", + "type": "build" }, + "Microsoft.Framework.NotNullAttribute.Sources": { + "version": "1.0.0-*", + "type": "build" + } + }, "frameworks": { "dnx451": { "frameworkAssemblies": { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs index 9e6216a9e1..b7ec8e7a30 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs @@ -26,7 +26,7 @@ public async Task FormCollectionModelBinder_ValidType_BindSuccessful() { "field2", new string[] { "value2" } } }); var httpContext = GetMockHttpContext(formCollection); - var bindingContext = GetBindingContext(typeof(FormCollection), httpContext); + var bindingContext = GetBindingContext(typeof(IFormCollection), httpContext); var binder = new FormCollectionModelBinder(); // Act @@ -64,34 +64,33 @@ public async Task FormCollectionModelBinder_InvalidType_BindFails() Assert.Null(result); } + // We only support IFormCollection here. Using the concrete type won't work. [Fact] - public async Task FormCollectionModelBinder_NoForm_BindSuccessful_ReturnsEmptyFormCollection() + public async Task FormCollectionModelBinder_FormCollectionConcreteType_BindFails() { // Arrange - var httpContext = GetMockHttpContext(null, hasForm: false); - var bindingContext = GetBindingContext(typeof(IFormCollection), httpContext); + var formCollection = new FormCollection(new Dictionary + { + { "field1", new string[] { "value1" } }, + { "field2", new string[] { "value2" } } + }); + var httpContext = GetMockHttpContext(formCollection); + var bindingContext = GetBindingContext(typeof(FormCollection), httpContext); var binder = new FormCollectionModelBinder(); // Act var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.NotNull(result); - Assert.IsType(typeof(FormCollection), result.Model); - Assert.Empty((FormCollection)result.Model); + Assert.Null(result); } [Fact] - public async Task FormCollectionModelBinder_CustomFormCollection_BindSuccessful() + public async Task FormCollectionModelBinder_NoForm_BindSuccessful_ReturnsEmptyFormCollection() { // Arrange - var formCollection = new MyFormCollection(new Dictionary - { - { "field1", new string[] { "value1" } }, - { "field2", new string[] { "value2" } } - }); - var httpContext = GetMockHttpContext(formCollection); - var bindingContext = GetBindingContext(typeof(FormCollection), httpContext); + var httpContext = GetMockHttpContext(null, hasForm: false); + var bindingContext = GetBindingContext(typeof(IFormCollection), httpContext); var binder = new FormCollectionModelBinder(); // Act @@ -100,9 +99,7 @@ public async Task FormCollectionModelBinder_CustomFormCollection_BindSuccessful( // Assert Assert.NotNull(result); var form = Assert.IsAssignableFrom(result.Model); - Assert.Equal(2, form.Count); - Assert.Equal("value1", form["field1"]); - Assert.Equal("value2", form["field2"]); + Assert.Empty(form); } private static HttpContext GetMockHttpContext(IFormCollection formCollection, bool hasForm = true) diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs index b6d10783d1..77dc3956d2 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs @@ -24,7 +24,7 @@ private class Address { public int Zip { get; set; } - public FormCollection FileCollection { get; set; } + public IFormCollection FileCollection { get; set; } } [Fact] @@ -61,7 +61,7 @@ public async Task BindProperty_WithData_WithEmptyPrefix_GetsBound() // Model var boundPerson = Assert.IsType(modelBindingResult.Model); Assert.NotNull(boundPerson.Address); - var formCollection = Assert.IsAssignableFrom(boundPerson.Address.FileCollection); + var formCollection = Assert.IsAssignableFrom(boundPerson.Address.FileCollection); var file = Assert.Single(formCollection.Files); Assert.Equal("form-data; name=Address.File; filename=text.txt", file.ContentDisposition); var reader = new StreamReader(file.OpenReadStream()); @@ -88,7 +88,7 @@ public async Task BindParameter_WithData_GetsBound() // Setting a custom parameter prevents it from falling back to an empty prefix. BinderModelName = "CustomParameter", }, - ParameterType = typeof(FormCollection) + ParameterType = typeof(IFormCollection) }; var data = "Some Data Is Better Than No Data."; @@ -109,7 +109,7 @@ public async Task BindParameter_WithData_GetsBound() Assert.True(modelBindingResult.IsModelSet); // Model - var formCollection = Assert.IsType(modelBindingResult.Model); + var formCollection = Assert.IsAssignableFrom(modelBindingResult.Model); var file = Assert.Single(formCollection.Files); Assert.NotNull(file); Assert.Equal("form-data; name=CustomParameter; filename=text.txt", file.ContentDisposition); @@ -133,7 +133,7 @@ public async Task BindParameter_NoData_BindsWithEmptyCollection() { BinderModelName = "CustomParameter", }, - ParameterType = typeof(FormCollection) + ParameterType = typeof(IFormCollection) }; // No data is passed. @@ -148,7 +148,7 @@ public async Task BindParameter_NoData_BindsWithEmptyCollection() // ModelBindingResult Assert.NotNull(modelBindingResult); - var collection = Assert.IsType(modelBindingResult.Model); + var collection = Assert.IsAssignableFrom(modelBindingResult.Model); // ModelState Assert.True(modelState.IsValid); diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/project.json b/test/Microsoft.AspNet.Mvc.Razor.Test/project.json index 58a3deb17a..10d0951d6f 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/project.json +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/project.json @@ -1,23 +1,24 @@ { - "compile": [ - "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileProvider.cs", - "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileInfo.cs", - "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileTrigger.cs" - ], - "dependencies": { - "Microsoft.AspNet.Mvc.DataAnnotations": "6.0.0-*", - "Microsoft.AspNet.Mvc.Formatters.Xml": "6.0.0-*", - "Microsoft.AspNet.Mvc.Razor": "6.0.0-*", - "Microsoft.AspNet.Mvc.TestCommon": { - "version": "6.0.0-*", - "type": "build" - }, - "Microsoft.AspNet.Testing": "1.0.0-*", - "Microsoft.Framework.DependencyInjection": "1.0.0-*", - "Microsoft.Framework.WebEncoders.Testing": "1.0.0-*", - "Microsoft.Dnx.Runtime": "1.0.0-*", - "xunit.runner.aspnet": "2.0.0-aspnet-*" + "compile": [ + "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileProvider.cs", + "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileInfo.cs", + "../Microsoft.AspNet.Mvc.Razor.Host.Test/TestFileTrigger.cs" + ], + "dependencies": { + "Microsoft.AspNet.Http": "1.0.0-*", + "Microsoft.AspNet.Mvc.DataAnnotations": "6.0.0-*", + "Microsoft.AspNet.Mvc.Formatters.Xml": "6.0.0-*", + "Microsoft.AspNet.Mvc.Razor": "6.0.0-*", + "Microsoft.AspNet.Mvc.TestCommon": { + "version": "6.0.0-*", + "type": "build" }, + "Microsoft.AspNet.Testing": "1.0.0-*", + "Microsoft.Framework.DependencyInjection": "1.0.0-*", + "Microsoft.Framework.WebEncoders.Testing": "1.0.0-*", + "Microsoft.Dnx.Runtime": "1.0.0-*", + "xunit.runner.aspnet": "2.0.0-aspnet-*" + }, "commands": { "test": "xunit.runner.aspnet" }, diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/project.json b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/project.json index 5538777630..809508acec 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/project.json +++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/project.json @@ -1,17 +1,18 @@ { - "dependencies": { - "Microsoft.AspNet.Mvc.DataAnnotations": "6.0.0-*", - "Microsoft.AspNet.Mvc.Formatters.Xml": "6.0.0-*", - "Microsoft.AspNet.Mvc.TagHelpers": "6.0.0-*", - "Microsoft.AspNet.Mvc.TestCommon": { - "version": "6.0.0-*", - "type": "build" - }, - "Microsoft.AspNet.Testing": "1.0.0-*", - "Microsoft.Framework.Logging.Abstractions": "1.0.0-*", - "Microsoft.Framework.WebEncoders.Testing": "1.0.0-*", - "xunit.runner.aspnet": "2.0.0-aspnet-*" + "dependencies": { + "Microsoft.AspNet.Http": "1.0.0-*", + "Microsoft.AspNet.Mvc.DataAnnotations": "6.0.0-*", + "Microsoft.AspNet.Mvc.Formatters.Xml": "6.0.0-*", + "Microsoft.AspNet.Mvc.TagHelpers": "6.0.0-*", + "Microsoft.AspNet.Mvc.TestCommon": { + "version": "6.0.0-*", + "type": "build" }, + "Microsoft.AspNet.Testing": "1.0.0-*", + "Microsoft.Framework.Logging.Abstractions": "1.0.0-*", + "Microsoft.Framework.WebEncoders.Testing": "1.0.0-*", + "xunit.runner.aspnet": "2.0.0-aspnet-*" + }, "commands": { "test": "xunit.runner.aspnet" }, diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/project.json b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/project.json index 70da31a14b..f6f93d5a49 100644 --- a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/project.json +++ b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/project.json @@ -1,13 +1,14 @@ { - "compilationOptions": { - "warningsAsErrors": "false" - }, - "dependencies": { - "Microsoft.AspNet.Mvc.WebApiCompatShim": "6.0.0-*", - "Microsoft.AspNet.Testing": "1.0.0-*", - "Microsoft.Framework.Logging.Testing": "1.0.0-*", - "xunit.runner.aspnet": "2.0.0-aspnet-*" - }, + "compilationOptions": { + "warningsAsErrors": "false" + }, + "dependencies": { + "Microsoft.AspNet.Http": "1.0.0-*", + "Microsoft.AspNet.Mvc.WebApiCompatShim": "6.0.0-*", + "Microsoft.AspNet.Testing": "1.0.0-*", + "Microsoft.Framework.Logging.Testing": "1.0.0-*", + "xunit.runner.aspnet": "2.0.0-aspnet-*" + }, "commands": { "test": "xunit.runner.aspnet" }, diff --git a/test/WebSites/ModelBindingWebSite/Controllers/FormCollectionController.cs b/test/WebSites/ModelBindingWebSite/Controllers/FormCollectionController.cs index 01304ccda9..e9fcf1854c 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/FormCollectionController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/FormCollectionController.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.IO; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc; namespace ModelBindingWebSite.Controllers @@ -26,7 +25,7 @@ public int ReturnCollectionCount(IFormCollection form) return form.Count; } - public ActionResult ReturnFileContent(FormCollection form) + public ActionResult ReturnFileContent(IFormCollection form) { var file = form.Files.GetFile("File"); using (var reader = new StreamReader(file.OpenReadStream()))