Expands navigation properties of derived types only if declaring navigation property is a containment

[Irvine Sunday (irvinesunday)]

Fixes#269

This PR:

• Fixes the logic of expanding the navigation properties of derived types only if the declaring navigation property is a containment. Initially, expansion covered both containment and non-containment navigation properties.
• Adds a check to ensure we don't add a derived type that has already been added in the path when expanding the type's navigation properties.
• Update tests and integration files to validate the above.

To fully resolve the aforementioned issue, the below convert settings need to be set to true:

• ExpandDerivedTypesNavigationProperties
• AppendBoundOperationsOnDerivedTypeCastSegments

With the above fix and the above settings set to true, the result, for the examples provided in the issue, will be:

• https://docs.microsoft.com/en-us/graph/api/ediscovery-caseexportoperation-getdownloadurl?view=graph-rest-beta
  

• https://docs.microsoft.com/en-us/graph/api/intune-apps-mobileappcontentfile-commit?view=graph-rest-1.0
  

[Andrew Omondi (andrueastman)]

👍🏼

[Andrew Omondi (andrueastman)]

Thanks Irvine Sunday (@irvinesunday)

[Irvine Sunday (irvinesunday)]

Adding more information for context:

The table below shows the number of beta paths generated when the two convert settings ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments are enabled/disabled.

Convert Setting	ExpandDerivedTypesNavigationProperties = true	ExpandDerivedTypesNavigationProperties = false
AppendBoundOperationsOnDerivedTypeCastSegments = true	21,484	16,969
AppendBoundOperationsOnDerivedTypeCastSegments = false	19,342	14,846

Peter Ombwa (@peombwa) / Tim (@timayabi2020) we might need to verify the generated OpenAPI document with PowerShell to verify whether there are any modules that might break AutoREST due to the increase in size.

[Peter Ombwa (peombwa)]

Adding more information for context:

The table below shows the number of beta paths generated when the two convert settings ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments are enabled/disabled.

Convert Setting ExpandDerivedTypesNavigationProperties = true ExpandDerivedTypesNavigationProperties = false
AppendBoundOperationsOnDerivedTypeCastSegments = true 21,484 16,969
AppendBoundOperationsOnDerivedTypeCastSegments = false 19,342 14,846
Peter Ombwa (@peombwa) / Tim (@timayabi2020) we might need to verify the generated OpenAPI document with PowerShell to verify whether there are any modules that might break AutoREST due to the increase in size.

Thanks Irvine Sunday (@irvinesunday) for the breakdown!

Since OData type cast is currently not enabled in DevX API, we may see an increase of ~4500 paths in beta. Can we have an experimental deployment of DevX API with ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments set to true to facilitate our validation?

Also, is ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments set to false by default? i.e., maintains the current nav property expansion logic?

[Irvine Sunday (irvinesunday)]

Adding more information for context:
The table below shows the number of beta paths generated when the two convert settings ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments are enabled/disabled.
Convert Setting ExpandDerivedTypesNavigationProperties = true ExpandDerivedTypesNavigationProperties = false
AppendBoundOperationsOnDerivedTypeCastSegments = true 21,484 16,969
AppendBoundOperationsOnDerivedTypeCastSegments = false 19,342 14,846
Peter Ombwa (@peombwa) / Tim (@timayabi2020) we might need to verify the generated OpenAPI document with PowerShell to verify whether there are any modules that might break AutoREST due to the increase in size.

Thanks Irvine Sunday (@irvinesunday) for the breakdown!

Since OData type cast is currently not enabled in DevX API, we may see an increase of ~4500 paths in beta. Can we have an experimental deployment of DevX API with ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments set to true to facilitate our validation?

Also, is ExpandDerivedTypesNavigationProperties and AppendBoundOperationsOnDerivedTypeCastSegments set to false by default? i.e., maintains the current nav property expansion logic?

Yes Peter Ombwa (@peombwa), both settings are currently set to false in DevX API (ExpandDerivedTypesNavigationProperties explicitly; AppendBoundOperationsOnDerivedTypeCastSegments implicitly by default). We can definitely update devxapitest with the bumped up version of the conversion lib. and configure both settings to true. We can use this for our validation.

[Vincent Biret (baywet)]

I'm a bit worried by the increase in the number of paths here, but I think we need to validate it before making a final decision. It'd be interesting to have a comparison of the newly added paths and see whether the service actually understands them. Especially for "deep" paths.

[Vincent Biret (baywet)]

(sorry about the auto-merge)