protoc-gen-connect-openapi icon indicating copy to clipboard operation
protoc-gen-connect-openapi copied to clipboard

Idempotency level "NO_SIDE_EFFECTS" generates both GET and POST

Open bvigar opened this issue 8 months ago • 3 comments

I'm not sure if i'm missing an option to the plugin, but when I run generate on the same example proto in the repo, I get BOTH the GET and POST definitions for the path instead of just the GET.

It also seems like the allow-get option is required to get the GET entry to even show up, otherwise it is generating a POST for the example Greeter.SayHello operation.

Here's my buf.gen.yaml

version: v2

plugins:
  - local: protoc-gen-connect-openapi
    out: gen/go/integration/spec
    strategy: all
    opt:
      - allow-get
      - path=openapi.yaml

It generates the following openapi definition for Greeter.SayHello

openapi: 3.1.0
info: {}
paths:
  /example.basic.Greeter/SayHello:
    get:
      tags:
        - example.basic.Greeter
      summary: SayHello
      description: This is a description just for OpenAPI
      operationId: example.basic.Greeter.SayHello.get
      parameters:
        - name: Connect-Protocol-Version
          in: header
          required: true
          schema:
            $ref: '#/components/schemas/connect-protocol-version'
        - name: Connect-Timeout-Ms
          in: header
          schema:
            $ref: '#/components/schemas/connect-timeout-header'
        - name: message
          in: query
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/example.basic.HelloRequest'
        - name: encoding
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/example.basic.HelloRequest'
        - name: encoding
          in: query
          required: true
          schema:
            $ref: '#/components/schemas/encoding'
        - name: base64
          in: query
          schema:
            $ref: '#/components/schemas/base64'
        - name: compression
          in: query
          schema:
            $ref: '#/components/schemas/compression'
        - name: connect
          in: query
          schema:
            $ref: '#/components/schemas/connect'
      responses:
        default:
          description: Error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/connect.error'
        "200":
          description: Success
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/example.basic.HelloReply'
    post:
      tags:
        - example.basic.Greeter
      summary: SayHello
      description: This is a description just for OpenAPI
      operationId: example.basic.Greeter.SayHello
      parameters:
        - name: Connect-Protocol-Version
          in: header
          required: true
          schema:
            $ref: '#/components/schemas/connect-protocol-version'
        - name: Connect-Timeout-Ms
          in: header
          schema:
            $ref: '#/components/schemas/connect-timeout-header'
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/example.basic.HelloRequest'
        required: true
      responses:
        default:
          description: Error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/connect.error'
        "200":
          description: Success
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/example.basic.HelloReply'

bvigar avatar Apr 16 '25 17:04 bvigar

This is actually intentional, because you can still use POST in these situations... And there are cases where it's preferred or required to use POST even though the RPC is marked as NO_SIDE_EFFECTS. The biggest reasons:

  • request size being larger than the max URL size (firewalls, proxies, servers often have a max size for URLs, which would include the entire request message in ConnectRPC)
  • intentional cache breaking
  • Lack of client support - GET requests are still relatively new for the Connect protocol, so some third party clients may have only implemented POST

All of that said, maybe there needs to be an option to give you the behavior that you're looking for. Maybe an option like prefer-get?:

version: v2

plugins:
  - local: protoc-gen-connect-openapi
    out: gen/go/integration/spec
    strategy: all
    opt:
      - allow-get
      - prefer-get
      - path=openapi.yaml

sudorandom avatar Apr 17 '25 07:04 sudorandom

That makes a lot of sense... thank you for the detail! I don't think it's a big enough deal to warrant adding support for a new prefer-get option to be honest, given your explanation, i'm on board with the POST hanging around in there. My initial skepticism was because i'm generating a typescript client from my openapi spec and I didn't want people to be confused about having both a GET and POST method for the same operation.

bvigar avatar Apr 17 '25 14:04 bvigar

Just FYI, i'm using the rtk-query/codegen-openapi npm package to generate it and the typescript compiler is not happy with the resulting client because of the number enums in the spec

index.ts(11,11): error TS2322: Type 'number' is not assignable to type 'string'.

bvigar avatar Apr 17 '25 14:04 bvigar