Skip to content

Commit 9e13363

Browse files
fix duplicate join issues
1 parent fa81478 commit 9e13363

File tree

5 files changed

+120
-1
lines changed

5 files changed

+120
-1
lines changed

src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9624,6 +9624,11 @@
96249624
Basically for $compute in $select and $expand
96259625
</summary>
96269626
</member>
9627+
<member name="P:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.QueryProvider">
9628+
<summary>
9629+
Gets or sets the query provider.
9630+
</summary>
9631+
</member>
96279632
<member name="M:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.GetParameter(System.String)">
96289633
<summary>
96299634
Gets the parameter using parameter name.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,11 @@ public static IQueryable ApplyBind(this ISelectExpandBinder binder, IQueryable s
275275
throw Error.ArgumentNull(nameof(context));
276276
}
277277

278+
if (string.IsNullOrEmpty(context.QueryProvider))
279+
{
280+
context.QueryProvider = source.Provider.GetType().Namespace;
281+
}
282+
278283
Type elementType = context.ElementClrType;
279284

280285
LambdaExpression projectionLambda = binder.BindSelectExpand(selectExpandClause, context) as LambdaExpression;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe
168168
/// </summary>
169169
public Expression Source { get;set; }
170170

171+
/// <summary>
172+
/// Gets or sets the query provider.
173+
/// </summary>
174+
internal string QueryProvider { get; set; }
175+
171176
/// <summary>
172177
/// Gets the parameter using parameter name.
173178
/// </summary>

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

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
using System.Collections.Generic;
1010
using System.Diagnostics.CodeAnalysis;
1111
using System.Diagnostics.Contracts;
12+
using System.Diagnostics.Tracing;
1213
using System.Linq;
1314
using System.Linq.Expressions;
1415
using System.Reflection;
16+
using System.Reflection.Emit;
1517
using Microsoft.AspNetCore.OData.Common;
1618
using Microsoft.AspNetCore.OData.Edm;
1719
using Microsoft.AspNetCore.OData.Query.Container;
@@ -29,6 +31,8 @@ namespace Microsoft.AspNetCore.OData.Query.Expressions
2931
/// </summary>
3032
public class SelectExpandBinder : QueryBinder, ISelectExpandBinder
3133
{
34+
private string queryProvider;
35+
3236
/// <summary>
3337
/// Initializes a new instance of the <see cref="SelectExpandBinder" /> class.
3438
/// Select and Expand binder depends on <see cref="IFilterBinder"/> and <see cref="IOrderByBinder"/> to process inner $filter and $orderby.
@@ -80,9 +84,12 @@ public virtual Expression BindSelectExpand(SelectExpandClause selectExpandClause
8084
IEdmNavigationSource navigationSource = context.NavigationSource;
8185
ParameterExpression source = context.CurrentParameter;
8286

87+
queryProvider = context.QueryProvider;
88+
8389
// expression looks like -> new Wrapper { Instance = source , Properties = "...", Container = new PropertyContainer { ... } }
8490
Expression projectionExpression = ProjectElement(context, source, selectExpandClause, structuredType, navigationSource);
8591

92+
8693
// expression looks like -> source => new Wrapper { Instance = source .... }
8794
LambdaExpression projectionLambdaExpression = Expression.Lambda(projectionExpression, source);
8895

@@ -368,9 +375,23 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
368375
bool isSelectedAll = IsSelectAll(selectExpandClause);
369376
if (isSelectedAll)
370377
{
378+
// If we have an EF query provider, then we remove the non-structural properties. The reason for this is to avoid
379+
// Creating an expression that will generate duplicate LEFT joins when a LEFT join already exists in the IQueryable
380+
// as observed here: https://github.com/OData/AspNetCoreOData/issues/497
381+
382+
Expression updatedExpression = null;
383+
if (IsEfQueryProvider())
384+
{
385+
updatedExpression = SelectExpandBinder.RemoveNonStructucalProperties(source, structuredType);
386+
}
387+
else
388+
{
389+
updatedExpression = source;
390+
}
391+
371392
// Initialize property 'Instance' on the wrapper class
372393
wrapperProperty = wrapperType.GetProperty("Instance");
373-
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, source));
394+
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, updatedExpression));
374395

375396
wrapperProperty = wrapperType.GetProperty("UseInstanceForProperties");
376397
wrapperTypeMemberAssignments.Add(Expression.Bind(wrapperProperty, Expression.Constant(true)));
@@ -427,6 +448,51 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
427448
return Expression.MemberInit(Expression.New(wrapperType), wrapperTypeMemberAssignments);
428449
}
429450

451+
// Generates the expression
452+
// { Instance = new Customer() {Id = $it.Id, Name= $it.Name}}
453+
private static Expression RemoveNonStructucalProperties(Expression source, IEdmStructuredType structuredType)
454+
{
455+
Expression updatedSource = null;
456+
457+
Type elementType = source.Type;
458+
IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();
459+
460+
PropertyInfo[] props = elementType.GetProperties();
461+
List<MemberBinding> bindings = new List<MemberBinding>();
462+
463+
foreach (PropertyInfo prop in props)
464+
{
465+
foreach (var sp in structuralProperties)
466+
{
467+
if (sp.Name == prop.Name)
468+
{
469+
MemberExpression propertyExpression = Expression.Property(source, prop);
470+
bindings.Add(Expression.Bind(prop, propertyExpression));
471+
break;
472+
}
473+
}
474+
}
475+
476+
NewExpression newExpression = Expression.New(source.Type);
477+
updatedSource = Expression.MemberInit(newExpression, bindings);
478+
479+
return updatedSource;
480+
}
481+
482+
private bool IsEfQueryProvider()
483+
{
484+
if (queryProvider != null &&
485+
queryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
486+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
487+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
488+
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
489+
{
490+
return true;
491+
}
492+
493+
return false;
494+
}
495+
430496
private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,
431497
out IList<string> computedProperties)
432498
{

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,44 @@ public void Bind_GeneratedExpression_ContainsExpandedObject()
209209
Assert.Same(_queryable.First(), innerInnerCustomer.Instance);
210210
}
211211

212+
[Theory]
213+
[InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)]
214+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)]
215+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)]
216+
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)]
217+
public void Bind_UsingEFQueryProvider_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider)
218+
{
219+
// Arrange
220+
SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context);
221+
222+
// Act
223+
SelectExpandBinder binder = new SelectExpandBinder();
224+
_queryBinderContext.QueryProvider = queryProvider;
225+
IQueryable queryable = binder.ApplyBind(_queryable, selectExpand.SelectExpandClause, _queryBinderContext);
226+
227+
// Assert
228+
IEnumerator enumerator = queryable.GetEnumerator();
229+
Assert.True(enumerator.MoveNext());
230+
var partialCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(enumerator.Current);
231+
Assert.False(enumerator.MoveNext());
232+
Assert.Null(partialCustomer.Instance);
233+
Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType);
234+
IEnumerable<SelectExpandWrapper<QueryOrder>> innerOrders = partialCustomer.Container
235+
.ToDictionary(PropertyMapper)["Orders"] as IEnumerable<SelectExpandWrapper<QueryOrder>>;
236+
Assert.NotNull(innerOrders);
237+
SelectExpandWrapper<QueryOrder> partialOrder = innerOrders.Single();
238+
239+
// We only write structural properties to the instance.
240+
// This means that navigation properties on the instance property will be null
241+
// when using any instance of EF query provider.
242+
Assert.Null(partialOrder.Instance.Customer);
243+
244+
object customer = partialOrder.Container.ToDictionary(PropertyMapper)["Customer"];
245+
SelectExpandWrapper<QueryCustomer> innerInnerCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(customer);
246+
247+
Assert.Null(innerInnerCustomer.Instance.Orders);
248+
}
249+
212250
[Fact]
213251
public void Bind_GeneratedExpression_CheckNullObjectWithinChainProjectionByKey()
214252
{

0 commit comments

Comments
 (0)