Skip to content

Updates navigation property expansion logic #160

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Feb 10, 2022
Merged

Updates navigation property expansion logic #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0fc32f0
Remove unnecessary code
irvinesunday Jan 17, 2022
3355bb8
Expand nav. prop. based on identifier name and not type
irvinesunday Jan 17, 2022
475fca7
Update tests
irvinesunday Jan 17, 2022
bbed6d2
Update comment
irvinesunday Jan 17, 2022
8d18b04
Re-add method and add obsolete attribute
irvinesunday Jan 17, 2022
f839829
Merge remote-tracking branch 'microsoft/master' into is/fix-nav-prop-…
irvinesunday Jan 19, 2022
2a0fe27
Expand all containment navigation properties
irvinesunday Jan 20, 2022
0b05d21
Update tests
irvinesunday Jan 20, 2022
d2b3686
Merge remote-tracking branch 'origin/master' into is/fix-nav-prop-exp…
irvinesunday Feb 9, 2022
808e768
Update Beta integration file and tests
irvinesunday Feb 9, 2022
0ef6d39
Update src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs
irvinesunday Feb 9, 2022
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
1 change: 1 addition & 0 deletions src/Microsoft.OpenApi.OData.Reader/Edm/EdmModelExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ public static IEnumerable<IEdmComplexType> FindAllBaseTypes(this IEdmComplexType
/// <param name="baseType">Type of the base type.</param>
/// <param name="subtype">Type of the sub type.</param>
/// <returns>true, if the <paramref name="baseType"/> is assignable to <paramref name="subtype"/>. Otherwise returns false.</returns>
[Obsolete]
public static bool IsAssignableFrom(this IEdmEntityType baseType, IEdmEntityType subtype)
{
Utils.CheckArgumentNull(baseType, nameof(baseType));
Expand Down
58 changes: 27 additions & 31 deletions src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,24 +324,44 @@ private void RetrieveMediaEntityStreamPaths(IEdmEntityType entityType, ODataPath
/// <param name="count">The count restrictions.</param>
/// <param name="currentPath">The current OData path.</param>
/// <param name="convertSettings">The settings for the current conversion.</param>
private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationProperty, CountRestrictionsType count, ODataPath currentPath, OpenApiConvertSettings convertSettings)
/// <param name="visitedNavigationProperties">A stack that holds the visited navigation properties in the <paramref name="currentPath"/>.</param>
private void RetrieveNavigationPropertyPaths(
IEdmNavigationProperty navigationProperty,
CountRestrictionsType count,
ODataPath currentPath,
OpenApiConvertSettings convertSettings,
Stack<string> visitedNavigationProperties = null)
{
Debug.Assert(navigationProperty != null);
Debug.Assert(currentPath != null);

if (visitedNavigationProperties == null)
{
visitedNavigationProperties = new ();
}

var navPropFullyQualifiedName = $"{navigationProperty.DeclaringType.FullTypeName()}/{navigationProperty.Name}";

// Check whether the navigation property has already been navigated in the path
if (visitedNavigationProperties.Contains(navPropFullyQualifiedName))
{
return;
}

// Check whether the navigation property should be part of the path
NavigationRestrictionsType navigation = _model.GetRecord<NavigationRestrictionsType>(navigationProperty, CapabilitiesConstants.NavigationRestrictions);
if (navigation != null && !navigation.IsNavigable)
{
return;
}

// test the expandable for the navigation property.
bool shouldExpand = ShouldExpandNavigationProperty(navigationProperty, currentPath);
// Whether to expand the navigation property
bool shouldExpand = navigationProperty.ContainsTarget;

// append a navigation property.
currentPath.Push(new ODataNavigationPropertySegment(navigationProperty));
AppendPath(currentPath.Clone());
visitedNavigationProperties.Push(navPropFullyQualifiedName);

// Check whether a collection-valued navigation property should be indexed by key value(s).
NavigationPropertyRestriction restriction = navigation?.RestrictedProperties?.FirstOrDefault();
Expand Down Expand Up @@ -412,7 +432,7 @@ private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationPr
{
if (CanFilter(subNavProperty))
{
RetrieveNavigationPropertyPaths(subNavProperty, count, currentPath, convertSettings);
RetrieveNavigationPropertyPaths(subNavProperty, count, currentPath, convertSettings, visitedNavigationProperties);
}
}
}
Expand All @@ -421,35 +441,11 @@ private void RetrieveNavigationPropertyPaths(IEdmNavigationProperty navigationPr
if (targetsMany)
{
currentPath.Pop();
}
}
}
currentPath.Pop();
}

private bool ShouldExpandNavigationProperty(IEdmNavigationProperty navigationProperty, ODataPath currentPath)
{
Debug.Assert(navigationProperty != null);
Debug.Assert(currentPath != null);

// not expand for the non-containment.
if (!navigationProperty.ContainsTarget)
{
return false;
}

// check the type is visited before, if visited, not expand it.
IEdmEntityType navEntityType = navigationProperty.ToEntityType();
foreach (ODataSegment segment in currentPath)
{
if (segment.EntityType != null &&
navEntityType.IsAssignableFrom(segment.EntityType))
{
return false;
}
}

return true;
}
visitedNavigationProperties.Pop();
}

/// <summary>
/// Create $ref paths.
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.OpenAPI.OData.Reader.Tests/Edm/ODataPathProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void GetPathsForGraphBetaModelReturnsAllPaths()

// Assert
Assert.NotNull(paths);
Assert.Equal(28227, paths.Count());
Assert.Equal(26436, paths.Count());
}

[Fact]
Expand All @@ -67,7 +67,7 @@ public void GetPathsForGraphBetaModelWithDerivedTypesConstraintReturnsAllPaths()

// Assert
Assert.NotNull(paths);
Assert.Equal(28193, paths.Count());
Assert.Equal(26394, paths.Count());
}

[Fact]
Expand Down
45 changes: 20 additions & 25 deletions test/Microsoft.OpenAPI.OData.Reader.Tests/Resources/Graph.Beta.OData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7495,11 +7495,6 @@
<String>microsoft.graph.orgContact</String>
</Collection>
</Annotation>
<Annotation Term="Org.OData.Capabilities.V1.NavigationRestrictions">
<Record>
<PropertyValue Property="Referenceable" Bool="true" />
</Record>
</Annotation>
</NavigationProperty>
<NavigationProperty Name="licenseDetails" Type="Collection(graph.licenseDetails)" ContainsTarget="true" />
<NavigationProperty Name="manager" Type="graph.directoryObject" />
Expand Down Expand Up @@ -7804,7 +7799,7 @@
<Property Name="uid" Type="Edm.String" />
<Property Name="webLink" Type="Edm.String" />
<NavigationProperty Name="attachments" Type="Collection(graph.attachment)" ContainsTarget="true" />
<NavigationProperty Name="calendar" Type="graph.calendar" ContainsTarget="true" />
<NavigationProperty Name="calendar" Type="graph.calendar" />
<NavigationProperty Name="exceptionOccurrences" Type="Collection(graph.event)" ContainsTarget="true" />
<NavigationProperty Name="extensions" Type="Collection(graph.extension)" ContainsTarget="true" />
<NavigationProperty Name="instances" Type="Collection(graph.event)" ContainsTarget="true" />
Expand Down Expand Up @@ -8125,11 +8120,11 @@
<NavigationProperty Name="drive" Type="graph.drive" />
<NavigationProperty Name="drives" Type="Collection(graph.drive)" />
<NavigationProperty Name="externalColumns" Type="Collection(graph.columnDefinition)" />
<NavigationProperty Name="items" Type="Collection(graph.baseItem)" ContainsTarget="true" />
<NavigationProperty Name="items" Type="Collection(graph.baseItem)" />
<NavigationProperty Name="lists" Type="Collection(graph.list)" ContainsTarget="true" />
<NavigationProperty Name="pages" Type="Collection(graph.sitePage)" ContainsTarget="true" />
<NavigationProperty Name="permissions" Type="Collection(graph.permission)" ContainsTarget="true" />
<NavigationProperty Name="sites" Type="Collection(graph.site)" ContainsTarget="true" />
<NavigationProperty Name="sites" Type="Collection(graph.site)" />
<NavigationProperty Name="termStore" Type="microsoft.graph.termStore.store" ContainsTarget="true" />
<NavigationProperty Name="onenote" Type="graph.onenote" ContainsTarget="true" />
</EntityType>
Expand Down Expand Up @@ -13883,7 +13878,7 @@
<Property Name="actor" Type="graph.identitySet" />
<Property Name="times" Type="graph.itemActivityTimeSet" />
<NavigationProperty Name="driveItem" Type="graph.driveItem" />
<NavigationProperty Name="listItem" Type="graph.listItem" ContainsTarget="true" />
<NavigationProperty Name="listItem" Type="graph.listItem" />
</EntityType>
<EntityType Name="driveItem" BaseType="graph.baseItem" OpenType="true">
<Property Name="audio" Type="graph.audio" />
Expand Down Expand Up @@ -13913,9 +13908,9 @@
<Property Name="video" Type="graph.video" />
<Property Name="webDavUrl" Type="Edm.String" />
<NavigationProperty Name="workbook" Type="graph.workbook" ContainsTarget="true" />
<NavigationProperty Name="activities" Type="Collection(graph.itemActivityOLD)" ContainsTarget="true" />
<NavigationProperty Name="activities" Type="Collection(graph.itemActivityOLD)" />
<NavigationProperty Name="analytics" Type="graph.itemAnalytics" />
<NavigationProperty Name="children" Type="Collection(graph.driveItem)" ContainsTarget="true" />
<NavigationProperty Name="children" Type="Collection(graph.driveItem)" />
<NavigationProperty Name="listItem" Type="graph.listItem" ContainsTarget="true" />
<NavigationProperty Name="permissions" Type="Collection(graph.permission)" ContainsTarget="true" />
<NavigationProperty Name="subscriptions" Type="Collection(graph.subscription)" ContainsTarget="true" />
Expand Down Expand Up @@ -15729,7 +15724,7 @@
<Property Name="requestApprovalSettings" Type="graph.approvalSettings" />
<Property Name="requestorSettings" Type="graph.requestorSettings" />
<NavigationProperty Name="accessPackage" Type="graph.accessPackage" />
<NavigationProperty Name="accessPackageCatalog" Type="graph.accessPackageCatalog" ContainsTarget="true" />
<NavigationProperty Name="accessPackageCatalog" Type="graph.accessPackageCatalog" />
</EntityType>
<EntityType Name="accessPackageAssignmentRequest" BaseType="graph.entity">
<Property Name="answers" Type="Collection(graph.accessPackageAnswer)" />
Expand All @@ -15743,14 +15738,14 @@
<Property Name="requestType" Type="Edm.String" />
<Property Name="schedule" Type="graph.requestSchedule" />
<NavigationProperty Name="accessPackage" Type="graph.accessPackage" />
<NavigationProperty Name="accessPackageAssignment" Type="graph.accessPackageAssignment" ContainsTarget="true" />
<NavigationProperty Name="accessPackageAssignment" Type="graph.accessPackageAssignment" />
<NavigationProperty Name="requestor" Type="graph.accessPackageSubject" ContainsTarget="true" />
</EntityType>
<EntityType Name="accessPackageAssignmentResourceRole" BaseType="graph.entity">
<Property Name="originId" Type="Edm.String" />
<Property Name="originSystem" Type="Edm.String" />
<Property Name="status" Type="Edm.String" />
<NavigationProperty Name="accessPackageAssignments" Type="Collection(graph.accessPackageAssignment)" ContainsTarget="true" />
<NavigationProperty Name="accessPackageAssignments" Type="Collection(graph.accessPackageAssignment)" />
<NavigationProperty Name="accessPackageResourceRole" Type="graph.accessPackageResourceRole" ContainsTarget="true" />
<NavigationProperty Name="accessPackageResourceScope" Type="graph.accessPackageResourceScope" ContainsTarget="true" />
<NavigationProperty Name="accessPackageSubject" Type="graph.accessPackageSubject" ContainsTarget="true" />
Expand Down Expand Up @@ -25187,8 +25182,8 @@
<Property Name="sectionGroupsUrl" Type="Edm.String" />
<Property Name="sectionsUrl" Type="Edm.String" />
<NavigationProperty Name="parentNotebook" Type="graph.notebook" />
<NavigationProperty Name="parentSectionGroup" Type="graph.sectionGroup" ContainsTarget="true" />
<NavigationProperty Name="sectionGroups" Type="Collection(graph.sectionGroup)" ContainsTarget="true" />
<NavigationProperty Name="parentSectionGroup" Type="graph.sectionGroup" />
<NavigationProperty Name="sectionGroups" Type="Collection(graph.sectionGroup)" />
<NavigationProperty Name="sections" Type="Collection(graph.onenoteSection)" ContainsTarget="true" />
</EntityType>
<EntityType Name="onenoteSection" BaseType="graph.onenoteEntityHierarchyModel">
Expand All @@ -25197,7 +25192,7 @@
<Property Name="pagesUrl" Type="Edm.String" />
<NavigationProperty Name="pages" Type="Collection(graph.onenotePage)" ContainsTarget="true" />
<NavigationProperty Name="parentNotebook" Type="graph.notebook" />
<NavigationProperty Name="parentSectionGroup" Type="graph.sectionGroup" ContainsTarget="true" />
<NavigationProperty Name="parentSectionGroup" Type="graph.sectionGroup" />
</EntityType>
<EntityType Name="operation" BaseType="graph.entity">
<Property Name="createdDateTime" Type="Edm.DateTimeOffset" />
Expand All @@ -25221,7 +25216,7 @@
<Property Name="title" Type="Edm.String" />
<Property Name="userTags" Type="Collection(Edm.String)" />
<NavigationProperty Name="parentNotebook" Type="graph.notebook" />
<NavigationProperty Name="parentSection" Type="graph.onenoteSection" ContainsTarget="true" />
<NavigationProperty Name="parentSection" Type="graph.onenoteSection" />
</EntityType>
<EntityType Name="onenoteResource" BaseType="graph.onenoteEntityBaseModel">
<Property Name="content" Type="Edm.Stream" />
Expand Down Expand Up @@ -25516,7 +25511,7 @@
<Property Name="registeredRoot" Type="Edm.String" />
<Property Name="status" Type="Edm.String" />
<Property Name="type" Type="Edm.String" />
<NavigationProperty Name="parent" Type="graph.governanceResource" ContainsTarget="true" />
<NavigationProperty Name="parent" Type="graph.governanceResource" />
<NavigationProperty Name="roleAssignmentRequests" Type="Collection(graph.governanceRoleAssignmentRequest)" ContainsTarget="true" />
<NavigationProperty Name="roleAssignments" Type="Collection(graph.governanceRoleAssignment)" ContainsTarget="true" />
<NavigationProperty Name="roleDefinitions" Type="Collection(graph.governanceRoleDefinition)" ContainsTarget="true" />
Expand All @@ -25533,7 +25528,7 @@
<Property Name="status" Type="graph.governanceRoleAssignmentRequestStatus" />
<Property Name="subjectId" Type="Edm.String" />
<Property Name="type" Type="Edm.String" Nullable="false" />
<NavigationProperty Name="resource" Type="graph.governanceResource" ContainsTarget="true" />
<NavigationProperty Name="resource" Type="graph.governanceResource" />
<NavigationProperty Name="roleDefinition" Type="graph.governanceRoleDefinition" ContainsTarget="true" />
<NavigationProperty Name="subject" Type="graph.governanceSubject" ContainsTarget="true" />
</EntityType>
Expand All @@ -25549,7 +25544,7 @@
<Property Name="status" Type="Edm.String" Nullable="false" />
<Property Name="subjectId" Type="Edm.String" />
<NavigationProperty Name="linkedEligibleRoleAssignment" Type="graph.governanceRoleAssignment" />
<NavigationProperty Name="resource" Type="graph.governanceResource" ContainsTarget="true" />
<NavigationProperty Name="resource" Type="graph.governanceResource" />
<NavigationProperty Name="roleDefinition" Type="graph.governanceRoleDefinition" ContainsTarget="true" />
<NavigationProperty Name="subject" Type="graph.governanceSubject" ContainsTarget="true" />
</EntityType>
Expand All @@ -25558,8 +25553,8 @@
<Property Name="externalId" Type="Edm.String" />
<Property Name="resourceId" Type="Edm.String" />
<Property Name="templateId" Type="Edm.String" />
<NavigationProperty Name="resource" Type="graph.governanceResource" ContainsTarget="true" />
<NavigationProperty Name="roleSetting" Type="graph.governanceRoleSetting" ContainsTarget="true" />
<NavigationProperty Name="resource" Type="graph.governanceResource" />
<NavigationProperty Name="roleSetting" Type="graph.governanceRoleSetting" />
</EntityType>
<EntityType Name="governanceRoleSetting" BaseType="graph.entity">
<Property Name="adminEligibleSettings" Type="Collection(graph.governanceRuleSetting)" />
Expand All @@ -25571,7 +25566,7 @@
<Property Name="roleDefinitionId" Type="Edm.String" />
<Property Name="userEligibleSettings" Type="Collection(graph.governanceRuleSetting)" />
<Property Name="userMemberSettings" Type="Collection(graph.governanceRuleSetting)" />
<NavigationProperty Name="resource" Type="graph.governanceResource" ContainsTarget="true" />
<NavigationProperty Name="resource" Type="graph.governanceResource" />
<NavigationProperty Name="roleDefinition" Type="graph.governanceRoleDefinition" ContainsTarget="true" />
</EntityType>
<EntityType Name="governanceSubject" BaseType="graph.entity">
Expand Down Expand Up @@ -71201,7 +71196,7 @@
</Record>
</Annotation>
</Annotations>
<Annotations Target="microsoft.graph.application/createdOnBehalfOf">
<Annotations Target="microsoft.graph.application/createdOnBehalfOf">
<Annotation Term="Org.OData.Capabilities.V1.NavigationRestrictions">
<Record>
<PropertyValue Property="Referenceable" Bool="true" />
Expand Down