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

translation to anyOf and oneOf has been inverted throughout the conversion process #245

Closed
4 tasks done
baywet opened this issue Jul 4, 2022 · 0 comments · Fixed by #264
Closed
4 tasks done

translation to anyOf and oneOf has been inverted throughout the conversion process #245

baywet opened this issue Jul 4, 2022 · 0 comments · Fixed by #264
Assignees
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days type:bug A broken experience
Milestone

Comments

@baywet
Copy link
Member

baywet commented Jul 4, 2022

According to the JSON schema spec

AnyOf

An instance validates successfully against this keyword if it
validates successfully against at least one schema defined by this
keyword's value.

OneOf

An instance validates successfully against this keyword if it
validates successfully against exactly one schema defined by this
keyword's value.

There are a number of places where the conversion library is using anyOf when it should be using instead oneOf, and it seems the confusion originates from the early days of the library, as this comment would indicate.

If we take the example of OData double to illustrate this issue, the library currently produces this.

                zoom:
                  anyOf:
                    - type: number
                    - type: string
                    - enum:
                        - '-INF'
                        - INF
                        - NaN
                  format: double
                  nullable: true

Which could be translated to a human sentence to "any instance that is a number (rational) or a string or -INF or INF or NaN is a double".
But the missing details here is that we're using "or" and not "xor" (exclusive or).
Semantically speaking, the value will (and should) never be a number AND -INF for example.

I believe the right human sentence should be "any instance that is a number (rational) or a string or -INF or INF or NaN to exactly one of these options is a double". Which would result in the following.

                zoom:
                  oneOf:
                    - type: number
                    - type: string
                    - enum:
                        - '-INF'
                        - INF
                        - NaN
                  format: double
                  nullable: true

This is even more evident with the geometric data structures where a geometric information can be a point or a line but not both (mathematically speaking, a line is defined a continuous set of points).

AnyOf = new List<OpenApiSchema>
{
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryPoint" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryLineString" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryPolygon" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryMultiPoint" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryMultiLineString" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryMultiPolygon" } },
new OpenApiSchema { UnresolvedReference = true, Reference = new OpenApiReference { Type = ReferenceType.Schema, Id = "Edm.GeometryCollection" } }

TODO

  • update geometric points (link above) to oneOf
  • update decimal and down to oneOf
  • update unit and integration tests
  • (optional) make components for Edm double, decimal and single
@baywet baywet added type:bug A broken experience priority:p3 Nice to have. Customer impact is very minimal labels Jul 4, 2022
@baywet baywet added this to the OData:1.0.11 milestone Jul 4, 2022
@baywet baywet added priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days and removed priority:p3 Nice to have. Customer impact is very minimal labels Jul 4, 2022
@CarolKigoonya CarolKigoonya modified the milestones: OData:1.0.11, OData:1.1 Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p2 Medium. Generally has a work-around and a smaller sub-set of customers is affected. SLA <=30 days type:bug A broken experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants