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

Support for readers and filters to access the Jandex index #1475

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

MikeEdgar
Copy link
Member

Allows implementations of OASModelReader and OASFilter to optionally provide a single-argument constructor accepting a Jandex IndexView to be used internally.

Relates to #1255 and #1370

@MikeEdgar
Copy link
Member Author

@Raptoer, this PR is for a concept that would allow you to use the Jandex index from an instance of an OASFilter in order to extract the Spring security information you mentioned on #1255. You could associate the Operations in the OpenAPI via operation Id or with some other custom @Extension of your choosing. Wdyt?

@Spindl, this is also meant to assist with your issue #1370. Using an OASFilter provided with an IndexView that runs following the JAX-RS annotation scanner should allow you fill in or replace the pieces in the OpenAPI model. Please share your thoughts if that could work.

For both issues, I would say implementing an AnnotationScanner is somewhat discouraged since it's meant to be an internal component and could change in ways that break external implementations, even in non-major releases of the scanner.

@MikeEdgar MikeEdgar force-pushed the issue-1255 branch 2 times, most recently from 4a5078e to cb92d1a Compare May 18, 2023 12:33
@sonarcloud
Copy link

sonarcloud bot commented May 18, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

65.5% 65.5% Coverage
0.0% 0.0% Duplication

@Spindl
Copy link

Spindl commented May 19, 2023

Hi @MikeEdgar,

thank you for considering helping us with our problem.
I fear that having the index and the operation does not do the trick for us.
We need the MethodInfo object of the method the operation refers to, e.g. because we look for additional annotations or thrown exceptions to extend the openapi documentation.

I understand that the Scanners are considered internal API, and would also prefer not to hook into the scanning process like that 😅
But if there is no way to get to the MethodInfo object from the operation I fear we have no other options.

@Spindl
Copy link

Spindl commented May 19, 2023

Being able to filter the active scanners would allow us to get rid of another ugly workaround though, which we need for now since we can't influence the order in which the scanners run. 😬

@MikeEdgar
Copy link
Member Author

We need the MethodInfo object of the method the operation refers to, e.g. because we look for additional annotations or thrown exceptions to extend the openapi documentation.

@Spindl, having the index would allow that. Suppose you have an endpoint method:

package com.example;

@Path("/widgets")
public class WidgetsResource {

    @GET
    @Path("{widgetId}")
    @Operation(operationId = "getWidget")
    @com.example.MyAnnotation(value = "Something Important")
    public Response getWidgets(String widgetId) {
       ...
    }

}

You could do something like in the filter:

public class MyFilter implements OASFilter {

    private IndexView index;
    
    public MyFilter(IndexView index) {
        this.index = index;
    }

    @Override
    public Operation filterOperation(Opertion operation) {
        if ("getWidget".equals(operation.getOperationId()) {
            MethodInfo getWidgets = index
                .getClassByName("com.example.WidgetsResource")
                .method("getWidget", Type.create(
                    DotName.createSimple("java.lang.String"), 
                    Type.Kind.CLASS));
            AnnotationInstance myAnnotation = getWidgets.annotation("com.example.MyAnnotation");
            // Do something with `myAnnotation` and the `operation`
        }
    }
}

@Spindl
Copy link

Spindl commented May 23, 2023

@MikeEdgar, I fear that would not work for us.

To do that, we would have to know the association between the operation ID and the fully qualified name of the class that contains it.
That is not so easy in our case though, since the plugin configuration and extension are defined in a base project, which is then used by other projects that define additional resources to be documented.
So we have no chance the know them all beforehand.

I guess we could maybe try to "put the cart before the horse" and guess the class the method belongs to from the operation ID (which is {tag}_{methodName} in our case) and all classes having the @Tag annotation or something like that. But that does not really seem any better than what we have now.

And I just checked, and we would have the same problem with the Schema as well, since we also adapt some things there (e.g. we have custom @ReadOnly annotations, and document those accordingly if they are present). So we would need a way to get to the ClassInfo from the Schema object as well, and I don't see how we could do that either 😅

@MikeEdgar
Copy link
Member Author

I guess we could maybe try to "put the cart before the horse" and guess the class the method belongs to from the operation ID (which is {tag}_{methodName} in our case) and all classes having the @Tag annotation or something like that. But that does not really seem any better than what we have now.

@Spindl , there are other options to access the MethodInfo that might work for you. For example, given an Operation model in the filter, you can find the matching @Operation annotation (assuming you have them to set the values to {tag}_{methodName}) like this:

index.getAnnotations(org.eclipse.microprofile.openapi.annotations.Operation.class)
    .stream()
    .filter(annotation -> operation.getOperationId().equals(annotation.value("operationId").asString()))
    .map(AnnotationInstance::target)
    .map(AnnotationTarget::asMethod)
    .findFirst()
    .ifPresent(methodInfo -> {
        // Do something with the methodInfo
    });

Something similar may work for schemas if there is a way to navigate back from the annotations you are interested in.

@Spindl
Copy link

Spindl commented Jun 13, 2023

Hi @MikeEdgar
sorry for the late reply, I did get sidetracked a little bit at work 😅

I assume we could do something like that for the operations, even though we still couldn't get rid of our custom scanner since we don't annotate our endpoints with the @operation annotation, but rather also calculate the operation_id in the scanner.

And for the Schema I see no way this could work.
We don't explicitly annotate any of our model classes either, and without the fully qualified class name, it would be difficult to find the correct ClassInfo object for a Schema.

@ChMThiel
Copy link

I have same problems like @Spindl : I want to add BeanValiation-Infos annotated at classes/methods/fields to OpenApi. so i need the relation between an OpenApi-Schema and the class/method/field.

@ChMThiel
Copy link

any progress on this? Only the low test-coverage prevents the merge 🤔

@MikeEdgar
Copy link
Member Author

any progress on this? Only the low test-coverage prevents the merge thinking

If this is useful to you as-is, I'm happy to fix up this PR and merge it. However, adding in some relation between the OpenAPI model pieces and the source that generated them will require something more involved.

@ChMThiel
Copy link

I hope it will suffice for me, to get access to the jandex.
We have a standard name-schema in our classes so i hope to get the releation between OpenApi <-> annotated field/method etc.

Nevertheless: my goal is to document custom BeanValidations in OpenApi.
The best solution for this would be to have a kind of 'OpenApiFilter for BeanValidations': a way to filter/enrich the results of io.smallrye.openapi.runtime.scanner.dataobject.BeanValidationScanner

@MikeEdgar MikeEdgar marked this pull request as ready for review July 27, 2023 19:33
@MikeEdgar MikeEdgar requested a review from a team as a code owner July 27, 2023 19:33
@MikeEdgar MikeEdgar added this to the 3.4.1 milestone Jul 27, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jul 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

92.3% 92.3% Coverage
0.0% 0.0% Duplication

@MikeEdgar MikeEdgar requested a review from phillip-kruger July 27, 2023 21:06
Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @MikeEdgar

@phillip-kruger phillip-kruger merged commit c07993b into smallrye:main Jul 27, 2023
@MikeEdgar MikeEdgar deleted the issue-1255 branch July 27, 2023 23:46
@ChMThiel
Copy link

@MikeEdgar
Hello Mike, i was wondering why you provide only an EMPTY_INDEX in OpenApiDocumentService? (see commit).
So the index is available during build-time, but during runtime it's always empty.

@phillip-kruger
Copy link
Member

The Jandex index is not available at runtime, the openapi schema document created from the index in build time is available.

@MikeEdgar
Copy link
Member Author

Right - that is obviously specific to Quarkus, so if you need to do something at run time that you could not have done at build time, your reader/filter will need to fall back to some other mechanism. I suppose you could also build your own Jandex index at build time with your app classes and load it yourself in a filter.

@ChMThiel
Copy link

My problem is that my openApiFilter is triggered twice: once during build time and once during runtime.
Even after setting the config quarkus.smallrye-openapi.store-schema-directory. Each time it's given a fresh input OpenApi (second run seems not to get the results of the first run). And the second time (runtime) creates the resulting OpenApi.

My hope was to find a configuration so that quarkus creates and filters OpenApi during build-time and stores the result in the jar. So during runtime this openApi could be used without any further filtering/processing.

@MikeEdgar
Copy link
Member Author

Each time it's given a fresh input OpenApi (second run seems not to get the results of the first run). And the second time (runtime) creates the resulting OpenApi.

That doesn't seem correct. I would expect the runtime input (loaded in OpenApiDocumentService) to have the output from the build time OpenAPI model. Are you certain of that?

@ChMThiel
Copy link

ChMThiel commented Sep 22, 2023

I created a small test quarkus-project (v.3.4.1) with an openApiFilter using Jandex:
code-with-quarkus.zip

In the filter i add a tag to the openApi with a timestamp and info if the Jandex was available:

public class OpenApiFilter implements OASFilter {

    IndexView index;

    public OpenApiFilter(IndexView aIndex) {
        index = aIndex;
    }

    @Override
    public void filterOpenAPI(OpenAPI aOpenAPI) {
        boolean jandexAvailable = index != null && !index.getKnownClasses().isEmpty();
        System.out.println("org.acme.OpenApiFilter.filterOpenAPI() Jandex available " + jandexAvailable);
        if (aOpenAPI.getTags() == null) {
            System.out.println("org.acme.OpenApiFilter.filterOpenAPI() tags are null");
        } else {
            System.out.println("org.acme.OpenApiFilter.filterOpenAPI() existing tags:");
            aOpenAPI.getTags().stream().map(Tag::getName).forEach(System.out::println);
        }
        Tag myTag = OASFactory.createTag().name("MyTag " + LocalTime.now() + " Jandex available " + jandexAvailable);
        System.out.println("org.acme.OpenApiFilter.filterOpenAPI() adding tag " + myTag.getName());
        aOpenAPI.addTag(myTag);
    }
}

I see that the filter is triggered during build and during runtime. In both cases
org.acme.OpenApiFilter.filterOpenAPI() tags are null
is printed to the console. So the tag that was added to OpenApi during build is not passed to the runtime filter.

I tried several configurations, like quarkus.smallrye-openapi.store-schema-directory=target/classes/META-INF/openapi. That leads to openapi-files (with added tag and jandex available) in the jar under META-INF/openapi, but it is not used during runtime...

@MikeEdgar
Copy link
Member Author

The output of the build should be placed in META-INF/quarkus-generated-openapi-doc.JSON. That would be in target/quarkus-app/quarkus/generated-bytecode.jar. How does the OpenAPI look there?

@ChMThiel
Copy link

The openApi unter META-INF/quarkus-generated-openapi-doc.JSON has no tags at all. I seems it does not contain the result of my filter.

@MikeEdgar
Copy link
Member Author

Ok, I see. We only execute the app's filter on the version written to the store-schema-directory [1]. The copy placed in META-INF was not run with the filter. @phillip-kruger do you see any reason we can't just move the processing of the app filter earlier in the process, from storeDocument to loadDocument?

[1] https://github.com/quarkusio/quarkus/blob/1e3fbb99d7e9c3db994ea6ffe32be0f831386054/extensions/smallrye-openapi/deployment/src/main/java/io/quarkus/smallrye/openapi/deployment/SmallRyeOpenApiProcessor.java#L1102-L1104

@MikeEdgar
Copy link
Member Author

@ChMThiel , another option may be to use an OASModelReader instead. That would run at build time and you would construct an OpenAPI with just the pieces you want. It would later be merged with the model from annotations.

@phillip-kruger
Copy link
Member

phillip-kruger commented Sep 23, 2023

The document that is stored and the one we pass to runtime might not be the same. Except if you have a reason for storing it, there should be no need to do so. That feature was a request from users that store and version control the stored schema, so that they can compare between releases. The reason we run the filter at build time in that case is that it usually only runs during build, and not at runtime. For the normal flow of events the user's filters are run at runtime.

We have two options here I think. 1) Allow users to define the stage a user defined filter should run, with the default being runtime as it is now. 2) Add built-in support for what you are trying to do (BeanValidationScanner).

Maybe we should do both ?

@ChMThiel
Copy link

I do not only filter/adjust the OpenApi in my OASFilter, i want to add infos. To get these infos (that are not present in given OpenApi) i had to use some tricks/hacks: During runtime i had access to CDI and was able to request classes implementing certain marker-interface.
Example: I have certain entity envers-audited. I want to add infos about that to the schema-description. In Order to do so i had to add a marker-interface, etc.

Now that the jandex is available, i can replace the CDI-hack with the an elegant Jandex-request (For my audit-example jandex.getAnnotations(Audited.class)). But because jandex is only available during build-time, i need an option to avoid running the filter during runtime again but keep/use the result of the build-time filter instead.

For the special case of adding BeanValidation-infos imho the optimal solution would be do make the BeanValidatioScanner more flexible/generic. But with something like

jandex.getKnownClasses().stream()
             .filter(ClassInfo::isAnnotation)
             .filter(i -> i.hasAnnotation(Constraint.class))

I can get all BeanValidation-annotations in the OASFilter, connect those to the matchinng OpenApi-Schema and add the info by myself.

I would really appreciate if you support both options because both options have it's use cases, but to define the run-stage of the filter is what i need most.

@phillip-kruger
Copy link
Member

Cool, I can look at making the change to allow running user filters during build. That is a Quarkus change I think. /cc @MikeEdgar

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

Successfully merging this pull request may close these issues.

4 participants