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

Expand fully qualified type #2160

Merged
merged 13 commits into from
Sep 18, 2021
3 changes: 2 additions & 1 deletion src/Microsoft.OData.Core/Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,8 @@ internal sealed class TextRes {
internal const string PathParser_TypeCastOnlyAllowedAfterStructuralCollection = "PathParser_TypeCastOnlyAllowedAfterStructuralCollection";
internal const string PathParser_TypeCastOnlyAllowedInDerivedTypeConstraint = "PathParser_TypeCastOnlyAllowedInDerivedTypeConstraint";
internal const string ODataResourceSet_MustNotContainBothNextPageLinkAndDeltaLink = "ODataResourceSet_MustNotContainBothNextPageLinkAndDeltaLink";
internal const string ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty = "ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty";
internal const string ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty = "ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty";
internal const string ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment = "ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment";
internal const string ODataExpandPath_InvalidExpandPathSegment = "ODataExpandPath_InvalidExpandPathSegment";
internal const string ODataSelectPath_CannotOnlyHaveTypeSegment = "ODataSelectPath_CannotOnlyHaveTypeSegment";
internal const string ODataSelectPath_InvalidSelectPathSegmentType = "ODataSelectPath_InvalidSelectPathSegmentType";
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.OData.Core/Microsoft.OData.Core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,8 @@ PathParser_TypeCastOnlyAllowedInDerivedTypeConstraint=Type cast segment '{0}' on

ODataResourceSet_MustNotContainBothNextPageLinkAndDeltaLink=A resource set may contain a next page link, a delta link or neither, but must not contain both.

ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty=The last segment, and only the last segment, must be a navigation property in $expand.
ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty =The last segment, and only the last segment, can be a navigation property in $expand.
ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment =The last segment must be a navigation property or type segment in $expand.
ODataExpandPath_InvalidExpandPathSegment=Found a segment of type '{0} in an expand path, but only NavigationProperty, Property and Type segments are allowed.

ODataSelectPath_CannotOnlyHaveTypeSegment=TypeSegment cannot be the only segment in a $select.
Expand Down
17 changes: 14 additions & 3 deletions src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6407,13 +6407,24 @@ internal static string ODataResourceSet_MustNotContainBothNextPageLinkAndDeltaLi
}

/// <summary>
/// A string like "The last segment, and only the last segment, must be a navigation property in $expand."
/// A string like "The last segment, and only the last segment, can be a navigation property in $expand."
/// </summary>
internal static string ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty
internal static string ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty
{
get
{
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty);
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty);
}
}

/// <summary>
/// A string like "The last segment must be a navigation property or type segment in $expand."
/// </summary>
internal static string ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment
{
get
{
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment);
}
}

Expand Down
46 changes: 38 additions & 8 deletions src/Microsoft.OData.Core/UriParser/Binders/SelectExpandBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,23 +369,46 @@ private SelectItem GenerateExpandItem(ExpandTermToken tokenIn)
currentNavProp = ParseComplexTypesBeforeNavigation(currentComplexProp, ref firstNonTypeToken, pathSoFar);
}

bool isRef = false;
bool isCount = false;
bool hasDerivedTypeSegment = false;
IEdmType derivedType = null;

// Handle $expand=Customer/Fully.Qualified.Namespace.VipCustomer
// The deriveTypeToken is Fully.Qualified.Namespace.VipCustomer
if (firstNonTypeToken.NextToken != null && firstNonTypeToken.NextToken.IsNamespaceOrContainerQualified())
{
hasDerivedTypeSegment = true;
derivedType = UriEdmHelpers.FindTypeFromModel(this.Model, firstNonTypeToken.NextToken.Identifier, this.configuration.Resolver);
Copy link
Member

Choose a reason for hiding this comment

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

derivedType = UriEdmHelpers.FindTypeFromModel

do you need any special handling for FindTypeFromModel not finding the type? In that case, I believe derivedType would return null, and it looks like CheckRelatedTo may throw a null ref exception as IsRelatedTo will return false, but the exception that we try to generate on line 57 of EdmUriHelpers will try to call FullTypeName() on the null value.

Minimally, we should fix EdmUriHelpers.CheckRelatedTo to handle the null childType and add tests for adding a cast segment with a type that is not found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When FindTypeFromModel doesn't find the type it returns a null. I have added code to throw an exception when the derivedType is null.

Regarding EdmUriHelpers.CheckRelatedTo, I have handled the null childType similar to how a null parentType is handled.


if (derivedType == null)
{
// Exception example: The type Fully.Qualified.Namespace.UndefinedType is not defined in the model.
throw new ODataException(ODataErrorStrings.ExpandItemBinder_CannotFindType(firstNonTypeToken.NextToken.Identifier));
}

// In this example: $expand=Customer/Fully.Qualified.Namespace.VipCustomer
// We validate that the derived type Fully.Qualified.Namespace.VipCustomer is related to Navigation property Customer.
UriEdmHelpers.CheckRelatedTo(currentNavProp.ToEntityType(), derivedType);
}

// ensure that we're always dealing with proper V4 syntax
if (firstNonTypeToken.NextToken != null && firstNonTypeToken.NextToken.NextToken != null)
if (firstNonTypeToken?.NextToken?.NextToken != null && !hasDerivedTypeSegment)
{
throw new ODataException(ODataErrorStrings.ExpandItemBinder_TraversingMultipleNavPropsInTheSamePath);
}

bool isRef = false;
bool isCount = false;

if (firstNonTypeToken.NextToken != null)
if ((firstNonTypeToken.NextToken != null && !hasDerivedTypeSegment) ||
(firstNonTypeToken?.NextToken?.NextToken != null && hasDerivedTypeSegment))
{
PathSegmentToken nextToken = hasDerivedTypeSegment ? firstNonTypeToken.NextToken.NextToken : firstNonTypeToken.NextToken;

// lastly... make sure that, since we're on a NavProp, that the next token isn't null.
if (firstNonTypeToken.NextToken.Identifier == UriQueryConstants.RefSegment)
if (nextToken.Identifier == UriQueryConstants.RefSegment)
{
isRef = true;
}
else if (firstNonTypeToken.NextToken.Identifier == UriQueryConstants.CountSegment)
else if (nextToken.Identifier == UriQueryConstants.CountSegment)
{
isCount = true;
}
Expand All @@ -398,7 +421,6 @@ private SelectItem GenerateExpandItem(ExpandTermToken tokenIn)
// Add the segments in select and expand to parsed segments
List<ODataPathSegment> parsedPath = new List<ODataPathSegment>(this.parsedSegments);
parsedPath.AddRange(pathSoFar);

IEdmNavigationSource targetNavigationSource = null;
if (this.NavigationSource != null)
{
Expand All @@ -409,6 +431,14 @@ private SelectItem GenerateExpandItem(ExpandTermToken tokenIn)
NavigationPropertySegment navSegment = new NavigationPropertySegment(currentNavProp, targetNavigationSource);
pathSoFar.Add(navSegment);
parsedPath.Add(navSegment); // Add the navigation property segment to parsed segments for future usage.

if (hasDerivedTypeSegment)
{
TypeSegment derivedTypeSegment = new TypeSegment(derivedType, targetNavigationSource);
pathSoFar.Add(derivedTypeSegment);
parsedPath.Add(derivedTypeSegment);
}

ODataExpandPath pathToNavProp = new ODataExpandPath(pathSoFar);

// $apply
Expand Down
17 changes: 5 additions & 12 deletions src/Microsoft.OData.Core/UriParser/SemanticAst/ODataExpandPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,30 +58,23 @@ private void ValidatePath()
bool foundNavProp = false;
foreach (ODataPathSegment segment in this)
{
if (segment is TypeSegment)
if (segment is PropertySegment)
{
if (index == this.Count - 1)
{
throw new ODataException(ODataErrorStrings.ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty);
}
}
else if (segment is PropertySegment)
{
if (index == this.Count - 1)
{
throw new ODataException(ODataErrorStrings.ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty);
throw new ODataException(ODataErrorStrings.ODataExpandPath_LastSegmentMustBeNavigationPropertyOrTypeSegment);
}
}
else if (segment is NavigationPropertySegment)
{
if (index < this.Count - 1 || foundNavProp)
if (foundNavProp)
{
throw new ODataException(ODataErrorStrings.ODataExpandPath_OnlyLastSegmentMustBeNavigationProperty);
throw new ODataException(ODataErrorStrings.ODataExpandPath_OnlyLastSegmentCanBeNavigationProperty);
}

foundNavProp = true;
}
else
else if (!(segment is TypeSegment))
{
throw new ODataException(ODataErrorStrings.ODataExpandPath_InvalidExpandPathSegment(segment.GetType().Name));
}
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.OData.Core/UriParser/UriEdmHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public static void CheckRelatedTo(IEdmType parentType, IEdmType childType)
{
// If the parentType is an open property, parentType will be null and can't have an ODataFullName.
string parentTypeName = (parentType != null) ? parentType.FullTypeName() : "<null>";
throw new ODataException(Strings.MetadataBinder_HierarchyNotFollowed(childType.FullTypeName(), parentTypeName));
string childTypeName = (childType != null) ? childType.FullTypeName() : "<null>";
throw new ODataException(Strings.MetadataBinder_HierarchyNotFollowed(childTypeName, parentTypeName));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,157 @@ public void ExpandWithNavigationPropCountWithSearchOptionWorks()
Assert.NotNull(expandedCountSelectItem.SearchOption);
}

// $expand=navProp/fully.qualified.type/$ref
[Theory]
[InlineData("MyPeople/Fully.Qualified.Namespace.Employee/$ref")]
[InlineData("MyPeople/MainAlias.Employee/$ref")]
public void ExpandWithNavigationPropRefWithFullyQualifiedTypeWorks(string query)
{
// Arrange
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetDogType(), HardCodedTestModel.GetDogsSet(),
new Dictionary<string, string>()
{
{"$expand", query}
});

// Act
var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand();

// Assert
Assert.NotNull(selectExpandClause);
ExpandedReferenceSelectItem expandedRefSelectItem = Assert.IsType<ExpandedReferenceSelectItem>(Assert.Single(selectExpandClause.SelectedItems));
Assert.Same(HardCodedTestModel.GetPeopleSet(), expandedRefSelectItem.NavigationSource);
Assert.Equal(2, expandedRefSelectItem.PathToNavigationProperty.Count);

NavigationPropertySegment navPropSegment = Assert.IsType<NavigationPropertySegment>(expandedRefSelectItem.PathToNavigationProperty.Segments.First());
TypeSegment typeSegment = Assert.IsType<TypeSegment>(expandedRefSelectItem.PathToNavigationProperty.Segments.Last());
Assert.Equal("MyPeople", navPropSegment.Identifier);
Assert.Equal("Collection(Fully.Qualified.Namespace.Person)", navPropSegment.EdmType.FullTypeName());
Assert.Equal("Fully.Qualified.Namespace.Employee", typeSegment.EdmType.FullTypeName());
}

// $expand=navProp/fully.qualified.type/$count
Copy link
Member

@mikepizzo mikepizzo Sep 13, 2021

Choose a reason for hiding this comment

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

should also have tests for /$expand=navProp/fully.qualified.type/$ref #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@mikepizzo mikepizzo Sep 13, 2021

Choose a reason for hiding this comment

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

Can we add tests for fully.qualified.type being an alias? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikepizzo I am not sure what you mean. Kindly clarify.
cc @gathogojr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the spec, parameter aliases are only used for $filter or $orderby https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_ParameterAliases

Copy link
Member

Choose a reason for hiding this comment

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

Sorry; i meant using a (namespace alias)[http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Alias] to qualify the type name, rather than the full namespace name.

For example:

...

$expand=navProp/alias.type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikepizzo I have added the tests

Copy link
Member

@mikepizzo mikepizzo Sep 13, 2021

Choose a reason for hiding this comment

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

Please add a (negative) test case for fully.qualified.undefinedType and verify meaningful exception (i.e., saying that the type fully.qualified.undefinedType was not found). #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

[Theory]
[InlineData("MyPeople/Fully.Qualified.Namespace.Employee/$count")]
[InlineData("MyPeople/MainAlias.Employee/$count")] // With schema alias
public void ExpandWithNavigationPropCountWithFullyQualifiedTypeWorks(string query)
{
// Arrange
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetDogType(), HardCodedTestModel.GetDogsSet(),
new Dictionary<string, string>()
{
{"$expand", query}
});

// Act
var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand();

// Assert
Assert.NotNull(selectExpandClause);
ExpandedCountSelectItem expandedCountSelectItem = Assert.IsType<ExpandedCountSelectItem>(Assert.Single(selectExpandClause.SelectedItems));
Assert.Same(HardCodedTestModel.GetPeopleSet(), expandedCountSelectItem.NavigationSource);
Assert.Null(expandedCountSelectItem.FilterOption);
Assert.Null(expandedCountSelectItem.SearchOption);
Assert.Equal(2, expandedCountSelectItem.PathToNavigationProperty.Count);

NavigationPropertySegment navPropSegment = Assert.IsType<NavigationPropertySegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.First());
TypeSegment typeSegment = Assert.IsType<TypeSegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.Last());
Assert.Equal("MyPeople", navPropSegment.Identifier);
Assert.Equal("Collection(Fully.Qualified.Namespace.Person)", navPropSegment.EdmType.FullTypeName());
Assert.Equal("Fully.Qualified.Namespace.Employee", typeSegment.EdmType.FullTypeName());
}

// $expand=navProp/fully.qualified.type/$count($filter=prop)
[Theory]
[InlineData("MyPeople/Fully.Qualified.Namespace.Employee/$count($filter=ID eq 1)")]
[InlineData("MyPeople/MainAlias.Employee/$count($filter=ID eq 1)")] // With schema alias
public void ExpandWithNavigationPropCountWithFilterAndFullyQualifiedTypeWorks(string query)
{
// Arrange
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetDogType(), HardCodedTestModel.GetDogsSet(),
new Dictionary<string, string>()
{
{"$expand", query}
});

// Act
var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand();

// Assert
Assert.NotNull(selectExpandClause);
ExpandedCountSelectItem expandedCountSelectItem = Assert.IsType<ExpandedCountSelectItem>(Assert.Single(selectExpandClause.SelectedItems));
Assert.Same(HardCodedTestModel.GetPeopleSet(), expandedCountSelectItem.NavigationSource);
Assert.NotNull(expandedCountSelectItem.FilterOption);
Assert.Null(expandedCountSelectItem.SearchOption);
Assert.Equal(2, expandedCountSelectItem.PathToNavigationProperty.Count);

NavigationPropertySegment navPropSegment = Assert.IsType<NavigationPropertySegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.First());
TypeSegment typeSegment = Assert.IsType<TypeSegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.Last());
Assert.Equal("MyPeople", navPropSegment.Identifier);
Assert.Equal("Collection(Fully.Qualified.Namespace.Person)", navPropSegment.EdmType.FullTypeName());
Assert.Equal("Fully.Qualified.Namespace.Employee", typeSegment.EdmType.FullTypeName());
}

// $expand=navProp/fully.qualified.type/$count($search=prop)
[Theory]
[InlineData("MyPeople/Fully.Qualified.Namespace.Employee/$count($search=blue)")]
[InlineData("MyPeople/MainAlias.Employee/$count($search=blue)")] // With schema alias
public void ExpandWithNavigationPropCountWithSearchAndFullyQualifiedTypeWorks(string query)
{
// Arrange
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetDogType(), HardCodedTestModel.GetDogsSet(),
new Dictionary<string, string>()
{
{"$expand", query}
});

// Act
var selectExpandClause = odataQueryOptionParser.ParseSelectAndExpand();

// Assert
Assert.NotNull(selectExpandClause);
ExpandedCountSelectItem expandedCountSelectItem = Assert.IsType<ExpandedCountSelectItem>(Assert.Single(selectExpandClause.SelectedItems));
Assert.Same(HardCodedTestModel.GetPeopleSet(), expandedCountSelectItem.NavigationSource);
Assert.Null(expandedCountSelectItem.FilterOption);
Assert.NotNull(expandedCountSelectItem.SearchOption);
Assert.Equal(2, expandedCountSelectItem.PathToNavigationProperty.Count);

NavigationPropertySegment navPropSegment = Assert.IsType<NavigationPropertySegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.First());
TypeSegment typeSegment = Assert.IsType<TypeSegment>(expandedCountSelectItem.PathToNavigationProperty.Segments.Last());
Assert.Equal("MyPeople", navPropSegment.Identifier);
Assert.Equal("Collection(Fully.Qualified.Namespace.Person)", navPropSegment.EdmType.FullTypeName());
Assert.Equal("Fully.Qualified.Namespace.Employee", typeSegment.EdmType.FullTypeName());
}

[Theory]
[InlineData("MyPeople/Fully.Qualified.Namespace.UndefinedType")]
[InlineData("MyPeople/Fully.Qualified.Namespace.UndefinedType/$ref")]
[InlineData("MyPeople/Fully.Qualified.Namespace.UndefinedType/$count")]
[InlineData("MyPeople/Fully.Qualified.Namespace.UndefinedType/$count($search=blue)")]
[InlineData("MyPeople/Fully.Qualified.Namespace.UndefinedType/$count($filter=ID eq 1)")]
public void ExpandWithNavigationPropWithUndefinedTypeThrows(string query)
{
// Arrange
var odataQueryOptionParser = new ODataQueryOptionParser(HardCodedTestModel.TestModel,
HardCodedTestModel.GetDogType(), HardCodedTestModel.GetDogsSet(),
new Dictionary<string, string>()
{
{"$expand", query}
});

// Act
Action action = () => odataQueryOptionParser.ParseSelectAndExpand();

// Assert

// Exception: The type Fully.Qualified.Namespace.UndefinedType is not defined in the model.
action.Throws<ODataException>(ODataErrorStrings.ExpandItemBinder_CannotFindType("Fully.Qualified.Namespace.UndefinedType"));
}

[Fact]
public void SelectWithNestedSelectWorks()
{
Expand Down
Loading