spring-projects/spring-boot

NPE in ConfigurationPropertiesReportEndpoint when only proxied subclass has annotation

Closed this issue · 9 comments

Spring Boot 1.1.9, JDK 1.7.0_60 Windows 7

I have a class using @ConfigurationProperties that extends another class that implements an interface. Both the interface and superclass have no annotations. When ConfigurationPropertiesReportEndpoint.extractPrefix() attempts to find the annotation, it first fails on the proxy instance and then tries interface but not the class itself, resulting in a NullPointerException.

public interface Executor {}

public abstract class AbstractExecutor {}

@Component
@ConfigurationProperties("executor.sql")
public class SqlExecutor extends AbstractExecutor {}
java.lang.NullPointerException: null
    at org.springframework.boot.actuate.endpoint.ConfigurationPropertiesReportEndpoint.extractPrefix(ConfigurationPropertiesReportEndpoint.java:180) ~[spring-boot-actuator-1.1.9.RELEASE.jar:1.1.9.RELEASE]
    at org.springframework.boot.actuate.endpoint.ConfigurationPropertiesReportEndpoint.extract(ConfigurationPropertiesReportEndpoint.java:123) ~[spring-boot-actuator-1.1.9.RELEASE.jar:1.1.9.RELEASE]
    at org.springframework.boot.actuate.endpoint.ConfigurationPropertiesReportEndpoint.invoke(ConfigurationPropertiesReportEndpoint.java:96) ~[spring-boot-actuator-1.1.9.RELEASE.jar:1.1.9.RELEASE]
    at org.springframework.boot.actuate.endpoint.ConfigurationPropertiesReportEndpoint.invoke(ConfigurationPropertiesReportEndpoint.java:63) ~[spring-boot-actuator-1.1.9.RELEASE.jar:1.1.9.RELEASE]
    at org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter.invoke(EndpointMvcAdapter.java:56) ~[spring-boot-actuator-1.1.9.RELEASE.jar:1.1.9.RELEASE]

I tried that sample class and I couldn't reproduce. You should probably have more than the code you pasted in the description.

Yes, here's the rest of the code to force the bean to use a proxy which causes the NullPointerException:

public interface Executor {
    public void execute();
}

public abstract class AbstractExecutor implements Executor {}

@Component
@ConfigurationProperties("executor.sql")
public class SqlExecutor extends AbstractExecutor {
    @Override
    @Transactional(propagation = Propagation.REQUIRES_NEW)
    public void execute() {
    }
}

I still can't reproduce so that must be an initialization order thing. That being said, your SqlExecutor becomes a JDK proxy and only has the methods on the interface (that is the sole execute there). There's no way you can bind to executor.sql.whatever anyway because even if you had a setWhatever on that class it won't be available since the exposed bean is the proxy. Said differently, if we fixed that, you'll get another "problem".

You should enable cglib for such use case (proxyTargetClass=true on the transaction annotation). I wouldn't recommend doing that though. I would move those properties to a dedicated class (say SqlExecutorProperties) and then inject the properties in your SqlExecutor. How does that sound?

I have created a sample Spring Boot application that demonstrates the error in the SampleActuatorApplicationTests.testConfigProps() method. The assertion fails and the NPE stack trace is in the log.

https://github.com/bonhamcm/spring-boot-gh-1927

Thanks for the project. I really expected @ConfigurationProperties handling to fail with a JDK proxy but the binder is able to manage it. It's an inconsistency in the model as ListableBeanFactory#getBeansWithAnnotation can figure out the actual underlying type but AnnotationUtils#findAnnotation cannot do the same check (as it does not know about the BeanDefinition). I guess we should be smarter there.

I still think that what you're doing here is weird and maybe it all works out by accident. You are effectively creating a JDK proxy (so resuming your public interface to Executor and not SqlExecutor). You really should resort to cglib proxy for this. If I add @EnableTransactionManagement(proxyTargetClass = true) to your sample app, it works (except a problem later in the test).

Once I added the @EnableTransactionManagement(proxyTargetClass = true) annotation I could read the configuration properties without error.

Thank you Stéphane!

Will you place a null annotation check in ConfigurationPropertiesReportEndpoint#extractPrefix?

I meant to answer yes to that question but Phil did it already :)

Hopefully the fix should deal with underlying issue and find the annotation correctly now.

Thank you Phil!