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

Setting quarkus.resteasy.path not reflected in OpenAPI or Swagger #19434

Closed
kjq opened this issue Aug 16, 2021 · 18 comments · Fixed by #19475
Closed

Setting quarkus.resteasy.path not reflected in OpenAPI or Swagger #19434

kjq opened this issue Aug 16, 2021 · 18 comments · Fixed by #19475

Comments

@kjq
Copy link

kjq commented Aug 16, 2021

Describe the bug

When I set the "quarkus.resteast.path" it works making API calls directly but does not get reflected in the OpenAPI or Swagger.

Using @ApplicationPath and setting eash @path looks to work.

Expected behavior

OpenAPI and Swagger reflect the path set in the application properties.

Actual behavior

RESTful calls works but OpenAPI and Swagger do not reflect the path. From Swagger invoking an endpoint gives a 404.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

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

No response

Additional information

No response

@kjq kjq added the kind/bug Something isn't working label Aug 16, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 16, 2021

@phillip-kruger
Copy link
Member

Do you perhaps have a small reproducer ?

@kjq
Copy link
Author

kjq commented Aug 17, 2021

I tried to build a small reproducer but I am not sure if it is valid or not.

For whatever reason, it seems when I use "quarkus.resteasy.path" it does not get recognized but using "rest.path" does. I've now managed to confused myself. Either way, if this is valid, the SwaggerUI does not reflect the path set in the application.yaml.

I will look into it a little more later today but maybe for starters this helps.

openapi.zip

Screenshot 2021-08-17 092207

@phillip-kruger
Copy link
Member

Maybe a yaml issue ? Have you tried with application.properties ? (that is how the test cases work)

@kjq
Copy link
Author

kjq commented Aug 17, 2021

Hmm, just tried with application.properties (just replaced the yaml with an application.properties) and it did the same thing.

As a side note, using the @ApplicationPath "seemed" to work as expected - both OAS and Swagger reflected the path.

@phillip-kruger
Copy link
Member

Maybe look at the tests I have referenced, and see if you can configure a test so that it replicate your setup ?

@kjq
Copy link
Author

kjq commented Aug 17, 2021

The "should_have_v1" test give me the error using either the yaml/properties file. The "should_not_have_v1" passes.

[ERROR] Failures:
[ERROR] OpenApiTest.should_have_v1_in_path:19 1 expectation failed.
JSON path paths doesn't match.
Expected: map containing ["/v1/ping/me"->ANYTHING]
Actual: <{/ping/me={get={responses={200={description=OK, content={application/json={schema={type=boolean}}}}}}}}>

@QuarkusTest
public class OpenApiTest {
        private static final String OPEN_API_PATH = "/q/openapi";

        @Test
        public void should_have_v1_in_path() {
                RestAssured.given().queryParam("format", "JSON").when().get(OPEN_API_PATH).then()
                                .header("Content-Type", "application/json;charset=UTF-8")
                                .body("openapi", Matchers.startsWith("3.0"))
                                .body("info.title", Matchers.equalTo("Generated API"))
                                .body("paths", Matchers.hasKey("/v1/ping/me"));
        }

        @Test
        public void should_not_have_v1_in_path() {
                RestAssured.given().queryParam("format", "JSON").when().get(OPEN_API_PATH).then()
                                .header("Content-Type", "application/json;charset=UTF-8")
                                .body("openapi", Matchers.startsWith("3.0"))
                                .body("info.title", Matchers.equalTo("Generated API"))
                                .body("paths", Matchers.hasKey("/ping/me"));
        }
}

@ebullient
Copy link
Member

Yes, should be quarkus.rest.path

This may also help:

https://quarkus.io/blog/path-resolution-in-quarkus/

Note that as of 2.x, the redirect behavior has been removed, but the rest is accurate.

@kjq
Copy link
Author

kjq commented Aug 17, 2021 via email

@phillip-kruger
Copy link
Member

@kjq thanks for the test cases, looking at this now.

@phillip-kruger
Copy link
Member

phillip-kruger commented Aug 17, 2021

Yes, should be quarkus.rest.path

@ebullient - Are you sure about this ? The test case use quarkus.resteasy.path
And the guide also still shows quarkus.resteasy.path (see https://quarkus.io/guides/all-config#quarkus-resteasy-server-common_quarkus.resteasy.path) default to /

Maybe you are thinking of quarkus.http.root-path ? (https://quarkus.io/guides/all-config#quarkus-vertx-http_quarkus.http.root-path)

@ebullient - After a quick look it seems like the quarkus.resteasy.path does not follow the new rules ? See

@phillip-kruger
Copy link
Member

@kjq ok @ebullient confirmed that the quarkus.resteasy.path is relative to rootpath so the tests are correct. I'll dive a bit deeper today to see if I can recreate your issue.

@phillip-kruger
Copy link
Member

phillip-kruger commented Aug 18, 2021

@kjq @ebullient I can confirm this is a bug. See attached your original reproducer, that is now working for normal requests (as you described) but the value in the openapi document is wrong. I'll prepare a PR to fix a.s.a.p.

B.t.w - If you use resteasy reactive, you need to use quarkus.resteasy-reactive.path and not quarkus.resteasy.path
openapi.zip

@ebullient
Copy link
Member

Yes, should be quarkus.rest.path

@ebullient - Are you sure about this ? The test case use quarkus.resteasy.path
And the guide also still shows quarkus.resteasy.path (see https://quarkus.io/guides/all-config#quarkus-resteasy-server-common_quarkus.resteasy.path) default to /

When we worked on the path support, quarkus.rest.path was used. That has since been deprecated (I didn't notice), and replaced with quarkus.resteasy.path and quarkus.resteasy-reactive.path.. so you're correct. I was remembering old things.

@phillip-kruger
Copy link
Member

Ok cool ! Thanks for confirming :)

@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Aug 19, 2021
@gsmet gsmet modified the milestones: 2.3 - main, 2.2.0.Final Aug 23, 2021
@magori
Copy link

magori commented Feb 8, 2022

The documentation https://quarkus.io/guides/resteasy-reactive only talks about this property "quarkus.rest.path" and does not define that it is deprecated.

@ebullient
Copy link
Member

Can you open a new issue?

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

Successfully merging a pull request may close this issue.

5 participants