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

Non-nullable restrictions on entity sets or navigation property paths #322

Open
HeikoTheissen opened this issue Dec 19, 2024 · 4 comments · May be fixed by #323
Open

Non-nullable restrictions on entity sets or navigation property paths #322

HeikoTheissen opened this issue Dec 19, 2024 · 4 comments · May be fixed by #323
Assignees

Comments

@HeikoTheissen
Copy link

HeikoTheissen commented Dec 19, 2024

Restrictions for a resource path are

  1. Capabilities.NavigationRestrictions/RestrictedProperties/…Restrictions if the resource path contains a navigation property path
  2. Capabilities.…Restrictions of the entity set or target path specified in a NavigationPropertyBinding for the resource path if no restrictions exist in step 1.

But TopSupported and SkipSupported are no …Restrictions in the sense of this rule because they are not nullable, hence automatically present as soon as there are Capabilities.NavigationRestrictions/RestrictedProperties for the navigation property path in question.

Hence the resource path /ReadOnlySingleton/ReadOnlyMany has both TopSupported and SkipSupported by default because the annotation found in step 1 does not contain these properties:

<Record>
<PropertyValue Property="NavigationProperty" NavigationPropertyPath="ReadOnlyMany" />
<PropertyValue Property="InsertRestrictions">
<Record>
<PropertyValue Property="Insertable" Path="switch" />
</Record>
</PropertyValue>
</Record>

But the OpenAPI description lacks the parameters top and skip:

"/ReadOnlySingleton/ReadOnlyMany": {
"get": {
"summary": "Get entities from related ReadOnlyMany",
"tags": [
"ReadOnlySingleton",
"TwoReadOnlySet"
],
"parameters": [],

@ralfhandl ralfhandl moved this to Open in OData TC Dec 19, 2024
@mikepizzo
Copy link

There should be a way to specify that the support of things like top/skip are unspecified at the navigationpropertyrestrictions, in which case the specification on the entity set (if any) applies -- and if not, the default is taken (as specified on the entityset).

This could be facilitated by making top/skip (etc.) nullable, and with no default, in the navigation property restrictions.

@HeikoTheissen HeikoTheissen moved this from Open to Resolved in OData TC Jan 9, 2025
@ralfhandl ralfhandl moved this from Resolved to Review in OData TC Jan 15, 2025
@ralfhandl ralfhandl moved this from Review to Open in OData TC Jan 15, 2025
@HeikoTheissen
Copy link
Author

HeikoTheissen commented Jan 16, 2025

In the example, the absence of TopSupported and SkipSupported in step 1 leads to omission of the parameters top and skip only because they are unsupported on the bound entity set in step 2:

<Annotation Term="Capabilities.SkipSupported" Bool="false" />
<Annotation Term="Capabilities.TopSupported" Bool="false" />

OData services generated by SAP omit TopSupported or SkipSupported not just in step 1 but also on the bound entity set in step 2. Parameters top and skip are therefore included in the OpenAPI description.

SAP has entity set and entity type in 1:1 correspondence and capabilities are maintained per "entity". It is therefore not possible to support top and skip via navigation property (in step 1) but reject them when directly accessing the entity set (in step 2).

@mikepizzo
Copy link

The current semantics of the vocabulary are well defined, but don't allow specifying navigation property restrictions without saying anything about support of things like top and skip.

The openapi generation tool semantics are different -- it assumes absence of top/skip in nav prop restrictions defers to the bound entity set.

We could:

  1. Add a feature to the protocol that allows specifying navigation property restrictions without making a statement about top/skip (i.e., by making top/skip nullable), or
  2. Fix the openapi generation tool to comply with the existing defined semantics

Option 1 has potential breaking change side-effects (existing customers not expecting to get back a null value, or understanding how to interpret it).

Option 2 has the potential of changing the openapi generated for existing services that had different annotations for the navigation property binding and for entity set. This would not apply to SAP (which uses the same annotation in both places), but could affect other services. Such services (one that specified a navigation property restriction but did not specify top/skip in navigation property restrictions and annotated the table with false) would have to update their navigation property restriction to explicitly specify false for top/skip.

Option 3 would be to keep the existing defined semantics, but have the tool continue to generate using the current semantics.

@mikepizzo
Copy link

Plan is to do option 2 (fix the tool to comply with defined vocabulary defaults) and seek community input from the behavior.

@mikepizzo mikepizzo moved this from Open to Resolved in OData TC Jan 22, 2025
@HeikoTheissen HeikoTheissen moved this from Resolved to Review in OData TC Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review
2 participants