Name of scanned @Configuration class affects @Import to work or fail

[abenneke]

Our projects rely heavily on bean overriding (yes, we know, disabled now by default for good reasons, but still useful at times and a different story).

We now traced down a phenomenon that the name of a @Configuration class found by the @ComponentScan seem to affect, whether overriding with @Import (or sub-classing) works properly. This is though the ConfigurationClassParser already does a lot of handling on this case, e. g. in processConfigurationClass.

Please find a test case in https://github.com/abenneke/sandbox/tree/master/spring-import-order:
It contains four @Configuration classes: Two for a "foo" bean (FooConfigurationA+B) and two for a "bar" bean (BarConfigurationA+B). However, the @Import order is different between both configurations: FooConfigurationB is importing FooConfigurationA while BarConfigurationA is importing BarConfigurationB.

In the test we now see that overriding works as expected for "foo", but fails for "bar"!?

Just to make sure we did not miss anything, we added the very same test using standard Spring methods and again using Spring Boot - but it does not make any difference here.

[encircled]

Hi,

In such case, BarConfigurationB is actually loaded twice: first by componentScan and then by your manual import. Component scan is name-dependent indeed (name based order).

The problem is that at componentScan point of time it is not aware of additional imports, causing different bean overriding than expected (ConfigurationClassParser#doProcessConfigurationClass:294).

I think it is not possible to tune such behavior (without big refactoring), you should probably redesign your configuration

[abenneke]

Thank you for looking into this!

I perfectly agree, that the configuration class is loaded twice. But this is normal and happens all the time: All @Configuration classes (also) imported somewhere are found by the componentScan and are processed a second time when the @Import is processed. Just the order of these processings may vary.

And for that case “a configuration class is found by the componentScan first and later imported by another configuration class (here FooConfigurationA is imported by FooConfigurationB)” this is already handled in ConfigurationClassParser:

First, FooConfigurationA is processed from componentScan and stored in this.configurationClasses as “not imported”. Then, while processing FooConfigurationB, FooConfigurationA is processed again, but this time as “imported”, the existingClass in processConfigurationClass gets the “not imported” information from the first processing and we end up at the return in line 236. This effectively ignores the second encounter of FooConfigurationA.

I might be wrong, but to me it looks like the order of the entries in this.configurationClasses is important here. What we see from this case is, that classes imported by others appear before the classes which are importing them (FooConfigurationA before FooConfigurationB in this case).

Now the other case:

While processing BarConfigurationA the @Import of BarConfigurationB is also already processed and both classes end up in this.configurationClasses, again the imported class before the class importing it (BarConfigurationB before BarConfigurationA here). So far, so fine.

However now BarConfigurationB is processed again, this time from the componentScan and “not imported”. The existingClass in processConfigurationClass gets the “imported” information from the first processing and we end up in line 241 of processConfigurationClass and are removing BarConfigurationB from this.configurationClasses. BarConfigurationB is then processed again and re-added later.

And here we have the problem IMHO: With this removal and re-adding the order in this.configurationClasses is destroyed. However, to me this order seems to be important for overriding to work properly.

I understand the comment here, that “explicit bean definition should replace an import” is intended, however this BarConfigurationB is not an “explicit bean definition” in this case, but a “scanned” one. It should therefore not replace the imported one, continue with this already processed import and the second processing should also be ignored.

To solve this, I think adding

if (configClassBeanDefinition instanceof ScannedGenericBeanDefinition) {
   // ignore scanned configuration classes already processed
   return;
}

before this removal (in line 240) should be enough, while configClassBeanDefinition is the definition belonging to the configClass. To do so, the bdCand available in line 303 of ConfigurationClassParser would have to be passed here as configClassBeanDefinition.

This does not sound like a major change to me?

[encircled]

Hi,

Thanks for the detailed response :) Let me double check it

[encircled]

The behavior is inconsistent indeed, I will prepare the PR soon

[encircled]

As I mentioned, it is not that simple unfortunately and I'm afraid of possible side effects. Anyway, lets try to fix it

[abenneke]

What side effects do you have in mind?