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

Commit

Permalink
Limit [FromServices] to apply only to parameters
Browse files Browse the repository at this point in the history 
Fixes #3507
  • Loading branch information
@pranavkm
pranavkm committed Nov 16, 2015
1 parent ccfd235 commit 742431e
Show file tree
Hide file tree
Showing 25 changed files with 101 additions and 243 deletions.
9 changes: 4 additions & 5 deletions samples/MvcSample.Web/HomeController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,12 @@ public ActionResult SaveUser(User user)
return View("MyView", user);
}

[FromServices]
public IHostingEnvironment HostingEnvironment { get; set; }

/// <summary>
/// Action that shows multiple file upload.
/// </summary>
public async Task<ActionResult> PostFile(IList<IFormFile> files)
public async Task<ActionResult> PostFile(
[FromServices] IHostingEnvironment hostingEnvironment,
IList<IFormFile> files)
{
if (!ModelState.IsValid)
{
Expand All @@ -108,7 +107,7 @@ public async Task<ActionResult> PostFile(IList<IFormFile> files)

foreach (var f in files)
{
await f.SaveAsAsync(Path.Combine(HostingEnvironment.WebRootPath, "test-file" + files.IndexOf(f)));
await f.SaveAsAsync(Path.Combine(hostingEnvironment.WebRootPath, "test-file" + files.IndexOf(f)));
}
return View();
}
Expand Down
22 changes: 3 additions & 19 deletions src/Microsoft.AspNet.Mvc.Core/FromServicesAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,9 @@
namespace Microsoft.AspNet.Mvc
{
/// <summary>
/// Specifies that a parameter or property should be bound using the request services.
/// Specifies that an action parameter should be bound using the request services.
/// </summary>
/// <example>
/// In this example, the LocationService property on the VehicleWithDealerViewModel class
/// will be bound to the value resolved for the ILocationService service from the request services.
///
/// <code>
/// public class VehicleWithDealerViewModel
/// {
/// [FromServices]
/// public ILocationService LocationService { get; set; }
///
/// public void Update()
/// {
/// LocationService.Update();
/// }
/// }
/// </code>
///
/// In this example an implementation of IProductModelRequestService is registered as a service.
/// Then in the GetProduct action, the parameter is bound to an instance of IProductModelRequestService
/// which is resolved from the the request services.
Expand All @@ -38,10 +22,10 @@ namespace Microsoft.AspNet.Mvc
/// }
/// </code>
/// </example>
[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)]
public class FromServicesAttribute : Attribute, IBindingSourceMetadata
{
/// <inheritdoc />
public BindingSource BindingSource { get { return BindingSource.Services; } }
public BindingSource BindingSource => BindingSource.Services;
}
}
60 changes: 54 additions & 6 deletions src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.Mvc.ModelBinding.Validation;
using Microsoft.AspNet.Mvc.WebApiCompatShim;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Net.Http.Headers;
using Newtonsoft.Json;

Expand All @@ -22,6 +23,9 @@ namespace System.Web.Http
public abstract class ApiController : IDisposable
{
private HttpRequestMessage _request;
private IModelMetadataProvider _metadataProvider;
private IObjectModelValidator _objectValidator;
private IUrlHelper _urlHelper;

/// <summary>
/// Gets the action context.
Expand Down Expand Up @@ -52,12 +56,42 @@ public HttpContext Context
/// Gets the <see cref="IModelMetadataProvider"/>.
/// </summary>
/// <remarks>The setter is intended for unit testing purposes only.</remarks>
[FromServices]
public IModelMetadataProvider MetadataProvider { get; set; }
public IModelMetadataProvider MetadataProvider
{
get
{
if (_metadataProvider == null)
{
_metadataProvider = Context?.RequestServices?.GetRequiredService<IModelMetadataProvider>();
}

return _metadataProvider;
}
set
{
_metadataProvider = value;
}
}

/// <summary>
/// Gets or sets the <see cref="IObjectModelValidator"/>.
/// </summary>
public IObjectModelValidator ObjectValidator
{
get
{
if (_objectValidator == null)
{
_objectValidator = Context?.RequestServices?.GetRequiredService<IObjectModelValidator>();
}

[FromServices]
public IObjectModelValidator ObjectValidator { get; set; }
return _objectValidator;
}
set
{
_objectValidator = value;
}
}

/// <summary>
/// Gets model state after the model binding process. This ModelState will be empty before model binding
Expand Down Expand Up @@ -96,8 +130,22 @@ public HttpRequestMessage Request
/// Gets a factory used to generate URLs to other APIs.
/// </summary>
/// <remarks>The setter is intended for unit testing purposes only.</remarks>
[FromServices]
public IUrlHelper Url { get; set; }
public IUrlHelper Url
{
get
{
if (_urlHelper == null)
{
_urlHelper = Context?.RequestServices?.GetRequiredService<IUrlHelper>();
}

return _urlHelper;
}
set
{
_urlHelper = value;
}
}

/// <summary>
/// Gets or sets the current principal associated with this request.
Expand Down
3 changes: 0 additions & 3 deletions test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Metadata/ModelAttributesTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ private class BaseModel

[Required]
public virtual int VirtualProperty { get; set; }

[FromServices]
public ICalculator Calculator { get; set; }
}

private class DerivedModel : BaseModel
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.AspNet.Mvc.FunctionalTests/RequestServicesTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ public RequestServicesTest(MvcTestFixture<RequestServicesWebSite.Startup> fixtur
[InlineData("http://localhost/Other/FromFilter")]
[InlineData("http://localhost/Other/FromView")]
[InlineData("http://localhost/Other/FromViewComponent")]
[InlineData("http://localhost/Other/FromModelProperty")]
[InlineData("http://localhost/Other/FromActionArgument")]
public async Task RequestServices(string url)
{
Expand All @@ -40,6 +39,7 @@ public async Task RequestServices(string url)
var response = await Client.SendAsync(request);

// Assert
response.EnsureSuccessStatusCode();
var body = (await response.Content.ReadAsStringAsync()).Trim();
Assert.Equal(requestId, body);
}
Expand Down
9 changes: 0 additions & 9 deletions test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,6 @@ private class Order2
private class Person2
{
public string Name { get; set; }

[FromServices]
public IActionBindingContextAccessor BindingContext { get; set; }
}

[Fact]
Expand Down Expand Up @@ -301,7 +298,6 @@ public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBind
var model = Assert.IsType<Order2>(modelBindingResult.Model);
Assert.NotNull(model.Customer);
Assert.Equal("bill", model.Customer.Name);
Assert.NotNull(model.Customer.BindingContext);

Assert.Equal(1, modelState.Count);
Assert.Equal(0, modelState.ErrorCount);
Expand Down Expand Up @@ -340,7 +336,6 @@ public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBind
var model = Assert.IsType<Order2>(modelBindingResult.Model);
Assert.NotNull(model.Customer);
Assert.Equal("bill", model.Customer.Name);
Assert.NotNull(model.Customer.BindingContext);

Assert.Equal(1, modelState.Count);
Assert.Equal(0, modelState.ErrorCount);
Expand Down Expand Up @@ -1489,9 +1484,6 @@ private class Person9
{
[FromBody]
public Address1 Address { get; set; }

[FromServices]
public IActionBindingContextAccessor BindingContext { get; set; }
}

// If a nested POCO object has all properties bound from a greedy source, then it should be populated
Expand Down Expand Up @@ -1524,7 +1516,6 @@ public async Task MutableObjectModelBinder_BindsNestedPOCO_WithAllGreedyBoundPro

var model = Assert.IsType<Order9>(modelBindingResult.Model);
Assert.NotNull(model.Customer);
Assert.NotNull(model.Customer.BindingContext);
Assert.NotNull(model.Customer.Address);
Assert.Equal(AddressStreetContent, model.Customer.Address.Street);

Expand Down
84 changes: 0 additions & 84 deletions test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,90 +13,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
{
public class ServicesModelBinderIntegrationTest
{
private class Person
{
public Address Address { get; set; }
}

private class Address
{
// Using a service type already in defaults.
[FromServices]
public JsonOutputFormatter OutputFormatter { get; set; }
}

[Fact]
public async Task BindPropertyFromService_WithData_WithPrefix_GetsBound()
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
{
Name = "Parameter1",
BindingInfo = new BindingInfo()
{
BinderModelName = "CustomParameter",
},

ParameterType = typeof(Person)
};

var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
var modelState = new ModelStateDictionary();

// Act
var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext);

// Assert

// ModelBindingResult
Assert.True(modelBindingResult.IsModelSet);

// Model
var boundPerson = Assert.IsType<Person>(modelBindingResult.Model);
Assert.NotNull(boundPerson);
Assert.NotNull(boundPerson.Address.OutputFormatter);

// ModelState
Assert.True(modelState.IsValid);
Assert.Empty(modelState.Keys);
}

[Fact]
public async Task BindPropertyFromService_WithData_WithEmptyPrefix_GetsBound()
{
// Arrange
var argumentBinder = ModelBindingTestHelper.GetArgumentBinder();
var parameter = new ParameterDescriptor()
{
Name = "Parameter1",
BindingInfo = new BindingInfo(),
ParameterType = typeof(Person)
};

var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
var modelState = new ModelStateDictionary();

// Act
var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext);

// Assert

// ModelBindingResult
Assert.True(modelBindingResult.IsModelSet);

// Model
var boundPerson = Assert.IsType<Person>(modelBindingResult.Model);
Assert.NotNull(boundPerson);
Assert.NotNull(boundPerson.Address.OutputFormatter);

// ModelState
Assert.True(modelState.IsValid);

// For non user bound models there should be no entry in model state.
Assert.Empty(modelState);
}

[Fact]
public async Task BindParameterFromService_WithData_GetsBound()
{
Expand Down
11 changes: 7 additions & 4 deletions test/WebSites/ActionResultsWebSite/Controllers/ActionResultsVerificationController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ namespace ActionResultsWebSite
{
public class ActionResultsVerificationController : Controller
{
public ActionResultsVerificationController(GuidLookupService guidLookupService)
{
GuidLookupService = guidLookupService;
}

[FromServices]
public GuidLookupService Service { get; set; }
public GuidLookupService GuidLookupService { get; }

public IActionResult Index([FromBody]DummyClass test)
{
Expand Down Expand Up @@ -107,7 +110,7 @@ public IActionResult GetNotFoundObjectResultWithDisposableObject(string guid)
public bool GetDisposeCalled(string guid)
{
bool value;
if (Service.IsDisposed.TryGetValue(guid, out value))
if (GuidLookupService.IsDisposed.TryGetValue(guid, out value))
{
return value;
}
Expand All @@ -131,7 +134,7 @@ private DummyClass CreateDummy()

private DisposableType CreateDisposableType(string guid)
{
return new DisposableType(Service, guid);
return new DisposableType(GuidLookupService, guid);
}

private class DisposableType : IDisposable
Expand Down
7 changes: 2 additions & 5 deletions test/WebSites/ActivatorWebSite/Controllers/PlainController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,19 @@ namespace ActivatorWebSite
{
public class PlainController
{
[FromServices]
public MyService Service { get; set; }

[ActionContext]
public ActionContext ActionContext { get; set; }

public HttpRequest Request => ActionContext.HttpContext.Request;

public HttpResponse Response => ActionContext.HttpContext.Response;

public IActionResult Index()
public IActionResult Index([FromServices] MyService service)
{
Response.Headers["X-Fake-Header"] = "Fake-Value";

var value = Request.Query["foo"];
return new ContentResult { Content = Service.Random + "|" + value };
return new ContentResult { Content = service.Random + "|" + value };
}
}
}
3 changes: 0 additions & 3 deletions test/WebSites/ApiExplorerWebSite/Models/OrderDTO.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ namespace ApiExplorerWebSite
{
public class OrderDTO
{
[FromServices]
public IOrderRepository Repository { get; set; }

public string CustomerId { get; set; }

[FromHeader(Name = "Referrer")]
Expand Down
Loading

0 comments on commit 742431e

Please sign in to comment.