• Bug
  • Status: Open
  • 2 Major
  • Resolution:
  • ehcache-core,ehcache-jcache
  • cdennis
  • Reporter: ryebrye
  • November 09, 2011
  • 0
  • Watchers: 4
  • November 19, 2012

Description

The closest match for the behavior of storeByValue in the jsr107 0.3 (and 0.4-SNAPSHOT) that I could find in ehcache is to configure the underlying ehcache to use copyOnRead and copyOnWrite. WIth this implementation, almost all of the TCK tests for ehcache-jcache pass fine.

Two of the storeByValue tests fail, and I believe it is because

    @Test
    public void get_Existing_MutateKey() {
        long now = System.currentTimeMillis();
        Date existingKey = new Date(now);
        Date existingValue = new Date(now);
        cache.put(existingKey, existingValue);
        existingKey.setTime(now + 1);
        assertEquals(new Date(now), cache.get(new Date(now)));
    }

The expected return of the above test is the date object - but it returns null. This seems to indicate that the underlying cache might not be storing references to the keys and not storing them by value.

Rewriting the above test to use ehcache’s underlying interface directly (and skipping over the the ehcache jsr107 layer) it looks like this (and fails in the same way)


    @Test
    public void get_Existing_MutateKey_inEhCache_Directly() {
        net.sf.ehcache.CacheManager manager = new CacheManager();
        net.sf.ehcache.config.CacheConfiguration cacheConfiguration = new net.sf.ehcache.config.CacheConfiguration();
        cacheConfiguration.setName("testCache");
        cacheConfiguration.setCopyOnRead(true);
        cacheConfiguration.setCopyOnWrite(true);
        net.sf.ehcache.Ehcache cache = new net.sf.ehcache.Cache(cacheConfiguration);
        manager.addCache(cache);        

        long now = System.currentTimeMillis();
        Date existingKey = new Date(now);
        Date existingValue = new Date(now);
        cache.put(new Element(existingKey, existingValue));
        existingKey.setTime(now + 1);
        assertEquals(new Date(now), cache.get(new Date(now)));
    }

I could work around this at the ehcache-jcache layer by cloning the keys myself to get those tck tests to pass, but it is probably better addressed at the ehcache level directly.

Comments

Alexander Snaps 2011-11-09

As far as my understanding goes, Ehcache only copies the value, not the key. It seems weird to mutate the key, but I guess one could argue this is a use case people might want to support. Currently, you’d have to provide your own copy strategy, so it does also copy the key.

gluck 2011-11-09

This change will be required for JSR107. The idea is to protect the cache from key mutation. Interestingly if you mutate a key in a map it get’s lost for ever.

Ryan Gardner 2011-11-14

I tried using a custom copy strategy - but it turns out that setting the key in the custom copy strategy doesn’t matter. The key that gets used to put the element into the cache is grabbed further up the chain.

Using a custom copy strategy I was, however, able to fix another issue related to specifying a classloader to load the classes out of with jsr107.

I addressed this issue in the adapter layer that sits above the ehcache and when the jsr107 cache is configure to storeByValue I have it handle cloning the key before doing the put into the underlying ehcache.

Fiona OShea 2011-11-15

Do you think we should do something more later?

Chris Dennis 2011-11-15

As Greg notes - we will need to do something for JSR-107 as the spec says that keys should be copied when a cache is store-by-value. The issue of whether the current lack of a copy in Ehcache is a bug there is separate question - but the answer to that question obviously affects how we might fix this for 107.

Tim Eck 2011-11-15

seems like we should be copying keys (in addition to values) when so configured. Is this only “broken” right now for standalone caches?

Chris Dennis 2011-11-15

That seems fine to me - in which case this issue can track the fix in Ehcache, and then once it’s fixed Ryan can back out his fix in ehcache-jcache.