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

Commit

Permalink
Fix #2527 - Remove FormCollection use
Browse files Browse the repository at this point in the history 
This removes the dependency on Microsoft.AspNet.Http from the Mvc.Core
code. Added the reference back to tests where needed (DefaultHttpContext).
  • Loading branch information
@rynowak
rynowak committed Aug 24, 2015
1 parent a045324 commit 1596cd9
Show file tree
Hide file tree
Showing 10 changed files with 172 additions and 96 deletions.
1 change: 0 additions & 1 deletion src/Microsoft.AspNet.Mvc.Core/AuthorizationFilterAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
100 changes: 86 additions & 14 deletions src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -19,8 +19,7 @@ public class FormCollectionModelBinder : IModelBinder
/// <inheritdoc />
public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingContext bindingContext)
{
if (bindingContext.ModelType != typeof(IFormCollection) &&
bindingContext.ModelType != typeof(FormCollection))
if (bindingContext.ModelType != typeof(IFormCollection))
{
return null;
}
Expand All @@ -30,19 +29,11 @@ public async Task<ModelBindingResult> 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<string, string[]>());
model = new EmptyFormCollection();
}

var validationNode =
Expand All @@ -57,5 +48,86 @@ public async Task<ModelBindingResult> 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<string> Keys
{
get
{
return new List<string>();
}
}

public bool ContainsKey(string key)
{
return false;
}

public string Get(string key)
{
return null;
}

public IEnumerator<KeyValuePair<string, string[]>> GetEnumerator()
{
return Enumerable.Empty<KeyValuePair<string, string[]>>().GetEnumerator();
}

public IList<string> GetValues(string key)
{
return null;
}

IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}

private class EmptyFormFileCollection : List<IFormFile>, IFormFileCollection
{
public IFormFile this[string name]
{
get
{
return null;
}
}

public IFormFile GetFile(string name)
{
return null;
}

IReadOnlyList<IFormFile> IFormFileCollection.GetFiles(string name)
{
throw new NotImplementedException();
}
}
}
}
1 change: 0 additions & 1 deletion src/Microsoft.AspNet.Mvc.Core/project.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down
37 changes: 22 additions & 15 deletions src/Microsoft.AspNet.Mvc.WebApiCompatShim/project.json
View file
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
33 changes: 15 additions & 18 deletions test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<string, string[]>
{
{ "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<string, string[]>
{
{ "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
Expand All @@ -100,9 +99,7 @@ public async Task FormCollectionModelBinder_CustomFormCollection_BindSuccessful(
// Assert
Assert.NotNull(result);
var form = Assert.IsAssignableFrom<IFormCollection>(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)
Expand Down
12 changes: 6 additions & 6 deletions test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private class Address
{
public int Zip { get; set; }

public FormCollection FileCollection { get; set; }
public IFormCollection FileCollection { get; set; }
}

[Fact]
Expand Down Expand Up @@ -61,7 +61,7 @@ public async Task BindProperty_WithData_WithEmptyPrefix_GetsBound()
// Model
var boundPerson = Assert.IsType<Person>(modelBindingResult.Model);
Assert.NotNull(boundPerson.Address);
var formCollection = Assert.IsAssignableFrom<FormCollection>(boundPerson.Address.FileCollection);
var formCollection = Assert.IsAssignableFrom<IFormCollection>(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());
Expand All @@ -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.";
Expand All @@ -109,7 +109,7 @@ public async Task BindParameter_WithData_GetsBound()
Assert.True(modelBindingResult.IsModelSet);

// Model
var formCollection = Assert.IsType<FormCollection>(modelBindingResult.Model);
var formCollection = Assert.IsAssignableFrom<IFormCollection>(modelBindingResult.Model);
var file = Assert.Single(formCollection.Files);
Assert.NotNull(file);
Assert.Equal("form-data; name=CustomParameter; filename=text.txt", file.ContentDisposition);
Expand All @@ -133,7 +133,7 @@ public async Task BindParameter_NoData_BindsWithEmptyCollection()
{
BinderModelName = "CustomParameter",
},
ParameterType = typeof(FormCollection)
ParameterType = typeof(IFormCollection)
};

// No data is passed.
Expand All @@ -148,7 +148,7 @@ public async Task BindParameter_NoData_BindsWithEmptyCollection()

// ModelBindingResult
Assert.NotNull(modelBindingResult);
var collection = Assert.IsType<FormCollection>(modelBindingResult.Model);
var collection = Assert.IsAssignableFrom<IFormCollection>(modelBindingResult.Model);

// ModelState
Assert.True(modelState.IsValid);
Expand Down
37 changes: 19 additions & 18 deletions test/Microsoft.AspNet.Mvc.Razor.Test/project.json
View file
Original file line number Diff line number Diff line change
@@ -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"
},
Expand Down
25 changes: 13 additions & 12 deletions test/Microsoft.AspNet.Mvc.TagHelpers.Test/project.json
View file
Original file line number Diff line number Diff line change
@@ -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"
},
Expand Down
Loading

0 comments on commit 1596cd9

Please sign in to comment.