Skip to content

fix duplicate join issues #993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -100,6 +100,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));
Expand Down Expand Up @@ -168,6 +170,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 @@ -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 = 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 +441,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(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<DynamicPathSegment> dynamicPathSegments, bool isSelectedAll,
out IList<string> computedProperties)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.Serialization;
using System.Xml.Linq;
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Query;
using Microsoft.AspNetCore.OData.Query.Container;
Expand Down Expand Up @@ -209,6 +208,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