Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question - is it possible to bind both FromQuery and Body object using this library #42

Closed
simmohall opened this issue Dec 11, 2020 · 14 comments

Comments

@simmohall
Copy link

It is possible to do this as I can't workout how based on the TRequest, TResponse definition. The below will obviously not compile.

Thanks

public class MyEndpoint : BaseEndpoint<PayloadBody, string>
...
[HttpPost("resource/{externalTicketId}")]
[Produces("application/json")]
[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
public override ActionResult Handle([FromQuery] string externalTicketId, PayloadBody payload)
{
....

@kjbetz
Copy link

kjbetz commented Dec 12, 2020

I have no idea if the below comment will be helpful or applicable to this library. I just started looking at it now.

But with MediatR I did this and it would work. On the query/command/request itself you put the attributes on the properties themselves where it's coming from. At least back then, [FromBody] had to map to an object, so you couldn't just put a bunch of [FromBody] attributes on different properties. It looked something like this:

public class ResourceRequest : TRequest<TResponse>
{
    [FromQuery]
    public int ExternalTicketId { get; set; }

    [FromBody]
    public ResourceRequestBody Body { get; set; }

    public class ResourceRequestBody
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }
}

Good luck, Again I have no idea if this works in this library.

I'll add though, looking back... I'd probably just remove the stuff from the query/route in these cases and just put those parameters into the body/payload if you can. Because I was using AutoMapper, and some other tools to make things "easier" I had to deal with these two levels of stuff... when I could have, in this case, just put that ExternalTicketId into the payload body.

@matt-lethargic
Copy link

I was about to ask the same question, or at least similar. There is a use case when I want to add a child to a parent with the request:

Url:
POST http://localhost/parent/{id}/children

Body:

{
"firstName": "Steve",
"lastName": "Smith"
}

In a "normal" controller we'd define the endpoint:

public async Task<ActionResult> AddChild([FromRoute] Guid id, [FromBody] ChildModel child)

We can't do this with Endpoints so I tried adding a wrapper class like above

public class WrappedRequest {
    [FromRoute] public Guid Id {get;set;}
    [FromBody] public ChildModel Child {get;set;}
}

public async Task<ActionResult> AddChild(WrappedRequest request)

and defining the endpoint thus:

public class AddChildEndpoint : BaseAsyncEndpoint
        .WithRequest<WrappedRequest >
        .WithoutResponse

This didn't work and failed to bind the properties.

At a loss as to what to do now, I don't want to stop using Endpoints, but I feel that this is a very common pattern and should be supported.... is it? Am I doing something wrong?

@simmohall
Copy link
Author

yeah I never solved this problem - which is unfortunate as I also really enjoy this library and pattern. I fallback to using the default controllers for now for these cases

@acasorran
Copy link

¿Could be this a possible solution?

public static class WithRequest<TRequest1, TRequest2>
{
    public abstract class WithResponse<TResponse> : BaseEndpointAsync
    {
        public abstract Task<ActionResult<TResponse>> HandleAsync(
            TRequest1 request1,
            TRequest2 request2,
            CancellationToken cancellationToken = default
        );
    }

    public abstract class WithoutResponse : BaseEndpointAsync
    {
        public abstract Task<ActionResult> HandleAsync(
            TRequest1 request1,
            TRequest2 request2,
            CancellationToken cancellationToken = default
        );
    }
}

Even with three

public static class WithRequest<TRequest1, TRequest2, TRequest3>
{
    public abstract class WithResponse<TResponse> : BaseEndpointAsync
    {
        public abstract Task<ActionResult<TResponse>> HandleAsync(
            TRequest1 request1,
            TRequest2 request2,
            TRequest3 request3,
            CancellationToken cancellationToken = default
        );
    }

    public abstract class WithoutResponse : BaseEndpointAsync
    {
        public abstract Task<ActionResult> HandleAsync(
            TRequest1 request1,
            TRequest2 request2,
            TRequest3 request3,
            CancellationToken cancellationToken = default
        );
    }
}

@garywoodfine
Copy link

garywoodfine commented Mar 24, 2021

I had the same issue but after further exploration and fiddling around I realise that the Model Binding actually takes care of this.

All that is needed is to create your Model Class and decorate your properties with the attributes you need. i.e.

 public class NewArticleRequest
    {
        [FromRoute(Name = "username")] public string Username { get; set; }
        [FromRoute(Name ="category")] public string Category { get; set; }

        [FromBody] public Article Article { get; set; }
        
        [FromHeader (Name = "Content-Type")] public string ContentType { get; set; }
    }

Then you can make use of it in your End Point as follows

 [Route("/article")]
    public class Post : BaseAsyncEndpoint
        .WithRequest<NewArticleRequest>
        .WithoutResponse
    {
        [HttpPost("{username}/{category}")]
        [SwaggerOperation(
            Summary = "Submit a new article",
            Description = "Enables the submission of new articles",
            OperationId = "B349A6C4-1198-4B53-B9BE-85232E06F16E",
            Tags = new[] {"Article"})
        ]
        public override Task<ActionResult> HandleAsync([FromRoute] NewArticleRequest request,
            CancellationToken cancellationToken = new CancellationToken())
        {
           //// your implementation
        }
    }

The only snag/issue/bug I seem to run into seems to be related only to the [FromRoute] is that it doesn't seem to bind those specific properties unless you explicity set the in the HandleAsync([FromRoute] NewArticleRequest request but other than that it seems to work

Another key point to remember is when making use of Model Binding on [FromRoute is that case sensitivity matters. So if you if have potentially different casing on route variable than on your Class property you can make use of the Name to provide the case sensitive name. i.e.

     [FromRoute(Name = "username")] public string Username { get; set; }

Also as in the case for the Content-Type header variable is hyphenated but we don't use the in C# naming convention we use as follows

 [FromHeader (Name = "Content-Type")] public string ContentType { get; set; }

@ardalis
Copy link
Owner

ardalis commented Mar 25, 2021

Can others confirm here if @garywoodfine 's approach works? If so I will be sure to get it added to the docs and will avoid adding additional complexity to the underlying package. If it doesn't work, please provide an example of what you're doing that doesn't work. Thanks!

@matt-lethargic
Copy link

I've just spun up an API from scratch. Confirmed that this way does work. The important part is having the [FromRoute] attribute in the method declaration eg public override async Task<ActionResult> HandleAsync([FromRoute] Blah request)

Endpoint

[Route("weather")]
public class WeatherForecastController : BaseAsyncEndpoint
    .WithRequest<CreateForecastRequest>
    .WithoutResponse
{
    [HttpPost("{day}/stuff")]
    public override async Task<ActionResult> HandleAsync([FromRoute] CreateForecastRequest request, CancellationToken cancellationToken = new CancellationToken())
    {
        //do work
    }
}

Models

public class CreateForecastRequest
{
    [FromRoute(Name="day")] public string Day { get; set; }
    [FromBody] public ForecastModel Forecast { get; set; }
}

public class ForecastModel
{
    public string Summary { get; set; }
}

Request

POST http://localhost:5000/weather/5/stuff

{
    "summary": "Bad weather"
}

I have closed PR #50

@Jestar342
Copy link

Jestar342 commented Mar 30, 2021

As an FYI this method confuses Swashbuckle - the generated UI will assume the request body is the parameter type and seemingly is ignorant of the property flag FromBody

So to follow @matt-lethargic's example, whilst the API endpoint will be expecting the request body in the form of a ForecastModel a la:

{
    "summary": "Bad weather"
}

The swagger UI and document will describe it as:

{
  "day": "string",
  "forecast": {
    "summary": "string"
  }
}

There may well be an easy workaround - I only tried this out about 90mins ago and haven't been able to locate a solution in that time. I'm also pretty certain this is something for Swashbuckle to fix but thought others may find this useful to know.

@fw2568
Copy link

fw2568 commented May 27, 2021

There may well be an easy workaround - I only tried this out about 90mins ago and haven't been able to locate a solution in that time. I'm also pretty certain this is something for Swashbuckle to fix but thought others may find this useful to know.

@Jestar342 have you reported this issue to Swashbuckle? I just faced same problem...

@ardalis
Copy link
Owner

ardalis commented Jun 4, 2021

I've documented this in the README.

@ffMathy
Copy link

ffMathy commented Feb 18, 2022

@Jestar342 and @fw2568 did you ever find a solution?

I filed an issue here: domaindrivendev/Swashbuckle.AspNetCore#2346

@jamesfernandes
Copy link

@ffMathy, @Jestar342, @fw2568

Running AspNet Core 6:

[Route("weather")]
public class WeatherForecastController : EndpointBaseAsync.WithRequest<CreateForecastRequest>.WithoutResult
{
  [HttpPost("{day}/stuff")]
  public override async Task<ActionResult> HandleAsync([FromRoute] CreateForecastRequest request, CancellationToken cancellationToken = new CancellationToken())
  {
    await Task.CompletedTask;
    return Ok();
  }
}

public record CreateForecastRequest
{
  [FromRoute] // don't need to specify for capitalization [FromRoute(Name = "day")]
  public string Day { get; set; }

  [FromBody]
  public ForecastModel Forecast { get; set; }
}

public record ForecastModel
{
  public string Summary { get; set; }
}

Renders in Swagger

ardalis-endpoints-weather-swagger

@ardalis
Copy link
Owner

ardalis commented Jun 14, 2022

I recently published this which I'm experimenting with for this purpose as well:
https://www.nuget.org/packages/Ardalis.RouteAndBodyModelBinding

@gius
Copy link

gius commented Oct 20, 2022

I recently published this which I'm experimenting with for this purpose as well: https://www.nuget.org/packages/Ardalis.RouteAndBodyModelBinding

How do you combine Ardalis.RouteAndBodyModelBinding with ApiEndpoints?

And one more question - is it possible to define request parameters' attributes (FromRoute, FromBody) in a fluent way, out of the request class?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests