Skip to content

Resolve bean post processor warnings#1361

Merged
OlgaMaciaszek merged 1 commit intospring-cloud:mainfrom
hpoettker:resolve-warnings
Jul 15, 2024
Merged

Resolve bean post processor warnings#1361
OlgaMaciaszek merged 1 commit intospring-cloud:mainfrom
hpoettker:resolve-warnings

Conversation

@hpoettker
Copy link
Copy Markdown
Contributor

@hpoettker hpoettker commented Jul 11, 2024

Introduction

The PR resolves the remaining warnings about beans not being eligible for getting processed by all BeanPostProcessors. Similar warnings have been reported in #1315 and fixed in #1319.

The warnings do not indicate a severe issue. But they lead to confusion among users and bad getting started experiences. And as the size of the PR shows, the required change is rather small.

Reproducing the issue

The effectiveness of the change can be seen in the following tests:

  • LoadBalancerRequestFactoryConfigurationTests
  • ReactorLoadBalancerClientAutoConfigurationTests

Without the change, the test LoadBalancerRequestFactoryConfigurationTests logs the following warnings:

Bean 'org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration$DeferringLoadBalancerInterceptorConfig' of type [org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration$DeferringLoadBalancerInterceptorConfig] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). The currently created BeanPostProcessor [lbRestClientPostProcessor] is declared through a non-static factory method on that class; consider declaring it as static instead.
Bean 'deferringLoadBalancerInterceptor' of type [org.springframework.cloud.client.loadbalancer.DeferringLoadBalancerInterceptor] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [lbRestClientPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

Without the change, the test ReactorLoadBalancerClientAutoConfigurationTests logs the following warnings:

Bean 'loadBalancerBeanPostProcessorAutoConfiguration' of type [org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). The currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor] is declared through a non-static factory method on that class; consider declaring it as static instead.
Bean 'org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration$ReactorDeferringLoadBalancerFilterConfig' of type [org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerBeanPostProcessorAutoConfiguration$ReactorDeferringLoadBalancerFilterConfig] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.
Bean 'reactorDeferringLoadBalancerExchangeFilterFunction' of type [org.springframework.cloud.client.loadbalancer.reactive.DeferringLoadBalancerExchangeFilterFunction] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying). Is this bean getting eagerly injected into a currently created BeanPostProcessor [loadBalancerWebClientBuilderBeanPostProcessor]? Check the corresponding BeanPostProcessor declaration and its dependencies.

With the change, no such warnings appear.

How it works

The PR works by using two mechanisms:

  • By declaring the bean factory methods as static, the respective configuration class no longer needs to be instantiated before the methods are invoked.
  • The bean post processors do not work with the injected DeferringLoadBalancerInterceptor or DeferringLoadBalancerExchangeFilterFunction themselves but only pass on references to builders. Therefore their injection points can be declared to be lazy.

@OlgaMaciaszek OlgaMaciaszek self-assigned this Jul 15, 2024
OlgaMaciaszek
OlgaMaciaszek requested changes Jul 15, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @hpoettker thanks for the PR. Looks good. Please add your full name and surname with the @author tag the javadocs of all the classes you've modified.

@hpoettker
Copy link
Copy Markdown
Contributor Author

hpoettker commented Jul 15, 2024

@OlgaMaciaszek done

@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.5 milestone Jul 15, 2024
OlgaMaciaszek
OlgaMaciaszek approved these changes Jul 15, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@OlgaMaciaszek OlgaMaciaszek merged commit 4e6540b into spring-cloud:main Jul 15, 2024
@hpoettker hpoettker deleted the resolve-warnings branch July 15, 2024 15:09
OlgaMaciaszek added a commit that referenced this pull request Jul 31, 2024
@OlgaMaciaszek
Revert "Resolve bean post processor warnings (#1361)"
246bc32 
This reverts commit 4e6540b.

# Conflicts:
#	spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerBeanPostProcessorAutoConfiguration.java
@OlgaMaciaszek
Copy link
Copy Markdown
Collaborator

OlgaMaciaszek commented Jul 31, 2024

@hpoettker Had to revert these changes as, unfortunately, they were messing up native images: https://ge.spring.io/s/elz5iwbaoj2me . Have tried using an ObjectProvider instead of @Lazy, but that also hasn't fixed the issue.

OlgaMaciaszek added a commit that referenced this pull request Jul 31, 2024
@OlgaMaciaszek
Reapply "Resolve bean post processor warnings (#1361)"
a14d644 
This reverts commit 246bc32.
@hpoettker
Copy link
Copy Markdown
Contributor Author

hpoettker commented Jul 31, 2024

@OlgaMaciaszek Thanks for the notice.

I think we've run into spring-projects/spring-framework#29309.

I could reproduce the problem locally. The ObjectProvider is a great idea and it actually fixes the issue for me. In the linked commit, you applied the change to LoadBalancerBeanPostProcessorAutoConfiguration. But to fix the AOT smoke test, it must be applied to LoadBalancerAutoConfiguration as the exception in the build log is about lbRestClientPostProcessor.

Are you interested in a PR with the ObjectProvider applied to both auto-configurations?

@OlgaMaciaszek
Copy link
Copy Markdown
Collaborator

OlgaMaciaszek commented Jul 31, 2024

@hpoettker - yes, that's right. This should fix it: https://github.com/spring-cloud/spring-cloud-commons/tree/use-object-provider-for-lb-restclient-postprocessor

@hpoettker hpoettker mentioned this pull request Aug 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OlgaMaciaszek OlgaMaciaszek OlgaMaciaszek approved these changes

Assignees

@OlgaMaciaszek OlgaMaciaszek

Labels

task

Projects

No open projects
2023.0.4
Status: Done

Milestone

4.1.5

Development

Successfully merging this pull request may close these issues.

3 participants

@hpoettker @OlgaMaciaszek @spring-cloud-issues