update changes based on review comments

OData · Aug 10, 2023 · ec61746 · ec61746
1 parent 552bf8c
commit ec61746
Showing 1 changed file with 42 additions and 33 deletions.
diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
@@ -30,6 +30,8 @@ namespace Microsoft.AspNetCore.OData.Query.Expressions
     /// </summary>
     public class SelectExpandBinder : QueryBinder, ISelectExpandBinder
     {
+        private string queryProvider;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="SelectExpandBinder" /> class.
         /// Select and Expand binder depends on <see cref="IFilterBinder"/> and <see cref="IOrderByBinder"/> to process inner $filter and $orderby.
@@ -59,8 +61,6 @@ internal SelectExpandBinder()
         /// </summary>
         public IOrderByBinder OrderByBinder { get; }
 
-        internal string QueryProvider { get; set; }
-
         /// <summary>
         /// Translate an OData $select or $expand tree represented by <see cref="SelectExpandClause"/> to an <see cref="Expression"/>.
         /// </summary>
@@ -83,7 +83,7 @@ public virtual Expression BindSelectExpand(SelectExpandClause selectExpandClause
             IEdmNavigationSource navigationSource = context.NavigationSource;
             ParameterExpression source = context.CurrentParameter;
 
-            QueryProvider = context.QueryProvider;
+            queryProvider = context.QueryProvider;
 
             // expression looks like -> new Wrapper { Instance = source , Properties = "...", Container = new PropertyContainer { ... } }
             Expression projectionExpression = ProjectElement(context, source, selectExpandClause, structuredType, navigationSource);
@@ -374,11 +374,17 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
             bool isSelectedAll = IsSelectAll(selectExpandClause);
             if (isSelectedAll)
             {
-                Expression sourceExpression = UpdateMemberInitExpression(source, structuredType);
+                // 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
+                if (IsEfQueryProvider())
+                { 
+                    source = SelectExpandBinder.RemoveNonStructucalProperties(source, structuredType);
+                }
 
                 // Initialize property 'Instance' on the wrapper class
                 wrapperProperty = wrapperType.GetProperty("Instance");
-                wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, sourceExpression));
+                wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, source));
 
                 wrapperProperty = wrapperType.GetProperty("UseInstanceForProperties");
                 wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(true)));
@@ -437,47 +443,50 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
 
         // Generates the expression
         //      { Instance = new Customer() {Id = $it.Id, Name= $it.Name}}
-        private Expression UpdateMemberInitExpression(Expression source, IEdmStructuredType structuredType)
+        private static Expression RemoveNonStructucalProperties(Expression source, IEdmStructuredType structuredType)
         {
-            Expression sourceExpression = null;
+            Expression updatedSource = null;
+
+            Type elementType = source.Type;
+            IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
+            List<string> structuralPropertyNames = new List<string>();
 
-            if (QueryProvider != null &&
-                QueryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
-                QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
-                QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
-                QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
+            foreach (IEdmStructuralProperty property in structuralProperties)
             {
-                Type elementType = source.Type;
-                IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
-                List<string> structuralPropertyNames = new List<string>();
+                structuralPropertyNames.Add(property.Name);
+            }
+
+            PropertyInfo[] props = elementType.GetProperties();
+            List<MemberBinding> bindings = new List<MemberBinding>();
 
-                foreach (IEdmStructuralProperty property in structuralProperties)
+            foreach (PropertyInfo prop in props)
+            {
+                if (structuralPropertyNames.Contains(prop.Name))
                 {
-                    structuralPropertyNames.Add(property.Name);
+                    MemberExpression propertyExpression = Expression.Property(source, prop);
+                    bindings.Add(Expression.Bind(prop, propertyExpression));
                 }
+            }
 
-                PropertyInfo[] props = elementType.GetProperties();
-                List<MemberBinding> bindings = new List<MemberBinding>();
+            NewExpression newExpression = Expression.New(source.Type);
 
-                foreach (PropertyInfo prop in props)
-                {
-                    if (structuralPropertyNames.Contains(prop.Name))
-                    {
-                        MemberExpression propertyExpression = Expression.Property(source, prop);
-                        bindings.Add(Expression.Bind(prop, propertyExpression));
-                    }
-                }
+            updatedSource = Expression.MemberInit(newExpression, bindings);
 
-                NewExpression newExpression = Expression.New(source.Type);
+            return updatedSource;     
+        }
 
-                sourceExpression = Expression.MemberInit(newExpression, bindings);
-            }
-            else
+        private bool IsEfQueryProvider()
+        {
+            if (queryProvider != null &&
+                queryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
+                queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
+                queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
+                queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
             {
-                sourceExpression = source;
+                return true;
             }
 
-            return sourceExpression;     
+            return false;
         }
 
         private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,