From 37b0af82c9f7b55195d86f542eb2ee27b94a83ec Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Tue, 26 May 2020 12:54:29 -0700 Subject: [PATCH 1/9] System.Diagnostics.EventLog fix --- .../Microsoft.Test.E2E.AspNetCore3x.OData.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj index 9f4e8316a1..420d3a7bdd 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj @@ -38,6 +38,7 @@ + From 76bdd9a424ea710a719e9ca08db5278f4cc85095 Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Tue, 26 May 2020 14:35:50 -0700 Subject: [PATCH 2/9] Use EF Core 5 --- .../Aggregation/AggregationTests.cs | 3 ++- .../Build.AspNetCore3x/Endpoint/EndpointDbContext.cs | 4 +++- .../Microsoft.Test.E2E.AspNetCore3x.OData.csproj | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs index 1f7c43815f..5b468cfa29 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs @@ -469,8 +469,9 @@ public async Task AggregateVirtualCountWorks() HttpResponseMessage response = await client.SendAsync(request); // Assert + var res = await response.Content.ReadAsStringAsync(); + JObject json = JObject.Parse(res); Assert.Equal(HttpStatusCode.OK, response.StatusCode); - JObject json = JObject.Parse(await response.Content.ReadAsStringAsync()); JToken value = json["value"].Children().First(); var anonymousResponse = new { Count = 0 }; diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Endpoint/EndpointDbContext.cs b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Endpoint/EndpointDbContext.cs index 82a2e36f7c..9d89589bf0 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Endpoint/EndpointDbContext.cs +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Endpoint/EndpointDbContext.cs @@ -22,11 +22,13 @@ protected override void OnModelCreating(EntityFrameworkCore.ModelBuilder modelBu modelBuilder.Entity().OwnsOne(c => c.HomeAddress); modelBuilder.Entity().OwnsMany(c => c.FavoriteAddresses, a => { - a.HasForeignKey("OwnerId"); + a.WithOwner().HasForeignKey("OwnerId"); a.Property("Id"); a.HasKey("Id"); }); + //modelBuilder.Entity().Navigation(c => c.FavoriteAddresses). + // But, in EF Core 3.x, it seems we can only use the following codes: // modelBuilder.Entity().OwnsOne(c => c.HomeAddress).WithOwner(); } diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj index 420d3a7bdd..b152e0f3a1 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj @@ -34,10 +34,10 @@ - - - - + + + + From c1b49e6c43ac73c8a2c792937ac512470d95dfe9 Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Wed, 27 May 2020 12:43:53 -0700 Subject: [PATCH 3/9] Hack: Solve serialization issues --- .../Serialization/DefaultODataSerializerProvider.cs | 7 +++++-- .../Formatter/Serialization/ODataSerializerContext.cs | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs index 1f183f16d9..5576d121fc 100644 --- a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs +++ b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs @@ -130,10 +130,13 @@ internal ODataSerializer GetODataPayloadSerializerImpl(Type type, Func().FirstOrDefault().EdmType.ToEdmTypeReference(true)); } + return null; } } } diff --git a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs index f98cfce09c..b5665b24a0 100644 --- a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs +++ b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Microsoft.AspNet.OData.Common; using Microsoft.AspNet.OData.Interfaces; using Microsoft.AspNet.OData.Query; @@ -284,6 +285,12 @@ internal IEdmTypeReference GetEdmType(object instance, Type type) { IEdmTypeReference edmType; + if (type.IsGenericType() && EdmLibHelpers.IsDynamicTypeWrapper(type.GetGenericArguments()[0])) + { + return this.Path.Segments.OfType().FirstOrDefault()?.EdmType?.ToEdmTypeReference(true); + } + + IEdmObject edmObject = instance as IEdmObject; if (edmObject != null) { From 47756e0ebd78fbe67d89fc17f9433393e68ff57a Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Thu, 28 May 2020 14:08:00 -0700 Subject: [PATCH 4/9] PoC for avoiding client eval --- .../Query/Expressions/AggregationBinder.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs index 8498cf2b7d..9547498d95 100644 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs +++ b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs @@ -402,16 +402,16 @@ private Expression CreatePropertyAggregateExpression(ParameterExpression accum, // we need cast it to IEnumerable during expression building (IEnumerable)$it // however for EF6 we need to use $it.AsQueryable() due to limitations in types of casts that will properly translated Expression asQuerableExpression = null; - if (ClassicEF) - { + //if (ClassicEF) + //{ var asQuerableMethod = ExpressionHelperMethods.QueryableAsQueryable.MakeGenericMethod(baseType); asQuerableExpression = Expression.Call(null, asQuerableMethod, accum); - } - else - { - var queryableType = typeof(IEnumerable<>).MakeGenericType(baseType); - asQuerableExpression = Expression.Convert(accum, queryableType); - } + //} + //else + //{ + // var queryableType = typeof(IEnumerable<>).MakeGenericType(baseType); + // asQuerableExpression = Expression.Convert(accum, queryableType); + //} // $count is a virtual property, so there's not a propertyLambda to create. if (expression.Method == AggregationMethod.VirtualPropertyCount) @@ -586,6 +586,7 @@ private IQueryable BindGroupBy(IQueryable query) // .GroupBy($it => new NoGroupByWrapper()) groupLambda = Expression.Lambda(Expression.New(this._groupByClrType), this.LambdaParameter); } + return ExpressionHelpers.GroupBy(query, groupLambda, elementType, this._groupByClrType); } From 9de18579735a4ed253fec225cd08caae75867e0d Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Fri, 29 May 2020 10:05:26 -0700 Subject: [PATCH 5/9] Preflatten even without group by --- .../Query/Expressions/AggregationBinder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs index 9547498d95..1450a0c57c 100644 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs +++ b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs @@ -164,8 +164,8 @@ private IQueryable FlattenReferencedProperties(IQueryable query) { if (_aggregateExpressions != null && _aggregateExpressions.OfType().Any(e => e.Method != AggregationMethod.VirtualPropertyCount) - && _groupingProperties != null - && _groupingProperties.Any() + //&& _groupingProperties != null + //&& _groupingProperties.Any() && (FlattenedPropertyContainer == null || !FlattenedPropertyContainer.Any())) { var wrapperType = typeof(FlatteningWrapper<>).MakeGenericType(this.ElementType); From be11a003ad5c39ace947bb423e462bce1731ccbb Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Fri, 29 May 2020 12:03:02 -0700 Subject: [PATCH 6/9] Hack: Unit tests for serialization --- .../Formatter/Serialization/DefaultODataSerializerProvider.cs | 3 +++ .../Formatter/Serialization/ODataSerializerContext.cs | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs index 5576d121fc..4429423cc9 100644 --- a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs +++ b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/DefaultODataSerializerProvider.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.AspNet.OData.Common; using Microsoft.Extensions.DependencyInjection; @@ -16,6 +17,7 @@ namespace Microsoft.AspNet.OData.Formatter.Serialization /// /// The default . /// + [SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Class coupling acceptable")] public partial class DefaultODataSerializerProvider : ODataSerializerProvider { private readonly IServiceProvider _rootContainer; @@ -75,6 +77,7 @@ public override ODataEdmTypeSerializer GetEdmTypeSerializer(IEdmTypeReference ed } /// + [SuppressMessage("Microsoft.Maintainability", "CA1506:AvoidExcessiveClassCoupling", Justification = "Class coupling acceptable")] internal ODataSerializer GetODataPayloadSerializerImpl(Type type, Func modelFunction, ODataPath path, Type errorType) { if (type == null) diff --git a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs index b5665b24a0..9a651d2c77 100644 --- a/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs +++ b/src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerContext.cs @@ -284,8 +284,7 @@ private set internal IEdmTypeReference GetEdmType(object instance, Type type) { IEdmTypeReference edmType; - - if (type.IsGenericType() && EdmLibHelpers.IsDynamicTypeWrapper(type.GetGenericArguments()[0])) + if (type != null && type.IsGenericType() && EdmLibHelpers.IsDynamicTypeWrapper(type.GetGenericArguments()[0])) { return this.Path.Segments.OfType().FirstOrDefault()?.EdmType?.ToEdmTypeReference(true); } From 1a14634dabe83107928371f1d128d5490338063b Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Fri, 29 May 2020 12:03:19 -0700 Subject: [PATCH 7/9] Unit tests for flattening --- .../Expressions/AggregationBinderTests.cs | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/AggregationBinderTests.cs b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/AggregationBinderTests.cs index 3f17c587f7..3bf915b115 100644 --- a/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/AggregationBinderTests.cs +++ b/test/UnitTest/Microsoft.AspNet.OData.Test.Shared/Query/Expressions/AggregationBinderTests.cs @@ -92,8 +92,9 @@ public void SingleSum() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with sum as SupplierID)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Sum($it => $it.SupplierID)), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.SupplierID), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value))), }, })"); } [Fact] @@ -101,8 +102,9 @@ public void SingleDynamicSum() { var filters = VerifyQueryDeserialization( "aggregate(ProductProperty with sum as ProductProperty)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = ProductProperty, Value = Convert(Convert($it).Sum($it => IIF($it.ProductProperties.ContainsKey(ProductProperty), $it.ProductPropertiesProductProperty, null).SafeConvertToDecimal())), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = IIF($it.ProductProperties.ContainsKey(ProductProperty), $it.ProductPropertiesProductProperty, null), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = ProductProperty, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value).SafeConvertToDecimal())), }, })"); } [Fact] @@ -110,8 +112,9 @@ public void SingleMin() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with min as SupplierID)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Min($it => $it.SupplierID)), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.SupplierID), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Min($it => Convert($it.GroupByContainer.Value))), }, })"); } [Fact] @@ -119,8 +122,9 @@ public void SingleDynamicMin() { var filters = VerifyQueryDeserialization( "aggregate(ProductProperty with min as MinProductProperty)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = MinProductProperty, Value = Convert($it).Min($it => IIF($it.ProductProperties.ContainsKey(ProductProperty), $it.ProductPropertiesProductProperty, null)), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = IIF($it.ProductProperties.ContainsKey(ProductProperty), $it.ProductPropertiesProductProperty, null), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = MinProductProperty, Value = $it.AsQueryable().Min($it => Convert($it.GroupByContainer.Value)), }, })"); } [Fact] @@ -128,8 +132,9 @@ public void SingleMax() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with max as SupplierID)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Max($it => $it.SupplierID)), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.SupplierID), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Max($it => Convert($it.GroupByContainer.Value))), }, })"); } [Fact] @@ -137,8 +142,9 @@ public void SingleAverage() { var filters = VerifyQueryDeserialization( "aggregate(UnitPrice with average as AvgUnitPrice)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = AvgUnitPrice, Value = Convert(Convert($it).Average($it => $it.UnitPrice)), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.UnitPrice), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = AvgUnitPrice, Value = Convert($it.AsQueryable().Average($it => Convert($it.GroupByContainer.Value))), }, })"); } [Fact] @@ -146,8 +152,9 @@ public void SingleCountDistinct() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with countdistinct as Count)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = Count, Value = Convert(Convert($it).Select($it => $it.SupplierID).Distinct().LongCount()), }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.SupplierID), }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = Count, Value = Convert($it.AsQueryable().Select($it => Convert($it.GroupByContainer.Value)).Distinct().LongCount()), }, })"); } [Fact] @@ -155,8 +162,9 @@ public void MultipleAggregate() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with sum as SupplierID, CategoryID with sum as CategoryID)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new AggregationPropertyContainer() {Name = CategoryID, Value = Convert(Convert($it).Sum($it => $it.CategoryID)), Next = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Sum($it => $it.SupplierID)), }, }, })"); + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new AggregationPropertyContainer() {Name = Property1, Value = Convert($it.SupplierID), Next = new LastInChain() {Name = Property0, Value = Convert($it.CategoryID), }, }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new AggregationPropertyContainer() {Name = CategoryID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Next.Value))), Next = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value))), }, }, })"); } [Fact] @@ -166,7 +174,7 @@ public void GroupByAndAggregate() "groupby((ProductName), aggregate(SupplierID with sum as SupplierID))", ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = Convert($it.SupplierID), }, })" + ".GroupBy($it => new GroupByWrapper() {GroupByContainer = new LastInChain() {Name = ProductName, Value = $it.Source.ProductName, }, })" - + ".Select($it => new AggregationWrapper() {GroupByContainer = $it.Key.GroupByContainer, Container = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Sum($it => Convert($it.GroupByContainer.Value))), }, })"); + + ".Select($it => new AggregationWrapper() {GroupByContainer = $it.Key.GroupByContainer, Container = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value))), }, })"); } [Fact] @@ -176,7 +184,7 @@ public void GroupByAndMultipleAggregations() "groupby((ProductName), aggregate(SupplierID with sum as SupplierID, CategoryID with sum as CategoryID))", ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new AggregationPropertyContainer() {Name = Property1, Value = Convert($it.SupplierID), Next = new LastInChain() {Name = Property0, Value = Convert($it.CategoryID), }, }, })" + ".GroupBy($it => new GroupByWrapper() {GroupByContainer = new LastInChain() {Name = ProductName, Value = $it.Source.ProductName, }, })" - + ".Select($it => new AggregationWrapper() {GroupByContainer = $it.Key.GroupByContainer, Container = new AggregationPropertyContainer() {Name = CategoryID, Value = Convert(Convert($it).Sum($it => Convert($it.GroupByContainer.Next.Value))), Next = new LastInChain() {Name = SupplierID, Value = Convert(Convert($it).Sum($it => Convert($it.GroupByContainer.Value))), }, }, })"); + + ".Select($it => new AggregationWrapper() {GroupByContainer = $it.Key.GroupByContainer, Container = new AggregationPropertyContainer() {Name = CategoryID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Next.Value))), Next = new LastInChain() {Name = SupplierID, Value = Convert($it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value))), }, }, })"); } [Fact] @@ -184,8 +192,9 @@ public void ClassicEFQueryShape() { var filters = VerifyQueryDeserialization( "aggregate(SupplierID with sum as SupplierID)", - ".GroupBy($it => new NoGroupByWrapper())" - + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = $it.AsQueryable().Sum($it => $it.SupplierID), }, })", + ".Select($it => new FlatteningWrapper`1() {Source = $it, GroupByContainer = new LastInChain() {Name = Property0, Value = $it.SupplierID, }, })" + + ".GroupBy($it => new NoGroupByWrapper())" + + ".Select($it => new NoGroupByAggregationWrapper() {Container = new LastInChain() {Name = SupplierID, Value = $it.AsQueryable().Sum($it => Convert($it.GroupByContainer.Value)), }, })", classicEF: true); } From 839941499f4e9996507e8aa2d22c6ac0928de83b Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Fri, 29 May 2020 13:21:53 -0700 Subject: [PATCH 8/9] Fix EneitySet aggregation after flattening --- .../Query/Expressions/AggregationBinder.cs | 6 +++--- .../EntitySetAggregation/EntitySetAggregationTests.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs index 1450a0c57c..235d7eb037 100644 --- a/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs +++ b/src/Microsoft.AspNet.OData.Shared/Query/Expressions/AggregationBinder.cs @@ -347,11 +347,11 @@ private Expression CreateEntitySetAggregateExpression( // Create method to get property collections to aggregate MethodInfo selectManyMethod - = ExpressionHelperMethods.EnumerableSelectManyGeneric.MakeGenericMethod(baseElementType, selectedElementType); + = ExpressionHelperMethods.EnumerableSelectManyGeneric.MakeGenericMethod(baseType == this.ElementType ? this.ElementType: baseElementType, selectedElementType); // Create the lambda that acceses the property in the selectMany clause. - var selectManyParam = Expression.Parameter(baseElementType, "$it"); - var propertyExpression = Expression.Property(selectManyParam, expression.Expression.NavigationProperty.Name); + var selectManyParam = baseType == this.ElementType ? this.LambdaParameter : Expression.Parameter(baseElementType, "$it"); + var propertyExpression = Expression.Property(source, expression.Expression.NavigationProperty.Name); var selectManyLambda = Expression.Lambda(propertyExpression, selectManyParam); // Get expression to get collection of entities diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/EntitySetAggregation/EntitySetAggregationTests.cs b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/EntitySetAggregation/EntitySetAggregationTests.cs index 48d7e39f38..a5118227d1 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/EntitySetAggregation/EntitySetAggregationTests.cs +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/EntitySetAggregation/EntitySetAggregationTests.cs @@ -109,8 +109,9 @@ public async Task AggregationOnEntitySetWorksWithPropertyAggregation() HttpResponseMessage response = client.SendAsync(request).Result; // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = await response.Content.ReadAsObject(); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); var value = result["value"]; var totalId = value.First["TotalId"].ToObject(); var orders = value.First["Orders"]; From 25eca4a60182e45c55dad0a6d0a11120625a89c6 Mon Sep 17 00:00:00 2001 From: Konstantin Kosinsky Date: Fri, 29 May 2020 14:49:11 -0700 Subject: [PATCH 9/9] Disable countdistinct tests for .NET Core 3.x --- .../Aggregation/AggregationTests.cs | 2 ++ .../Microsoft.Test.E2E.AspNetCore3x.OData.csproj | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs index 5b468cfa29..b9b444a66d 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Aggregation/AggregationTests.cs @@ -624,8 +624,10 @@ public async Task MethodsNotDefinedHaveAppropriateAnswer(string query) [InlineData("?$apply=aggregate(Order/Price with min as Result)", "0")] [InlineData("?$apply=aggregate(Order/Price with max as Result)", "900")] [InlineData("?$apply=aggregate(Order/Price with average as Result)", "450")] +#if !NETCORE3x [InlineData("?$apply=aggregate(Order/Price with countdistinct as Result)", "10")] [InlineData("?$apply=aggregate(Order/Price with countdistinct as Result)&$orderby=Result", "10")] +#endif public async Task AggregateMethodWorks(string query, string expectedResult) { // Arrange diff --git a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj index b152e0f3a1..67a87e6add 100644 --- a/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj +++ b/test/E2ETest/Microsoft.Test.E2E.AspNet.OData/Build.AspNetCore3x/Microsoft.Test.E2E.AspNetCore3x.OData.csproj @@ -14,7 +14,7 @@ Microsoft.Test.E2E.AspNetCore3x.OData $(WebStackRootPath)bin\$(Configuration)\E2ETest\AspNetCore\ $(WebStackRootPath)sln\ - $(DefineConstants);NETCORE;PORTABLELIB + $(DefineConstants);NETCORE;NETCORE3x;PORTABLELIB 512 false