Skip to content

Commit

Permalink
fix duplicate join issues
Browse files Browse the repository at this point in the history
  • Loading branch information
ElizabethOkerio committed Aug 10, 2023
1 parent fa81478 commit 9e13363
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 1 deletion.
5 changes: 5 additions & 0 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9624,6 +9624,11 @@
Basically for $compute in $select and $expand
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.QueryProvider">
<summary>
Gets or sets the query provider.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.GetParameter(System.String)">
<summary>
Gets the parameter using parameter name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe
/// </summary>
public Expression Source { get;set; }

/// <summary>
/// Gets or sets the query provider.
/// </summary>
internal string QueryProvider { get; set; }

/// <summary>
/// Gets the parameter using parameter name.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Contracts;
using System.Diagnostics.Tracing;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Reflection.Emit;
using Microsoft.AspNetCore.OData.Common;
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Query.Container;
Expand All @@ -29,6 +31,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.
Expand Down Expand Up @@ -80,9 +84,12 @@ public virtual Expression BindSelectExpand(SelectExpandClause selectExpandClause
IEdmNavigationSource navigationSource = context.NavigationSource;
ParameterExpression source = context.CurrentParameter;

queryProvider = context.QueryProvider;

// expression looks like -> new Wrapper { Instance = source , Properties = "...", Container = new PropertyContainer { ... } }
Expression projectionExpression = ProjectElement(context, source, selectExpandClause, structuredType, navigationSource);


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

Expand Down Expand Up @@ -368,9 +375,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())
{
updatedExpression = SelectExpandBinder.RemoveNonStructucalProperties(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)));
Expand Down Expand Up @@ -427,6 +448,51 @@ 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 RemoveNonStructucalProperties(Expression source, IEdmStructuredType structuredType)
{
Expression updatedSource = null;

Type elementType = source.Type;
IEnumerable<IEdmStructuralProperty> structuralProperties = structuredType.StructuralProperties();

PropertyInfo[] props = elementType.GetProperties();
List<MemberBinding> bindings = new List<MemberBinding>();

foreach (PropertyInfo prop in props)
{
foreach (var sp in structuralProperties)
{
if (sp.Name == prop.Name)
{
MemberExpression propertyExpression = Expression.Property(source, prop);
bindings.Add(Expression.Bind(prop, propertyExpression));
break;
}
}
}

NewExpression newExpression = Expression.New(source.Type);
updatedSource = Expression.MemberInit(newExpression, bindings);

return updatedSource;
}

private bool IsEfQueryProvider()
{
if (queryProvider != null &&
queryProvider == HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace ||
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5 ||
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6 ||
queryProvider == HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)
{
return true;
}

return false;
}

private static bool ParseComputedDynamicProperties(QueryBinderContext context, IList<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,
out IList<string> computedProperties)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,44 @@ 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(_queryable, selectExpand.SelectExpandClause, _queryBinderContext);

// Assert
IEnumerator enumerator = queryable.GetEnumerator();
Assert.True(enumerator.MoveNext());
var partialCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(enumerator.Current);
Assert.False(enumerator.MoveNext());
Assert.Null(partialCustomer.Instance);
Assert.Equal("Microsoft.AspNetCore.OData.Tests.Query.Expressions.QueryCustomer", partialCustomer.InstanceType);
IEnumerable<SelectExpandWrapper<QueryOrder>> innerOrders = partialCustomer.Container
.ToDictionary(PropertyMapper)["Orders"] as IEnumerable<SelectExpandWrapper<QueryOrder>>;
Assert.NotNull(innerOrders);
SelectExpandWrapper<QueryOrder> partialOrder = innerOrders.Single();

// 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.
Assert.Null(partialOrder.Instance.Customer);

object customer = partialOrder.Container.ToDictionary(PropertyMapper)["Customer"];
SelectExpandWrapper<QueryCustomer> innerInnerCustomer = Assert.IsAssignableFrom<SelectExpandWrapper<QueryCustomer>>(customer);

Assert.Null(innerInnerCustomer.Instance.Orders);
}

[Fact]
public void Bind_GeneratedExpression_CheckNullObjectWithinChainProjectionByKey()
{
Expand Down

0 comments on commit 9e13363

Please sign in to comment.