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

Ambiguity in oneOf responses causing deserialization issues #154

Open
datsirul opened this issue Jul 24, 2024 · 1 comment
Open

Ambiguity in oneOf responses causing deserialization issues #154

datsirul opened this issue Jul 24, 2024 · 1 comment

Comments

@datsirul
Copy link
Contributor

We have encountered an issue with the OpenAPI endpoints specifications related to the use of the oneOf keyword for some model response validations. The problem arises due to ambiguity between the possible responses, specifically:

  • beaconBooleanResponse
  • beaconCountResponse
  • beaconResultsetsResponse

Problem Details:

Here are the specifics:

  1. beaconBooleanResponse requires one property titled exist in responseSummary.
  2. beaconCountResponse requires two properties titled exist and numTotalResults in responseSummary.
  3. beaconResultsetsResponse could be ambiguous as well, as the other responses can also be valid when deserializing it.

When generating code from the OpenAPI documents, this ambiguity causes problems in certain languages, such as Python. The issue occurs because the deserialization process can mistakenly identify a beaconCountResponse or beaconResultsetsResponse as valid for multiple response types due to overlapping properties.

This leads to exceptions being thrown because the code cannot determine which response type to use.

Example Scenario:

  • Deserializing a beaconCountResponse object:

    • The object contains both exist and numTotalResults.
    • Since beaconBooleanResponse also requires exist, the deserialization process might mistakenly validate it as a beaconBooleanResponse.
    • The code then throws an exception due to this ambiguity.
  • Deserializing a beaconResultsetsResponse object:

    • The object might contain properties that overlap with beaconBooleanResponse or beaconCountResponse.
    • This causes similar issues in distinguishing the correct response type.

Code Snippet:

components:
  responses:
    ResultsOKResponse:
      description: Successful operation.
      content:
        application/json:
          schema:
            oneOf:
              - $ref: .../responses/beaconBooleanResponse.json
              - $ref: .../responses/beaconCountResponse.json
              - $ref: .../responses/beaconResultsetsResponse.json

Possible Solution:

A possible solution to this issue would be to add an additional discriminator property to each response type that clearly indicates which response type is being used. This would help the deserialization process to accurately identify the correct response type and avoid ambiguity.

For example:

components:
  responses:
    ResultsOKResponse:
      description: Successful operation.
      content:
        application/json:
          schema:
            oneOf:
              - $ref: .../responses/beaconBooleanResponse.json
              - $ref: .../responses/beaconCountResponse.json
              - $ref: .../responses/beaconResultsetsResponse.json
            discriminator:
              propertyName: objectType

Additionally, add another required property objectType to each response type. When returning a response, the server should include this property with the appropriate response value (beaconBooleanResponse, beaconCountResponse or beaconResultsetsResponse) to indicate the type of response being returned.

Impact:

This issue impacts the generation and usage of client libraries in languages like Python, where the deserialization process cannot accurately distinguish between beaconBooleanResponse, beaconCountResponse, and beaconResultsetsResponse. This leads to runtime errors and exceptions, hampering the usability and reliability of the generated code.

Note:

I understand the current move away from "OpenAPI first" (#40), but though this is important as even generated OpenAPI docs may contains this issue.

@redmitry
Copy link
Collaborator

@datsirul
Hi Daniel,

IMHO, the "quick and dirt" fix would be to put "not": { "required": [ "numTotalResults" ] } in the beaconBooleanResponseSection.

D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants