HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Using EHCache the right way

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thewayehcacheusingright

Problem

I use EHCache to maintain a mostly read-only cache of results from database queries. It works perfectly for over a dozen queries.

However, there is one table which needs an odd access method that is causing me grief. I hope someone with a knowledgeable eye could see what I am doing wrong, if anything.

Note that these classes are marked static because I have torn them out of live code and put them all into a single Test class - just for you. :)

Here is the Cache object which wraps an ehcache object. It works fine in all case except my special case I will post further down.

// My Cache object which wraps the EHCache object into a simple API.
public static class Cache {

    // The cache.
    private final Ehcache cache;

    private Cache(final Ehcache cache, final String name) {
        // Plain ordinary.
        this.cache = cache;
    }

    public Cache(final String name) {
        this(CacheManager.getInstance().addCacheIfAbsent(name), name);
    }

    public Cache(final String name, CacheEntryFactory factory) {
        // Wrap it with a factory.
        this(new SelfPopulatingCache(CacheManager.getInstance().addCacheIfAbsent(name), factory), name);
    }

    public void put(final K key, final V value) {
        cache.put(new Element(key, value));
    }

    @SuppressWarnings("unchecked")
    public V get(final K key) {
        final Element element = cache.get(key);
        if (element != null) {
            return (V) element.getObjectValue();
        }
        return null;
    }

    public void remove(final K key) {
        cache.remove(key);
    }

    public void clear() {
        cache.removeAll();
    }

    public boolean expired(final K key) {
        boolean expired = true;
        final Element element = cache.get(key);
        if (element != null) {
            expired = cache.isExpired(element);
        }
        return expired;
    }

    public Ehcache getCache() {
        return cache;
    }

}


Note that this is cut down from

Solution

I believe I have finally found the problem.

Spot the stupid mistake:

public boolean expired(final K key) {
    boolean expired = true;
    final Element element = cache.get(key);
    if (element != null) {
        expired = cache.isExpired(element);
    }
    return expired;
}


Should be:

public boolean expired(final K key) {
    boolean expired = true;
    // Do a quiet get so we don't change the last access time.
    final Element element = cache.getQuiet(key);
    if (element != null) {
        expired = cache.isExpired(element);
    }
    return expired;
}


Note the use of cache.getQuiet(key) instead of cache.get(key).

What was happening was that every time I checked to see if the cache had expired I was resetting the access time of the pill - and therefore not expiring the cache.

Code Snippets

public boolean expired(final K key) {
    boolean expired = true;
    final Element element = cache.get(key);
    if (element != null) {
        expired = cache.isExpired(element);
    }
    return expired;
}
public boolean expired(final K key) {
    boolean expired = true;
    // Do a quiet get so we don't change the last access time.
    final Element element = cache.getQuiet(key);
    if (element != null) {
        expired = cache.isExpired(element);
    }
    return expired;
}

Context

StackExchange Code Review Q#60393, answer score: 6

Revisions (0)

No revisions yet.