Trying to get my simple aged cache to pass all unit tests, keeps failing on one of them

Question

Simple Aged Cache: I have created a simple aged cache and I have got it to pass all unit tests except getExpired. I have tried running the cleanExpiredRecords method after the put, and before, really not sure how to fix this. Any suggestions would be appreciated.

package io.collective;

import java.time.Clock;
import java.util.HashMap;
import java.util.Map;

public class SimpleAgedCache {
    private final Clock clock;
    private final Map<Object, CacheEntry> cacheMap;

    public SimpleAgedCache(Clock clock) {
        this.clock = clock;
        this.cacheMap = new HashMap<>();
    }

    public SimpleAgedCache() {
        this(Clock.system(Clock.systemDefaultZone().getZone()));
    }

    public void put(Object key, Object value, int retentionInMillis) {
        if (key != null && retentionInMillis > 0){
            long expirationTime = clock.millis() + retentionInMillis;
            cacheMap.put(key, new CacheEntry(value, expirationTime));
        }
    }

    public boolean isEmpty() {
        return cacheMap.isEmpty();
    }

    public int size() {
        return cacheMap.size();
    }

    public Object get(Object key) {
        cleanExpiredRecords();
        CacheEntry entry = cacheMap.get(key);
        if (entry != null){
            return entry.value;
        }
        return null;
    }

    private void cleanExpiredRecords(){
        long currentTime = clock.millis();
        cacheMap.entrySet().removeIf(entry -> entry.getValue().isExpired(currentTime));
    }

    private static class CacheEntry{
        Object value;
        long expirationTime;

        CacheEntry(Object value, long expirationTime){
            this.value = value;
            this.expirationTime = expirationTime;
        }

        boolean isExpired(long currentTime){
            return currentTime >= expirationTime;
        }

        boolean isNotExpired(long currentTime){
            return !isExpired(currentTime);
        }
    }
}

The Junit test that it keeps failing is below. I am trying to get my code to pass the unit test. However, it keeps returning 2 when the expected answer is 1. Expected :1 Actual :2

@Test
    public void getExpired() {
        TestClock clock = new TestClock();

        SimpleAgedCache expired = new SimpleAgedCache(clock);
        expired.put("aKey", "aValue", 2000);
        expired.put("anotherKey", "anotherValue", 4000);

        clock.offset(Duration.ofMillis(3000));

        assertEquals(1, expired.size());
        assertEquals("anotherValue", expired.get("anotherKey"));
    }

Answer 1

Your cleanup method is triggered when the client calls get(), but not when it calls size(). In other words, when you call size(), your cache actually holds two key -> value pairs, and the expired record is removed only when you call get().

This should not fail:

@Test
public void getExpired() {
    TestClock clock = new TestClock();

    SimpleAgedCache expired = new SimpleAgedCache(clock);
    expired.put("aKey", "aValue", 2000);
    expired.put("anotherKey", "anotherValue", 4000);

    clock.offset(Duration.ofMillis(3000));

    assertEquals("anotherValue", expired.get("anotherKey"));
    assertEquals(1, expired.size());
}