Skip to content

Commit

Permalink
Make orderBy unique for paging.
Browse files Browse the repository at this point in the history 
OrderBy is not fully unique AutoMapper#221
  • Loading branch information
@NetanelPersikGoogle
NetanelPersikGoogle committed Nov 23, 2024
1 parent 05a0068 commit 9e9fb74
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 59 deletions.
58 changes: 25 additions & 33 deletions AutoMapper.AspNetCore.OData.EFCore/LinqExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +123,17 @@ public static Expression<Func<T, bool>> ToFilterExpression<T>(this ODataQueryOpt
/// <returns></returns>
public static Expression<Func<IQueryable<T>, IQueryable<T>>> GetQueryableExpression<T>(this ODataQueryOptions<T> options, ODataSettings oDataSettings = null)
{
if (NoQueryableMethod(options, oDataSettings))
return null;

ParameterExpression param = Expression.Parameter(typeof(IQueryable<T>), "q");
var param = Expression.Parameter(typeof(IQueryable<T>), "q");

return Expression.Lambda<Func<IQueryable<T>, IQueryable<T>>>
(
param.GetOrderByMethod(options, oDataSettings), param
);
}

public static Expression GetOrderByMethod<T>(this Expression expression,
private static Expression GetOrderByMethod<T>(this Expression expression,
ODataQueryOptions<T> options, ODataSettings oDataSettings = null)
{
if (NoQueryableMethod(options, oDataSettings))
return null;

return expression.GetQueryableMethod
(
options.Context,
Expand Down Expand Up @@ -168,35 +162,33 @@ public static Expression GetOrderByMethod<T>(this Expression expression,
public static Expression GetQueryableMethod(this Expression expression,
ODataQueryContext context, OrderByClause orderByClause, Type type, int? skip, int? top)
{
if (orderByClause is null && skip is null && top is null)
return null;

if (orderByClause is null && (skip is not null || top is not null))
var orderBySettings = context.FindSortableProperties(type);
switch (orderByClause)
{
var orderBySettings = context.FindSortableProperties(type);

if (orderBySettings is null)
return null;
case null when skip is null && top is null:
{
return orderBySettings is null
? null
: expression.GetDefaultOrderByCall(orderBySettings);
}
case null when skip is not null || top is not null:
{
if (orderBySettings is null) return null;

return expression
.GetDefaultOrderByCall(orderBySettings)
.GetSkipCall(skip)
.GetTakeCall(top);
return expression
.GetDefaultOrderByCall(orderBySettings)
.GetSkipCall(skip)
.GetTakeCall(top);
}
default:
return expression
.GetOrderByCall(orderByClause, context)
.GetDefaultThenByCall(orderBySettings)
.GetSkipCall(skip)
.GetTakeCall(top);
}

return expression
.GetOrderByCall(orderByClause, context)
.GetSkipCall(skip)
.GetTakeCall(top);
}

private static bool NoQueryableMethod(ODataQueryOptions options, ODataSettings oDataSettings)
=> options.OrderBy is null
&& options.Top is null
&& options.Skip is null
&& oDataSettings?.PageSize is null;



private static Expression GetDefaultThenByCall(this Expression expression, OrderBySetting settings)
{
return settings.ThenBy is null
Expand Down
6 changes: 6 additions & 0 deletions AutoMapper.OData.EFCore.Tests/Data/DatabaseInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ public static void SeedDatabase(MyDbContext context)
new TBuilding { Identity = Guid.NewGuid(), LongName = "Two L3", BuilderId = builders.First(b => b.Name == "Mark").Id }
}
});
context.MandatorSet.Add(new TMandator
{
Identity = Guid.Empty, // The first guide in order.
Name = "Two", // Duplicate name.
CreatedDate = new DateTime(2011, 12, 12)
});
context.SaveChanges();
}
}
Expand Down
96 changes: 70 additions & 26 deletions AutoMapper.OData.EFCore.Tests/GetQueryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public async void OpsTenantExpandBuildingsNestedFilterEqUsingThisParameterWithNo

static void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Equal(3, collection.Count);
Assert.Empty(collection.First().Buildings);
Assert.Empty(collection.Last().Buildings);
}
Expand All @@ -165,9 +165,9 @@ public async void OpsTenantExpandBuildingsNestedFilterEqUsingThisParameterWithMa

static void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Single(collection.First().Buildings);
Assert.Equal("Two L1", collection.First().Buildings.First().Name);
Assert.Equal(3, collection.Count);
Assert.Single(collection.Skip(1).First().Buildings);
Assert.Equal("Two L1", collection.Skip(1).First().Buildings.First().Name);
Assert.Empty(collection.Last().Buildings);
}
}
Expand All @@ -182,9 +182,8 @@ public async void OpsTenantExpandBuildingsFilterNeAndOrderBy()

void Test(ICollection<OpsTenant> collection)
{
Assert.Single(collection);
Assert.Equal(3, collection.First().Buildings.Count);
Assert.Equal("Two", collection.First().Name);
Assert.Equal(3, collection.Skip(1).First().Buildings.Count);
Assert.Equal("Two", collection.Skip(1).First().Name);
}
}

Expand Down Expand Up @@ -216,14 +215,14 @@ void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Null(collection.First().Buildings);
Assert.Equal("One", collection.First().Name);
Assert.Contains("One", collection.First().Name);
}
}

[Fact]
public async void OpsTenantFilterLtDateNoExpand()
{
string query = "/opstenant?$filter=CreatedDate lt 2012-11-11T12:00:00.00Z";
string query = "/opstenant?$filter=CreatedDate lt 2010-11-11T12:00:00.00Z";
Test(Get<OpsTenant, TMandator>(query));
Test(await GetAsync<OpsTenant, TMandator>(query));
Test(await GetUsingCustomNameSpace<OpsTenant, TMandator>(query));
Expand All @@ -244,8 +243,8 @@ public async void OpsTenantExpandBuildingsNoFilterAndOrderBy()

void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Equal(3, collection.First().Buildings.Count);
Assert.Equal(3, collection.Count);
Assert.Equal(3, collection.Skip(1).First().Buildings.Count);
Assert.Equal("Two", collection.First().Name);
}
}
Expand All @@ -260,11 +259,58 @@ public async void OpsTenantNoExpandNoFilterAndOrderBy()

void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Equal(3, collection.Count);
Assert.Null(collection.First().Buildings);
Assert.Equal("Two", collection.First().Name);
}
}

[Fact]
public async void OpsTenantNoExpandNoFilterNoOrderByShouldApplyByPk()
{
const string query = "/opstenant";
Test(Get<OpsTenant, TMandator>(query));
Test(await GetAsync<OpsTenant, TMandator>(query));
Test(await GetUsingCustomNameSpace<OpsTenant, TMandator>(query));
return;

void Test(ICollection<OpsTenant> collection)
{
var expected = collection
.Select(x => x.Identity)
.OrderBy(identity => identity)
.ToList();

Assert.True(collection
.Select(x => x.Identity)
.SequenceEqual(expected));
}
}

[Fact]
public async Task OpsTenantNoExpandNoFilterWithOrderByShouldApplyByPk()
{
const string query = "/opstenant?$orderby=Name desc";

// Test multiple scenarios
Test(Get<OpsTenant, TMandator>(query));
Test(await GetAsync<OpsTenant, TMandator>(query));
Test(await GetUsingCustomNameSpace<OpsTenant, TMandator>(query));

return;

void Test(ICollection<OpsTenant> collection)
{
// Check if the collection is correctly ordered by Name (desc) and then by Identity (asc)
var expected = collection
.OrderByDescending(x => x.Name)
.ThenBy(x => x.Identity)
.ToList();

Assert.True(collection.SequenceEqual(expected),
"Collection is not ordered by Name (desc) and Identity (asc).");
}
}

[Fact]
public async void OpsTenantNoExpandFilterEqAndOrderBy()
Expand Down Expand Up @@ -292,12 +338,11 @@ public async void OpsTenantExpandBuildingsSelectNameAndBuilderExpandBuilderExpan

void Test(ICollection<OpsTenant> collection)
{
Assert.Single(collection);
Assert.Equal(3, collection.First().Buildings.Count);
Assert.NotNull(collection.First().Buildings.First().Builder);
Assert.NotNull(collection.First().Buildings.First().Builder.City);
Assert.NotEqual(default, collection.First().Buildings.First().Builder.City.Name);
Assert.Equal("Two", collection.First().Name);
Assert.Equal(3, collection.Skip(1).First().Buildings.Count);
Assert.NotNull(collection.Skip(1).First().Buildings.First().Builder);
Assert.NotNull(collection.Skip(1).First().Buildings.First().Builder.City);
Assert.NotEqual(default, collection.Skip(1).First().Buildings.First().Builder.City.Name);
Assert.Equal("Two", collection.Skip(1).First().Name);
}
}

Expand All @@ -311,11 +356,10 @@ public async void OpsTenantExpandBuildingsExpandBuilderExpandCityFilterNeAndOrde

void Test(ICollection<OpsTenant> collection)
{
Assert.Single(collection);
Assert.Equal(3, collection.First().Buildings.Count);
Assert.NotNull(collection.First().Buildings.First().Builder);
Assert.NotNull(collection.First().Buildings.First().Builder.City);
Assert.Equal("Two", collection.First().Name);
Assert.Equal(3, collection.Skip(1).First().Buildings.Count);
Assert.NotNull(collection.Skip(1).First().Buildings.First().Builder);
Assert.NotNull(collection.Skip(1).First().Buildings.First().Builder.City);
Assert.Equal("Two", collection.Skip(1).First().Name);
}
}

Expand Down Expand Up @@ -779,11 +823,11 @@ public async void OpsTenantOrderByCountOfReference()

void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Equal(3, collection.Count);
Assert.NotNull(collection.First().Buildings);
Assert.Equal("Two", collection.First().Name);
Assert.Equal(3, collection.First().Buildings.Count);
Assert.Equal(2, collection.Last().Buildings.Count);
Assert.Equal(2, collection.SkipLast(1).Last().Buildings.Count);
}
}

Expand All @@ -798,7 +842,7 @@ public async void OpsTenantOrderByFilteredCount()

void Test(ICollection<OpsTenant> collection)
{
Assert.Equal(2, collection.Count);
Assert.Equal(3, collection.Count);
Assert.NotNull(collection.First().Buildings);
Assert.Equal("One", collection.First().Name);
Assert.Equal(2, collection.First().Buildings.Count);
Expand Down

0 comments on commit 9e9fb74

Please sign in to comment.