Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#1023 - Fix all structural properties ignored when using $expand without $select and camel cased property names #1024

Merged
merged 2 commits into from
Aug 22, 2023
Merged

#1023 - Fix all structural properties ignored when using $expand without $select and camel cased property names #1024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87b07db
#1023 - Fix all structural properties ignored when using $expand with…
Aug 18, 2023
96c2b4d
Added unit tests
Aug 21, 2023
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
1 change: 0 additions & 1 deletion src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13961,7 +13961,6 @@
<member name="P:Microsoft.AspNetCore.OData.Routing.ODataRouteOptions.Order">
<summary>
Gets or sets the route order in conventional routing.
By default, move the conventional routing later as much as possible.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just rebuilt the solution to validate my fix, and the comment must have been removed in a previous commit without committing the regeneration of the documentation XML. I can revert it if, but it might come back the next time someone rebuilds the solution.

</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.Routing.ODataRouteOptions.EnableDollarCountRouting">
Expand Down
7 changes: 4 additions & 3 deletions src/Microsoft.AspNetCore.OData/Query/Expressions/SelectExpandBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source
Expression updatedExpression = null;
if (IsEfQueryProvider(context))
{
updatedExpression = SelectExpandBinder.RemoveNonStructucalProperties(source, structuredType);
updatedExpression = SelectExpandBinder.RemoveNonStructucalProperties(context, source, structuredType);
}
else
{
Expand Down Expand Up @@ -443,8 +443,9 @@ internal Expression ProjectElement(QueryBinderContext context, Expression source

// Generates the expression
// { Instance = new Customer() {Id = $it.Id, Name= $it.Name}}
private static Expression RemoveNonStructucalProperties(Expression source, IEdmStructuredType structuredType)
private static Expression RemoveNonStructucalProperties(QueryBinderContext context, Expression source, IEdmStructuredType structuredType)
{
IEdmModel model = context.Model;
Expression updatedSource = null;

Type elementType = source.Type;
Expand All @@ -457,7 +458,7 @@ private static Expression RemoveNonStructucalProperties(Expression source, IEdmS
{
foreach (var sp in structuralProperties)
{
if (sp.Name == prop.Name)
if (model.GetClrPropertyName(sp) == prop.Name)
{
MemberExpression propertyExpression = Expression.Property(source, prop);
bindings.Add(Expression.Bind(prop, propertyExpression));
Expand Down
109 changes: 107 additions & 2 deletions test/Microsoft.AspNetCore.OData.Tests/Query/Expressions/SelectExpandBinderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,35 @@ public class SelectExpandBinderTest
private static IPropertyMapper PropertyMapper = new IdentityPropertyMapper();

private readonly SelectExpandBinder _binder;
private readonly SelectExpandBinder _binder_lowerCamelCased;
private readonly IQueryable<QueryCustomer> _queryable;
private readonly IQueryable<QueryCustomer> _queryable_lowerCamelCased;
private readonly ODataQueryContext _context;
private readonly ODataQueryContext _context_lowerCamelCased;
private readonly ODataQuerySettings _settings;
private readonly ODataQuerySettings _settings_lowerCamelCased;
private readonly QueryBinderContext _queryBinderContext;
private readonly QueryBinderContext _queryBinderContext_lowerCamelCased;

private readonly IEdmModel _model;
private readonly IEdmModel _model_lowerCamelCased;
private readonly IEdmEntityType _customer;
private readonly IEdmEntityType _customer_lowerCamelCased;
private readonly IEdmEntityType _order;
private readonly IEdmEntityType _order_lowerCamelCased;
private readonly IEdmEntityType _product;
private readonly IEdmEntityType _product_lowerCamelCased;
private readonly IEdmEntitySet _customers;
private readonly IEdmEntitySet _customers_lowerCamelCased;
private readonly IEdmEntitySet _orders;
private readonly IEdmEntitySet _orders_lowerCamelCased;
private readonly IEdmEntitySet _products;
private readonly IEdmEntitySet _products_lowerCamelCased;

public SelectExpandBinderTest()
{
#region PascalCase EdmModel

_model = GetEdmModel();
_customer = _model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryCustomer");
_order = _model.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryOrder");
Expand All @@ -64,7 +78,7 @@ public SelectExpandBinderTest()
{
Orders = new List<QueryOrder>()
};
QueryOrder order = new QueryOrder { Customer = customer };
QueryOrder order = new QueryOrder { Id = 42, Title = "The order", Customer = customer };
customer.Orders.Add(order);

_queryable = new[] { customer }.AsQueryable();
Expand All @@ -75,6 +89,40 @@ public SelectExpandBinderTest()
{
NavigationSource = _context.NavigationSource
};

#endregion

#region camelCase EdmModel

_model_lowerCamelCased = GetEdmModel_lowerCamelCased();
_customer_lowerCamelCased = _model_lowerCamelCased.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryCustomer");
_order_lowerCamelCased = _model_lowerCamelCased.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryOrder");
_product_lowerCamelCased = _model_lowerCamelCased.SchemaElements.OfType<IEdmEntityType>().First(c => c.Name == "QueryProduct");
_customers_lowerCamelCased = _model_lowerCamelCased.EntityContainer.FindEntitySet("Customers");
_orders_lowerCamelCased = _model_lowerCamelCased.EntityContainer.FindEntitySet("Orders");
_products_lowerCamelCased = _model_lowerCamelCased.EntityContainer.FindEntitySet("Products");

_settings_lowerCamelCased = new ODataQuerySettings { HandleNullPropagation = HandleNullPropagationOption.False };
_context_lowerCamelCased = new ODataQueryContext(_model_lowerCamelCased, typeof(QueryCustomer)) { RequestContainer = new MockServiceProvider() };
_binder_lowerCamelCased = new SelectExpandBinder(new FilterBinder(), new OrderByBinder());

QueryCustomer customer_lowerCamelCased = new QueryCustomer
{
Orders = new List<QueryOrder>()
};
QueryOrder order_lowerCamelCased = new QueryOrder { Id = 42, Title = "The order", Customer = customer_lowerCamelCased };
customer_lowerCamelCased.Orders.Add(order_lowerCamelCased);

_queryable_lowerCamelCased = new[] { customer_lowerCamelCased }.AsQueryable();

SelectExpandQueryOption selectExpandQueryOption_lowerCamelCased = new SelectExpandQueryOption("Orders", expand: null, context: _context_lowerCamelCased);

_queryBinderContext_lowerCamelCased = new QueryBinderContext(_model_lowerCamelCased, _settings_lowerCamelCased, selectExpandQueryOption_lowerCamelCased.Context.ElementClrType)
{
NavigationSource = _context_lowerCamelCased.NavigationSource
};

#endregion
}

private static SelectExpandBinder GetBinder<T>(IEdmModel model, HandleNullPropagationOption nullPropagation = HandleNullPropagationOption.False)
Expand Down Expand Up @@ -237,15 +285,59 @@ public void Bind_UsingEFQueryProvider_GeneratedExpression__DoesNot_ContainExpand

// 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.
// when using any instance of EF query provider, and all structural properties
// will be assigned.
Assert.Null(partialOrder.Instance.Customer);
Assert.NotEqual(0, partialOrder.Instance.Id);
Assert.NotNull(partialOrder.Instance.Title);

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

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

[Theory]
[InlineData(HandleNullPropagationOptionHelper.EntityFrameworkQueryProviderNamespace)]
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEFCore2)]
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF5)]
[InlineData(HandleNullPropagationOptionHelper.ObjectContextQueryProviderNamespaceEF6)]
public void Bind_UsingEFQueryProvider_LowerCamelCasedModel_GeneratedExpression__DoesNot_ContainExpandedObject(string queryProvider)
{
// Arrange
SelectExpandQueryOption selectExpand = new SelectExpandQueryOption("Orders", "Orders,Orders($expand=Customer)", _context_lowerCamelCased);

// Act
SelectExpandBinder binder = new SelectExpandBinder();
_queryBinderContext_lowerCamelCased.QueryProvider = queryProvider;
IQueryable queryable = binder.ApplyBind(_queryable_lowerCamelCased, selectExpand.SelectExpandClause, _queryBinderContext_lowerCamelCased);

// 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, and all structural properties
// will be assigned.
Assert.Null(partialOrder.Instance.Customer);
Assert.NotEqual(0, partialOrder.Instance.Id);
Assert.NotNull(partialOrder.Instance.Title);

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 Expand Up @@ -1991,6 +2083,19 @@ public static IEdmModel GetEdmModel()
return builder.GetEdmModel();
}

public static IEdmModel GetEdmModel_lowerCamelCased()
{
var builder = new ODataConventionModelBuilder();
var customer = builder.EntitySet<QueryCustomer>("Customers").EntityType;
builder.EntitySet<QueryOrder>("Orders");
builder.EntitySet<QueryCity>("Cities");
builder.EntitySet<QueryProduct>("Products");

customer.Collection.Function("IsUpgraded").Returns<bool>().Namespace="NS";
customer.Collection.Action("UpgradeAll").Namespace = "NS";
builder.EnableLowerCamelCase();
return builder.GetEdmModel();
}
public static SelectExpandClause ParseSelectExpand(string select, string expand, IEdmModel model, IEdmType edmType, IEdmNavigationSource navigationSource)
{
return new ODataQueryOptionParser(model, edmType, navigationSource,
Expand Down