AutoMapper.Extensions.OData: $skip is not applied when $orderby is not defined

I’ve encountered issue when using standalone $skip parameter, for example corebuilding?$skip=2

I think problem lies in GetQueryableMethod method:

if (orderByClause == null)
{
    return Expression.Call
    (
        expression.Type.IsIQueryable() ? typeof(Queryable) : typeof(Enumerable),
        "Take",
        new[] { type },
        expression,
        Expression.Constant(top.Value)
    );;
}

So, if OrderByClause is not defined, only Take expression is going to be returned, without applying Skip expression.

Before starting with the fix, could you tell me if this is actually a bug, or intended behavior. To me, it looks like the bug.

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 1
Comments: 18 (11 by maintainers)

Most upvoted comments

Yeah, so currently I’m generating a default order for the following cases:

orderby is null but skip is not null
orderby is null but top is not null
orderby is null but both top and skip are not null

I agree with you about using the same method for generating an order, keep things consistent. I’ll start on creating such a method and go from there.

wbuck on Feb 16, 2022

That’s a good point point regarding Top. Do we not want to apply the same rules? Maybe we should.

With regard to generating an OrdeyBy, the algorithm itself does not matter - better to use the same one in both cases.

Two ways to go:

Select the first public property which is a literal type.
Select all properties which are keys (If my memory is correct ASP.NET OData requires each entity to have an Id property or have the [Key] attribute applied).

Here’s an example of some code getting the first literal type.

I like the 2nd option better. You’d be calling MemberInfo.GetCustomAttributes() to see if System.ComponentModel.DataAnnotations.KeyAttribute has been applied (if there is no Id property).

So if GenerateStableOrder is not available for both cases I would just create a new method which does the same thing.

BlaiseD on Feb 16, 2022

@darjanbogdan neat, thank you! 😃

DanielGlos on Aug 9, 2021

Hi @DanielGlos, I also don’t work on it, didn’t have time to properly implement it back then.

What I did instead is a hack which is acceptable in my case, snippet example:

private static readonly PropertyInfo _orderByPropertyInfo = typeof(ODataQueryOptions).GetProperty("OrderBy");
public static void EnsureStableOrdering(this ODataQueryOptions queryOptions)
{
    if (queryOptions.OrderBy is null)
    {
        _orderByPropertyInfo.SetValue(queryOptions, queryOptions.GenerateStableOrder());
    }
}

GenerateStableOrder() is a method from the Microsoft.AspNet.OData.Query namespace and similarly should be used here in my opinion - just without reflection trickery 😃

darjanbogdan on Aug 9, 2021