Skip to content

Refactor the state of order expressions as OneOrMany. #509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2025
Merged

Refactor the state of order expressions as OneOrMany. #509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 70 additions & 50 deletions src/Ardalis.Specification/Evaluators/OrderEvaluator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,79 +9,99 @@ private OrderEvaluator() { }

public IQueryable<T> GetQuery<T>(IQueryable<T> query, ISpecification<T> specification) where T : class
{
if (specification.OrderExpressions != null)
if (specification is Specification<T> spec)
{
if (specification.OrderExpressions.Count(x => x.OrderType == OrderTypeEnum.OrderBy
|| x.OrderType == OrderTypeEnum.OrderByDescending) > 1)
if (spec.OneOrManyOrderExpressions.IsEmpty) return query;
if (spec.OneOrManyOrderExpressions.SingleOrDefault is { } orderExpression)
{
throw new DuplicateOrderChainException();
return orderExpression.OrderType switch
{
OrderTypeEnum.OrderBy => query.OrderBy(orderExpression.KeySelector),
OrderTypeEnum.OrderByDescending => query.OrderByDescending(orderExpression.KeySelector),
_ => query
};
}
}

IOrderedQueryable<T>? orderedQuery = null;
foreach (var orderExpression in specification.OrderExpressions)
IOrderedQueryable<T>? orderedQuery = null;
var chainCount = 0;
foreach (var orderExpression in specification.OrderExpressions)
{
if (orderExpression.OrderType == OrderTypeEnum.OrderBy)
{
if (orderExpression.OrderType == OrderTypeEnum.OrderBy)
{
orderedQuery = query.OrderBy(orderExpression.KeySelector);
}
else if (orderExpression.OrderType == OrderTypeEnum.OrderByDescending)
{
orderedQuery = query.OrderByDescending(orderExpression.KeySelector);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenBy)
{
orderedQuery = orderedQuery!.ThenBy(orderExpression.KeySelector);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenByDescending)
{
orderedQuery = orderedQuery!.ThenByDescending(orderExpression.KeySelector);
}
chainCount++;
if (chainCount == 2) throw new DuplicateOrderChainException();
orderedQuery = query.OrderBy(orderExpression.KeySelector);
}

if (orderedQuery != null)
else if (orderExpression.OrderType == OrderTypeEnum.OrderByDescending)
{
query = orderedQuery;
chainCount++;
if (chainCount == 2) throw new DuplicateOrderChainException();
orderedQuery = query.OrderByDescending(orderExpression.KeySelector);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenBy)
{
orderedQuery = orderedQuery!.ThenBy(orderExpression.KeySelector);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenByDescending)
{
orderedQuery = orderedQuery!.ThenByDescending(orderExpression.KeySelector);
}
}

if (orderedQuery is not null)
{
query = orderedQuery;
}

return query;
}

public IEnumerable<T> Evaluate<T>(IEnumerable<T> query, ISpecification<T> specification)
{
if (specification.OrderExpressions != null)
if (specification is Specification<T> spec)
{
if (specification.OrderExpressions.Count(x => x.OrderType == OrderTypeEnum.OrderBy
|| x.OrderType == OrderTypeEnum.OrderByDescending) > 1)
if (spec.OneOrManyOrderExpressions.IsEmpty) return query;
if (spec.OneOrManyOrderExpressions.SingleOrDefault is { } orderExpression)
{
throw new DuplicateOrderChainException();
return orderExpression.OrderType switch
{
OrderTypeEnum.OrderBy => query.OrderBy(orderExpression.KeySelectorFunc),
OrderTypeEnum.OrderByDescending => query.OrderByDescending(orderExpression.KeySelectorFunc),
_ => query
};
}
}

IOrderedEnumerable<T>? orderedQuery = null;
foreach (var orderExpression in specification.OrderExpressions)
IOrderedEnumerable<T>? orderedQuery = null;
var chainCount = 0;
foreach (var orderExpression in specification.OrderExpressions)
{
if (orderExpression.OrderType == OrderTypeEnum.OrderBy)
{
if (orderExpression.OrderType == OrderTypeEnum.OrderBy)
{
orderedQuery = query.OrderBy(orderExpression.KeySelectorFunc);
}
else if (orderExpression.OrderType == OrderTypeEnum.OrderByDescending)
{
orderedQuery = query.OrderByDescending(orderExpression.KeySelectorFunc);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenBy)
{
orderedQuery = orderedQuery!.ThenBy(orderExpression.KeySelectorFunc);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenByDescending)
{
orderedQuery = orderedQuery!.ThenByDescending(orderExpression.KeySelectorFunc);
}
chainCount++;
if (chainCount == 2) throw new DuplicateOrderChainException();
orderedQuery = query.OrderBy(orderExpression.KeySelectorFunc);
}

if (orderedQuery != null)
else if (orderExpression.OrderType == OrderTypeEnum.OrderByDescending)
{
query = orderedQuery;
chainCount++;
if (chainCount == 2) throw new DuplicateOrderChainException();
orderedQuery = query.OrderByDescending(orderExpression.KeySelectorFunc);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenBy)
{
orderedQuery = orderedQuery!.ThenBy(orderExpression.KeySelectorFunc);
}
else if (orderExpression.OrderType == OrderTypeEnum.ThenByDescending)
{
orderedQuery = orderedQuery!.ThenByDescending(orderExpression.KeySelectorFunc);
}
}

if (orderedQuery is not null)
{
query = orderedQuery;
}

return query;
Expand Down
12 changes: 6 additions & 6 deletions src/Ardalis.Specification/Specification.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class Specification<T, TResult> : Specification<T>, ISpecification<T, TRe
public class Specification<T> : ISpecification<T>
{
private const int DEFAULT_CAPACITY_SEARCH = 2;
private const int DEFAULT_CAPACITY_ORDER = 2;
private const int DEFAULT_CAPACITY_INCLUDE = 2;
private const int DEFAULT_CAPACITY_INCLUDESTRING = 1;

Expand All @@ -43,7 +42,7 @@ public class Specification<T> : ISpecification<T>
// This will be reconsidered for version 10 where we may store the whole state as a single array of structs.
private OneOrMany<WhereExpressionInfo<T>> _whereExpressions = new();
private List<SearchExpressionInfo<T>>? _searchExpressions;
private List<OrderExpressionInfo<T>>? _orderExpressions;
private OneOrMany<OrderExpressionInfo<T>> _orderExpressions = new();
private List<IncludeExpressionInfo>? _includeExpressions;
private List<string>? _includeStrings;
private Dictionary<string, object>? _items;
Expand Down Expand Up @@ -94,7 +93,7 @@ public class Specification<T> : ISpecification<T>

// Specs are not intended to be thread-safe, so we don't need to worry about thread-safety here.
internal void Add(WhereExpressionInfo<T> whereExpression) => _whereExpressions.Add(whereExpression);
internal void Add(OrderExpressionInfo<T> orderExpression) => (_orderExpressions ??= new(DEFAULT_CAPACITY_ORDER)).Add(orderExpression);
internal void Add(OrderExpressionInfo<T> orderExpression) => _orderExpressions.Add(orderExpression);
internal void Add(IncludeExpressionInfo includeExpression) => (_includeExpressions ??= new(DEFAULT_CAPACITY_INCLUDE)).Add(includeExpression);
internal void Add(string includeString) => (_includeStrings ??= new(DEFAULT_CAPACITY_INCLUDESTRING)).Add(includeString);
internal void Add(SearchExpressionInfo<T> searchExpression)
Expand Down Expand Up @@ -130,7 +129,7 @@ internal void Add(SearchExpressionInfo<T> searchExpression)
public IEnumerable<SearchExpressionInfo<T>> SearchCriterias => _searchExpressions ?? Enumerable.Empty<SearchExpressionInfo<T>>();

/// <inheritdoc/>
public IEnumerable<OrderExpressionInfo<T>> OrderExpressions => _orderExpressions ?? Enumerable.Empty<OrderExpressionInfo<T>>();
public IEnumerable<OrderExpressionInfo<T>> OrderExpressions => _orderExpressions.Values;

/// <inheritdoc/>
public IEnumerable<IncludeExpressionInfo> IncludeExpressions => _includeExpressions ?? Enumerable.Empty<IncludeExpressionInfo>();
Expand All @@ -142,6 +141,7 @@ internal void Add(SearchExpressionInfo<T> searchExpression)
public IEnumerable<string> QueryTags => _queryTags.Values;

internal OneOrMany<WhereExpressionInfo<T>> OneOrManyWhereExpressions => _whereExpressions;
internal OneOrMany<OrderExpressionInfo<T>> OneOrManyOrderExpressions => _orderExpressions;
internal OneOrMany<string> OneOrManyQueryTags => _queryTags;

/// <inheritdoc/>
Expand Down Expand Up @@ -189,9 +189,9 @@ void ISpecification<T>.CopyTo(Specification<T> otherSpec)
otherSpec._includeStrings = _includeStrings.ToList();
}

if (_orderExpressions is not null)
if (!_orderExpressions.IsEmpty)
{
otherSpec._orderExpressions = _orderExpressions.ToList();
otherSpec._orderExpressions = _orderExpressions.Clone();
}

if (_searchExpressions is not null)
Expand Down
33 changes: 32 additions & 1 deletion tests/Ardalis.Specification.Tests/Evaluators/OrderEvaluatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public class OrderEvaluatorTests
public record Customer(int Id, string? Name = null);

[Fact]
public void ThrowsDuplicateOrderChainException_GivenMultipleOrderChains()
public void ThrowsDuplicateOrderChainException_GivenMultipleOrderByChains()
{
List<Customer> input = [new(3), new(1), new(2), new(5), new(4)];
List<Customer> expected = [new(1), new(2), new(3), new(4), new(5)];
Expand All @@ -24,6 +24,24 @@ public void ThrowsDuplicateOrderChainException_GivenMultipleOrderChains()
sut2.Should().Throw<DuplicateOrderChainException>();
}

[Fact]
public void ThrowsDuplicateOrderChainException_GivenMultipleOrderByDescendingChains()
{
List<Customer> input = [new(3), new(1), new(2), new(5), new(4)];
List<Customer> expected = [new(1), new(2), new(3), new(4), new(5)];

var spec = new Specification<Customer>();
spec.Query
.OrderByDescending(x => x.Id)
.OrderByDescending(x => x.Name);

var sut1 = new Action(() => _evaluator.Evaluate(input, spec));
var sut2 = new Action(() => _evaluator.GetQuery(input.AsQueryable(), spec));

sut1.Should().Throw<DuplicateOrderChainException>();
sut2.Should().Throw<DuplicateOrderChainException>();
}

[Fact]
public void OrdersItemsAscending_GivenOrderBy()
{
Expand All @@ -50,6 +68,19 @@ public void OrdersItemsDescending_GivenOrderByDescending()
Assert(spec, input, expected);
}

[Fact]
public void DoesNothing_GivenInvalidRootChain()
{
List<Customer> input = [new(3), new(1), new(2), new(5), new(4)];
List<Customer> expected = [new(3), new(1), new(2), new(5), new(4)];

var spec = new Specification<Customer>();
var expr = new OrderExpressionInfo<Customer>(x => x.Id, OrderTypeEnum.ThenBy);
spec.Add(expr);

Assert(spec, input, expected);
}

[Fact]
public void OrdersItems_GivenOrderByThenBy()
{
Expand Down