Seems like a bug...

[chrisstockton]

I'm doing an update on a cached entry using an EntryProcessor. The first time that I call cache.invoke(), the non-existing value is inserted into the cache and a second call to cache.invoke() does not trigger a loader.load().

Once the cached entry is expired however, any subsequent calls to cache.invoke() always trigger a call to loader.load().

Here's a test case:

  @Test
  public void itLoadsTwice() throws InterruptedException {
    CachingProvider provider = Caching.getCachingProvider(CaffeineCachingProvider.class.getName());
    CacheManager manager = provider.getCacheManager(provider.getDefaultURI(), provider.getDefaultClassLoader());
    Cache<String, Integer> cache = manager.getCache("test-cache");
    cache.invoke("a", new EntryProcessorImpl());
    assertEquals(1, CacheLoaderImpl.loads);

    cache.invoke("a", new EntryProcessorImpl());
    assertEquals(1, CacheLoaderImpl.loads);

    // Expire the entry
    Thread.sleep(5000);

    // expect only 1 more load here, but there were 2
    cache.invoke("a", new EntryProcessorImpl());
    assertEquals(2, CacheLoaderImpl.loads);

    // don't expect a load here
    cache.invoke("a", new EntryProcessorImpl());
    // fails, CacheLoaderImpl.loads == 3 here
    assertEquals(2, CacheLoaderImpl.loads);
  }

  static class EntryProcessorImpl implements EntryProcessor<String, Integer, Object> {
    @Override
    public Object process(MutableEntry<String, Integer> entry, Object... arguments) throws EntryProcessorException {
      Integer value = entry.getValue();
      if (value == null) {
        value = 0;
      }
      entry.setValue(++value);

      return null;
    }
  }

  static Map<String, Integer> map = new HashMap<>();

  public static class CacheWriterImpl implements CacheWriter<String, Integer> {
    static int writes = 0;

    @Override
    public void write(Entry<? extends String, ? extends Integer> entry) throws CacheWriterException {
      writes++;
      map.put(entry.getKey(), entry.getValue());
    }

    @Override
    public void writeAll(Collection<Entry<? extends String, ? extends Integer>> entries) throws CacheWriterException {
      entries.forEach(this::write);
    }

    @Override
    public void delete(Object key) throws CacheWriterException {
      map.remove(key);
    }

    @Override
    public void deleteAll(Collection<?> keys) throws CacheWriterException {
      keys.forEach(this::delete);
    }
  }

  public static class CacheLoaderImpl implements CacheLoader<String, Integer> {
    static int loads = 0;

    @Override
    public Integer load(String key) throws CacheLoaderException {
      loads++;
      return map.get(key);
    }

    @Override
    public Map<String, Integer> loadAll(Iterable<? extends String> keys) throws CacheLoaderException {
      Map<String, Integer> result = new HashMap<>();
      keys.forEach(k -> {
        Integer v = map.get(k);
        if (v != null) {
          result.put(k, v);
        }
      });

      return result;
    }
  }

application.conf:

  test-cache {
  	read-through {
  	  enabled = true
  	  loader = "test.EntryProcessorTest$CacheLoaderImpl"
  	}
  	
  	write-through {
  	  enabled = true
  	  writer = "test.EntryProcessorTest$CacheWriterImpl"
  	}
  
    policy {
      lazy-expiration {
        creation = 5s
      }
      
      maximum {
        size = 200
      }
    }
  }

Should that 4th call to cache.invoke() be loading from the loader?

[ben-manes]

Sorry that I won't get a chance to dig into this immediately. Thanks a lot for the test case and I will try to prioritize it. I am trying to take a look tonight, but I can see how this week is going.

[ben-manes]

I think the reason is because you only redefined lazy-expiration.creation. The reference.conf (perhaps incorrectly) defaults creation, update, and access to eternal. That means the next read or update marks the entry as not expirable. I think this should have been defaulted to null to indicate no change.

The test was failing at the 3rd invoke and now fails on the 4th. Need to debug that next.

[ben-manes]

The second issue seems to be due to EntryProcessorEntry#setValue. The entry action is LOADED, but the if condition incorrectly resets it to UPDATED. If I update to check both CREATE and LOADED actions, it then passes your test case. It also continues to pass the TCK, so that change seems good.

[ben-manes]

Please try master (use snapshot repository). I ported your unit test and it seems to pass.

I don't think that I can change the reference.conf defaults in a patch release, but maybe in a minor? So I'll hold off on that fix until the next bump.

[chrisstockton]

Okay, I'll add the configs in my application.conf and test out with the snapshot. Thanks for the quick work!

[ben-manes]

Let me know how it goes. I can give you a release when I get the thumbs up.

[chrisstockton]

Seems to be working perfectly. Thank you!

[ben-manes]

Released