gnostic icon indicating copy to clipboard operation
gnostic copied to clipboard

protoc-gen-openapi: Nullable for fields in OpenAPI

Open jan-sykora opened this issue 2 years ago • 5 comments

Would it be possible to add an option to protoc-gen-openapi to generate proto3 fields labeled as optional to OpenAPI object properties with label nullable: true?

For example this proto file:

syntax = "proto3";

message Foo {
  optional int32 bar = 1;
}

when generated by protoc-gen-openapi with some new option (e.g. proto3-optional-nullable=true) would be generated as OpenAPI spec:

openapi: 3.0.3
components:
  schemas:
    Foo:
      properties:
        bar:
          type: integer
          format: int32
          nullable: true

If I understand it correctly:

  • All fields in proto3 are optional
  • When a field in proto3 is not explicitly set, it is set with default (zero) value when (un)marshaling from/into wire format.
  • To distinguish in proto3 whether a field was explicitly set with some value or not (explicit field presence), we can use the optional label
    • then the generated stubs use for instance in case of Go *int32 instead of int32 so the nil value represents whether the field was explicitly set
  • By default all object properties in OpenAPI3.0 are optional (but unlike proto3 can be marked as required).
  • When an optional object property is not set in OpenAPI3.0, no default value is used during (de)serialization from/into JSON format. The object property is just not present.
    • However, the property presence can be also 'simulated' by using the same approach as in protobuf -> setting a null value to the object property. But to make it work, the property must be labeled as nullable.

The similar feature is supported in grpc-gateway/protoc-gen-openapiv2 - it supports option to generate object properties in OAS2 as nullable -> the main motivation for this option is that some other tools generating code from the (generated) OpenAPI spec cannot handle default (null) values set during the transition from proto3 marshaled wire format to openapi json.

This issue is related to #275.

jan-sykora avatar Apr 22 '22 09:04 jan-sykora

Hi @jan-sykora, it would make a lot of sense to have this. I would even say that it should probably be default behaviour to add nullable for optional fields, since it's a non-breaking change that adds valuable information for both humans and machines.

morphar avatar Apr 27 '22 07:04 morphar

I have noticed a potential inconsistency regarding this mapping. When the optional proto field is a message, it is converted into OpenAPI as a schema component that is then referenced.

The problem is when there's a protofile like this:

message Foo {
  optional google.protobuf.Duration optional_bar_duration = 1;
  google.protobuf.Duration bar_duration = 1;
}

message google.protobuf.Duration {
  int64 seconds = 1;
  int32 nanos = 2;
}

then it's generated as

components:
  schemas:
    Foo:
      properties:
        optionalBarDuration:
          $ref: '#/components/schemas/Duration'
        barDuration:
          $ref: '#/components/schemas/Duration'
    Duration:
      type: object
      properties:
        seconds:
          type: integer
          format: int64
        nanos:
          type: integer
          format: int32

-> we cannot mark the whole Duration object as nullable because then it would be applied for both optional_bar_duration and bar_duration (but we want it to be applied only on optional_bar_duration).

jan-sykora avatar Apr 27 '22 09:04 jan-sykora

Also created related issue #351.

jan-sykora avatar Apr 27 '22 12:04 jan-sykora

Good catch :) I agree that this should be a string. We should look into how we can handle optional message types best.

morphar avatar Apr 28 '22 06:04 morphar

@morphar / @jan-sykora the optional protocol buffer option doesn't actually make the field nullable. To make a nullable int32 (or any other primitive type nullable) you need to use one of the wrapper types.

#366 added support for the wrapper types, however, it looks like it doesn't add the nullable: true property to the output. We can probably update that code to spit out nullable.

Maybe fix this along with #351 as they are sort of related to the well known types.

jeffsawatzky avatar Apr 07 '23 02:04 jeffsawatzky