OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
454 stars 159 forks source link

Is FromODataBody implemented ? I'm getting Below exception #651

Closed sharathgt closed 2 years ago

sharathgt commented 2 years ago

Tried to add [FromODataBody] for delta in patch request, as FromBody was not working, got the below exception.

System.ArgumentNullException: Value cannot be null. (Parameter 'provider')
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetService[T](IServiceProvider provider)
   at Microsoft.AspNetCore.OData.Formatter.ODataBodyModelBinder.ReadODataBodyAsync(ModelBindingContext bindingContext)
   at Microsoft.AspNetCore.OData.Formatter.ODataBodyModelBinder.BindModelAsync(ModelBindingContext bindingContext)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Binders.BinderTypeModelBinder.BindModelAsync(ModelBindingContext bindingContext)
   at Microsoft.AspNetCore.Mvc.ModelBinding.ParameterBinder.BindModelAsync(ActionContext actionContext, IModelBinder modelBinder, IValueProvider valueProvider, ParameterDescriptor parameter, ModelMetadata metadata, Object value, Object container)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerBinderDelegateProvider.<>c__DisplayClass0_0.<<CreateBinderDelegate>g__Bind|0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Authorization.Policy.AuthorizationMiddlewareResultHandler.HandleAsync(RequestDelegate next, HttpContext context, AuthorizationPolicy policy, PolicyAuthorizationResult authorizeResult)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
julealgon commented 2 years ago

What was happening when you were trying to use [FromBody]? Is your endpoint being detected as a proper OData endpoint (can you share the output of the $odata endpoint)?

xuzhg commented 2 years ago

@sharathgt [FromODataBody] is designed to 'flatten' action parameters from ODataActionParameters. It should work in 'basic' scenarios. Would you please share with me a repro to help me dig more?

sharathgt commented 2 years ago

@julealgon , I'm not getting 'delta' when I use [FromBody]. changedParameters are empty. And the endpoint, I'm not sure, afraid it is not. Please tell me a way to check. I have mixed/duplicate endpoints with different versions. I just started thinking of enabling Odata now.

@xuzhg , It is a simple to start approach, nothing much here. I wanted to enable OData for my existing APIs,

my Program.cs is


using SDUI.Context;
using SDUI.Entities;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.OData;
using Microsoft.AspNetCore.OData.Batch;
using Microsoft.EntityFrameworkCore;
using Microsoft.IdentityModel.Tokens;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;
using System.Text;
using System.Text.Json.Serialization;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
          .AddJwtBearer(options =>
          {
              options.TokenValidationParameters = new TokenValidationParameters
              {
                  ValidateIssuer = true,
                  ValidateAudience = true,
                  ValidateLifetime = true,
                  ValidateIssuerSigningKey = true,
                  ValidIssuer = builder.Configuration["Jwt:Issuer"],
                  ValidAudience = builder.Configuration["Jwt:Issuer"],
                  IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(builder.Configuration["Jwt:Key"]))
              };
          });

builder.Services.AddDbContext<sdui_dbContext>(
    options => options.UseSqlServer(builder.Configuration.GetConnectionString("sdui_dbContext")));
// .UseLazyLoadingProxies());

// Add services to the container.
IEdmModel model1 = GetEdmModel1();
//IEdmModel model2 = GetEdmModel2();

builder.Services
    .AddCors()
    .AddLogging()
    .AddControllers()
    .AddJsonOptions(opt => opt.JsonSerializerOptions.ReferenceHandler = ReferenceHandler.IgnoreCycles)
    .AddOData(options =>
    {
        options.Select().Filter().OrderBy().Count().SetMaxTop(5000).Expand();

        var defaultBatchHandler = new DefaultODataBatchHandler();
        defaultBatchHandler.MessageQuotas.MaxNestingDepth = 2;
        options.AddRouteComponents("api/v2", model1, defaultBatchHandler);

        /*options.AddRouteComponents("api/v3", model2);*/
    });

// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle
builder.Services.AddEndpointsApiExplorer();

builder.Services.AddSwaggerGen(opt =>
{

    opt.ResolveConflictingActions(apiDescriptions => apiDescriptions.First());
    opt.SwaggerDoc("v1", new Microsoft.OpenApi.Models.OpenApiInfo
    {
        Title = "SDUI API",
        Version = "v1"
    });
    opt.AddSecurityDefinition("Bearer", new Microsoft.OpenApi.Models.OpenApiSecurityScheme
    {
        In = Microsoft.OpenApi.Models.ParameterLocation.Header,
        Description = "Please enter token",
        Name = "Authorization",
        Type = Microsoft.OpenApi.Models.SecuritySchemeType.Http,
        BearerFormat = "JWT",
        Scheme = "bearer"
    });
    opt.AddSecurityRequirement(new Microsoft.OpenApi.Models.OpenApiSecurityRequirement
                {
                    {
                        new Microsoft.OpenApi.Models.OpenApiSecurityScheme
                        {
                            Reference = new Microsoft.OpenApi.Models.OpenApiReference
                            {
                                Type=Microsoft.OpenApi.Models.ReferenceType.SecurityScheme,
                                Id="Bearer"
                            }
                        },
                        new string[]{}
                    }
                });
});

IEdmModel GetEdmModel1()
{
    ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
    builder.EntitySet<Agency>("Agencies");

    return builder.GetEdmModel();
}

var app = builder.Build();

if (app.Environment.IsDevelopment())
{
    app.UseSwagger();
    app.UseSwaggerUI();
}

app.UseRouting();

app.UseAuthentication();

app.UseAuthorization();

app.UseSwagger();

app.UseCors(
    options => options.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader()
);

app.UseSwaggerUI(c =>
{
    c.SwaggerEndpoint("/swagger/v1/swagger.json", "SDUI API v1");
    c.RoutePrefix = string.Empty;
    c.DocumentTitle = "SDUI API V1.0";
});

app.UseEndpoints(endpoints =>
{
    endpoints.MapControllers(); // (GetEdmModel1());

});

app.Run();

and the Controller

namespace SDUI.Controllers
{
    [Route("api/[controller]")]
    [Authorize]

    public class AgenciesController : ODataController
    {
        private readonly sdui_dbContext _context;

        public AgenciesController (sdui_dbContext context)
        {
            _context = context;
        }

    [HttpPatch("{id}")]
        public async Task<IActionResult> Patch(long id,[FromODataBody] Delta<Agency> patch)
        {
            if (!ModelState.IsValid)
            {
                return BadRequest(ModelState);
            }

            var agency = await _context.Agencies
                .FindAsync(id);

            if (agency == null)
            {
                return NotFound();
            }

            patch.Patch(agency);
            try
            {
                await _context.SaveChangesAsync();
            }
            catch (DbUpdateConcurrencyException)
            {
                bool rec_Exists = await _context.Agencies.AnyAsync(a => a.AgencyId == id);
                if (!rec_Exists)
                {
                    return NotFound();
                }
                else
                {
                    throw;
                }
            }

            return Updated(agency);
        }
    }
}
sharathgt commented 2 years ago

I'm sorry to bother you people. I think It's not right to discuss such things here, Closing this off as not planned.

julealgon commented 2 years ago

@sharathgt , my first suggestion when dealing with OData is to enable the route debugging endpoint. You can do that as described in the sample here:

    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
        // Send "~/$odata" to debug routing if enable the following middleware
        // app.UseODataRouteDebug();

This endpoint will quickly tell whether or not your endpoints are being properly detected by OData.

Now, for your specific example, I can see at least 2 issues there.

  1. When you enable OData in startup, you use the "api/v2" prefix, but your controller/method do not include that. You have to either explicitly add the version to the routes, or you need to try using an API Versioning package. This is a good library for adding proper versioning to OData:

  2. On your controller's Route attribute, you are using the [controller] replacement. This currently doesn't work properly with OData v8, so you have to specify your controller name there explicitly.

Try making the following changes and see if it works for you: Full explicit route prefix:

    [Route("api/v2/agencies")]
    [Authorize]

    public class AgenciesController : ODataController

Use FromBody for Delta:

    [HttpPatch("{id}")]
        public async Task<IActionResult> Patch(long id, [FromBody] Delta<Agency> patch)

Also, IIRC, Delta is already preconfigured to automatically bind from the body, so you can also try just removing [FromBody] there altogether.

sharathgt commented 2 years ago

It was a great help @julealgon , True, [FromBody] is also not needed. As you rightly guessed, It was routing issue. It's solved. Thank you for your time.

One last thing ; Still I see two Patch requests in my swagger. one without {Id}, But I have only one with [HttpPatch("{id}")] attribute in my code. any idea?

image

julealgon commented 2 years ago

@sharathgt I suspect what you are seeing is a result of another problem that I documented some time ago:

Can you test a couple options?

  1. Rename your "Patch" method in the controller to something else, so it stops matching the OData convention (and only your explicit route is respected)
  2. Rename your id parameter to key to match the convention. I suspect this will actually cause an exception for swagger since it will start detecting 2 routes with the same template

The cleanest solution that I've found for this problem is to actually disable the OData conventions altogether by removing them from the list of conventions in startup. Unfortunately there is no elegant mechanism to do this.

You can see how the existing conventions are configured here: https://github.com/OData/AspNetCoreOData/blob/69eec03c7003fe12d92cdc619efdc16781683694/src/Microsoft.AspNetCore.OData/ODataOptionsSetup.cs#L38-L57

Basically, you only need to keep the MetadataRoutingConvention and AttributeRoutingConvention to have everything work with explicit routes as far as I know.

This ODataOptions is the very same model you are configuring when calling the AddOData on services, so you can manipulate the list there.

Let me know if that is of any help.

sharathgt commented 2 years ago

I made those changes as per conventions, the explicit attribute and method naming did the trick. Now it is only one Patch in the list. Thanks again for the great help. I am trying the versioning and the route convention setup. Thanks again.

sharathgt commented 2 years ago

Hi @julealgon, sorry to bother you again...

Everything is fine, but few and only few methods are not considered as OData endpoints. Especially ones which has attribute routing.

[HttpGet("AgencyStatus/{month}/{year}")]
[HttpGet("AgencyStatus/{month}/{year}/{key}")]
public async Task<ActionResult> FetchAgencyStatus(int month, int year, long key)
{
    ....
    return Ok();
}

Below is the warning

warn: Microsoft.AspNetCore.OData.Routing.Conventions.AttributeRoutingConvention[0]
      The path template 'api/v2/AttendanceFiles/AgencyStatus/{month}/{year}' on the action 'FetchAgencyStatus' in controller 'AttendanceFiles' is not a valid OData path template. Bad Request - Error in query syntax.

For the ones with single routes also. Please help me. I don't find it anywhere to solve this. It will be a lot helpful !!

julealgon commented 2 years ago

@sharathgt as far as I'm aware, you can't use arbitrary data as route segments in OData. OData imposes some restrictions in that you can only navigate on entities and relationships defined in the EDM.

My suggestion is for you to look into using a custom bound function to model that route. With bound functions you can pass arbitrary data (with a slightly different syntax than what you are using though).

If you are having challenges with creating the bound function, please open a new question topic under 'Discussions' here, as that could be useful for other people as well.

sharathgt commented 2 years ago

@julealgon, Thank you for your time. I will bind them and try that out, update you later.

sharathgt commented 2 years ago

@julealgon Created a discussion as I couldn't pass the parameters... https://github.com/OData/AspNetCoreOData/discussions/670

I tried a lot doing many things googling around but nothing worked out.