-
Notifications
You must be signed in to change notification settings - Fork 359
Add support for collection parameters in URI functions #3444
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.
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
base: main
Are you sure you want to change the base?
Add support for collection parameters in URI functions #3444
Conversation
1b0ab84 to
a06f317
Compare
6504a2e to
13346b1
Compare
13346b1 to
2cb423d
Compare
| /// Promotes types of arguments to match signature if possible. | ||
| /// </summary> | ||
| /// <param name="signature">The signature to match the types to.</param> | ||
| /// <param name="argumentNodes">The types to promote.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update this comment as well
WanjohiSammy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| OrderByClause orderBy = this.ParseOrderBy(); | ||
| SearchClause search = this.ParseSearch(); | ||
| ApplyClause apply = this.ParseApply(); | ||
| ComputeClause compute = this.ParseCompute(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not move 'ParseApply()'?
ParseApply() should be called first
xuzhg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
| for (int i = 0; i < candidate.Value.ArgumentTypes.Length; i++) | ||
| { | ||
| if (!CanPromoteNodeTo(argumentNodes[i], argumentTypes[i], candidate.Value.ArgumentTypes[i])) | ||
| if (!CanPromoteNodeTo(argumentMetadata[i].Node, argumentMetadata[i].TypeReference, candidate.Value.ArgumentTypes[i])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this code do?
| { | ||
| object result = ODataUriUtils.ConvertFromUriLiteral( | ||
| value: constantNode.LiteralText, | ||
| version: ODataVersion.V4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the behaviour be the same for v4.01?
| if(enumType.TryParse(memberIntegralValue, out IEdmEnumMember enumMember)) | ||
| { | ||
| string literalText = ODataUriUtils.ConvertToUriLiteral(enumMember.Name, default(ODataVersion)); | ||
| string literalText = ODataUriUtils.ConvertToUriLiteral(enumMember.Name, default); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
| if (convertedNode == null) | ||
| { | ||
| // Try to keep original literal text if meaningful | ||
| string literal = item.LiteralText ?? ODataUriUtils.ConvertToUriLiteral(item.Value, ODataVersion.V4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the behaviour be the same on v4.01?
Issues
This pull request fixes #3385.
Description
This pull request adds first-class support for passing collection-valued properties and collection-valued literals as parameters to functions invoked inside the
$computequery option, and fixes a parse-order defect that prevented selecting a computed property in$selectwhen defined via$compute.Before these changes:
$computecould only reliably accept single-valued parameters (primitive/scalar).[1,2,3]) was not consistently recognized as a semantic collection.$selectreferencing a newly introduced alias from$compute(e.g.&$select=Name,Bonus) could throw, because$select/$expandwas parsed before$compute, so the alias and function call hadn’t been materialized yet.Key enhancements
Collection-valued property parameters in
$computeCustomUriFunctions.AddCustomUriFunction) or model function inside$compute.$compute=calculateTaxablePay(Salary,OvertimeHours) as TaxablePay&$select=Name,Salary,TaxablePay.Collection-valued literal parameters
$computecan accept a bracketed collection literal as a parameter.$$compute=calculateSalaryBand(Salary,[5000,7000,9000]) as SalaryBand&$select=Name,SalaryBand.Edm function collection parameters
$compute=Default.CalculateOvertimePay(salary=Salary,overtimeHours=OvertimeHours) as OvertimePay&$select=Name,Salary,OvertimePay.$compute=Default.GetRating(scale=[1,2,3,4,5]) as Rating&$select=Name,Rating.Parse order fix in
ODataUriParser.ParseUriParseCompute()is now invoked beforeParseSelectAndExpand().$selectis processed, eliminating the previous exception when selecting a computed property.Internal changes
ParseUriprevents premature$selectbinding on non-existent aliases.Backward compatibility
$computeare unaffected.ParseComputebeforeParseSelectExpand) only resolves previously failing scenarios; no semantic regressions expected.Checklist (Uncheck if it is not completed)
Additional work necessary