Skip to content

Commit ec61746

Browse files
update changes based on review comments
1 parent 552bf8c commit ec61746

File tree

1 file changed

+42
-33
lines changed

1 file changed

+42
-33
lines changed

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

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ namespace Microsoft.AspNetCore.OData.Query.Expressions
3030
/// </summary>
3131
public class SelectExpandBinder : QueryBinder, ISelectExpandBinder
3232
{
33+
private string queryProvider;
34+
3335
/// <summary>
3436
/// Initializes a new instance of the <see cref="SelectExpandBinder" /> class.
3537
/// 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()
5961
/// </summary>
6062
public IOrderByBinder OrderByBinder { get; }
6163

62-
internal string QueryProvider { get; set; }
63-
6464
/// <summary>
6565
/// Translate an OData $select or $expand tree represented by <see cref="SelectExpandClause"/> to an <see cref="Expression"/>.
6666
/// </summary>
@@ -83,7 +83,7 @@ public virtual Expression BindSelectExpand(SelectExpandClause selectExpandClause
8383
IEdmNavigationSource navigationSource = context.NavigationSource;
8484
ParameterExpression source = context.CurrentParameter;
8585

86-
QueryProvider = context.QueryProvider;
86+
queryProvider = context.QueryProvider;
8787

8888
// expression looks like -> new Wrapper { Instance = source , Properties = "...", Container = new PropertyContainer { ... } }
8989
Expression projectionExpression = ProjectElement(context, source, selectExpandClause, structuredType, navigationSource);
@@ -374,11 +374,17 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
374374
bool isSelectedAll = IsSelectAll(selectExpandClause);
375375
if (isSelectedAll)
376376
{
377-
Expression sourceExpression = UpdateMemberInitExpression(source, structuredType);
377+
// If we have an EF query provider, then we remove the non-structural properties. The reason for this is to avoid
378+
// Creating an expression that will generate duplicate LEFT joins when a LEFT join already exists in the IQueryable
379+
// as observed here: https://github.com/OData/AspNetCoreOData/issues/497
380+
if (IsEfQueryProvider())
381+
{
382+
source = SelectExpandBinder.RemoveNonStructucalProperties(source, structuredType);
383+
}
378384

379385
// Initialize property 'Instance' on the wrapper class
380386
wrapperProperty = wrapperType.GetProperty("Instance");
381-
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, sourceExpression));
387+
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, source));
382388

383389
wrapperProperty = wrapperType.GetProperty("UseInstanceForProperties");
384390
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(true)));
@@ -437,47 +443,50 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
437443

438444
// Generates the expression
439445
// { Instance = new Customer() {Id = $it.Id, Name= $it.Name}}
440-
private Expression UpdateMemberInitExpression(Expression source, IEdmStructuredType structuredType)
446+
private static Expression RemoveNonStructucalProperties(Expression source, IEdmStructuredType structuredType)
441447
{
442-
Expression sourceExpression = null;
448+
Expression updatedSource = null;
449+
450+
Type elementType = source.Type;
451+
IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
452+
List<string> structuralPropertyNames = new List<string>();
443453

444-
if (QueryProvider != null &&
445-
QueryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
446-
QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
447-
QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
448-
QueryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
454+
foreach (IEdmStructuralProperty property in structuralProperties)
449455
{
450-
Type elementType = source.Type;
451-
IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
452-
List<string> structuralPropertyNames = new List<string>();
456+
structuralPropertyNames.Add(property.Name);
457+
}
458+
459+
PropertyInfo[] props = elementType.GetProperties();
460+
List<MemberBinding> bindings = new List<MemberBinding>();
453461

454-
foreach (IEdmStructuralProperty property in structuralProperties)
462+
foreach (PropertyInfo prop in props)
463+
{
464+
if (structuralPropertyNames.Contains(prop.Name))
455465
{
456-
structuralPropertyNames.Add(property.Name);
466+
MemberExpression propertyExpression = Expression.Property(source, prop);
467+
bindings.Add(Expression.Bind(prop, propertyExpression));
457468
}
469+
}
458470

459-
PropertyInfo[] props = elementType.GetProperties();
460-
List<MemberBinding> bindings = new List<MemberBinding>();
471+
NewExpression newExpression = Expression.New(source.Type);
461472

462-
foreach (PropertyInfo prop in props)
463-
{
464-
if (structuralPropertyNames.Contains(prop.Name))
465-
{
466-
MemberExpression propertyExpression = Expression.Property(source, prop);
467-
bindings.Add(Expression.Bind(prop, propertyExpression));
468-
}
469-
}
473+
updatedSource = Expression.MemberInit(newExpression, bindings);
470474

471-
NewExpression newExpression = Expression.New(source.Type);
475+
return updatedSource;
476+
}
472477

473-
sourceExpression = Expression.MemberInit(newExpression, bindings);
474-
}
475-
else
478+
private bool IsEfQueryProvider()
479+
{
480+
if (queryProvider != null &&
481+
queryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
482+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
483+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
484+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
476485
{
477-
sourceExpression = source;
486+
return true;
478487
}
479488

480-
return sourceExpression;
489+
return false;
481490
}
482491

483492
private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,

0 commit comments

Comments
 (0)