Browse Source

Assert preconditions for MergedAnnotations.from() factory methods

Prior to this commit, if null values were supplied for the
RepeatableContainers or AnnotationFilter arguments to `from()` factory
methods in MergedAnnotations, certain operations would eventually
result in a NullPointerException. This is to be expected; however, the
NullPointerException is often swallowed and only logged at INFO level
with an exception message similar to the following.

> Failed to introspect annotations on org.example.MyClass: NullPointerException

In such cases, the INFO log message is not helpful in diagnosing the
problem. Furthermore, since the exception is swallowed, the desired
operation (e.g., MergedAnnotations.stream(...)) simply returns no
results.

This commit improves the user experience by eagerly asserting non-null
preconditions for required arguments in MergedAnnotations.from()
factory methods.

Closes gh-25568
pull/25798/head
Sam Brannen 5 years ago
parent
commit
e25e690ad4
  1. 5
      spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java
  2. 21
      spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java

5
spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java

@ -25,6 +25,7 @@ import java.util.function.Predicate; @@ -25,6 +25,7 @@ import java.util.function.Predicate;
import java.util.stream.Stream;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
/**
* Provides access to a collection of merged annotations, usually obtained
@ -345,6 +346,8 @@ public interface MergedAnnotations extends Iterable<MergedAnnotation<Annotation> @@ -345,6 +346,8 @@ public interface MergedAnnotations extends Iterable<MergedAnnotation<Annotation>
static MergedAnnotations from(AnnotatedElement element, SearchStrategy searchStrategy,
RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) {
Assert.notNull(repeatableContainers, "RepeatableContainers must not be null");
Assert.notNull(annotationFilter, "AnnotationFilter must not be null");
return TypeMappedAnnotations.from(element, searchStrategy, repeatableContainers, annotationFilter);
}
@ -405,6 +408,8 @@ public interface MergedAnnotations extends Iterable<MergedAnnotation<Annotation> @@ -405,6 +408,8 @@ public interface MergedAnnotations extends Iterable<MergedAnnotation<Annotation>
static MergedAnnotations from(Object source, Annotation[] annotations,
RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) {
Assert.notNull(repeatableContainers, "RepeatableContainers must not be null");
Assert.notNull(annotationFilter, "AnnotationFilter must not be null");
return TypeMappedAnnotations.from(source, annotations, repeatableContainers, annotationFilter);
}

21
spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java

@ -53,6 +53,7 @@ import org.springframework.util.ReflectionUtils; @@ -53,6 +53,7 @@ import org.springframework.util.ReflectionUtils;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.entry;
@ -73,6 +74,26 @@ import static org.assertj.core.api.Assertions.entry; @@ -73,6 +74,26 @@ import static org.assertj.core.api.Assertions.entry;
*/
class MergedAnnotationsTests {
@Test
void fromPreconditions() {
SearchStrategy strategy = SearchStrategy.DIRECT;
RepeatableContainers containers = RepeatableContainers.standardRepeatables();
assertThatIllegalArgumentException()
.isThrownBy(() -> MergedAnnotations.from(getClass(), strategy, null, AnnotationFilter.PLAIN))
.withMessage("RepeatableContainers must not be null");
assertThatIllegalArgumentException()
.isThrownBy(() -> MergedAnnotations.from(getClass(), strategy, containers, null))
.withMessage("AnnotationFilter must not be null");
assertThatIllegalArgumentException()
.isThrownBy(() -> MergedAnnotations.from(getClass(), new Annotation[0], null, AnnotationFilter.PLAIN))
.withMessage("RepeatableContainers must not be null");
assertThatIllegalArgumentException()
.isThrownBy(() -> MergedAnnotations.from(getClass(), new Annotation[0], containers, null))
.withMessage("AnnotationFilter must not be null");
}
@Test
void streamWhenFromNonAnnotatedClass() {
assertThat(MergedAnnotations.from(NonAnnotatedClass.class).

Loading…
Cancel
Save