Skip to content

Commit 6a39a74

Browse files
Fix $expand with null properties
1 parent fb8c212 commit 6a39a74

File tree

2 files changed

+247
-2
lines changed

2 files changed

+247
-2
lines changed

src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,81 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
427427
return Expression.MemberInit(Expression.New(wrapperType), wrapperTypeMemberAssignments);
428428
}
429429

430+
// Generates the expression
431+
// { Instance = new Customer() {Id = $it.Id, Name= $it.Name}}
432+
private static Expression RemoveNonStructucalProperties(QueryBinderContext context, Expression source, IEdmStructuredType structuredType)
433+
{
434+
IEdmModel model = context.Model;
435+
Expression updatedSource = null;
436+
437+
Type elementType = source.Type;
438+
IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
439+
440+
PropertyInfo[] props = elementType.GetProperties();
441+
List<MemberBinding> bindings = new List<MemberBinding>();
442+
ODataQuerySettings settings = context.QuerySettings;
443+
bool isSourceNullable = ExpressionBinderHelper.IsNullable(source.Type);
444+
445+
foreach (PropertyInfo prop in props)
446+
{
447+
bool isPropertyNullable = ExpressionBinderHelper.IsNullable(prop.PropertyType);
448+
449+
foreach (var sp in structuralProperties)
450+
{
451+
if (prop.CanWrite && model.GetClrPropertyName(sp) == prop.Name)
452+
{
453+
Expression propertyValue = Expression.Property(source, prop);
454+
Type nullablePropertyType = TypeHelper.ToNullable(propertyValue.Type);
455+
Expression nullablePropertyValue = ExpressionHelpers.ToNullable(propertyValue);
456+
457+
if ((settings.HandleNullPropagation == HandleNullPropagationOption.True) || (isSourceNullable && isPropertyNullable))
458+
{
459+
propertyValue = Expression.Condition(
460+
test: Expression.Equal(source, Expression.Constant(value: null)),
461+
ifTrue: Expression.Constant(value: null, type: nullablePropertyType),
462+
ifFalse: nullablePropertyValue);
463+
}
464+
else if (isSourceNullable)
465+
{
466+
object defaultValue = prop.PropertyType.IsValueType ? Activator.CreateInstance(prop.PropertyType) : null;
467+
468+
// property is non-nullable
469+
propertyValue = Expression.Condition(
470+
test: Expression.Equal(source, Expression.Constant(value: null)),
471+
ifTrue: Expression.Constant(value: defaultValue),
472+
ifFalse: propertyValue);
473+
}
474+
else if (isPropertyNullable)
475+
{
476+
propertyValue = nullablePropertyValue;
477+
}
478+
479+
bindings.Add(Expression.Bind(prop, propertyValue));
480+
break;
481+
}
482+
}
483+
}
484+
485+
NewExpression newExpression = Expression.New(source.Type);
486+
updatedSource = Expression.MemberInit(newExpression, bindings);
487+
488+
return updatedSource;
489+
}
490+
491+
private bool IsEfQueryProvider(QueryBinderContext context)
492+
{
493+
if (context.QueryProvider != null &&
494+
context.QueryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
495+
context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
496+
context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
497+
context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
498+
{
499+
return true;
500+
}
501+
502+
return false;
503+
}
504+
430505
private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,
431506
out IList<string> computedProperties)
432507
{

test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs

Lines changed: 172 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Collections.Generic;
1111
using System.ComponentModel;
1212
using System.ComponentModel.DataAnnotations;
13+
using System.ComponentModel.DataAnnotations.Schema;
1314
using System.Linq;
1415
using System.Linq.Expressions;
1516
using System.Reflection;
@@ -36,6 +37,7 @@ public class SelectExpandBinderTest
3637
private readonly SelectExpandBinder _binder;
3738
private readonly SelectExpandBinder _binder_lowerCamelCased;
3839
private readonly IQueryable<QueryCustomer> _queryable;
40+
private readonly IQueryable<QueryCustomer> _queryable1;
3941
private readonly IQueryable<QueryCustomer> _queryable_lowerCamelCased;
4042
private readonly ODataQueryContext _context;
4143
private readonly ODataQueryContext _context_lowerCamelCased;
@@ -82,7 +84,19 @@ public SelectExpandBinderTest()
8284
QueryOrder order = new QueryOrder { Id = 42, Title = "The order", Customer = customer };
8385
customer.Orders.Add(order);
8486

87+
QueryCustomer customer1 = new QueryCustomer
88+
{
89+
Orders = new List<QueryOrder>()
90+
};
91+
92+
QueryOrder order1 = new QueryOrder { Id = 42, Title = "The order", Customer = customer1 };
93+
QueryOrder order2 = null;
94+
95+
customer1.Orders.Add(order1);
96+
customer1.Orders.Add(order2);
97+
8598
_queryable = new[] { customer }.AsQueryable();
99+
_queryable1 = new[] { customer1 }.AsQueryable();
86100

87101
SelectExpandQueryOption selectExpandQueryOption = new SelectExpandQueryOption("Orders", expand: null, context: _context);
88102

@@ -257,6 +271,138 @@ public void Bind_GeneratedExpression_ContainsExpandedObject()
257271
Assert.Same(_queryable.First(), innerInnerCustomer.Instance);
258272
}
259273

274+
[Theory]
275+
[InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)]
276+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)]
277+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)]
278+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)]
279+
public void Bind_UsingEFQueryProvider_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider)
280+
{
281+
// Arrange
282+
SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context);
283+
284+
// Act
285+
SelectExpandBinder binder = new SelectExpandBinder();
286+
_queryBinderContext.QueryProvider = queryProvider;
287+
IQueryable queryable = binder.ApplyBind(_queryable1, selectExpand.SelectExpandClause, _queryBinderContext);
288+
289+
// Assert
290+
IEnumerator enumerator = queryable.GetEnumerator();
291+
Assert.True(enumerator.MoveNext());
292+
var partialCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(enumerator.Current);
293+
Assert.False(enumerator.MoveNext());
294+
Assert.Null(partialCustomer.Instance);
295+
Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType);
296+
IEnumerable<SelectExpandWrapper<QueryOrder>> innerOrders = partialCustomer.Container
297+
.ToDictionary(PropertyMapper)["Orders"] as IEnumerable<SelectExpandWrapper<QueryOrder>>;
298+
Assert.NotNull(innerOrders);
299+
SelectExpandWrapper<QueryOrder> partialOrder = innerOrders.FirstOrDefault();
300+
301+
// We only write structural properties to the instance.
302+
// This means that navigation properties on the instance property will be null
303+
// when using any instance of EF query provider, and all structural properties
304+
// will be assigned.
305+
Assert.Null(partialOrder.Instance.Customer);
306+
Assert.NotEqual(0, partialOrder.Instance.Id);
307+
Assert.NotNull(partialOrder.Instance.Title);
308+
309+
object customer = partialOrder.Container.ToDictionary(PropertyMapper)["Customer"];
310+
SelectExpandWrapper<QueryCustomer> innerInnerCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(customer);
311+
312+
Assert.Null(innerInnerCustomer.Instance.Orders);
313+
Assert.Equal(2, innerInnerCustomer.Instance.TestReadonlyProperty.Count);
314+
Assert.Equal("Test1", innerInnerCustomer.Instance.TestReadonlyProperty[0]);
315+
Assert.Equal("Test2", innerInnerCustomer.Instance.TestReadonlyProperty[1]);
316+
Assert.Equal(2, innerInnerCustomer.Instance.TestReadOnlyWithAttribute);
317+
}
318+
319+
[Theory]
320+
[InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)]
321+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)]
322+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)]
323+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)]
324+
public void Bind_UsingEFQueryProvider_LowerCamelCasedModel_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider)
325+
{
326+
// Arrange
327+
SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context_lowerCamelCased);
328+
329+
// Act
330+
SelectExpandBinder binder = new SelectExpandBinder();
331+
_queryBinderContext_lowerCamelCased.QueryProvider = queryProvider;
332+
IQueryable queryable = binder.ApplyBind(_queryable_lowerCamelCased, selectExpand.SelectExpandClause, _queryBinderContext_lowerCamelCased);
333+
334+
// Assert
335+
IEnumerator enumerator = queryable.GetEnumerator();
336+
Assert.True(enumerator.MoveNext());
337+
var partialCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(enumerator.Current);
338+
Assert.False(enumerator.MoveNext());
339+
Assert.Null(partialCustomer.Instance);
340+
Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType);
341+
IEnumerable<SelectExpandWrapper<QueryOrder>> innerOrders = partialCustomer.Container
342+
.ToDictionary(PropertyMapper)["orders"] as IEnumerable<SelectExpandWrapper<QueryOrder>>;
343+
Assert.NotNull(innerOrders);
344+
SelectExpandWrapper<QueryOrder> partialOrder = innerOrders.FirstOrDefault();
345+
346+
// We only write structural properties to the instance.
347+
// This means that navigation properties on the instance property will be null
348+
// when using any instance of EF query provider, and all structural properties
349+
// will be assigned.
350+
Assert.Null(partialOrder.Instance.Customer);
351+
Assert.NotEqual(0, partialOrder.Instance.Id);
352+
Assert.NotNull(partialOrder.Instance.Title);
353+
354+
object customer = partialOrder.Container.ToDictionary(PropertyMapper)["customer"];
355+
SelectExpandWrapper<QueryCustomer> innerInnerCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(customer);
356+
357+
Assert.Null(innerInnerCustomer.Instance.Orders);
358+
}
359+
360+
[Fact]
361+
public void Bind_SelectAndExpand_WithNullProperties_DoesNotThrowException()
362+
{
363+
// Arrange
364+
IQueryable<User> users;
365+
string expand = "FileRefNavigation";
366+
367+
User user = new User
368+
{
369+
UserId = 1,
370+
Name = "Alex",
371+
Age = 35,
372+
DataFileRef = null,
373+
FileRefNavigation = null
374+
};
375+
376+
users = new[] { user }.AsQueryable();
377+
ODataQueryContext context = new ODataQueryContext(_model, typeof(User)) { RequestContainer = new MockServiceProvider() };
378+
379+
SelectExpandQueryOption selectExpand = new SelectExpandQueryOption(select: null, expand: expand, context: context);
380+
381+
QueryBinderContext queryBinderContext = new QueryBinderContext(_model, _settings, selectExpand.Context.ElementClrType)
382+
{
383+
NavigationSource = context.NavigationSource
384+
};
385+
386+
queryBinderContext.QueryProvider = HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace;
387+
388+
// Act
389+
SelectExpandBinder binder = new SelectExpandBinder();
390+
IQueryable queryable = binder.ApplyBind(users, selectExpand.SelectExpandClause, queryBinderContext);
391+
392+
// Assert
393+
Assert.NotNull(queryable);
394+
395+
IEnumerator enumerator = queryable.GetEnumerator();
396+
Assert.True(enumerator.MoveNext());
397+
var usr = Assert.IsAssignableFrom<SelectExpandWrapper<User>>(enumerator.Current);
398+
Assert.False(enumerator.MoveNext());
399+
Assert.NotNull(usr.Instance);
400+
Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.User", usr.Instance.GetType().ToString());
401+
IEnumerable<SelectExpandWrapper<DataFile>> fileRefsNavigations = usr.Container
402+
.ToDictionary(PropertyMapper)["FileRefNavigation"] as IEnumerable<SelectExpandWrapper<DataFile>>;
403+
Assert.Null(fileRefsNavigations);
404+
}
405+
260406
[Fact]
261407
public void Bind_GeneratedExpression_CheckNullObjectWithinChainProjectionByKey()
262408
{
@@ -1996,6 +2142,7 @@ public static IEdmModel GetEdmModel()
19962142
builder.EntitySet<QueryOrder>("Orders");
19972143
builder.EntitySet<QueryCity>("Cities");
19982144
builder.EntitySet<QueryProduct>("Products");
2145+
builder.EntitySet<User>("Users");
19992146

20002147
customer.Collection.Function("IsUpgraded").Returns<bool>().Namespace="NS";
20012148
customer.Collection.Action("UpgradeAll").Namespace = "NS";
@@ -2143,9 +2290,9 @@ public class QueryCustomer
21432290

21442291
public IList<QueryAddress> Addresses { get; set; }
21452292

2146-
public QueryOrder PrivateOrder { get; set; }
2293+
public QueryOrder? PrivateOrder { get; set; }
21472294

2148-
public IList<QueryOrder> Orders { get; set; }
2295+
public IList<QueryOrder>? Orders { get; set; }
21492296

21502297
public List<string> TestReadonlyProperty
21512298
{
@@ -2206,4 +2353,27 @@ public enum QueryColor
22062353

22072354
Blue
22082355
}
2356+
2357+
public class User
2358+
{
2359+
[Key]
2360+
public int UserId { get; set; }
2361+
[Required]
2362+
public string Name { get; set; }
2363+
[Required]
2364+
public int Age { get; set; }
2365+
2366+
//Navigations
2367+
[ForeignKey("FileRefNavigation")]
2368+
public int? DataFileRef { get; set; }
2369+
public DataFile? FileRefNavigation { get; set; }
2370+
}
2371+
2372+
public class DataFile
2373+
{
2374+
[Key]
2375+
public int FileId { get; set; }
2376+
[Required]
2377+
public string FileName { get; set; }
2378+
}
22092379
}

0 commit comments

Comments
 (0)