Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SmallRye OpenAPI 3.6.2 changes contract for post/put/delete/patch when consumes/produces content type is not defined #36297

Closed
rsvoboda opened this issue Oct 4, 2023 · 8 comments
Labels
Milestone

Comments

@rsvoboda
Copy link
Member

rsvoboda commented Oct 4, 2023

Describe the bug

SmallRye OpenAPI 3.6.2 (introduced in #36279) changes generated contract in yaml schema file (text/plain vs. application/json) for post/put/delete/patch when consumes/produces primitives and content type is not explicitly defined

After fixing #34700 through #36198 the default content type was text/plain for primitives. This was true for response and also request content type.

I'm using simple endpoint that works with String - https://github.com/quarkus-qe/beefy-scenarios/blob/main/603-spring-web-smallrye-openapi/src/main/java/org/acme/spring/web/PostController.java#L13

I would expect the behaviour that was before SmallRye OpenAPI 3.6.2 merge, where request and response in generated schema yaml file were on the same content type (text/plain) when working with String.

Expected behavior

  /post/no-type:
    post:
      tags:
      - Post Controller
      requestBody:
        content:
          text/plain:
            schema:
              type: string
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: string

Actual behavior

  /post/no-type:
    post:
      tags:
      - Post Controller
      requestBody:
        content:
          application/json:
            schema:
              type: string
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: string

How to Reproduce?

Run https://github.com/quarkus-qe/beefy-scenarios/blob/main/603-spring-web-smallrye-openapi app in dev mode
Open http://localhost:8080/q/dev-ui/io.quarkus.quarkus-smallrye-openapi/schema-yaml
Search for /post/no-type:

Output of uname -a or ver

macOS

Output of java -version

Java 17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

841b3a1

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@rsvoboda rsvoboda added the kind/bug Something isn't working label Oct 4, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 4, 2023

/cc @EricWittmann (openapi), @Ladicek (smallrye), @MikeEdgar (openapi), @jmartisk (smallrye), @phillip-kruger (openapi,smallrye), @radcortez (smallrye)

@michalvavrik
Copy link
Member

I don't understand why expected type is text/plain nor why actual type is application/json. This is Spring Web and I don't see defaults here https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/bind/annotation/PostMapping.html#consumes(), so I tried to call it and endpoint accepts any type, while it produces by default application/octet-stream.

Is expected / actual behavior according to how endpoints behave? Isn't actual type closer to */*?

[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://0.0.0.0:8080/post/no-type
*   Trying 0.0.0.0:8080...
* Connected to 0.0.0.0 (127.0.0.1) port 8080 (#0)
> POST /post/no-type HTTP/1.1
> Host: 0.0.0.0:8080
> User-Agent: curl/8.0.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: application/octet-stream
< content-length: 0
< 
* Connection #0 to host 0.0.0.0 left intact
[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://0.0.0.0:8080/post/no-type -H "Content-type: application/json"
*   Trying 0.0.0.0:8080...
* Connected to 0.0.0.0 (127.0.0.1) port 8080 (#0)
> POST /post/no-type HTTP/1.1
> Host: 0.0.0.0:8080
> User-Agent: curl/8.0.1
> Accept: */*
> Content-type: application/json
> 
< HTTP/1.1 200 OK
< Content-Type: application/octet-stream
< content-length: 0
< 
* Connection #0 to host 0.0.0.0 left intact
[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://0.0.0.0:8080/post/no-type -H "Content-type: text/plain"
*   Trying 0.0.0.0:8080...
* Connected to 0.0.0.0 (127.0.0.1) port 8080 (#0)
> POST /post/no-type HTTP/1.1
> Host: 0.0.0.0:8080
> User-Agent: curl/8.0.1
> Accept: */*
> Content-type: text/plain
> 
< HTTP/1.1 200 OK
< Content-Type: application/octet-stream
< content-length: 0
< 
* Connection #0 to host 0.0.0.0 left intact

@rsvoboda
Copy link
Member Author

rsvoboda commented Oct 4, 2023

This issue is about content generated through open api (http://localhost:8080/q/dev-ui/io.quarkus.quarkus-smallrye-openapi/schema-yaml)

Spring Web extension is just about API, so the defaults are the same as with Quarkus "native" REST endpoints.

while it produces by default application/octet-stream ... you would need to specify something like -H 'Accept: application/json' in curl command to hint about what client expects/handles.

Adjusted the title and description to be more explicit about generated schema yaml file content.

@rsvoboda rsvoboda changed the title SmallRye OpenAPI 3.6.2 changes behaviour for post/put/delete/patch when consumes/produces content type is not defined SmallRye OpenAPI 3.6.2 changes contract for post/put/delete/patch when consumes/produces content type is not defined Oct 4, 2023
@michalvavrik
Copy link
Member

This issue is about content generated through open api (http://localhost:8080/q/dev-ui/io.quarkus.quarkus-smallrye-openapi/schema-yaml)

OpenAPI document establishes contract, therefore I expect it to tell me exactly what is expected to accept. Why should it default to text/plain, is that default accept type?

Spring Web extension is just about API, so the defaults are the same as with Quarkus "native" REST endpoints.

I adjusted getting-started org.acme.getting.started.GreetingResource and replaced /hello/greeting/{name} GET for POST and dropped @Produces and see:

[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://localhost:8080/hello/greeting/my-name -H "Accept: text/plain"
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /hello/greeting/my-name HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.0.1
> Accept: text/plain
> 
< HTTP/1.1 200 OK
< content-length: 13
< Content-Type: text/plain;charset=UTF-8
< 
* Connection #0 to host localhost left intact
hello my-name[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://localhost:8080/hello/greeting/my-name -H "Accept: application/json"
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /hello/greeting/my-name HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/8.0.1
> Accept: application/json
> 
< HTTP/1.1 406 Not Acceptable
< content-length: 0
< 
* Connection #0 to host localhost left intact

for Spring Web endpoint, both requests are accepted, therefore it is not same?

while it produces by default application/octet-stream ... you would need to specify something like -H 'Accept: application/json' in curl command to hing about what client expects/handles.

Still accepts any content type:

[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://0.0.0.0:8080/post/no-type -H "Accept: application/json"
*   Trying 0.0.0.0:8080...
* Connected to 0.0.0.0 (127.0.0.1) port 8080 (#0)
> POST /post/no-type HTTP/1.1
> Host: 0.0.0.0:8080
> User-Agent: curl/8.0.1
> Accept: application/json
> 
< HTTP/1.1 200 OK
< Content-Type: application/json
< content-length: 0
< 
* Connection #0 to host 0.0.0.0 left intact
[mvavrik@apc-ap8641-3a-d11d 603-spring-web-smallrye-openapi]$ curl -X POST -v http://0.0.0.0:8080/post/no-type -H "Accept: multipart/form-data"
*   Trying 0.0.0.0:8080...
* Connected to 0.0.0.0 (127.0.0.1) port 8080 (#0)
> POST /post/no-type HTTP/1.1
> Host: 0.0.0.0:8080
> User-Agent: curl/8.0.1
> Accept: multipart/form-data
> 
< HTTP/1.1 200 OK
< Content-Type: multipart/form-data
< content-length: 0
< 
* Connection #0 to host 0.0.0.0 left intact

@phillip-kruger
Copy link
Member

In SmallRye OpenAPI , the default type when not specified is */*, that is the most correct default I think, as the content negotiation can happen at runtime. However, Quarkus RESTEasy and SpringWeb extensions set some defaults. In general, when nothing is specified, the defaults are derived from the response type and is mostly application/json. Initially, the openapi quarkus extension tried to align with that by making the default application/json rather than */*.

This was the case for a while, until a bug was logged that there is a mismatch with the default when the response type is a primitive or String, and in that case the response type should be text/plain. This is the changes that was made recently.

As with any defaults, there will be cases where the default is not what a user want, and they need to add an annotation to create what they want. The default is just trying to cover the usual case.

W.r.t consume vs produce, on the RESTEasy side of things, the Produce (response type) is fairly straight forward to decide the type, as it's based on the response type as described above. For the consume (input parameters) it's a bit different. The RestEasy extension does not have to set any value of the response matching this, and in most cases the parameters are define in the path, query or form parameters, so the only case where we try to set this automatically is in the case of the body in a POST/PUT. After some discussion it was decided to leave this default as application/json as this align with what is assumed in RESTEasy.

What we did realized with this effort is that we need to decide if we are doing sensible defaults, or if we want to correctly set the value automatically. The latter is much more involved and would need some collaboration between open api and resteasy extensions.

// cc @cescoffier @geoand @FroMage

@cescoffier
Copy link
Member

I discussed the matter with @phillip-kruger yesterday, and his summary is totally accurate.

The generated openapi descriptor must be align with what RestEasy (reactive) is doing. Otherwise it would break (like it did) swagger and discard almost all usage of the descriptor.

We also discussed that in a first step we will only fix String. Then we will create a test case to discuss the other types (primitives, buffer, json objects/array, file, byebuff, byte[]....). We may need to introduce new build item to give to RR a way to indicate the types/media types to openapi.

@geoand
Copy link
Contributor

geoand commented Oct 5, 2023

Thanks folks.

For sure, let's make things easier for OpenAPI and more correct for users!

@rsvoboda
Copy link
Member Author

rsvoboda commented Nov 8, 2023

Fixed in #36514, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants