diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs index d2bcca4c8..4b8c62705 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs @@ -277,6 +277,11 @@ public static IQueryable ApplyBind(this ISelectExpandBinder binder, IQueryable s throw Error.ArgumentNull(nameof(context)); } + if (string.IsNullOrEmpty(context.QueryProvider)) + { + context.QueryProvider = source.Provider.GetType().Namespace; + } + Type elementType = context.ElementClrType; LambdaExpression projectionLambda = binder.BindSelectExpand(selectExpandClause, context) as LambdaExpression; diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs index 5a3640bef..fdc8c6983 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs @@ -107,6 +107,8 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe ElementType = Model.GetEdmTypeReference(ElementClrType)?.Definition; + QueryProvider = context.QueryProvider; + if (ElementType == null) { throw new ODataException(Error.Format(SRResources.ClrTypeNotInModel, ElementClrType.FullName)); @@ -175,6 +177,11 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe /// public Expression Source { get;set; } + /// + /// Gets or sets the query provider. + /// + internal string QueryProvider { get; set; } + /// /// Gets the parameter using parameter name. /// diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs index c7d05325c..ec4516054 100644 --- a/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs +++ b/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs @@ -368,9 +368,23 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source bool isSelectedAll = IsSelectAll(selectExpandClause); if (isSelectedAll) { + // If we have an EF query provider, then we remove the non-structural properties. The reason for this is to avoid + // Creating an expression that will generate duplicate LEFT joins when a LEFT join already exists in the IQueryable + // as observed here: https://github.com/OData/AspNetCoreOData/issues/497 + + Expression updatedExpression = null; + if (IsEfQueryProvider(context)) + { + updatedExpression = RemoveNonStructuralProperties(context, source, structuredType); + } + else + { + updatedExpression = source; + } + // Initialize property 'Instance' on the wrapper class wrapperProperty = wrapperType.GetProperty("Instance"); - wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, source)); + wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, updatedExpression)); wrapperProperty = wrapperType.GetProperty("UseInstanceForProperties"); wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(true))); @@ -427,6 +441,81 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source return Expression.MemberInit(Expression.New(wrapperType), wrapperTypeMemberAssignments); } + // Generates the expression + // { Instance = new Customer() {Id = $it.Id, Name= $it.Name}} + private static Expression RemoveNonStructuralProperties(QueryBinderContext context, Expression source, IEdmStructuredType structuredType) + { + IEdmModel model = context.Model; + Expression updatedSource = null; + + Type elementType = source.Type; + IEnumerable structuralProperties = structuredType.StructuralProperties(); + + PropertyInfo[] props = elementType.GetProperties(); + List bindings = new List(); + ODataQuerySettings settings = context.QuerySettings; + bool isSourceNullable = ExpressionBinderHelper.IsNullable(source.Type); + + foreach (PropertyInfo prop in props) + { + bool isPropertyNullable = ExpressionBinderHelper.IsNullable(prop.PropertyType); + + foreach (var sp in structuralProperties) + { + if (prop.CanWrite && model.GetClrPropertyName(sp) == prop.Name) + { + Expression propertyValue = Expression.Property(source, prop); + Type nullablePropertyType = TypeHelper.ToNullable(propertyValue.Type); + Expression nullablePropertyValue = ExpressionHelpers.ToNullable(propertyValue); + + if ((settings.HandleNullPropagation == HandleNullPropagationOption.True) || (isSourceNullable && isPropertyNullable)) + { + propertyValue = Expression.Condition( + test: Expression.Equal(source, Expression.Constant(value: null)), + ifTrue: Expression.Constant(value: null, type: nullablePropertyType), + ifFalse: nullablePropertyValue); + } + else if (isSourceNullable) + { + object defaultValue = prop.PropertyType.IsValueType ? Activator.CreateInstance(prop.PropertyType) : null; + + // property is non-nullable + propertyValue = Expression.Condition( + test: Expression.Equal(source, Expression.Constant(value: null)), + ifTrue: Expression.Constant(value: defaultValue), + ifFalse: propertyValue); + } + else if (isPropertyNullable) + { + propertyValue = nullablePropertyValue; + } + + bindings.Add(Expression.Bind(prop, propertyValue)); + break; + } + } + } + + NewExpression newExpression = Expression.New(source.Type); + updatedSource = Expression.MemberInit(newExpression, bindings); + + return updatedSource; + } + + private bool IsEfQueryProvider(QueryBinderContext context) + { + if (context.QueryProvider != null && + context.QueryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace || + context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 || + context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 || + context.QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2) + { + return true; + } + + return false; + } + private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList dynamicPathSegments, bool isSelectedAll, out IList computedProperties) { diff --git a/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs b/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs index adae7f691..11b2d22af 100644 --- a/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs +++ b/test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs @@ -10,6 +10,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -36,6 +37,7 @@ public class SelectExpandBinderTest private readonly SelectExpandBinder _binder; private readonly SelectExpandBinder _binder_lowerCamelCased; private readonly IQueryable _queryable; + private readonly IQueryable _queryable1; private readonly IQueryable _queryable_lowerCamelCased; private readonly ODataQueryContext _context; private readonly ODataQueryContext _context_lowerCamelCased; @@ -82,7 +84,19 @@ public SelectExpandBinderTest() QueryOrder order = new QueryOrder { Id = 42, Title = "The order", Customer = customer }; customer.Orders.Add(order); + QueryCustomer customer1 = new QueryCustomer + { + Orders = new List() + }; + + QueryOrder order1 = new QueryOrder { Id = 42, Title = "The order", Customer = customer1 }; + QueryOrder order2 = null; + + customer1.Orders.Add(order1); + customer1.Orders.Add(order2); + _queryable = new[] { customer }.AsQueryable(); + _queryable1 = new[] { customer1 }.AsQueryable(); SelectExpandQueryOption selectExpandQueryOption = new SelectExpandQueryOption("Orders", expand: null, context: _context); @@ -257,6 +271,139 @@ public void Bind_GeneratedExpression_ContainsExpandedObject() Assert.Same(_queryable.First(), innerInnerCustomer.Instance); } + [Theory] + [InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)] + public void Bind_UsingEFQueryProvider_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider) + { + // Arrange + SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context); + + // Act + SelectExpandBinder binder = new SelectExpandBinder(); + _queryBinderContext.QueryProvider = queryProvider; + IQueryable queryable = binder.ApplyBind(_queryable1, selectExpand.SelectExpandClause, _queryBinderContext); + + // Assert + IEnumerator enumerator = queryable.GetEnumerator(); + Assert.True(enumerator.MoveNext()); + var partialCustomer = Assert.IsAssignableFrom>(enumerator.Current); + Assert.False(enumerator.MoveNext()); + Assert.Null(partialCustomer.Instance); + Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType); + IEnumerable> innerOrders = partialCustomer.Container + .ToDictionary(PropertyMapper)["Orders"] as IEnumerable>; + Assert.NotNull(innerOrders); + SelectExpandWrapper partialOrder = innerOrders.FirstOrDefault(); + + // We only write structural properties to the instance. + // This means that navigation properties on the instance property will be null + // when using any instance of EF query provider, and all structural properties + // will be assigned. + Assert.Null(partialOrder.Instance.Customer); + Assert.NotEqual(0, partialOrder.Instance.Id); + Assert.NotNull(partialOrder.Instance.Title); + + object customer = partialOrder.Container.ToDictionary(PropertyMapper)["Customer"]; + SelectExpandWrapper innerInnerCustomer = Assert.IsAssignableFrom>(customer); + + Assert.Null(innerInnerCustomer.Instance.Orders); + Assert.Equal(2, innerInnerCustomer.Instance.TestReadonlyProperty.Count); + Assert.Equal("Test1", innerInnerCustomer.Instance.TestReadonlyProperty[0]); + Assert.Equal("Test2", innerInnerCustomer.Instance.TestReadonlyProperty[1]); + Assert.Equal(2, innerInnerCustomer.Instance.TestReadOnlyWithAttribute); + } + + [Theory] + [InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)] + [InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)] + public void Bind_UsingEFQueryProvider_LowerCamelCasedModel_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider) + { + // Arrange + SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context_lowerCamelCased); + + // Act + SelectExpandBinder binder = new SelectExpandBinder(); + _queryBinderContext_lowerCamelCased.QueryProvider = queryProvider; + IQueryable queryable = binder.ApplyBind(_queryable_lowerCamelCased, selectExpand.SelectExpandClause, _queryBinderContext_lowerCamelCased); + + // Assert + IEnumerator enumerator = queryable.GetEnumerator(); + Assert.True(enumerator.MoveNext()); + var partialCustomer = Assert.IsAssignableFrom>(enumerator.Current); + Assert.False(enumerator.MoveNext()); + Assert.Null(partialCustomer.Instance); + Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType); + IEnumerable> innerOrders = partialCustomer.Container + .ToDictionary(PropertyMapper)["orders"] as IEnumerable>; + Assert.NotNull(innerOrders); + SelectExpandWrapper partialOrder = innerOrders.FirstOrDefault(); + + // We only write structural properties to the instance. + // This means that navigation properties on the instance property will be null + // when using any instance of EF query provider, and all structural properties + // will be assigned. + Assert.Null(partialOrder.Instance.Customer); + Assert.NotEqual(0, partialOrder.Instance.Id); + Assert.NotNull(partialOrder.Instance.Title); + + object customer = partialOrder.Container.ToDictionary(PropertyMapper)["customer"]; + SelectExpandWrapper innerInnerCustomer = Assert.IsAssignableFrom>(customer); + + Assert.Null(innerInnerCustomer.Instance.Orders); + } + + [Fact] + public void Bind_SelectAndExpand_WithNullProperties_DoesNotThrowException() + { + // Arrange + IQueryable users; + string expand = "FileRefNavigation"; + + User user = new User + { + UserId = 1, + Name = "Alex", + Age = 35, + DataFileRef = null, + FileRefNavigation = null + }; + + users = new[] { user }.AsQueryable(); + ODataQueryContext context = new ODataQueryContext(_model, typeof(User)) { RequestContainer = new MockServiceProvider() }; + + SelectExpandQueryOption selectExpand = new SelectExpandQueryOption(select: null, expand: expand, context: context); + + QueryBinderContext queryBinderContext = new QueryBinderContext(_model, _settings, selectExpand.Context.ElementClrType) + { + NavigationSource = context.NavigationSource + }; + + queryBinderContext.QueryProvider = HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace; + + // Act + SelectExpandBinder binder = new SelectExpandBinder(); + IQueryable queryable = binder.ApplyBind(users, selectExpand.SelectExpandClause, queryBinderContext); + + // Assert + Assert.NotNull(queryable); + + IEnumerator enumerator = queryable.GetEnumerator(); + Assert.True(enumerator.MoveNext()); + var usr = Assert.IsAssignableFrom>(enumerator.Current); + Assert.False(enumerator.MoveNext()); + Assert.NotNull(usr.Instance); + Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.User", usr.Instance.GetType().ToString()); + IEnumerable> fileRefsNavigations = usr.Container + .ToDictionary(PropertyMapper)["FileRefNavigation"] as IEnumerable>; + + Assert.Null(fileRefsNavigations); + } + [Fact] public void Bind_GeneratedExpression_CheckNullObjectWithinChainProjectionByKey() { @@ -1996,6 +2143,7 @@ public static IEdmModel GetEdmModel() builder.EntitySet("Orders"); builder.EntitySet("Cities"); builder.EntitySet("Products"); + builder.EntitySet("Users"); customer.Collection.Function("IsUpgraded").Returns().Namespace="NS"; customer.Collection.Action("UpgradeAll").Namespace = "NS"; @@ -2143,9 +2291,9 @@ public class QueryCustomer public IList Addresses { get; set; } - public QueryOrder PrivateOrder { get; set; } + public QueryOrder? PrivateOrder { get; set; } - public IList Orders { get; set; } + public IList? Orders { get; set; } public List TestReadonlyProperty { @@ -2206,4 +2354,27 @@ public enum QueryColor Blue } + + public class User + { + [Key] + public int UserId { get; set; } + [Required] + public string Name { get; set; } + [Required] + public int Age { get; set; } + + //Navigations + [ForeignKey("FileRefNavigation")] + public int? DataFileRef { get; set; } + public DataFile? FileRefNavigation { get; set; } + } + + public class DataFile + { + [Key] + public int FileId { get; set; } + [Required] + public string FileName { get; set; } + } }