Inconsistent configuration report with bean registration phase checks

[snicoll]

When caching is not enabled on a project, we get the following output:

 RedisCacheConfiguration matched:
      - Cache org.springframework.boot.autoconfigure.cache.RedisCacheConfiguration automatic cache type (CacheCondition)
   NoOpCacheConfiguration matched:
      - Cache org.springframework.boot.autoconfigure.cache.NoOpCacheConfiguration automatic cache type (CacheCondition)
   SimpleCacheConfiguration matched:
      - Cache org.springframework.boot.autoconfigure.cache.SimpleCacheConfiguration automatic cache type (CacheCondition)
  GenericCacheConfiguration matched:
      - Cache org.springframework.boot.autoconfigure.cache.GenericCacheConfiguration automatic cache type (CacheCondition)

Ignoring the fact the Redis one should not be there in the first place (#13508), each of those configuration classes have a CacheManager bean and the configuration report does not have a reference to that in the "did not match" section.

When an auto-configuration does not match, then it's easy because it is in the non matched section. When it does and no bean was contributed, for instance because of a bean registration phase condition at class level that doesn't match, it would be nice to have some sort of indication.

Those are perhaps two separate issue in the end but we should at least make sure that reference to, e.g. SimpleCacheConfiguration#cacheManager, are available in the report.

[snicoll]

The same app has the following for CacheAutoConfiguration

 CacheAutoConfiguration:
      Did not match:
         - @ConditionalOnBean (types: org.springframework.cache.interceptor.CacheAspectSupport; SearchStrategy: all) did not find any beans of type org.springframework.cache.interceptor.CacheAspectSupport (OnBeanCondition)
      Matched:
         - @ConditionalOnClass found required class 'org.springframework.cache.CacheManager'; @ConditionalOnMissingClass did not find unwanted class (OnClassCondition)

The problem is that the @ConditionalOnBean condition is not going to prevent the import selector to kick in (as the former happens at the bean registration phase while the latter at the parse configuration phase).

So CacheAutoConfiguration backs off but still contributes extra configuration classes for no valid purpose to the context.

There are two ways to fix this really:

1. Repeat the condition again on every configuration class that are imported that way. They'll back off the same way with the same message (rather than being flagged enabled and then not producing anything)
2. Have a DeferredImportSelector that only returns the classes to consider based on the fact that @EnableCaching has been set by the user

With that said, I am super confused now. If SimpleCacheConfiguration matches as the report states, I don't understand why a CacheManager is not contributed to the context (since the backoff condition on @EnableCaching doesn't apply there). I can see that the import selector is invoked but the constructor of those configuration classes is never reached.

[snicoll]

so @philwebb helped me track down this. Consider the following scenario:

@Configuration
@ConditionOnBean(Acme.class)
@Import(MySelector.class)
public class MyAutoConfiguration { ... }

The key here is TrackedConditionEvaluator. Imports are going to be evaluated and parsed no matter what because, at the time, we don't know if an Acme bean will be available. Before instantiating MyAutoConfiguration we check if a Acme bean is available. If it is not, then the configuration class is skipped. Down the road, we process all classes that have been imported by MySelector.

TrackedConditionEvaluator will check if the configuration class that has imported this configuration class has been skipped. If that's the case, it will skip the class as well.

So those 4 configurations there have been processed in the "parse configuration" phase but they have been skipped in the "bean registration" phase. Sadly, the configuration report does not reflect that situation.

There does not seem to be a hook point to check this. What we could do is look at the bean registry and check if the configuration class is there. That looks like a bit of a hack unfortunately.

[wilkinsona]

I think #13721 is another variant of this. Adding @EnableRedisHttpSession causes RedisSessionConfiguration to back off, but the condition evaluation report shows the following in the matched section:

   RedisSessionConfiguration matched:
      - @ConditionalOnClass found required classes 'org.springframework.data.redis.core.RedisTemplate', 'org.springframework.session.data.redis.RedisOperationsSessionRepository'; @ConditionalOnMissingClass did not find unwanted class (OnClassCondition)
      - Session Condition did not find property 'spring.session.store-type' (ServletSessionCondition)

The class has the following conditions:

@ConditionalOnClass({ RedisTemplate.class, RedisOperationsSessionRepository.class })
@ConditionalOnMissingBean(SessionRepository.class)
@ConditionalOnBean(RedisConnectionFactory.class)
@Conditional(ServletSessionCondition.class)

Note the absence of any information about the bean conditions, particularly the missing bean condition which causes the configuration to back off.

[snicoll]

It is going challenging to fix this for 1.x I am afraid as it looks like we need a hook point in framework to be able to retrieve that information. The closest we could find is ImportRegistry but it's not really meant for that and it is package private anyway.

Looking a bit more at ConditionEvaluationReport, we have a recordConditionEvaluation that assumes that the source can be a FQN of configuration class. When a condition is registered matching such class, the class is removed from the unconditionalClasses set.

We should probably have a similar method that notifies the report that a particular source was discarded "after all". This would then remove all conditions evaluations matching that source. A remove on the outcomes map might be all that's necessary.

There's not much we can do at this point until SPR-17066 is fixed.

[philwebb]

Moving to 2.0.x is fine I think if we can't easily fix this in 1.5.x

[snicoll]

I think we're talking Backlog now I am afraid.