`ExpandDerivedTypesNavigationProperties` setting granularity support

[andrueastman]

Due to previous issues with the graph surface size seen in microsoft/OpenAPI.NET#866 the ExpandDerivedTypesNavigationProperties setting is currently disabled in generating the openAPI from the CSDL metadata.

This however may lead to some missing paths like actions/functions bound on the derived types or convenient nav Properties just one level deep.

• 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

We could possibly consider adding a "depth control" setting to to limit how deep the paths would go to compromise on this rather than a yes/no situation.

[irvinesunday]

The last time I tested this by generating derived types on navigation properties by going just one level deep I ended up with a significant number of paths generated.
Maybe we can figure out the possibility of having a custom annotation that we can attach to the derived types that require expansion?

[millicentachieng]

Should we also consider having a list of entity types to expand in the settings?

[irvinesunday]

Should we also consider having a list of entity types to expand in the settings?

That should probably be configured in the metadata as an annotation, as this would ideally be handled by the workload when they are updating their API.

[Edited]
On 2nd thought, having this provided via a config value in the convert settings doesn't also seem like a terrible idea. A list of the fully qualified names of derived types that should be expanded on can be provided via this config.

[irvinesunday]

For context, at the moment, we generate about 7K paths for v1.0. Expanding the navigation properties of derived types for the single type cast segment only generates about 126K paths.

@darrelmiller @baywet

[baywet]

Should we also consider having a list of entity types to expand in the settings?

I don't think this would be the best approach here, because I'd require the code that does the conversion (eg hidi, other tools) to know about the content of the CSDL in advance, which is not always the case (hidi, again).

I believe this problem of over expansion of methods has multiple facets:

• type cast segments should only have the methods of the target cast type under them, not the ones from the base type as those can be accessed by simply not casting.
• methods should only be added to singletons, entity sets, entity sets entries, single contained navigation properties, collection contained navigation properties and collection contained navigation properties entries.
• I believe @darrelmiller introduced an annotation to support unbound methods, but I don't remember the specifics.

Then there's also the over expansion of navigation properties themselves:

• Contains target is wrong at a bunch of places and we should either ask workloads to fix it or "fix it in post".
• Only contained properties should be expanded, since the non-contained ones can be addressed via their canonical path.
• I believe @darrelmiller introduced an annotation to force expansion or stop expansion in edge cases, but I don't remember the specifics.

Once we iron out those implementation aspects, we should be in a position where, when the metadata is correct, we generate an accurately expanded description for it.

[darrelmiller]

I agree that we want to enable ExpandDerivedTypesNavigationProperties. However, before we do that we must ensure the following things:

• Where only a subset of derived types are supported on a navigation property we need to add the DerivedTypeConstraint annotation to limit the surface area generated to only valid types. e.g. Manager is declared as directoryObject but can only be a user.
• ContainsTargets that are wrong should be corrected to prevent auto expansion of navigation properties
• Where we want non-contained navigation properties to have child paths, we must explicitly add annotations to describe the supported capabilities
• As mentioned by Vincent, we must ensure that Operations bound to the base type are not regenerated for the path with the cast to the derived type.

@baywet There is no support for unbound operations.

[irvinesunday]

@andrueastman to get operations bound on derived types of contained navigation properties, you just need to set AppendBoundOperationsOnDerivedTypeCastSegments = true
For the first example provided: https://docs.microsoft.com/en-us/graph/api/ediscovery-caseexportoperation-getdownloadurl?view=graph-rest-beta, the path can be generated as expected:

[andrueastman]

For context, at the moment, we generate about 7K paths for v1.0. Expanding the navigation properties of derived types for the single type cast segment only generates about 126K paths.

With the change made in #302, does this mean that this is no longer valid/correct as we were unnecessarily expanding path with ContainsTarget=False?.

[irvinesunday]

For context, at the moment, we generate about 7K paths for v1.0. Expanding the navigation properties of derived types for the single type cast segment only generates about 126K paths.

With the change made in #302, does this mean that this is no longer valid/correct as we were unnecessarily expanding path with ContainsTarget=False?.

Correct @andrueastman. Please check out this table for the appr. path figures: