[feature/netcore] $skip doesn't work

[gordon-matt]

The following doesn't return any values:

http://localhost:5376/odata/mantle/common/RegionApi?$filter=RegionType eq Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType'Country' and ParentId eq 1&%24format=json&%24top=10&%24skip=10&%24orderby=Name&%24count=true

When removing $skip, it works fine.. and one more REALLY weird thing is when using ? instead of %24, it again does something different, which is return everything (Yes, that was quite unexpected, as I didn't think that should make any difference - and I tested with both FireFox and Edge with the same results):

http://localhost:5376/odata/mantle/common/RegionApi?$filter=RegionType%20eq%20Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType%27Country%27%20and%20ParentId%20eq%201&?format=json&?top=10&?skip=10&?orderby=Name&?count=true

The RegionType enum is as follows (but I think you can use any enum to test):

public enum RegionType : byte
{
    Other = 0,
    Continent = 1,
    Country = 2,
    State = 3,
    City = 4,
}

NOTE: At first I thought it was probably something with the enum filtering, but while writing this I decided to try a different query which doesn't use enums and I get the same results.

[gordon-matt]

This is quite a serious issue. Any chance it can be addressed soon? $skip simply doesn't work at all.

[Compufreak345]

Regarding your second issue, it should be $count etc., not ?count ;)

For me this works fine (using .Net Core & EF Core Preview 2.1), you might need to provide some more code/context to reproduce your issue: https://localhost:5001/odata/SolColumnDefinition?$filter=endswith(InternalName,'Title')&$orderby=Id desc&$top=2&$skip=2&$select=Id,TableId,InternalName,DisplayName,FieldTypeKind,MaxLength

        [EnableQuery]
        public IQueryable<SolColumnDefinition> Get()
        {
            return _dbContext.SolColumnDefinition.AsQueryable();
        }

[gordon-matt]

@Compufreak345 Thanks for your response. I don't know what I was thinking using ?. Must've been one of those long days... Anyway, $skip still doesn't work. I have a repo I use for testing OData stuff. I have updated it for this use case. You can find it here: OData_Test. Just change the connection string as needed and use Update-Database from the package manager console to create the db. When you run it, a table called Mantle_Regions will be populated. Then simply try running some queries similar to the ones I showed above. For starters, we can make it simple querying with and without $skip:

https://localhost:44367/odata/RegionApi?&$format=json&$top=10&$orderby=Name&$count=true

https://localhost:44367/odata/RegionApi?&$format=json&$top=10&$skip=10&$orderby=Name&$count=true

You will notice the latter doesn't return any values.. just the total row count.

[Compufreak345]

Interesting. For me it seems it's not that skip isn't working, it seems to skip 2 times the amount of entries you define. I put 14 entries in my database, starting with an Id of 2, and that is what happened: I set skip to 6 and it actually skipped 12 entries. I'll have to try this with my own API-project after I got it running again (I'll edit the results into this post).

[Compufreak345]

@gordon-matt I think I found the issue in your code. You do this:

var results = options.ApplyTo(query);
return await (results as IQueryable<TEntity>).ToHashSetAsync();

This applies the skip twice, once in your code and once in, I assume, the middleware, because you did set the "EnableQuery"-annotation.

If I change it to this:

return await query.ToHashSetAsync();

It skips only once.

[gordon-matt]

Strange. I have the exact same code in MVC5 and it doesn't cause that issue there. For now, I can remove the [EnableQuery] attribute and that seems to be working fine.. but surely it shouldn't matter.. Shouldn't the rules kind of "cascade" ? By that I mean, if one of my projects disables $count or $select or whatever in the middleware, but for a specific controller, maybe I want to allow everything using the attribute.. then it should be coded in such a way as to deal with the functions that are "enabled" in both places so as to only call them once... or basically: if the attribute exists, use that and ignore the middleware config.

[Compufreak345]

I'm quite new to the library, I'd be interested in what @robward-ms says about how it is intended to work.

[robward-ms]

Here's the result of my investigation, in short: yes, using [EnableQuery] and ODataQueryOptions will double apply, making a top 10, skip 10 return no results (second apply of skip 10 skips all records).

Documentation: https://docs.microsoft.com/en-us/aspnet/web-api/overview/odata-support-in-aspnet-web-api/supporting-odata-query-options

Invoking Query Options Directly Instead of using the [Queryable] attribute, you can invoke the query options directly in your controller. To do so, add an ODataQueryOptions parameter to the controller method. In this case, you don't need the [Queryable] attribute.

The docs were written for the V3 series but in the V4 product, [EnableQuery] and [Queryable] are similar.

@gordon-matt, you mentioned this is different than MVC5? Any chance you are not using the ".ToHashSetAsync();" in MVC5?

Test Results

http://localhost:5376/odata/mantle/common/RegionApi?$filter=RegionType eq Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType'Country' and ParentId eq 1&%24format=json&%24top=10&%24skip=10&%24orderby=Name&%24count=true 404

http://localhost:5376/odata/RegionApi?$filter=RegionType eq Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType'Country' and ParentId eq 1&%24format=json&%24top=10&%24skip=10&%24orderby=Name&%24count=true 500: ODataException: The string 'Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType'Country'' is not a valid enumeration type constant.

http://localhost:5376/odata/mantle/common/RegionApi?$filter=RegionType%20eq%20Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType%27Country%27%20and%20ParentId%20eq%201&?format=json&?top=10&?skip=10&?orderby=Name&?count=true 500: ODataException: The string 'Mantle.Web.Common.Areas.Admin.Regions.Domain.RegionType'Country'' is not a valid enumeration type constant.

http://localhost:5376/odata/RegionApi?format=json&?top=10&?skip=10&?orderby=Name&?count=true 200: returns Ids 1-258, count not returned. query options start with "?" instead of "$" so they were ignored. Hex 24 (%24) = "$" so using "?" and $24 return different result, see below.

http://localhost:5376/odata/RegionApi?format=json&$top=10&$skip=10&$orderby=Name&$count=true 200: returns count only (258). The root cause here is skip was applied by the controller and enable query attribute. When applied twice with top-10 & skip=10, the controller returns 10 records starting at Id 254 then Enable query skips those 10 records to produce no output.

200: http://localhost:5376/odata/RegionApi?format=json&$top=10&$orderby=Name&$count=true returns 10 records & count: ids 88, 5, 201, 229, 142, 63, 230, 173, 8, 7

http://localhost:5376/odata/RegionApi?format=json&$top=20&$skip=10&$orderby=Name&$count=true 200: Returns 10 records & count: ids 32, 118, 119, 33, 232, 233, 34, 174, 35, 120 Controller return 20 records & count: starting id 254, ending id 120. EnableQueryAttribute returns 10 records & count: ids 32, 118, 119, 33, 232, 233, 34, 174, 35, 120 So top 20 skip to once returned starting id 254, ending id 120 and another top 20 skip 10 returned the second half of the original list of 20.

Removed [EnableQuery] from controller. http://localhost:5376/odata/RegionApi?format=json&$top=10&$skip=10&$orderby=Name&$count=true 200: Returns 10 records & count: ids 254, 30, 49, 116, 31, 4, 3, 85, 231, 117

[gordon-matt]

@robward-ms To answer your questions, I am using ToHashSetAsync() in MVC5 and it's fine... and moreover, I did try using ToHashSet() (non async version) in .NET Core, but it still had the same error. So it can't be that. The only thing that fixed it was to remove the [EnableQuery] attribute.

Regarding your test results, some of those URLs are a bit dated and/or from a different project - not my OData_Test project (so just use /odata and NOT /odata/mantle/common). The one with the question marks instead of dollar sign was already addressed above (my mistake and not important).

The only 2 queries we need to be concerned with are as follows:

WITHOUT $skip:

https://localhost:44367/odata/RegionApi?&$format=json&$top=10&$orderby=Name&$count=true

WITH $skip:

https://localhost:44367/odata/RegionApi?&$format=json&$top=10&$skip=10&$orderby=Name&$count=true

And then test with and without the [EnableQuery] attribute.. Can you please make it so that if the [EnableQuery] attribute is present, then the config from middleware (example: routes.Select().Expand().Filter().OrderBy().MaxTop(null).Count();) is ignored? Then it won't run $skip twice.

EDIT: I may have misunderstood about middleware being involved - you say it's not from that (routes.Select().Expand().Filter().OrderBy().MaxTop(null).Count();), but rather from the ODataQueryOptions being applied that we are seeing $skip applied twice, right? Whatever the reason, it's not a problem in MVC5 and a simple solution is needed.

[gordon-matt]

I must apologize - MVC5 does have the same problem... It seems I already had [EnableQuery] commented out there.. I don't remember doing that, but at some point it seems I did. OK, ignore this then I guess. The only question I have then is this: If I wanted to enable all filters by default (as I am doing now on the routes.Select().. etc.., then what if I want to disable certain functions (like $skip or $select or whatever) for one of my controllers? Don't I have to use the [EnableQuery] attribute and use the AllowedQueryOptions property? Or is there another way? Because if I use that attribute, then we have a problem with $skip being applied twice as you see above.

[dozer75]

We had the problem in one of our projects on MVC5, we created an EnableQueryAttribute override we'er using instead of default where we override the ApplyQuery this way (not sure where this code originates from):

    public override IQueryable ApplyQuery(IQueryable queryable, ODataQueryOptions queryOptions)
    {
        // get the original request before the alterations
        var originalRequest = queryOptions.Request;
        if (originalRequest.Method == HttpMethod.Get)
        {
            // remove $skip (we have already skipped!)
            var uri = new Uri(Regex.Replace(originalRequest.RequestUri.AbsoluteUri, @"(\$skip=\d*[&]?)", "", RegexOptions.IgnoreCase | RegexOptions.Singleline));
            originalRequest.RequestUri = uri;
            queryOptions = new ODataQueryOptions(queryOptions.Context, originalRequest);
        }
        return base.ApplyQuery(queryable, queryOptions);
    }

[robward-ms]

@gordon-matt - Yep, it would be applied twice if you use a global and scoped EnableQueryAttribute. I was fixing E2E tests this weekend and found the expected behavior. I files #1340 for the issue and submitted #1345 to fix it.

Since this issue was something different (but related), I suggest we leave the tags and close it provided you are satisfied with the answer for the original question, i.e. EnableQueryAttribute and ODataQueryOptions.

Interestingly, ODataQueryProvider is used to wrap EnableQueryAttribute for the global case and handles this case by not applying the global query if the controller takes a ODataQueryOptions parameter. We could consider this issue a feature request to see if we can apply the same login to a decorated EnableQueryAttribute to avoid the issue.

I'll let you choose if you want to close or switch it to a feature request.

[gordon-matt]

@robward-ms Thanks, but I don't think the issue with AddODataQueryFilter applies to me, because I am not even using that. As @Compufreak345 pointed out in this post, it seems that I was getting it applied twice because I had an [EnableQuery] attribute while at the same time using options.ApplyTo(query);. My solution is removing the [EnableQuery] attribute for now, but I was wondering what do I do then if I want to restrict the query options (for example: allow everything except $expand)? As far as I can see, I can only use the AllowedQueryOptions property on the [EnableQuery] attribute to achieve that. But I have to remove my [EnableQuery] attribute, because I need to use options.ApplyTo(query).

[robward-ms]

You can set the allowed query options during apply, see this test.

[gordon-matt]

Excellent. Since my solution is a generic one, what I have done is create a virtual property on the controller as follows:

protected virtual AllowedQueryOptions AllowedQueryOptions => AllowedQueryOptions.All;

Then I will override as needed.

My generic Get method is now as follows:

public virtual async Task<IEnumerable<TEntity>> Get(ODataQueryOptions<TEntity> options)
{
    if (!CheckPermission(ReadPermission))
    {
        return Enumerable.Empty<TEntity>().AsQueryable();
    }

    var connection = GetDisposableConnection();
    var query = connection.Query();
    query = ApplyMandatoryFilter(query); // Custom filtering for this controller (example: maybe only retrieve records for current tenant)
    var results = options.ApplyTo(query, AllowedQueryOptions);

    // Recommended not to use ToHashSetAsync(). See: https://github.com/OData/WebApi/issues/1235#issuecomment-371322404
    //return await (results as IQueryable<TEntity>).ToHashSetAsync();

    return await Task.FromResult((results as IQueryable<TEntity>).ToHashSet());
}

I haven't tested it yet, but I am certain it will be fine. I will close this now. Thanks

[gordon-matt]

@robward-ms FYI - I closed this issue assuming that setting the query options during apply would be fine, but I was wrong. I just opened a new issue for this: #1357

[GavinHome]

Microsoft.AspNetCore.OData： 7.0.0 .net core 2.0： 2.0.7

I use postman to get results. There is no response result.

with out '$skip', it is working fine:

I add this line of code, var appliedQueryOptions = AllowedQueryOptions.Skip | AllowedQueryOptions.Filter | AllowedQueryOptions.Expand; options.ApplyTo(await GetAsync<T>(), appliedQueryOptions)

There is a response result, but the result is always the first page.

The OData project config: OData V7.0.0 for ASP.NET Core 2.x(http://odata.github.io/WebApi/#14-01-netcore-beta1),

so $skip doesn't work, Please help me，

Thanks