OData / AspNetCoreOData

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

POST $batch 200 --- but response is Not Found #705

Open siasty opened 1 year ago

siasty commented 1 year ago

ASP.NET Core OData 8.0.11

Hi, i have problem with batch request. Request finished HTTP/1.1 POST http://localhost:5000/odata/$batch multipart/mixed;+boundary=batch_id-1664271614387-17 343 - 200 but the GET inside batch return > 404 Not Found

` ----------------------------------------------------------------------REQUEST----------------------------------------------------------- --batch_id-1664271614387-17 Content-Type:application/http Content-Transfer-Encoding:binary

GET Contracts(nrum='4104264',rokum='2021') HTTP/1.1 Accept:application/json;odata.metadata=minimal;IEEE754Compatible=true Accept-Language:pl Content-Type:application/json;charset=UTF-8;IEEE754Compatible=true

--------------------------------------------------------------------RESPOND---------------------------------------------------------- --batch_id-1664271614387-17--

--batchresponse_a336531a-38f4-4844-ba1e-f94afb0f2e73 Content-Type: application/http Content-Transfer-Encoding: binary

HTTP/1.1 404 Not Found

--batchresponse_a336531a-38f4-4844-ba1e-f94afb0f2e73--

`

I integrated odata with sap ui5...

from begining i have problem with CORS to get rid of the error, I had to change the order in startup.cs cors after Routing and Batch after CORS

app.UseRouting(); app.UseCors("AllowAllOrigins"); app.UseODataQueryRequest(); app.UseODataBatching(); app.UseEndpoints(endpoints => { endpoints.MapControllers(); });

then i received problem with HEAD in bach request so i use my own Middleware ` app.UseMyOdataMiddleware(); app.Use((context, next) => { context.Response.Headers["OData-Version"] = "4.0";
context.Response.Headers["OData-MaxVersion"] = "4.0";

            return next.Invoke();
        });

`

`public class HttpGetOrHeadAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new[] { "GET", "HEAD" };

    public HttpGetOrHeadAttribute() : base(_supportedMethods) { }

    public HttpGetOrHeadAttribute(string template) : base(_supportedMethods, template)
    {
        if (template == null)
        {
            throw new ArgumentNullException(nameof(template));
        }
    }
}

public static class ODataMiddlewareExtensions
{
    public static IApplicationBuilder UseMyOdataMiddleware(this IApplicationBuilder builder)
    {
        return builder.UseMiddleware<ODataMiddleware>();
    }
} 

public class ODataMiddleware
{
    private readonly RequestDelegate _next;

    private readonly ILogger _logger;

    public ODataMiddleware(RequestDelegate next, ILoggerFactory logFactory)
    {
         _next = next ?? throw new ArgumentNullException(nameof(next));
        _logger = logFactory.CreateLogger("ODataMiddleware");
    }

    public async Task Invoke(HttpContext context)
    {
        _logger.LogInformation("ODataMiddlewaree executing..");

        bool methodSwitched = false;

        if (HttpMethods.IsHead(context.Request.Method))
        {
            methodSwitched = true;
           _logger.LogInformation("metoda:" +context.Request.Method);
            context.Request.Method = HttpMethods.Get;
            context.Response.Body = Stream.Null;
        }

        await _next(context);

        if (methodSwitched)
        {
            context.Request.Method = HttpMethods.Head;
        }
    }
}`

and finally when I have no mistakes, receives no answer

does anyone have any idea what could be wrong?

siasty commented 1 year ago

I set break point in constructor in controler odata but I have no stop in the point

working only when i use app.UseODataBatching(); befor app.UseRouting()

but then I have an error CORS

siasty commented 1 year ago

`

public class Startup

{
    IEdmModel GetEdmModel()
    {
        var odataBuilder = new ODataConventionModelBuilder();
        odataBuilder.EntitySet<Contract>("Contracts").EntityType.HasKey(c => new { c.NRUM, c.ROKUM });
        var contractConfiguration = odataBuilder.EntitySet<Contract>("Contracts");

        odataBuilder.EntitySet<Customer>("Customers").EntityType.HasKey(c => new { c.KLIENTID });
        odataBuilder.EntitySet<ContractList>("ContractsShort").EntityType.HasKey(c => new { c.ID_ZUMOWYL });

        odataBuilder.Namespace = "LeasingService";
        odataBuilder.EnableLowerCamelCase();
        EdmModel model = odataBuilder.GetEdmModel() as EdmModel;

        return model;
    }

    public Startup(IConfiguration configuration)
    {
        Configuration = configuration;
    }

    public IConfiguration Configuration { get; }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {

        services.AddDbContext<zumleDbContext>(opt => opt.UseInMemoryDatabase("zumle"));

        services.AddCors(options =>
        {
            options.AddPolicy(
                 "AllowAllOrigins",
                  builder => builder
                    .AllowAnyOrigin()
                    .AllowAnyHeader()
                    .AllowAnyMethod()
                    );
        });

        var batchHandler = new DefaultODataBatchHandler();
        services.AddControllers()
        .AddOData(opt => opt
            .AddRouteComponents("odata", GetEdmModel(),batchHandler)
            .Count().Filter().Expand().Select().OrderBy().SetMaxTop(Convert.ToInt32(Configuration["OdataMaxPageSize"]))
            .EnableQueryFeatures()
        );

        services.AddSwaggerGen(c =>
        {   
            c.SwaggerDoc("v1", new OpenApiInfo { Title = "LeasingService", Version = "v1" });
        });
     }

    // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
    public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {

        if (env.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
            app.UseSwagger();
            app.UseSwaggerUI(c => c.SwaggerEndpoint("/swagger/v1/swagger.json", "LeasingService v1"));
            // app.UseHsts();
            app.UseODataRouteDebug();
        }

          app.UseDirectoryBrowser(new DirectoryBrowserOptions
            {
                FileProvider = new PhysicalFileProvider(Path.Combine(Directory.GetCurrentDirectory(), "wwwroot", "webapp")),
                RequestPath = "/webapp"
            });

        var provider = new FileExtensionContentTypeProvider();
        provider.Mappings[".properties"] = "application/text";
        app.UseStaticFiles(new StaticFileOptions
        {
            FileProvider = new PhysicalFileProvider(
                Path.Combine(Directory.GetCurrentDirectory(), "wwwroot", "webapp")),
            RequestPath = "/webapp",
            ContentTypeProvider = provider
        });
        app.UseDefaultFiles("/webapp/index.html");
        // app.UseHttpsRedirection();

        app.UseForwardedHeaders(new ForwardedHeadersOptions
        {
            ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto
        });

        // app.UseAuthentication();
        // app.UseAuthorization();

     //   app.UseResponseCaching();

        app.UseMyOdataMiddleware();
        app.Use((context, next) =>
        {
             context.Response.Headers["OData-Version"] = "4.0";           
             context.Response.Headers["OData-MaxVersion"] = "4.0";

            return next.Invoke();
        });

        app.UseRouting();

        app.UseCors("AllowAllOrigins");

        app.UseODataQueryRequest();
        app.UseODataBatching();

        app.UseEndpoints(endpoints =>
        {
            endpoints.MapControllers();
        });
      }
}`
julealgon commented 1 year ago

Ok... I kindly suggest that you try to reduce your problem space here. You start talking about the batch endpoint, then about a request inside the batch request, and then you introduce CORS complications.

  1. Does the single, isolated GET Contracts(nrum='4104264',rokum='2021') produce the result you expect, or does it also return 404? Depending on the answer, this has nothing to do with the batching endpoint
  2. If you make the requests through Postman or similar, do you get the behavior you expect? That should eliminate any CORS-related problem

Lastly, your OP has a ton of formatting issues that make it exceedingly hard to read. Would be appreciated if you could fix that. Use

```csharp
/```

For C# code blocks to get syntax highlighting.

siasty commented 1 year ago
  1. Yes, single get is ok
  2. No, result is the same as app.

I found a solution but I don't think it should work that way (in AddCors i allow any origin, method and header)

`

        app.UseMyOdataMiddleware();
        app.Use((context, next) =>
        {
             context.Response.Headers["OData-Version"] = "4.0";           
             context.Response.Headers["OData-MaxVersion"] = "4.0";

             if (context.Request.Path.ToString().Contains("/odata/$batch"))
             {
                context.Response.Headers["Access-Control-Allow-Origin"] = "*";
                context.Response.Headers["Access-Control-Allow-Headers"]= "Origin, X-Requested-With, Content-Type, Accept";
                context.Response.Headers["Access-Control-Allow-Methods"]= "GET, POST PUT, DELETE, OPTIONS";
                context.Response.Headers["Access-Control-Allow-Credentials"]= "true";

            }
            return next.Invoke();
        });

        app.UseODataQueryRequest();
        app.UseODataBatching();
        app.UseRouting();
        app.UseCors("AllowAllOrigins");

        app.UseEndpoints(endpoints =>
        {
            endpoints.MapControllers();
        });

`

xuzhg commented 1 year ago

https://github.com/OData/WebApi/pull/2167

siasty commented 1 year ago

Hi @xuzhg i also have problem with OData-Version in heders I check odata batch manual and "Batch requests SHOULD contain the applicable OData-Version header" image http://docs.oasis-open.org/odata/odata/v4.0/errata02/os/complete/part1-protocol/odata-v4.0-errata02-os-part1-protocol-complete.html#_Toc406398359

can you tell me how can I add to the batch request?

I think it is related to CORS which it regulates the visibility of the response headers

siasty commented 1 year ago

i added to the

app.Use((context, next) => {
context.Response.Headers["Access-Control-Expose-Headers"]= "OData-Version"; 
});

And it works

but when i set ".WithExposedHeaders("OData-Version")" in addCors i have problem

siasty commented 1 year ago

I have prepared a fully working example https://github.com/siasty/Ui5_Odatav4

MichaelUlloa commented 8 months ago

I had the same problem. The $batch endpoint wasn't showing the HTTP header "Access-Control-Allow-Origin: *" as I had specified on my default CORS policy. My initial thought was moving the UseODataBatching() middleware after the UseCors() middleware, and changed the order to "UseRouting > UseCors > UseODataBatching".

This fixed my CORS problem but brought the second problem: every batch request was returning a 200, but on the response, all the requested endpoints appeared as a 404 (as this issue). Why was this happening? app.UseODataBatching() must be called before app.UseRouting(). But since the recommended order as per Microsoft docs was "UseRouting > UseCors", and I needed UseODataBatching to be before UseRouting and after UseCors, I had a cyclic problem (if you could call it that way?)

Since my project is hosted on Azure App Service, my solution was to setup the CORS on Azure App Service instead of setting it up on the back-end, and using the recommended order:

app.UseODataBatching();
app.UseRouting();
app.UseCors();
julealgon commented 8 months ago

Since my project is hosted on Azure App Service, my solution was to setup the CORS on Azure App Service instead of setting it up on the back-end, and using the recommended order:

app.UseODataBatching();
app.UseRouting();
app.UseCors();

If you are configuring CORS at the AppService level, you don't need to have UseCors() in your code, do you @MichaelUlloa ?

siasty commented 8 months ago

@MichaelUlloa can you show how your batch is built? I tested for ui5 where the application generates a batch request "Microsoft.AspNetCore.OData" Version="8.0.11"

siasty commented 8 months ago

in my case problem was in context.Response.Headers["Access-Control-Expose-Headers"]= "OData-Version";

do you tray add this https://github.com/OData/AspNetCoreOData/issues/705#issuecomment-1260826967

image

MichaelUlloa commented 8 months ago

If you are configuring CORS at the AppService level, you don't need to have UseCors() in your code, do you @MichaelUlloa ?

You're right. Might not be the best in my case, since using the app service CORS config only allows me to set the allowed origins, but not the allowed methods or headers. I'll try @siasty's workaround instead and let y'all know

MichaelUlloa commented 8 months ago

@siasty's workaround worked, but I realized it wasn't the Access-Control-Expose-Headers that worked for me, but setting the Access-Control-Allow-Origin header to all origins ("*"). I ended up checking if the host's origin was in my allowed origins. My Program.cs ended up looking as such (any improvement or comment is welcome):

var builder = WebApplication.CreateBuilder(args);

// ...

string[] allowedOrigins = builder.Configuration.GetSection("Cors:Origins").Get<string[]>();

builder.Services.AddCors(options =>
{
    options.AddDefaultPolicy(policy => policy
        .WithOrigins(allowedOrigins)
        .WithMethods(HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch, HttpMethods.Delete)
        .AllowAnyHeader()
        .WithExposedHeaders(HeaderNames.Location));
});

var app = builder.Build();

// ...

app.Use((context, next) =>
{
    string origin = context.Request.Headers.Origin.ToString();

    if (context.Request.Path.ToString().Contains("/$batch", StringComparison.OrdinalIgnoreCase)
        && allowedOrigins.Contains(origin))
    {
        context.Response.Headers.AccessControlAllowOrigin = origin;
    }

    return next.Invoke();
});

app.UseODataBatching();

// ...

app.Run();