Fixes #1126: enable built-in expression in $orderby (#1150)

* Fixes #1126: enable built-in expression in $orderby * Fixes the failing test case * Add the comments * Update src/Microsoft.AspNetCore.OData/Query/Validator/OrderByQueryValidator.cs Co-authored-by: John Gathogo <john.gathogo@microsoft.com> * Apply suggestions from code review Co-authored-by: John Gathogo <john.gathogo@microsoft.com> --------- Co-authored-by: John Gathogo <john.gathogo@microsoft.com>
OData · Jan 11, 2024 · e02297c · e02297c
1 parent c4de3fa
commit e02297c
Show file tree

Hide file tree

Showing 26 changed files with 1,626 additions and 76 deletions.
diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
diff --git a/src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs b/src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs
diff --git a/src/Microsoft.AspNetCore.OData/Properties/SRResources.resx b/src/Microsoft.AspNetCore.OData/Properties/SRResources.resx
@@ -453,6 +453,9 @@
   <data name="NotAllowedFunction" xml:space="preserve">
     <value>Function '{0}' is not allowed. To allow it, set the '{1}' property on EnableQueryAttribute or QueryValidationSettings.</value>
   </data>
+  <data name="OrderByClauseInvalid" xml:space="preserve">
+    <value>OrderBy clause kind '{0}' is not valid. Only kind '{1}' is accepted.</value>
+  </data>
   <data name="OrderByClauseNotSupported" xml:space="preserve">
     <value>Only ordering by properties is supported for non-primitive collections. Expressions are not supported.</value>
   </data>

diff --git a/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt b/src/Microsoft.AspNetCore.OData/PublicAPI.Unshipped.txt
@@ -1027,10 +1027,15 @@ Microsoft.AspNetCore.OData.Query.ODataRawQueryOptions.Select.get -> string
 Microsoft.AspNetCore.OData.Query.ODataRawQueryOptions.Skip.get -> string
 Microsoft.AspNetCore.OData.Query.ODataRawQueryOptions.SkipToken.get -> string
 Microsoft.AspNetCore.OData.Query.ODataRawQueryOptions.Top.get -> string
+Microsoft.AspNetCore.OData.Query.OrderByClauseNode
+Microsoft.AspNetCore.OData.Query.OrderByClauseNode.OrderByClause.get -> Microsoft.OData.UriParser.OrderByClause
+Microsoft.AspNetCore.OData.Query.OrderByClauseNode.OrderByClauseNode(Microsoft.OData.UriParser.OrderByClause orderByClause) -> void
 Microsoft.AspNetCore.OData.Query.OrderByCountNode
 Microsoft.AspNetCore.OData.Query.OrderByCountNode.OrderByClause.get -> Microsoft.OData.UriParser.OrderByClause
 Microsoft.AspNetCore.OData.Query.OrderByCountNode.OrderByCountNode(Microsoft.OData.UriParser.OrderByClause orderByClause) -> void
 Microsoft.AspNetCore.OData.Query.OrderByItNode
+Microsoft.AspNetCore.OData.Query.OrderByItNode.Name.get -> string
+Microsoft.AspNetCore.OData.Query.OrderByItNode.OrderByItNode(Microsoft.OData.UriParser.OrderByClause clause) -> void
 Microsoft.AspNetCore.OData.Query.OrderByItNode.OrderByItNode(Microsoft.OData.UriParser.OrderByDirection direction) -> void
 Microsoft.AspNetCore.OData.Query.OrderByNode
 Microsoft.AspNetCore.OData.Query.OrderByNode.Direction.get -> Microsoft.OData.UriParser.OrderByDirection
@@ -1178,6 +1183,12 @@ Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings.MaxTop.set ->
 Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings.ODataValidationSettings() -> void
 Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator
 Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.OrderByQueryValidator() -> void
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext.IncrementNodeCount() -> void
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext.OrderBy.get -> Microsoft.AspNetCore.OData.Query.OrderByQueryOption
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext.OrderBy.set -> void
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext.OrderByNodeCount.get -> int
+Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext.OrderByValidatorContext() -> void
 Microsoft.AspNetCore.OData.Query.Validator.QueryValidatorContext
 Microsoft.AspNetCore.OData.Query.Validator.QueryValidatorContext.Context.get -> Microsoft.AspNetCore.OData.Query.ODataQueryContext
 Microsoft.AspNetCore.OData.Query.Validator.QueryValidatorContext.Context.set -> void
@@ -1960,6 +1971,30 @@ virtual Microsoft.AspNetCore.OData.Query.Validator.FilterQueryValidator.Validate
 virtual Microsoft.AspNetCore.OData.Query.Validator.FilterQueryValidator.ValidateUnaryOperatorNode(Microsoft.OData.UriParser.UnaryOperatorNode unaryOperatorNode, Microsoft.AspNetCore.OData.Query.Validator.FilterValidatorContext validatorContext) -> void
 virtual Microsoft.AspNetCore.OData.Query.Validator.ODataQueryValidator.Validate(Microsoft.AspNetCore.OData.Query.ODataQueryOptions options, Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings validationSettings) -> void
 virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.Validate(Microsoft.AspNetCore.OData.Query.OrderByQueryOption orderByOption, Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings validationSettings) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateAllNode(Microsoft.OData.UriParser.AllNode allNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateAnyNode(Microsoft.OData.UriParser.AnyNode anyNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateBinaryOperatorNode(Microsoft.OData.UriParser.BinaryOperatorNode binaryOperatorNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateCollectionComplexNode(Microsoft.OData.UriParser.CollectionComplexNode collectionComplexNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateCollectionNode(Microsoft.OData.UriParser.CollectionNode node, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateCollectionPropertyAccessNode(Microsoft.OData.UriParser.CollectionPropertyAccessNode propertyAccessNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateCollectionResourceCastNode(Microsoft.OData.UriParser.CollectionResourceCastNode collectionResourceCastNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateConstantNode(Microsoft.OData.UriParser.ConstantNode constantNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateConvertNode(Microsoft.OData.UriParser.ConvertNode convertNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateCountNode(Microsoft.OData.UriParser.CountNode countNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateInNode(Microsoft.OData.UriParser.InNode inNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateNavigationPropertyNode(Microsoft.OData.UriParser.CollectionNavigationNode collectionNavigation, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateNavigationPropertyNode(Microsoft.OData.UriParser.SingleNavigationNode singleNavigation, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateOrderBy(Microsoft.OData.UriParser.OrderByClause orderByClause, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateQueryNode(Microsoft.OData.UriParser.QueryNode node, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateRangeVariable(Microsoft.OData.UriParser.RangeVariable rangeVariable, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleComplexNode(Microsoft.OData.UriParser.SingleComplexNode singleComplexNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleResourceCastNode(Microsoft.OData.UriParser.SingleResourceCastNode singleResourceCastNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleResourceFunctionCallNode(Microsoft.OData.UriParser.SingleResourceFunctionCallNode node, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleValueFunctionCallNode(Microsoft.OData.UriParser.SingleValueFunctionCallNode node, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleValueNode(Microsoft.OData.UriParser.SingleValueNode node, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext, bool skipRangeVariable) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleValueOpenPropertyNode(Microsoft.OData.UriParser.SingleValueOpenPropertyAccessNode openPropertyNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateSingleValuePropertyAccessNode(Microsoft.OData.UriParser.SingleValuePropertyAccessNode propertyAccessNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
+virtual Microsoft.AspNetCore.OData.Query.Validator.OrderByQueryValidator.ValidateUnaryOperatorNode(Microsoft.OData.UriParser.UnaryOperatorNode unaryOperatorNode, Microsoft.AspNetCore.OData.Query.Validator.OrderByValidatorContext validatorContext) -> void
 virtual Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.Validate(Microsoft.AspNetCore.OData.Query.SelectExpandQueryOption selectExpandQueryOption, Microsoft.AspNetCore.OData.Query.Validator.ODataValidationSettings validationSettings) -> void
 virtual Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateExpandedCountSelectItem(Microsoft.OData.UriParser.ExpandedCountSelectItem expandCountItem, Microsoft.AspNetCore.OData.Query.Validator.SelectExpandValidatorContext validatorContext) -> void
 virtual Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateExpandedNavigationSelectItem(Microsoft.OData.UriParser.ExpandedNavigationSelectItem expandItem, Microsoft.AspNetCore.OData.Query.Validator.SelectExpandValidatorContext validatorContext) -> void

diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByClauseNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByClauseNode.cs
@@ -0,0 +1,37 @@
+//-----------------------------------------------------------------------------
+// <copyright file="OrderByClauseNode.cs" company=".NET Foundation">
+//      Copyright (c) .NET Foundation and Contributors. All rights reserved.
+//      See License.txt in the project root for license information.
+// </copyright>
+//------------------------------------------------------------------------------
+
+using Microsoft.OData.UriParser;
+
+namespace Microsoft.AspNetCore.OData.Query
+{
+    /// <summary>
+    /// Represents the order by expression in the $orderby clause.
+    /// Use this to represent other $orderby except 'Property,OpenProperty,$count, $it' orderBy expression.
+    /// </summary>
+    /// <remarks>
+    /// Again, in the next major release, we don't need this class.
+    /// Track on it at: https://github.com/OData/AspNetCoreOData/issues/1153
+    /// </remarks>
+    public class OrderByClauseNode : OrderByNode
+    {
+        /// <summary>
+        /// Instantiates a new instance of <see cref="OrderByClauseNode"/> class.
+        /// </summary>
+        /// <param name="orderByClause">The order by clause.</param>
+        public OrderByClauseNode(OrderByClause orderByClause)
+            : base(orderByClause)
+        {
+            OrderByClause = orderByClause;
+        }
+
+        /// <summary>
+        /// Gets the <see cref="OrderByClause"/> of this node.
+        /// </summary>
+        public OrderByClause OrderByClause { get; }
+    }
+}
diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByCountNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByCountNode.cs
@@ -5,7 +5,7 @@
 // </copyright>
 //------------------------------------------------------------------------------
 
-using System;
+using Microsoft.OData;
 using Microsoft.OData.Edm;
 using Microsoft.OData.UriParser;
 
@@ -21,9 +21,13 @@ public class OrderByCountNode : OrderByNode
         /// </summary>
         /// <param name="orderByClause">The orderby clause representing property access.</param>
         public OrderByCountNode(OrderByClause orderByClause)
+            : base(orderByClause)
         {
-            OrderByClause = orderByClause ?? throw Error.ArgumentNull(nameof(orderByClause));
-            Direction = orderByClause.Direction;
+            OrderByClause = orderByClause;
+            if (!(orderByClause.Expression is CountNode))
+            {
+                throw new ODataException(string.Format(SRResources.OrderByClauseInvalid, orderByClause.Expression.Kind, QueryNodeKind.Count));
+            }
         }
 
         /// <summary>

diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByItNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByItNode.cs
@@ -5,6 +5,7 @@
 // </copyright>
 //------------------------------------------------------------------------------
 
+using Microsoft.OData;
 using Microsoft.OData.UriParser;
 
 namespace Microsoft.AspNetCore.OData.Query
@@ -21,6 +22,39 @@ public class OrderByItNode : OrderByNode
         public OrderByItNode(OrderByDirection direction)
             : base(direction)
         {
+            Name = "$it";
         }
+
+        /// <summary>
+        /// Instantiates a new instance of <see cref="OrderByItNode"/> class.
+        /// </summary>
+        /// <param name="clause">The orderby clause.</param>
+        public OrderByItNode(OrderByClause clause)
+            : base(clause)
+        {
+            if (clause == null)
+            {
+                throw Error.ArgumentNull(nameof(clause));
+            }
+
+            if (clause.Expression is NonResourceRangeVariableReferenceNode nonResourceVarNode)
+            {
+                Name = nonResourceVarNode.Name;
+            }
+            else if (clause.Expression is ResourceRangeVariableReferenceNode resourceVarNode)
+            {
+                Name = resourceVarNode.Name;
+            }
+            else
+            {
+                throw new ODataException(string.Format(SRResources.OrderByClauseInvalid, clause.Expression.Kind,
+                    "NonResourceRangeVariableReferenceNode or ResourceRangeVariableReferenceNode"));
+            }
+        }
+
+        /// <summary>
+        /// Gets the range variable name
+        /// </summary>
+        public string Name { get; }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByNode.cs
@@ -15,6 +15,10 @@ namespace Microsoft.AspNetCore.OData.Query
     /// <summary>
     /// Represents a single order by expression in the $orderby clause.
     /// </summary>
+    /// <remarks>
+    /// Why do we need this class and its derived type? only fetch the PropertyPath? In the next major release, we can consider to remove all of these.
+    /// Track on it at: https://github.com/OData/AspNetCoreOData/issues/1153
+    /// </remarks>
     public abstract class OrderByNode
     {
         /// <summary>
@@ -42,10 +46,6 @@ protected OrderByNode(OrderByClause orderByClause)
             PropertyPath = RestorePropertyPath(orderByClause.Expression);
         }
 
-        internal OrderByNode()
-        {
-        }
-
         /// <summary>
         /// Gets the <see cref="OrderByDirection"/> for the current node.
         /// </summary>
@@ -66,24 +66,25 @@ public static IList<OrderByNode> CreateCollection(OrderByClause orderByClause)
                 if (clause.Expression is CountNode)
                 {
                     result.Add(new OrderByCountNode(clause));
-                    continue;
                 }
-
-                if (clause.Expression is NonResourceRangeVariableReferenceNode ||
+                else if (clause.Expression is NonResourceRangeVariableReferenceNode ||
                     clause.Expression is ResourceRangeVariableReferenceNode)
                 {
-                    result.Add(new OrderByItNode(clause.Direction));
-                    continue;
+                    result.Add(new OrderByItNode(clause));
                 }
-
-                if (clause.Expression is SingleValueOpenPropertyAccessNode)
+                else if (clause.Expression is SingleValueOpenPropertyAccessNode)
                 {
                     result.Add(new OrderByOpenPropertyNode(clause));
                 }
-                else
+                else if(clause.Expression is SingleValuePropertyAccessNode)
                 {
                     result.Add(new OrderByPropertyNode(clause));
                 }
+                else
+                {
+                    // For other, let's create a wrapper. In next major release, we don't need this wrapper.
+                    result.Add(new OrderByClauseNode(clause));
+                }
             }
 
             return result;

diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByOpenPropertyNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByOpenPropertyNode.cs
@@ -27,8 +27,7 @@ public OrderByOpenPropertyNode(OrderByClause orderByClause)
             var openPropertyExpression = orderByClause.Expression as SingleValueOpenPropertyAccessNode;
             if (openPropertyExpression == null)
             {
-                // TODO: need refactor the error message
-                throw new ODataException(SRResources.OrderByClauseNotSupported);
+                throw new ODataException(string.Format(SRResources.OrderByClauseInvalid, orderByClause.Expression.Kind, QueryNodeKind.SingleValueOpenPropertyAccess));
             }
 
             PropertyName = openPropertyExpression.Name;

diff --git a/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByPropertyNode.cs b/src/Microsoft.AspNetCore.OData/Query/Nodes/OrderByPropertyNode.cs
@@ -34,7 +34,7 @@ public OrderByPropertyNode(OrderByClause orderByClause)
             SingleValuePropertyAccessNode propertyExpression = orderByClause.Expression as SingleValuePropertyAccessNode;
             if (propertyExpression == null)
             {
-                throw new ODataException(SRResources.OrderByClauseNotSupported);
+                throw new ODataException(string.Format(SRResources.OrderByClauseInvalid, orderByClause.Expression.Kind, QueryNodeKind.SingleValuePropertyAccess));
             }
             else
             {