Skip to content

Commit 2b5de89

Browse files
alexmo16alexmo16
committed
instead of adding the EnsureStableOrdering reuse the AlwaysSortByPrimaryKey
To fix the problem, I fix the code where ordering by primary key was added even when AlwaysSortByPrimaryKey was disabled.
1 parent 8e88fb5 commit 2b5de89

File tree

5 files changed

+51
-44
lines changed

5 files changed

+51
-44
lines changed

‎AutoMapper.AspNetCore.OData.EF6/QueryableExtensions.cs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public static ICollection<TModel> Get<TModel, TData>(this IQueryable<TData> quer
2828
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>
2929
(
3030
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.Default,
31-
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
32-
ensureStableOrdering: querySettings?.ODataSettings?.EnsureStableOrdering ?? true
31+
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true
3332
);
3433
query.ApplyOptions(mapper, filter, options, querySettings);
3534
return query.Get
@@ -57,8 +56,7 @@ public static async Task<ICollection<TModel>> GetAsync<TModel, TData>(this IQuer
5756
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>
5857
(
5958
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.Default,
60-
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
61-
ensureStableOrdering: querySettings?.ODataSettings?.EnsureStableOrdering ?? true
59+
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true
6260
);
6361
await query.ApplyOptionsAsync(mapper, filter, options, querySettings);
6462
return await query.GetAsync
@@ -87,8 +85,7 @@ public static async Task<IQueryable<TModel>> GetQueryAsync<TModel, TData>(this I
8785
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>
8886
(
8987
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
90-
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
91-
ensureStableOrdering: querySettings?.ODataSettings?.EnsureStableOrdering ?? true
88+
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true
9289
);
9390
await query.ApplyOptionsAsync(mapper, filter, options, querySettings);
9491
return query.GetQueryable(mapper, options, querySettings, filter);
@@ -110,8 +107,7 @@ public static IQueryable<TModel> GetQuery<TModel, TData>(this IQueryable<TData>
110107
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>
111108
(
112109
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
113-
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
114-
ensureStableOrdering: querySettings?.ODataSettings?.EnsureStableOrdering ?? true
110+
enableConstantParameterization: querySettings?.ODataSettings?.EnableConstantParameterization ?? true
115111
);
116112
query.ApplyOptions(mapper, filter, options, querySettings);
117113
return query.GetQueryable(mapper, options, querySettings, filter);

‎AutoMapper.AspNetCore.OData.EFCore/LinqExtensions.cs‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ public static Expression<Func<T, bool>> ToFilterExpression<T>(
3636
this FilterQueryOption filterOption,
3737
HandleNullPropagationOption handleNullPropagation = HandleNullPropagationOption.Default,
3838
TimeZoneInfo timeZone = null,
39-
bool enableConstantParameterization = true,
40-
bool ensureStableOrdering = true)
39+
bool enableConstantParameterization = true)
4140
{
4241
if (filterOption == null)
4342
return null;
@@ -46,7 +45,7 @@ public static Expression<Func<T, bool>> ToFilterExpression<T>(
4645

4746
queryable = filterOption.ApplyTo(
4847
queryable,
49-
new ODataQuerySettings() { HandleNullPropagation = handleNullPropagation, TimeZone = timeZone, EnableConstantParameterization = enableConstantParameterization, EnsureStableOrdering = ensureStableOrdering });
48+
new ODataQuerySettings() { HandleNullPropagation = handleNullPropagation, TimeZone = timeZone, EnableConstantParameterization = enableConstantParameterization });
5049

5150
MethodCallExpression whereMethodCallExpression = (MethodCallExpression)queryable.Expression;
5251

@@ -63,16 +62,15 @@ public static Expression<Func<T, bool>> ToSearchExpression<T>(
6362
this SearchQueryOption filterOption,
6463
HandleNullPropagationOption handleNullPropagation = HandleNullPropagationOption.Default,
6564
TimeZoneInfo timeZone = null,
66-
bool enableConstantParameterization = true,
67-
bool ensureStableOrdering = true)
65+
bool enableConstantParameterization = true)
6866
{
6967
if (filterOption == null)
7068
return null;
7169

7270
IQueryable queryable = Enumerable.Empty<T>().AsQueryable();
7371
queryable = filterOption.ApplyTo(
7472
queryable,
75-
new ODataQuerySettings() { HandleNullPropagation = handleNullPropagation, TimeZone = timeZone, EnableConstantParameterization = enableConstantParameterization, EnsureStableOrdering = ensureStableOrdering });
73+
new ODataQuerySettings() { HandleNullPropagation = handleNullPropagation, TimeZone = timeZone, EnableConstantParameterization = enableConstantParameterization });
7674

7775
MethodCallExpression whereMethodCallExpression = (MethodCallExpression)queryable.Expression;
7876

@@ -82,8 +80,7 @@ public static Expression<Func<T, bool>> ToSearchExpression<T>(
8280
public static Expression<Func<T, bool>> ToFilterExpression<T>(this ODataQueryOptions<T> options,
8381
HandleNullPropagationOption handleNullPropagation = HandleNullPropagationOption.Default,
8482
TimeZoneInfo timeZone = null,
85-
bool enableConstantParameterization = true,
86-
bool ensureStableOrdering = true)
83+
bool enableConstantParameterization = true)
8784
{
8885
if (options is null || options.Filter is null && options.Search is null)
8986
{
@@ -95,14 +92,14 @@ public static Expression<Func<T, bool>> ToFilterExpression<T>(this ODataQueryOpt
9592
Expression filterExpression = null;
9693
if (options.Filter is not null)
9794
{
98-
var raw = options.Filter.ToFilterExpression<T>(handleNullPropagation, timeZone, enableConstantParameterization, ensureStableOrdering);
95+
var raw = options.Filter.ToFilterExpression<T>(handleNullPropagation, timeZone, enableConstantParameterization);
9996
filterExpression = raw.Body.ReplaceParameter(raw.Parameters[0], parameter);
10097
}
10198

10299
Expression searchExpression = null;
103100
if (options.Search is not null)
104101
{
105-
var raw = options.Search.ToSearchExpression<T>(handleNullPropagation, timeZone, enableConstantParameterization, ensureStableOrdering);
102+
var raw = options.Search.ToSearchExpression<T>(handleNullPropagation, timeZone, enableConstantParameterization);
106103
searchExpression = raw.Body.ReplaceParameter(raw.Parameters[0], parameter);
107104
}
108105

@@ -190,11 +187,7 @@ public static Expression GetQueryableMethod(this Expression expression,
190187
}
191188

192189
if (orderByClause is null && skip is null && top is null)
193-
return null;
194-
195-
bool ensureStableOrdering = oDataSettings?.EnsureStableOrdering ?? true;
196-
if (orderByClause is null && !ensureStableOrdering)
197-
return expression.GetSkipCall(skip).GetTakeCall(top);
190+
return null;
198191

199192
if (orderByClause is null && (skip is not null || top is not null))
200193
{
@@ -203,6 +196,9 @@ public static Expression GetQueryableMethod(this Expression expression,
203196
if (orderBySettings is null)
204197
return null;
205198

199+
if (oDataSettings?.AlwaysSortByPrimaryKey is false)
200+
return expression.GetSkipCall(skip).GetTakeCall(top);
201+
206202
return expression
207203
.GetDefaultOrderByCall(orderBySettings)
208204
.GetSkipCall(skip)

‎AutoMapper.AspNetCore.OData.EFCore/ODataSettings.cs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,5 @@ public class ODataSettings
5858
/// Default is false.
5959
/// </value>
6060
public bool AlwaysSortByPrimaryKey { get; set; }
61-
62-
/// <summary>
63-
/// Gets or sets a value indicating whether query composition should
64-
/// alter the original query when necessary to ensure a stable sort order.
65-
/// </summary>
66-
/// <value>
67-
/// Default is true.
68-
/// </value>
69-
public bool EnsureStableOrdering { get; set; } = true;
7061
}
7162
}

‎AutoMapper.AspNetCore.OData.EFCore/QueryableExtensions.cs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ public static ICollection<TModel> Get<TModel, TData>(this IQueryable<TData> quer
1919
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>(
2020
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
2121
querySettings?.ODataSettings?.TimeZone,
22-
querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
23-
querySettings?.ODataSettings?.EnsureStableOrdering ?? true);
22+
querySettings?.ODataSettings?.EnableConstantParameterization ?? true);
2423

2524
query.ApplyOptions(mapper, filter, options, querySettings);
2625
return query.Get
@@ -38,8 +37,7 @@ public static async Task<ICollection<TModel>> GetAsync<TModel, TData>(this IQuer
3837
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>(
3938
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
4039
querySettings?.ODataSettings?.TimeZone,
41-
querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
42-
querySettings?.ODataSettings?.EnsureStableOrdering ?? true);
40+
querySettings?.ODataSettings?.EnableConstantParameterization ?? true);
4341
await query.ApplyOptionsAsync(mapper, filter, options, querySettings);
4442
return await query.GetAsync
4543
(
@@ -57,8 +55,7 @@ public static async Task<IQueryable<TModel>> GetQueryAsync<TModel, TData>(this I
5755
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>(
5856
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
5957
querySettings?.ODataSettings?.TimeZone,
60-
querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
61-
querySettings?.ODataSettings?.EnsureStableOrdering ?? true);
58+
querySettings?.ODataSettings?.EnableConstantParameterization ?? true);
6259

6360
await query.ApplyOptionsAsync(mapper, filter, options, querySettings);
6461
return query.GetQueryable(mapper, options, querySettings, filter);
@@ -70,8 +67,7 @@ public static IQueryable<TModel> GetQuery<TModel, TData>(this IQueryable<TData>
7067
Expression<Func<TModel, bool>> filter = options.ToFilterExpression<TModel>(
7168
querySettings?.ODataSettings?.HandleNullPropagation ?? HandleNullPropagationOption.False,
7269
querySettings?.ODataSettings?.TimeZone,
73-
querySettings?.ODataSettings?.EnableConstantParameterization ?? true,
74-
querySettings?.ODataSettings?.EnsureStableOrdering ?? true);
70+
querySettings?.ODataSettings?.EnableConstantParameterization ?? true);
7571
query.ApplyOptions(mapper, filter, options, querySettings);
7672
return query.GetQueryable(mapper, options, querySettings, filter);
7773
}

‎AutoMapper.OData.EFCore.Tests/GetQueryTests.cs‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -897,10 +897,10 @@ void Test(IQueryable<CoreBuilding> queryable)
897897
}
898898

899899
[Fact]
900-
public void BuildingsFilterNameDisableStableOrdering()
900+
public void BuildingsFilterNameWithTopAndDisabledAlwaysSortByPrimaryKey()
901901
{
902902
string query = "/corebuilding?$filter=contains(Name, 'Two L2')&$top=10";
903-
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { EnsureStableOrdering = false } }));
903+
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { AlwaysSortByPrimaryKey = false } }));
904904

905905
void Test(IQueryable<CoreBuilding> queryable)
906906
{
@@ -911,10 +911,10 @@ void Test(IQueryable<CoreBuilding> queryable)
911911
}
912912

913913
[Fact]
914-
public void BuildingsFilterNameEnableStableOrdering()
914+
public void BuildingsFilterNameWithTopAndEnabledAlwaysSortByPrimaryKey()
915915
{
916916
string query = "/corebuilding?$filter=contains(Name, 'Two L2')&$top=10";
917-
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { EnsureStableOrdering = true } }));
917+
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { AlwaysSortByPrimaryKey = true } }));
918918

919919
void Test(IQueryable<CoreBuilding> queryable)
920920
{
@@ -924,6 +924,34 @@ void Test(IQueryable<CoreBuilding> queryable)
924924
}
925925
}
926926

927+
[Fact]
928+
public void BuildingsFilterNameWithSkipAndDisabledAlwaysSortByPrimaryKey()
929+
{
930+
string query = "/corebuilding?$filter=contains(Name, 'Two L2')&$skip=10";
931+
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { AlwaysSortByPrimaryKey = false } }));
932+
933+
void Test(IQueryable<CoreBuilding> queryable)
934+
{
935+
string sqlQuery = queryable.ToQueryString();
936+
Assert.Contains("OFFSET", sqlQuery);
937+
Assert.DoesNotContain("ORDER BY [o].[Identifier]", sqlQuery);
938+
}
939+
}
940+
941+
[Fact]
942+
public void BuildingsFilterNameWithSkipAndEnabledAlwaysSortByPrimaryKey()
943+
{
944+
string query = "/corebuilding?$filter=contains(Name, 'Two L2')&$skip=10";
945+
Test(GetQuery<CoreBuilding, TBuilding>(query, querySettings: new() { ODataSettings = new() { AlwaysSortByPrimaryKey = true } }));
946+
947+
void Test(IQueryable<CoreBuilding> queryable)
948+
{
949+
string sqlQuery = queryable.ToQueryString();
950+
Assert.Contains("OFFSET", sqlQuery);
951+
Assert.Contains("ORDER BY [o].[Identifier]", sqlQuery);
952+
}
953+
}
954+
927955
[Fact]
928956
public async Task OpsTenantOrderByCountOfReference()
929957
{

0 commit comments

Comments
 (0)