MergedAnnotation swallows IllegalAccessException for attribute method

[mikereiche]

AnnotationBasedPersistentProperty.validateAnnotation NullPointerException on project-specific annotations.

    <groupId>org.springframework.data.build</groupId>
    <artifactId>spring-data-parent</artifactId>
    <version>2.6.0-SNAPSHOT</version>

I hit this when running the tests of spring-data-couchbase project with JDK11 and adding a java-module.java. I assume there is some sort of introspection failure.

mergedAnnotation passed into validateAnnotation and the NPE occurs when candidate is dereferenced.

https://github.com/spring-projects/spring-data-commons/blob/db8431ee41b3b73dd87364de9c6cff158790452e/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java#L125

It's the .orElse((Object)null) that gets returned.

return !AnnotationFilter.PLAIN.matches(annotationType) && !AnnotationsScanner.hasPlainJavaAnnotationsOnly(element) ? (Annotation)getAnnotations(element).get(annotationType, (Predicate)null, MergedAnnotationSelectors.firstDirectlyDeclared()).synthesize(MergedAnnotation::isPresent).orElse((Object)null) : element.getDeclaredAnnotation(annotationType);

[mikereiche]

Down in the bowels of AttributeMethods:

  boolean isValid(Annotation annotation) {
    this.assertAnnotation(annotation);

    for(int i = 0; i < this.size(); ++i) {
      if (this.canThrowTypeNotPresentException(i)) {
        try {
          this.get(i).invoke(annotation);
        } catch (Throwable var4) {
          return false;
        }
      }
    }

    return true;
  }

The exception thrown here is swallowed, but it gives the exact action necessary to avoid it.

class org.springframework.core.annotation.AttributeMethods (in module spring.core) cannot access interface org.springframework.data.couchbase.core.mapping.id.GeneratedValue (in module spring.data.couchbase) because module spring.data.couchbase does not export org.springframework.data.couchbase.core.mapping.id to module spring.core

[mikereiche]

Exception looks like this:

Caused by: java.lang.NullPointerException: null
	at [email protected]/org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.validateAnnotation(AnnotationBasedPersistentProperty.java:164)
	at [email protected]/org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.lambda$populateAnnotationCache$10(AnnotationBasedPersistentProperty.java:144)
	at java.base/java.util.Optional.ifPresent(Optional.java:183)
	at [email protected]/org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.populateAnnotationCache(AnnotationBasedPersistentProperty.java:137)
	at [email protected]/org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.<init>(AnnotationBasedPersistentProperty.java:103)
	at spring.data.couchbase/org.springframework.data.couchbase.core.mapping.BasicCouchbasePersistentProperty.<init>(BasicCouchbasePersistentProperty.java:58)

[sbrannen]

The exception thrown here is swallowed, but it gives the exact action necessary to avoid it.

What happens if you add a suitable --add-opens declaration?

[mikereiche]

Then it works fine. But because the original exception is swallowed and results in an NPE, what should be a trivial diagnosis becomes an extended session in the debugger.

[jhoeller]

The given scenario has effectively been addressed through #29448 already, avoiding reflection for annotation attribute retrieval to begin with. Even non-exported annotation types can be introspected for certain purposes that way, not leading to IllegalAccessExceptions in regular scenarios. I have nevertheless revised IllegalAccessException handling for the fallback reflection code path there.

As a bonus, we also avoid reflection for TypeVariable comparisons now (which typically happen during annotation retrieval on common classes and methods), and we log a clearer-worded message for annotation attribute retrieval failure at info level (e.g. on Google App Engine in case of a late-occurring TypeNotPresentException, logged at the same level as an early-occurring TypeNotPresentException on a regular JVM now).