dotnet / dotnet/aspnetcore

Support multiple `[From...]` attributes on the same member: convert to `CompositeBindingSource`

Open
#62,809 4 comments 0 reactions 0 assignees View on GitHub
area-mvc
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Is your feature request related to a problem? Please describe the problem.

I created a custom binding source that represents the current request's host:

```csharp
public static class ExtendedBindingSources
{
///
/// A for the request host.
///
public static readonly BindingSource Host = new(
"Host",
"A binding source that fetches data from the request's host",
isGreedy: false,
isFromRequest: true);
}
```

I then created my own `[FromHost]` attribute, inheriting from `IBindingSourceMetadata`, to indicate that a member should fetch from that source. After, I created an associated `HostValueProvider`, inheriting from `BindingSourceValueProvider`, that is responsible for actually fetching the host value. Because the host doesn't vary per input, and is a single value, this provider always provides the same value no matter the name or prefix of the inputs:

```csharp
public class HostValueProvider(HttpRequest httpRequest) : BindingSourceValueProvider(ExtendedBindingSources.Host)
{
public override bool ContainsPrefix(string prefix) => true;

public override ValueProviderResult GetValue(string key) => new(httpRequest.Host.Host);
}
```

However, in my specific use case, I only want to fetch the value from the host _if the value is not available from the route_. Since I registered my `HostValueProvider` + `HostValueProviderFactory` in `MvcOptions` as the last factory, it would be called after all other value providers were exhausted. This allows me to naturally "fallback" to the host value if other sources did not provide a value for the member/parameter.

This works perfectly fine mechanically and I'm quite happy with how clean it ended up, but there is a limitation I ran into with regards to how the `[From...]` attributes work.

At first, I tried something like this:

```csharp
[FromRoute][FromHost] myParameter
```

My hope was that with multiple `[From...]` attributes, the framework would consider all of them when going through the associated value sources.

But I quickly realized that when multiple such parameters are specified, _only the first one is honored_. In the example above, _even when the route did not have a value for `myParameter`_, it would not try to fetch the value from my custom host provider.

If I inverted the attribute order:
```csharp
[FromHost][FromRoute] myParameter
```

It would then _only fetch the value from my custom host provider_, and the value on the route would be ignored, even if present.

To work around this, I created a somewhat convoluted custom attribute to combine 2 sources into a single `CompositeBindingSource`:

```csharp
[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class FromMultipleAttribute() : FromMultipleAttribute(new TSource1(), new TSource2())
where TSource1 : IBindingSourceMetadata, new()
where TSource2 : IBindingSourceMetadata, new();

[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
public class FromMultipleAttribute(params IBindingSourceMetadata[] bindingSources) : Attribute, IBindingSourceMetadata
{
///
public BindingSource BindingSource { get; } = CompositeBindingSource.Create(
bindingSources.Select(s => s.BindingSource),
string.Join(',', bindingSources.Select(s => s.BindingSource.DisplayName)));
}
```

Which then allowed me to do this:
```csharp
[FromMultiple] myParameter
```

Which works perfectly: both the host and the route are now considered as sources for the value, in the order I would expect (first the route is checked, then the host, based on the order that the value provider factories are registered, which is exactly how it was designed to work).

I would love for this to work more naturally instead though. It should be fairly straightforward for the framework to detect and combine multiple `IBindingSourceMetadata` attributes into a single `CompositeBindingSource` automatically, which would remove the need for this somewhat complicated (and limiting) combined attribute, and allow me to just do what I initially tried:

```csharp
[FromRoute][FromHost] myParameter
```

### Describe the solution you'd like

AspNetCore MVC should see multiple `[From...]` attributes on a member, and respect all of them by combining their binding sources.

I would like to just specify the attributes like this:
```csharp
[FromRoute][FromQuery] myParameter
```

And this should mean that _either_ the route _or_ the queryString could provide values for my argument.

The behavior today is that the first such attribute completely overrides the mechanism and makes all other subsequent attributes irrelevant, which I think is very unintuitive behavior since adding multiple attributes also doesn't produce any runtime errors.

### Additional context

I'm not sure if the logic would need to take special care about `BindingSource` values with the `isGreedy` flag turned on. This is currently outside of my use case as both sources in my scenario are not greedy.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.