add gRPC exception advice support for factory beans

[bitbrain]

This pull request addresses #265

When currently using @GRpcServiceAdvice on a bean that is being created within a @Configuration like so (Spring Boot 2.5.6):

 @Bean
 public GrpcExceptionAdvice grpcExceptionAdvice() {
     return new GrpcExceptionAdvice();
 }

it will produce the following exception on startup:

Caused by: java.lang.NullPointerException
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:315)
	at org.lognet.springboot.grpc.autoconfigure.OnMissingErrorHandlerCondition.getMatchOutcome(OnMissingErrorHandlerCondition.java:35)
	at org.springframework.boot.autoconfigure.condition.SpringBootCondition.matches(SpringBootCondition.java:47)
	... 95 more

The reason is that the current logic does not support beans created through factory methods:

grpc-spring-boot-starter/grpc-spring-boot-starter/src/main/java/org/lognet/springboot/grpc/autoconfigure/OnMissingErrorHandlerCondition.java

Line 29 in 19fc751

.getBeanClassName();

The solution

• added a check as suggested on stackoverflow to lookup the correct name from the MethodMetadata.

[codecov]

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.09%. Comparing base (d70346b) to head (860950d).
Report is 102 commits behind head on master.

Files	Patch %	Lines
.../autoconfigure/OnMissingErrorHandlerCondition.java	71.42%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #266      +/-   ##
============================================
- Coverage     88.24%   88.09%   -0.15%     
- Complexity      317      318       +1     
============================================
  Files            50       50              
  Lines          1531     1537       +6     
  Branches         86       87       +1     
============================================
+ Hits           1351     1354       +3     
- Misses          143      144       +1     
- Partials         37       39       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bitbrain]

I will add tests for this.

[jvmlet]

what is the spring boot version you are running with?

[bitbrain]

what is the spring boot version you are running with?

@jvmlet 2.5.6

[bitbrain]

@jvmlet any recommendations on how I can get the code coverage to pass? I checked https://app.codecov.io/gh/LogNet/grpc-spring-boot-starter/compare/266/changes and it seems to have no changes and I also added tests to ensure the bean loading works correctly but it complains about +0.15% coverage decrease.

[bitbrain]

On Apple M1 this project currently does not build due to missing artifacts. This is a recommended workaround. I can remove this again if you want.

[jvmlet]

If the platform can't be inferred dynamically, please add it as configuration option here+ readme

[bitbrain]

This is just a workaround and should not be needed as a property of this Spring boot starter. In case we still want to have it I'd suggest to make it as part of another pull request.

In the meantime, I will revert these changes to the grpc-spring-boot.gradle file.

[jvmlet]

Yes, this is not starter's feature, but gradle plugin's one. You can have this logic in extension class and grpc-spring-boot.gradle in separate PR.

[bitbrain]

I did some digging why that newly added code is not called during the integration tests. Here is my summary. Feedback welcome on how we could change these tests to reproduce this behaviour, as I am currently unable to determine what specifically is causing this behaviour to occur.

Scenario 1: AnnotatedGenericBeanDefinition

This is the current logic within the unit tests of this Spring Boot starter. Setting up the bean like this:

grpc-spring-boot-starter/grpc-spring-boot-starter-demo/src/test/java/org/lognet/springboot/grpc/recovery/GRpcRecoveryTest.java

Line 77 in 860950d

public BeanCustomErrorHandler otherErrorHandler() {

...will internally create an AnnotatedGenericBeanDefinition and set it on the internal beanFactoryMap within the beanFactory. As a result, beanDefinition.getBeanClassName() is not null.

Scenario 2: ConfigurationClassBeanDefinitionReader$ConfigurationClassBeanDefinition

Within our codebase, we use a custom Spring Boot starter to define a custom advice like so:

@Bean
public GrpcExceptionAdvice grpcExceptionAdvice() {
        return new GrpcExceptionAdvice();
 }

which itself looks like this:

@GRpcServiceAdvice
public class GrpcExceptionAdvice {
 /* exception handlers go here*/
}

This is literally the same setup, however, for some reason it is using a different bean definition where beanDefinition.getBeanClassName() returns null and instead the class name can be obtained from the internal MethodMetadata on the AnnotatedBeanDefinition.

Question time!

How can I change the tests within this Spring Boot starter to replicate Scenario 2?

[jvmlet]

See behavior difference if you define the inner advice class as static/non-static