Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document that SimpleCacheManager requires caches to be initialized when not used as a bean #22988

Closed
akokskis opened this issue May 17, 2019 · 2 comments
Assignees
Labels
type: documentation A documentation task
Milestone

Comments

@akokskis
Copy link

When creating a org.springframework.cache.support.SimpleCacheManager and explicitly adding caches to it, the configured object does not maintain the contract of the org.springframework.cache.CacheManager interface.

Specifically, getCacheNames() and getCache(String name) do not behave as one would expect, returning an empty collection and null respectively.

This issue is reproducible with version 5.1.7.RELEASE.

This issue is demonstrated with the following test:

package com.ak;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import java.util.Collection;
import java.util.Collections;

import org.junit.Test;
import org.springframework.cache.Cache;
import org.springframework.cache.concurrent.ConcurrentMapCache;
import org.springframework.cache.support.SimpleCacheManager;

public class TestSimpleCacheManager {

    private static final String CACHE_NAME = "this is a cache";
    
    @Test
    public void testGetCacheNames() {
        SimpleCacheManager simpleCacheManager = setUpSimpleCacheManager();
        
        Collection<String> cacheNames = simpleCacheManager.getCacheNames();
        assertEquals(1, cacheNames.size());
        assertTrue(cacheNames.contains(CACHE_NAME));
    }
    
    @Test
    public void testGetCache() {
        SimpleCacheManager simpleCacheManager = setUpSimpleCacheManager();
        
        Cache cache = simpleCacheManager.getCache(CACHE_NAME);
        
        assertNotNull(cache);
    }

    private SimpleCacheManager setUpSimpleCacheManager() {
        Cache cache = new ConcurrentMapCache(CACHE_NAME);
        
        SimpleCacheManager simpleCacheManager = new SimpleCacheManager();
        simpleCacheManager.setCaches(Collections.singleton(cache));
        return simpleCacheManager;
    }
}

using the following pom file:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.ak</groupId>
  <artifactId>ak-testing</artifactId>
  <version>0.0.1-SNAPSHOT</version>
  <name>ak-testing</name>
  <dependencies>
      <dependency>
          <groupId>org.springframework</groupId>
          <artifactId>spring-context</artifactId>
          <version>5.1.7.RELEASE</version>
      </dependency>
      <dependency>
          <groupId>junit</groupId>
          <artifactId>junit</artifactId>
          <version>4.11</version>
          <scope>test</scope>
      </dependency>
  </dependencies>
</project>

I'd be happy to take a crack at creating a patch for this issue, should this be something that is desired to be fixed.

Thanks

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 17, 2019
@jhoeller jhoeller self-assigned this May 18, 2019
@snicoll
Copy link
Member

snicoll commented May 19, 2019

The contract of SimpleCacheManager (and more particularly AbstractCacheManager) was designed so to be created as a bean and there is an afterProperties() callback that is invoked automatically when you are doing so. If you add that in setUpSimpleCacheManager before returning the cache manager both your tests pass.

I agree it is a bit confusing when using the API directly (especially since there are methods to mutate the cache manager once it is initialized).

@akokskis
Copy link
Author

Aha, that makes a lot more sense - thanks.

If no change is required or desired, which makes sense to me given your explanation, then I would wonder whether or not it's worth making a note of this in the documentation. I don't think I feel too strongly about this one way or another, but something to consider.

Thanks!

@snicoll snicoll added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 7, 2020
@snicoll snicoll added this to the 5.2.9 milestone Aug 7, 2020
@snicoll snicoll self-assigned this Aug 7, 2020
@snicoll snicoll changed the title SimpleCacheManager does not maintain CacheManager contract Document that SimpleCacheManager requires caches to be initialized when not used as a bean Aug 7, 2020
@snicoll snicoll closed this as completed in 6acbc50 Aug 7, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
@jhoeller jhoeller removed their assignment Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants