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

Limit [FromServices] to apply only to parameters #3568

Closed
wants to merge 1 commit into from
Closed

Limit [FromServices] to apply only to parameters #3568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions samples/MvcSample.Web/HomeController.cs
View file
Open in desktop
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
Open in desktop
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
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why kill the example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Property injection doesn't work any more. The other example talks about parameter injection and I've left that alone.

/// <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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test isn't valid any more. This should be updated to use [FromBody] or something similar. If [FromBody] is already covered then this is good enough.

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
Open in desktop
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
Open in desktop
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
Open in desktop
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 };
}
}
}
9 changes: 0 additions & 9 deletions test/WebSites/ApiExplorerWebSite/Models/IOrderRepository.cs
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions test/WebSites/ApiExplorerWebSite/Models/OrderDTO.cs
View file
Open in desktop
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