[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework #9397

gitchenjh · 2021-12-13T06:38:23Z

What is the purpose of the change

Brief changelog

fix DubboConfigEarlyInitializationPostProcessor counted as BeanPostProcess twice in PostProcessorRegistrationDelegate.registerBeanPostProcessors()

Verifying this change

before

DubboConfigEarlyInitializationPostProcessor in beanFactory.getBeanPostProcessors() and postProcessorNames
The beanProcessorTargetCount is 17
After registration the beanFactory.getBeanPostProcessors() should be 16

after

No more dubboConfigEarlyInitializationPostProcessor in postProcessorNames
The beanProcessorTargetCount is 16,
After registration the beanFactory.getBeanPostProcessors() should be 16

Checklist

Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Add some description to dubbo-website project if you are requesting to add a feature.
GitHub Actions works fine on your own branch.
If this contribution is large, please follow the Software Donation Guide.

…eanPostProcess twice in PostProcessorRegistrationDelegate.registerBeanPostProcessors()(#9370)

…rocessBeanFactory()

kylixs · 2021-12-14T12:10:30Z

...ache/dubbo/config/spring/beans/factory/config/DubboConfigEarlyRegistrationPostProcessor.java

+        if (beanFactory != null) {
+            DubboConfigEarlyInitializationPostProcessor dceiPostProcessor =
+                    DubboConfigEarlyInitializationPostProcessor.getSingleton(beanFactory);
+            // Register itself


// Register itself

change this comment

kylixs · 2021-12-14T12:10:46Z

...ache/dubbo/config/spring/beans/factory/config/DubboConfigEarlyRegistrationPostProcessor.java

+                    DubboConfigEarlyInitializationPostProcessor.getSingleton(beanFactory);
+            // Register itself
+            if (logger.isInfoEnabled()) {
+                logger.info("BeanFactory is about to be initialized, trying to resolve the Dubbo Config Beans early " +


change this log message

kylixs · 2021-12-14T12:13:29Z

...he/dubbo/config/spring/beans/factory/config/DubboConfigEarlyInitializationPostProcessor.java

-        if (beanFactory == null) { // try again if postProcessBeanDefinitionRegistry method does not effect.
-            this.beanFactory = unwrap(beanFactory);
-            initBeanFactory();
+    public static DubboConfigEarlyInitializationPostProcessor getSingleton(DefaultListableBeanFactory beanFactory) {


The global instance of DubboConfigEarlyInitializationPostProcessor holds in DubboConfigEarlyRegistrationPostProcessor is better.
DubboConfigEarlyInitializationPostProcessor just a BeanPostProcessor , it should not care for how to create and be registered.

I think the DubboConfigEarlyInitializationPostProcessor can be changed to a inner class of DubboConfigEarlyRegistrationPostProcessor, it's managed by DubboConfigEarlyRegistrationPostProcessor. Then the relationship is more clear and reduce confusion.

kylixs · 2021-12-14T12:15:41Z

The root cause of the incorrect beanProcessorCount is that DubboConfigEarlyInitializationPostProcessor has been registered twice, once by manual registration and once by Spring automatic registration. A more elegant solution is to register only once, and there is no problem with registering in BeanDefinitionRegistryPostProcessor. It is best to explain its role more accurately in the javadoc.

…Processor

codecov-commenter · 2021-12-15T03:15:44Z

Codecov Report

Merging #9397 (586e65b) into master (7cb6bff) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##             master    #9397      +/-   ##
============================================
- Coverage     60.85%   60.85%   -0.01%     
+ Complexity      448      447       -1     
============================================
  Files          1100     1100              
  Lines         44507    44509       +2     
  Branches       6473     6475       +2     
============================================
  Hits          27086    27086              
+ Misses        14454    14452       -2     
- Partials       2967     2971       +4

Impacted Files	Coverage Δ
...fig/DubboConfigEarlyRegistrationPostProcessor.java	`50.00% <50.00%> (ø)`
...pache/dubbo/config/spring/util/DubboBeanUtils.java	`54.54% <50.00%> (ø)`
...in/java/org/apache/dubbo/common/utils/JVMUtil.java	`81.13% <0.00%> (-11.33%)`	⬇️
...g/apache/dubbo/remoting/p2p/support/FileGroup.java	`41.50% <0.00%> (-9.44%)`	⬇️
...rg/apache/dubbo/rpc/protocol/dubbo/DubboCodec.java	`70.53% <0.00%> (-3.58%)`	⬇️
...port/identifier/BaseServiceMetadataIdentifier.java	`57.14% <0.00%> (-3.58%)`	⬇️
...mmon/serialize/kryo/utils/AbstractKryoFactory.java	`75.43% <0.00%> (-3.51%)`	⬇️
...he/dubbo/remoting/transport/netty/NettyServer.java	`70.17% <0.00%> (-3.51%)`	⬇️
...e/dubbo/remoting/transport/netty/NettyChannel.java	`55.68% <0.00%> (-3.41%)`	⬇️
...ava/org/apache/dubbo/config/ServiceConfigBase.java	`57.83% <0.00%> (-1.21%)`	⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cb6bff...586e65b. Read the comment docs.

…tionPostProcessor

chickenlj

LGTM.

kylixs · 2021-12-15T06:35:44Z

...ache/dubbo/config/spring/beans/factory/config/DubboConfigEarlyRegistrationPostProcessor.java

+        private boolean hasRegisteredCommonAnnotationBeanPostProcessor() {
+            for (BeanPostProcessor beanPostProcessor : beanFactory.getBeanPostProcessors()) {
+                if (CommonAnnotationBeanPostProcessor.class.equals(beanPostProcessor.getClass())) {
+                    return true;
+                }
+            }
+            return false;
+        }


It's better to cache the result if has found CommonAnnotationBeanPostProcessor

It's better to cache the result if has found CommonAnnotationBeanPostProcessor

Thanks for your reply. I can't cache the result all the time. Here is my reason:
There might be some BeanPostProcessors not instantiated before some Dubbo Config Bean instantiated (which will call this method)，so the result of beanFactory.getBeanPostProcessors() may not be the same when this method is called.
But, it can be cached after CommonAnnotationBeanPostProcessor instantiated, I will submit another PR.

new PR #9414

…ssor (#9397) (#9414)

[master] Fix DubboConfigEarlyInitializationPostProcessor counted as B…

55e002b

…eanPostProcess twice in PostProcessorRegistrationDelegate.registerBeanPostProcessors()(#9370)

gitchenjh mentioned this pull request Dec 13, 2021

[master] Fix DubboConfigEarlyInitializationPostProcessor problem #9371

Closed

8 tasks

gitchenjh added 2 commits December 13, 2021 16:54

[master] Fix logic of DubboConfigEarlyRegistrationPostProcessor.postP…

e21918e

…rocessBeanFactory()

[master] Add unit-test for DubboConfigEarlyInitializationPostProcessor

8568bad

kylixs reviewed Dec 14, 2021

View reviewed changes

gitchenjh added 2 commits December 14, 2021 21:48

[master] Reorganize code about DubboConfigEarlyRegistrationPostProcessor

d2ed671

[master] Add javadoc explanation for DubboConfigEarlyRegistrationPost…

586e65b

…Processor

gitchenjh changed the title ~~[master] Fix DubboConfigEarlyInitializationPostProcessor problem~~ [master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring-Framework Dec 15, 2021

gitchenjh changed the title ~~[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring-Framework~~ [master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework Dec 15, 2021

[master] Modify some javadoc explanation for DubboConfigEarlyRegistra…

d3c83ba

…tionPostProcessor

chickenlj approved these changes Dec 15, 2021

View reviewed changes

chickenlj added this to the 2.7.15 milestone Dec 15, 2021

chickenlj merged commit 9cfb1ae into apache:master Dec 15, 2021

kylixs reviewed Dec 15, 2021

View reviewed changes

chickenlj mentioned this pull request Dec 15, 2021

DubboConfigEarlyInitializationPostProcessor counted as BeanPostProcessor twice. Cause print log 'is not eligible for getting processed by all BeanPostProcessors' during most spring bean Initialization #9370

Closed

gitchenjh mentioned this pull request Dec 15, 2021

Optimize some code for DubboConfigEarlyRegistrationPostProcessor (#9397) #9414

Merged

8 tasks

chickenlj pushed a commit that referenced this pull request Dec 16, 2021

[master] Optimize some code for DubboConfigEarlyRegistrationPostProce…

1296ff8

…ssor (#9397) (#9414)

CrazyHZM mentioned this pull request Dec 17, 2021

DubboConfigEarlyInitializationPostProcessor cause PostProcessorRegistrationDelegate$BeanPostProcessorChecker log 'not eligible for getting processed by all BeanPostProcessors' #8347

Closed

2 tasks

owen200008 mentioned this pull request Feb 17, 2022

fix_early_postprocessor_bug #7752

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework #9397

[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework #9397

gitchenjh commented Dec 13, 2021 •

edited

Loading

kylixs Dec 14, 2021

kylixs Dec 14, 2021

kylixs Dec 14, 2021

kylixs Dec 14, 2021 •

edited

Loading

kylixs commented Dec 14, 2021

codecov-commenter commented Dec 15, 2021 •

edited

Loading

chickenlj left a comment

kylixs Dec 15, 2021

gitchenjh Dec 15, 2021

gitchenjh Dec 15, 2021

[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework #9397

[master] Fix DubboConfigEarlyInitializationPostProcessor registered twice in Spring Framework #9397

Conversation

gitchenjh commented Dec 13, 2021 • edited Loading

What is the purpose of the change

Brief changelog

Verifying this change

before

after

Checklist

kylixs Dec 14, 2021

Choose a reason for hiding this comment

kylixs Dec 14, 2021

Choose a reason for hiding this comment

kylixs Dec 14, 2021

Choose a reason for hiding this comment

kylixs Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

kylixs commented Dec 14, 2021

codecov-commenter commented Dec 15, 2021 • edited Loading

Codecov Report

chickenlj left a comment

Choose a reason for hiding this comment

kylixs Dec 15, 2021

Choose a reason for hiding this comment

gitchenjh Dec 15, 2021

Choose a reason for hiding this comment

gitchenjh Dec 15, 2021

Choose a reason for hiding this comment

gitchenjh commented Dec 13, 2021 •

edited

Loading

kylixs Dec 14, 2021 •

edited

Loading

codecov-commenter commented Dec 15, 2021 •

edited

Loading