oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

bug: MarshalJSON (and UnmarshalJSON) are not generated for the ResponseObject when the response is a oneOf schema

Open akishichinibu opened this issue 1 year ago • 8 comments

❯ go version
go version go1.22.1 darwin/arm64
❯ oapi-codegen --version
github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen
v2.3.0

Hi👋 , I think there's a bug in oapi-codegen v2.3.0 and there's a minimal example to reproduce the problem

oapi-codegen -config config.yaml test.yaml

config.yaml

# yaml-language-server: $schema=https://raw.githubusercontent.com/deepmap/oapi-codegen/HEAD/configuration-schema.json
package: "api"
generate:
  echo-server: true
  models: true
  strict-server: true
  embedded-spec: false
output: api.gen.go
output-options:
  nullable-type: true
  initialism-overrides: true

test.yaml

openapi: 3.0.3
info:
  title: Swagger Petstore - OpenAPI 3.0
  version: 1.0.11
paths:
  /pet/findByStatus:
    get:
      tags:
        - pet
      summary: Finds Pets by status
      description: Multiple status values can be provided with comma separated strings
      operationId: findPetsByStatus
      responses:
        '200':
          description: successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/PP'
components:
  schemas:
    Pet1:
      type: object
      properties:
        id:
          type: integer
          format: int64
          example: 10
        name:
          type: string
          example: doggie
    Pet2:
      type: object
      properties:
        id:
          type: integer
          format: int64
          example: 10
        name:
          type: string
          example: doggie
    PP:
      type: object
      oneOf:
        - "$ref": "#/components/schemas/Pet1"
        - "$ref": "#/components/schemas/Pet2"

Part of the generated server code

type PP struct {
	union json.RawMessage
}

func (t PP) MarshalJSON() ([]byte, error) {
	b, err := t.union.MarshalJSON()
	return b, err
}

func (t *PP) UnmarshalJSON(b []byte) error {
	err := t.union.UnmarshalJSON(b)
	return err
}

type FindPetsByStatusResponseObject interface {
	VisitFindPetsByStatusResponse(w http.ResponseWriter) error
}

type FindPetsByStatus200JSONResponse PP

func (response FindPetsByStatus200JSONResponse) VisitFindPetsByStatusResponse(w http.ResponseWriter) error {
	w.Header().Set("Content-Type", "application/json")
	w.WriteHeader(200)

	return json.NewEncoder(w).Encode(response)
}

The MarshalJSON was generated correctly for PP but NOT for the response object in strict-server mode. Because there's only one private member union in PP, when the user try to return a FindPetsByStatusResponseObject in handler like the code below

var p PP
p.FromPet1(...)
return FindPetsByStatus200JSONResponse(p), nil

the responseObject will be marshal to an empty JSON.

I think an additional MarshalJSON(and UnmarshalJSON) should be generated when the response is a oneOf schema.

func (t FindPetsByStatus200JSONResponse) MarshalJSON() ([]byte, error) {
	b, err := t.union.MarshalJSON()
	return b, err
}

akishichinibu avatar Jun 19 '24 09:06 akishichinibu

I think this issue is related to https://github.com/oapi-codegen/oapi-codegen/issues/1620

akishichinibu avatar Jun 19 '24 09:06 akishichinibu

I found a temporary workaround by using a user template. In the strict-interface template, check if the response's OpenAPI schema is oneOf when the contentType is JSON.

curl https://raw.githubusercontent.com/oapi-codegen/oapi-codegen/v2.3.0/pkg/codegen/templates/strict/strict-interface.tmpl -o strict-interface.tmpl
patch strict-interface.tmpl -o strict-interface.patched.tmpl  < strict-interface.patch
74a75,81
>             {{if eq .NameTagOrContentType "JSON" -}}
>             {{if or (ne 0 (len .Schema.OAPISchema.OneOf)) (ne 0 (len .Schema.OAPISchema.AnyOf)) (ne 0 (len .Schema.OAPISchema.AllOf)) -}}
>             func (response {{$receiverTypeName}}) MarshalJSON() ([]byte, error) {
>                 return response.union.MarshalJSON()
>             }
>             {{end -}}
>             {{end -}}

config.yaml

# yaml-language-server: $schema=https://raw.githubusercontent.com/deepmap/oapi-codegen/HEAD/configuration-schema.json
package: "api"
generate:
  echo-server: true
  models: true
  strict-server: true
  embedded-spec: false
output: api.gen.go
output-options:
  nullable-type: true
  initialism-overrides: true
  user-templates:
    strict/strict-interface.tmpl: ./strict-interface.patched.tmpl

akishichinibu avatar Jun 20 '24 04:06 akishichinibu

This issue is also discussed here https://github.com/oapi-codegen/oapi-codegen/issues/970. It comes with a PR #1231 but I'm not sure if it should be merged due to my final comment in the issue

dan-j avatar Jun 20 '24 20:06 dan-j

Also discussed under https://github.com/oapi-codegen/oapi-codegen/issues/970#issuecomment-2185437462 I seems that there're two ways to fix this bug.

akishichinibu avatar Jun 24 '24 02:06 akishichinibu

I found a temporary workaround by using a user template. In the strict-interface template, check if the response's OpenAPI schema is oneOf when the contentType is JSON.

curl https://raw.githubusercontent.com/oapi-codegen/oapi-codegen/v2.3.0/pkg/codegen/templates/strict/strict-interface.tmpl -o strict-interface.tmpl
patch strict-interface.tmpl -o strict-interface.patched.tmpl  < strict-interface.patch
74a75,81
>             {{if eq .NameTagOrContentType "JSON" -}}
>             {{if or (ne 0 (len .Schema.OAPISchema.OneOf)) (ne 0 (len .Schema.OAPISchema.AnyOf)) (ne 0 (len .Schema.OAPISchema.AllOf)) -}}
>             func (response {{$receiverTypeName}}) MarshalJSON() ([]byte, error) {
>                 return response.union.MarshalJSON()
>             }
>             {{end -}}
>             {{end -}}

config.yaml

# yaml-language-server: $schema=https://raw.githubusercontent.com/deepmap/oapi-codegen/HEAD/configuration-schema.json
package: "api"
generate:
  echo-server: true
  models: true
  strict-server: true
  embedded-spec: false
output: api.gen.go
output-options:
  nullable-type: true
  initialism-overrides: true
  user-templates:
    strict/strict-interface.tmpl: ./strict-interface.patched.tmpl

@akishichinibu i cant get your curl to work as I dont know what your specific strict-interface.patch file is. Can you provide an outline or a link to a working patch you have so I can diff it?

stefanmcshane avatar Nov 14 '24 00:11 stefanmcshane

@stefanmcshane The content of the patch file is here. Just below the curl.

strict-interface.patch

74a75,81
>             {{if eq .NameTagOrContentType "JSON" -}}
>             {{if or (ne 0 (len .Schema.OAPISchema.OneOf)) (ne 0 (len .Schema.OAPISchema.AnyOf)) (ne 0 (len .Schema.OAPISchema.AllOf)) -}}
>             func (response {{$receiverTypeName}}) MarshalJSON() ([]byte, error) {
>                 return response.union.MarshalJSON()
>             }
>             {{end -}}
>             {{end -}}

akishichinibu avatar Nov 14 '24 00:11 akishichinibu

Thanks - I see my mistake now. This applies the fix for responses, but not request bodies. Works a charm on 2.4.1 responses FWIW

stefanmcshane avatar Nov 14 '24 17:11 stefanmcshane

For anyone that applies this patch to the latest 2.4.1 version, make sure to adjust the line number because things shifted a bit in the latest strict-interface.tmpl file.

nustiueudinastea avatar Dec 04 '24 13:12 nustiueudinastea