From ae7efb769afe588c4119544af4b2f1db63f20aab Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 8 Oct 2026 15:12:09 -0700 Subject: [PATCH 1/5] Introduce SimpleKeyMappingCache and ContainerKeySimpleCache --- api/src/org/labkey/api/cache/Cache.java | 1 + .../org/labkey/api/cache/CacheManager.java | 46 +++++- .../org/labkey/api/cache/CacheWrapper.java | 4 +- .../api/cache/ContainerKeySimpleCache.java | 14 ++ .../labkey/api/cache/NoopCacheProvider.java | 10 +- api/src/org/labkey/api/cache/SimpleCache.java | 22 ++- .../api/cache/SimpleKeyMappingCache.java | 140 ++++++++++++++++++ .../api/cache/ehcache/EhSimpleCache.java | 23 +-- .../org/labkey/api/data/DatabaseCache.java | 24 ++- .../api/settings/FolderSettingsCache.java | 2 +- .../api/property/DomainPropertyManager.java | 4 +- 11 files changed, 248 insertions(+), 42 deletions(-) create mode 100644 api/src/org/labkey/api/cache/ContainerKeySimpleCache.java create mode 100644 api/src/org/labkey/api/cache/SimpleKeyMappingCache.java diff --git a/api/src/org/labkey/api/cache/Cache.java b/api/src/org/labkey/api/cache/Cache.java index 6017002ea51..0764479d866 100644 --- a/api/src/org/labkey/api/cache/Cache.java +++ b/api/src/org/labkey/api/cache/Cache.java @@ -51,6 +51,7 @@ public boolean test(String s) } } + // TODO: Consider switching this to Stream, as in SimpleCache. There are very few callers, and most are tests. Set getKeys(); void clear(); diff --git a/api/src/org/labkey/api/cache/CacheManager.java b/api/src/org/labkey/api/cache/CacheManager.java index 7fabda87583..e7655ac675c 100644 --- a/api/src/org/labkey/api/cache/CacheManager.java +++ b/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,9 +73,22 @@ public class CacheManager /** Marker indicating unlimited entries or unlimited time-to-live (do not expire entries) */ public static final int UNLIMITED = 0; - private static TrackingCache 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, Function, SimpleCache>> KEY_MAPPINGS = Map.of( + Container.class, cache -> new ContainerKeySimpleCache<>((SimpleCache) cache) + ); + + @SuppressWarnings("unchecked") + private static SimpleCache getSimpleCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName, boolean temporary) + { + SimpleCache cache = PROVIDER.getSimpleCache(debugName, limit, defaultTimeToLive, UNLIMITED, temporary); + Function, SimpleCache> mapping = null == keyClass ? null : KEY_MAPPINGS.get(keyClass); + return (SimpleCache) (null == mapping ? cache : mapping.apply(cache)); + } + + private static TrackingCache createCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName) { - CacheWrapper cache = new CacheWrapper<>(PROVIDER.getSimpleCache(debugName, limit, defaultTimeToLive, UNLIMITED, false), debugName, null, Thread.currentThread().getStackTrace()); + CacheWrapper 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; @@ -81,30 +96,45 @@ private static TrackingCache createCache(int limit, long defaultTim public static TrackingCache 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 TrackingCache getCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName) + { + return createCache(keyClass, limit, defaultTimeToLive, debugName); } public static Cache getStringKeyCache(int limit, long defaultTimeToLive, String debugName) { - return createCache(limit, defaultTimeToLive, debugName); + return getCache(String.class, limit, defaultTimeToLive, debugName); } public static BlockingCache getBlockingCache(int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader loader) { - TrackingCache> cache = getCache(limit, defaultTimeToLive, debugName); + return getBlockingCache(null, limit, defaultTimeToLive, debugName, loader); + } + + public static BlockingCache getBlockingCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader loader) + { + TrackingCache> cache = getCache(keyClass, limit, defaultTimeToLive, debugName); return new BlockingCache<>(cache, loader); } public static BlockingCache getBlockingStringKeyCache(int limit, long defaultTimeToLive, String debugName, @Nullable CacheLoader loader) { - Cache> 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. public static Cache 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); + } + + public static Cache getTemporaryCache(@Nullable Class 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 SHARED_CACHE = getStringKeyCache(10000, DEFAULT_TIMEOUT, "Shared"); diff --git a/api/src/org/labkey/api/cache/CacheWrapper.java b/api/src/org/labkey/api/cache/CacheWrapper.java index 7d5e83ee60a..7eadafa6092 100644 --- a/api/src/org/labkey/api/cache/CacheWrapper.java +++ b/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 implements TrackingCache, CacheMXBean @@ -37,6 +38,7 @@ class CacheWrapper implements TrackingCache, 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 @@ -153,7 +155,7 @@ public int removeUsingFilter(Predicate kFilter) @Override public Set getKeys() { - return _cache.getKeys(); + return _cache.getKeys().collect(Collectors.toUnmodifiableSet()); } @Override diff --git a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java new file mode 100644 index 00000000000..30f96b1ff33 --- /dev/null +++ b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java @@ -0,0 +1,14 @@ +package org.labkey.api.cache; + +import org.jetbrains.annotations.NotNull; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.util.GUID; + +public class ContainerKeySimpleCache extends SimpleKeyMappingCache +{ + public ContainerKeySimpleCache(@NotNull SimpleCache delegate) + { + super(delegate, Container::getEntityId, ContainerManager::getForId); + } +} diff --git a/api/src/org/labkey/api/cache/NoopCacheProvider.java b/api/src/org/labkey/api/cache/NoopCacheProvider.java index 7ad0478f04b..7bfa3ad9a07 100644 --- a/api/src/org/labkey/api/cache/NoopCacheProvider.java +++ b/api/src/org/labkey/api/cache/NoopCacheProvider.java @@ -17,15 +17,11 @@ import org.jetbrains.annotations.Nullable; -import java.util.Collections; -import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Stream; import static org.labkey.api.cache.CacheType.DeterministicLRU; -/** - * Created by adam on 4/19/2016. - */ public class NoopCacheProvider implements CacheProvider { @Override @@ -70,9 +66,9 @@ public int removeUsingFilter(Predicate filter) } @Override - public Set getKeys() + public Stream getKeys() { - return Collections.emptySet(); + return Stream.empty(); } @Override diff --git a/api/src/org/labkey/api/cache/SimpleCache.java b/api/src/org/labkey/api/cache/SimpleCache.java index 65f6c9f16cb..4dfa74b9de2 100644 --- a/api/src/org/labkey/api/cache/SimpleCache.java +++ b/api/src/org/labkey/api/cache/SimpleCache.java @@ -15,10 +15,11 @@ */ package org.labkey.api.cache; +import org.apache.commons.lang3.mutable.MutableInt; import org.jetbrains.annotations.Nullable; -import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Stream; // Cache providers return caches that implement this interface, which presents a minimal set of cache operations, // without support for standard LabKey features such as null markers, cache loaders, statistics, blocking, etc. @@ -39,7 +40,24 @@ public interface SimpleCache */ int removeUsingFilter(Predicate filter); - Set getKeys(); + /** + * Convenience method that removes every key in the provided stream from this cache. Returns the number of elements + * that were removed. removeUsingFilter() implementations call this, allowing more sharing between SimpleCache + * implementations, specifically EhSimpleCache and SimpleKeyMappingCache. + */ + default int removeAll(Stream keys) + { + MutableInt removes = new MutableInt(); + + keys.forEach(key -> { + remove(key); + removes.increment(); + }); + + return removes.intValue(); + } + + Stream getKeys(); void clear(); diff --git a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java new file mode 100644 index 00000000000..bba2611a186 --- /dev/null +++ b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.cache; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Objects; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Stream; + +// A cache that accepts keys of one type but caches using keys of a different type. Functions are passed in to map the +// keys in both directions. The purpose is to allow cache calling code to work with cache keys that are convenient to +// use, but holding those keys for long periods of time is expensive or otherwise undesirable. +public class SimpleKeyMappingCache implements SimpleCache +{ + private final SimpleCache _delegate; + private final Function _toKey; + private final Function _fromKey; + + public SimpleKeyMappingCache(@NotNull SimpleCache delegate, Function toKey, Function fromKey) + { + _delegate = delegate; + _toKey = toKey; + _fromKey = fromKey; + } + + @Override + public void put(K1 key, V value) + { + _delegate.put(_toKey.apply(key), value); + } + + @Override + public void put(K1 key, V value, long timeToLive) + { + _delegate.put(_toKey.apply(key), value, timeToLive); + } + + @Override + public @Nullable V get(K1 key) + { + return _delegate.get(_toKey.apply(key)); + } + + @Override + public void remove(K1 key) + { + _delegate.remove(_toKey.apply(key)); + } + + @Override + public int removeUsingFilter(Predicate filter) + { + return removeAll(getKeys().filter(filter)); + } + + @Override + public Stream getKeys() + { + // A key whose underlying entity no longer exists (e.g., a deleted Container) maps to null + return _delegate.getKeys() + .map(_fromKey) + .filter(Objects::nonNull); + } + + @Override + public void clear() + { + _delegate.clear(); + } + + @Override + public int getLimit() + { + return _delegate.getLimit(); + } + + @Override + public int size() + { + return _delegate.size(); + } + + @Override + public int getExpirations() + { + return _delegate.getExpirations(); + } + + @Override + public int getEvictions() + { + return _delegate.getEvictions(); + } + + @Override + public boolean isEmpty() + { + return _delegate.isEmpty(); + } + + @Override + public long getDefaultExpires() + { + return _delegate.getDefaultExpires(); + } + + @Override + public void close() + { + _delegate.close(); + } + + @Override + public CacheType getCacheType() + { + return _delegate.getCacheType(); + } + + @Override + public void log() + { + _delegate.log(); + } +} diff --git a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java index 4e494f6968f..39a5702cfb2 100644 --- a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java +++ b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java @@ -25,10 +25,9 @@ import org.labkey.api.cache.SimpleCache; import org.labkey.api.util.IntegerUtils; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Stream; class EhSimpleCache implements SimpleCache { @@ -72,26 +71,14 @@ public void remove(@NotNull K key) @Override public int removeUsingFilter(Predicate filter) { - int removes = 0; - List keys = _cache.getKeys(); - - for (K key : keys) - { - if (filter.test(key)) - { - remove(key); - removes++; - } - } - - return removes; + return removeAll(getKeys().filter(filter)); } @Override - public Set getKeys() + public Stream getKeys() { - // EhCache provides keys as a "set-like" list; make it a real Set - return new HashSet(_cache.getKeys()); + // Stream EhCache's "set-like" list of keys + return (Stream)_cache.getKeys().stream(); } @Override diff --git a/api/src/org/labkey/api/data/DatabaseCache.java b/api/src/org/labkey/api/data/DatabaseCache.java index c9f6c0700ef..f2c4e7cbf6b 100644 --- a/api/src/org/labkey/api/data/DatabaseCache.java +++ b/api/src/org/labkey/api/data/DatabaseCache.java @@ -43,19 +43,26 @@ */ public class DatabaseCache implements Cache { + private final @Nullable Class _keyClass; private final Cache _sharedCache; private final DbScope _scope; // Use the factory methods that return a BlockingDatabaseCache instead private DatabaseCache(DbScope scope, int maxSize, long defaultTimeToLive, String debugName) { + this(null, scope, maxSize, defaultTimeToLive, debugName); + } + + private DatabaseCache(@Nullable Class keyClass, DbScope scope, int maxSize, long defaultTimeToLive, String debugName) + { + _keyClass = keyClass; _sharedCache = createSharedCache(maxSize, defaultTimeToLive, debugName); _scope = scope; } public static BlockingCache get(DbScope scope, int maxSize, long defaultTimeToLive, String debugName, @Nullable CacheLoader cacheLoader) { - return new BlockingDatabaseCache<>(new DatabaseCache<>(scope, maxSize, defaultTimeToLive, debugName), cacheLoader); + return get(null, scope, maxSize, defaultTimeToLive, debugName, cacheLoader); } public static BlockingCache get(DbScope scope, int maxSize, String debugName, @Nullable CacheLoader cacheLoader) @@ -64,6 +71,17 @@ public static BlockingCache get(DbScope scope, int maxSize, String return get(scope, maxSize, CacheManager.UNLIMITED, debugName, cacheLoader); } + /** Uses a cache implementation specialized for keyClass, if one is registered; null keyClass means no specialization */ + public static BlockingCache get(@Nullable Class keyClass, DbScope scope, int maxSize, long defaultTimeToLive, String debugName, @Nullable CacheLoader cacheLoader) + { + return new BlockingDatabaseCache<>(new DatabaseCache<>(keyClass, scope, maxSize, defaultTimeToLive, debugName), cacheLoader); + } + + public static BlockingCache get(@Nullable Class keyClass, DbScope scope, int maxSize, String debugName, @Nullable CacheLoader cacheLoader) + { + return get(keyClass, scope, maxSize, CacheManager.UNLIMITED, debugName, cacheLoader); + } + /** * A DatabaseCache wrapped by a BlockingCache that knows how to replay load() events that take place on a * transaction's private cache into the shared cache after successful commit. This can result in a big performance @@ -209,7 +227,7 @@ public String toString() protected Cache createSharedCache(int maxSize, long defaultTimeToLive, String debugName) { - return CacheManager.getCache(maxSize, defaultTimeToLive, debugName); + return CacheManager.getCache(_keyClass, maxSize, defaultTimeToLive, debugName); } @Override @@ -220,7 +238,7 @@ public final Cache createTemporaryCache() private Cache createTemporaryCache(TrackingCache trackingCache) { - return CacheManager.getTemporaryCache(trackingCache.getLimit(), trackingCache.getDefaultExpires(), "transaction cache: " + trackingCache.getDebugName(), trackingCache.getTransactionStats()); + return CacheManager.getTemporaryCache(_keyClass, trackingCache.getLimit(), trackingCache.getDefaultExpires(), "transaction cache: " + trackingCache.getDebugName(), trackingCache.getTransactionStats()); } protected @Nullable TransactionImpl getCurrentTransaction() diff --git a/api/src/org/labkey/api/settings/FolderSettingsCache.java b/api/src/org/labkey/api/settings/FolderSettingsCache.java index 3a3a92e5a61..ba795bf72d9 100644 --- a/api/src/org/labkey/api/settings/FolderSettingsCache.java +++ b/api/src/org/labkey/api/settings/FolderSettingsCache.java @@ -27,7 +27,7 @@ // properties on a per-container basis and clear the entire cache on every change of look and feel settings. public class FolderSettingsCache { - private static final BlockingCache CACHE = CacheManager.getBlockingCache(Constants.getMaxContainers(), CacheManager.DAY, "Folder settings", (c, argument) -> new FolderSettings(c)); + private static final BlockingCache CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxContainers(), CacheManager.DAY, "Folder settings", (c, _) -> new FolderSettings(c)); public static String getDefaultDateFormat(Container c) { diff --git a/experiment/src/org/labkey/experiment/api/property/DomainPropertyManager.java b/experiment/src/org/labkey/experiment/api/property/DomainPropertyManager.java index dbc7137c295..8421ba98448 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainPropertyManager.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainPropertyManager.java @@ -75,7 +75,7 @@ public List load(@NotNull Container container, } private static final ConditionalFormatLoader CONDITIONAL_FORMAT_LOADER = new ConditionalFormatLoader(); - private static final BlockingCache> CONDITIONAL_FORMAT_CACHE = DatabaseCache.get(getExpSchema().getScope(), Constants.getMaxContainers(), CacheManager.DAY, "Conditional formats", CONDITIONAL_FORMAT_LOADER); + private static final BlockingCache> CONDITIONAL_FORMAT_CACHE = DatabaseCache.get(Container.class, getExpSchema().getScope(), Constants.getMaxContainers(), CacheManager.DAY, "Conditional formats", CONDITIONAL_FORMAT_LOADER); private DomainPropertyManager(){} @@ -184,7 +184,7 @@ public List getConditionalFormats(Container con return validators.isEmpty() ? MultiMapUtils.emptyMultiValuedMap() : MultiMapUtils.unmodifiableMultiValuedMap(validators); }; - private static final Cache> VALIDATOR_CACHE = DatabaseCache.get(getExpSchema().getScope(), Constants.getMaxContainers(), CacheManager.HOUR, "Property validators", PV_LOADER); + private static final Cache> VALIDATOR_CACHE = DatabaseCache.get(Container.class, getExpSchema().getScope(), Constants.getMaxContainers(), CacheManager.HOUR, "Property validators", PV_LOADER); private static final Collection EMPTY_COLLECTION = Collections.emptyList(); From 76bf2c1e551a76428e18e9b6583af17545d4d07d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 8 Oct 2026 16:05:38 -0700 Subject: [PATCH 2/5] junit tests --- api/src/org/labkey/api/ApiModule.java | 6 +- .../api/cache/ContainerKeySimpleCache.java | 45 ++++++++++++++ .../api/cache/SimpleKeyMappingCache.java | 60 ++++++++++++++++++- 3 files changed, 107 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 9312bfea331..dcef2ffef26 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/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; @@ -494,6 +496,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, @@ -545,6 +548,7 @@ public void registerServlets(ServletContext servletCtx) CompareType.TestCase.class, ContainerDisplayColumn.TestCase.class, ContainerFilter.TestCase.class, + ContainerKeySimpleCache.TestCase.class, ContainerManager.TestCase.class, ContentSecurityPolicyFilter.TestCase.class, DbSchema.DDLMethodsTestCase.class, @@ -556,7 +560,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, diff --git a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java index 30f96b1ff33..39715a0a464 100644 --- a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java +++ b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java @@ -1,9 +1,16 @@ 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 extends SimpleKeyMappingCache { @@ -11,4 +18,42 @@ public ContainerKeySimpleCache(@NotNull SimpleCache 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); + Cache cache = CacheManager.getTemporaryCache(Container.class, 10, CacheManager.UNLIMITED, "ContainerKeySimpleCache test", null); + + try + { + 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 + { + cache.close(); + + if (null != ContainerManager.getForId(child.getEntityId())) + ContainerManager.delete(child, user); + } + } + } } diff --git a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java index bba2611a186..2998acaf73a 100644 --- a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java +++ b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java @@ -17,15 +17,22 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.cache.ehcache.EhCacheProvider; import java.util.Objects; +import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Collectors; import java.util.stream.Stream; -// A cache that accepts keys of one type but caches using keys of a different type. Functions are passed in to map the -// keys in both directions. The purpose is to allow cache calling code to work with cache keys that are convenient to -// use, but holding those keys for long periods of time is expensive or otherwise undesirable. +// A cache that accepts keys of one type but actually caches using keys of a different type. Functions are passed in to +// map the keys in both directions. The purpose is to allow cache calling code to work with cache keys that are +// convenient to use, but holding those keys for long periods of time is expensive or otherwise undesirable. +// Note that getKeys() and removeUsingFilter() apply fromKey to every entry, which could be costly (e.g., a DB lookup +// per key), but these methods are not use widely, especially not on non-String keys. public class SimpleKeyMappingCache implements SimpleCache { private final SimpleCache _delegate; @@ -137,4 +144,51 @@ public void log() { _delegate.log(); } + + public static class TestCase extends Assert + { + // Negative numbers stand in for keys whose underlying entity no longer exists + private static @Nullable Integer fromKey(String key) + { + int i = Integer.parseInt(key); + return i < 0 ? null : i; + } + + @Test + public void testKeyMapping() + { + SimpleCache delegate = EhCacheProvider.getInstance().getSimpleCache("SimpleKeyMappingCache test", 100, CacheManager.UNLIMITED, CacheManager.UNLIMITED, true); + + try + { + SimpleCache cache = new SimpleKeyMappingCache<>(delegate, String::valueOf, TestCase::fromKey); + cache.put(1, "one"); + cache.put(2, "two", CacheManager.HOUR); + cache.put(-1, "orphan"); + + assertEquals("one", delegate.get("1")); + assertEquals("two", cache.get(2)); + assertEquals("orphan", cache.get(-1)); + assertEquals(3, cache.size()); + assertEquals(Set.of(1, 2), cache.getKeys().collect(Collectors.toSet())); + + cache.remove(1); + assertNull(delegate.get("1")); + assertNull(cache.get(1)); + + // The filter never sees the orphan, so it survives + assertEquals(1, cache.removeUsingFilter(_ -> true)); + assertNull(cache.get(2)); + assertEquals("orphan", cache.get(-1)); + assertEquals(1, cache.size()); + + cache.clear(); + assertTrue(cache.isEmpty()); + } + finally + { + delegate.close(); + } + } + } } From 8b705c8602d3bc6e611223284791dc67f8c71f5d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 8 Oct 2026 16:08:25 -0700 Subject: [PATCH 3/5] Fix comment --- api/src/org/labkey/api/cache/SimpleKeyMappingCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java index 2998acaf73a..08569c30ca7 100644 --- a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java +++ b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java @@ -31,8 +31,8 @@ // A cache that accepts keys of one type but actually caches using keys of a different type. Functions are passed in to // map the keys in both directions. The purpose is to allow cache calling code to work with cache keys that are // convenient to use, but holding those keys for long periods of time is expensive or otherwise undesirable. -// Note that getKeys() and removeUsingFilter() apply fromKey to every entry, which could be costly (e.g., a DB lookup -// per key), but these methods are not use widely, especially not on non-String keys. +// getKeys() and removeUsingFilter() apply fromKey to every entry, which could be costly (e.g., a DB lookup per key), +// but these methods are not used widely, especially not on non-String keys. public class SimpleKeyMappingCache implements SimpleCache { private final SimpleCache _delegate; From c21779a86a6c2fa8651cdbca00424189848b7080 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 8 Oct 2026 16:48:03 -0700 Subject: [PATCH 4/5] Small cleanups --- api/src/org/labkey/api/cache/CacheManager.java | 16 ++++++---------- api/src/org/labkey/api/data/DatabaseCache.java | 6 +++--- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/cache/CacheManager.java b/api/src/org/labkey/api/cache/CacheManager.java index e7655ac675c..52829f3baaf 100644 --- a/api/src/org/labkey/api/cache/CacheManager.java +++ b/api/src/org/labkey/api/cache/CacheManager.java @@ -86,14 +86,6 @@ private static SimpleCache getSimpleCache(@Nullable Class keyCla return (SimpleCache) (null == mapping ? cache : mapping.apply(cache)); } - private static TrackingCache createCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName) - { - CacheWrapper 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 TrackingCache getCache(int limit, long defaultTimeToLive, String debugName) { return getCache(null, limit, defaultTimeToLive, debugName); @@ -102,7 +94,10 @@ public static TrackingCache getCache(int limit, long defaultTimeToL /** Returns a cache implementation specialized for keyClass, if one is registered; null keyClass means no specialization */ public static TrackingCache getCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName) { - return createCache(keyClass, limit, defaultTimeToLive, debugName); + CacheWrapper 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 Cache getStringKeyCache(int limit, long defaultTimeToLive, String debugName) @@ -126,12 +121,13 @@ public static BlockingCache getBlockingStringKeyCache(int limit, 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 Cache getTemporaryCache(int limit, long defaultTimeToLive, String debugName, @Nullable Stats stats) { 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 Cache getTemporaryCache(@Nullable Class keyClass, int limit, long defaultTimeToLive, String debugName, @Nullable Stats stats) { return new CacheWrapper<>(getSimpleCache(keyClass, limit, defaultTimeToLive, debugName, true), debugName, stats, null); diff --git a/api/src/org/labkey/api/data/DatabaseCache.java b/api/src/org/labkey/api/data/DatabaseCache.java index f2c4e7cbf6b..6a85dcd47f7 100644 --- a/api/src/org/labkey/api/data/DatabaseCache.java +++ b/api/src/org/labkey/api/data/DatabaseCache.java @@ -336,7 +336,7 @@ public String toString() public static class TestCase extends Assert { - public static class TempDatabaseCache extends DatabaseCache + public static class TempDatabaseCache extends DatabaseCache { public TempDatabaseCache(DbScope scope, int maxSize, String debugName) { @@ -345,9 +345,9 @@ public TempDatabaseCache(DbScope scope, int maxSize, String debugName) // Shared cache needs to be a temporary cache, otherwise we'll leak a cache on every invocation because of KNOWN_CACHES @Override - protected Cache createSharedCache(int maxSize, long defaultTimeToLive, String debugName) + protected Cache createSharedCache(int maxSize, long defaultTimeToLive, String debugName) { - return CacheManager.getTemporaryCache(maxSize, defaultTimeToLive, debugName, null); + return CacheManager.getTemporaryCache(String.class, maxSize, defaultTimeToLive, debugName, null); } } From aaaa6ab6cd4bc548574fb2a92b9154f310f6736b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 9 Oct 2026 08:34:11 -0700 Subject: [PATCH 5/5] Extend AutoCloseable on SimpleCache and CacheWrapper. Use it in test code. remove() now returns whether an entry was removed or not. --- .../org/labkey/api/cache/BlockingCache.java | 4 - api/src/org/labkey/api/cache/Cache.java | 19 +- .../org/labkey/api/cache/CacheManager.java | 2 +- api/src/org/labkey/api/cache/CacheType.java | 5 - .../org/labkey/api/cache/CacheWrapper.java | 20 +- .../api/cache/ContainerKeySimpleCache.java | 5 +- .../labkey/api/cache/NoopCacheProvider.java | 10 +- api/src/org/labkey/api/cache/SimpleCache.java | 32 +-- .../api/cache/SimpleKeyMappingCache.java | 19 +- api/src/org/labkey/api/cache/Tracking.java | 5 - .../api/cache/ehcache/EhSimpleCache.java | 11 +- .../org/labkey/api/data/DatabaseCache.java | 234 +++++++++--------- api/src/org/labkey/api/data/DbScope.java | 9 +- 13 files changed, 150 insertions(+), 225 deletions(-) diff --git a/api/src/org/labkey/api/cache/BlockingCache.java b/api/src/org/labkey/api/cache/BlockingCache.java index 33d3dd2de4a..0a9ff4116dd 100644 --- a/api/src/org/labkey/api/cache/BlockingCache.java +++ b/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) { diff --git a/api/src/org/labkey/api/cache/Cache.java b/api/src/org/labkey/api/cache/Cache.java index 0764479d866..3ab70acf319 100644 --- a/api/src/org/labkey/api/cache/Cache.java +++ b/api/src/org/labkey/api/cache/Cache.java @@ -22,7 +22,7 @@ import java.util.Set; import java.util.function.Predicate; -public interface Cache +public interface Cache extends AutoCloseable { void put(@NotNull K key, V value); @@ -33,23 +33,25 @@ public interface Cache /** * The wrapped calls to get() and put() are not guaranteed synchronous (see subclass/wrapper impl) */ - V get(@NotNull K key, @Nullable Object arg, CacheLoader loader); + V get(@NotNull K key, @Nullable Object arg, CacheLoader 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 filter); record StringPrefixFilter(String prefix) implements Predicate + { + @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, as in SimpleCache. There are very few callers, and most are tests. Set getKeys(); @@ -60,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 diff --git a/api/src/org/labkey/api/cache/CacheManager.java b/api/src/org/labkey/api/cache/CacheManager.java index 52829f3baaf..e07b87b9901 100644 --- a/api/src/org/labkey/api/cache/CacheManager.java +++ b/api/src/org/labkey/api/cache/CacheManager.java @@ -133,7 +133,7 @@ public static Cache getTemporaryCache(@Nullable Class keyClass, return new CacheWrapper<>(getSimpleCache(keyClass, limit, defaultTimeToLive, debugName, true), debugName, stats, null); } - private static final Cache SHARED_CACHE = getStringKeyCache(10000, DEFAULT_TIMEOUT, "Shared"); + private static final Cache SHARED_CACHE = getCache(String.class, 10000, DEFAULT_TIMEOUT, "Shared"); public static Cache getSharedCache() { diff --git a/api/src/org/labkey/api/cache/CacheType.java b/api/src/org/labkey/api/cache/CacheType.java index 5ffe14028e0..6a2abd097b6 100644 --- a/api/src/org/labkey/api/cache/CacheType.java +++ b/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, diff --git a/api/src/org/labkey/api/cache/CacheWrapper.java b/api/src/org/labkey/api/cache/CacheWrapper.java index 7eadafa6092..488fe7b6f4e 100644 --- a/api/src/org/labkey/api/cache/CacheWrapper.java +++ b/api/src/org/labkey/api/cache/CacheWrapper.java @@ -41,8 +41,8 @@ class CacheWrapper implements TrackingCache, CacheMXBean @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; @@ -74,7 +74,6 @@ public void put(@NotNull K key, V value) } } - @Override public void put(@NotNull K key, V value, long timeToLive) { @@ -93,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 loader) { @@ -127,7 +124,6 @@ else if (null != loader) } } - @Override public void remove(@NotNull K key) { @@ -144,14 +140,12 @@ public void remove(@NotNull K key) } } - @Override public int removeUsingFilter(Predicate kFilter) { return trackRemoves(_cache.removeUsingFilter(kFilter)); } - @Override public Set getKeys() { @@ -165,14 +159,12 @@ public void clear() trackClear(); } - @Override public int getLimit() { return _cache.getLimit(); } - @Override public int size() { @@ -197,14 +189,12 @@ public long getDefaultExpires() return _cache.getDefaultExpires(); } - @Override public void close() { _cache.close(); } - @Override public String getDebugName() { @@ -311,12 +301,6 @@ private void trackClear() _stats.clears.incrementAndGet(); } - - public SimpleCache getWrappedCache() - { - return _cache; - } - /* CacheMBean */ @Override diff --git a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java index 39715a0a464..74b2aae35c2 100644 --- a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java +++ b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java @@ -27,9 +27,8 @@ public void testDeletedContainer() User user = TestContext.get().getUser(); Container parent = JunitUtil.getTestContainer(); Container child = ContainerManager.ensureContainer(parent, "ContainerKeySimpleCache", user); - Cache cache = CacheManager.getTemporaryCache(Container.class, 10, CacheManager.UNLIMITED, "ContainerKeySimpleCache test", null); - try + try (Cache cache = CacheManager.getTemporaryCache(Container.class, 10, CacheManager.UNLIMITED, "ContainerKeySimpleCache test", null)) { cache.put(parent, "parent"); cache.put(child, "child"); @@ -49,8 +48,6 @@ public void testDeletedContainer() } finally { - cache.close(); - if (null != ContainerManager.getForId(child.getEntityId())) ContainerManager.delete(child, user); } diff --git a/api/src/org/labkey/api/cache/NoopCacheProvider.java b/api/src/org/labkey/api/cache/NoopCacheProvider.java index 7bfa3ad9a07..42eab1e8561 100644 --- a/api/src/org/labkey/api/cache/NoopCacheProvider.java +++ b/api/src/org/labkey/api/cache/NoopCacheProvider.java @@ -17,7 +17,6 @@ import org.jetbrains.annotations.Nullable; -import java.util.function.Predicate; import java.util.stream.Stream; import static org.labkey.api.cache.CacheType.DeterministicLRU; @@ -55,14 +54,9 @@ public V get(K key) } @Override - public void remove(K key) + public boolean remove(K key) { - } - - @Override - public int removeUsingFilter(Predicate filter) - { - return 0; + return false; } @Override diff --git a/api/src/org/labkey/api/cache/SimpleCache.java b/api/src/org/labkey/api/cache/SimpleCache.java index 4dfa74b9de2..200851046cb 100644 --- a/api/src/org/labkey/api/cache/SimpleCache.java +++ b/api/src/org/labkey/api/cache/SimpleCache.java @@ -15,7 +15,6 @@ */ package org.labkey.api.cache; -import org.apache.commons.lang3.mutable.MutableInt; import org.jetbrains.annotations.Nullable; import java.util.function.Predicate; @@ -24,7 +23,7 @@ // Cache providers return caches that implement this interface, which presents a minimal set of cache operations, // without support for standard LabKey features such as null markers, cache loaders, statistics, blocking, etc. // Implementations must be thread-safe. -public interface SimpleCache +public interface SimpleCache extends AutoCloseable { void put(K key, V value); @@ -32,29 +31,21 @@ public interface SimpleCache @Nullable V get(K key); - void remove(K key); - /** - * Removes every element in the cache where filter.test(K key) evaluates to true. - * Returns the number of elements that were removed. + * @return true if the element was removed, false if it was not found in the cache */ - int removeUsingFilter(Predicate filter); + boolean remove(K key); /** - * Convenience method that removes every key in the provided stream from this cache. Returns the number of elements - * that were removed. removeUsingFilter() implementations call this, allowing more sharing between SimpleCache - * implementations, specifically EhSimpleCache and SimpleKeyMappingCache. + * Removes every element in the cache where filter.test(K key) evaluates to true. + * @return the number of elements that were removed. */ - default int removeAll(Stream keys) + default int removeUsingFilter(Predicate filter) { - MutableInt removes = new MutableInt(); - - keys.forEach(key -> { - remove(key); - removes.increment(); - }); - - return removes.intValue(); + return (int)getKeys() + .filter(filter) + .filter(this::remove) + .count(); } Stream getKeys(); @@ -85,9 +76,10 @@ default int removeAll(Stream keys) long getDefaultExpires(); /** - * Some CacheProviders (e.g., Ehcache) hold onto the caches they create. close() lets us discard temporary + * Some CacheProviders (e.g., Ehcache) hold onto 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(); CacheType getCacheType(); diff --git a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java index 08569c30ca7..a95c97ba879 100644 --- a/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java +++ b/api/src/org/labkey/api/cache/SimpleKeyMappingCache.java @@ -24,7 +24,6 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -65,15 +64,9 @@ public void put(K1 key, V value, long timeToLive) } @Override - public void remove(K1 key) + public boolean remove(K1 key) { - _delegate.remove(_toKey.apply(key)); - } - - @Override - public int removeUsingFilter(Predicate filter) - { - return removeAll(getKeys().filter(filter)); + return _delegate.remove(_toKey.apply(key)); } @Override @@ -157,9 +150,7 @@ public static class TestCase extends Assert @Test public void testKeyMapping() { - SimpleCache delegate = EhCacheProvider.getInstance().getSimpleCache("SimpleKeyMappingCache test", 100, CacheManager.UNLIMITED, CacheManager.UNLIMITED, true); - - try + try (SimpleCache delegate = EhCacheProvider.getInstance().getSimpleCache("SimpleKeyMappingCache test", 100, CacheManager.UNLIMITED, CacheManager.UNLIMITED, true)) { SimpleCache cache = new SimpleKeyMappingCache<>(delegate, String::valueOf, TestCase::fromKey); cache.put(1, "one"); @@ -185,10 +176,6 @@ public void testKeyMapping() cache.clear(); assertTrue(cache.isEmpty()); } - finally - { - delegate.close(); - } } } } diff --git a/api/src/org/labkey/api/cache/Tracking.java b/api/src/org/labkey/api/cache/Tracking.java index c10848fe6cf..6bea00cda90 100644 --- a/api/src/org/labkey/api/cache/Tracking.java +++ b/api/src/org/labkey/api/cache/Tracking.java @@ -17,11 +17,6 @@ import org.jetbrains.annotations.Nullable; -/** - * User: adam - * Date: Jul 8, 2010 - * Time: 10:44:59 AM - */ public interface Tracking { String getDebugName(); diff --git a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java index 39a5702cfb2..155dae99e60 100644 --- a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java +++ b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java @@ -26,7 +26,6 @@ import org.labkey.api.util.IntegerUtils; import java.util.List; -import java.util.function.Predicate; import java.util.stream.Stream; class EhSimpleCache implements SimpleCache @@ -63,15 +62,9 @@ public void put(@NotNull K key, V value, long timeToLive) } @Override - public void remove(@NotNull K key) + public boolean remove(@NotNull K key) { - _cache.remove(key); - } - - @Override - public int removeUsingFilter(Predicate filter) - { - return removeAll(getKeys().filter(filter)); + return _cache.remove(key); } @Override diff --git a/api/src/org/labkey/api/data/DatabaseCache.java b/api/src/org/labkey/api/data/DatabaseCache.java index 6a85dcd47f7..d2acdbbaa0a 100644 --- a/api/src/org/labkey/api/data/DatabaseCache.java +++ b/api/src/org/labkey/api/data/DatabaseCache.java @@ -357,8 +357,6 @@ public void testDatabaseCache() { MyScope scope = new MyScope(); - DatabaseCache cache = new TempDatabaseCache<>(scope, 10, "Test Cache"); - // basic cache testing // Hold values so we can test equality below @@ -367,133 +365,134 @@ public void testDatabaseCache() for (int i = 0; i < values.length; i++) values[i] = "value_" + i; - TrackingCache trackingCache = cache.getCache().getTrackingCache(); - - for (int i = 1; i <= 20; i++) + try (DatabaseCache cache = new TempDatabaseCache<>(scope, 10, "Test Cache")) { - cache.put("key_" + i, values[i]); - assertTrue(trackingCache.size() <= 10); - } + TrackingCache trackingCache = cache.getCache().getTrackingCache(); - int correctCount = 0; - - // access in reverse order - for (int i = 10; i >= 1; i--) - { - if (null == cache.get("key_" + i)) - correctCount++; + for (int i = 1; i <= 20; i++) + { + cache.put("key_" + i, values[i]); + assertTrue(trackingCache.size() <= 10); + } - if (cache.get("key_" + (i + 10)) == values[i + 10]) - correctCount++; - } + int correctCount = 0; - // A DeterministicLRU cache guarantees that the least recently used element is always kicked out when capacity - // is reached. A NonDeterministicLRU cache (e.g., an Ehcache implementation) attempts to kick out the least - // recently used element, but provides no guarantee since it uses sampling for performance reasons. This test - // is not very useful for a NonDeterministicLRU cache. Adjust the check below if the test fails. - switch (trackingCache.getCacheType()) - { - case DeterministicLRU -> assertEquals("Count was " + correctCount, 20, correctCount); - case NonDeterministicLRU -> assertTrue("Count was " + correctCount, correctCount > 11); - default -> fail("Unknown cache type"); - } + // access in reverse order + for (int i = 10; i >= 1; i--) + { + if (null == cache.get("key_" + i)) + correctCount++; - // add 5 more (if deterministic, should kick out 16-20 which are now LRU) - for (int i = 21; i <= 25; i++) - cache.put("key_" + i, values[i]); + if (cache.get("key_" + (i + 10)) == values[i + 10]) + correctCount++; + } - assertEquals(10, trackingCache.size()); - correctCount = 0; + // A DeterministicLRU cache guarantees that the least recently used element is always kicked out when capacity + // is reached. A NonDeterministicLRU cache (e.g., an Ehcache implementation) attempts to kick out the least + // recently used element, but provides no guarantee since it uses sampling for performance reasons. This test + // is not very useful for a NonDeterministicLRU cache. Adjust the check below if the test fails. + switch (trackingCache.getCacheType()) + { + case DeterministicLRU -> assertEquals("Count was " + correctCount, 20, correctCount); + case NonDeterministicLRU -> assertTrue("Count was " + correctCount, correctCount > 11); + default -> fail("Unknown cache type"); + } - for (int i = 11; i <= 15; i++) - { - if (cache.get("key_" + i) == values[i]) - correctCount++; + // add 5 more (if deterministic, should kick out 16-20 which are now LRU) + for (int i = 21; i <= 25; i++) + cache.put("key_" + i, values[i]); - if (cache.get("key_" + (i + 10)) == values[i + 10]) - correctCount++; - } + assertEquals(10, trackingCache.size()); + correctCount = 0; - // As above, this test isn't very useful for a NonDeterministicLRU cache. - switch (trackingCache.getCacheType()) - { - case DeterministicLRU -> assertEquals("Count was " + correctCount, 10, correctCount); - case NonDeterministicLRU -> { - assertTrue("Count was " + correctCount, correctCount > 4); + for (int i = 11; i <= 15; i++) + { + if (cache.get("key_" + i) == values[i]) + correctCount++; - // Make sure key_11 is in the cache - cache.put("key_11", values[11]); - assertSame(cache.get("key_11"), values[11]); + if (cache.get("key_" + (i + 10)) == values[i + 10]) + correctCount++; } - default -> fail("Unknown cache type"); - } - // transaction testing - try (DbScope.Transaction transaction = scope.beginTransaction()) - { - assertTrue(scope.isTransactionActive()); - TransactionCache tCache = (TransactionCache) cache.getCache(); - Cache privateCache = tCache._privateCache; - Cache sharedCache = tCache._sharedCache; - assertEquals(10, tCache.getKeys().size()); - assertEquals(10, sharedCache.getKeys().size()); - assertEquals(0, privateCache.getKeys().size()); - - // Test read-through transaction cache - assertSame(cache.get("key_11"), values[11]); - // Nothing should change after a read - assertEquals(10, sharedCache.getKeys().size()); - assertEquals(10, tCache.getKeys().size()); - assertEquals(0, privateCache.getKeys().size()); - - cache.remove("key_11"); - assertNull(cache.get("key_11")); - assertEquals(10, sharedCache.getKeys().size()); - assertEquals(10, tCache.getKeys().size()); // Unique keys are still 10 - assertEquals(1, privateCache.getKeys().size()); // But private cache should now have a remove entry - - for (int i = 30; i < 35; i++) + // As above, this test isn't very useful for a NonDeterministicLRU cache. + switch (trackingCache.getCacheType()) { - cache.put("key_" + i, values[i]); - } - assertEquals(10, sharedCache.getKeys().size()); - assertEquals(15, tCache.getKeys().size()); // 15 unique now - assertEquals(6, privateCache.getKeys().size()); - - // Remove all the even keys - cache.removeUsingFilter(key -> Integer.valueOf(key.substring(4)) % 2 == 0); - assertNull(cache.get("key_12")); - assertNull(cache.get("key_22")); - assertNull(cache.get("key_24")); - assertNull(cache.get("key_30")); - assertNotNull(privateCache.get("key_30")); - assertNull(cache.get("key_34")); - assertNotNull(privateCache.get("key_34")); - - // imitate another thread: toggle transaction and test - scope.setOverrideTransactionActive(Boolean.FALSE); - assertSame(cache.get("key_11"), values[11]); - assertNull(cache.get("key_31")); - assertNull(cache.get("key_33")); - scope.setOverrideTransactionActive(null); - - // This should close the transaction caches - transaction.commit(); - - // Test that remove got applied to shared cache - assertNull(cache.get("key_11")); - // Test that even keys got removed from the shared cache - cache.getKeys().stream() - .filter(key -> Integer.valueOf(key.substring(4)) % 2 == 0) - .findAny() - .ifPresent(key -> fail("Found an even key: " + key)); - // No test for puts since DatabaseCache doesn't replay them (that's solely a BlockingDatabaseCache thing) + case DeterministicLRU -> assertEquals("Count was " + correctCount, 10, correctCount); + case NonDeterministicLRU -> + { + assertTrue("Count was " + correctCount, correctCount > 4); - cache.removeUsingFilter(new Cache.StringPrefixFilter("key")); - assert trackingCache.size() == 0; + // Make sure key_11 is in the cache + cache.put("key_11", values[11]); + assertSame(cache.get("key_11"), values[11]); + } + default -> fail("Unknown cache type"); + } - // This should close the (temporary) shared cache - cache.close(); + // transaction testing + try (DbScope.Transaction transaction = scope.beginTransaction()) + { + assertTrue(scope.isTransactionActive()); + TransactionCache tCache = (TransactionCache) cache.getCache(); + Cache privateCache = tCache._privateCache; + Cache sharedCache = tCache._sharedCache; + assertEquals(10, tCache.getKeys().size()); + assertEquals(10, sharedCache.getKeys().size()); + assertEquals(0, privateCache.getKeys().size()); + + // Test read-through transaction cache + assertSame(cache.get("key_11"), values[11]); + // Nothing should change after a read + assertEquals(10, sharedCache.getKeys().size()); + assertEquals(10, tCache.getKeys().size()); + assertEquals(0, privateCache.getKeys().size()); + + cache.remove("key_11"); + assertNull(cache.get("key_11")); + assertEquals(10, sharedCache.getKeys().size()); + assertEquals(10, tCache.getKeys().size()); // Unique keys are still 10 + assertEquals(1, privateCache.getKeys().size()); // But private cache should now have a remove entry + + for (int i = 30; i < 35; i++) + { + cache.put("key_" + i, values[i]); + } + assertEquals(10, sharedCache.getKeys().size()); + assertEquals(15, tCache.getKeys().size()); // 15 unique now + assertEquals(6, privateCache.getKeys().size()); + + // Remove all the even keys + cache.removeUsingFilter(key -> Integer.valueOf(key.substring(4)) % 2 == 0); + assertNull(cache.get("key_12")); + assertNull(cache.get("key_22")); + assertNull(cache.get("key_24")); + assertNull(cache.get("key_30")); + assertNotNull(privateCache.get("key_30")); + assertNull(cache.get("key_34")); + assertNotNull(privateCache.get("key_34")); + + // imitate another thread: toggle transaction and test + scope.setOverrideTransactionActive(Boolean.FALSE); + assertSame(cache.get("key_11"), values[11]); + assertNull(cache.get("key_31")); + assertNull(cache.get("key_33")); + scope.setOverrideTransactionActive(null); + + // This should close the transaction caches + transaction.commit(); + + // Test that remove got applied to shared cache + assertNull(cache.get("key_11")); + // Test that even keys got removed from the shared cache + cache.getKeys().stream() + .filter(key -> Integer.valueOf(key.substring(4)) % 2 == 0) + .findAny() + .ifPresent(key -> fail("Found an even key: " + key)); + // No test for puts since DatabaseCache doesn't replay them (that's solely a BlockingDatabaseCache thing) + + cache.removeUsingFilter(new Cache.StringPrefixFilter("key")); + assert trackingCache.size() == 0; + } } } @@ -505,9 +504,7 @@ public void testDatabaseCacheTransactionLimit() DatabaseCache> dbCache = new TempDatabaseCache<>(scope, maxSize, "Test Cache"); - BlockingDatabaseCache cache = new BlockingDatabaseCache<>(dbCache, new TestCacheLoader()); - - try (DbScope.Transaction transaction = scope.beginTransaction()) + try (BlockingDatabaseCache cache = new BlockingDatabaseCache<>(dbCache, new TestCacheLoader()); DbScope.Transaction transaction = scope.beginTransaction()) { int taskCount; for (int i = 1; i <= 15; i++) @@ -531,9 +528,6 @@ public void testDatabaseCacheTransactionLimit() } } transaction.commit(); - - // This should close the (temporary) shared cache - cache.close(); } } diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 10539fbb4a5..a8e82f41411 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -3331,10 +3331,9 @@ public void tesCommitTaskFailure() public void testCommitAndKeepConnection() { DbScope scope = getLabKeyScope(); - // TempDatabaseCache's shared cache is temporary, so it stays out of KNOWN_CACHES; close() it below - DatabaseCache cache = new DatabaseCache.TestCase.TempDatabaseCache<>(scope, 10, "commitAndKeepConnection test"); - try + // TempDatabaseCache's shared cache is temporary, so it stays out of KNOWN_CACHES; close() it below + try (DatabaseCache cache = new DatabaseCache.TestCase.TempDatabaseCache<>(scope, 10, "commitAndKeepConnection test")) { cache.put("key_1", "value_1"); cache.put("key_2", "value_2"); @@ -3374,10 +3373,6 @@ public void testCommitAndKeepConnection() assertFalse("commit() must invalidate the shared cache", cache.getKeys().contains("key_2")); assertFalse(scope.isTransactionActive()); } - finally - { - cache.close(); - } } @Test