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

Fix grouping by nested dynamic property to behave like grouping by nested standard property #2971

Merged
merged 5 commits into from
May 23, 2024
Merged

Fix grouping by nested dynamic property to behave like grouping by nested standard property #2971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb6047b
Attempt
clemvnt May 8, 2024
53be4d9
Fix redundant cast
clemvnt May 20, 2024
8240a64
Add test
May 20, 2024
8e730a7
Update test/FunctionalTests/Microsoft.OData.Core.Tests/UriParser/Exte…
gathogojr May 23, 2024
b907709
Fix typo
gathogojr May 23, 2024
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
35 changes: 18 additions & 17 deletions src/Microsoft.OData.Core/UriParser/Aggregation/ApplyBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,30 +230,23 @@ private GroupByTransformationNode BindGroupByToken(GroupByToken token)
foreach (EndPathToken propertyToken in token.Properties)
{
QueryNode bindResult = this.bindMethod(propertyToken);
SingleValuePropertyAccessNode property = bindResult as SingleValuePropertyAccessNode;
SingleComplexNode complexProperty = bindResult as SingleComplexNode;

if (property != null)
if (bindResult is SingleValuePropertyAccessNode property)
{
RegisterProperty(properties, ReversePropertyPath(property));
}
else if (complexProperty != null)
else if (bindResult is SingleComplexNode complexProperty)
{
RegisterProperty(properties, ReversePropertyPath(complexProperty));
}
else if (bindResult is SingleValueOpenPropertyAccessNode openProperty)
{
RegisterProperty(properties, ReversePropertyPath(openProperty));
}
else
{
SingleValueOpenPropertyAccessNode openProperty = bindResult as SingleValueOpenPropertyAccessNode;
if (openProperty != null)
{
IEdmTypeReference type = GetTypeReferenceByPropertyName(openProperty.Name);
properties.Add(new GroupByPropertyNode(openProperty.Name, openProperty, type));
}
else
{
throw new ODataException(
ODataErrorStrings.ApplyBinder_GroupByPropertyNotPropertyAccessValue(propertyToken.Identifier));
}
throw new ODataException(
ODataErrorStrings.ApplyBinder_GroupByPropertyNotPropertyAccessValue(propertyToken.Identifier));
}
}

Expand Down Expand Up @@ -299,11 +292,15 @@ private static Stack<SingleValueNode> ReversePropertyPath(SingleValueNode node)
}
else if (node.Kind == QueryNodeKind.SingleComplexNode)
{
node = (SingleValueNode)((SingleComplexNode)node).Source;
node = ((SingleComplexNode)node).Source;
}
else if (node.Kind == QueryNodeKind.SingleNavigationNode)
{
node = ((SingleNavigationNode)node).Source as SingleValueNode;
node = ((SingleNavigationNode)node).Source;
}
else if (node.Kind == QueryNodeKind.SingleValueOpenPropertyAccess)
{
node = ((SingleValueOpenPropertyAccessNode)node).Source;
}
}
while (node != null && IsPropertyNode(node));
Expand Down Expand Up @@ -350,6 +347,10 @@ private static string GetNodePropertyName(SingleValueNode property)
{
return ((SingleNavigationNode)property).NavigationProperty.Name;
}
else if (property.Kind == QueryNodeKind.SingleValueOpenPropertyAccess)
{
return ((SingleValueOpenPropertyAccessNode)property).Name;
}
else
{
throw new NotSupportedException();
Expand Down
105 changes: 67 additions & 38 deletions ...nctionalTests/Microsoft.OData.Core.Tests/UriParser/Extensions/Binders/ApplyBinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ public void BindApplyWithCountInAggregateShouldReturnApplyClause()
public void BindApplyWithAggregateAndFilterShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("aggregate(StockQuantity with sum as TotalPrice)/filter(TotalPrice eq 100)");
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -124,7 +124,7 @@ public void BindApplyWithAggregateAndFilterShouldReturnApplyClause()
Assert.NotNull(binaryOperation.Left);
ConvertNode propertyConvertNode = Assert.IsType<ConvertNode>(binaryOperation.Left);
Assert.NotNull(propertyConvertNode.Source);
SingleValueOpenPropertyAccessNode propertyAccess = Assert.IsType< SingleValueOpenPropertyAccessNode>(propertyConvertNode.Source);
SingleValueOpenPropertyAccessNode propertyAccess = Assert.IsType<SingleValueOpenPropertyAccessNode>(propertyConvertNode.Source);
Assert.Equal("TotalPrice", propertyAccess.Name);
}

Expand Down Expand Up @@ -185,9 +185,9 @@ public void BindApplyWitGroupByWithComplexShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyAddress/City))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -208,14 +208,43 @@ public void BindApplyWitGroupByWithComplexShouldReturnApplyClause()
Assert.Empty(cityNode.ChildTransformations);
}

[Fact]
public void BindApplyWitGroupByWithOpenComplexShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyOpenAddress/City))");

MetadataBinder metadataBinder = new MetadataBinder(_bindingState);


ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
TransformationNode transformation = Assert.Single(actual.Transformations);
GroupByTransformationNode groupBy = Assert.IsType<GroupByTransformationNode>(transformation);

Assert.Equal(TransformationNodeKind.GroupBy, groupBy.Kind);
Assert.Null(groupBy.ChildTransformations);
Assert.NotNull(groupBy.GroupingProperties);
GroupByPropertyNode addressNode = Assert.Single(groupBy.GroupingProperties);

Assert.Equal("MyOpenAddress", addressNode.Name);
Assert.Null(addressNode.Expression);

GroupByPropertyNode cityNode = Assert.Single(addressNode.ChildTransformations);
Assert.Equal("City", cityNode.Name);
Assert.NotNull(cityNode.Expression);
Assert.Empty(cityNode.ChildTransformations);
}

[Fact]
public void BindApplyWitGroupByWithDeepNavigationShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyDog/FastestOwner/FirstName))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -245,9 +274,9 @@ public void BindApplyWitGroupByWithDeepComplexShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyAddress/NextHome/City))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -278,9 +307,9 @@ public void BindApplyWitGroupByWithNavigationAndComplexShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyFavoritePainting/ArtistAddress/City))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -311,9 +340,9 @@ public void BindApplyWitGroupByWithComplexAndNavigationShouldReturnApplyClause()
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyAddress/PostBoxPainting/Artist))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -344,9 +373,9 @@ public void BindApplyWitGroupByWithDeepNavigationAndComplexShouldReturnApplyClau
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyDog/LionWhoAteMe/LionHeartbeat/Frequency))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -381,9 +410,9 @@ public void BindApplyWitGroupByWithDeepComplexAndNavigationShouldReturnApplyClau
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyAddress/NextHome/PostBoxPainting/Artist))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -418,9 +447,9 @@ public void BindApplyWitGroupByWithComplexAndDeepNavigationAndComplexShouldRetur
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyAddress/PostBoxPainting/Owner/MyAddress/City))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -459,9 +488,9 @@ public void BindApplyWitGroupByWithNavigationAndDeepComplexAndNavigationShouldRe
{
IEnumerable<QueryToken> tokens = _parser.ParseApply("groupby((MyFavoritePainting/ArtistAddress/NextHome/PostBoxPainting/Artist))");

MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -551,9 +580,9 @@ public void BindApplyWithComputeAfterGroupByShouldReturnApplyClause()
var tokens = _parser.ParseApply("groupby((ID, SSN), aggregate(LifeTime with sum as TotalLife))/compute(TotalLife as TotalLife2)");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
var actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -571,9 +600,9 @@ public void BindApplyWithMultipleGroupBysShouldReturnApplyClause()
var tokens = _parser.ParseApply("groupby((MyDog/Color, MyDog/Breed))/groupby((MyDog/Color), aggregate(MyDog/Breed with max as MaxBreed))");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
var actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -590,9 +619,9 @@ public void BindApplyWitMultipleTokensShouldReturnApplyClause()
"groupby((ID, SSN, LifeTime))/aggregate(LifeTime with sum as TotalLife)/groupby((TotalLife))/aggregate(TotalLife with sum as TotalTotalLife)");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -639,9 +668,9 @@ public void BindApplyWithEntitySetAggregationReturnApplyClause()
"groupby((LifeTime),aggregate(MyPaintings($count as Count)))");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -653,7 +682,7 @@ public void BindApplyWithEntitySetAggregationReturnApplyClause()
AggregateTransformationNode aggregate = Assert.IsType<AggregateTransformationNode>(groupBy.ChildTransformations);
Assert.NotNull(aggregate.AggregateExpressions);

Assert.IsType< EntitySetAggregateExpression>(Assert.Single(aggregate.AggregateExpressions));
Assert.IsType<EntitySetAggregateExpression>(Assert.Single(aggregate.AggregateExpressions));
}

[Fact]
Expand All @@ -664,9 +693,9 @@ public void BindApplyWithEntitySetAggregationWithoutGroupByReturnApplyClause()
"aggregate(MyPaintings(Value with sum as TotalValue))");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -686,9 +715,9 @@ public void BindApplyWithExpandReturnApplyClause()
"expand(MyPaintings, filter(FrameColor eq 'Red'))/groupby((LifeTime),aggregate(MyPaintings($count as Count)))");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState, V4configuration, new ODataPathInfo(HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()));
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState, V4configuration, new ODataPathInfo(HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()));
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down Expand Up @@ -717,9 +746,9 @@ public void BindApplyWithNestedExpandReturnApplyClause()
"expand(MyPaintings, filter(FrameColor eq 'Red'), expand(Owner, filter(Name eq 'Me')))");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState, V4configuration, new ODataPathInfo(HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()));
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState, V4configuration, new ODataPathInfo(HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()));
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand All @@ -744,9 +773,9 @@ public void BindVirtualPropertiesAfterCollapseReturnsApplyClause()
"groupby((ID))/aggregate($count as Count)");

BindingState state = new BindingState(_configuration);
MetadataBinder metadataBiner = new MetadataBinder(_bindingState);
MetadataBinder metadataBinder = new MetadataBinder(_bindingState);

ApplyBinder binder = new ApplyBinder(metadataBiner.Bind, _bindingState);
ApplyBinder binder = new ApplyBinder(metadataBinder.Bind, _bindingState);
ApplyClause actual = binder.BindApply(tokens);

Assert.NotNull(actual);
Expand Down