Prevent duplicate @Import processing and ImportBeanDefinitionRegistrar invocation [SPR-9925]

[spring-projects-issues]

Oliver Drotbohm opened SPR-9925 and commented

When implementing custom @Enable annotations using ImportBeanDefinitionRegistrar instances, the implementation gets called twice. Chris and I could debug this down into ConfigurationClassParser.findAllAnnotationAttributes(…) where the config class is scanned for annotation attributes. It does an explicit check against meta-annotation attributes (explicitly excluding the annotation type while iterating over the local annotations) and against local annotation attributes right after that.

The crucial issue here is that the local attribute metadata inspected already contains the entire attribute hierarchy. Seems like it has already been resolved against meta annotations at that place. The net effect is that the @Import annotation is found twice - first, through the explicit meta-annotation lookup in my @Enable annotation and second during inspecting the local attributes.

We essentially have to decide to drop the explicit meta-annotation lookup as we seem to be able to find all necessary information through the local scanning already. The other option is to let the attributes only contain local attributes in the first place but this probably has wider implications.

---

Affects: 3.1.2, 3.2 M2

Sub-tasks:

• Backport fix for multiple invocations of ImportBeanDefinitionRegistrars [SPR-9939] #14572 Backport fix for multiple invocations of ImportBeanDefinitionRegistrars

Issue Links:

• @Enable registrars invoked with subclass for annotation placed on superclass (3.2.x) [SPR-11251] #15876 @Enable registrars invoked with subclass for annotation placed on superclass (3.2.x)
• ConfigurationClassParser needs to load annotations through source class loader [SPR-10343] #14977 ConfigurationClassParser needs to load annotations through source class loader

[spring-projects-issues]

Chris Beams commented

Ollie and I verified this in a screen sharing session. Scheduling for 3.2 RC1

[spring-projects-issues]

Phil Webb commented

#176

[spring-projects-issues]

Chris Beams commented

This is now fixed against master for 3.2 RC1. I'll hold off on backporting to 3.1.x for now, because (a) it's a benign enough issue as far as we know, mostly causing irritating duplicate log messages depending on log levels; (b) the changes made to fix this could have negative side effects, and we're too close to the 3.1.3 boundary at this point to get further feedback.

Folks are free to request a backport on this in the future, of course.

commit 6d8b37d8bbce8c6e6cb4890291469c80742132f7
Author: Phillip Webb <pwebb@vmware.com>
Commit: Chris Beams <cbeams@vmware.com>

    Prevent duplicate @Import processing
    
    Refactor ConfigurationClassParser to recursively find values from
    all @Import annotations, combining them into a single unique set.
    
    This change prevents ImportBeanDefinitionRegistrars from being
    invoked twice.
    
    Issue: SPR-9925

[spring-projects-issues]

Oliver Drotbohm commented

Thanks guys. I verified the fix against my sample project as well as Spring Data JPA and MongoDB. Invocations work as expected now.

[spring-projects-issues]

Phil Webb commented

Excellent news! Thanks for testing that before RC1.

[spring-projects-issues]

Clancy Kornie commented

I ran into a situation where classes can be scanned twice. It's quite easy to reproduce:

Given an annotation class:

@Import(TestImport.class)
public @interface AnotherImport {
}

And a configuration class which is annotated with the above:

@Configuration
@AnotherImport
public class TestConfiguration {
}

And the imported class:

public class TestImport implements ImportBeanDefinitionRegistrar {
    private static final Logger LOGGER = LoggerFactory.getLogger(TestImport.class);

    @Override
    public void registerBeanDefinitions(@NotNull AnnotationMetadata anImport, @NotNull BeanDefinitionRegistry aRegistry) {
        LOGGER.warn("I am scanned twice!");
    }
}

You can see that the log message is printed twice. I don't believe this is correct behaviour. After debugging I believe the issue occurs in ConfigurationClassParser.collectImports where the annotation is resolved on AnotherImport as a class and on TestConfiguration as a string. Hence it is placed into the imports HashSet twice.

[spring-projects-issues]

Oliver Drotbohm commented

Clancy Kornie Which version are you using? The ticket is resolved against 3.2 RC1, so any recent 3.2.x version should be fine.

[spring-projects-issues]

Clancy Kornie commented

I've bumped to version 3.2.4.RELEASE locally and the issue above is present. I feel like this may be a new issue, but very similar in nature.

[spring-projects-issues]

Oliver Drotbohm commented

Would you ming creating a new ticket for this against 3.2.4 then? If you have one handy, feel free to attach a test case.

[spring-projects-issues]

Clancy Kornie commented

No trouble: https://jira.springsource.org/browse/SPR-10918. I don't have a test case, but the code snippet should be pretty close to a functioning test.