Skip to content

Commit fafad0a

Browse files
ificatorxuzhg
authored andcommitted
Make sure 'top' doesn't apply on a null collection
1 parent 4dda969 commit fafad0a

2 files changed

Lines changed: 46 additions & 9 deletions

File tree

src/Microsoft.AspNet.OData.Shared/Query/Expressions/SelectExpandBinder.cs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -978,8 +978,12 @@ private Expression GetNullCheckExpression(IEdmNavigationProperty propertyToExpan
978978

979979
// new CollectionWrapper<ElementType> { Instance = source.Select((ElementType element) => new Wrapper { }) }
980980
[SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity", Justification = "These are simple conversion function and cannot be split up.")]
981-
private Expression ProjectCollection(Expression source, Type elementType,
982-
SelectExpandClause selectExpandClause, IEdmStructuredType structuredType, IEdmNavigationSource navigationSource,
981+
private Expression ProjectCollection(
982+
Expression source,
983+
Type elementType,
984+
SelectExpandClause selectExpandClause,
985+
IEdmStructuredType structuredType,
986+
IEdmNavigationSource navigationSource,
983987
OrderByClause orderByClause,
984988
long? topOption,
985989
long? skipOption,
@@ -1005,9 +1009,10 @@ private Expression ProjectCollection(Expression source, Type elementType,
10051009
// (ElementType element) => new Wrapper { }
10061010
LambdaExpression selector = Expression.Lambda(projection, element);
10071011

1012+
Expression modifiedSource = source;
10081013
if (orderByClause != null)
10091014
{
1010-
source = AddOrderByQueryForSource(source, orderByClause, elementType);
1015+
modifiedSource = AddOrderByQueryForSource(modifiedSource, orderByClause, elementType);
10111016
}
10121017

10131018
bool hasTopValue = topOption != null && topOption.HasValue;
@@ -1033,7 +1038,7 @@ private Expression ProjectCollection(Expression source, Type elementType,
10331038
bool alreadyOrdered = false;
10341039
foreach (var prop in properties)
10351040
{
1036-
source = ExpressionHelpers.OrderByPropertyExpression(source, prop.Name, elementType, alreadyOrdered);
1041+
modifiedSource = ExpressionHelpers.OrderByPropertyExpression(modifiedSource, prop.Name, elementType, alreadyOrdered);
10371042
if (!alreadyOrdered)
10381043
{
10391044
alreadyOrdered = true;
@@ -1046,14 +1051,14 @@ private Expression ProjectCollection(Expression source, Type elementType,
10461051
if (hasSkipvalue)
10471052
{
10481053
Contract.Assert(skipOption.Value <= Int32.MaxValue);
1049-
source = ExpressionHelpers.Skip(source, (int)skipOption.Value, elementType,
1054+
modifiedSource = ExpressionHelpers.Skip(modifiedSource, (int)skipOption.Value, elementType,
10501055
_settings.EnableConstantParameterization);
10511056
}
10521057

10531058
if (hasTopValue)
10541059
{
10551060
Contract.Assert(topOption.Value <= Int32.MaxValue);
1056-
source = ExpressionHelpers.Take(source, (int)topOption.Value, elementType,
1061+
modifiedSource = ExpressionHelpers.Take(modifiedSource, (int)topOption.Value, elementType,
10571062
_settings.EnableConstantParameterization);
10581063
}
10591064

@@ -1066,12 +1071,12 @@ private Expression ProjectCollection(Expression source, Type elementType,
10661071
{
10671072
if (_settings.PageSize.HasValue)
10681073
{
1069-
source = ExpressionHelpers.Take(source, _settings.PageSize.Value + 1, elementType,
1074+
modifiedSource = ExpressionHelpers.Take(modifiedSource, _settings.PageSize.Value + 1, elementType,
10701075
_settings.EnableConstantParameterization);
10711076
}
10721077
else if (_settings.ModelBoundPageSize.HasValue)
10731078
{
1074-
source = ExpressionHelpers.Take(source, modelBoundPageSize.Value + 1, elementType,
1079+
modifiedSource = ExpressionHelpers.Take(modifiedSource, modelBoundPageSize.Value + 1, elementType,
10751080
_settings.EnableConstantParameterization);
10761081
}
10771082
}
@@ -1081,7 +1086,7 @@ private Expression ProjectCollection(Expression source, Type elementType,
10811086
// expression
10821087
// source.Select((ElementType element) => new Wrapper { })
10831088
var selectMethod = GetSelectMethod(elementType, projection.Type);
1084-
Expression selectedExpresion = Expression.Call(selectMethod, source, selector);
1089+
Expression selectedExpresion = Expression.Call(selectMethod, modifiedSource, selector);
10851090

10861091
// Append ToList() to collection as a hint to LINQ provider to buffer correlated subqueries in memory and avoid executing N+1 queries
10871092
if (_settings.EnableCorrelatedSubqueryBuffering)

test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/SelectExpandBinderTest.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,38 @@ public void ProjectAsWrapper_NullExpandedProperty_HasNullValueInProjectedWrapper
289289
Assert.Null(projectCustomer.Instance);
290290
}
291291

292+
[Fact]
293+
public void ProjectAsWrapper_Collection_TopOnNullExpand()
294+
{
295+
// Arrange
296+
_settings.HandleNullPropagation = HandleNullPropagationOption.True;
297+
298+
IEdmEntityType vipCustomer = _model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryVipCustomer");
299+
IEdmNavigationProperty specialOrdersNav = vipCustomer.DeclaredNavigationProperties().Single(c => c.Name == "SpecialOrders");
300+
ExpandedNavigationSelectItem expandItem = new ExpandedNavigationSelectItem(
301+
new ODataExpandPath(new NavigationPropertySegment(specialOrdersNav, navigationSource: _orders)),
302+
_orders,
303+
selectAndExpand: null,
304+
filterOption: null,
305+
orderByOption: null,
306+
topOption: 10,
307+
skipOption: null,
308+
countOption: null,
309+
searchOption: null,
310+
levelsOption: null);
311+
SelectExpandClause selectExpand = new SelectExpandClause(new SelectItem[] { expandItem }, allSelected: true);
312+
313+
QueryVipCustomer customer = new QueryVipCustomer();
314+
Expression source = Expression.Constant(customer);
315+
316+
// Act
317+
Expression projection = _binder.ProjectAsWrapper(source, selectExpand, vipCustomer, _customers);
318+
319+
// Assert
320+
IEnumerable<SelectExpandWrapper<QueryOrder>> projectedOrders = Expression.Lambda(projection).Compile().DynamicInvoke() as IEnumerable<SelectExpandWrapper<QueryOrder>>;
321+
Assert.Null(projectedOrders);
322+
}
323+
292324
[Fact]
293325
public void ProjectAsWrapper_Collection_ProjectedValueNullAndHandleNullPropagationTrue()
294326
{

0 commit comments

Comments
 (0)