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

Allow Build time OpenAPI Filters #36152

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

phillip-kruger
Copy link
Member

@phillip-kruger phillip-kruger commented Sep 26, 2023

This PR allows users to define Multiple Filters, and allow users to define the filters to run in build time. Users can use an annotation to do this:

/**
 * Filter to add custom elements
 */
@OpenApiFilter(OpenApiFilter.RunStage.BUILD)
public class MyBuildTimeFilter implements OASFilter {

    private IndexView view;

    public MyBuildTimeFilter(IndexView view) {
        this.view = view;
    }

    @Override
    public void filterOpenAPI(OpenAPI openAPI) {
        Collection<ClassInfo> knownClasses = this.view.getKnownClasses();
        Info info = OASFactory.createInfo();
        info.setDescription("Created from Annotated Buildtime filter with " + knownClasses.size() + " known indexed classes");
        openAPI.setInfo(info);
    }

}

Draft for now, waiting for SmallRye release.

/cc @MikeEdgar
/cc @ChMThiel

see this discussion: smallrye/smallrye-open-api#1475

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/openapi area/smallrye labels Sep 26, 2023
@geoand
Copy link
Contributor

geoand commented Sep 26, 2023

Interesting.

What happens if there are only build filters? Is there any runtime overhead associated?

@phillip-kruger
Copy link
Member Author

Shouldn't be, I can double check, maybe need to check for empty list before calling the recorder

@phillip-kruger
Copy link
Member Author

I'll address all issues when I do the final PR, with the release SmallRye lib. Thanks @geoand

@phillip-kruger phillip-kruger force-pushed the openapi-build-filter branch 2 times, most recently from a8e2f7a to ccbac8a Compare September 26, 2023 13:29
@phillip-kruger phillip-kruger marked this pull request as ready for review September 26, 2023 13:29
@quarkus-bot

This comment has been minimized.

Signed-off-by: Phillip Kruger <[email protected]>
@phillip-kruger phillip-kruger added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 26, 2023
@quarkus-bot

This comment has been minimized.

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 27, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@phillip-kruger phillip-kruger merged commit 05ea722 into quarkusio:main Sep 27, 2023
51 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 27, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 27, 2023
@brunobat brunobat mentioned this pull request Sep 27, 2023
@ChMThiel
Copy link

ChMThiel commented Dec 5, 2023

@phillip-kruger
Thank you very much for implementing that feature! Access to jandex and build-time filtering is great and helps me a lot!

But there is one question left:
Since i updated to quarkus 3.6.0 to use this feature, the filter seems to be run several times (4 times in my case).
Do i miss something? Do i have to make my filter idempotent, so that multiple filter-runs will lead to the same result?
Or is there some kind of bug? Is it just coincidence that there are (in my case) 4 OASFilter-implementations to be run (quarkus' AutoBearerTokenSecurityFiler, DisabledRestEndpointsFilter, openApiCore's UnusedSchemaFilter and my own)?

@phillip-kruger
Copy link
Member Author

This sounds like a bug. Do you have a reproducer?

@ChMThiel
Copy link

ChMThiel commented Dec 6, 2023

For now i have the effect in my production code only. I will try to reproduce in in small sample project if i find some time for that...

@ChMThiel
Copy link

@phillip-kruger i created a minimal quarkus project with an OASFilter at stage BUILD:

@OpenApiFilter(OpenApiFilter.RunStage.BUILD)
public class MyApiFilter implements OASFilter {

    IndexView jandex;
    private static final AtomicInteger RUN = new AtomicInteger();

    public MyApiFilter(IndexView aIndex) {
        jandex = aIndex;
    }

    @Override
    public void filterOpenAPI(OpenAPI aOpenAPI) {
        Logger.getLogger("MyApiFilter").info("OpenApiFilter.filterOpenAPI() Attempt no " + RUN.incrementAndGet());
        aOpenAPI.info(OASFactory.createInfo().description("MyApiFilter was visited " + RUN.get() + " times"));
    }
}

It seems the Filter is triggered twice:

2023-12-20 15:32:16,012 INFO  [MyApiFilter] (build-14) OpenApiFilter.filterOpenAPI() Attempt no 1
2023-12-20 15:32:16,074 INFO  [MyApiFilter] (build-14) OpenApiFilter.filterOpenAPI() Attempt no 2

See also the generated OpenApi.yaml under /target/generated-resources/openapi:

---
openapi: 3.0.3
info:
  title: Generated API
  description: MyApiFilter was visited 2 times
  version: 1.0.0-SNAPSHOT
servers:
- url: http://localhost:8080
  description: Auto generated value
- url: http://0.0.0.0:8080
  description: Auto generated value
paths:
  /hello:
    get:
      tags:
      - Greeting Resource
      summary: Hello API
      responses:
        "200":
          description: OK
          content:
            text/plain:
              schema:
                type: string

code-with-quarkus.zip

@phillip-kruger
Copy link
Member Author

phillip-kruger commented Dec 21, 2023

@ChMThiel - Yes you will see that it runs twice. So the first time is when we create a static document, that gets included in the runtime artifact. The static document will again be loaded in runtime by OpenAPI and all runtime filters will also run, and a final document will be created, and that is the one you see in runtime (http://localhost:8080/q/openapi?format=json). In you example you will see that this document description is "MyApiFilter was visited 1 times".

The second one is where we, in build time, try and create a similar document, so we run the runtime filters on build time, as some might work. In fact, before this change when all users filters was runtime, this was to included those (if possible). This document can then be stored (like what you do with the config option) but even if you do not store it, it gets passed along the build chain for other extensions to pick up. Example, extensions that creates code from this document.

I hope this explains it ? If not please let me know. Unfortunately we can not reuse the same document as the one (maybe) has runtime filters added where the other has not. We can maybe check if there are any runtime filters, and if not, we don't do this ?

Can you provide more context on why this (running twice) a problem for you ?

@ChMThiel
Copy link

Can you provide more context on why this (running twice) a problem for you ?

@phillip-kruger Thanks for the quick response. I think i do understand now, why the filters are visited more than once.

The Problem i have is that the stored openapi.yaml during build differs (slightly) from the one you get from the runtime-download if i don't do all my filter-stuff idempotently (which i didn't up to now).
For example:
If i have an Operation

    @GET
    @Produces(MediaType.TEXT_PLAIN)
    @Operation(summary = "Hello API", description = "hello description from schema")
    public String hello() {
        return "Hello RESTEasy";
    }

and my filter:

    @Override
    public Operation filterOperation(Operation aOperation) {
        return aOperation.description(aOperation.getDescription() + " my added description");
    }

my openapi.yaml.file stored in /target/generated-resources/openapi will contain
description: hello description from schema my added description my added description
the dowloaded file (and so swagger) will contain
description: hello description from schema my added description

(In my real project i search for custom beanvalidations and add the findings as description-texts)

We use the build-time openapi.yaml in our pipeline to build a client-library, store it versionized in a repo to have an audit-trail of the API-changes, etc. So this might lead to problems if the file we use here differs from the one you see in swagger or get by download (...even if it's currently only differences in descriptions/summaries).

@ChMThiel
Copy link

ChMThiel commented Dec 21, 2023

Another point:
As far as i can see from SmallRyeOpenApiProcessor.getUserDefinedFilters there is currently no way to specify the order in which the Filters are applied.
So if i have multiple filters, adding something to a description, it's not deterministic, which is done first, second, etc.

What do you think: Should the filter support ordering? Something like @jakarta.annotation.Priority or may be the @OpenApiFilter-annotation itself could have an int priority() default 1 value...

@phillip-kruger
Copy link
Member Author

Yeah, so that is not ideal, I think we can get a better way to do this so that your filter is not run twice. I can look at this. Basically we need to create a document (for inclusion in the runtime), and then use that and only add runtime filters when we store.

I am about to take a break (summer holiday) so I'll only look at this next year. If you are in a hurry, you are welcome to do a PR ?

W.r.t priority, yeah that is something we can add. Having multiple filters is Quarkus specific (the spec only allow one) so from a spec p.o.w a user needs to do everything in one filter. But because we allow many we can look at a way to order them for sure

@ChMThiel
Copy link

. If you are in a hurry, you are welcome to do a PR ?

Thank you very much! It's not urgent...and i will be on holiday myself, too 🎄

@ChMThiel
Copy link

ChMThiel commented Jan 12, 2024

@phillip-kruger As mentioned above, i use the jandex in an OpenApiFilter to recognize BeanValidations and document them. I finally finished my first POC for this (see OpenApiBeanValidationFilter). Maybe you want to have a look.

I just published a complete Quarkus-demo-application, but basically all you need is the OpenApiBeanValidationFilter and some application.config-properties.
It's not a quarkus-extension, so it won't fit into quarkiverse, or? Is there a place to publish OpenApiFilters that might be usefull for somebody?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/openapi area/smallrye
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants