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.

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

Fixes #3174: Enable $select=colProperty/$count query option parser #3175

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions src/Microsoft.OData.Core/PublicAPI/net8.0/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -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<T>(Microsoft.OData.UriParser.SelectItemTranslator<T> translator) -> T
virtual Microsoft.OData.UriParser.SelectItemHandler.Handle(Microsoft.OData.UriParser.PathCountSelectItem item) -> void
virtual Microsoft.OData.UriParser.SelectItemTranslator<T>.Translate(Microsoft.OData.UriParser.PathCountSelectItem item) -> T
Original file line number Diff line number Diff line change
Expand Up @@ -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
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<T>(Microsoft.OData.UriParser.SelectItemTranslator<T> translator) -> T
virtual Microsoft.OData.UriParser.SelectItemHandler.Handle(Microsoft.OData.UriParser.PathCountSelectItem item) -> void
virtual Microsoft.OData.UriParser.SelectItemTranslator<T>.Translate(Microsoft.OData.UriParser.PathCountSelectItem item) -> T
45 changes: 36 additions & 9 deletions src/Microsoft.OData.Core/SRResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 12 additions & 3 deletions src/Microsoft.OData.Core/SRResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2087,6 +2087,18 @@
<data name="SelectExpandBinder_SystemTokenInSelect" xml:space="preserve">
<value>Found a system token, '{0}', while parsing a select clause.</value>
</data>
<data name="SelectExpandBinder_NoSegmentAllowedAfterDollarCount" xml:space="preserve">
<value>No more segment '{0}' is allowed after '/$count' segment.</value>
</data>
<data name="SelectExpandBinder_NotAllowedDollarCountAsFirstSegment" xml:space="preserve">
<value>'/$count' segment is not allowed as first segment.</value>
</data>
<data name="SelectExpandBinder_NotAllowedDollarCountOnNonCollection" xml:space="preserve">
<value>'/$count' segment is only allowed on the collection type segment.</value>
</data>
<data name="SelectExpandBinder_NotAllowedDollarCountOnNavigationPropertyInDollarSelect" xml:space="preserve">
<value>In $select query option, '/$count' segment is not allowed on the navigation property segment '{0}'.</value>
</data>
<data name="SelectionItemBinder_NoExpandForSelectedProperty" xml:space="preserve">
<value>Only properties specified in $expand can be traversed in $select query options. Selected item was '{0}'.</value>
</data>
Expand Down Expand Up @@ -2444,9 +2456,6 @@
<data name="ExpressionToken_OnlyRefAllowWithStarInExpand" xml:space="preserve">
<value>Only $ref is allowed with star in $expand option.</value>
</data>
<data name="ExpressionToken_DollarCountNotAllowedInSelect" xml:space="preserve">
<value>$count is not allowed in $select option.</value>
</data>
<data name="ExpressionToken_NoPropAllowedAfterDollarCount" xml:space="preserve">
<value>No property is allowed after $count segment.</value>
</data>
Expand Down
47 changes: 43 additions & 4 deletions src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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));
}
Expand All @@ -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();

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand All @@ -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<ODataPathSegment> selectedPath, SelectTermToken tokenIn)
private static bool VerifySelectedNavigationProperty(IList<ODataPathSegment> 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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<char> propertyName;

Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
//---------------------------------------------------------------------
// <copyright file="PathCountSelectItem.cs" company="Microsoft">
// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information.
// </copyright>
//---------------------------------------------------------------------

using Microsoft.OData.Edm;

namespace Microsoft.OData.UriParser
{
/// <summary>
/// Class to represent $select=collectionProperty/$count.
/// </summary>
public sealed class PathCountSelectItem : SelectItem
{
/// <summary>
/// Initializes a new <see cref="PathCountSelectItem"/>.
/// </summary>
/// <param name="selectedPath">The selected path.</param>
public PathCountSelectItem(ODataSelectPath selectedPath)
: this(selectedPath, null, null, null)
{ }

/// <summary>
/// Initializes a new <see cref="PathCountSelectItem"/>.
/// </summary>
/// <param name="selectedPath">The selected path.</param>
/// <param name="navigationSource">The navigation source for this select item.</param>
/// <param name="filter">A filter clause for this select (can be null).</param>
/// <param name="search">A search clause for this select (can be null).</param>
public PathCountSelectItem(ODataSelectPath selectedPath,
IEdmNavigationSource navigationSource,
FilterClause filter,
SearchClause search)
{
ExceptionUtils.CheckArgumentNotNull(selectedPath, "selectedPath");

SelectedPath = selectedPath;
NavigationSource = navigationSource;
Filter = filter;
Search = search;
}

/// <summary>
/// Gets the selected path.
/// </summary>
public ODataSelectPath SelectedPath { get; }

/// <summary>
/// Gets the navigation source for this select level.
/// </summary>
public IEdmNavigationSource NavigationSource { get; }

/// <summary>
/// The filter clause for this select level.
/// </summary>
public FilterClause Filter { get; }

/// <summary>
/// Gets the search clause for this select level.
/// </summary>
public SearchClause Search { get; }

/// <summary>
/// Translate using a <see cref="SelectItemTranslator{T}"/>.
/// </summary>
/// <typeparam name="T">Type that the translator will return after visiting this item.</typeparam>
/// <param name="translator">An implementation of the translator interface.</param>
/// <returns>An object whose type is determined by the type parameter of the translator.</returns>
/// <exception cref="System.ArgumentNullException">Throws if the input translator is null.</exception>
public override T TranslateWith<T>(SelectItemTranslator<T> translator)
{
return translator.Translate(this);
}

/// <summary>
/// Handle using a <see cref="SelectItemHandler"/>.
/// </summary>
/// <param name="handler">An implementation of the handler interface.</param>
/// <exception cref="System.ArgumentNullException">Throws if the input handler is null.</exception>
public override void HandleWith(SelectItemHandler handler)
{
handler.Handle(this);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ public virtual void Handle(WildcardSelectItem item)
throw new NotImplementedException();
}

/// <summary>
/// Handle a PathCountSelectItem
/// </summary>
/// <param name="item">the item to Handle</param>
public virtual void Handle(PathCountSelectItem item)
{
throw new NotImplementedException();
}

/// <summary>
/// Handle a PathSelectItem
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ public virtual T Translate(PathSelectItem item)
throw new NotImplementedException();
}

/// <summary>
/// Translate a PathCountSelectItem
/// </summary>
/// <param name="item">the item to Translate</param>
/// <returns>Defined by the implementer</returns>
public virtual T Translate(PathCountSelectItem item)
{
throw new NotImplementedException();
}

/// <summary>
/// Translate a ContainerQualifiedWildcardSelectItem
/// </summary>
Expand Down
Loading