diff --git a/src/Microsoft.OData.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/src/Microsoft.OData.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt index e69de29bb2..f9275e026f 100644 --- a/src/Microsoft.OData.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/src/Microsoft.OData.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -0,0 +1,11 @@ +Microsoft.OData.UriParser.PathCountSelectItem +Microsoft.OData.UriParser.PathCountSelectItem.Filter.get -> Microsoft.OData.UriParser.FilterClause +Microsoft.OData.UriParser.PathCountSelectItem.NavigationSource.get -> Microsoft.OData.Edm.IEdmNavigationSource +Microsoft.OData.UriParser.PathCountSelectItem.PathCountSelectItem(Microsoft.OData.UriParser.ODataSelectPath selectedPath) -> void +Microsoft.OData.UriParser.PathCountSelectItem.PathCountSelectItem(Microsoft.OData.UriParser.ODataSelectPath selectedPath, Microsoft.OData.Edm.IEdmNavigationSource navigationSource, Microsoft.OData.UriParser.FilterClause filter, Microsoft.OData.UriParser.SearchClause search) -> void +Microsoft.OData.UriParser.PathCountSelectItem.Search.get -> Microsoft.OData.UriParser.SearchClause +Microsoft.OData.UriParser.PathCountSelectItem.SelectedPath.get -> Microsoft.OData.UriParser.ODataSelectPath +override Microsoft.OData.UriParser.PathCountSelectItem.HandleWith(Microsoft.OData.UriParser.SelectItemHandler handler) -> void +override Microsoft.OData.UriParser.PathCountSelectItem.TranslateWith(Microsoft.OData.UriParser.SelectItemTranslator translator) -> T +virtual Microsoft.OData.UriParser.SelectItemHandler.Handle(Microsoft.OData.UriParser.PathCountSelectItem item) -> void +virtual Microsoft.OData.UriParser.SelectItemTranslator.Translate(Microsoft.OData.UriParser.PathCountSelectItem item) -> T \ No newline at end of file diff --git a/src/Microsoft.OData.Core/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/src/Microsoft.OData.Core/PublicAPI/net9.0/PublicAPI.Unshipped.txt index 85bb724ee9..6e37f2b631 100644 --- a/src/Microsoft.OData.Core/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/src/Microsoft.OData.Core/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -2488,4 +2488,15 @@ static Microsoft.OData.UriParser.ODataPathExtensions.ToResourcePathString(this M static Microsoft.OData.UriParser.ODataPathExtensions.TrimEndingKeySegment(this Microsoft.OData.UriParser.ODataPath path) -> Microsoft.OData.UriParser.ODataPath static Microsoft.OData.UriParser.ODataPathExtensions.TrimEndingTypeSegment(this Microsoft.OData.UriParser.ODataPath path) -> Microsoft.OData.UriParser.ODataPath Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool -Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void \ No newline at end of file +Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void +Microsoft.OData.UriParser.PathCountSelectItem +Microsoft.OData.UriParser.PathCountSelectItem.Filter.get -> Microsoft.OData.UriParser.FilterClause +Microsoft.OData.UriParser.PathCountSelectItem.NavigationSource.get -> Microsoft.OData.Edm.IEdmNavigationSource +Microsoft.OData.UriParser.PathCountSelectItem.PathCountSelectItem(Microsoft.OData.UriParser.ODataSelectPath selectedPath) -> void +Microsoft.OData.UriParser.PathCountSelectItem.PathCountSelectItem(Microsoft.OData.UriParser.ODataSelectPath selectedPath, Microsoft.OData.Edm.IEdmNavigationSource navigationSource, Microsoft.OData.UriParser.FilterClause filter, Microsoft.OData.UriParser.SearchClause search) -> void +Microsoft.OData.UriParser.PathCountSelectItem.Search.get -> Microsoft.OData.UriParser.SearchClause +Microsoft.OData.UriParser.PathCountSelectItem.SelectedPath.get -> Microsoft.OData.UriParser.ODataSelectPath +override Microsoft.OData.UriParser.PathCountSelectItem.HandleWith(Microsoft.OData.UriParser.SelectItemHandler handler) -> void +override Microsoft.OData.UriParser.PathCountSelectItem.TranslateWith(Microsoft.OData.UriParser.SelectItemTranslator translator) -> T +virtual Microsoft.OData.UriParser.SelectItemHandler.Handle(Microsoft.OData.UriParser.PathCountSelectItem item) -> void +virtual Microsoft.OData.UriParser.SelectItemTranslator.Translate(Microsoft.OData.UriParser.PathCountSelectItem item) -> T \ No newline at end of file diff --git a/src/Microsoft.OData.Core/SRResources.Designer.cs b/src/Microsoft.OData.Core/SRResources.Designer.cs index b0dd1fc399..0fcc65256b 100644 --- a/src/Microsoft.OData.Core/SRResources.Designer.cs +++ b/src/Microsoft.OData.Core/SRResources.Designer.cs @@ -673,15 +673,6 @@ internal static string ExpressionLexer_UnterminatedStringLiteral { } } - /// - /// Looks up a localized string similar to $count is not allowed in $select option.. - /// - internal static string ExpressionToken_DollarCountNotAllowedInSelect { - get { - return ResourceManager.GetString("ExpressionToken_DollarCountNotAllowedInSelect", resourceCulture); - } - } - /// /// Looks up a localized string similar to An identifier was expected at position {0}.. /// @@ -6273,6 +6264,42 @@ internal static string SelectExpandBinder_InvalidQueryOptionNestedSelection { } } + /// + /// Looks up a localized string similar to No more segment '{0}' is allowed after '/$count' segment.. + /// + internal static string SelectExpandBinder_NoSegmentAllowedAfterDollarCount { + get { + return ResourceManager.GetString("SelectExpandBinder_NoSegmentAllowedAfterDollarCount", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to '/$count' segment is not allowed as first segment.. + /// + internal static string SelectExpandBinder_NotAllowedDollarCountAsFirstSegment { + get { + return ResourceManager.GetString("SelectExpandBinder_NotAllowedDollarCountAsFirstSegment", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to In $select query option, '/$count' segment is not allowed on the navigation property segment '{0}'.. + /// + internal static string SelectExpandBinder_NotAllowedDollarCountOnNavigationPropertyInDollarSelect { + get { + return ResourceManager.GetString("SelectExpandBinder_NotAllowedDollarCountOnNavigationPropertyInDollarSelect", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to '/$count' segment is only allowed on the collection type segment.. + /// + internal static string SelectExpandBinder_NotAllowedDollarCountOnNonCollection { + get { + return ResourceManager.GetString("SelectExpandBinder_NotAllowedDollarCountOnNonCollection", resourceCulture); + } + } + /// /// Looks up a localized string similar to Found a system token, '{0}', while parsing a select clause.. /// diff --git a/src/Microsoft.OData.Core/SRResources.resx b/src/Microsoft.OData.Core/SRResources.resx index 8fea11fe95..c246b9c1a6 100644 --- a/src/Microsoft.OData.Core/SRResources.resx +++ b/src/Microsoft.OData.Core/SRResources.resx @@ -2087,6 +2087,18 @@ Found a system token, '{0}', while parsing a select clause. + + No more segment '{0}' is allowed after '/$count' segment. + + + '/$count' segment is not allowed as first segment. + + + '/$count' segment is only allowed on the collection type segment. + + + In $select query option, '/$count' segment is not allowed on the navigation property segment '{0}'. + Only properties specified in $expand can be traversed in $select query options. Selected item was '{0}'. @@ -2444,9 +2456,6 @@ Only $ref is allowed with star in $expand option. - - $count is not allowed in $select option. - No property is allowed after $count segment. diff --git a/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs index 43d9082166..f4164f266f 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs @@ -265,7 +265,7 @@ private SelectItem GenerateSelectItem(SelectTermToken tokenIn) ExceptionUtils.CheckArgumentNotNull(tokenIn, "tokenIn"); ExceptionUtils.CheckArgumentNotNull(tokenIn.PathToProperty, "pathToProperty"); - VerifySelectedPath(tokenIn); + VerifySelectedPath(tokenIn, configuration.EnableCaseInsensitiveUriFunctionIdentifier, out bool isCount); SelectItem newSelectItem; if (ProcessWildcardTokenPath(tokenIn, out newSelectItem)) @@ -277,7 +277,7 @@ private SelectItem GenerateSelectItem(SelectTermToken tokenIn) Debug.Assert(selectedPath.Count > 0); // Navigation property should be the last segment in select path. - if (VerifySelectedNavigationProperty(selectedPath, tokenIn)) + if (VerifySelectedNavigationProperty(selectedPath, tokenIn, isCount)) { return new PathSelectItem(new ODataSelectPath(selectedPath)); } @@ -290,6 +290,10 @@ private SelectItem GenerateSelectItem(SelectTermToken tokenIn) { targetElementType = collection.ElementType.Definition; } + else if (isCount && lastSegment is not PathTemplateSegment) + { + throw new ODataException(SRResources.SelectExpandBinder_NotAllowedDollarCountOnNonCollection); + } IEdmTypeReference elementTypeReference = targetElementType.ToTypeReference(); @@ -319,6 +323,15 @@ private SelectItem GenerateSelectItem(SelectTermToken tokenIn) parsedPath.AddRange(selectedPath); SelectExpandClause selectExpand = BindSelectExpand(null, tokenIn.SelectOption, parsedPath, this.ResourcePathNavigationSource, targetNavigationSource, elementTypeReference, generatedProperties); + if (isCount) + { + // Do we need to throw exception if orderBy and nested select expand are not null within /$count ? + return new PathCountSelectItem(new ODataSelectPath(selectedPath), + this.NavigationSource, + filter, + search); + } + return new PathSelectItem(new ODataSelectPath(selectedPath), targetNavigationSource, selectExpand, @@ -953,8 +966,10 @@ private static BindingState CreateBindingState(ODataUriParserConfiguration confi return state; } - private static void VerifySelectedPath(SelectTermToken selectedToken) + private static void VerifySelectedPath(SelectTermToken selectedToken, bool enableCaseInsensitive, out bool isCount) { + isCount = false; + PathSegmentToken previous = null; PathSegmentToken current = selectedToken.PathToProperty; while (current != null) { @@ -964,15 +979,39 @@ private static void VerifySelectedPath(SelectTermToken selectedToken) throw new ODataException(Error.Format(SRResources.SelectExpandBinder_SystemTokenInSelect, current.Identifier)); } + // Be design, the "/$count" should be built using SystemToken, but now, ODL is using NonSystemToken. + // Shall we support "no-dollar sign"? + if (current.Identifier.Equals(UriQueryConstants.CountSegment, enableCaseInsensitive ? System.StringComparison.OrdinalIgnoreCase : System.StringComparison.Ordinal)) + { + if (current.NextToken != null) + { + throw new ODataException(Error.Format(SRResources.SelectExpandBinder_NoSegmentAllowedAfterDollarCount, current.NextToken.Identifier)); + } + + if (previous == null) + { + throw new ODataException(SRResources.SelectExpandBinder_NotAllowedDollarCountAsFirstSegment); + } + + previous.NextToken = null; // Now remove the last "/$count" from the PathToProperty. + isCount = true; + } + + previous = current; current = current.NextToken; } } - private static bool VerifySelectedNavigationProperty(IList selectedPath, SelectTermToken tokenIn) + private static bool VerifySelectedNavigationProperty(IList selectedPath, SelectTermToken tokenIn, bool isCount) { NavigationPropertySegment navPropSegment = selectedPath.LastOrDefault() as NavigationPropertySegment; if (navPropSegment != null) { + if (isCount) + { + throw new ODataException(Error.Format(SRResources.SelectExpandBinder_NotAllowedDollarCountOnNavigationPropertyInDollarSelect, navPropSegment.NavigationProperty.Name)); + } + // After navigation property, it's not allowed to nest query options VerifyNoQueryOptionsNested(tokenIn, navPropSegment.Identifier); diff --git a/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandTermParser.cs b/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandTermParser.cs index 776286f6e5..5e7e1be668 100644 --- a/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandTermParser.cs +++ b/src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandTermParser.cs @@ -136,11 +136,6 @@ private PathSegmentToken ParseSegment(PathSegmentToken previousSegment, bool all } } - if (this.lexer.CurrentToken.Span.Equals(UriQueryConstants.CountSegment, StringComparison.Ordinal) && isSelect) - { - // $count is not allowed in $select e.g $select=NavProperty/$count - throw new ODataException(SRResources.ExpressionToken_DollarCountNotAllowedInSelect); - } ReadOnlySpan propertyName; @@ -170,6 +165,8 @@ private PathSegmentToken ParseSegment(PathSegmentToken previousSegment, bool all this.lexer.NextToken(); } + // By design, "/$count" or "/$ref" should return using SystemToken, but the existing implementation is using "NonSystemToken". + // So far, let's keep it unchanged. return new NonSystemToken(propertyName.ToString(), null, previousSegment); } } diff --git a/src/Microsoft.OData.Core/UriParser/SemanticAst/PathCountSelectItem.cs b/src/Microsoft.OData.Core/UriParser/SemanticAst/PathCountSelectItem.cs new file mode 100644 index 0000000000..a0fddb14db --- /dev/null +++ b/src/Microsoft.OData.Core/UriParser/SemanticAst/PathCountSelectItem.cs @@ -0,0 +1,86 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +using Microsoft.OData.Edm; + +namespace Microsoft.OData.UriParser +{ + /// + /// Class to represent $select=collectionProperty/$count. + /// + public sealed class PathCountSelectItem : SelectItem + { + /// + /// Initializes a new . + /// + /// The selected path. + public PathCountSelectItem(ODataSelectPath selectedPath) + : this(selectedPath, null, null, null) + { } + + /// + /// Initializes a new . + /// + /// The selected path. + /// The navigation source for this select item. + /// A filter clause for this select (can be null). + /// A search clause for this select (can be null). + public PathCountSelectItem(ODataSelectPath selectedPath, + IEdmNavigationSource navigationSource, + FilterClause filter, + SearchClause search) + { + ExceptionUtils.CheckArgumentNotNull(selectedPath, "selectedPath"); + + SelectedPath = selectedPath; + NavigationSource = navigationSource; + Filter = filter; + Search = search; + } + + /// + /// Gets the selected path. + /// + public ODataSelectPath SelectedPath { get; } + + /// + /// Gets the navigation source for this select level. + /// + public IEdmNavigationSource NavigationSource { get; } + + /// + /// The filter clause for this select level. + /// + public FilterClause Filter { get; } + + /// + /// Gets the search clause for this select level. + /// + public SearchClause Search { get; } + + /// + /// Translate using a . + /// + /// Type that the translator will return after visiting this item. + /// An implementation of the translator interface. + /// An object whose type is determined by the type parameter of the translator. + /// Throws if the input translator is null. + public override T TranslateWith(SelectItemTranslator translator) + { + return translator.Translate(this); + } + + /// + /// Handle using a . + /// + /// An implementation of the handler interface. + /// Throws if the input handler is null. + public override void HandleWith(SelectItemHandler handler) + { + handler.Handle(this); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemHandler.cs b/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemHandler.cs index ead9861065..1a2377be08 100644 --- a/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemHandler.cs +++ b/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemHandler.cs @@ -22,6 +22,15 @@ public virtual void Handle(WildcardSelectItem item) throw new NotImplementedException(); } + /// + /// Handle a PathCountSelectItem + /// + /// the item to Handle + public virtual void Handle(PathCountSelectItem item) + { + throw new NotImplementedException(); + } + /// /// Handle a PathSelectItem /// diff --git a/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemTranslator.cs b/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemTranslator.cs index 59656d8898..a4b8a82133 100644 --- a/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemTranslator.cs +++ b/src/Microsoft.OData.Core/UriParser/Visitors/SelectItemTranslator.cs @@ -34,6 +34,16 @@ public virtual T Translate(PathSelectItem item) throw new NotImplementedException(); } + /// + /// Translate a PathCountSelectItem + /// + /// the item to Translate + /// Defined by the implementer + public virtual T Translate(PathCountSelectItem item) + { + throw new NotImplementedException(); + } + /// /// Translate a ContainerQualifiedWildcardSelectItem /// diff --git a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/SelectExpandFunctionalTests.cs b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/SelectExpandFunctionalTests.cs index cf69c7a08b..0f1f66e39b 100644 --- a/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/SelectExpandFunctionalTests.cs +++ b/test/UnitTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/SelectExpandFunctionalTests.cs @@ -61,10 +61,24 @@ public void SelectPropertiesWithRefOperationThrows() } [Fact] - public void SelectPropertiesWithDollarCountOperationThrows() + public void SelectPropertiesWithDollarCountOperationDoesNotThrow() { - Action readResult = () => RunParseSelectExpand("MyLions/$count", null, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); - readResult.Throws(SRResources.ExpressionToken_DollarCountNotAllowedInSelect); + Action readResult = () => RunParseSelectExpand("RelatedIDs/$count", null, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + readResult.DoesNotThrow(); + } + + [Fact] + public void SelectPropertiesWithDollarCountAfterNonCollectionPropertyThrows() + { + Action readResult = () => RunParseSelectExpand("Name/$count", null, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + readResult.Throws(SRResources.SelectExpandBinder_NotAllowedDollarCountOnNonCollection); + } + + [Fact] + public void SelectPropertiesWithDollarCountAfterNavigationPropertyThrows() + { + Action readResult = () => RunParseSelectExpand("MyPaintings/$count", null, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + readResult.Throws(Error.Format(SRResources.SelectExpandBinder_NotAllowedDollarCountOnNavigationPropertyInDollarSelect, "MyPaintings")); } [Fact] @@ -587,6 +601,14 @@ public void ExpandNavigationWithNavigationAfterDollarCountOperationThrows() readResult.Throws(SRResources.ExpressionToken_NoPropAllowedAfterDollarCount); } + [Fact] + public void SelectCollectionPropertyWithMoreSegmentAfterDollarCountOperationThrows() + { + const string selectClauseText = "RelatedIDs/$count/MyPeople"; + Action readResult = () => RunParseSelectExpand(selectClauseText, null, HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet()); + readResult.Throws(Error.Format(SRResources.SelectExpandBinder_NoSegmentAllowedAfterDollarCount, "MyPeople")); + } + [Fact] public void ExpandNavigationWithNestedQueryOptionOnRef() { @@ -1654,6 +1676,87 @@ public void SelectWithNestedTopSkipAndCountWorks() Assert.Equal(2, pathSelectItem.SkipOption); } + // $select=colProperty/$count + [Fact] + public void SelectWithCollectionPrimitivePropCountWorks() + { + // Arrange + var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel, + HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(), + new Dictionary() + { + {"$select", "RelatedIDs/$count"} + }); + + // Act + var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand(); + + // Assert + Assert.NotNull(selectExpandClause); + PathCountSelectItem selectCountItem = Assert.IsType(Assert.Single(selectExpandClause.SelectedItems)); + PropertySegment propertySegment = Assert.IsType(Assert.Single(selectCountItem.SelectedPath)); + Assert.Equal("RelatedIDs", propertySegment.Property.Name); + + Assert.Null(selectCountItem.Filter); + Assert.Null(selectCountItem.Search); + } + + // $select=colProperty/$count($filter=...) + [Fact] + public void SelectWithCollectionPrimitivePropCountWithFilterOptionWorks() + { + // Arrange + var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel, + HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(), + new Dictionary() + { + {"$select", "RelatedIDs/$count($filter=$this lt 42)"} + }); + + // Act + var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand(); + + // Assert + PathCountSelectItem selectCountItem = Assert.IsType(Assert.Single(selectExpandClause.SelectedItems)); + PropertySegment propertySegment = Assert.IsType(Assert.Single(selectCountItem.SelectedPath)); + Assert.Equal("RelatedIDs", propertySegment.Property.Name); + + Assert.NotNull(selectCountItem.Filter); + BinaryOperatorNode binaryNode = Assert.IsType(selectCountItem.Filter.Expression); + Assert.Equal(BinaryOperatorKind.LessThan, binaryNode.OperatorKind); + NonResourceRangeVariableReferenceNode leftNode = Assert.IsType(binaryNode.Left); + Assert.Equal("$this", leftNode.Name); + + ConstantNode rightNode = Assert.IsType(binaryNode.Right); + Assert.Equal(42, rightNode.Value); + + Assert.Null(selectCountItem.Search); + } + + // $select=colProperty/$count($search=...) + [Fact] + public void SelectWithCollectionPrimitivePropCountWithSearchOptionWorks() + { + // Arrange + var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel, + HardCodedTestModel.GetPersonType(), HardCodedTestModel.GetPeopleSet(), + new Dictionary() + { + {"$select", "RelatedIDs/$count($search=abc)"} + }); + + // Act + var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand(); + + // Assert + PathCountSelectItem selectCountItem = Assert.IsType(Assert.Single(selectExpandClause.SelectedItems)); + PropertySegment propertySegment = Assert.IsType(Assert.Single(selectCountItem.SelectedPath)); + Assert.Equal("RelatedIDs", propertySegment.Property.Name); + + Assert.Null(selectCountItem.Filter); + Assert.NotNull(selectCountItem.Search); + } + // $expand=navProp/$count [Fact] public void ExpandWithNavigationPropCountWorks()