Skip to content

Conversation

@gturri
Copy link
Contributor

@gturri gturri commented May 9, 2025

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

This fixes #21256 (and this subsequently fixes #13334, which is a subset of the other one).

This PR implements the 3 fixes that are required to be able to handle all the endpoints described in this OpenApi specification:

openapi: 3.0.3
info:
  title: repro issue openapi-generator
  version: 0.0.1
paths:
  /get-bool:
    get:
      summary: get a boolean
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: boolean
  /get-image:
    get:
      summary: get an image
      responses:
        "200":
          description: OK
          content:
            image/png:
              schema:
                type: string
                format: binary
  /get-string:
    get:
      summary: get a string
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: string
  /get-int:
    get:
      summary: get an int
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: integer
  /get-number:
    get:
      summary: get a number
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: number

Namely this:

  • ensures the generated method Controller.getOutputFormat returns a valid format when the user accepts */*
  • does not try to call ->serialize($data, ...)`` when the 2nd argument is null` (it would crash (error 500)). Instead it uses a straightforward way to handle the serialization in those simple cases
  • define relevant return type when the specification declares response with context text/plain

(nb: to make it easier to review I did a separate commit for each of those 3 fixes)

Ping technical committed, ie: @jebentier @dkarlovi @mandrean @jfastnacht @ybelenko @renepardon (and thanks for your work!)

gturri added 3 commits May 9, 2025 22:30
When a query has header "Accept" set to "*/*" it means it accepts
everything. It is hence weird to return a 406.
This patch ensures it does not occur: when the query accepts everything
then we take any produced type.

This fixes #13334. This also partly makes the open PR #15560 obsolete
(or at least, it provides a workaround)
$this->convertFormat may return "null". When it's the case we end up
calling

    ...->serialize($data, null);

but this crashes at runtime because that serialize method declares that
the 2nd parameter is of type "string" (so null is not accepted).

With this patch we avoid having an error 500. Instead we return something
that makes perfect sense when the OpenApi specification declares a content
of type "text/plain" and that the returned value is for instance a string,
an int, or a boolean.
This fixes the generated returned type of controller methods for
endpoint with a response declared like

    content:
      text/plain:
        schema:
          type: <boolean|string|integer|number>

or for

    content:
      image/png:
        schema:
          type: string
          format: binary

Without this commit the generated method *had to* return a value that
matched "array|object|null", which does not work in this case.
This commit makes it possible to return the proper type.
@gturri
Copy link
Contributor Author

gturri commented May 10, 2025

I'm afraid I'm seeing an issue in the last commit that I did not spot yesterday. I'm closing that PR and will recreate a new one once I have more thoroughly tested it.
Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant