diff --git a/api/src/org/labkey/api/cache/BlockingCache.java b/api/src/org/labkey/api/cache/BlockingCache.java index 917780e8f55..867909b77ce 100644 --- a/api/src/org/labkey/api/cache/BlockingCache.java +++ b/api/src/org/labkey/api/cache/BlockingCache.java @@ -23,7 +23,6 @@ import org.labkey.api.test.TestWhen; import org.labkey.api.util.DeadlockPreventingException; import org.labkey.api.util.DebugInfoDumper; -import org.labkey.api.util.Filter; import java.util.HashMap; import java.util.Map; @@ -31,6 +30,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Predicate; /** * This is a decorator for any Cache instance, it will provide for synchronizing object load @@ -236,7 +236,7 @@ public void remove(@NotNull K key) } @Override - public int removeUsingFilter(Filter filter) + public int removeUsingFilter(Predicate filter) { return _cache.removeUsingFilter(filter); } @@ -296,7 +296,7 @@ public void setUp() } @Override public Wrapper get(@NotNull Integer key, @Nullable Object arg, CacheLoader> loader) { throw new UnsupportedOperationException(); } @Override public void remove(@NotNull Integer key) { throw new UnsupportedOperationException(); } - @Override public int removeUsingFilter(Filter filter) { throw new UnsupportedOperationException(); } + @Override public int removeUsingFilter(Predicate filter) { throw new UnsupportedOperationException(); } @Override public Set getKeys() { throw new UnsupportedOperationException(); } @Override public void clear() { throw new UnsupportedOperationException(); } @Override public void close() { throw new UnsupportedOperationException(); } diff --git a/api/src/org/labkey/api/cache/Cache.java b/api/src/org/labkey/api/cache/Cache.java index 49a1ba3273a..cbf64023a89 100644 --- a/api/src/org/labkey/api/cache/Cache.java +++ b/api/src/org/labkey/api/cache/Cache.java @@ -18,15 +18,9 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.labkey.api.util.Filter; import java.util.Set; - -/** - * User: adam - * Date: Jul 8, 2010 - * Time: 9:32:10 AM - */ +import java.util.function.Predicate; public interface Cache { @@ -46,9 +40,9 @@ public interface Cache /** Removes every element in the cache where filter.accept(K key) evaluates to true. * Returns the number of elements that were removed. */ - int removeUsingFilter(Filter filter); + int removeUsingFilter(Predicate filter); - class StringPrefixFilter implements Filter + class StringPrefixFilter implements Predicate { private final String _prefix; @@ -58,7 +52,7 @@ public StringPrefixFilter(String prefix) } @Override - public boolean accept(String s) + public boolean test(String s) { return s.startsWith(_prefix); } diff --git a/api/src/org/labkey/api/cache/CacheWrapper.java b/api/src/org/labkey/api/cache/CacheWrapper.java index 5322da3b686..2c2b2e135ca 100644 --- a/api/src/org/labkey/api/cache/CacheWrapper.java +++ b/api/src/org/labkey/api/cache/CacheWrapper.java @@ -20,18 +20,12 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.mbean.CacheMXBean; import org.labkey.api.util.ContextListener; -import org.labkey.api.util.Filter; import org.labkey.api.util.HeartBeat; import javax.management.DynamicMBean; import javax.management.StandardMBean; import java.util.Set; - -/** - * User: adam - * Date: Jul 8, 2010 - * Time: 9:47:03 AM - */ +import java.util.function.Predicate; // Wraps a SimpleCache to provide a full Cache implementation. Adds null markers, loaders, statistics gathering and debug name. class CacheWrapper implements TrackingCache, CacheMXBean @@ -150,7 +144,7 @@ public void remove(@NotNull K key) @Override - public int removeUsingFilter(Filter kFilter) + public int removeUsingFilter(Predicate kFilter) { return trackRemoves(_cache.removeUsingFilter(kFilter)); } diff --git a/api/src/org/labkey/api/cache/NoopCacheProvider.java b/api/src/org/labkey/api/cache/NoopCacheProvider.java index 1dd83cc56da..7aa7a6bb07c 100644 --- a/api/src/org/labkey/api/cache/NoopCacheProvider.java +++ b/api/src/org/labkey/api/cache/NoopCacheProvider.java @@ -16,10 +16,10 @@ package org.labkey.api.cache; import org.jetbrains.annotations.Nullable; -import org.labkey.api.util.Filter; import java.util.Collections; import java.util.Set; +import java.util.function.Predicate; import static org.labkey.api.cache.CacheType.DeterministicLRU; @@ -64,7 +64,7 @@ public void remove(K key) } @Override - public int removeUsingFilter(Filter filter) + public int removeUsingFilter(Predicate filter) { return 0; } diff --git a/api/src/org/labkey/api/cache/SimpleCache.java b/api/src/org/labkey/api/cache/SimpleCache.java index 7917aa7a6aa..becc409fd0c 100644 --- a/api/src/org/labkey/api/cache/SimpleCache.java +++ b/api/src/org/labkey/api/cache/SimpleCache.java @@ -16,15 +16,9 @@ package org.labkey.api.cache; import org.jetbrains.annotations.Nullable; -import org.labkey.api.util.Filter; import java.util.Set; - -/** - * User: adam - * Date: 12/25/11 - * Time: 8:11 PM - */ +import java.util.function.Predicate; // 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. @@ -40,10 +34,10 @@ public interface SimpleCache void remove(K key); /** - * Removes every element in the cache where filter.accept(K key) evaluates to true. + * Removes every element in the cache where filter.test(K key) evaluates to true. * Returns the number of elements that were removed. */ - int removeUsingFilter(Filter filter); + int removeUsingFilter(Predicate filter); Set getKeys(); diff --git a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java index b428518d21b..e16609ca658 100644 --- a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java +++ b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java @@ -23,17 +23,12 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.CacheType; import org.labkey.api.cache.SimpleCache; -import org.labkey.api.util.Filter; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Predicate; -/** -* User: adam -* Date: 12/25/11 -* Time: 8:17 PM -*/ class EhSimpleCache implements SimpleCache { private static final Logger LOG = LogManager.getLogger(EhSimpleCache.class); @@ -74,14 +69,14 @@ public void remove(@NotNull K key) } @Override - public int removeUsingFilter(Filter filter) + public int removeUsingFilter(Predicate filter) { int removes = 0; List keys = _cache.getKeys(); for (K key : keys) { - if (filter.accept(key)) + if (filter.test(key)) { remove(key); removes++; diff --git a/api/src/org/labkey/api/collections/SparseBitSet.java b/api/src/org/labkey/api/collections/SparseBitSet.java index 5d7512d4f58..f66d0047538 100644 --- a/api/src/org/labkey/api/collections/SparseBitSet.java +++ b/api/src/org/labkey/api/collections/SparseBitSet.java @@ -1,8 +1,8 @@ package org.labkey.api.collections; -/** - * copied from https://github.com/brettwooldridge/SparseBitSet - * apache license: https://github.com/brettwooldridge/SparseBitSet/blob/master/LICENSE +/* + copied from https://github.com/brettwooldridge/SparseBitSet + apache license: https://github.com/brettwooldridge/SparseBitSet/blob/master/LICENSE */ /*- This software is the work of Paladin Software International, Incorporated, @@ -164,7 +164,6 @@ when choosing to increase the size (see resize()). The only place where * 4 "levels". Respectively (from the least significant end), level4, the * address within word, the address within a level3 block, the address within * a level2 area, and the level1 address of that area within the set. - * * LEVEL4 is the number of bits of the level4 address (number of bits need * to address the bits in a long) */ @@ -702,9 +701,8 @@ private CopyStrategy _getCopyStrategy() public boolean equals(Object obj) { /* Sanity and quick checks. */ - if (!(obj instanceof SparseBitSet)) + if (!(obj instanceof SparseBitSet b)) return false; - final SparseBitSet b = (SparseBitSet) obj; if (this == b) return true; // Identity diff --git a/api/src/org/labkey/api/data/DatabaseCache.java b/api/src/org/labkey/api/data/DatabaseCache.java index 786d461c767..b9c0c367cfb 100644 --- a/api/src/org/labkey/api/data/DatabaseCache.java +++ b/api/src/org/labkey/api/data/DatabaseCache.java @@ -26,16 +26,15 @@ import org.labkey.api.cache.CacheLoader; import org.labkey.api.cache.CacheManager; import org.labkey.api.cache.TrackingCache; -import org.labkey.api.cache.TransactionCache; import org.labkey.api.cache.Wrapper; import org.labkey.api.data.DbScope.TransactionImpl; -import org.labkey.api.util.Filter; import org.labkey.api.util.logging.LogHelper; import java.sql.Connection; import java.sql.SQLException; import java.util.Objects; import java.util.Set; +import java.util.function.Predicate; /** * Implements a thread-safe, transaction-aware cache by deferring to a TransactionCache when transactions are in progress. @@ -54,13 +53,6 @@ private DatabaseCache(DbScope scope, int maxSize, long defaultTimeToLive, String _scope = scope; } - @Deprecated // Use the factory methods that return a BlockingDatabaseCache instead - public DatabaseCache(DbScope scope, int maxSize, String debugName) - { - // TODO: UNLIMITED default TTL seems aggressive, but that's what we've used for years... - this(scope, maxSize, CacheManager.UNLIMITED, debugName); - } - 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); @@ -68,7 +60,8 @@ public static BlockingCache get(DbScope scope, int maxSize, long de public static BlockingCache get(DbScope scope, int maxSize, String debugName, @Nullable CacheLoader cacheLoader) { - return new BlockingDatabaseCache<>(new DatabaseCache<>(scope, maxSize, debugName), cacheLoader); + // TODO: UNLIMITED default TTL seems aggressive, but that's what we've used for years... + return get(scope, maxSize, CacheManager.UNLIMITED, debugName, cacheLoader); } /** @@ -91,7 +84,7 @@ public BlockingDatabaseCache(DatabaseCache> cache, @Nullable Cache protected V load(@NotNull K key, @Nullable Object argument, CacheLoader loader) { V value = super.load(key, argument, loader); - LOG.debug("Just loaded: " + key + " (" + getTrackingCache().getDebugName() + ")"); + LOG.debug("Just loaded: {} ({})", key, getTrackingCache().getDebugName()); TransactionImpl t = _databaseCache.getCurrentTransaction(); // Only add post commit cache reload if there's a transaction and the transaction post commit queue is not @@ -99,10 +92,10 @@ protected V load(@NotNull K key, @Nullable Object argument, CacheLoader lo if (null != t) { // Create a new or get existing post commit counter task - CacheReloadCounterTask task = t.addCommitTask(new CacheReloadCounterTask(this) , DbScope.CommitTaskOption.POSTCOMMIT); - if( task.shouldAddReloadTask() ) + CacheReloadCounterTask task = t.addCommitTask(new CacheReloadCounterTask(this), DbScope.CommitTaskOption.POSTCOMMIT); + if (task.shouldAddReloadTask()) { - // Add reload task, if doesn't already exist, and decrement transaction reload counter + // Add reload task, if it doesn't already exist, and decrement transaction reload counter CacheReloadCommitTask cacheTask = new CacheReloadCommitTask<>(this, key, argument, loader); boolean alreadyQueued = (cacheTask != t.addCommitTask(cacheTask, DbScope.CommitTaskOption.POSTCOMMIT)); if (!alreadyQueued) @@ -113,6 +106,100 @@ protected V load(@NotNull K key, @Nullable Object argument, CacheLoader lo return value; } + // This is a post commit task that counts how many cache reload post commits have been queued and is + // scoped to the transaction. + private static class CacheReloadCounterTask implements Runnable + { + private final Cache _cache; + private int _remainingReloadCommitTasks; + + public CacheReloadCounterTask(Cache cache) + { + _cache = cache; + _remainingReloadCommitTasks = cache.getTrackingCache().getLimit(); + } + + @Override + public void run() + { + // noop + } + + @Override + public boolean equals(Object o) + { + if (o == null || getClass() != o.getClass()) return false; + CacheReloadCounterTask that = (CacheReloadCounterTask) o; + return Objects.equals(_cache, that._cache); + } + + @Override + public int hashCode() + { + return Objects.hashCode(_cache); + } + + public boolean shouldAddReloadTask() + { + return _remainingReloadCommitTasks > 0; + } + + public void decrementReloadCount() + { + _remainingReloadCommitTasks--; + } + } + + // This is added as a commit task when load operations take place inside a transaction, ensuring that they are + // re-played on successful commit. + private static class CacheReloadCommitTask implements Runnable + { + private final Cache _cache; + private final K _key; + private final Object _arg; + private final CacheLoader _loader; + + public CacheReloadCommitTask(Cache cache, K key, Object arg, CacheLoader loader) + { + _cache = cache; + _key = key; + _arg = arg; + _loader = loader; + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + @SuppressWarnings("unchecked") + CacheReloadCommitTask that = (CacheReloadCommitTask) o; + + if (!getCache().equals(that.getCache())) return false; + return Objects.equals(_key, that._key); + } + + protected Cache getCache() + { + return _cache; + } + + @Override + public int hashCode() + { + int result = getCache().hashCode(); + result = 31 * result + (_key != null ? _key.hashCode() : 0); + return result; + } + + @Override + public void run() + { + getCache().get(_key, _arg, _loader); + } + } + @Override public String toString() { @@ -200,7 +287,7 @@ public void clear() } @Override - public int removeUsingFilter(Filter filter) + public int removeUsingFilter(Predicate filter) { return getCache().removeUsingFilter(filter); } @@ -229,106 +316,13 @@ public String toString() return "DatabaseCache over \"" + _sharedCache.toString() + "\""; } - // This is a post commit task that counts how many cache reload post commits have been queued and is - // scoped to the transaction. - public static class CacheReloadCounterTask implements Runnable - { - private final Cache _cache; - private int _remainingReloadCommitTasks; - - public CacheReloadCounterTask(Cache cache) - { - _cache = cache; - _remainingReloadCommitTasks = cache.getTrackingCache().getLimit(); - } - - @Override - public void run() - { - // noop - } - - @Override - public boolean equals(Object o) - { - if (o == null || getClass() != o.getClass()) return false; - CacheReloadCounterTask that = (CacheReloadCounterTask) o; - return Objects.equals(_cache, that._cache); - } - - @Override - public int hashCode() - { - return Objects.hashCode(_cache); - } - - public boolean shouldAddReloadTask() - { - return _remainingReloadCommitTasks > 0; - } - - public void decrementReloadCount() - { - _remainingReloadCommitTasks--; - } - } - - // This is added as a commit task when load operations take place inside a transaction, ensuring that they are - // re-played on successful commit. - public static class CacheReloadCommitTask implements Runnable - { - private final Cache _cache; - private final K _key; - private final Object _arg; - private final CacheLoader _loader; - - public CacheReloadCommitTask(Cache cache, K key, Object arg, CacheLoader loader) - { - _cache = cache; - _key = key; - _arg = arg; - _loader = loader; - } - - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - CacheReloadCommitTask that = (CacheReloadCommitTask) o; - - if (!getCache().equals(that.getCache())) return false; - return Objects.equals(_key, that._key); - } - - protected Cache getCache() - { - return _cache; - } - - @Override - public int hashCode() - { - int result = getCache().hashCode(); - result = 31 * result + (_key != null ? _key.hashCode() : 0); - return result; - } - - @Override - public void run() - { - getCache().get(_key, _arg, _loader); - } - } - public static class TestCase extends Assert { public static class TempDatabaseCache extends DatabaseCache { public TempDatabaseCache(DbScope scope, int maxSize, String debugName) { - super(scope, maxSize, debugName); + super(scope, maxSize, CacheManager.UNLIMITED, debugName); } // Shared cache needs to be a temporary cache, otherwise we'll leak a cache on every invocation because of KNOWN_CACHES @@ -381,7 +375,7 @@ public void testDatabaseCache() // 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, correctCount, 20); + case DeterministicLRU -> assertEquals("Count was " + correctCount, 20, correctCount); case NonDeterministicLRU -> assertTrue("Count was " + correctCount, correctCount > 11); default -> fail("Unknown cache type"); } @@ -405,7 +399,7 @@ public void testDatabaseCache() // As above, this test isn't very useful for a NonDeterministicLRU cache. switch (trackingCache.getCacheType()) { - case DeterministicLRU -> assertEquals("Count was " + correctCount, correctCount, 10); + case DeterministicLRU -> assertEquals("Count was " + correctCount, 10, correctCount); case NonDeterministicLRU -> { assertTrue("Count was " + correctCount, correctCount > 4); @@ -420,22 +414,62 @@ public void testDatabaseCache() 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; diff --git a/api/src/org/labkey/api/cache/TransactionCache.java b/api/src/org/labkey/api/data/TransactionCache.java similarity index 88% rename from api/src/org/labkey/api/cache/TransactionCache.java rename to api/src/org/labkey/api/data/TransactionCache.java index ad77280cd9f..a875ccee540 100644 --- a/api/src/org/labkey/api/cache/TransactionCache.java +++ b/api/src/org/labkey/api/data/TransactionCache.java @@ -1,281 +1,277 @@ -/* - * Copyright (c) 2012-2019 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.apache.commons.lang3.mutable.MutableInt; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.DatabaseCache; -import org.labkey.api.data.DbScope; -import org.labkey.api.data.DbScope.Transaction; -import org.labkey.api.util.Filter; - -import java.util.Objects; -import java.util.Set; - -/** - * A read-through, transaction-specific cache. Reads through to the shared cache for each entry until a write operation - * (put or remove) occurs on that entry, at which point only the private cache is consulted for this entry for the - * remainder of the transaction. This should provide good performance while avoiding pollution of the shared cache - * during the transaction and in the case of a rollback. On successful commit, remove and clear operations are replayed - * into the shared cache. Using a BlockingDatabaseCache ensures that load() operations are also replayed. - */ -public class TransactionCache implements Cache -{ - /** Need our own markers so we can distinguish missing vs. cached miss and missing vs. removed */ - @SuppressWarnings("unchecked") - private final V NULL_MARKER = (V)new Object(); - @SuppressWarnings("unchecked") - private final V REMOVED_MARKER = (V)new Object(); - - /** Cache shared by other threads */ - private final Cache _sharedCache; - private final DatabaseCache _databaseCache; - private final Transaction _transaction; - - /** Our own private, transaction-specific cache, which may contain database changes that have not yet been committed */ - private final Cache _privateCache; - - /** Whether the cache has been cleared. Once clear() is invoked, the shared cache is ignored. */ - private boolean _hasBeenCleared = false; - - public TransactionCache(Cache sharedCache, Cache privateCache, DatabaseCache databaseCache, Transaction transaction) - { - _privateCache = privateCache; - _sharedCache = sharedCache; - _databaseCache = databaseCache; - _transaction = transaction; - } - - @Override - public V get(@NotNull K key) - { - return get(key, null, null); - } - - @Override - public V get(@NotNull K key, Object arg, @Nullable CacheLoader loader) - { - // No locks or synchronization below because this code is always single-threaded (unlike BlockingCache) - - V v = _privateCache.get(key); - - if (v == REMOVED_MARKER) - { - v = null; // Entry has been removed from private cache, so treat as missing - } - else if (null == v && !_hasBeenCleared) - { - try - { - // Entry has not been modified in the private cache, so read through to shared cache - v = _sharedCache.get(key, null, (key1, argument) -> { - throw new MissingCacheEntryException(); - }); - // Shared cache has an entry for this key, so return it and don't invoke the loader - if (null == v) - v = NULL_MARKER; // Cached miss in shared cache; use null marker to skip loading. Issue 47234 - } - catch (MissingCacheEntryException e) - { - // Missing from private & shared cache; fall through to private cache load/put - } - } - - // If removed/cleared from private cache or missing from both caches, attempt to load and put into private cache - if (null == v && null != loader) - { - v = loader.load(key, arg); - put(key, v); - } - - return v == NULL_MARKER ? null : v; - } - - @Override - public void put(@NotNull K key, V value) - { - if (null == value) - value = NULL_MARKER; - _privateCache.put(key, value); - } - - @Override - public void put(@NotNull K key, V value, long timeToLive) - { - if (null == value) - value = NULL_MARKER; - _privateCache.put(key, value, timeToLive); - } - - @Override - public void remove(@NotNull K key) - { - _transaction.addCommitTask(new CacheKeyRemovalCommitTask(key), DbScope.CommitTaskOption.POSTCOMMIT); - _privateCache.put(key, REMOVED_MARKER); - } - - @Override - public int removeUsingFilter(Filter filter) - { - _transaction.addCommitTask(new CachePrefixRemovalCommitTask(filter), DbScope.CommitTaskOption.POSTCOMMIT); - MutableInt count = new MutableInt(0); - _privateCache.getKeys().stream() - .filter(filter::accept) - .forEach(key -> { - remove(key); - count.increment(); - }); - _sharedCache.getKeys().stream() - .filter(filter::accept) - .forEach(key -> { - remove(key); - count.increment(); - }); - return count.intValue(); - } - - @Override - public Set getKeys() - { - throw new UnsupportedOperationException("getKeys() is not implemented"); - } - - @Override - public void clear() - { - _transaction.addCommitTask(new CacheClearingCommitTask(), DbScope.CommitTaskOption.POSTCOMMIT); - _hasBeenCleared = true; - _privateCache.clear(); - } - - @Override - public void close() - { - _privateCache.close(); - } - - @Override - public TrackingCache getTrackingCache() - { - return _privateCache.getTrackingCache(); - } - - @Override - public Cache createTemporaryCache() - { - throw new UnsupportedOperationException(); - } - - private class CacheClearingCommitTask implements Runnable - { - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - CacheClearingCommitTask that = (CacheClearingCommitTask) o; - - return getCache().equals(that.getCache()); - } - - private Cache getCache() - { - return _databaseCache.getCache(); - } - - @Override - public int hashCode() - { - return getCache().hashCode(); - } - - @Override - public void run() - { - getCache().clear(); - } - } - - private abstract class AbstractCacheRemovalCommitTask implements Runnable - { - protected final ObjectType object; - - public AbstractCacheRemovalCommitTask(ObjectType object) - { - this.object = object; - } - - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - AbstractCacheRemovalCommitTask that = (AbstractCacheRemovalCommitTask) o; - - if (!getCache().equals(that.getCache())) return false; - return Objects.equals(object, that.object); - } - - protected Cache getCache() - { - return _databaseCache.getCache(); - } - - @Override - public int hashCode() - { - int result = getCache().hashCode(); - result = 31 * result + (object != null ? object.hashCode() : 0); - return result; - } - } - - private class CacheKeyRemovalCommitTask extends AbstractCacheRemovalCommitTask - { - public CacheKeyRemovalCommitTask(K key) - { - super(key); - } - - @Override - public void run() - { - getCache().remove(object); - } - } - - private class CachePrefixRemovalCommitTask extends AbstractCacheRemovalCommitTask> - { - public CachePrefixRemovalCommitTask(Filter filter) - { - super(filter); - } - - @Override - public void run() - { - getCache().removeUsingFilter(object); - } - } - - private static class MissingCacheEntryException extends RuntimeException - { - } -} +/* + * Copyright (c) 2012-2019 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.data; + +import com.google.common.collect.Sets; +import org.apache.commons.lang3.mutable.MutableInt; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.Cache; +import org.labkey.api.cache.CacheLoader; +import org.labkey.api.cache.TrackingCache; +import org.labkey.api.data.DbScope.Transaction; + +import java.util.Objects; +import java.util.Set; +import java.util.function.Predicate; + +/** + * A read-through, transaction-specific cache. Reads through to the shared cache for each entry until a write operation + * (put or remove) occurs on that entry, at which point only the private cache is consulted for this entry for the + * remainder of the transaction. This should provide good performance while avoiding pollution of the shared cache + * during the transaction and in the case of a rollback. On successful commit, remove and clear operations are replayed + * into the shared cache. Using a BlockingDatabaseCache ensures that load() operations are also replayed. + */ +public class TransactionCache implements Cache +{ + /** Need our own markers so we can distinguish missing vs. cached miss and missing vs. removed */ + @SuppressWarnings("unchecked") + private final V NULL_MARKER = (V)new Object(); + @SuppressWarnings("unchecked") + private final V REMOVED_MARKER = (V)new Object(); + + /** Cache shared by other threads */ + final Cache _sharedCache; + private final DatabaseCache _databaseCache; + private final Transaction _transaction; + + /** Our own private, transaction-specific cache, which may contain database changes that have not yet been committed */ + final Cache _privateCache; + + /** Whether the cache has been cleared. Once clear() is invoked, the shared cache is ignored. */ + private boolean _hasBeenCleared = false; + + public TransactionCache(Cache sharedCache, Cache privateCache, DatabaseCache databaseCache, Transaction transaction) + { + _privateCache = privateCache; + _sharedCache = sharedCache; + _databaseCache = databaseCache; + _transaction = transaction; + } + + @Override + public V get(@NotNull K key) + { + return get(key, null, null); + } + + @Override + public V get(@NotNull K key, Object arg, @Nullable CacheLoader loader) + { + // No locks or synchronization below because this code is always single-threaded (unlike BlockingCache) + + V v = _privateCache.get(key); + + if (v == REMOVED_MARKER) + { + v = null; // Entry has been removed from private cache, so treat as missing + } + else if (null == v && !_hasBeenCleared) + { + try + { + // Entry has not been modified in the private cache, so read through to shared cache + v = _sharedCache.get(key, null, (key1, argument) -> { + throw new MissingCacheEntryException(); + }); + // Shared cache has an entry for this key, so return it and don't invoke the loader + if (null == v) + v = NULL_MARKER; // Cached miss in shared cache; use null marker to skip loading. Issue 47234 + } + catch (MissingCacheEntryException e) + { + // Missing from private & shared cache; fall through to private cache load/put + } + } + + // If removed/cleared from private cache or missing from both caches, attempt to load and put into private cache + if (null == v && null != loader) + { + v = loader.load(key, arg); + put(key, v); + } + + return v == NULL_MARKER ? null : v; + } + + @Override + public void put(@NotNull K key, V value) + { + if (null == value) + value = NULL_MARKER; + _privateCache.put(key, value); + } + + @Override + public void put(@NotNull K key, V value, long timeToLive) + { + if (null == value) + value = NULL_MARKER; + _privateCache.put(key, value, timeToLive); + } + + @Override + public void remove(@NotNull K key) + { + _transaction.addCommitTask(new CacheKeyRemovalCommitTask(key), DbScope.CommitTaskOption.POSTCOMMIT); + _privateCache.put(key, REMOVED_MARKER); + } + + @Override + public int removeUsingFilter(Predicate filter) + { + _transaction.addCommitTask(new CachePrefixRemovalCommitTask(filter), DbScope.CommitTaskOption.POSTCOMMIT); + MutableInt count = new MutableInt(0); + getKeys().stream() + .filter(filter) + .forEach(key -> { + remove(key); + count.increment(); + }); + return count.intValue(); + } + + @Override + public Set getKeys() + { + return Sets.union(_privateCache.getKeys(), _sharedCache.getKeys()); + } + + @Override + public void clear() + { + _transaction.addCommitTask(new CacheClearingCommitTask(), DbScope.CommitTaskOption.POSTCOMMIT); + _hasBeenCleared = true; + _privateCache.clear(); + } + + @Override + public void close() + { + _privateCache.close(); + } + + @Override + public TrackingCache getTrackingCache() + { + return _privateCache.getTrackingCache(); + } + + @Override + public Cache createTemporaryCache() + { + throw new UnsupportedOperationException(); + } + + private class CacheClearingCommitTask implements Runnable + { + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + CacheClearingCommitTask that = (CacheClearingCommitTask) o; + + return getCache().equals(that.getCache()); + } + + private Cache getCache() + { + return _databaseCache.getCache(); + } + + @Override + public int hashCode() + { + return getCache().hashCode(); + } + + @Override + public void run() + { + getCache().clear(); + } + } + + private abstract class AbstractCacheRemovalCommitTask implements Runnable + { + protected final ObjectType object; + + public AbstractCacheRemovalCommitTask(ObjectType object) + { + this.object = object; + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + AbstractCacheRemovalCommitTask that = (AbstractCacheRemovalCommitTask) o; + + if (!getCache().equals(that.getCache())) return false; + return Objects.equals(object, that.object); + } + + protected Cache getCache() + { + return _databaseCache.getCache(); + } + + @Override + public int hashCode() + { + int result = getCache().hashCode(); + result = 31 * result + (object != null ? object.hashCode() : 0); + return result; + } + } + + private class CacheKeyRemovalCommitTask extends AbstractCacheRemovalCommitTask + { + public CacheKeyRemovalCommitTask(K key) + { + super(key); + } + + @Override + public void run() + { + getCache().remove(object); + } + } + + private class CachePrefixRemovalCommitTask extends AbstractCacheRemovalCommitTask> + { + public CachePrefixRemovalCommitTask(Predicate filter) + { + super(filter); + } + + @Override + public void run() + { + getCache().removeUsingFilter(object); + } + } + + private static class MissingCacheEntryException extends RuntimeException + { + } +} diff --git a/api/src/org/labkey/api/iterator/CloseableFilteredIterator.java b/api/src/org/labkey/api/iterator/CloseableFilteredIterator.java index 753e8ad863d..a980ad3bab5 100644 --- a/api/src/org/labkey/api/iterator/CloseableFilteredIterator.java +++ b/api/src/org/labkey/api/iterator/CloseableFilteredIterator.java @@ -15,20 +15,17 @@ */ package org.labkey.api.iterator; -import org.labkey.api.util.Filter; - import java.io.IOException; +import java.util.function.Predicate; /** * Combination of {@link CloseableIterator} and {@link FilteredIterator} - * User: adam - * Date: Aug 20, 2010 */ public class CloseableFilteredIterator extends FilteredIterator implements CloseableIterator { private final CloseableIterator _iter; - public CloseableFilteredIterator(CloseableIterator iter, Filter filter) + public CloseableFilteredIterator(CloseableIterator iter, Predicate filter) { super(iter, filter); _iter = iter; diff --git a/api/src/org/labkey/api/iterator/FilteredIterator.java b/api/src/org/labkey/api/iterator/FilteredIterator.java index 9dbc2f998cc..d5ac42d84c8 100644 --- a/api/src/org/labkey/api/iterator/FilteredIterator.java +++ b/api/src/org/labkey/api/iterator/FilteredIterator.java @@ -15,23 +15,20 @@ */ package org.labkey.api.iterator; -import org.labkey.api.util.Filter; - import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.function.Predicate; /** * Wrapper over some other Iterator that filters out certain objects. - * User: adam - * Date: Apr 10, 2009 */ public class FilteredIterator implements Iterator { private final Iterator _iterator; - private final Filter _filter; + private final Predicate _filter; private T _next; - public FilteredIterator(Iterator iterator, Filter filter) + public FilteredIterator(Iterator iterator, Predicate filter) { _iterator = iterator; _filter = filter; @@ -69,7 +66,7 @@ private void toNext() while (_iterator.hasNext()) { T item = _iterator.next(); - if (item != null && _filter.accept(item)) + if (item != null && _filter.test(item)) { _next = item; break; diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index b724b5d5514..a68487682e9 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -48,7 +48,6 @@ import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.ValidationException; import org.labkey.api.util.ExceptionUtil; -import org.labkey.api.util.Filter; import org.labkey.api.util.SimpleTime; import org.labkey.api.util.StringUtilsLabKey; @@ -65,6 +64,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -112,7 +112,7 @@ public static DataLoaderService get() // true if the results can be scrolled by the DataIterator created in .getDataIterator() protected Boolean _scrollable = null; protected boolean _preserveEmptyString = false; - private Filter> _mapFilter = null; + private Predicate> _mapFilter = null; protected DataLoader() { @@ -482,7 +482,7 @@ public void setScanAheadLineCount(int count) } // This is convenient when configuring a DataLoader that is returned or passed to other code - public void setMapFilter(Filter> mapFilter) + public void setMapFilter(Predicate> mapFilter) { _mapFilter = mapFilter; } diff --git a/api/src/org/labkey/api/study/SpecimenImportStrategy.java b/api/src/org/labkey/api/study/SpecimenImportStrategy.java deleted file mode 100644 index d405fd5cef5..00000000000 --- a/api/src/org/labkey/api/study/SpecimenImportStrategy.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2013 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.study; - -import org.labkey.api.data.Filter; - -import java.io.Closeable; -import java.util.Map; - -/* -* User: adam -* Date: Feb 13, 2013 -* Time: 2:02:57 PM -*/ -public interface SpecimenImportStrategy extends Closeable -{ - org.labkey.api.util.Filter> getImportFilter(); - Filter getDeleteFilter(); - Filter getInsertFilter(); -} \ No newline at end of file diff --git a/api/src/org/labkey/api/study/SpecimenImportStrategyFactory.java b/api/src/org/labkey/api/study/SpecimenImportStrategyFactory.java deleted file mode 100644 index 77d281c09fa..00000000000 --- a/api/src/org/labkey/api/study/SpecimenImportStrategyFactory.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2013 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.study; - -import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.Container; -import org.labkey.api.data.DbSchema; -import org.labkey.api.writer.VirtualFile; - -/* -* User: adam -* Date: Feb 13, 2013 -* Time: 2:09:06 PM -*/ -public interface SpecimenImportStrategyFactory -{ - // Returns a SpecimenImportStrategy if this factory claims the current file, otherwise returns null - @Nullable - SpecimenImportStrategy get(DbSchema schema, Container c, VirtualFile dir, String fileName); // TODO: Other context! Logger? -} diff --git a/api/src/org/labkey/api/study/SpecimenService.java b/api/src/org/labkey/api/study/SpecimenService.java index 4e8661e33ba..e87261f302d 100644 --- a/api/src/org/labkey/api/study/SpecimenService.java +++ b/api/src/org/labkey/api/study/SpecimenService.java @@ -76,10 +76,6 @@ static SpecimenService get() void importSpecimens(User user, Container container, List> rows, boolean merge) throws SQLException, IOException, ValidationException; - void registerSpecimenImportStrategyFactory(SpecimenImportStrategyFactory factory); - - Collection getSpecimenImportStrategyFactories(); - void registerSpecimenTransform(SpecimenTransform transform); Collection getSpecimenTransforms(Container container); diff --git a/api/src/org/labkey/api/util/Filter.java b/api/src/org/labkey/api/util/Filter.java deleted file mode 100644 index 2669aca0a6e..00000000000 --- a/api/src/org/labkey/api/util/Filter.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright (c) 2009-2017 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.util; - -/** - * User: adam - * Date: Apr 10, 2009 - * Time: 9:05:45 AM - */ -// TODO: Eliminate this in favor of Predicate? -public interface Filter -{ - boolean accept(T object); -} diff --git a/api/src/org/labkey/api/webdav/WebdavResolverImpl.java b/api/src/org/labkey/api/webdav/WebdavResolverImpl.java index bea84a285f7..cedb04c222f 100644 --- a/api/src/org/labkey/api/webdav/WebdavResolverImpl.java +++ b/api/src/org/labkey/api/webdav/WebdavResolverImpl.java @@ -33,7 +33,6 @@ import org.labkey.api.security.roles.NoPermissionsRole; import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.util.FileUtil; -import org.labkey.api.util.Filter; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.Path; @@ -116,14 +115,8 @@ protected void invalidate(Path containerPath, boolean recursive) final Path path = getRootPath().append(containerPath); _folderCache.remove(path); if (recursive) - _folderCache.removeUsingFilter(new Filter() { - @Override - public boolean accept(Path test) - { - return test.startsWith(path); - } - }); - if (containerPath.size() == 0) + _folderCache.removeUsingFilter(test -> test.startsWith(path)); + if (containerPath.isEmpty()) { synchronized (WebdavResolverImpl.this) { diff --git a/query/src/org/labkey/query/olap/MemberSet.java b/query/src/org/labkey/query/olap/MemberSet.java index b0553353abb..b73e84bb90a 100644 --- a/query/src/org/labkey/query/olap/MemberSet.java +++ b/query/src/org/labkey/query/olap/MemberSet.java @@ -100,6 +100,7 @@ public void seal() } + @Override public boolean isSealed() { return sealed; @@ -152,7 +153,7 @@ public MemberSet onlyFor(Level l) { LevelMemberSet s = levelMap.get(l.getUniqueName()); if (null == s) - return new MemberSet(l, new HashSet()); + return new MemberSet(l, new HashSet<>()); return new MemberSet(s); } @@ -308,7 +309,7 @@ Level getLevel() @Nullable Hierarchy getHierarchy() { - if (levelMap.size() == 0) + if (levelMap.isEmpty()) return null; Hierarchy h = null; for (LevelMemberSet l : levelMap.values()) @@ -336,8 +337,7 @@ public Iterator iterator() } // sort levels by depth - List list = new ArrayList<>(); - list.addAll(levelMap.values()); + List list = new ArrayList<>(levelMap.values()); list.sort((s1, s2) -> s2._level.getDepth() - s1._level.getDepth()); List> iterators = new ArrayList<>(); for (LevelMemberSet s : list) @@ -371,7 +371,7 @@ public boolean addAll(MemberSet from) @Override - public boolean addAll(Collection c) + public boolean addAll(@NotNull Collection c) { if (c instanceof MemberSet) return addAll((MemberSet)c); @@ -382,7 +382,7 @@ public boolean addAll(Collection c) @Override - public boolean retainAll(Collection c) + public boolean retainAll(@NotNull Collection c) { if (c instanceof MemberSet) { @@ -432,9 +432,8 @@ public boolean isEmpty() @Override public boolean contains(Object o) { - if (!(o instanceof Member)) + if (!(o instanceof Member m)) return false; - Member m = (Member)o; LevelMemberSet s = levelMap.get(m.getLevel().getUniqueName()); return s != null && s.contains(m); } @@ -457,9 +456,8 @@ public boolean add(Member member) @Override public boolean remove(Object o) { - if (!(o instanceof Member)) + if (!(o instanceof Member m)) return false; - Member m = (Member)o; LevelMemberSet s = levelMap.get(m.getLevel().getUniqueName()); return s != null && s.remove(m); } @@ -491,9 +489,9 @@ public boolean containsAny(MemberSet other) - /** inner implmentation for a members of a single level, with natural ordering/ordinality */ + /** inner implementation for a members of a single level, with natural ordering/ordinality */ - private class LevelMemberSet implements Set + private static class LevelMemberSet implements Set { Level _level; final SparseBitSet _set; @@ -575,9 +573,8 @@ public boolean isEmpty() @Override public boolean contains(Object o) { - if (!(o instanceof Member)) + if (!(o instanceof Member m)) return false; - Member m = (Member)o; if (m.getLevel() != _level) return false; return _set.get(m.getOrdinal()); @@ -590,16 +587,14 @@ public Iterator iterator() return new _Iterator(); } - @NotNull @Override - public Object[] toArray() + public @NotNull Object[] toArray() { throw new UnsupportedOperationException(); } - @NotNull @Override - public T[] toArray(T[] a) + public @NotNull T[] toArray(T @NotNull [] a) { throw new UnsupportedOperationException(); } @@ -621,9 +616,8 @@ public boolean add(Member member) @Override public boolean remove(Object o) { - if (!(o instanceof Member)) + if (!(o instanceof Member m)) return false; - Member m = (Member)o; if (m.getLevel() != _level) return false; boolean ret = _set.get(m.getOrdinal()); @@ -632,7 +626,7 @@ public boolean remove(Object o) } @Override - public boolean containsAll(Collection c) + public boolean containsAll(@NotNull Collection c) { throw new UnsupportedOperationException(); } @@ -647,7 +641,7 @@ public boolean addAll(Collection c) @Override - public boolean retainAll(Collection c) + public boolean retainAll(@NotNull Collection c) { if (c instanceof LevelMemberSet) return retainAll((LevelMemberSet)c); @@ -663,7 +657,7 @@ public boolean retainAll(LevelMemberSet from) @Override - public boolean removeAll(Collection c) + public boolean removeAll(@NotNull Collection c) { throw new UnsupportedOperationException(); } @@ -782,15 +776,12 @@ static class MockLevelProxy extends MockOlapProxy @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - switch (method.getName()) + return switch (method.getName()) { - case "getHierarchy": - return getHierarchy(); - case "getMembers": - return getMembers(proxy); - default: - return super.invoke(proxy, method, args); - } + case "getHierarchy" -> getHierarchy(); + case "getMembers" -> getMembers(proxy); + default -> super.invoke(proxy, method, args); + }; } Hierarchy getHierarchy() { @@ -798,7 +789,7 @@ Hierarchy getHierarchy() } List getMembers(Object proxy) { - ArrayList ret = new ArrayList(memberNames.length); + ArrayList ret = new ArrayList<>(memberNames.length); for (int i=0 ; i getLevel(); + case "getHierarchy" -> getHierarchy(); + default -> super.invoke(proxy, method, args); + }; } Hierarchy getHierarchy() diff --git a/specimen/src/org/labkey/specimen/SpecimenModule.java b/specimen/src/org/labkey/specimen/SpecimenModule.java index 74e052e11e6..e3bb95b5d27 100644 --- a/specimen/src/org/labkey/specimen/SpecimenModule.java +++ b/specimen/src/org/labkey/specimen/SpecimenModule.java @@ -68,7 +68,6 @@ import org.labkey.specimen.actions.SpecimenApiController; import org.labkey.specimen.actions.SpecimenController; import org.labkey.specimen.importer.AbstractSpecimenTask; -import org.labkey.specimen.importer.DefaultSpecimenImportStrategyFactory; import org.labkey.specimen.importer.RequestabilityManager; import org.labkey.specimen.importer.SimpleSpecimenImporter; import org.labkey.specimen.importer.SpecimenImporter; @@ -238,7 +237,6 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) ContainerManager.addContainerListener(new SpecimenRequestContainerListener()); StudyService.get().registerStudyTabProvider(tabs -> tabs.add(new SpecimensPage("Specimen Data"))); - SpecimenService.get().registerSpecimenImportStrategyFactory(new DefaultSpecimenImportStrategyFactory()); AuditLogService.get().registerAuditType(new SpecimenCommentAuditProvider()); PipelineService.get().registerPipelineProvider(new SpecimenPipeline(this)); StudyInternalService.get().registerManageStudyViewFactory(ctx -> ctx.getContainer().hasActiveModuleByName("specimen") ? new ManageSpecimenView() : null); diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index 38d50984089..0f136e247e5 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -46,7 +46,6 @@ import org.labkey.api.specimen.model.SpecimenTablesProvider; import org.labkey.api.study.ParticipantVisit; import org.labkey.api.study.SpecimenChangeListener; -import org.labkey.api.study.SpecimenImportStrategyFactory; import org.labkey.api.study.SpecimenService; import org.labkey.api.study.SpecimenTablesTemplate; import org.labkey.api.study.SpecimenTransform; @@ -75,7 +74,6 @@ public class SpecimenServiceImpl implements SpecimenService { - private final List _importStrategyFactories = new CopyOnWriteArrayList<>(); private final Map _specimenTransformMap = new ConcurrentHashMap<>(); private final List _changeListeners = new CopyOnWriteArrayList<>(); @@ -330,19 +328,6 @@ public void importSpecimens(User user, Container container, List getSpecimenImportStrategyFactories() - { - return _importStrategyFactories; - } - @Override public void registerSpecimenTransform(SpecimenTransform transform) { diff --git a/specimen/src/org/labkey/specimen/importer/DefaultSpecimenImportStrategyFactory.java b/specimen/src/org/labkey/specimen/importer/DefaultSpecimenImportStrategyFactory.java deleted file mode 100644 index bb9ad140fb2..00000000000 --- a/specimen/src/org/labkey/specimen/importer/DefaultSpecimenImportStrategyFactory.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (c) 2013 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.specimen.importer; - -import org.labkey.api.data.Container; -import org.labkey.api.data.DbSchema; -import org.labkey.api.study.SpecimenImportStrategy; -import org.labkey.api.study.SpecimenImportStrategyFactory; -import org.labkey.api.writer.VirtualFile; - -/* -* User: adam -* Date: Feb 13, 2013 -* Time: 2:28:23 PM -*/ -public class DefaultSpecimenImportStrategyFactory implements SpecimenImportStrategyFactory -{ - @Override - public SpecimenImportStrategy get(DbSchema schema, Container c, VirtualFile dir, String fileName) - { - return new StandardSpecimenImportStrategy(c); - } -} diff --git a/specimen/src/org/labkey/specimen/importer/EditableSpecimenImporter.java b/specimen/src/org/labkey/specimen/importer/EditableSpecimenImporter.java index 6f475eb1313..037989a9049 100644 --- a/specimen/src/org/labkey/specimen/importer/EditableSpecimenImporter.java +++ b/specimen/src/org/labkey/specimen/importer/EditableSpecimenImporter.java @@ -31,7 +31,6 @@ import org.labkey.api.specimen.importer.RollupHelper; import org.labkey.api.specimen.importer.RollupHelper.RollupMap; import org.labkey.api.specimen.importer.RollupInstance; -import org.labkey.api.study.SpecimenImportStrategy; import org.labkey.specimen.SpecimenColumns; import org.labkey.specimen.SpecimenRequestManager; @@ -89,9 +88,8 @@ private void _process(List> rows, boolean merge, Logger logg specimenRows.add(specimenRow); } - SpecimenImportStrategy strategy = new StandardSpecimenImportStrategy(getContainer()); Map sifMap = new HashMap<>(); - addSpecimenImportFile(sifMap, _specimensTableType, specimenRows, strategy); + addSpecimenImportFile(sifMap, _specimensTableType, specimenRows); try { @@ -105,11 +103,10 @@ private void _process(List> rows, boolean merge, Logger logg } } - - private void addSpecimenImportFile(Map fileNameMap, SpecimenTableType type, List> specimenRows, SpecimenImportStrategy strategy) + private void addSpecimenImportFile(Map fileNameMap, SpecimenTableType type, List> specimenRows) { assert null != type : "Unknown type!"; - fileNameMap.put(type, new IteratorSpecimenImportFile(specimenRows, strategy, type)); + fileNameMap.put(type, new IteratorSpecimenImportFile(specimenRows, type)); } public List> mapColumnNamesToTsvColumnNames(List> rows) diff --git a/specimen/src/org/labkey/specimen/importer/FileSystemSpecimenImportFile.java b/specimen/src/org/labkey/specimen/importer/FileSystemSpecimenImportFile.java index 711187ba915..9ca6c21e4e9 100644 --- a/specimen/src/org/labkey/specimen/importer/FileSystemSpecimenImportFile.java +++ b/specimen/src/org/labkey/specimen/importer/FileSystemSpecimenImportFile.java @@ -18,32 +18,22 @@ import org.labkey.api.reader.Readers; import org.labkey.api.reader.TabLoader; -import org.labkey.api.study.SpecimenImportStrategy; -import org.labkey.api.util.Filter; import org.labkey.api.writer.VirtualFile; import java.io.IOException; import java.io.InputStream; import java.io.Reader; -import java.util.Map; -/* -* User: adam -* Date: Feb 10, 2013 -* Time: 5:16:12 PM -*/ public class FileSystemSpecimenImportFile implements SpecimenImportFile { private final VirtualFile _dir; private final String _fileName; - private final SpecimenImportStrategy _strategy; private final SpecimenTableType _tableType; - public FileSystemSpecimenImportFile(VirtualFile dir, String fileName, SpecimenImportStrategy strategy, SpecimenTableType tableType) + public FileSystemSpecimenImportFile(VirtualFile dir, String fileName, SpecimenTableType tableType) { _dir = dir; _fileName = fileName; - _strategy = strategy; _tableType = tableType; } @@ -53,12 +43,6 @@ public SpecimenTableType getTableType() return _tableType; } - @Override - public SpecimenImportStrategy getStrategy() - { - return _strategy; - } - @Override public TabLoader getDataLoader() throws IOException { @@ -66,11 +50,6 @@ public TabLoader getDataLoader() throws IOException TabLoader loader = new TabLoader(reader, true, null, true); // Close on complete loader.setInferTypes(false); - Filter> filter = getStrategy().getImportFilter(); - - if (null != filter) - loader.setMapFilter(filter); - return loader; } diff --git a/specimen/src/org/labkey/specimen/importer/IteratorSpecimenImportFile.java b/specimen/src/org/labkey/specimen/importer/IteratorSpecimenImportFile.java index 97c9de6a76c..b217abd32d7 100644 --- a/specimen/src/org/labkey/specimen/importer/IteratorSpecimenImportFile.java +++ b/specimen/src/org/labkey/specimen/importer/IteratorSpecimenImportFile.java @@ -18,25 +18,17 @@ import org.labkey.api.reader.DataLoader; import org.labkey.api.reader.MapLoader; -import org.labkey.api.study.SpecimenImportStrategy; import java.util.List; import java.util.Map; -/* -* User: adam -* Date: Feb 16, 2013 -* Time: 7:35:23 AM -*/ public class IteratorSpecimenImportFile implements SpecimenImportFile { - private final SpecimenImportStrategy _strategy; private final List> _rows; private final SpecimenTableType _tableType; - public IteratorSpecimenImportFile(List> rows, SpecimenImportStrategy strategy, SpecimenTableType tableType) + public IteratorSpecimenImportFile(List> rows, SpecimenTableType tableType) { - _strategy = strategy; _rows = rows; _tableType = tableType; } @@ -47,12 +39,6 @@ public SpecimenTableType getTableType() return _tableType; } - @Override - public SpecimenImportStrategy getStrategy() - { - return _strategy; - } - @Override public DataLoader getDataLoader() { diff --git a/specimen/src/org/labkey/specimen/importer/SimpleSpecimenImporter.java b/specimen/src/org/labkey/specimen/importer/SimpleSpecimenImporter.java index 2ae5d34520f..09d4f5f5b1d 100644 --- a/specimen/src/org/labkey/specimen/importer/SimpleSpecimenImporter.java +++ b/specimen/src/org/labkey/specimen/importer/SimpleSpecimenImporter.java @@ -33,7 +33,6 @@ import org.labkey.api.reader.TabLoader; import org.labkey.api.security.User; import org.labkey.api.specimen.SpecimenSchema; -import org.labkey.api.study.SpecimenImportStrategy; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; import org.labkey.api.study.StudyUtils; @@ -226,11 +225,10 @@ public void process(List> rows, boolean merge) throws IOExce specimenRows.add(specimenRow); } - SpecimenImportStrategy strategy = new StandardSpecimenImportStrategy(container); Map sifMap = new HashMap<>(); - addSpecimenImportFile(sifMap, _specimensTableType, specimenRows, strategy); + addSpecimenImportFile(sifMap, _specimensTableType, specimenRows); for (LookupTable lookupTable : lookupTables.values()) - addSpecimenImportFile(sifMap, lookupTable.getTableType(), lookupTable.toMaps(), strategy); + addSpecimenImportFile(sifMap, lookupTable.getTableType(), lookupTable.toMaps()); try { @@ -250,10 +248,10 @@ public void process(List> rows, boolean merge) throws IOExce } - private void addSpecimenImportFile(Map fileNameMap, SpecimenTableType type, List> specimenRows, SpecimenImportStrategy strategy) + private void addSpecimenImportFile(Map fileNameMap, SpecimenTableType type, List> specimenRows) { assert null != type : "Unknown type!"; - fileNameMap.put(type, new IteratorSpecimenImportFile(specimenRows, strategy, type)); + fileNameMap.put(type, new IteratorSpecimenImportFile(specimenRows, type)); } diff --git a/specimen/src/org/labkey/specimen/importer/SpecimenImportFile.java b/specimen/src/org/labkey/specimen/importer/SpecimenImportFile.java index 8102249c186..3ea37386ee5 100644 --- a/specimen/src/org/labkey/specimen/importer/SpecimenImportFile.java +++ b/specimen/src/org/labkey/specimen/importer/SpecimenImportFile.java @@ -17,18 +17,11 @@ package org.labkey.specimen.importer; import org.labkey.api.reader.DataLoader; -import org.labkey.api.study.SpecimenImportStrategy; import java.io.IOException; -/* -* User: adam -* Date: Feb 16, 2013 -* Time: 6:58:21 AM -*/ public interface SpecimenImportFile { - SpecimenImportStrategy getStrategy(); SpecimenTableType getTableType(); DataLoader getDataLoader() throws IOException; } diff --git a/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java b/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java index 21bb1fa1cd5..88eb4b8f4d7 100644 --- a/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java +++ b/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java @@ -90,8 +90,6 @@ import org.labkey.api.specimen.model.SpecimenComment; import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.study.Location; -import org.labkey.api.study.SpecimenImportStrategy; -import org.labkey.api.study.SpecimenImportStrategyFactory; import org.labkey.api.study.SpecimenService; import org.labkey.api.study.Study; import org.labkey.api.study.StudyInternalService; @@ -1119,7 +1117,7 @@ private Map populateFileMap(VirtualFile d SpecimenTableType type = getForName(canonicalName); if (null != type) - fileNameMap.put(type, getSpecimenImportFile(getContainer(), dir, fileName, type)); + fileNameMap.put(type, getSpecimenImportFile(dir, fileName, type)); } } @@ -1127,20 +1125,9 @@ private Map populateFileMap(VirtualFile d } // TODO: Pass in merge (or import strategy)? - private SpecimenImportFile getSpecimenImportFile(Container c, VirtualFile dir, String fileName, SpecimenTableType type) + private SpecimenImportFile getSpecimenImportFile(VirtualFile dir, String fileName, SpecimenTableType type) { - DbSchema schema = SpecimenSchema.get().getSchema(); - - // Enumerate the import filter factories... first one to claim the file gets associated with it - for (SpecimenImportStrategyFactory factory : SpecimenService.get().getSpecimenImportStrategyFactories()) - { - SpecimenImportStrategy strategy = factory.get(schema, c, dir, fileName); - - if (null != strategy) - return new FileSystemSpecimenImportFile(dir, fileName, strategy, type); - } - - throw new IllegalStateException("No SpecimenImportStrategyFactory claimed this import!"); + return new FileSystemSpecimenImportFile(dir, fileName, type); } private void info(String message) @@ -1788,10 +1775,6 @@ private void mergeTable(DbSchema schema, SpecimenImportFile file, TableInfo targ { mergeTable(schema, type.getTableName(), target, type.getColumns(), loader, entityIdCol, hasContainerColumn); } - finally - { - file.getStrategy().close(); - } } /** @@ -1975,22 +1958,20 @@ public Object getValue(Map row) int rowCount; ColumnDescriptor[] tsvColumns; - try + if (null == target) { - if (null == target) - { - String dbname = tableName; - if (dbname.startsWith(schema.getName() + ".")) - dbname = dbname.substring(schema.getName().length() + 1); - target = schema.getTable(dbname); - } - if (null == target) - throw new IllegalStateException("Could not resolve table: " + tableName); + String dbname = tableName; + if (dbname.startsWith(schema.getName() + ".")) + dbname = dbname.substring(schema.getName().length() + 1); + target = schema.getTable(dbname); + } + if (null == target) + throw new IllegalStateException("Could not resolve table: " + tableName); - DataIteratorContext dix = new DataIteratorContext(); - dix.setInsertOption(QueryUpdateService.InsertOption.IMPORT); - DataLoader tsv = loadTsv(file); - tsvColumns = tsv.getColumns(); + DataIteratorContext dix = new DataIteratorContext(); + dix.setInsertOption(QueryUpdateService.InsertOption.IMPORT); + DataLoader tsv = loadTsv(file); + tsvColumns = tsv.getColumns(); /* // DEBUG: Dump data StringBuilder infoCol = new StringBuilder(""); @@ -2007,57 +1988,52 @@ public Object getValue(Map row) } info(""); */ - // CONSIDER turn off data conversion - //for (ColumnDescriptor cd : tsvColumns) cd.clazz = String.class; - // CONSIDER use AsyncDataIterator - //DataIteratorBuilder asyncIn = new AsyncDataIterator.Builder(tsv); - DataIteratorBuilder asyncIn = tsv; - DataIteratorBuilder specimenWrapped = new SpecimenImportBuilder(asyncIn, file.getTableType().getColumns(), computedColumns); - DataIteratorBuilder standardEtl = StandardDataIteratorBuilder.forInsert(target, specimenWrapped, getContainer(), getUser(), dix); - DataIteratorBuilder persist = ((UpdateableTableInfo)target).persistRows(standardEtl, dix); - Pump pump = new Pump(persist, dix); - pump.setProgress(new ListImportProgress() - { - long heartBeat = HeartBeat.currentTimeMillis(); + // CONSIDER turn off data conversion + //for (ColumnDescriptor cd : tsvColumns) cd.clazz = String.class; + // CONSIDER use AsyncDataIterator + //DataIteratorBuilder asyncIn = new AsyncDataIterator.Builder(tsv); + DataIteratorBuilder asyncIn = tsv; + DataIteratorBuilder specimenWrapped = new SpecimenImportBuilder(asyncIn, file.getTableType().getColumns(), computedColumns); + DataIteratorBuilder standardEtl = StandardDataIteratorBuilder.forInsert(target, specimenWrapped, getContainer(), getUser(), dix); + DataIteratorBuilder persist = ((UpdateableTableInfo)target).persistRows(standardEtl, dix); + Pump pump = new Pump(persist, dix); + pump.setProgress(new ListImportProgress() + { + long heartBeat = HeartBeat.currentTimeMillis(); - @Override - public void setTotalRows(int rows) - { - } + @Override + public void setTotalRows(int rows) + { + } - @Override - public void setCurrentRow(int currentRow) + @Override + public void setCurrentRow(int currentRow) + { + if (0 == currentRow % SQL_BATCH_SIZE) { - if (0 == currentRow % SQL_BATCH_SIZE) - { - if (0 == currentRow % (SQL_BATCH_SIZE*100)) - info(currentRow + " rows loaded..."); - long hb = HeartBeat.currentTimeMillis(); - if (hb == heartBeat) - return; - ensureNotCanceled(); - heartBeat = hb; - } + if (0 == currentRow % (SQL_BATCH_SIZE*100)) + info(currentRow + " rows loaded..."); + long hb = HeartBeat.currentTimeMillis(); + if (hb == heartBeat) + return; + ensureNotCanceled(); + heartBeat = hb; } - }); - pump.run(); - if (dix.getErrors().hasErrors()) - { - throw new ValidationException(dix.getErrors().getLastRowError().getMessage() + " (File: " + file.getTableType().getName() + ")"); } - rowCount = pump.getRowCount(); - - info(tableName + ": Replaced all data with " + rowCount + " new rows."); - } - finally + }); + pump.run(); + if (dix.getErrors().hasErrors()) { - file.getStrategy().close(); + throw new ValidationException(dix.getErrors().getLastRowError().getMessage() + " (File: " + file.getTableType().getName() + ")"); } + rowCount = pump.getRowCount(); + + info(tableName + ": Replaced all data with " + rowCount + " new rows."); // Note: this duplicates the logic in SpecimenImportIterator (just below). Keep these code paths in sync. Map importMap = (Map)createImportMap(file.getTableType().getColumns()); final var seen = new HashSet(); - List availableColumns = Arrays.stream(tsvColumns).map(tsv -> importMap.get(tsv.getColumnName())) + List availableColumns = Arrays.stream(tsvColumns).map(tsv2 -> importMap.get(tsv2.getColumnName())) .filter(Objects::nonNull) .filter(cd -> seen.add(cd.getPrimaryTsvColumnName())) .collect(Collectors.toList()); diff --git a/specimen/src/org/labkey/specimen/importer/StandardSpecimenImportStrategy.java b/specimen/src/org/labkey/specimen/importer/StandardSpecimenImportStrategy.java deleted file mode 100644 index b30278230b6..00000000000 --- a/specimen/src/org/labkey/specimen/importer/StandardSpecimenImportStrategy.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright (c) 2013-2018 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.specimen.importer; - -import org.labkey.api.data.Container; -import org.labkey.api.data.Filter; -import org.labkey.api.study.SpecimenImportStrategy; - -import java.util.Map; - -/** -* User: adam -* Date: 5/19/13 -* Time: 4:48 PM -*/ -public class StandardSpecimenImportStrategy implements SpecimenImportStrategy -{ - private final Container _c; - - public StandardSpecimenImportStrategy(Container c) - { - _c = c; - } - - @Override - public org.labkey.api.util.Filter> getImportFilter() - { - return null; - } - - @Override - public Filter getDeleteFilter() - { - return null; - } - - @Override - public Filter getInsertFilter() - { - return null; - } - - @Override - public void close() - { - } -}