nytimes / openapi2proto

A tool for generating Protobuf v3 schemas and gRPC service definitions from OpenAPI specifications
Apache License 2.0
961 stars 98 forks source link

Support data model using allOf #89

Open dobl1 opened 6 years ago

dobl1 commented 6 years ago

Exemple file

openapi: "3.0.0"
info:
  description: "This is a example swagger file"
  version: "1.0.0"
  title: "Swagger Example"
components:
  schemas:
    Person:
      type: object
      required:
      - name
      properties:
        gender:
            type: string
            description: Gender of the person
            default: Mr
            enum:
            - Miss
            - Mr
            - Mrs
            - Ms
        name:
          type: string
        address:
          $ref: '#/definitions/Address'
        age:
          type: integer
          format: int32
          minimum: 0
        map:
          type: object
          additionalProperties:
            type: string
        hobbies:
          type: array
          items:
            type: string
          example: ["sport", "music", "movies"]
    ExtendedPerson:
      properties:
        allOf:
        - $ref: '#/definitions/Person'
        - type: object
          required:
          - birthday
          properties:
            birthday:
              type: string
              format: date
    Address: 
      type: object 
      properties: 
        streetNumber: 
          type: integer
          format: int32
        streetName: 
          type: string 
        postalCode: 
          type: integer

Give

    syntax = "proto3";

    package ;

    message Address {
        int32 postalCode = 1;
        string streetName = 2;
        int32 streetNumber = 3;
    }

    message ExtendedPerson {}

    message Person {
        enum PersonGender {
            PERSON_GENDER_MISS = 0;
            PERSON_GENDER_MR = 1;
            PERSON_GENDER_MRS = 2;
            PERSON_GENDER_MS = 3;
        }

        Address address = 1;
        int32 age = 2;

        // Gender of the person
        PersonGender gender = 3;
        repeated string hobbies = 4;
        map<string, string> map = 5;
        string name = 6;
    }

message ExtendedPerson {} is not generated

Should return

    message ExtendedPerson {
        int32 postalCode = 1;
        string streetName = 2;
        int32 streetNumber = 3;
        string birthday = 4;
    }
lestrrat commented 5 years ago

Before diving into the details, I think there are a few things to note:

  1. I believe this tool only works with openapi: 2.0.
  2. properties should not contain allOf. it should be directly underneath ExtendedPerson

notes: the structure needs to be restricted to a hash (I believe it already is), and we basically just perform a merge first, and then apply the compilation on the result.

If the merge finds conflicts, it should probably report an error.

How should the merge work? probably the correct way to do it is to merge every level. Does that make sense in terms of cost and what not?

darkantoine commented 5 years ago

I believe allOf is supported in openapi 2.0 : https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md

lestrrat commented 5 years ago

hi thanks for pointing it out. I know it is supported. it's just an observation.

it doesn't change the fact that 1. OP might be confused to begin with (including the error on properties bit) and 2. this tool is not meant to support openapi v3 and yet users may not be necessarily aware of it.

the latter is one of the things that really but me while rewriting significant chunks of this software a few months back. namely, it does not tell you of errors in the document e.g. missing required fields, wrong element names, etc. so I thought I'd point that out

kevinmu commented 5 years ago

Just tried it; the tool doesn't work w/ allOf, even when it's placed directly underneath the defined entity. Adding this feature would definitely increase the usefulness of this tool!

lestrrat commented 5 years ago

Well, I have my own itch to scratch, and have been working on https://github.com/lestrrat-go/openapi. I just tried writing up preliminary allOf support there, and have been able to generate the following thus far:

swagger: "2.0"
info:
  version: "1.0.0"
  title: "definition using allOf"
  description: "blah"
definitions:
  foo:
    type: object
    properties:
      one:
        type: string
      two:
        type: string
  bar:
    type: object
    properties:
      three:
        type: integer
      four:
        type: integer
paths:
  "/":
    get:
      operationId: "root"
      description: "root"
      parameters:
        - name: param
          in: body
          schema:
            allOf:
              - $ref: "#/definitions/foo"
              - $ref: "#/definitions/bar"
      responses:
        default:
          description: success
syntax = "proto3";

package myapp;

import "google/protobuf/empty.proto";

message Bar {
  int32 four = 1;
  int32 three = 2;
}

message Foo {
  string one = 1;
  string two = 2;
}

message RootRequest {
  message Param {
    int32 four = 1;
    string one = 2;
    int32 three = 3;
    string two = 4;
  }
  Param param = 1;
}

service Myapp {
  // root
  rpc Root(RootRequest) returns (google.protobuf.Empty) {
  }
}

Right now I have to finish this up, so don't know when I'll be back to implementing it here,

darkantoine commented 5 years ago

looks promising!

it's a bit sad protocol buffer does not support inheritance. In this example we lose completly the fact the Param is made of Foo and Bar. Perhaps we could imagine having the option to use composition and produce:

message Param {
    Foo foo = 1;
    Bar bar = 2;
}

WDYT?

elyashivlavi commented 4 years ago

@darkantoine This will not translate well if we are using grpc-gateway. allOf means you will have a single object with all the properties specified, not 2 different objects.

elyashivlavi commented 4 years ago

I made a pull request that handles a single allOf case, which is useful if you want a different description.

For example, consider the following:

definitions:
  foo:
    type: object
    properties:
      one:
        $ref: '#/definitions/bar'
        description: Bar2
  bar:
    type: string
     description: Bar

The Bar2 description is ignored due to the $ref. So a possible solution is:

definitions:
  foo:
    type: object
    properties:
      one:
        allOf:
             $ref: '#/definitions/bar'
        description: Bar2
  bar:
    type: string
     description: Bar