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

protoc-gen-openapi: Support oneof generation for messages having only one such field #446

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coxley
Copy link

@coxley coxley commented Feb 5, 2025

Summary

Related discussion is in #251, with a big disclaimer that this only changes generation for messages with a single oneof block. I've gated this behind --openapi_out=oneof=1 to make it opt-in.

We sync the generated output to API documentation tooling, and the lack of oneof support makes it confusing for consumers to understand how to use endpoints. The techniques in the linked issue don't really address this — they're focused on marking required combinations for the fields. What we need to convey is that the fields can't be provided in the same payload at all.

This leaves us with a structure that looks like this:

Message:
  type: object
  allOf:
    - type: object
      properties:
      message_id:
        type: string
      text:
        type: string
      double:
        $ref: '#/components/schemas/Double'
    - oneOf:
      - title: email
        type: object
        properties:
        email:
          type: string
          description: Email address of the sender
          format: email
      - title: name
        type: object
        properties:
        name:
          type: string
          description: Full name of the sender

For a message that has multiple oneof, we fallback to normal behavior. Examples of how documentation generators parse this:

Redocly image image
Readme.com image image

(Using these as a benchmark since they're both popular. One for open source use, the other for enterprise)

Why limited the support?

I started by making each oneOf group a descendant of allOf to allow for multiple in a message. While this "works", I can't find a documentation tool that supports it nicely. It ends up showing a permutation of each oneOf combination. For example:

message Double {
  oneof v1 {
    string foo = 1;
    string bar = 2;
  }
  oneof v2 {
    string baz = 3;
    string qux = 4;
  }
}

Would generate this:

Double:
  description: |-
    Double demonstrates the generated output for a message with more than one `oneof`
      group
  allOf:
    - oneOf:
      - title: foo
        type: object
        properties:
        foo:
          type: string
      - title: bar
        type: object
        properties:
        bar:
          type: string
    - oneOf:
      - title: baz
        type: object
        properties:
        baz:
          type: string
      - title: qux
        type: object
        properties:
        qux:
          type: string

But it renders terribly on documentation sites:

Redocly image image image

Declaring each oneOf with a title like {"title": "v1", "oneOf": [...]} doesn't change anything on either redocly or readme.com.

Test Plan

> go install .
> go test . -v
=== RUN   TestGenerateFixturesIsFalse
--- PASS: TestGenerateFixturesIsFalse (0.00s)
=== RUN   TestOpenAPIProtobufNaming
=== RUN   TestOpenAPIProtobufNaming/Google_Library_example
=== RUN   TestOpenAPIProtobufNaming/Body_mapping
=== RUN   TestOpenAPIProtobufNaming/Map_fields
=== RUN   TestOpenAPIProtobufNaming/Path_params
=== RUN   TestOpenAPIProtobufNaming/Protobuf_types
=== RUN   TestOpenAPIProtobufNaming/RPC_types
=== RUN   TestOpenAPIProtobufNaming/JSON_options
=== RUN   TestOpenAPIProtobufNaming/Ignore_services_without_annotations
=== RUN   TestOpenAPIProtobufNaming/Enum_Options
=== RUN   TestOpenAPIProtobufNaming/OpenAPIv3_Annotations
=== RUN   TestOpenAPIProtobufNaming/AllOf_Wrap_Message
=== RUN   TestOpenAPIProtobufNaming/Additional_Bindings
--- PASS: TestOpenAPIProtobufNaming (0.33s)
    --- PASS: TestOpenAPIProtobufNaming/Google_Library_example (0.04s)
    --- PASS: TestOpenAPIProtobufNaming/Body_mapping (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/Map_fields (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/Path_params (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/Protobuf_types (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/RPC_types (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/JSON_options (0.02s)
    --- PASS: TestOpenAPIProtobufNaming/Ignore_services_without_annotations (0.02s)
    --- PASS: TestOpenAPIProtobufNaming/Enum_Options (0.02s)
    --- PASS: TestOpenAPIProtobufNaming/OpenAPIv3_Annotations (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/AllOf_Wrap_Message (0.03s)
    --- PASS: TestOpenAPIProtobufNaming/Additional_Bindings (0.02s)
=== RUN   TestOpenAPIFQSchemaNaming
=== RUN   TestOpenAPIFQSchemaNaming/Google_Library_example
=== RUN   TestOpenAPIFQSchemaNaming/Body_mapping
=== RUN   TestOpenAPIFQSchemaNaming/Map_fields
=== RUN   TestOpenAPIFQSchemaNaming/Path_params
=== RUN   TestOpenAPIFQSchemaNaming/Protobuf_types
=== RUN   TestOpenAPIFQSchemaNaming/RPC_types
=== RUN   TestOpenAPIFQSchemaNaming/JSON_options
=== RUN   TestOpenAPIFQSchemaNaming/Ignore_services_without_annotations
=== RUN   TestOpenAPIFQSchemaNaming/Enum_Options
=== RUN   TestOpenAPIFQSchemaNaming/OpenAPIv3_Annotations
--- PASS: TestOpenAPIFQSchemaNaming (0.37s)
    --- PASS: TestOpenAPIFQSchemaNaming/Google_Library_example (0.04s)
    --- PASS: TestOpenAPIFQSchemaNaming/Body_mapping (0.03s)
    --- PASS: TestOpenAPIFQSchemaNaming/Map_fields (0.03s)
    --- PASS: TestOpenAPIFQSchemaNaming/Path_params (0.03s)
    --- PASS: TestOpenAPIFQSchemaNaming/Protobuf_types (0.03s)
    --- PASS: TestOpenAPIFQSchemaNaming/RPC_types (0.03s)
    --- PASS: TestOpenAPIFQSchemaNaming/JSON_options (0.02s)
    --- PASS: TestOpenAPIFQSchemaNaming/Ignore_services_without_annotations (0.04s)
    --- PASS: TestOpenAPIFQSchemaNaming/Enum_Options (0.04s)
    --- PASS: TestOpenAPIFQSchemaNaming/OpenAPIv3_Annotations (0.04s)
=== RUN   TestOpenAPIJSONNaming
=== RUN   TestOpenAPIJSONNaming/Google_Library_example
=== RUN   TestOpenAPIJSONNaming/Body_mapping
=== RUN   TestOpenAPIJSONNaming/Map_fields
=== RUN   TestOpenAPIJSONNaming/Path_params
=== RUN   TestOpenAPIJSONNaming/Protobuf_types
=== RUN   TestOpenAPIJSONNaming/RPC_types
=== RUN   TestOpenAPIJSONNaming/JSON_options
=== RUN   TestOpenAPIJSONNaming/Ignore_services_without_annotations
=== RUN   TestOpenAPIJSONNaming/Enum_Options
=== RUN   TestOpenAPIJSONNaming/OpenAPIv3_Annotations
--- PASS: TestOpenAPIJSONNaming (0.27s)
    --- PASS: TestOpenAPIJSONNaming/Google_Library_example (0.02s)
    --- PASS: TestOpenAPIJSONNaming/Body_mapping (0.03s)
    --- PASS: TestOpenAPIJSONNaming/Map_fields (0.02s)
    --- PASS: TestOpenAPIJSONNaming/Path_params (0.03s)
    --- PASS: TestOpenAPIJSONNaming/Protobuf_types (0.03s)
    --- PASS: TestOpenAPIJSONNaming/RPC_types (0.02s)
    --- PASS: TestOpenAPIJSONNaming/JSON_options (0.03s)
    --- PASS: TestOpenAPIJSONNaming/Ignore_services_without_annotations (0.03s)
    --- PASS: TestOpenAPIJSONNaming/Enum_Options (0.03s)
    --- PASS: TestOpenAPIJSONNaming/OpenAPIv3_Annotations (0.03s)
=== RUN   TestOpenAPIStringEnums
=== RUN   TestOpenAPIStringEnums/Google_Library_example
=== RUN   TestOpenAPIStringEnums/Body_mapping
=== RUN   TestOpenAPIStringEnums/Map_fields
=== RUN   TestOpenAPIStringEnums/Path_params
=== RUN   TestOpenAPIStringEnums/Protobuf_types
=== RUN   TestOpenAPIStringEnums/RPC_types
=== RUN   TestOpenAPIStringEnums/JSON_options
=== RUN   TestOpenAPIStringEnums/Ignore_services_without_annotations
=== RUN   TestOpenAPIStringEnums/Enum_Options
=== RUN   TestOpenAPIStringEnums/OpenAPIv3_Annotations
--- PASS: TestOpenAPIStringEnums (0.27s)
    --- PASS: TestOpenAPIStringEnums/Google_Library_example (0.02s)
    --- PASS: TestOpenAPIStringEnums/Body_mapping (0.02s)
    --- PASS: TestOpenAPIStringEnums/Map_fields (0.03s)
    --- PASS: TestOpenAPIStringEnums/Path_params (0.03s)
    --- PASS: TestOpenAPIStringEnums/Protobuf_types (0.03s)
    --- PASS: TestOpenAPIStringEnums/RPC_types (0.02s)
    --- PASS: TestOpenAPIStringEnums/JSON_options (0.03s)
    --- PASS: TestOpenAPIStringEnums/Ignore_services_without_annotations (0.03s)
    --- PASS: TestOpenAPIStringEnums/Enum_Options (0.04s)
    --- PASS: TestOpenAPIStringEnums/OpenAPIv3_Annotations (0.03s)
=== RUN   TestOpenAPIDefaultResponse
=== RUN   TestOpenAPIDefaultResponse/Google_Library_example
=== RUN   TestOpenAPIDefaultResponse/Body_mapping
=== RUN   TestOpenAPIDefaultResponse/Map_fields
=== RUN   TestOpenAPIDefaultResponse/Path_params
=== RUN   TestOpenAPIDefaultResponse/Protobuf_types
=== RUN   TestOpenAPIDefaultResponse/RPC_types
=== RUN   TestOpenAPIDefaultResponse/JSON_options
=== RUN   TestOpenAPIDefaultResponse/Ignore_services_without_annotations
=== RUN   TestOpenAPIDefaultResponse/Enum_Options
=== RUN   TestOpenAPIDefaultResponse/OpenAPIv3_Annotations
--- PASS: TestOpenAPIDefaultResponse (0.26s)
    --- PASS: TestOpenAPIDefaultResponse/Google_Library_example (0.03s)
    --- PASS: TestOpenAPIDefaultResponse/Body_mapping (0.02s)
    --- PASS: TestOpenAPIDefaultResponse/Map_fields (0.03s)
    --- PASS: TestOpenAPIDefaultResponse/Path_params (0.02s)
    --- PASS: TestOpenAPIDefaultResponse/Protobuf_types (0.03s)
    --- PASS: TestOpenAPIDefaultResponse/RPC_types (0.02s)
    --- PASS: TestOpenAPIDefaultResponse/JSON_options (0.02s)
    --- PASS: TestOpenAPIDefaultResponse/Ignore_services_without_annotations (0.02s)
    --- PASS: TestOpenAPIDefaultResponse/Enum_Options (0.03s)
    --- PASS: TestOpenAPIDefaultResponse/OpenAPIv3_Annotations (0.03s)
=== RUN   TestOpenAPIOneOfs
=== RUN   TestOpenAPIOneOfs/OneOf_Fields
--- PASS: TestOpenAPIOneOfs (0.03s)
    --- PASS: TestOpenAPIOneOfs/OneOf_Fields (0.03s)
PASS
ok      github.com/google/gnostic/cmd/protoc-gen-openapi        1.897s

@coxley coxley requested a review from a team as a code owner February 5, 2025 22:09
While this falls back to previous behavior for messages having more
than one 'oneof', the overwhelming majority of our cases have a
single field. This lets us optimize documentation generation for
semantics where possible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant