Spring ImportSelector#getExclusionFilter does not exclude matching candidates with import selector

ImportSelector#getExclusionFilter is a way to let an import selector apply an exclude to classes that are imported. We use this in Spring Boot to filter out classes that are about to be imported and that have conditions we can run statically eagerly and to avoid loading unnecessary classes.

CacheAutoConfiguration has an import selector that loads the various configuration candidates. That's handled primarily by the following code:

https://github.com/spring-projects/spring-framework/blob/df588e030f069dd31997ff747839f1d8d5d8d7c7/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java#L581-L583

The candidates for the import selector are as follows:

0 = "org.springframework.boot.autoconfigure.cache.GenericCacheConfiguration"
1 = "org.springframework.boot.autoconfigure.cache.JCacheCacheConfiguration"
2 = "org.springframework.boot.autoconfigure.cache.EhCacheCacheConfiguration"
3 = "org.springframework.boot.autoconfigure.cache.HazelcastCacheConfiguration"
4 = "org.springframework.boot.autoconfigure.cache.InfinispanCacheConfiguration"
5 = "org.springframework.boot.autoconfigure.cache.CouchbaseCacheConfiguration"
6 = "org.springframework.boot.autoconfigure.cache.RedisCacheConfiguration"
7 = "org.springframework.boot.autoconfigure.cache.CaffeineCacheConfiguration"
8 = "org.springframework.boot.autoconfigure.cache.SimpleCacheConfiguration"
9 = "org.springframework.boot.autoconfigure.cache.NoOpCacheConfiguration"

The exclusion filter is going to match for a number of them. As a result the importSourceClasses collection is the following:

0 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@3426} "org.springframework.boot.autoconfigure.cache.GenericCacheConfiguration"
1 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
2 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
3 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
4 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
5 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
6 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
7 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@2464} "java.lang.Object"
8 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@3427} "org.springframework.boot.autoconfigure.cache.SimpleCacheConfiguration"
9 = {org.springframework.context.annotation.ConfigurationClassParser$SourceClass@3428} "org.springframework.boot.autoconfigure.cache.NoOpCacheConfiguration"

Those 7 "configuration classes" are then later on processed. It leads ultimately to a java.lang.Object bean to be defined.

The reason for this is because of a shortcut when the exclude filter matches:

Perhaps an improvement would be a way to not contribute the SourceClass at all so that it's not processed?

Comment From: injae-kim

        // @return null when exclustionFilter matches className
        @Nullable
    SourceClass asSourceClass(@Nullable String className, Predicate<String> exclusionFilter) throws IOException {
        if (className == null)
            return this.objectSourceClass;
        }
                if (exclusionFilter.test(className) { // 👈👈 should be excluded
            return null
                }
         }

        // usage example
    private Collection<SourceClass> asSourceClasses(String[] classNames, Predicate<String> filter) throws IOException {
        List<SourceClass> annotatedClasses = new ArrayList<>(classNames.length);
        for (String className : classNames) {
                        SourceClass sourceClass = asSourceClass(className, filter);
                        if (sourceClass != null) { // 👈👈 should exclude sourceClass
                            annotatedClasses.add(sourceClass);
                        }
        }
        return annotatedClasses;
    }

(just idea) maybe we can explicitly return null on asSourceClass() when exclusionFilter matches className. and exclude it properly on caller side like above asSourceClasses? 🤔

Comment From: snicoll

@injae-kim I don't know. I think the first step is to reproduce this scenario in a test here and then investigate. Import selector is quite involved already.

Comment From: injae-kim

I think the first step is to reproduce this scenario in a test here and then investigate

aha~ I understood. if you don't start to investigate this issue yet, may I investigate this more?

I'll check this case and share the result to you within next friday~!

Comment From: snicoll

Thanks for asking. Yes, I am interested as I am busy on other issues at the moment.

Comment From: injae-kim

Investigation

https://github.com/spring-projects/spring-framework/commit/aa4e56b251521610999d72fb8c962575bb8bc27 Optimize @configuration class parsing a little

       private final SourceClass objectSourceClass = new SourceClass(Object.class);

    SourceClass asSourceClass(@Nullable Class<?> classType) throws IOException {
        if (classType == null || classType.getName().startsWith("java.lang.annotation")) { // 👈
            return this.objectSourceClass;
        }

(based on commit message) I think at first, we just want to skip java.lang.annotation types which were often processed but would never provide useful results. Also use a single shared immutable SourceClass instance to represent Object.class.

https://github.com/spring-projects/spring-framework/commit/d93303c0089d311f2b014f45f1b345ca7ab9cb1f ImportSelector.getCandidateFilter() for transitive filtering of classes

After that, we introduced ImportSelector#getExclusionFilter().

         SourceClass asSourceClass(@Nullable String className, Predicate<String> filter) throws IOException {
        if (className == null || filter.test(className)) { // 👈
            return this.objectSourceClass;
        }

https://github.com/spring-projects/spring-framework/blob/4f16297e454808ec8227035b793aec26b9abfc4f/spring-context/src/main/java/org/springframework/context/annotation/ImportSelector.java#L73-L75

But unlike above comment, we don't bypass when candidates match exclusionFilter. Instead we just return this.objectSourceClass so "java.lang.Object" bean is defined as you comment on issue.

Proposal

https://github.com/injae-kim/spring-framework/commit/753668b29a78537763825fc9bd9867b56ffb3d93

    @Nullable
    SourceClass asSourceClass(@Nullable Class<?> classType, Predicate<String> filter) throws IOException {
        if (filter.test(classType.getName())) {
            return null;  // 👈
        }

So my proposal is, explicitly return null when exclusionFilter matches and don't process it. (I checked test passed on above commit)

https://github.com/spring-projects/spring-framework/blob/4f16297e454808ec8227035b793aec26b9abfc4f/spring-context/src/test/java/org/springframework/context/annotation/ImportSelectorTests.java#L368-L370

But I'm not sure how to test it 😅 maybe above test can cover this change?

@snicoll PTAL when you have some times, and feel free to share your opinion and how can we test it. thanks!

Comment From: snicoll

@injae-kim thanks for your help. I went with quite a pragmatic approach of filtering them out for import selectors only. But we might have to revisit this part of the code at some point (cc @jhoeller).

Comment From: injae-kim

https://github.com/spring-projects/spring-framework/commit/533149939371ee124ce02746cfade992d34a6e89 oh~ I understood your approach. if you need any help in the future, feel free to mention me ;)