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.smallrye-openapi.open-api-version" in application.yml has no effect #31197

Closed
jehrenzweig-leagueapps opened this issue Feb 15, 2023 · 18 comments
Labels

Comments

@jehrenzweig-leagueapps
Copy link
Contributor

Describe the bug

The following code in a Quarkus project's application.yml file will not change the generated OpenAPI document's openapi: root property value, as the documentation on the "Using OpenAPI and Swagger UI" page indicates it should:

quarkus:
  smallrye-openapi:
    enable: true
    open-api-version: 3.0.1

Expected behavior

Quarkus 2.16.2.Final targets OpenAPI 3.0.3 by default. However, if you attempt to override this value with an earlier supported version (e.g. 3.0.1) in application.yml, the override value should appear in the root openapi: property in the OpenAPI document generated by the URL /q/openapi?format=json.

Example:

{
    "openapi" : "3.0.1",
    "info" : {
      "title" : "Testing OpenAPI Version Setter",
    }
}

Actual behavior

Quarkus' default OpenAPI value (3.0.3) is still visible in the OpenAPI document generated by the URL /q/openapi?format=json; the value defined in application.yml is not applied.

Example:

{
    "openapi" : "3.0.3",
    "info" : {
      "title" : "Testing OpenAPI Version Setter",
    }
}

How to Reproduce?

Steps to reproduce the behavior:

  1. Verify that your Quarkus project's OpenAPI specification targets version 3.0.3 in the /q/openapi?format=json URL route, as described here.
  2. Update your Quarkus project's application.yml code so that the open-api-version property value is set to 3.0.1:
quarkus:
  smallrye-openapi:
    enable: true
    open-api-version: 3.0.1
  1. Verify that your Quarkus project's OpenAPI specification targets version still targets 3.0.3.

Output of uname -a or ver

Darwin LeagueAppss-MacBook-Pro.local 21.6.0 Darwin Kernel Version 21.6.0: Thu Sep 29 20:12:57 PDT 2022; root:xnu-8020.240.7~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "17.0.5" 2022-10-18

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.2.Final

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

Gradle 7.5.1

Additional information

No response

@quarkus-bot
Copy link

quarkus-bot bot commented Feb 15, 2023

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

@phillip-kruger
Copy link
Member

Have you tried mp.openapi.extensions.smallrye.openapi=3.0.1 ?
or the yaml would be something like:

---
mp:
   openapi:
      extensions:
         smallrye:
            openapi: 3.0.1

see https://quarkus.io/guides/openapi-swaggerui#changing-the-openapi-version

@jehrenzweig-leagueapps
Copy link
Contributor Author

jehrenzweig-leagueapps commented Feb 16, 2023

@phillip-kruger Yep, that's my current workaround -- works in both application.properties using mp.openapi.extensions.smallrye.openapi=3.0.1, or in application.yml using the exact code you pasted above:

mp:
   openapi:
      extensions:
         smallrye:
            openapi: 3.0.1

I guess I'm not sure if this is a Quarkus bug, a documentation bug, or simply me misunderstanding the documentation.

  • The "Using OpenAPI and Swagger UI" page's "Changing the OpenAPI Version" section is what pointed me at the mp.openapi.extensions.smallrye.openapi=3.0.1 workaround that I'm using now.

  • The "Autogeneration of Operation Id" section immediately below says that mp.openapi.extensions.smallrye.operationIdStrategy=CLASS_METHOD can be used to override Quarkus' default. The following equivalent YAML code is also capable of overriding that behavior:

    quarkus:
      smallrye-openapi:
        enable: true
          operation-id-strategy: 'class-method'
    

If I can use mp.openapi.extensions.smallrye.operationIdStrategy=CLASS_METHOD in application.properties or the YAML equivalent in application.yml, why can't I use the YAML equivalent of mp.openapi.extensions.smallrye.openapi=3.0.1 in application.yml? Both configuration settings are documented in the same way.

In summary, this code added to application.properties overrides both values correctly:

mp.openapi.extensions.smallrye.openapi=3.0.1
mp.openapi.extensions.smallrye.operationIdStrategy=CLASS_METHOD

However, this code added to application.yml only overrides the operation ID strategy correctly:

quarkus:
  smallrye-openapi:
    open-api-version: 3.0.1
    operation-id-strategy: 'class-method'

This is the code I have to use in application.yml to override both values correctly:

quarkus:
  smallrye-openapi:
    operation-id-strategy: 'class-method'
mp:
  openapi:
    extensions:
      smallrye:
        openapi: 3.0.1

@phillip-kruger
Copy link
Member

It might not have been mapped. Let me check. Hold on

@phillip-kruger
Copy link
Member

Yes it's not mapped. If you keen you can do a PR for that ? It's basically a mapping here: https://github.com/quarkusio/quarkus/blob/main/extensions/smallrye-openapi/runtime/src/main/java/io/quarkus/smallrye/openapi/runtime/OpenApiConfigMapping.java

@jehrenzweig-leagueapps
Copy link
Contributor Author

Sure thing, I'll give it a shot in the AM. This'll be my first time contributing to a public repo, so I might have some more questions tomorrow. Thanks for investigating & pointing me at the relevant code!

@jehrenzweig-leagueapps
Copy link
Contributor Author

jehrenzweig-leagueapps commented Feb 16, 2023

Actually, is the existing code mapping the wrong io.smallrye.openapi.api.constants.OpenApiConstants value? Here is the mapping I'm looking at:

mapKey(relocations, io.smallrye.openapi.api.constants.OpenApiConstants.OPEN_API_VERSION, QUARKUS_OPEN_API_VERSION);

According to the OpenApiConstants documentation, the value of OpenApiConstants.OPEN_API_VERSION is "3.0.3". All of the other mappings in OpenApiConfigMapping.java are mapping keys, not assigning values.

Seems to be the above line should be changed to this:

mapKey(relocations, io.smallrye.openapi.api.constants.OpenApiConstants.VERSION, QUARKUS_OPEN_API_VERSION);

The value of OpenApiConstants.VERSION is "mp.openapi.extensions.smallrye.openapi", which would make it a mapping of two keys, not key-value.

@phillip-kruger
Copy link
Member

Yes ! Good catch ! That should solve it. So it was mapped to a constant and not the config key. Please make a PR with your change !

@jehrenzweig-leagueapps
Copy link
Contributor Author

Will do, tx again!

@jehrenzweig-leagueapps
Copy link
Contributor Author

jehrenzweig-leagueapps commented Feb 16, 2023

@phillip-kruger I have a local branch building with the code fix; now I want to build a test or two, to confirm the fix.

Here are the two existing tests that most closely approximate the tests I want to build:

  • src/test/java/io/quarkus/smallrye/openapi/test/jaxrs/OpenApiWithConfigTestCase.java
  • src/test/java/io/quarkus/smallrye/openapi/test/vertx/OpenApiWithConfigTestCase.java

However, I'm not sure what terminal command I should use. I tried this to run a single test, but it doesn't work:

./mvnw test -f smallrye/openapi/ -Dtest=OpenApiRuntimeFilterTestCase

Btw, if there's a better forum for asking these types of questions, please say the word & I'll repost there instead. This is all a bit new to me.

@phillip-kruger
Copy link
Member

Hi @jehrenzweig-leagueapps , no worries you can ask here. Try this:

(I usually cd into the extension/deployment folder)

./mvnw -Dtest=io.quarkus.smallrye.openapi.test.jaxrs.OpenApiWithConfigTestCase surefire:test

@jehrenzweig-leagueapps
Copy link
Contributor Author

Getting closer. I ran this command:

./mvnw -f extensions/smallrye-openapi/deployment -Dtest=io.quarkus.smallrye.openapi.test.jaxrs.OpenApiWithConfigTestCase surefire:tests

Which returns the following error:

[ERROR] Could not find goal 'tests' in plugin org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M9 among available goals help, test -> [Help 1]

@phillip-kruger
Copy link
Member

it's surefire:test not surefire:tests (note the s).

@phillip-kruger
Copy link
Member

And on Windows you might have to put the test in "" ,so :

./mvnw -f extensions/smallrye-openapi/deployment -Dtest="io.quarkus.smallrye.openapi.test.jaxrs.OpenApiWithConfigTestCase" surefire:test

@jehrenzweig-leagueapps
Copy link
Contributor Author

🤦 Well, at least it was something simple. Got the existing test case running just now -- thank you! Will resume writing my own tests in the AM.

@jehrenzweig-leagueapps
Copy link
Contributor Author

@phillip-kruger I forked the repo & created a draft PR for review here: #31302

Unfortunately, I'm still not able to get my proposed code fix working properly. I tried updating the existing io.quarkus.smallrye.openapi.test.jaxrs.ConfigMappingTest test assertion to verify the fix; your code tries to set the version of OpenAPI that's already in use by default (3.0.3), which is why I think it doesn't catch the existing bug. When I try to update the version being tested to 3.0.2, your test fails, even after I updated the code in OpenApiConfigMapping.

phillip-kruger added a commit that referenced this issue Feb 21, 2023
Updated OpenAPI `mp.openapi.extensions.smallrye.openapi` key mapping
@ygyg70
Copy link
Contributor

ygyg70 commented Feb 12, 2024

I am able to configure the version using mp.openapi.extensions.smallrye.openapi when running dev UI, and by using openApiVersion configuration when using the maven plugin.
Was anyone able to use the application.properties value when running the maven plugin?

@phillip-kruger
Copy link
Member

For the maven plugin you need to use maven properties. Also, I am not sure if all properties is supported in the maven plugin.

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

3 participants