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

methods().that().areAnnotatedWith() does not match methods which have multiple instances of the same annotation type #419

Closed
m-ismar opened this issue Sep 4, 2020 · 4 comments

Comments

@m-ismar
Copy link

m-ismar commented Sep 4, 2020

Hello,

I don't know if this is due to me misconfiguring something, or due to ArchUnit, but I have a case where areAnnotatedWith() is not finding all the instances in a class.

In the following arch test:

    ArchCondition<JavaMethod> haveDescriptionInResponseDefinition =
            new ArchCondition<JavaMethod>(
                    "not have description in @ApiResponse definition") {
                @Override
                public void check(JavaMethod javaMethod, ConditionEvents conditionEvents) {

                    if (Util.readApiResponseDescription(javaMethod).isEmpty()) {
                        String message = String.format(
                                "@ApiResponse %s in method %s must have a description",
                                javaMethod.getAnnotationOfType(ApiResponse.class).responseCode(),
                                javaMethod.getFullName()
                        );
                        conditionEvents.add(SimpleConditionEvent.violated(javaMethod, message));
                    }
                }
            };
   
    @ArchTest
    private final ArchRule no_description_in_api_response = methods().that().
            areAnnotatedWith(ApiResponse.class)
            .should(haveDescriptionInResponseDefinition);

Not all methods get matched:

    //Recognized
    @GET
    @RolesAllowed(ROLE_RESTCLIENT)
    @Path("/{id}")
    @Produces({IMAGE_JPEG, IMAGE_JPG})
    @Operation(description = "Provides downloading images.")
    @ApiResponse(responseCode = "200", description = "The image binary.")
    public File getImage(
            @PathParam("id")
            String imageId, 
           @Context MessageContext context
     ) {...}

    //Not recognized
    @PUT
    @RolesAllowed(ROLE_RESTCLIENT)
    @Path("/{id}")
    @Consumes(MULTIPART_FORM_DATA)
    @Produces(APPLICATION_JSON)
    @Operation(description = "Handles upload of images.")
    @ApiResponse(responseCode = "204", description = "Successfully uploaded image")
    @ApiResponse(responseCode = "401", description = "Missing authorization for putting image")
    @ApiResponse(responseCode = "400", description = "A bad request, e.g., an invalid image ID")
    public Response putImage(
            @PathParam("id")
            @Parameter(description = "The ID (client-generated UUID) of the image to upload.") String imageId,
            MultipartBody body
    ) {...}

I figured that that was because putImage() has multiple @ApiResponse annotations, but I'm not sure why that happens and what can I do to avoid it.

Is this intended ArchUnit behaviour or am I missing something?

Sorry if this question was asked before, but I found nothing on the topic.

@rweisleder
Copy link
Contributor

rweisleder commented Sep 7, 2020

This behavior is due to the fact of how Java handles repeatable annotations.

In your example, @ApiResponse is a repeatable annotation. Let me abbreviate that to

@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ApiResponses.class)
public @interface ApiResponse {
    int status();
}

@Retention(RetentionPolicy.RUNTIME)
public @interface ApiResponses {
    ApiResponse[] value();
}

Also, consider these two example usages:

@ApiResponse(status = 200)
public class FirstAnnotatedClass {
}

@ApiResponse(status = 200)
@ApiResponse(status = 404)
public class SecondAnnotatedClass {
}

If we use Java's Reflection API to get the annotations, we get the following result:

System.out.println(FirstAnnotatedClass.class.getAnnotation(ApiResponse.class)); // prints @org.example.ApiResponse(status=200)
System.out.println(SecondAnnotatedClass.class.getAnnotation(ApiResponse.class)); // prints null

What happened? Somewhere between your code and the reflective call, the repeatable annotation is wrapped into the containing annotation type, but only if the annotation is repeated. (I'm not sure if this happens during compile time or during runtime, but it doesn't matter.)

If we try to read the containing annotation type with Java's Reflection API, we get:

System.out.println(FirstAnnotatedClass.class.getAnnotation(ApiResponses.class)); // prints null
System.out.println(SecondAnnotatedClass.class.getAnnotation(ApiResponses.class)); // @org.example.ApiResponses(value=[@org.example.ApiResponse(status=200), @org.example.ApiResponse(status=404)])

And now compared to the behavior of ArchUnit:

JavaClasses classes = new ClassFileImporter().importClasses(FirstAnnotatedClass.class, SecondAnnotatedClass.class);

System.out.println(classes.get(FirstAnnotatedClass.class).tryGetAnnotationOfType(ApiResponse.class).orNull()); // prints @org.example.ApiResponse(status=200)
System.out.println(classes.get(SecondAnnotatedClass.class).tryGetAnnotationOfType(ApiResponse.class).orNull()); // prints null

System.out.println(classes.get(FirstAnnotatedClass.class).tryGetAnnotationOfType(ApiResponses.class).orNull()); // prints null
System.out.println(classes.get(SecondAnnotatedClass.class).tryGetAnnotationOfType(ApiResponses.class).orNull()); // @org.example.ApiResponses(value=[@org.example.ApiResponse(status=200), @org.example.ApiResponse(status=404)])

Compared to Java's Reflection API, ArchUnit produces the same results. So I would say this is the intended behavior.

My summary is, if you have to deal with repeatable annotations, you have to check both the annotation type and the containing annotation type.

@codecholeric
Copy link
Collaborator

What happened? Somewhere between your code and the reflective call, the repeatable annotation is wrapped into the containing annotation type, but only if the annotation is repeated. (I'm not sure if this happens during compile time or during runtime, but it doesn't matter.)

ArchUnit mirrors the bytecode, so that should answer the question 😉

Other than that, yes, ArchUnit tries to mirror the Reflection API to avoid confusion (and duplicate confusion if the Reflection API is confusing 😂 )

@m-ismar
Copy link
Author

m-ismar commented Sep 8, 2020

Thank you for the quick answer, helps a lot 🙂

@m-ismar m-ismar closed this as completed Sep 8, 2020
hankem added a commit that referenced this issue Dec 9, 2022
Bumps [actions/setup-java](https://github.com/actions/setup-java) from
3.6.0 to 3.8.0.

from [actions/setup-java's releases](https://github.com/actions/setup-java/releases):

> # v3.8.0
> 
> In scope of this release we added logic to pass the token input through on GHES for Microsoft Build of OpenJDK ([actions/setup-java#395](https://github-redirect.dependabot.com/actions/setup-java/pull/395)) and updated [minimatch](https://github-redirect.dependabot.com/actions/setup-java/pull/413) dependency.

Commits

*   [`c3ac5dd`](actions/setup-java@c3ac5dd) Revert "Add support for Oracle JDK ([#401](https://github-redirect.dependabot.com/actions/setup-java/issues/401))" ([#421](https://github-redirect.dependabot.com/actions/setup-java/issues/421))
*   [`dcd29da`](actions/setup-java@dcd29da) Fix typo in README.md ([#419](https://github-redirect.dependabot.com/actions/setup-java/issues/419))
*   [`19eeec5`](actions/setup-java@19eeec5) Update to latest `actions/publish-action` ([#411](https://github-redirect.dependabot.com/actions/setup-java/issues/411))
*   [`bd7e5d2`](actions/setup-java@bd7e5d2) Update minimatch to 3.1.2 ([#413](https://github-redirect.dependabot.com/actions/setup-java/issues/413))
*   [`6cdf39a`](actions/setup-java@6cdf39a) Add support for Oracle JDK ([#401](https://github-redirect.dependabot.com/actions/setup-java/issues/401))
*   [`7db6b45`](actions/setup-java@7db6b45) Eclipse Temurin instead of Adopt OpenJDK ([#398](https://github-redirect.dependabot.com/actions/setup-java/issues/398))
*   [`bf2f02c`](actions/setup-java@bf2f02c) Pass the token input through on GHES for Microsoft Build of OpenJDK ([#395](https://github-redirect.dependabot.com/actions/setup-java/issues/395))
*   See full diff in [compare view](actions/setup-java@v3.6.0...v3.8.0)
@KalCramer
Copy link

I wanted to write up a little workaround that seemed to work for me if anyone else finds themselves in this situation.

Instead of using methods().that().areAnnotatedWith().should(), use methods().should().

And pass in a custom ArchCondition which does something like the following in this case checking OP's ApiResponse.class:

        @Override
        public void check(JavaMethod method, ConditionEvents conditionEvents) {
          Arrays.stream(method.reflect().getAnnotationsByType(ApiResponse.class))
              .forEach(annotation -> doCheckHere(annotation));
        }

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

No branches or pull requests

4 participants