Introduce SimpleKeyMappingCache and ContainerKeySimpleCache

api/src/org/labkey/api/ApiModule.java

@@ -37,6 +37,8 @@
 import org.labkey.api.audit.provider.SystemUpgradeAuditProvider;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.cache.BlockingCache;
+import org.labkey.api.cache.ContainerKeySimpleCache;
+import org.labkey.api.cache.SimpleKeyMappingCache;
 import org.labkey.api.collections.ArrayListMap;
 import org.labkey.api.collections.CaseInsensitiveHashMap;
 import org.labkey.api.collections.CaseInsensitiveHashSet;
@@ -495,6 +497,7 @@ public void registerServlets(ServletContext servletCtx)
             SimpleFilter.FilterTestCase.class,
             SimpleFilter.InClauseTestCase.class,
             SimpleFilter.SqlClauseTestCase.class,
+            SimpleKeyMappingCache.TestCase.class,
             SqlExecutingSelector.TestCase.class,
             SqlScanner.TestCase.class,
             StrictBoundedReader.TestCase.class,
@@ -547,6 +550,7 @@ public void registerServlets(ServletContext servletCtx)
             ConnectionUsage.TestCase.class,
             ContainerDisplayColumn.TestCase.class,
             ContainerFilter.TestCase.class,
+            ContainerKeySimpleCache.TestCase.class,
             ContainerManager.TestCase.class,
             ContentSecurityPolicyFilter.TestCase.class,
             DbSchema.DDLMethodsTestCase.class,
@@ -558,7 +562,7 @@ public void registerServlets(ServletContext servletCtx)
             DbScope.PoolStatisticsTestCase.class,
             DbScope.TransactionTestCase.class,
             DbSequenceManager.TestCase.class,
-                DisplayColumn.TestCase.class,
+            DisplayColumn.TestCase.class,
             DomTestCase.class,
             DomainTemplateGroup.TestCase.class,
             Encryption.TestCase.class,

api/src/org/labkey/api/cache/BlockingCache.java

@@ -205,7 +205,6 @@ public void refresh(@NotNull K key, @Nullable Object argument)
         put(key, value);
     }
 
-
     @Override
     public void put(@NotNull K key, final V value)
     {
@@ -214,21 +213,18 @@ public void put(@NotNull K key, final V value)
         get(key, null, (key1, argument) -> value);
     }
 
-
     @Override
     public void put(@NotNull K key, V value, long timeToLive)
     {
         throw new UnsupportedOperationException("use get(loader)");
     }
 
-
     @Override
     public V get(@NotNull K key)
     {
         return get(key, null);
     }
 
-
     @Override
     public void remove(@NotNull K key)
     {

api/src/org/labkey/api/cache/Cache.java

@@ -22,7 +22,7 @@
 import java.util.Set;
 import java.util.function.Predicate;
 
-public interface Cache<K, V>
+public interface Cache<K, V> extends AutoCloseable
 {
     void put(@NotNull K key, V value);
 
@@ -33,24 +33,27 @@ public interface Cache<K, V>
     /**
      * The wrapped calls to get() and put() are not guaranteed synchronous (see subclass/wrapper impl)
      */
-    V get(@NotNull K key, @Nullable Object arg, CacheLoader<K,V> loader);
+    V get(@NotNull K key, @Nullable Object arg, CacheLoader<K, V> loader);
 
+    // TODO: Return boolean like SimpleCache
     void remove(@NotNull K key);
 
-    /** Removes every element in the cache where filter.accept(K key) evaluates to true.
+    /**
+     * Removes every element in the cache where filter.accept(K key) evaluates to true.
      * Returns the number of elements that were removed.
      */
     int removeUsingFilter(Predicate<K> filter);
 
     record StringPrefixFilter(String prefix) implements Predicate<String>
+    {
+        @Override
+        public boolean test(String s)
         {
-            @Override
-            public boolean test(String s)
-            {
-                return s.startsWith(prefix);
-            }
+            return s.startsWith(prefix);
         }
+    }
 
+    // TODO: Consider switching this to Stream<K>, as in SimpleCache. There are very few callers, and most are tests.
     Set<K> getKeys();
 
     void clear();
@@ -59,6 +62,7 @@ public boolean test(String s)
      * Some CacheProviders (e.g., Ehcache) hold on to the caches they create. close() lets us discard temporary
      * caches when we're done with them (e.g., after a transaction is complete) so we don't leak them.
      */
+    @Override
     void close();
 
     // Get the underlying implementation cache

api/src/org/labkey/api/cache/CacheManager.java

@@ -25,6 +25,7 @@
 import org.labkey.api.data.Project;
 import org.labkey.api.mbean.LabKeyManagement;
 import org.labkey.api.security.User;
+import org.labkey.api.util.GUID;
 import org.labkey.api.util.logging.LogHelper;
 
 import java.lang.ref.Reference;
@@ -42,6 +43,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.function.Function;
 
 public class CacheManager
 {
@@ -71,43 +73,67 @@ public class CacheManager
     /** Marker indicating unlimited entries or unlimited time-to-live (do not expire entries) */
     public static final int UNLIMITED = 0;
 
-    private static <K, V> TrackingCache<K, V> createCache(int limit, long defaultTimeToLive, String debugName)
+    // Key classes whose caches store a cheaper surrogate key. Matched on the exact declared key class.
+    private static final Map<Class<?>, Function<SimpleCache<?, ?>, SimpleCache<?, ?>>> KEY_MAPPINGS = Map.of(
+        Container.class, cache -> new ContainerKeySimpleCache<>((SimpleCache<GUID, ?>) cache)
+    );
+
+    @SuppressWarnings("unchecked")
+    private static <K, V> SimpleCache<K, V> getSimpleCache(@Nullable Class<K> keyClass, int limit, long defaultTimeToLive, String debugName, boolean temporary)
     {
-        CacheWrapper<K, V> cache = new CacheWrapper<>(PROVIDER.getSimpleCache(debugName, limit, defaultTimeToLive, UNLIMITED, false), debugName, null, Thread.currentThread().getStackTrace());
-        addToKnownCaches(cache);  // Permanent cache -- hold onto it
-        LabKeyManagement.register(cache.createDynamicMBean(), debugName, "Cache");
-        return cache;
+        SimpleCache<?, ?> cache = PROVIDER.getSimpleCache(debugName, limit, defaultTimeToLive, UNLIMITED, temporary);
+        Function<SimpleCache<?, ?>, SimpleCache<?, ?>> mapping = null == keyClass ? null : KEY_MAPPINGS.get(keyClass);
+        return (SimpleCache<K, V>) (null == mapping ? cache : mapping.apply(cache));
     }
 
     public static <K, V> TrackingCache<K, V> getCache(int limit, long defaultTimeToLive, String debugName)
     {
-        return createCache(limit, defaultTimeToLive, debugName);
+        return getCache(null, limit, defaultTimeToLive, debugName);
+    }
+
+    /** Returns a cache implementation specialized for keyClass, if one is registered; null keyClass means no specialization */
+    public static <K, V> TrackingCache<K, V> getCache(@Nullable Class<K> keyClass, int limit, long defaultTimeToLive, String debugName)
+    {
+        CacheWrapper<K, V> cache = new CacheWrapper<>(getSimpleCache(keyClass, limit, defaultTimeToLive, debugName, false), debugName, null, Thread.currentThread().getStackTrace());
+        addToKnownCaches(cache);  // Permanent cache -- hold onto it
+        LabKeyManagement.register(cache.createDynamicMBean(), debugName, "Cache");
+        return cache;
     }
 
     public static <V> Cache<String, V> getStringKeyCache(int limit, long defaultTimeToLive, String debugName)
     {
-        return createCache(limit, defaultTimeToLive, debugName);
+        return getCache(String.class, limit, defaultTimeToLive, debugName);
     }
 
     public static <K, V> BlockingCache<K, V> getBlockingCache(int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader<K, V> loader)
     {
-        TrackingCache<K, Wrapper<V>> cache = getCache(limit, defaultTimeToLive, debugName);
+        return getBlockingCache(null, limit, defaultTimeToLive, debugName, loader);
+    }
+
+    public static <K, V> BlockingCache<K, V> getBlockingCache(@Nullable Class<K> keyClass, int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader<K, V> loader)
+    {
+        TrackingCache<K, Wrapper<V>> cache = getCache(keyClass, limit, defaultTimeToLive, debugName);
         return new BlockingCache<>(cache, loader);
     }
 
     public static <V> BlockingCache<String, V> getBlockingStringKeyCache(int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader<String, V> loader)
     {
-        Cache<String, Wrapper<V>> cache = getStringKeyCache(limit, defaultTimeToLive, debugName);
-        return new BlockingCache<>(cache, loader);
+        return getBlockingCache(String.class, limit, defaultTimeToLive, debugName, loader);
     }
 
-    // Temporary caches must be closed when no longer needed. Their statistics can accumulate to another cache's stats.
+    @Deprecated // TODO: Migrate the one remaining caller
     public static <K, V> Cache<K, V> getTemporaryCache(int limit, long defaultTimeToLive, String debugName, @Nullable Stats stats)
     {
-        return new CacheWrapper<>(PROVIDER.getSimpleCache(debugName, limit, defaultTimeToLive, UNLIMITED, true), debugName, stats, null);
+        return getTemporaryCache(null, limit, defaultTimeToLive, debugName, stats);
+    }
+
+    // Temporary caches must be closed when no longer needed. Their statistics can accumulate to another cache's stats.
+    public static <K, V> Cache<K, V> getTemporaryCache(@Nullable Class<K> keyClass, int limit, long defaultTimeToLive, String debugName, @Nullable Stats stats)
+    {
+        return new CacheWrapper<>(getSimpleCache(keyClass, limit, defaultTimeToLive, debugName, true), debugName, stats, null);
     }
 
-    private static final Cache<String, Object> SHARED_CACHE = getStringKeyCache(10000, DEFAULT_TIMEOUT, "Shared");
+    private static final Cache<String, Object> SHARED_CACHE = getCache(String.class, 10000, DEFAULT_TIMEOUT, "Shared");
 
     public static <V> Cache<String, V> getSharedCache()
     {

api/src/org/labkey/api/cache/CacheType.java

@@ -15,11 +15,6 @@
  */
 package org.labkey.api.cache;
 
-/**
-* User: adam
-* Date: 12/26/11
-* Time: 9:31 AM
-*/
 public enum CacheType
 {
     DeterministicLRU,

api/src/org/labkey/api/cache/CacheWrapper.java

@@ -26,6 +26,7 @@
 import javax.management.StandardMBean;
 import java.util.Set;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 // Wraps a SimpleCache to provide a full Cache implementation. Adds null markers, loaders, statistics gathering and debug name.
 class CacheWrapper<K, V> implements TrackingCache<K, V>, CacheMXBean
@@ -37,10 +38,11 @@ class CacheWrapper<K, V> implements TrackingCache<K, V>, CacheMXBean
     private final Stats _stats;
     private final Stats _transactionStats;
     private final @Nullable StackTraceElement[] _stackTrace;
+    @SuppressWarnings("unchecked")
     private final V _nullMarker = (V)NULL_MARKER;
 
-    // Issue 51702. Calculating the size of large caches can be expensive. It's OK to be a little stale or
-    // miss a brief spike by only updating the max size stat every so often
+    // Issue 51702. Calculating the size of large caches can be expensive. It's OK to be a little stale or miss a brief
+    // spike by only updating the max size stat every so often
     private boolean _maxSizeDirty;
     private long _maxSizeNextUpdate;
 
@@ -72,7 +74,6 @@ public void put(@NotNull K key, V value)
         }
     }
 
-
     @Override
     public void put(@NotNull K key, V value, long timeToLive)
     {
@@ -91,14 +92,12 @@ public void put(@NotNull K key, V value, long timeToLive)
         }
     }
 
-
     @Override
     public V get(@NotNull K key)
     {
         return get(key, null, null);
     }
 
-
     @Override
     public V get(@NotNull K key, @Nullable Object arg, @Nullable CacheLoader<K, V> loader)
     {
@@ -125,7 +124,6 @@ else if (null != loader)
         }
     }
 
-
     @Override
     public void remove(@NotNull K key)
     {
@@ -142,18 +140,16 @@ public void remove(@NotNull K key)
         }
     }
 
-
     @Override
     public int removeUsingFilter(Predicate<K> kFilter)
     {
         return trackRemoves(_cache.removeUsingFilter(kFilter));
     }
 
-
     @Override
     public Set<K> getKeys()
     {
-        return _cache.getKeys();
+        return _cache.getKeys().collect(Collectors.toUnmodifiableSet());
     }
 
     @Override
@@ -163,14 +159,12 @@ public void clear()
         trackClear();
     }
 
-
     @Override
     public int getLimit()
     {
         return _cache.getLimit();
     }
 
-
     @Override
     public int size()
     {
@@ -195,14 +189,12 @@ public long getDefaultExpires()
         return _cache.getDefaultExpires();
     }
 
-
     @Override
     public void close()
     {
         _cache.close();
     }
 
-
     @Override
     public String getDebugName()
     {
@@ -309,12 +301,6 @@ private void trackClear()
         _stats.clears.incrementAndGet();
     }
 
-
-    public SimpleCache<K, V> getWrappedCache()
-    {
-        return _cache;
-    }
-
     /* CacheMBean */
 
     @Override

api/src/org/labkey/api/cache/ContainerKeySimpleCache.java

@@ -0,0 +1,56 @@
+package org.labkey.api.cache;
+
+import org.jetbrains.annotations.NotNull;
+import org.junit.Assert;
+import org.junit.Test;
+import org.labkey.api.data.Container;
+import org.labkey.api.data.ContainerManager;
+import org.labkey.api.security.User;
+import org.labkey.api.util.GUID;
+import org.labkey.api.util.JunitUtil;
+import org.labkey.api.util.TestContext;
+
+import java.util.Set;
+
+public class ContainerKeySimpleCache<V> extends SimpleKeyMappingCache<Container, GUID, V>
+{
+    public ContainerKeySimpleCache(@NotNull SimpleCache<GUID, V> delegate)
+    {
+        super(delegate, Container::getEntityId, ContainerManager::getForId);
+    }
+
+    public static class TestCase extends Assert
+    {
+        @Test
+        public void testDeletedContainer()
+        {
+            User user = TestContext.get().getUser();
+            Container parent = JunitUtil.getTestContainer();
+            Container child = ContainerManager.ensureContainer(parent, "ContainerKeySimpleCache", user);
+
+            try (Cache<Container, String> cache = CacheManager.getTemporaryCache(Container.class, 10, CacheManager.UNLIMITED, "ContainerKeySimpleCache test", null))
+            {
+                cache.put(parent, "parent");
+                cache.put(child, "child");
+                assertEquals(Set.of(parent, child), cache.getKeys());
+
+                assertTrue(ContainerManager.delete(child, user));
+
+                // A plain Container-keyed cache would still report the deleted child here
+                assertEquals(Set.of(parent), cache.getKeys());
+                assertEquals(1, cache.removeUsingFilter(_ -> true));
+                assertNull(cache.get(parent));
+
+                // The deleted Container object still has its GUID, so direct access works
+                assertEquals("child", cache.get(child));
+                cache.remove(child);
+                assertEquals(0, cache.getTrackingCache().size());
+            }
+            finally
+            {
+                if (null != ContainerManager.getForId(child.getEntityId()))
+                    ContainerManager.delete(child, user);
+            }
+        }
+    }
+}