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

Make AnnotationsUtils.findAnnotation() recognize annotations on anonymous classes again #28896

Conversation

janeisklar
Copy link

@janeisklar janeisklar commented Jul 31, 2022

Overview

In Spring 5, AnnotationUtils.findAnnotation(Class<?>, Class<A>) would find the annotation on the following class: Object foo = new @MyAnnotation Object() {}; -- for example, AnnotationUtils.findAnnotation(foo, MyAnnotation.class) would not return null.

With Spring 6.0.0-M5 / Java 17 the annotation will no longer be found.

See gh-28895

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 31, 2022
@snicoll snicoll added this to the Triage Queue milestone Aug 1, 2022
@rstoyanchev rstoyanchev removed this from the Triage Queue milestone Jan 20, 2023
@sbrannen sbrannen self-assigned this Jan 24, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Jan 26, 2023
@sbrannen sbrannen added the type: regression A bug that is also a regression label Feb 9, 2023
@sbrannen sbrannen added this to the 6.0.13 milestone Oct 4, 2023
@sbrannen sbrannen removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 4, 2023
@sbrannen sbrannen changed the title Make AnnotationsUtils::findAnnotation recognize annotations on anonymous classes again Find annotations on anonymous classes again in AnnotationsUtils.findAnnotation() Oct 4, 2023
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Oct 4, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 4, 2023

Hi @janeisklar,

I ran the following example application against Spring Framework 6.0.x, 5.3.x, 5.2.x, 5.1.x, and 5.0.0, and it fails every time.

For Spring 6.0.x, I ran against JDK 17.0.8+9-LTS-211.

For the Spring 5.x variants, I ran against JDK 1.8.0_362-b09.

package example;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.util.Assert;

public class TestMain {

	public static void main(String[] args) {
		Object object = new @TypeUseAnnotation Object() {};
		Class<?> clazz = object.getClass();
		TypeUseAnnotation annotation = AnnotationUtils.findAnnotation(clazz, TypeUseAnnotation.class);
		Assert.state(annotation != null, "annotation is null");
	}

	@Target(ElementType.TYPE_USE)
	@Retention(RetentionPolicy.RUNTIME)
	@interface TypeUseAnnotation {
	}

}

Are you certain that your tests passed on Spring Framework 5.x and JDK 8?

If so, which exact versions of Spring and the JDK did you test against?

In the interim, I am closing this PR in favor of #31360.

Unless I'm mistaken, Spring Framework has never provided explicit/intentional support for finding TYPE_USE annotations.

Though, if you can provide a working example to prove otherwise, please do.

Thanks,

Sam

@sbrannen sbrannen added type: enhancement A general enhancement and removed type: regression A bug that is also a regression labels Oct 4, 2023
@sbrannen sbrannen changed the title Find annotations on anonymous classes again in AnnotationsUtils.findAnnotation() Add support for finding TYPE_USE annotations in the MergedAnnotations infrastructure Oct 4, 2023
@sbrannen sbrannen modified the milestones: 6.0.13, 6.1.0-RC2 Oct 4, 2023
@sbrannen sbrannen closed this Oct 4, 2023
@sbrannen sbrannen added the status: superseded An issue that has been superseded by another label Oct 4, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 4, 2023

@sbrannen sbrannen changed the title Add support for finding TYPE_USE annotations in the MergedAnnotations infrastructure Make AnnotationsUtils.findAnnotation() recognize annotations on anonymous classes again Oct 4, 2023
@sbrannen sbrannen removed this from the 6.1.0-RC2 milestone Oct 4, 2023
sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Oct 4, 2023
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Oct 4, 2023
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Oct 4, 2023
@janeisklar
Copy link
Author

janeisklar commented Oct 4, 2023

Hi @sbrannen,

I did not notice that before, but it seems to be important that you add Element.TYPE in addition to Element.TYPE_USE to your annotation.

The following test passes on my system:

$ java -version
openjdk version "1.8.0_382"
OpenJDK Runtime Environment (build 1.8.0_382-8u382-ga-1~22.04.1-b05)
OpenJDK 64-Bit Server VM (build 25.382-b05, mixed mode)
package com.foo;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.junit.jupiter.api.Test;
import org.springframework.core.annotation.AnnotationUtils;

class ReproducerTest {

	@Test
	void findsAnnotationOnAnnotatedObject() {
		final Object a = new @TypeAnnotation Object() {};
		assertNotNull(AnnotationUtils.findAnnotation(a.getClass(), TypeAnnotation.class));
	}

	@Test
	void doesNotFindAnnotationOnAnnotatedObject() {
		final Object a = new @TypeUseAnnotation Object() {};
		TypeUseAnnotation annotation = AnnotationUtils.findAnnotation(a.getClass(), TypeUseAnnotation.class);
		assertNull(annotation);
	}

	@Target({ ElementType.TYPE, ElementType.TYPE_USE })
	@Retention(RetentionPolicy.RUNTIME)
	@Documented
	@interface TypeAnnotation {}

	@Target(ElementType.TYPE_USE)
	@Retention(RetentionPolicy.RUNTIME)
	@interface TypeUseAnnotation {}
}
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
	<modelVersion>4.0.0</modelVersion>

	<groupId>org.example</groupId>
	<artifactId>type-use-bug-reproducer</artifactId>
	<version>1.0-SNAPSHOT</version>

	<properties>
		<maven.compiler.source>8</maven.compiler.source>
		<maven.compiler.target>8</maven.compiler.target>
		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
	</properties>

	<dependencies>
		<dependency>
			<groupId>org.springframework</groupId>
			<artifactId>spring-core</artifactId>
			<version>5.3.13</version>
		</dependency>
		<dependency>
			<groupId>org.junit.jupiter</groupId>
			<artifactId>junit-jupiter</artifactId>
			<version>5.7.2</version>
			<scope>test</scope>
		</dependency>
	</dependencies>

</project>

Thank you for looking into this!

Best,
André

@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed type: enhancement A general enhancement labels Oct 8, 2023
@sbrannen
Copy link
Member

sbrannen commented Oct 8, 2023

Thanks for the feedback, @janeisklar.

I did not notice that before, but it seems to be important that you add Element.TYPE in addition to Element.TYPE_USE to your annotation.

That was the missing piece of the puzzle.

After further investigation, I've determined that your use case was never intended to work.

Spring uses Class#getDeclaredAnnotations to find annotations in its search algorithms, and that method should never have returned TYPE_USE annotations. Instead, TYPE_USE annotations should only be returned via the AnnotatedType APIs introduced in Java 8 alongside support for TYPE_USE annotations.

  • java.lang.Class.getAnnotatedSuperclass()
  • java.lang.Class.getAnnotatedInterfaces()

There were apparently changes made in Java 12 that fixed the bug that your code relied on (though, I was not able to locate the specific issue for that change).

Consequently, your test passes on Java 8-11 but fails with Java 12+.

Furthermore, all of the traditional methods that Spring has been using for years (like Class#getDeclaredAnnotations) have the following "disclaimer" in their Javadoc since Java 15.

Note that any annotations returned by this method are declaration annotations.

Similarly, methods such as java.lang.reflect.AnnotatedType.getDeclaredAnnotations() have the following "disclaimer" in their Javadoc since Java 15.

Note that any annotations returned by this method are type annotations.

In light of that, I have changed the label for this issue to invalid; however, we will consider introducing support for TYPE_USE annotations in #31360.

@janeisklar
Copy link
Author

janeisklar commented Oct 8, 2023

Thank you for the insights, @sbrannen.

In case it helps: I have been using this featurebug to create beans in a @TestConfiguration class that needed to be annotated with some custom annotations.

It's not a feature that one's life depends on, but it can be quite handy.

@sbrannen
Copy link
Member

sbrannen commented Oct 8, 2023

Thank you for the insights, @sbrannen.

You're welcome!

In case it helps: I have been using this featurebug to create beans in a @TestConfiguration class that needed to be annotated with some custom annotations.

Ahhh, OK. Thanks for providing the use case.

FWIW, if that's your goal, you can achieve the same via a "local class" instead of an "anonymous class" as follows.

class AnnotatedLocalClassTests {

	@Test
	void test() {
		@TypeUseAnnotation
		class AnnotatedObject extends Object /* or some bean/component type */ {
		};

		Object object = new AnnotatedObject();
		Class<?> clazz = object.getClass();
		TypeUseAnnotation annotation = AnnotationUtils.findAnnotation(clazz, TypeUseAnnotation.class);
		assertThat(annotation).isNotNull();
	}

	@Target(ElementType.TYPE_USE)
	@Retention(RetentionPolicy.RUNTIME)
	@interface TypeUseAnnotation {
	}

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid status: superseded An issue that has been superseded by another
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants