OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
855 stars 473 forks source link

[.net core/odata] OData query with $skip doesn't work. #1521

Open alkozlov opened 6 years ago

alkozlov commented 6 years ago

Hi guys. I have a pretty simple .NET Core v2.1 application with OData v4.0 (NuGet package Microsoft.AspNetCore.OData v7.0.0). I faced problem with $skip query parameter. As only I add this parameter to request I couldn't get response. This query works as expected: http://localhost:54229/odata/Apartment?$top=10 This query doesn't work: http://localhost:54229/odata/Apartment?$skip=10&$top=10

Generally any query doesn't work if $skip query parameter is present.

Assemblies affected

.NET Core v2.1 with Web API (angular application template). Entity Framework Core Microsoft.AspNetCore.OData v7.0.0 Angular 5

Reproduce steps

Request: http://localhost:54229/odata/Apartment?$skip=10&$top=10 Any request with $skip query parameter.

Expected result

Response contains data with applied query parameters.

Actual result

No response: noresponse

Correct response (without $skip): correctresponse

Details from application in browser: browserbehavior1 browserbehavior2

Additional detail

This may seem strange, but this problem is relevant only for my working computer in the office. If I run my application on my personal laptop, everything works fine. I tried to clean all the temporary project files, deleted the project completely and downloaded it. Nothing helps. Below is an example of controller code and settings.

My controller:

`public class ApartmentController : Controller { private readonly ApplicationDbContext _context;

    public ApartmentController(ApplicationDbContext context)
    {
        this._context = context;
    }

    [HttpGet]
    [EnableQuery]
    public IQueryable<Apartment> Get() => this._context.Apartments.AsQueryable();
}`

Startup.cs

`public void ConfigureServices(IServiceCollection services) { services.AddAutoMapper(); services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) .AddJwtBearer(options => { options.TokenValidationParameters = new TokenValidationParameters { ValidateIssuer = true, ValidateAudience = true, ValidateLifetime = true, ValidateIssuerSigningKey = true, ValidIssuer = this.Configuration["Jwt:Issuer"], ValidAudience = this.Configuration["Jwt:Issuer"], IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(this.Configuration["Jwt:Key"])), RequireExpirationTime = false }; });

        services.AddAuthorization(options =>
        {
            options.AddPolicy("OnlyApiAdmin", policy => { policy.RequireClaim("api_admin"); });
        });

        services.AddOData();
        services.AddTransient<Kpd37GomelModelBuilder>();

        services.AddMvc();

        var connection = this.Configuration.GetConnectionString("DefaultConnection");
        services.AddDbContext<ApplicationDbContext>(options => options.UseSqlServer(connection));

        // In production, the Angular files will be served from this directory
        services.AddSpaStaticFiles(configuration =>
        {
            configuration.RootPath = "ClientApp/dist";
        });

        #region IoC Config

        services.AddTransient<IApartmentService, ApartmentService>();
        services.AddTransient<ITenantService, TenantService>();
        services.AddTransient<IVoteService, VoteService>();

        #endregion
    }`

`public void Configure(IApplicationBuilder app, IHostingEnvironment env, Kpd37GomelModelBuilder modelBuilder) { if (env.IsDevelopment()) { app.UseDeveloperExceptionPage(); } else { app.UseExceptionHandler("/Error"); }

        app.UseStaticFiles();
        app.UseSpaStaticFiles();

        app.UseCors(x => x
            .AllowAnyOrigin()
            .AllowAnyMethod()
            .AllowAnyHeader()
            .AllowCredentials());

        app.UseAuthentication();

        var builder = new ODataConventionModelBuilder();
        builder.EntitySet<Apartment>(nameof(Apartment));

        app.UseMvc(routes =>
        {
            routes
                .Select()
                .Expand()
                .Filter()
                .OrderBy(QueryOptionSetting.Allowed)
                .MaxTop(null)
                .Count();

            routes.MapODataServiceRoute("OData", "odata", builder.GetEdmModel());

            routes.MapRoute(
                name: "default",
                template: "{controller}/{action=Index}/{id?}");
        });

        app.UseSpa(spa =>
        {
            // To learn more about options for serving an Angular SPA from ASP.NET Core,
            // see https://go.microsoft.com/fwlink/?linkid=864501

            spa.Options.SourcePath = "ClientApp";

            if (env.IsDevelopment())
            {
                //spa.UseAngularCliServer(npmScript: "start");
                spa.UseProxyToSpaDevelopmentServer("http://localhost:4200");
            }
        });
    }`
alkozlov commented 6 years ago

One more note: I deploy my application to Azure and it's works properly. Probably there are some problems with the system settings on my work computer. But I do not know what the problem is.

biaol-odata commented 6 years ago

@alkozlov Since it works on both laptop and Azure deployment, it doesn't sound like a code issue. Instead, it seems like an environment issue on your office dev box. Here is one link that might be helpful to resolve the connectivity issue. Please let us know what you find out.

alkozlov commented 6 years ago

@biaol-odata Thank you for help. I solved my problem. But the solution was not at all where I was looking for it. When writing my application, I was guided by this article https://blogs.msdn.microsoft.com/odatateam/2018/07/03/asp-net-core-odata-now-available/. It turned out that the .NET Core dispose data before they can serialize. And the example described in the article above does not work for me. The solution to the problem is an additional configuration in the Startup.cs

services.AddMvc() .SetCompatibilityVersion(CompatibilityVersion.Version_2_1) .AddJsonOptions(options => { options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; });

Without this option options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; I continue get error, but if specify this option the error ceases to occur.

biaol-odata commented 6 years ago

@alkozlov Thanks for the update. The addition information provided above is not environment-related, maybe your laptop and azure deployment were working and didn't hit the exact cases that were failing on you dev box? This needs to be looked into further for sure.

ShenglinGuo commented 6 years ago

I am just going to add another possible cause of $skip not working. We use odata controller to drive UI grid so, paging is important feature. We notice that the first page worked when $skip is not apply, but as soon as $skip is apply the count is still correctly returned but the value collection is empty array.

I been digging and looking at all the configuration and settings. Eventually I found the our issue by looking at the SQL Profiler and the query generated.

For some reason, our odata controller have [EnableQuery] attribute set at both the controller class level and at the method level. i.e.

[EnableQuery] public someController : ControllerBase{ [EnableQuery] public IActionResult Get() ..... }

This configuration somehow cause a query generated in following fashion

SELECt col1, col2 ..... From ( SELECT col1, col2 ..... from table where ....... order by id offset $skip rows $rows ROWS ONLY ) order by id offset $skip row $rows ROWS ONLY

i.e. by having the [EnableQuery] attribute assigned to both the controller class and method, the odata framework will generate order by and offset twice into the query. This will not work.

We fixed our issue by remove the class level [EnableQuery]. Then the query generated is like

SELECT col1, col2 ..... from table where ....... order by id offset $skip rows $rows ROWS ONLY

(without doing the ordering and offset twice)

I know we probably did not do the right thing by having the attribute twice, but there were no warnings, and apart from $skip everything else seems to work fine. It took me a few hours of digging to figure this one out. Hopefully this can save somebody hours of headache.

Suggestion to put restriction on [EnableQuery] attribute if it is not meant to be used at the class level, or provide some kind of warning / error.

sorcerb commented 6 years ago

@alkozlov, services.AddMvc() .SetCompatibilityVersion(CompatibilityVersion.Version_2_1) .AddJsonOptions(options => { options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; }); didn't help to me

http://localhost:5001/api/Customers?$skip=10&$top=10 ->Could not get any response http://localhost:5001/api/Customers?$top=10 -> 200 Ok with data

 public class CustomersController : ODataController
    {
private readonly DBContext _db = new DBContext();

// GET: Customers
        [HttpGet]
        [EnableQuery(AllowedQueryOptions = AllowedQueryOptions.All)]
        public IQueryable<Customers> Get()
        {
            var r = _db.Customers;
            return r;
        }
}
alkozlov commented 6 years ago

@sorcerb Did you configure serialization in Startup.cs? .AddJsonOptions(options => { options.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore; })

I noticed that there is any problems with serializations of objects with complex structure. And the serialization options specified above is required for solving this problem.

sorcerb commented 6 years ago

@alkozlov yes, here is my startup class:

public partial class Startup
    {
        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<DBContext>();

            services.AddOData();
            services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1).AddJsonOptions(option => option.SerializerSettings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore);

            services.AddTransient<ITokenRepository, TokenRepository>();
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
            loggerFactory.AddConsole(Configuration.GetSection("Logging"));
            loggerFactory.AddDebug();

            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            var oData = new ODataConventionModelBuilder(app.ApplicationServices);

            oData.EntitySet<Customers>("Customers").EntityType.HasKey(s => s.CustomerId);

            //app.UseDefaultFiles();
            app.UseStaticFiles();

            app.UseCors(builder => builder
                    .AllowAnyOrigin()
                    .AllowAnyMethod()
                    .AllowAnyHeader()
                    .AllowCredentials());

            app.UseMvc(r =>
            {
                r.Select()
                    .Expand()
                    .Filter()
                    .OrderBy(QueryOptionSetting.Allowed)
                    .MaxTop(null)
                    .Count();
                r.MapODataServiceRoute("odata", "api", oData.GetEdmModel());
                r.EnableDependencyInjection();
            });

        }
    }

.NET Core v2.1

Microsoft.AspNetCore.All (2.1.2) Microsoft.AspNetCore.Cors (2.1.1) Microsoft.AspNetCore.OData (7.0.1)

alkozlov commented 6 years ago

@sorcerb I compared your and my code. It looks the same. The only difference is that I overrided GetEdmModel() method. This is my code:

`static class Kpd37ODataConventionModelBuilder { public static IEdmModel GetEdmModel() { ODataConventionModelBuilder builder = new ODataConventionModelBuilder();

        builder.EntitySet<Apartment>("Apartment");
        builder.EntitySet<Tenant>("Tenant");
        builder.EntitySet<Vote>("Vote");
        builder.EntitySet<VoteVariant>("VoteVariant");
        builder.EntitySet<ApartmentVoteChoice>("ApartmentVoteChoice");

        builder.EntityType<Vote>()
            .Action("SendVote")
            .Parameter<Guid>("VariantId");

        var getCommonResultsFunction = builder.EntityType<Vote>().Function("GetCommonResults");
        getCommonResultsFunction.ReturnsCollection<VoteChoiseTinyDTO>();

        var getDetailedResultsFunction = builder.EntityType<Vote>().Function("GetDetailedResults");
        getDetailedResultsFunction.ReturnsCollectionFromEntitySet<ApartmentVoteChoice>("ApartmentVoteChoice");

        return builder.GetEdmModel();
    }
}

`

`app.UseMvc(routes => { routes.Select().Expand().Filter().OrderBy().MaxTop(null).Count(); routes.MapODataServiceRoute("OData", "odata", Kpd37ODataConventionModelBuilder.GetEdmModel());

            routes.MapRoute(
                name: "default",
                template: "{controller}/{action=Index}/{id?}");
        });`

Try to do the same. Maybe it will help you.

sorcerb commented 6 years ago

@alkozlov , thanks for try, but not help to me...

I check sql query by SQL Profiler. Got sql error:

Msg 102, Level 15, State 1, Line 4
Incorrect syntax near 'OFFSET'.
Msg 153, Level 15, State 2, Line 4
Invalid usage of the option NEXT in the FETCH statement.

I have SQL Server 2008, max compatibility level is 2008 (100) OFFSET was added in SQL Server 2012, so in 2008 this keyword is not available.

upd: UseRowNumberForPaging() help me. Skip is work now.

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
   if (!optionsBuilder.IsConfigured)
   {
      optionsBuilder.UseSqlServer("Server=***;Database=***;Trusted_Connection=True;", opt=>opt.UseRowNumberForPaging());
   }
}