AbstractCacheManager.getCache() breaks contract of CacheManager.getCache()

[Shpota]

Affects: Spring Boot version 2.1.3

---

Problem: According to the documentation, CacheManager.getCache(String) must return

the associated cache, or null if none found

However, AbstractCacheManager doesn't implement this contract. Instead, it always returns a not-null value disregards to what was requested. In fact, it creates a new cache every time an unknown cache is requested.

It is misleading and error-prone. I relied on this functionality in my code and ended up with broken business logic.

More details.
Here is the documentation of CacheManager.getCache(String):

Return the cache associated with the given name.
Params: name – the cache identifier (must not be null)
Returns: the associated cache, or null if none found

And here is the implementation of AbstractCacheManager.getCache(String):

@Override
@Nullable
public Cache getCache(String name) {
    Cache cache = this.cacheMap.get(name);
    if (cache != null) {
        return cache;
    }
    else {
        // Fully synchronize now for missing cache creation...
        synchronized (this.cacheMap) {
            cache = this.cacheMap.get(name);
            if (cache == null) {
                cache = getMissingCache(name);
                if (cache != null) {
                    cache = decorateCache(cache);
                    this.cacheMap.put(name, cache);
                    updateCacheNames(name);
                }
            }
            return cache;
        }
    }
}

Potential solutions: Please either update the documentation and make sure it is mentioned that the method could also create a cache or update the implementation, or, if I am missing something, make the documentation transparent.

[sbrannen]

In fact, it creates a new cache every time an unknown cache is requested.

It should only create a new cache if AbstractCacheManager.getMissingCache(String) creates a "missing cache", but that method returns null by default.

What is the concrete subclass of AbstractCacheManager that you are experiencing that behavior with?

[sbrannen]

Please either update the documentation and make sure it is mentioned that the method could also create a cache

I think it's worth updating the documentation to reflect that in any case, since any custom implementation of CacheManager could choose to do that apart from the fact that a subclass of AbstractCacheManager could choose to do that by overriding getMissingCache().

[sbrannen]

@Shpota, I updated the Javadoc in ac29e9d.

Please note that CaffeineCacheManager — which coincidentally does not subclass AbstractCacheManager — supports dynamic cache creation by default. So if you're using Caffeine, that would explain the behavior you witnessed.

[Shpota]

@sbrannen, in my case the implementation is org.springframework.data.redis.cache.RedisCacheManager. I tried, it always returns a non-null object no matter what string I pass.

And regarding the documentation, I still find it unclear. It states:

or null if such a cache does not exist or could be not created

But it is simply not true for RedisCacheManager. It does return a non-null value "if such a cache does not exist".

[sbrannen]

The RedisCacheManager will create a missing cache by default:

https://github.com/spring-projects/spring-data-redis/blob/c7298f6851f86623631fbfc164c07cf1c822cbd2/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java#L240-L242

To disable that feature, you will need to set the allowInFlightCacheCreation flag to false.

And regarding the documentation, I still find it unclear. It states:

or null if such a cache does not exist or could be not created

But it is simply not true for RedisCacheManager. It does return a non-null value "if such a cache does not exist".

The updated documentation does not state that it returns null only "if such a cache does not exist".

Rather, it states that it returns null "if such a cache does not exist or could be not created". The "or" is important there, and it implicitly refers to the fact that the CacheManager may choose to create a new cache on-the-fly, which the RedisCacheManager does by default.