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

Create replacement API for handling merged annotations [SPR-17161] #21697

Closed
spring-issuemaster opened this issue Aug 10, 2018 · 1 comment
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Aug 10, 2018

Phil Webb opened SPR-17161 and commented

The existing AnnotationUtils and AnnotatedElementUtils have grown quite a bit over the years and could do with being revisited now that we have pretty comprehensive support for merged annotations throughout the framework.

Some common issues with the current utils classes include:

  • Quite a broad API (~58 public methods between them) that can make it hard for users to find the right method.
  • Some performance concerns. The classes often appear near the top when profiling applications and a typical application will synthesize quite a few annotations.
  • Difficult to evolve. We have some ideas around using annotation processing at compile time to pre-compute some information. It's quite hard to currently plug this in to the existing code.
  • Complicated meta-data relationships. There's quite a complicated relationship between AnnotatedTypeMetadata and the utils and the meta-data API is different.

A new API could help us to determine which methods are really needed and provide the potential for more intelligent caching or pre-computed data.


Affects: 5.1 RC1

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 10, 2018

Phil Webb commented

Some initial prototype work for this is here: https://github.com/philwebb/spring-framework/tree/annotations

The new API tries to separate the representation of merged annotations from the way that they're discovered. This helps to reduce the surface area of the API since find... vs get... semantic methods are no longer needed. For example, to get only the direct annotations and related meta-annotations you would call MergedAnnotations.from(element, SearchStrategy.DIRECT). For a complete search of all superclasses and interface you would do MergedAnnotations.from(element, SearchStrategy.EXHAUSTIVE). Regardless of the method used to find the MergedAnnotations, the API is identical and contains contains methods to:

  • Determine if an annotation is present
  • Get a single merged annotation
  • Get a stream of a specific annotation type
  • Get a stream of all annotations

The MergedAnnotation interface allows you to inspect information about the annotation and synthesize it if required. The method on the interface are also designed to be used when working with streams. Some typical examples:

// is an annotation present or meta-present
mergedAnnotations.isPresent(ExampleAnnotation.class);

// get the merged "value" attribute of ExampleAnnotation (either direct or meta-present)
mergedAnnotations.get(ExampleAnnotation.class).getString("value");

// get all meta-annotations but no direct annotations
mergedAnnotations.stream().anyMatch(MergedAnnotation::isMetaPresent);

// get all ExampleAnnotation declarations (include any meta-annotations) and print the merged "value" attributes
mergedAnnotations.stream(ExampleAnnotation.class).map(a -> a.getString("value")).forEach(System.out::println);
 
@spring-issuemaster spring-issuemaster added this to the 5.x Backlog milestone Jan 11, 2019
@philwebb philwebb self-assigned this Jan 24, 2019
@sbrannen sbrannen added the in: core label Mar 12, 2019
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that
attempt to provide a uniform way for dealing with merged annotations.

Specifically, the new API provides alternatives for the static methods
in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's
comprehensive annotation attribute `@AliasFor` features. The interfaces
also open the possibility of the same API being exposed from the
`AnnotationMetadata` interface.

Additional utility classes for collecting, filtering and selecting
annotations have also been added.

Typical usage for the new API would be something like:

	MergedAnnotations.from(Example.class)
		.stream(MyAnnotation.class)
		.map(a -> a.getString("value"))
		.forEach(System.out::println);

Closes spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add `createIfAnnotationPresent` method to `AnnotationAttributes` so that
a `MergedAnnotation` instance can be quickly converted to an
`AnnotationAttributes` Map.

The method is specifically designed to work with the `asMap` method
when the new API is not being used directly. For example:

	AnnotationAttributes attributes = mergedAnnotation.asMap(
		AnnotationAttributes::createIfAnnotationPresent);

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that
attempt to provide a uniform way for dealing with merged annotations.

Specifically, the new API provides alternatives for the static methods
in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's
comprehensive annotation attribute `@AliasFor` features. The interfaces
also open the possibility of the same API being exposed from the
`AnnotationMetadata` interface.

Additional utility classes for collecting, filtering and selecting
annotations have also been added.

Typical usage for the new API would be something like:

	MergedAnnotations.from(Example.class)
		.stream(MyAnnotation.class)
		.map(a -> a.getString("value"))
		.forEach(System.out::println);

Closes spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add `createIfAnnotationPresent` method to `AnnotationAttributes` so that
a `MergedAnnotation` instance can be quickly converted to an
`AnnotationAttributes` Map.

The method is specifically designed to work with the `asMap` method
when the new API is not being used directly. For example:

	AnnotationAttributes attributes = mergedAnnotation.asMap(
		AnnotationAttributes::createIfAnnotationPresent);

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that
attempt to provide a uniform way for dealing with merged annotations.

Specifically, the new API provides alternatives for the static methods
in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's
comprehensive annotation attribute `@AliasFor` features. The interfaces
also open the possibility of the same API being exposed from the
`AnnotationMetadata` interface.

Additional utility classes for collecting, filtering and selecting
annotations have also been added.

Typical usage for the new API would be something like:

	MergedAnnotations.from(Example.class)
		.stream(MyAnnotation.class)
		.map(a -> a.getString("value"))
		.forEach(System.out::println);

Closes spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Add `createIfAnnotationPresent` method to `AnnotationAttributes` so that
a `MergedAnnotation` instance can be quickly converted to an
`AnnotationAttributes` Map.

The method is specifically designed to work with the `asMap` method
when the new API is not being used directly. For example:

	AnnotationAttributes attributes = mergedAnnotation.asMap(
		AnnotationAttributes::createIfAnnotationPresent);

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 13, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See spring-projectsgh-21697
snicoll added a commit to snicoll/spring-framework that referenced this issue Mar 13, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See spring-projectsgh-21697
snicoll added a commit to snicoll/spring-framework that referenced this issue Mar 13, 2019
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that
attempt to provide a uniform way for dealing with merged annotations.

Specifically, the new API provides alternatives for the static methods
in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's
comprehensive annotation attribute `@AliasFor` features. The interfaces
also open the possibility of the same API being exposed from the
`AnnotationMetadata` interface.

Additional utility classes for collecting, filtering and selecting
annotations have also been added.

Typical usage for the new API would be something like:

	MergedAnnotations.from(Example.class)
		.stream(MyAnnotation.class)
		.map(a -> a.getString("value"))
		.forEach(System.out::println);

Closes spring-projectsgh-21697
snicoll added a commit to snicoll/spring-framework that referenced this issue Mar 13, 2019
Add `createIfAnnotationPresent` method to `AnnotationAttributes` so that
a `MergedAnnotation` instance can be quickly converted to an
`AnnotationAttributes` Map.

The method is specifically designed to work with the `asMap` method
when the new API is not being used directly. For example:

	AnnotationAttributes attributes = mergedAnnotation.asMap(
		AnnotationAttributes::createIfAnnotationPresent);

See spring-projectsgh-21697
snicoll added a commit to snicoll/spring-framework that referenced this issue Mar 13, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 14, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 14, 2019
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that
attempt to provide a uniform way for dealing with merged annotations.

Specifically, the new API provides alternatives for the static methods
in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's
comprehensive annotation attribute `@AliasFor` features. The interfaces
also open the possibility of the same API being exposed from the
`AnnotationMetadata` interface.

Additional utility classes for collecting, filtering and selecting
annotations have also been added.

Typical usage for the new API would be something like:

	MergedAnnotations.from(Example.class)
		.stream(MyAnnotation.class)
		.map(a -> a.getString("value"))
		.forEach(System.out::println);

Closes spring-projectsgh-21697
philwebb added a commit to philwebb/spring-framework that referenced this issue Mar 14, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See spring-projectsgh-21697
jhoeller added a commit that referenced this issue Mar 23, 2019
Add a new test class to help cover annotation introspection failure
handling. These tests were previously missing and are important to
ensure that annotation util code changes don't introduce regressions.

See gh-21697
@jhoeller jhoeller closed this in 4972d85 Mar 23, 2019
jhoeller added a commit that referenced this issue Mar 23, 2019
Refine the element filtering performed by `AnnotationsScanner` to also
cover `org.springframework.util` and most `com.sun` classes which turn
out to be referenced quite frequently and which we know contain no
useful annotations.

See gh-21697
@jhoeller jhoeller removed this from the 5.x Backlog milestone Mar 23, 2019
@jhoeller jhoeller added this to the 5.2 M1 milestone Mar 23, 2019
sbrannen added a commit that referenced this issue Apr 3, 2019
For consistency within the framework, this commit renames the SUPER_CLASS enum constant
to SUPERCLASS.

See gh-21697
philwebb added a commit that referenced this issue Apr 5, 2019
Add tests for `MissingMergedAnnotation`

See gh-21697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.