billbogaiv / hybrid-model-binding

Provides the ability to bind models using both ModelBinder and ValueProvider in ASP.NET Core.
MIT License
104 stars 19 forks source link

Combine with ApiController attribute #20

Closed artyom-p closed 5 years ago

artyom-p commented 5 years ago

Hi, I'm using .NET core 2.2 and hybrid binding works fine without ApiControllerAttribute. I'm trying to bind a model which consists of values from route and body, and when attribute is applied, only body binding works. Is it possible to combine them together?

billbogaiv commented 5 years ago

Is it possible to combine them together?

Yes. This is the main feature of the library. Depending on your particular use-case, the body-binding process might be overriding bound-data from the route. If you post a sample of your controller-action and model, I might be able to provide better insight into what's happening.

artyom-p commented 5 years ago

So, i was testing it on a newly created .net core 2.2 webapi project.

Here's the Startup's ConfigureServices method

     public void ConfigureServices(IServiceCollection services)
     {
         services.AddMvc()
              .SetCompatibilityVersion(CompatibilityVersion.Version_2_2)
             .AddHybridModelBinder();
     }

and here's a controller and a model

    [Route("api/[controller]")]
    [ApiController]
    public class ValuesController : ControllerBase
    {
        // GET api/values
        [HttpGet("{Id}")]
        public ActionResult<bool> Get(Test model)
        {
            return true;
        }
    }

    public class Test
    {
        [From(Source.Route)]
        public int Id { get; set; }

        [From(Source.Body)]
        public string Value { get; set; }
    }

It would be great if it worked together with attribute, because attribute gives a few useful behaviors for controller.

billbogaiv commented 5 years ago

Quick fix is to add the [FromHybrid] attribute to the controller-action parameter:

public ActionResult<bool> Get([FromHybrid]Test model)
{
    return true;
}

Here's what I discovered which explains why your version isn't working:

I'm going to update the behavior of the convention to also look at the action-parameter's attributes and apply the hybrid binding source if no other binding-related attributes exist. This will cover cases like the current API behavior along with not overriding a user's intent to use some other non-hybrid binding for a particular parameter.

artyom-p commented 5 years ago

Wow, great!

billbogaiv commented 5 years ago

When https://www.nuget.org/packages/HybridModelBinding/0.11.0 becomes available, you should be able to remove the [FromHybrid]-attribute.

New behavior looks at the parameter's attributes and applies the hybrid binder only if no other IBindingSourceMetadata-derived (i.e. [FromBody]) attribute is applied.

artyom-p commented 5 years ago

Works great! Thank you!