fix $orderby with $apply and groupby aggregations (#919)

* fix $orderby with $apply and group by aggregations * fix tests * update based on review comments
OData · May 16, 2023 · d615834 · d615834
1 parent 73c1ed9
commit d615834
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 3 deletions.
diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
@@ -9530,6 +9530,11 @@
             Gets the compute expressions.
             </summary>
         </member>
+        <member name="P:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.FlattenedProperties">
+            <summary>
+            Flattened list of properties from base query, for case when binder is applied for aggregated query.
+            </summary>
+        </member>
         <member name="P:Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.ElementType">
             <summary>
             Gets the <see cref="T:Microsoft.OData.Edm.IEdmType"/> of the element type.

diff --git a/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt b/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt
@@ -920,6 +920,8 @@ Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.ComputedProperti
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.CurrentParameter.get -> System.Linq.Expressions.ParameterExpression
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.ElementClrType.get -> System.Type
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.ElementType.get -> Microsoft.OData.Edm.IEdmType
+Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.FlattenedProperties.get -> System.Collections.Generic.IDictionary<string, System.Linq.Expressions.Expression>
+Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.FlattenedProperties.set -> void
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.GetParameter(string name) -> System.Linq.Expressions.ParameterExpression
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.Model.get -> Microsoft.OData.Edm.IEdmModel
 Microsoft.AspNetCore.OData.Query.Expressions.QueryBinderContext.NavigationSource.get -> Microsoft.OData.Edm.IEdmNavigationSource

diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/BinderExtensions.cs
@@ -7,6 +7,7 @@
 
 using System;
 using System.Collections;
+using System.Collections.Generic;
 using System.Diagnostics.Contracts;
 using System.Linq;
 using System.Linq.Expressions;
@@ -163,6 +164,8 @@ public static IQueryable ApplyBind(this IOrderByBinder binder, IQueryable query,
                 throw Error.ArgumentNull(nameof(context));
             }
 
+            context.EnsureFlattenedProperties(context.CurrentParameter, query);
+
             OrderByBinderResult orderByResult = binder.BindOrderBy(orderByClause, context);
             IQueryable querySoFar = query;
 

diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinder.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinder.cs
@@ -1101,14 +1101,14 @@ protected static PropertyInfo GetDynamicPropertyContainer(SingleValueOpenPropert
         /// <returns>Returns null if no aggregations were used so far</returns>
         protected Expression GetFlattenedPropertyExpression(string propertyPath, QueryBinderContext context)
         {
-            if (context == null || context.ComputedProperties == null || !context.ComputedProperties.Any())
+            if (context == null || context.FlattenedProperties == null || !context.FlattenedProperties.Any())
             {
                 return null;
             }
 
-            if (context.ComputedProperties.TryGetValue(propertyPath, out var expression))
+            if (context.FlattenedProperties.TryGetValue(propertyPath, out var expression))
             {
-                return Bind(expression.Expression, context);
+                return expression;
             }
 
             return null;

diff --git a/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs b/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinderContext.cs
@@ -8,6 +8,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics.Contracts;
+using System.Linq;
 using System.Linq.Expressions;
 using Microsoft.AspNetCore.OData.Edm;
 using Microsoft.AspNetCore.OData.Query.Wrapper;
@@ -139,6 +140,11 @@ public QueryBinderContext(QueryBinderContext context, ODataQuerySettings querySe
         /// </summary>
         public IDictionary<string, ComputeExpression> ComputedProperties { get; } = new Dictionary<string, ComputeExpression>();
 
+        /// <summary>
+        /// Flattened list of properties from base query, for case when binder is applied for aggregated query.
+        /// </summary>
+        internal IDictionary<string, Expression> FlattenedProperties { get; private set; } = new Dictionary<string, Expression>();
+
         /// <summary>
         /// Gets the <see cref="IEdmType"/> of the element type.
         /// </summary>
@@ -239,5 +245,15 @@ internal void AddComputedProperties(IEnumerable<ComputeExpression> computedPrope
                 ComputedProperties[property.Alias] = property;
             }
         }
+
+        internal void EnsureFlattenedProperties(ParameterExpression source, IQueryable query)
+        {
+            TransformationBinderBase binder = new TransformationBinderBase(this.QuerySettings, this.AssembliesResolver, this.ElementClrType, this.Model)
+            {
+                BaseQuery = query
+            };
+
+            this.FlattenedProperties = binder.GetFlattenedProperties(source);
+        }
     }
 }
diff --git a/test/Microsoft.AspNetCore.OData.E2E.Tests/EntitySetAggregation/EntitySetAggregationTests.cs b/test/Microsoft.AspNetCore.OData.E2E.Tests/EntitySetAggregation/EntitySetAggregationTests.cs
@@ -103,6 +103,36 @@ public async Task MultipleAggregationOnEntitySetWorks(string queryString)
             Assert.Equal(1 + 2 + 3 + 4 + 5 + 6, totalId); 
         }
 
+#if NET6_0_OR_GREATER
+
+        [Fact]
+        public async Task GroupByWithAggregationAndOrderByWorks()
+        {
+            // Arrange
+            string queryUrl = AggregationTestBaseUrl + "?$apply=groupby((Name),aggregate(Orders(Price with sum as TotalPrice)))&$orderby=Name desc";
+            string expectedResult = "{\"value\":[{\"Name\":\"Customer1\",\"Orders\":[{\"TotalPrice\":200}]},{\"Name\":\"Customer0\",\"Orders\":[{\"TotalPrice\":400}]}]}";
+            HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, queryUrl);
+            request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json;odata.metadata=none"));
+
+            // Act
+            HttpResponseMessage response = Client.SendAsync(request).Result;
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            var stringObject = await response.Content.ReadAsStringAsync();
+
+            Assert.Equal(expectedResult, stringObject.ToString());
+
+            var result = await response.Content.ReadAsObject<JObject>();    
+            var value = result["value"];
+            var orders = value.First["Orders"];
+            var totalPrice = orders.First["TotalPrice"].ToObject<int>();
+
+            Assert.Equal(200, totalPrice);
+        }
+
+#endif
+
         [Fact(Skip = "See the comments above")]
         public async Task AggregationOnEntitySetWorksWithPropertyAggregation()
         {