Skip to content

Handles whether a request body is set as required or not in the specification.#1608

Closed
emilien-puget wants to merge 1 commit into
oapi-codegen:mainfrom
emilien-puget:request-body-require
Closed

Handles whether a request body is set as required or not in the specification.#1608
emilien-puget wants to merge 1 commit into
oapi-codegen:mainfrom
emilien-puget:request-body-require

Conversation

@emilien-puget

@emilien-puget emilien-puget commented May 11, 2024

Copy link
Copy Markdown

As briefly discussed there

The current behavior of the code generation is a mix between always required and always optional bodies.
However the open api specification is pretty clear about that requirement of body
see https://spec.openapis.org/oas/v3.1.0#fixed-fields-10 or https://swagger.io/docs/specification/describing-request-body/

Request bodies are optional by default.

this PR aims to bring the code generation closer to the intent of the open api specification.

using this spec as an example

  /requiredrequests/:
    post:
      requestBody:
        required: true
        content:
....
  /requests/:
    post:
      requestBody:
        required: false

type declaration

The issue here is that a body is always optional, using a pointer to convey this intent.

current behavior:

type PostRequiredrequestsRequestObject struct {
	Body *PostRequiredrequestsJSONRequestBody
}

a required type will always be a pointer.

suggested behavior:

type PostRequiredrequestsRequestObject struct {
	Body PostRequiredrequestsJSONRequestBody
}

we drop the ptr since the body was marqued as required in the open api specification.

as seen here, the type generation is considering a request body as being always optional.

strict function handler

The issue here is that a body is always required.

current behavior

var request PostRequestsRequestObject

var body PostRequestsJSONRequestBody
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
	sh.options.RequestErrorHandlerFunc(w, r, fmt.Errorf("can't decode JSON body: %w", err))
	return
}
request.Body = &body

suggested behavior:

var request PostRequestsRequestObject

if strings.HasPrefix(r.Header.Get("Content-Type"), "application/json") {

	var body PostRequestsJSONRequestBody
	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
		sh.options.RequestErrorHandlerFunc(w, r, fmt.Errorf("can't decode JSON body: %w", err))
		return
	}
	request.JSONBody = &body
}

since the api specification states that the body is not required, which is the default value for the required attribute, we need to check that the body is given to us before trying to decode it.

this may not be a great way to check that the body is there, since a user of the api could set the content type header and not setting a body.

I think this proposed change should be at one point the default behavior of the code generation as it is the default behavior of the open api specification

wdyt ?

  • implements the same behavior for all the strict server
  • body check detection using content type header may be a bit si bit simplistic

@mromaszewicz

Copy link
Copy Markdown
Member

Thank you for submitting this PR, @emilien-puget — but we need to close it because the primary bug (strict server unconditionally erroring when no body was sent for optional request bodies) was fixed by #2222 (commit 99615d04). All five strict-server templates (stdhttp, echo, gin, fiber, iris) now correctly check for io.EOF and treat an absent body as nil for optional bodies while still requiring one for required: true operations. The secondary enhancement you proposed (non-pointer struct fields for required bodies) wasn't adopted, but the core bug is resolved.

Thanks for the contribution.

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.

2 participants