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

Make aliases created in compute() transformation visible for following transforms/query options #1369

Merged
merged 4 commits into from
Jan 16, 2019
Merged

Make aliases created in compute() transformation visible for following transforms/query options #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
982f58d
Ensure that we can use aliases genearted in compute on later stages
kosinsky Mar 13, 2018
e7f9b69
Ensure that we can use aliases genearted in compute on later stages
kosinsky Jan 2, 2019
6dd4921
Extra tests for compute
kosinsky Jan 2, 2019
2b5609f
Code alignment
kosinsky Jan 7, 2019
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: 1 addition & 0 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 @@ -61,6 +61,7 @@ public ApplyClause BindApply(IEnumerable<QueryToken> tokens)
case QueryTokenKind.Compute:
var compute = BindComputeToken((ComputeToken)token);
transformations.Add(compute);
state.AggregatedPropertyNames = compute.Expressions.Select(statement => statement.Alias).ToList();
break;
case QueryTokenKind.Expand:
SelectExpandClause expandClause = SelectExpandSemanticBinder.Bind(this.odataPathInfo, (ExpandToken)token, null, this.configuration);
Expand Down
22 changes: 19 additions & 3 deletions src/Microsoft.OData.Core/UriParser/Aggregation/ApplyClause.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public sealed class ApplyClause

private readonly IEnumerable<GroupByPropertyNode> lastGroupByPropertyNodes;

private readonly List<ComputeExpression> lastComputeExpressions;

/// <summary>
/// Create a ApplyClause.
/// </summary>
Expand Down Expand Up @@ -51,6 +53,11 @@ public ApplyClause(IList<TransformationNode> transformations)

break;
}
else if (transformations[i].Kind == TransformationNodeKind.Compute)
{
lastComputeExpressions = lastComputeExpressions ?? new List<ComputeExpression>();
lastComputeExpressions.AddRange((transformations[i] as ComputeTransformationNode).Expressions);
}
}
}

Expand All @@ -73,12 +80,21 @@ internal string GetContextUri()

internal List<string> GetLastAggregatedPropertyNames()
{
if (lastAggregateExpressions != null)
if (lastAggregateExpressions == null && lastComputeExpressions == null)
{
return lastAggregateExpressions.Select(statement => statement.Alias).ToList();
return null;
}

return null;
List<string> result = new List<string>();
if (lastAggregateExpressions != null)
{
result.AddRange(lastAggregateExpressions.Select(statement => statement.Alias));
}
if (lastComputeExpressions != null)
{
result.AddRange(lastComputeExpressions.Select(statement => statement.Alias));
}
return result;
}

private string CreatePropertiesUriSegment(
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Core/UriParser/Parsers/UriQueryExpressionParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public sealed class UriQueryExpressionParser
/// <summary>
/// List of supported $apply keywords
/// </summary>
private static readonly string supportedKeywords = string.Join("|", new string[] { ExpressionConstants.KeywordAggregate, ExpressionConstants.KeywordFilter, ExpressionConstants.KeywordGroupBy });
private static readonly string supportedKeywords = string.Join("|", new string[] { ExpressionConstants.KeywordAggregate, ExpressionConstants.KeywordFilter, ExpressionConstants.KeywordGroupBy, ExpressionConstants.KeywordCompute, ExpressionConstants.KeywordExpand });

/// <summary>
/// Set of parsed parameters
Expand Down
2 changes: 1 addition & 1 deletion ...alTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/ExapndOptionFunctionalTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ public void ApplyWithInvalidExpression()
{
Action action = () => this.Run("MyFriendsDogs($apply=Invalid Expression)", PersonType, PeopleSet);
var exception = Assert.Throws<ODataException>(action);
Assert.Equal("'aggregate|filter|groupby' expected at position 0 in 'Invalid Expression'.", exception.Message);
Assert.Equal("'aggregate|filter|groupby|compute|expand' expected at position 0 in 'Invalid Expression'.", exception.Message);
}
#endregion $apply

Expand Down
68 changes: 68 additions & 0 deletions ...sts/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,22 @@ public void AggregatedPropertyTreatedAsOpenPropertyInOrderBy()
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("Total");
}

[Fact]
public void ComputedPropertyTreatedAsOpenPropertyInOrderBy()
{
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(),
new Dictionary<string, string>()
{
{"$orderby", "DoubleTotal asc"},
{"$apply", "aggregate(FavoriteNumber with sum as Total)/compute(Total mul 2 as DoubleTotal)"}
});
odataQueryOptionParser.ParseApply();
var orderByClause = odataQueryOptionParser.ParseOrderBy();
orderByClause.Direction.Should().Be(OrderByDirection.Ascending);
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("DoubleTotal");
}

[Fact]
public void AggregatedPropertiesTreatedAsOpenPropertyInOrderBy()
{
Expand All @@ -1163,6 +1179,58 @@ public void AggregatedPropertiesTreatedAsOpenPropertyInOrderBy()
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("Max");
}

[Fact]
public void AggregatedAndComputePropertiesTreatedAsOpenPropertyInOrderBy()
{
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(),
new Dictionary<string, string>()
{
{"$orderby", "DoubleTotal asc, Total desc"},
{"$apply", "aggregate(FavoriteNumber with sum as Total)/compute(Total mul 2 as DoubleTotal)"}
});
odataQueryOptionParser.ParseApply();
var orderByClause = odataQueryOptionParser.ParseOrderBy();
orderByClause.Direction.Should().Be(OrderByDirection.Ascending);
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("DoubleTotal");
orderByClause = orderByClause.ThenBy;
orderByClause.Direction.Should().Be(OrderByDirection.Descending);
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("Total");
}

[Fact]
public void MultipleComputePropertiesTreatedAsOpenPropertyInOrderBy()
{
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(),
new Dictionary<string, string>()
{
{"$orderby", "DoubleTotal1 asc, DoubleTotal2 desc"},
{"$apply", "aggregate(FavoriteNumber with sum as Total)/compute(Total mul 2 as DoubleTotal1)/compute(DoubleTotal1 mul 2 as DoubleTotal2)"}
});
odataQueryOptionParser.ParseApply();
var orderByClause = odataQueryOptionParser.ParseOrderBy();
orderByClause.Direction.Should().Be(OrderByDirection.Ascending);
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("DoubleTotal1");
orderByClause = orderByClause.ThenBy;
orderByClause.Direction.Should().Be(OrderByDirection.Descending);
orderByClause.Expression.ShouldBeSingleValueOpenPropertyAccessQueryNode("DoubleTotal2");
}

[Fact]
public void ReferenceComputeAliasCreatedBeforeAggrageteThrows()
{
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(),
new Dictionary<string, string>()
{
{"$orderby", "DoubleFavorite asc"},
{"$apply", "compute(FavoriteNumber mul 2 as DoubleFavorite)/aggregate(DoubleFavorite with sum as Total)"}
});
Action parseAction = () => { odataQueryOptionParser.ParseApply(); odataQueryOptionParser.ParseOrderBy(); };
parseAction.ShouldThrow<ODataException>().WithMessage(ODataErrorStrings.MetadataBinder_PropertyNotDeclared("Fully.Qualified.Namespace.Person", "DoubleFavorite"));
}

[Fact]
public void ActionsThrowOnClosedInOrderby()
{
Expand Down
24 changes: 24 additions & 0 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 @@ -247,6 +247,30 @@ public void BindApplyWitMultipleTokensShouldReturnApplyClause()
scecondAggregate.Should().NotBeNull();
}

[Fact]
public void BindApplyWithComputeShouldReturnApplyClause()
{
var tokens = _parser.ParseApply("compute(UnitPrice mul 5 as BigPrice)");

var binder = new ApplyBinder(FakeBindMethods.BindSingleComplexProperty, _bindingState);
var actual = binder.BindApply(tokens);

actual.Should().NotBeNull();
actual.Transformations.Should().HaveCount(1);

var transformations = actual.Transformations.ToList();
var compute = transformations[0] as ComputeTransformationNode;

compute.Should().NotBeNull();
compute.Kind.Should().Be(TransformationNodeKind.Compute);
compute.Expressions.Should().HaveCount(1);

var statements = compute.Expressions.ToList();
var statement = statements[0];
VerifyIsFakeSingleValueNode(statement.Expression);
statement.Alias.ShouldBeEquivalentTo("BigPrice");
}

[Fact]
public void BindApplyWithEntitySetAggregationReturnApplyClause()
{
Expand Down
28 changes: 26 additions & 2 deletions ...tionalTests/Microsoft.OData.Core.Tests/UriParser/Parsers/UriQueryExpressionParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void ParseApplyWithInvalidTransformationIdentifierShouldThrow()
{
string apply = "invalid(UnitPrice with sum as TotalPrice)";
Action parse = () => this.testSubject.ParseApply(apply);
parse.ShouldThrow<ODataException>().Where(e => e.Message == ErrorStrings.UriQueryExpressionParser_KeywordOrIdentifierExpected("aggregate|filter|groupby",0,apply));
parse.ShouldThrow<ODataException>().Where(e => e.Message == ErrorStrings.UriQueryExpressionParser_KeywordOrIdentifierExpected("aggregate|filter|groupby|compute|expand", 0,apply));
}

[Fact]
Expand Down Expand Up @@ -407,7 +407,7 @@ public void ParseApplyWithGroupByAndAggregateMissingCloseParenShouldThrow()
{
string apply = "groupBy((UnitPrice), aggregate(UnitPrice with sum as TotalPrice)";
Action parse = () => this.testSubject.ParseApply(apply);
parse.ShouldThrow<ODataException>().Where(e => e.Message == ErrorStrings.UriQueryExpressionParser_KeywordOrIdentifierExpected("aggregate|filter|groupby", 0, apply));
parse.ShouldThrow<ODataException>().Where(e => e.Message == ErrorStrings.UriQueryExpressionParser_KeywordOrIdentifierExpected("aggregate|filter|groupby|compute|expand", 0, apply));
}

[Fact]
Expand Down Expand Up @@ -653,6 +653,30 @@ public void ParseApplyWithNestedFunctionAggregation()
funcToken.Name.ShouldBeEquivalentTo("cast");
}

[Fact]
public void ParseApplyWithComputeWithMathematicalOperations()
{
string compute = "compute(Prop1 mul Prop2 as Product,Prop1 div Prop2 as Ratio,Prop2 mod Prop2 as Remainder)";
ComputeToken token = this.testSubject.ParseApply(compute).First() as ComputeToken;
token.Kind.ShouldBeEquivalentTo(QueryTokenKind.Compute);
List<ComputeExpressionToken> tokens = token.Expressions.ToList();
tokens.Count.Should().Be(3);
tokens[0].Kind.ShouldBeEquivalentTo(QueryTokenKind.ComputeExpression);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's more clear to create a private method to verify # 1, # 2, # 3 separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have test ParseComputeWithMathematicalOperations that checks $compute for that case. I'm adding the same for compute()

tokens[1].Kind.ShouldBeEquivalentTo(QueryTokenKind.ComputeExpression);
tokens[2].Kind.ShouldBeEquivalentTo(QueryTokenKind.ComputeExpression);
tokens[0].Alias.ShouldBeEquivalentTo("Product");
tokens[1].Alias.ShouldBeEquivalentTo("Ratio");
tokens[2].Alias.ShouldBeEquivalentTo("Remainder");
(tokens[0].Expression as BinaryOperatorToken).OperatorKind.ShouldBeEquivalentTo(BinaryOperatorKind.Multiply);
(tokens[1].Expression as BinaryOperatorToken).OperatorKind.ShouldBeEquivalentTo(BinaryOperatorKind.Divide);
(tokens[2].Expression as BinaryOperatorToken).OperatorKind.ShouldBeEquivalentTo(BinaryOperatorKind.Modulo);

Action accept1 = () => tokens[0].Accept<ComputeExpression>(null);
accept1.ShouldThrow<NotImplementedException>();
Action accept2 = () => token.Accept<ComputeExpression>(null);
accept2.ShouldThrow<NotImplementedException>();
}

[Fact]
public void ParseComputeWithMathematicalOperations()
{
Expand Down