From f2309df85e12d3d99fcf0e615c0c25dc79829c8e Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 2 May 2024 03:31:39 -0700 Subject: [PATCH 01/32] Migrate DbCache use to DatabaseCache --- .../org/labkey/api/cache/CachingTestCase.java | 10 +++ api/src/org/labkey/api/cache/DbCache.java | 76 ++++++++++++++++++- .../org/labkey/api/util/ExceptionUtil.java | 11 +-- 3 files changed, 91 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/cache/CachingTestCase.java b/api/src/org/labkey/api/cache/CachingTestCase.java index 5748e4bf1e9..7f40ecf937f 100644 --- a/api/src/org/labkey/api/cache/CachingTestCase.java +++ b/api/src/org/labkey/api/cache/CachingTestCase.java @@ -27,6 +27,7 @@ public void testCaching() assertNotNull(testTable); DbCache.clear(testTable); + DbCache.trackRemove(testTable); Map mm = new HashMap<>(); // Modifiable map Map umm = Collections.unmodifiableMap(mm); // Unmodifiable map @@ -36,6 +37,7 @@ public void testCaching() mm.put("IntNotNull", 0); mm.put("Container", JunitUtil.getTestContainer()); mm = Table.insert(ctx.getUser(), testTable, mm); + DbCache.trackRemove(testTable); Integer rowId1 = ((Integer) mm.get("RowId")); String key = "RowId" + rowId1; @@ -45,17 +47,20 @@ public void testCaching() //Does cache get cleared on delete Table.delete(testTable, rowId1); + DbCache.trackRemove(testTable); m2 = (Map) DbCache.get(testTable, key); assertNull(m2); //Does cache get cleared on insert mm.remove("RowId"); mm = Table.insert(ctx.getUser(), testTable, mm); + DbCache.trackRemove(testTable); int rowId2 = ((Integer) mm.get("RowId")); key = "RowId" + rowId2; DbCache.put(testTable, key, umm); mm.remove("RowId"); mm = Table.insert(ctx.getUser(), testTable, mm); + DbCache.trackRemove(testTable); int rowId3 = ((Integer) mm.get("RowId")); m2 = (Map) DbCache.get(testTable, key); assertNull(m2); @@ -66,6 +71,7 @@ public void testCaching() try (DbScope.Transaction ignored = testSchema.getScope().beginTransaction()) { mm = Table.insert(ctx.getUser(), testTable, mm); + DbCache.trackRemove(testTable); int rowId4 = ((Integer) mm.get("RowId")); key2 = "RowId" + rowId4; DbCache.put(testTable, key2, umm); @@ -75,6 +81,10 @@ public void testCaching() // Clean up Table.delete(testTable, rowId2); + DbCache.trackRemove(testTable); Table.delete(testTable, rowId3); + DbCache.trackRemove(testTable); + + DbCache.logUnmatched(); } } diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index 76690d29e73..068119b78e5 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -16,9 +16,14 @@ package org.labkey.api.cache; +import org.apache.commons.collections4.Bag; +import org.apache.commons.collections4.bag.HashBag; +import org.apache.logging.log4j.Logger; import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.TableInfo; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.Path; +import org.labkey.api.util.logging.LogHelper; import java.util.HashMap; import java.util.Map; @@ -33,7 +38,8 @@ @Deprecated public class DbCache { - private static final Map> CACHES = new HashMap<>(100); + private static final Logger LOG = LogHelper.getLogger(DbCache.class, "DbCache invalidations"); + private static final Map> CACHES = new HashMap<>(10); public static DatabaseCache getCache(TableInfo tinfo, boolean create) { @@ -84,14 +90,20 @@ public static void invalidateAll(TableInfo tinfo) { DatabaseCache cache = CACHES.get(tinfo.getNotificationKey()); if (null != cache) + { + trackInvalidate(tinfo); cache.clear(); + } } public static void clear(TableInfo tinfo) { DatabaseCache cache = getCache(tinfo, false); if (null != cache) + { + trackInvalidate(tinfo); cache.clear(); + } } public static void removeUsingPrefix(TableInfo tinfo, String name) @@ -100,4 +112,66 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) if (null != cache) cache.removeUsingFilter(new Cache.StringPrefixFilter(name)); } + + /** + * Everything below is temporary, meant to help irradicate the use of DbCache. If a TableInfo is deemed interesting: + * - Each call to invalidateAll() or clear() causes its stack trace to be added to the tracking bag + * - Each call to trackRemove() causes its stack trace to be removed from the tracking bag + * A remove that's unsuccessful indicates no corresponding invalidateAll(), so log that. Anything left in the bag + * indicates invalidateAll()/clear() calls with no corresponding remove. Use this to migrate a DbCache to our normal + * DatabaseCache pattern, with explicit removes for invalidation. Leave the DbCache in place (until migration is + * complete) and add calls to trackRemove() immediately after Table.insert(), Table.update(), and Table.delete() + * calls. Ensure that nothing is left in the bag, meaning all Table-initiated invalidateAll() calls have + * corresponding removes on the new cache. See CachingTestCase as an example. + */ + + private static final Bag TRACKING_BAG = new HashBag<>(); + + private static boolean isInteresting(TableInfo tinfo) + { + return getCache(tinfo, false) != null; + } + + public static void trackInvalidate(TableInfo tinfo) + { + if (isInteresting(tinfo)) + { + StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); + int linesToTrim = 2; + + // Trim all Table lines, if present + for (int i = 2; i < stackTrace.length; i++) + { + if ("Table.java".equals(stackTrace[i].getFileName())) + linesToTrim = i; + else if (linesToTrim > 2) + break; + } + + String key = tinfo.getName() + ExceptionUtil.renderStackTrace(stackTrace, linesToTrim + 1); + TRACKING_BAG.add(key); + } + } + + public static void trackRemove(TableInfo tinfo) + { + if (isInteresting(tinfo)) + { + StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); + String trimmed = ExceptionUtil.renderStackTrace(stackTrace, 2); + + // Subtract one from the first line number so it matches the Table.* line (so the stack traces match exactly) + int from = trimmed.indexOf(':') + 1; + int to = trimmed.indexOf(')', from); + String lineNumber = trimmed.substring(from, to); + String key = tinfo.getName() + trimmed.replaceFirst(lineNumber, String.valueOf(Integer.valueOf(lineNumber) - 1)); + if (!TRACKING_BAG.remove(key, 1)) + LOG.info("Failed to remove " + key); + } + } + + public static void logUnmatched() + { + TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched " + key)); + } } diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 314ef3421cf..8510b08c826 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -119,8 +119,12 @@ private ExceptionUtil() { } - public static String renderStackTrace(@Nullable StackTraceElement[] stackTrace) + { + return renderStackTrace(stackTrace, 2); + } + + public static String renderStackTrace(@Nullable StackTraceElement[] stackTrace, int linesToSkip) { if (stackTrace == null) { @@ -128,7 +132,7 @@ public static String renderStackTrace(@Nullable StackTraceElement[] stackTrace) } StringBuilder trace = new StringBuilder(); - for (int i = 2; i < stackTrace.length; i++) + for (int i = linesToSkip; i < stackTrace.length; i++) { String line = String.valueOf(stackTrace[i]); if (line.startsWith("javax.servlet.http.HttpServlet.service(")) @@ -140,7 +144,6 @@ public static String renderStackTrace(@Nullable StackTraceElement[] stackTrace) return trace.toString(); } - @NotNull public static Throwable unwrapException(@NotNull Throwable ex) { @@ -168,7 +171,6 @@ else if (ex instanceof BatchUpdateException) return ex; } - public static HtmlString renderException(Throwable e) { StringWriter sw = new StringWriter(); @@ -179,7 +181,6 @@ public static HtmlString renderException(Throwable e) return HtmlString.unsafe("
\n" + s + "
\n"); } - public static HtmlString getUnauthorizedMessage(ViewContext context) { return HtmlString.unsafe("
" + From ce48fee757204e7f2cc198e749325fc4fbb6a202 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 2 May 2024 23:25:13 -0700 Subject: [PATCH 02/32] Experiment run cache --- api/src/org/labkey/api/cache/DbCache.java | 5 +- .../api/data/AtomicDatabaseInteger.java | 3 + api/src/org/labkey/api/data/DbSchema.java | 4 ++ .../org/labkey/api/data/TableViewForm.java | 6 ++ .../labkey/api/exp/api/ExperimentService.java | 2 + .../org/labkey/api/security/LimitedUser.java | 2 +- .../labkey/core/CoreContainerListener.java | 7 +- .../core/view/TableViewFormTestCase.java | 1 + .../api/ExpIdentifiableBaseImpl.java | 6 +- .../org/labkey/experiment/api/ExpRunImpl.java | 1 + .../labkey/experiment/api/ExperimentRun.java | 2 - .../experiment/api/ExperimentServiceImpl.java | 68 +++++++++++++++++-- .../labkey/pipeline/api/PipelineManager.java | 5 +- 13 files changed, 92 insertions(+), 20 deletions(-) diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index 068119b78e5..918ce10f115 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -27,9 +27,9 @@ import java.util.HashMap; import java.util.Map; +import java.util.Set; /** - * * Don't use this! Use CacheManager.getCache() or DatabaseCache instead. DbCache associates a DatabaseCache with each * participating TableInfo. The Table layer then invalidates the entire cache anytime it touches (insert, update, delete) * that TableInfo. This is easy, but very inefficient. Managers should use DatabaseCaches directly and handle @@ -126,10 +126,11 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) */ private static final Bag TRACKING_BAG = new HashBag<>(); + private static final Set INTERESTING = Set.of("TestTable", "ExperimentRun"); private static boolean isInteresting(TableInfo tinfo) { - return getCache(tinfo, false) != null; + return getCache(tinfo, false) != null && INTERESTING.contains(tinfo.getName()); } public static void trackInvalidate(TableInfo tinfo) diff --git a/api/src/org/labkey/api/data/AtomicDatabaseInteger.java b/api/src/org/labkey/api/data/AtomicDatabaseInteger.java index ff3c2abf251..143e11604be 100644 --- a/api/src/org/labkey/api/data/AtomicDatabaseInteger.java +++ b/api/src/org/labkey/api/data/AtomicDatabaseInteger.java @@ -20,6 +20,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.labkey.api.cache.DbCache; import org.labkey.api.security.User; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.TestContext; @@ -144,6 +145,7 @@ public void setup() map.put("BitNotNull", true); map = Table.insert(user, table, map); + DbCache.trackRemove(table); _rowId = (Integer)map.get("RowId"); _adi = new AtomicDatabaseInteger(table.getColumn("IntNotNull"), c, _rowId); @@ -207,6 +209,7 @@ public void cleanup() { TableInfo table = TestSchema.getInstance().getTableInfoTestTable(); Table.delete(table, _rowId); + DbCache.trackRemove(table); } } } diff --git a/api/src/org/labkey/api/data/DbSchema.java b/api/src/org/labkey/api/data/DbSchema.java index dfd9e1e8ef1..9f108f335d7 100644 --- a/api/src/org/labkey/api/data/DbSchema.java +++ b/api/src/org/labkey/api/data/DbSchema.java @@ -21,6 +21,7 @@ import org.junit.After; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.dialect.JdbcMetaDataLocator; import org.labkey.api.data.dialect.SqlDialect; @@ -564,6 +565,7 @@ public void testTransactions() throws Exception { assertTrue("Not in transaction when should be.", testSchema.getScope().isTransactionActive()); m = Table.insert(ctx.getUser(), testTable, m); + DbCache.trackRemove(testTable); rowId = ((Integer) m.get("RowId")); assertNotNull("Inserted Row doesn't have Id", rowId); assertTrue(rowId != 0); @@ -583,6 +585,7 @@ public void testTransactions() throws Exception { m.put("IntNotNull", 1); m = Table.update(ctx.getUser(), testTable, m, rowId); + DbCache.trackRemove(testTable); assertEquals("Update is consistent in transaction?", 1, (int)m.get("IntNotNull")); } @@ -594,6 +597,7 @@ public void testTransactions() throws Exception assertEquals("Rollback did not appear to work.", 0, (int)m.get("IntNotNull")); Table.delete(testTable, rowId); + DbCache.trackRemove(testTable); } } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 3d208878168..8b4ea6c998b 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -33,6 +33,7 @@ import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.ReturnUrlForm; import org.labkey.api.action.SpringActionController; +import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.query.SchemaKey; import org.labkey.api.security.permissions.DeletePermission; @@ -162,6 +163,7 @@ public void doInsert() throws SQLException set("container", _c.getId()); Map newMap = Table.insert(_user, _tinfo, getTypedValues()); + if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); // Temporary hack setTypedValues(newMap, false); } @@ -187,6 +189,7 @@ public void doUpdate() throws SQLException Object[] pkVal = getPkVals(); Map newMap = Table.update(_user, _tinfo, getTypedValues(), pkVal); + if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); setTypedValues(newMap, true); } @@ -215,7 +218,10 @@ public void doDelete() { Object[] pkVal = getPkVals(); if (null != pkVal && null != pkVal[0]) + { Table.delete(_tinfo, pkVal); + if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); + } else //Hmm, throw an exception here???? _log.warn("Nothing to delete for table " + _tinfo.getName() + " on request " + _request.getRequestURI()); } diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index bd56b1685c7..1fc73247abf 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -746,6 +746,8 @@ static void validateParentAlias(Map aliasMap, Set reserv void clearCaches(); + void clearExperimentRunCache(); + List getProtocolApplicationParameters(int rowId); void moveContainer(Container c, Container cOldParent, Container cNewParent) throws ExperimentException; diff --git a/api/src/org/labkey/api/security/LimitedUser.java b/api/src/org/labkey/api/security/LimitedUser.java index 4bf01ae0422..33cfe2e3050 100644 --- a/api/src/org/labkey/api/security/LimitedUser.java +++ b/api/src/org/labkey/api/security/LimitedUser.java @@ -131,7 +131,7 @@ public void testElevatedUser() int roleCount = user.getAssignedRoles(c.getPolicy()).size(); int siteRolesCount = user.getSiteRoles().size(); User elevated = ElevatedUser.getElevatedUser(user); - assertEquals(groupCount, elevated.getGroups().stream().count()); + assertEquals(user.getGroups() + " vs. " + elevated.getGroups(), groupCount, elevated.getGroups().stream().count()); assertEquals(roleCount, elevated.getAssignedRoles(c.getPolicy()).size()); assertEquals(siteRolesCount, elevated.getSiteRoles().size()); } diff --git a/core/src/org/labkey/core/CoreContainerListener.java b/core/src/org/labkey/core/CoreContainerListener.java index d5b02e5303b..f001c99ae18 100644 --- a/core/src/org/labkey/core/CoreContainerListener.java +++ b/core/src/org/labkey/core/CoreContainerListener.java @@ -22,6 +22,7 @@ import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.provider.ContainerAuditProvider; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; @@ -38,11 +39,6 @@ import java.util.Collection; import java.util.Collections; -/** - * User: adam - * Date: Nov 5, 2008 - * Time: 2:00:32 PM - */ public class CoreContainerListener implements ContainerManager.ContainerListener { private static final Logger _log = LogManager.getLogger(CoreContainerListener.class); @@ -70,6 +66,7 @@ public void containerDeleted(Container c, User user) // Delete any rows in test.TestTable associated with this container SimpleFilter containerFilter = SimpleFilter.createContainerFilter(c); Table.delete(TestSchema.getInstance().getTableInfoTestTable(), containerFilter); + DbCache.trackRemove(TestSchema.getInstance().getTableInfoTestTable()); // Data States Table.delete(CoreSchema.getInstance().getTableInfoDataStates(), containerFilter); diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index abed3c1a058..6469df1d979 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -19,6 +19,7 @@ import org.junit.Assert; import org.junit.Test; import org.labkey.api.action.NullSafeBindException; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.TableViewForm; import org.labkey.api.data.TestSchema; diff --git a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java index ce5eadab849..2d8aea10479 100644 --- a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java @@ -17,6 +17,7 @@ package org.labkey.experiment.api; import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.DbScope; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlSelector; @@ -131,12 +132,13 @@ protected void save(User user, TableInfo table, boolean ensureObject, boolean is { if (ensureObject) { - assert !(_object instanceof IdentifiableEntity) || null == ((IdentifiableEntity)_object).getObjectId(); + assert !(_object instanceof IdentifiableEntity) || null == _object.getObjectId(); _objectId = OntologyManager.ensureObject(getContainer(), getLSID(), getParentObjectId()); _object.setObjectId(_objectId); } _object = Table.insert(user, table, _object); - assert !ensureObject || !(_object instanceof IdentifiableEntity) || _objectId == ((IdentifiableEntity)_object).getObjectId(); + if (this instanceof ExpRunImpl) DbCache.trackRemove(table); + assert !ensureObject || !(_object instanceof IdentifiableEntity) || _objectId == _object.getObjectId(); } else { diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index 09d8c3adaa0..c1d71218024 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -328,6 +328,7 @@ protected void save(User user, TableInfo table, boolean ensureObject) { assert ensureObject; super.save(user, table, true); + ExperimentServiceImpl.EXPERIMENT_RUN_CACHE.remove(getLSID()); } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExperimentRun.java b/experiment/src/org/labkey/experiment/api/ExperimentRun.java index 5a8addeac17..900b407f093 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentRun.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentRun.java @@ -23,8 +23,6 @@ /** * Bean class for the exp.experimentrun table. - * User: migra - * Date: Jun 14, 2005 */ public class ExperimentRun extends IdentifiableEntity { diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index b428b3a5e5e..1122ee16470 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -52,6 +52,7 @@ import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.provider.ContainerAuditProvider; import org.labkey.api.cache.Cache; +import org.labkey.api.cache.CacheLoader; import org.labkey.api.cache.CacheManager; import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; @@ -298,6 +299,7 @@ public class ExperimentServiceImpl implements ExperimentService, ObjectReference private final Cache PROTOCOL_LSID_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, CacheManager.HOUR, "Protocol by LSID", (key, argument) -> toExpProtocol(new TableSelector(getTinfoProtocol(), new SimpleFilter(FieldKey.fromParts("LSID"), key), null).getObject(Protocol.class))); + static final Cache EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, "Experiment Runs", new ExperimentRunCacheLoader()); private final Cache> dataClassCache = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Data classes", (containerId, argument) -> { @@ -3880,6 +3882,11 @@ public TableInfo getTinfoExperiment() @Override public TableInfo getTinfoExperimentRun() + { + return getTableInfoExperimentRun(); + } + + private static TableInfo getTableInfoExperimentRun() { return getExpSchema().getTable("ExperimentRun"); } @@ -4141,6 +4148,31 @@ public List getExperiments(Container container, User user, bo } public ExperimentRun getExperimentRun(String LSID) + { + ExperimentRun runOld = getExperimentRunOld(LSID); + ExperimentRun runNew = getExperimentRunNew(LSID); + + if (null != runOld) + { + assert runOld.equals(runNew) : "Experiment runs are not equal!"; + + assert Objects.equals(runOld.getComments(), runNew.getComments()); + assert Objects.equals(runOld.getBatchId(), runNew.getBatchId()); + assert Objects.equals(runOld.getJobId(), runNew.getJobId()); + assert Objects.equals(runOld.getFilePathRoot(), runNew.getFilePathRoot()); + assert Objects.equals(runOld.getName(), runNew.getName()); + assert Objects.equals(runOld.getWorkflowTask(), runNew.getWorkflowTask()); + assert Objects.equals(runOld.getModified(), runNew.getModified()); + } + else + { + assert null == runNew; + } + + return runOld; + } + + private ExperimentRun getExperimentRunOld(String LSID) { //Use main cache so updates/deletes through table layer get handled String cacheKey = getCacheKey(LSID); @@ -4155,6 +4187,21 @@ public ExperimentRun getExperimentRun(String LSID) return run; } + private ExperimentRun getExperimentRunNew(String LSID) + { + return EXPERIMENT_RUN_CACHE.get(LSID); + } + + private static class ExperimentRunCacheLoader implements CacheLoader + { + @Override + public ExperimentRun load(@NotNull String lsid, @Nullable Object argument) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("LSID"), lsid); + return new TableSelector(getTableInfoExperimentRun(), filter, null).getObject(ExperimentRun.class); + } + } + @Override public void clearCaches() { @@ -4163,6 +4210,13 @@ public void clearCaches() PROTOCOL_ROW_ID_CACHE.clear(); PROTOCOL_LSID_CACHE.clear(); DomainPropertyManager.clearCaches(); + clearExperimentRunCache(); + } + + @Override + public void clearExperimentRunCache() + { + EXPERIMENT_RUN_CACHE.clear(); } @Override @@ -6802,12 +6856,12 @@ private void saveAll(Iterable objects, User user) throws Ba public ExpRun saveSimpleExperimentRun( ExpRun run, Map inputMaterials, - Map inputDatas, + Map inputDatas, Map outputMaterials, - Map outputDatas, - Map transformedDatas, - ViewBackgroundInfo info, - Logger log, + Map outputDatas, + Map transformedDatas, + ViewBackgroundInfo info, + Logger log, boolean loadDataFiles ) throws ExperimentException { @@ -9859,10 +9913,10 @@ public static class LineageQueryTestCase extends Assert static public final int A11 = startId+8; static public final int AZ = startId+9; static public final int Z21 = startId+10; - + static edge e(int a,int b) {return new edge(a,b);}; static public final List edges = List.of( - e(Q,QQ), + e(Q,QQ), e(A1,A), e(A2,A), e(A2, Q), e(Z1, Q), e(Z1, Z), e(Z2, Z), e(A11, A1), e(AZ,A2), e(AZ, Z1), e(Z21, Z2) ); diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java index d54ae8f3dec..5437d47e165 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java @@ -28,6 +28,7 @@ import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DbSchemaCache; import org.labkey.api.data.DbScope; import org.labkey.api.data.Filter; import org.labkey.api.data.ObjectFactory; @@ -221,8 +222,10 @@ static public void purge(Container container, User user) try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) { - DbCache.clear(ExperimentService.get().getTinfoExperimentRun()); new SqlExecutor(PipelineSchema.getInstance().getSchema()).execute(sql); + DbCache.clear(ExperimentService.get().getTinfoExperimentRun()); + DbCache.trackRemove(ExperimentService.get().getTinfoExperimentRun()); + ExperimentService.get().clearCaches(); ContainerUtil.purgeTable(pipeline.getTableInfoStatusFiles(), container, "Container"); From d156d2aa3d6099e36629cd9ca0e3722bbeb5371a Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 4 May 2024 00:03:32 -0700 Subject: [PATCH 03/32] Migrate StudyCache/QueryHelper to DatabaseCache --- api/src/org/labkey/api/cache/DbCache.java | 9 +-- api/src/org/labkey/api/data/Table.java | 2 +- .../org/labkey/api/study/StudyCachable.java | 2 - .../api/specimen/SpecimenRequestStatus.java | 22 +++++-- .../org/labkey/api/study/QueryHelper.java | 22 ++++--- .../org/labkey/api/study/StudyCache.java | 63 ++++++++++++++++--- .../study/controllers/StudyController.java | 4 +- .../reports/ReportsController.java | 4 +- .../study/model/AssaySpecimenConfigImpl.java | 16 +++++ .../org/labkey/study/model/CohortImpl.java | 16 +++++ .../org/labkey/study/model/StudyManager.java | 22 ++++--- .../src/org/labkey/study/model/VisitImpl.java | 28 +++++---- .../study/reports/StudyCrosstabReport.java | 2 +- .../study/view/createCrosstabReport.jsp | 2 +- study/src/org/labkey/study/view/overview.jsp | 2 +- 15 files changed, 164 insertions(+), 52 deletions(-) diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index 918ce10f115..5670064e8ae 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -114,7 +114,8 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) } /** - * Everything below is temporary, meant to help irradicate the use of DbCache. If a TableInfo is deemed interesting: + * Everything below is temporary, meant to help irradicate the use of DbCache. If a TableInfo is deemed + * "interesting" (it's in the process of being migrated): * - Each call to invalidateAll() or clear() causes its stack trace to be added to the tracking bag * - Each call to trackRemove() causes its stack trace to be removed from the tracking bag * A remove that's unsuccessful indicates no corresponding invalidateAll(), so log that. Anything left in the bag @@ -126,11 +127,11 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) */ private static final Bag TRACKING_BAG = new HashBag<>(); - private static final Set INTERESTING = Set.of("TestTable", "ExperimentRun"); + private static final Set INTERESTING = Set.of("TestTable", "ExperimentRun", "Study"); private static boolean isInteresting(TableInfo tinfo) { - return getCache(tinfo, false) != null && INTERESTING.contains(tinfo.getName()); + return getCache(tinfo, false) != null; } public static void trackInvalidate(TableInfo tinfo) @@ -173,6 +174,6 @@ public static void trackRemove(TableInfo tinfo) public static void logUnmatched() { - TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched " + key)); + TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched {}", key)); } } diff --git a/api/src/org/labkey/api/data/Table.java b/api/src/org/labkey/api/data/Table.java index e337e20d77e..8c93bfc64c2 100644 --- a/api/src/org/labkey/api/data/Table.java +++ b/api/src/org/labkey/api/data/Table.java @@ -632,7 +632,7 @@ protected static Map _getTableData(TableInfo table, K from, //noinspection unchecked ObjectFactory f = ObjectFactory.Registry.getFactory((Class)from.getClass()); if (null == f) - throw new IllegalArgumentException("Cound not find a matching object factory."); + throw new IllegalArgumentException("Could not find a matching object factory."); fields = f.toMap(from, null); return _getTableData(table, fields, insert); } diff --git a/api/src/org/labkey/api/study/StudyCachable.java b/api/src/org/labkey/api/study/StudyCachable.java index 87a934b52a1..eeda5c40a6b 100644 --- a/api/src/org/labkey/api/study/StudyCachable.java +++ b/api/src/org/labkey/api/study/StudyCachable.java @@ -20,8 +20,6 @@ /** * An object owned by a study that's eligible to be cached. - * User: brittp - * Date: Feb 10, 2006 */ public interface StudyCachable { diff --git a/study/api-src/org/labkey/api/specimen/SpecimenRequestStatus.java b/study/api-src/org/labkey/api/specimen/SpecimenRequestStatus.java index dfc857cf89f..b8e239cf106 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenRequestStatus.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenRequestStatus.java @@ -19,11 +19,8 @@ import org.labkey.api.data.Container; import org.labkey.api.study.AbstractStudyCachable; -/** - * User: brittp - * Date: Feb 8, 2006 - * Time: 4:18:11 PM - */ +import java.util.Objects; + public class SpecimenRequestStatus extends AbstractStudyCachable { private int _rowId; @@ -111,4 +108,19 @@ public boolean isSystemStatus() { return _sortOrder < 0; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SpecimenRequestStatus that = (SpecimenRequestStatus) o; + return _rowId == that._rowId && _specimensLocked == that._specimensLocked && _finalState == that._finalState && Objects.equals(_container, that._container) && Objects.equals(_sortOrder, that._sortOrder) && Objects.equals(_label, that._label); + } + + @Override + public int hashCode() + { + return Objects.hash(_rowId, _container, _sortOrder, _label, _specimensLocked, _finalState); + } } diff --git a/study/api-src/org/labkey/api/study/QueryHelper.java b/study/api-src/org/labkey/api/study/QueryHelper.java index 62abecc28b8..92cc9744890 100644 --- a/study/api-src/org/labkey/api/study/QueryHelper.java +++ b/study/api-src/org/labkey/api/study/QueryHelper.java @@ -18,6 +18,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.CacheLoader; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.Filter; import org.labkey.api.data.SimpleFilter; @@ -32,11 +33,6 @@ import java.util.Collections; import java.util.List; -/** - * User: brittp - * Date: Feb 24, 2006 - * Time: 2:04:14 PM - */ public class QueryHelper { private final Class _objectClass; @@ -133,8 +129,10 @@ private K get(final Container c, final Object rowId, final String rowIdColumnNam public K create(User user, K obj) { + K ret = Table.insert(user, getTableInfo(), obj); + DbCache.trackRemove(getTableInfo()); clearCache(obj); - return Table.insert(user, getTableInfo(), obj); + return ret; } public K update(User user, K obj) @@ -144,14 +142,17 @@ public K update(User user, K obj) public K update(User user, K obj, Object... pk) { + K ret = Table.update(user, getTableInfo(), obj, pk); + DbCache.trackRemove(getTableInfo()); clearCache(obj); - return Table.update(user, getTableInfo(), obj, pk); + return ret; } public void delete(K obj) { - clearCache(obj); Table.delete(getTableInfo(), obj.getPrimaryKey()); + DbCache.trackRemove(getTableInfo()); + clearCache(obj); } public TableInfo getTableInfo() @@ -169,6 +170,11 @@ public void clearCache(K obj) StudyCache.uncache(getTableInfo(), obj.getContainer(), obj.getPrimaryKey().toString()); } + public void clearCache() + { + StudyCache.clearCache(getTableInfo()); + } + protected String getCacheId(Filter filter) { if (filter == null) diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index c50e1e16d27..85905de0951 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -17,20 +17,44 @@ package org.labkey.api.study; import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheLoader; import org.labkey.api.cache.CacheManager; import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.TableInfo; +import org.labkey.api.util.Path; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; -/** - * User: brittp - * Date: Feb 21, 2006 - * Time: 10:19:30 AM - */ public class StudyCache { + // TODO: Generics! + // TODO: Switch to BlockingCache + private static final Map> CACHES = new HashMap<>(10); + + private static DatabaseCache getCache(TableInfo tinfo, boolean create) + { + Path cacheKey = tinfo.getNotificationKey(); + assert null != cacheKey : "DbCache not supported for " + tinfo; + + synchronized(CACHES) + { + DatabaseCache cache = CACHES.get(cacheKey); + + if (null == cache && create) + { + cache = new DatabaseCache(tinfo.getSchema().getScope(), tinfo.getCacheSize(), "StudyCache: " + tinfo.getName()); + CACHES.put(cacheKey, cache); + } + + return cache; + } + } + private static String getCacheName(Container c, @Nullable Object cacheKey) { return c.getId() + "/" + (null != cacheKey ? cacheKey : ""); @@ -41,16 +65,25 @@ public static void cache(TableInfo tinfo, Container c, String objectId, StudyCac if (cachable != null) cachable.lock(); DbCache.put(tinfo, getCacheName(c, objectId), cachable, CacheManager.HOUR); + DatabaseCache cache = getCache(tinfo, true); + cache.put(getCacheName(c, objectId), cachable, CacheManager.HOUR); } public static void uncache(TableInfo tinfo, Container c, Object cacheKey) { DbCache.remove(tinfo, getCacheName(c, cacheKey)); + clearCache(tinfo, c); // TODO: this method is broken/inconsistent -- the cacheKey passed in doesn't match the put() keys } public static Object getCached(TableInfo tinfo, Container c, Object cacheKey) { - return DbCache.get(tinfo, getCacheName(c, cacheKey)); + Object oldObj = DbCache.get(tinfo, getCacheName(c, cacheKey)); + DatabaseCache cache = getCache(tinfo, false); + Object newObj = cache != null ? cache.get(getCacheName(c, cacheKey)) : null; + + assert Objects.equals(oldObj, newObj); + + return newObj; } public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoader loader) @@ -59,11 +92,27 @@ public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoa // load when all other DB connections are in use in threads, including one // that holds the BlockingCache's lock DatabaseCache cache = DbCache.getCache(tinfo, true); - return cache.get(getCacheName(c, cacheKey), null, loader); + Object oldObj = cache.get(getCacheName(c, cacheKey), null, loader); + DatabaseCache cache2 = getCache(tinfo, true); + Object newObj = cache2 != null ? cache2.get(getCacheName(c, cacheKey), null, loader) : null; + + assert Objects.equals(oldObj, newObj); + + return newObj; } public static void clearCache(TableInfo tinfo, Container c) { DbCache.removeUsingPrefix(tinfo, getCacheName(c, null)); + DatabaseCache cache = getCache(tinfo, false); + if (null != cache) + cache.removeUsingFilter(new Cache.StringPrefixFilter(getCacheName(c, null))); + } + + public static void clearCache(TableInfo tinfo) + { + DatabaseCache cache = getCache(tinfo, false); + if (null != cache) + cache.clear(); } } diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 9e0bd334a27..ae2403bbb99 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -911,7 +911,7 @@ protected ModelAndView getHtmlView(DatasetFilterForm form, BindException errors) if (null == typeURI) return new TypeNotFoundAction().getView(form, errors); - _visitId = NumberUtils.toInt((String)context.get(VisitImpl.VISITKEY), 0); + _visitId = NumberUtils.toInt((String)context.get(VisitImpl.VISIT_KEY), 0); VisitImpl visit; if (_visitId != 0) { @@ -3339,7 +3339,7 @@ private static List generateParticipantListFromURL(ViewContext context, if (null == typeURI) return Collections.emptyList(); - int visitRowId = null == context.get(VisitImpl.VISITKEY) ? 0 : Integer.parseInt((String) context.get(VisitImpl.VISITKEY)); + int visitRowId = null == context.get(VisitImpl.VISIT_KEY) ? 0 : Integer.parseInt((String) context.get(VisitImpl.VISIT_KEY)); if (visitRowId != 0) { VisitImpl visit = studyMgr.getVisitForRowId(study, visitRowId); diff --git a/study/src/org/labkey/study/controllers/reports/ReportsController.java b/study/src/org/labkey/study/controllers/reports/ReportsController.java index 319ef7875a3..0a2e3485f7e 100644 --- a/study/src/org/labkey/study/controllers/reports/ReportsController.java +++ b/study/src/org/labkey/study/controllers/reports/ReportsController.java @@ -486,7 +486,7 @@ public CrosstabReport getReport(ContainerUser cu) throws Exception CrosstabReportDescriptor descriptor = (CrosstabReportDescriptor) report.getDescriptor(); - if (_visitRowId != -1) descriptor.setProperty(VisitImpl.VISITKEY, Integer.toString(_visitRowId)); + if (_visitRowId != -1) descriptor.setProperty(VisitImpl.VISIT_KEY, Integer.toString(_visitRowId)); if (!StringUtils.isEmpty(_rowField)) descriptor.setProperty("rowField", _rowField); if (!StringUtils.isEmpty(_colField)) descriptor.setProperty("colField", _colField); if (!StringUtils.isEmpty(_statField)) descriptor.setProperty("statField", _statField); @@ -1215,7 +1215,7 @@ private void _addNavTrail(NavTree root, String name, int datasetId, int visitRow label += " (All Visits)"; ActionURL datasetUrl = getViewContext().getActionURL().clone(); - datasetUrl.deleteParameter(VisitImpl.VISITKEY); + datasetUrl.deleteParameter(VisitImpl.VISIT_KEY); datasetUrl.setAction(StudyController.DatasetAction.class); root.addChild(label, datasetUrl.getLocalURIString()); } diff --git a/study/src/org/labkey/study/model/AssaySpecimenConfigImpl.java b/study/src/org/labkey/study/model/AssaySpecimenConfigImpl.java index 74e3f71bc66..e319c4eabb0 100644 --- a/study/src/org/labkey/study/model/AssaySpecimenConfigImpl.java +++ b/study/src/org/labkey/study/model/AssaySpecimenConfigImpl.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; /** * Represents an assay/specimen configuration for a study @@ -278,4 +279,19 @@ public static AssaySpecimenConfigImpl fromJSON(@NotNull JSONObject o, Container return assay; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AssaySpecimenConfigImpl that = (AssaySpecimenConfigImpl) o; + return _rowId == that._rowId && Objects.equals(_assayName, that._assayName) && Objects.equals(_dataset, that._dataset) && Objects.equals(_description, that._description) && Objects.equals(_source, that._source) && Objects.equals(_locationId, that._locationId) && Objects.equals(_primaryTypeId, that._primaryTypeId) && Objects.equals(_derivativeTypeId, that._derivativeTypeId) && Objects.equals(_tubeType, that._tubeType) && Objects.equals(_lab, that._lab) && Objects.equals(_sampleType, that._sampleType) && Objects.equals(_sampleQuantity, that._sampleQuantity) && Objects.equals(_sampleUnits, that._sampleUnits); + } + + @Override + public int hashCode() + { + return Objects.hash(_rowId, _assayName, _dataset, _description, _source, _locationId, _primaryTypeId, _derivativeTypeId, _tubeType, _lab, _sampleType, _sampleQuantity, _sampleUnits); + } } diff --git a/study/src/org/labkey/study/model/CohortImpl.java b/study/src/org/labkey/study/model/CohortImpl.java index ac7e8d8c911..94dcabb1a04 100644 --- a/study/src/org/labkey/study/model/CohortImpl.java +++ b/study/src/org/labkey/study/model/CohortImpl.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; /** @@ -179,4 +180,19 @@ public static CohortImpl fromJSON(@NotNull JSONObject o) return cohort; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CohortImpl cohort = (CohortImpl) o; + return _rowId == cohort._rowId && _enrolled == cohort._enrolled && Objects.equals(_lsid, cohort._lsid) && Objects.equals(_subjectCount, cohort._subjectCount) && Objects.equals(_description, cohort._description); + } + + @Override + public int hashCode() + { + return Objects.hash(_rowId, _lsid, _enrolled, _subjectCount, _description); + } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index c51cb1ffa36..1331f7b721b 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -332,7 +332,7 @@ public void clearCache(Container c) @Override public void clearCache(StudyImpl obj) { - super.clearCache(obj); + clearCache(obj.getContainer()); // Need to clear /~ALL plus / entries clearCachedStudies(); } }; @@ -400,13 +400,13 @@ private class DatasetHelper @Override public void clearCache(Container c) { - super.clearCache(c); + super.clearCache(); // Big hammer, but we're caching datasets in multiple containers } @Override public void clearCache(DatasetDefinition obj) { - super.clearCache(obj.getContainer()); + super.clearCache(); // Big hammer, but we're caching datasets in multiple containers } }; @@ -514,7 +514,7 @@ public StudyImpl getStudy(@NotNull Container c) while (true) { List studies = _studyHelper.get(c); - if (studies == null || studies.size() == 0) + if (studies == null || studies.isEmpty()) return null; else if (studies.size() > 1) throw new IllegalStateException("Only one study is allowed per container"); @@ -2805,10 +2805,16 @@ public void deleteAllStudyData(Container c, User user) Table.delete(StudySchema.getInstance().getTableInfoUploadLog(), containerFilter); assert deletedTables.add(StudySchema.getInstance().getTableInfoUploadLog()); Table.delete(_datasetHelper.getTableInfo(), containerFilter); + DbCache.trackRemove(_datasetHelper.getTableInfo()); + _datasetHelper.clearCache(c); assert deletedTables.add(_datasetHelper.getTableInfo()); Table.delete(_visitHelper.getTableInfo(), containerFilter); + DbCache.trackRemove(_visitHelper.getTableInfo()); + _visitHelper.clearCache(c); assert deletedTables.add(_visitHelper.getTableInfo()); Table.delete(_studyHelper.getTableInfo(), containerFilter); + DbCache.trackRemove(_studyHelper.getTableInfo()); + _studyHelper.clearCache(c); assert deletedTables.add(_studyHelper.getTableInfo()); // participant lists @@ -2831,7 +2837,9 @@ public void deleteAllStudyData(Container c, User user) assert deletedTables.add(StudySchema.getInstance().getTableInfoVisitAliases()); Table.delete(SCHEMA.getTableInfoParticipant(), containerFilter); assert deletedTables.add(SCHEMA.getTableInfoParticipant()); - Table.delete(StudySchema.getInstance().getTableInfoCohort(), containerFilter); + Table.delete(_cohortHelper.getTableInfo(), containerFilter); + DbCache.trackRemove(_cohortHelper.getTableInfo()); + _cohortHelper.clearCache(c); assert deletedTables.add(StudySchema.getInstance().getTableInfoCohort()); Table.delete(StudySchema.getInstance().getTableInfoParticipantView(), containerFilter); assert deletedTables.add(StudySchema.getInstance().getTableInfoParticipantView()); @@ -4148,7 +4156,7 @@ else if (!study.isDataspaceStudy()) TableInfo table = StudySchema.getInstance().getTableInfoParticipant(); ArrayList containers = new SqlSelector(table.getSchema(), new SQLFragment("SELECT container FROM study.participant WHERE participantid=?",ptid)) .getArrayList(String.class); - if (containers.size() == 0) + if (containers.isEmpty()) return null; else if (containers.size() == 1) return ContainerManager.getForId(containers.get(0)); @@ -4432,7 +4440,7 @@ private static void indexDataset(@Nullable IndexTask task, DatasetDefinition dsd public static void indexParticipants(final IndexTask task, @NotNull final Container c, @Nullable List ptids, @Nullable Date modifiedSince) { - if (null != ptids && ptids.size() == 0) + if (null != ptids && ptids.isEmpty()) return; final StudyImpl study = StudyManager.getInstance().getStudy(c); diff --git a/study/src/org/labkey/study/model/VisitImpl.java b/study/src/org/labkey/study/model/VisitImpl.java index 2d69878d25f..483276dff35 100644 --- a/study/src/org/labkey/study/model/VisitImpl.java +++ b/study/src/org/labkey/study/model/VisitImpl.java @@ -36,32 +36,23 @@ import java.io.Serializable; import java.math.BigDecimal; -import java.math.MathContext; import java.math.RoundingMode; import java.text.DecimalFormat; import java.text.NumberFormat; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; -/** - * User: brittp - * Date: Jan 6, 2006 - * Time: 10:28:55 AM - */ public class VisitImpl extends AbstractStudyEntity implements Cloneable, Serializable, Visit { // standard strings to use in URLs etc - public static final String VISITKEY = "visitRowId"; - public static final String SEQUENCEKEY = "sequenceNum"; + public static final String VISIT_KEY = "visitRowId"; public static final double DEMOGRAPHICS_VISIT = -1; // Sequence numbers and protocol day are currently stored as NUMERIC(15, 4); below are all the related constants. - private static final int PRECISION = 15; private static final int MAX_SCALE = 4; - private static final double SCALE_FACTOR = Math.pow(10, MAX_SCALE); private static final NumberFormat SEQUENCE_FORMAT = new DecimalFormat("0.0###"); - private static final MathContext ROUNDING_CONTEXT = new MathContext(PRECISION); private int _rowId = 0; private BigDecimal _sequenceMin = BigDecimal.ZERO; @@ -470,6 +461,21 @@ public String toString() return (null != _label ? _label + " (" : "(") + getSequenceString() + ")"; } + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + VisitImpl visit = (VisitImpl) o; + return _rowId == visit._rowId && _chronologicalOrder == visit._chronologicalOrder && Objects.equals(_sequenceMin, visit._sequenceMin) && Objects.equals(_sequenceMax, visit._sequenceMax) && Objects.equals(_protocolDay, visit._protocolDay) && Objects.equals(_typeCode, visit._typeCode) && Objects.equals(_visitDateDatasetid, visit._visitDateDatasetid) && Objects.equals(_cohortId, visit._cohortId) && _sequenceHandling == visit._sequenceHandling && Objects.equals(_description, visit._description); + } + + @Override + public int hashCode() + { + return Objects.hash(_rowId, _sequenceMin, _sequenceMax, _protocolDay, _typeCode, _visitDateDatasetid, _cohortId, _chronologicalOrder, _sequenceHandling, _description); + } + public static class TestCase extends Assert { @Test diff --git a/study/src/org/labkey/study/reports/StudyCrosstabReport.java b/study/src/org/labkey/study/reports/StudyCrosstabReport.java index 678bbb46844..01ffdeed3b7 100644 --- a/study/src/org/labkey/study/reports/StudyCrosstabReport.java +++ b/study/src/org/labkey/study/reports/StudyCrosstabReport.java @@ -52,7 +52,7 @@ protected ReportQueryView createQueryView(ViewContext context, ReportDescriptor { final String queryName = descriptor.getProperty(QueryParam.queryName.toString()); final String viewName = descriptor.getProperty(QueryParam.viewName.toString()); - final String visitRowId = descriptor.getProperty(VisitImpl.VISITKEY); + final String visitRowId = descriptor.getProperty(VisitImpl.VISIT_KEY); ReportQueryView view = ReportQueryViewFactory.get().generateQueryView(context, descriptor, queryName, viewName); diff --git a/study/src/org/labkey/study/view/createCrosstabReport.jsp b/study/src/org/labkey/study/view/createCrosstabReport.jsp index 91fda59d4ab..2eb3478493f 100644 --- a/study/src/org/labkey/study/view/createCrosstabReport.jsp +++ b/study/src/org/labkey/study/view/createCrosstabReport.jsp @@ -58,7 +58,7 @@
Visit - <% for (VisitImpl visit : bean.getVisits()) diff --git a/study/src/org/labkey/study/view/overview.jsp b/study/src/org/labkey/study/view/overview.jsp index a8b8b504346..16e5253ba28 100644 --- a/study/src/org/labkey/study/view/overview.jsp +++ b/study/src/org/labkey/study/view/overview.jsp @@ -353,7 +353,7 @@ else if (userCanRead) { ActionURL datasetLink = new ActionURL(DatasetAction.class, container); - datasetLink.addParameter(VisitImpl.VISITKEY, visit.getRowId()); + datasetLink.addParameter(VisitImpl.VISIT_KEY, visit.getRowId()); datasetLink.addParameter(Dataset.DATASETKEY, dataset.getDatasetId()); if (selectedCohort != null) bean.cohortFilter.addURLParameters(study, datasetLink, null); From 1948a7039f54fd5cfd66b724b6e214ec154622f1 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 10 May 2024 07:40:24 -0700 Subject: [PATCH 04/32] Finish merge --- api/src/org/labkey/api/security/LimitedUser.java | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/org/labkey/api/security/LimitedUser.java b/api/src/org/labkey/api/security/LimitedUser.java index 80143087c83..1c44bfa3315 100644 --- a/api/src/org/labkey/api/security/LimitedUser.java +++ b/api/src/org/labkey/api/security/LimitedUser.java @@ -132,7 +132,6 @@ public void testElevatedUser() int siteRolesCount = user.getSiteRoles().size(); User elevated = ElevatedUser.getElevatedUser(user); assertEquals(groupCount, elevated.getGroups().size()); - assertEquals(user.getGroups() + " vs. " + elevated.getGroups(), groupCount, elevated.getGroups().stream().count()); assertEquals(roleCount, elevated.getAssignedRoles(c.getPolicy()).size()); assertEquals(siteRolesCount, elevated.getSiteRoles().size()); } From d5d84940ca06d37971220cbb783c9262b6ee950b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 May 2024 10:28:09 -0700 Subject: [PATCH 05/32] Fix most dataset caching issues (temporarily). Fix warnings in dataset import test. --- .../org/labkey/api/study/StudyCache.java | 8 +- .../study/model/DatasetImportTestCase.jsp | 123 ++++++++---------- 2 files changed, 64 insertions(+), 67 deletions(-) diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index 85905de0951..bcc137a3726 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -39,7 +39,7 @@ public class StudyCache private static DatabaseCache getCache(TableInfo tinfo, boolean create) { Path cacheKey = tinfo.getNotificationKey(); - assert null != cacheKey : "DbCache not supported for " + tinfo; + assert null != cacheKey : "StudyCache not supported for " + tinfo; synchronized(CACHES) { @@ -81,6 +81,9 @@ public static Object getCached(TableInfo tinfo, Container c, Object cacheKey) DatabaseCache cache = getCache(tinfo, false); Object newObj = cache != null ? cache.get(getCacheName(c, cacheKey)) : null; + if (oldObj instanceof Dataset) + return oldObj; + assert Objects.equals(oldObj, newObj); return newObj; @@ -96,6 +99,9 @@ public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoa DatabaseCache cache2 = getCache(tinfo, true); Object newObj = cache2 != null ? cache2.get(getCacheName(c, cacheKey), null, loader) : null; + if (cache2.toString().contains("DataSet")) + return oldObj; + assert Objects.equals(oldObj, newObj); return newObj; diff --git a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp index cbe88fee263..cb3a8fd6094 100644 --- a/study/src/org/labkey/study/model/DatasetImportTestCase.jsp +++ b/study/src/org/labkey/study/model/DatasetImportTestCase.jsp @@ -33,6 +33,7 @@ <%@ page import="org.labkey.api.gwt.client.model.PropertyValidatorType" %> <%@ page import="org.labkey.api.module.FolderTypeManager" %> <%@ page import="org.labkey.api.qc.DataState" %> +<%@ page import="org.labkey.api.qc.QCStateManager" %> <%@ page import="org.labkey.api.query.BatchValidationException" %> <%@ page import="org.labkey.api.query.FieldKey" %> <%@ page import="org.labkey.api.query.QueryService" %> @@ -74,7 +75,6 @@ <%@ page import="static org.junit.Assert.*" %> <%@ page import="static org.labkey.study.model.StudyManager.TEST_LOGGER" %> <%@ page import="static org.labkey.study.dataset.DatasetAuditProvider.DATASET_AUDIT_EVENT" %> -<%@ page import="org.labkey.api.qc.QCStateManager" %> <%@ page extends="org.labkey.api.jsp.JspTest.DRT" %> @@ -265,7 +265,7 @@ public void test() throws Throwable catch (BatchValidationException x) { List l = x.getRowErrors(); - if (null != l && l.size() > 0) + if (!l.isEmpty()) throw l.get(0); throw x; } @@ -275,7 +275,6 @@ public void test() throws Throwable } } - private void _testDatasetUpdateService(StudyImpl study) throws Throwable { StudyQuerySchema ss = StudyQuerySchema.createSchema(study, _context.getUser()); @@ -290,10 +289,9 @@ private void _testDatasetUpdateService(StudyImpl study) throws Throwable List> rows = new ArrayList<>(); // insert one row - rows.clear(); errors.clear(); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++this.counterRow), "Value", 1.0)); List> ret = qus.insertRows(_context.getUser(), study.getContainer(), rows, errors, null, null); - String msg = errors.getRowErrors().size() > 0 ? errors.getRowErrors().get(0).toString() : "no message"; + String msg = !errors.getRowErrors().isEmpty() ? errors.getRowErrors().get(0).toString() : "no message"; assertFalse(msg, errors.hasErrors()); Map firstRowMap = ret.get(0); String lsidRet = (String)firstRowMap.get("lsid"); @@ -391,7 +389,6 @@ private void _testDatasetUpdateService(StudyImpl study) throws Throwable qus.insertRows(_context.getUser(), study.getContainer(), rows, errors, null, null); assertFalse(errors.hasErrors()); - // validation test rows.clear(); errors.clear(); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", 1, "Number", 101)); @@ -431,7 +428,6 @@ private void _testDatasetUpdateService(StudyImpl study) throws Throwable assert(ret.size() == 1); } - private void _testDatasetDetailedLogging(StudyImpl study) throws Throwable { // first get last audit record @@ -450,15 +446,15 @@ private void _testDatasetDetailedLogging(StudyImpl study) throws Throwable Date Jan1 = new Date(DateUtil.parseISODateTime("2011-01-01")); Date Feb1 = new Date(DateUtil.parseISODateTime("2011-02-01")); - List> rows = new ArrayList<>(); - Map config = Map.of(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, AuditBehaviorType.DETAILED); + Map config = Map.of(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, AuditBehaviorType.DETAILED); // INSERT - rows.clear(); errors.clear(); + List> rows = new ArrayList<>(); + errors.clear(); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", "Initial", "Value", 1.0)); List> ret = qus.insertRows(_context.getUser(), study.getContainer(), rows, errors, config, null); - String msg = errors.getRowErrors().size() > 0 ? errors.getRowErrors().get(0).toString() : "no message"; + String msg = !errors.getRowErrors().isEmpty() ? errors.getRowErrors().get(0).toString() : "no message"; assertFalse(msg, errors.hasErrors()); Map firstRowMap = ret.get(0); String lsidRet = (String)firstRowMap.get("lsid"); @@ -500,7 +496,7 @@ private void _testDatasetDetailedLogging(StudyImpl study) throws Throwable // and since merge is a different code path... rows.clear(); errors.clear(); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", "Merged", "Value", 3.0)); - int count = qus.mergeRows(_context.getUser(), study.getContainer(), new ListofMapsDataIterator(null,rows), errors, config, null); + int count = qus.mergeRows(_context.getUser(), study.getContainer(), new ListofMapsDataIterator(null, rows), errors, config, null); assertEquals(1, count); events = AuditLogService.get().getAuditEvents(study.getContainer(),_context.getUser(),DATASET_AUDIT_EVENT,f,new Sort("-RowId")); @@ -532,7 +528,7 @@ private void _testImportDatasetDataAllowImportGuid(Study study) throws Throwable List> rows = new ArrayList<>(); String guid = "GUUUUID"; - Map map = PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++, "GUID", guid); + Map map = Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++, "GUID", guid); importRowVerifyGuid(def, map, tt, TEST_LOGGER); // duplicate row @@ -544,16 +540,16 @@ private void _testImportDatasetDataAllowImportGuid(Study study) throws Throwable // assertTrue(-1 != errors.get(0).indexOf("duplicate key value violates unique constraint")); //same participant, guid, different sequenceNum - importRowVerifyGuid(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++, "GUID", guid), tt, TEST_LOGGER); + importRowVerifyGuid(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++, "GUID", guid), tt, TEST_LOGGER); // same GUID,sequenceNum, different different participant - importRowVerifyGuid(def, PageFlowUtil.map("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum, "GUID", guid), tt, TEST_LOGGER); + importRowVerifyGuid(def, Map.of("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum, "GUID", guid), tt, TEST_LOGGER); //same subject, sequenceNum, GUID not provided - importRowVerifyGuid(def, PageFlowUtil.map("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum), tt, TEST_LOGGER); + importRowVerifyGuid(def, Map.of("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum), tt, TEST_LOGGER); //repeat: should still work - importRowVerifyGuid(def, PageFlowUtil.map("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum), tt, TEST_LOGGER); + importRowVerifyGuid(def, Map.of("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum), tt, TEST_LOGGER); } private void _testImportDatasetData(Study study) throws Throwable @@ -566,11 +562,10 @@ private void _testImportDatasetData(Study study) throws Throwable Date Jan1 = new Date(DateUtil.parseISODateTime("2011-01-01")); Date Feb1 = new Date(DateUtil.parseISODateTime("2011-02-01")); - List> rows = new ArrayList<>(); // insert one row - rows.clear(); - rows.add((Map)PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++)); + List> rows = new ArrayList<>(); + rows.add(Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++)); importRows(def, rows); try (Results results = new TableSelector(tt).getResults()) @@ -584,35 +579,35 @@ private void _testImportDatasetData(Study study) throws Throwable importRows(def, rows, "Duplicates were found"); // different participant - Map map2 = PageFlowUtil.map("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum++); + Map map2 = Map.of("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum++); importRow(def, map2); - importRow(def, PageFlowUtil.map("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "B2", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 2.0, "SequenceNum", sequenceNum++)); // different date - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Feb1, "Measure", "Test"+(counterRow), "Value", "X", "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "A1", "Date", Feb1, "Measure", "Test"+(counterRow), "Value", "X", "SequenceNum", sequenceNum++)); // different measure - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", "X", "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test" + (++counterRow), "Value", "X", "SequenceNum", sequenceNum++)); // duplicates in batch rows.clear(); - rows.add((Map)PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum)); - rows.add((Map)PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 1.0, "SequenceNum", sequenceNum++)); + rows.add(Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum)); + rows.add(Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 1.0, "SequenceNum", sequenceNum++)); //study:Label: Only one row is allowed for each Subject/Visit/Measure Triple. Duplicates were found in the database or imported data.; Duplicate: Subject = A1Date = Sat Jan 01 00:00:00 PST 2011, Measure = Test3 importRows(def, rows, "Duplicates were found in the database or imported data"); // missing participantid - importRow(def, PageFlowUtil.map("SubjectId", null, "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++), "required", "SubjectID"); + importRow(def, PageFlowUtil.mapInsensitive("SubjectId", null, "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++), "required", "SubjectID"); // missing date if (study==_studyDateBased) //irrelevant for sequential visits - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", null, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++), "required", "date"); + importRow(def, PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", null, "Measure", "Test"+(++counterRow), "Value", 1.0, "SequenceNum", sequenceNum++), "required", "date"); // missing required property field - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", null, "Value", 1.0, "SequenceNum", sequenceNum++), "required", "Measure"); + importRow(def, PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", null, "Value", 1.0, "SequenceNum", sequenceNum++), "required", "Measure"); // legal MV indicator - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "X", "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "X", "SequenceNum", sequenceNum++)); // count rows with "X" final MutableInt Xcount = new MutableInt(0); @@ -622,7 +617,7 @@ private void _testImportDatasetData(Study study) throws Throwable }); // legal MV indicator - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", null, "ValueMVIndicator", "X", "SequenceNum", sequenceNum++)); + importRow(def, PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", null, "ValueMVIndicator", "X", "SequenceNum", sequenceNum++)); // should have two rows with "X" final MutableInt XcountAgain = new MutableInt(0); @@ -633,18 +628,18 @@ private void _testImportDatasetData(Study study) throws Throwable assertEquals(Xcount.intValue() + 1, XcountAgain.intValue()); // illegal MV indicator - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "N/A", "SequenceNum", sequenceNum++), "Value"); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "N/A", "SequenceNum", sequenceNum++), "Value"); // conversion test - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "100", "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", "100", "SequenceNum", sequenceNum++)); // validation test - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1, "Number", 101, "SequenceNum", sequenceNum++), "is invalid"); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(++counterRow), "Value", 1, "Number", 101, "SequenceNum", sequenceNum++), "is invalid"); - importRow(def, PageFlowUtil.map("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 1, "Number", 99, "SequenceNum", sequenceNum++)); + importRow(def, Map.of("SubjectId", "A1", "Date", Jan1, "Measure", "Test"+(counterRow), "Value", 1, "Number", 99, "SequenceNum", sequenceNum++)); } -private void importRowVerifyKey(Dataset def, Map map, TableInfo tt, String key, @NotNull Logger logger, String... expectedErrors) throws Exception +private void importRowVerifyKey(Dataset def, Map map, TableInfo tt, String key, @NotNull Logger logger, String... expectedErrors) throws Exception { importRow(def, map, logger, expectedErrors); String expectedKey = (String) map.get(key); @@ -656,7 +651,7 @@ private void importRowVerifyKey(Dataset def, Map map, TableInfo tt, String key, } } -private void importRowVerifyGuid(Dataset def, Map map, TableInfo tt, @NotNull Logger logger, String... expectedErrors) throws Exception +private void importRowVerifyGuid(Dataset def, Map map, TableInfo tt, @NotNull Logger logger, String... expectedErrors) throws Exception { if (map.containsKey("GUID")) importRowVerifyKey(def, map, tt, "GUID", logger, expectedErrors); @@ -668,7 +663,7 @@ private void importRowVerifyGuid(Dataset def, Map map, TableInfo tt, @NotNull Lo { rs.last(); String actualKey = rs.getString("GUID"); - assertTrue("No GUID generated when null GUID provided", actualKey.length() > 0); + assertFalse("No GUID generated when null GUID provided", actualKey.isEmpty()); } } } @@ -703,12 +698,12 @@ private void importRows(Dataset def, List> rows, @Nullable L } } -private void importRow(Dataset def, Map map, String... expectedErrors) throws Exception +private void importRow(Dataset def, Map map, String... expectedErrors) throws Exception { importRow(def, map, null, expectedErrors); } -private void importRow(Dataset def, Map map, @Nullable Logger logger, String... expectedErrors) throws Exception +private void importRow(Dataset def, Map map, @Nullable Logger logger, String... expectedErrors) throws Exception { importRows(def, Arrays.asList(map), logger, expectedErrors); } @@ -720,14 +715,13 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable TableInfo tt = def.getTableInfo(_context.getUser()); Date Feb1 = new Date(DateUtil.parseISODateTime("2011-02-01")); - List rows = new ArrayList(); - TimepointType time = study.getTimepointType(); + List> rows = new ArrayList<>(); if (time == TimepointType.VISIT) { // insert one row w/visit - importRow(def, PageFlowUtil.map("SubjectId", "A1", "SequenceNum", 1.0, "Measure", "Test"+(++counterRow), "Value", 1.0)); + importRow(def, Map.of("SubjectId", "A1", "SequenceNum", 1.0, "Measure", "Test"+(++counterRow), "Value", 1.0)); try (ResultSet rs = new TableSelector(tt).getResultSet()) { @@ -736,8 +730,7 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable } // insert one row w/o visit - rows.clear(); - rows.add(PageFlowUtil.map("SubjectId", "A2", "Measure", "Test"+(++counterRow), "Value", 1.0)); + rows.add(Map.of("SubjectId", "A2", "Measure", "Test"+(++counterRow), "Value", 1.0)); importRows(def, rows); try (ResultSet rs = new TableSelector(tt).getResultSet()) @@ -753,8 +746,7 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable else { // insert one row w/ date - rows.clear(); - rows.add(PageFlowUtil.map("SubjectId", "A1", "Date", Feb1, "Measure", "Test"+(++counterRow), "Value", 1.0)); + rows.add(Map.of("SubjectId", "A1", "Date", Feb1, "Measure", "Test"+(++counterRow), "Value", 1.0)); importRows(def, rows); try (ResultSet rs = new TableSelector(tt).getResultSet()) @@ -763,7 +755,7 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable assertEquals(Feb1, new java.util.Date(rs.getTimestamp("date").getTime())); } - Map map = PageFlowUtil.map("SubjectId", "A2", "Measure", "Test"+(++counterRow), "Value", 1.0); + Map map = Map.of("SubjectId", "A2", "Measure", "Test"+(++counterRow), "Value", 1.0); importRow(def, map); try (ResultSet rs = new TableSelector(tt).getResultSet()) @@ -771,7 +763,7 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable assertTrue(rs.next()); // issue 47344 : demographics date no longer required or initialized with the study start date if ("A2".equals(rs.getString("SubjectId"))) - assertEquals(null, rs.getTimestamp("date")); + assertNull(rs.getTimestamp("date")); } } } @@ -781,27 +773,26 @@ private void _testImportDemographicDatasetData(Study study) throws Throwable */ private void _testDaysSinceStartCalculation(Study study) throws Throwable { - StudyQuerySchema ss = StudyQuerySchema.createSchema((StudyImpl) study, _context.getUser()); - Dataset dem = createDataset(study, "Dem", true); - Dataset ds = createDataset(study, "DS", false); + TimepointType time = study.getTimepointType(); - TableInfo tableInfo = ss.getTable(ds.getName()); - QueryUpdateService qus = tableInfo.getUpdateService(); + if (time == TimepointType.DATE) + { + StudyQuerySchema ss = StudyQuerySchema.createSchema((StudyImpl) study, _context.getUser()); + Dataset dem = createDataset(study, "Dem", true); + Dataset ds = createDataset(study, "DS", false); - Date Feb1 = new Date(DateUtil.parseISODateTime("2016-02-01")); - List rows = new ArrayList(); - List errors = new ArrayList<>(); + TableInfo tableInfo = ss.getTable(ds.getName()); + QueryUpdateService qus = tableInfo.getUpdateService(); - TimepointType time = study.getTimepointType(); + Date Feb1 = new Date(DateUtil.parseISODateTime("2016-02-01")); + List errors = new ArrayList<>(); - Map validValues = new HashMap<>(); - validValues.put("A1", 0); - validValues.put("A2", 1); + Map validValues = new HashMap<>(); + validValues.put("A1", 0); + validValues.put("A2", 1); - if (time == TimepointType.DATE) - { // insert rows with a startDate - rows.clear(); + List> rows = new ArrayList<>(); errors.clear(); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A1", "Date", Feb1, "Measure", "Test"+(++counterRow), "Value", 1.0, "StartDate", Feb1)); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A2", "Date", Feb1, "Measure", "Test"+(++counterRow), "Value", 1.0, "StartDate", Feb1)); @@ -814,7 +805,7 @@ private void _testDaysSinceStartCalculation(Study study) throws Throwable rows.add(PageFlowUtil.mapInsensitive("SubjectId", "A2", "Date", "2016-02-02 10:00", "Measure", "Test"+(++counterRow), "Value", 1.0)); List> ret = qus.insertRows(_context.getUser(), study.getContainer(), rows, qusErrors, null, null); - String msg = qusErrors.getRowErrors().size() > 0 ? qusErrors.getRowErrors().get(0).toString() : "no message"; + String msg = !qusErrors.getRowErrors().isEmpty() ? qusErrors.getRowErrors().get(0).toString() : "no message"; assertFalse(msg, qusErrors.hasErrors()); try (ResultSet rs = new TableSelector(tableInfo).getResultSet()) @@ -846,7 +837,7 @@ private void _testDatasetTransformExport(Study study) throws Throwable assertNotNull(qus); // insert one row - List rows = new ArrayList(); + List> rows = new ArrayList<>(); Date jan1 = new Date(DateUtil.parseISODateTime("2012-01-01")); rows.add(PageFlowUtil.mapInsensitive("SubjectId", "DS1", "Date", jan1, "Measure", "Test" + (++this.counterRow), "Value", 0.0)); List> ret = qus.insertRows(_context.getUser(), study.getContainer(), rows, errors, null, null); From 28d0ee4fbf2b876fcd2985a49ccbbabb0f7c1652 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 May 2024 10:37:40 -0700 Subject: [PATCH 06/32] Clear last dataset caching error? --- study/src/org/labkey/study/model/StudyManager.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 1331f7b721b..4108dc14346 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -400,12 +400,14 @@ private class DatasetHelper @Override public void clearCache(Container c) { + super.clearCache(c); super.clearCache(); // Big hammer, but we're caching datasets in multiple containers } @Override public void clearCache(DatasetDefinition obj) { + super.clearCache(obj); super.clearCache(); // Big hammer, but we're caching datasets in multiple containers } }; From 2120d86dad4fe57cf59af148501fbb1536a38c10 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 May 2024 13:33:22 -0700 Subject: [PATCH 07/32] Remove exceptions for dataset caching comparisons --- study/api-src/org/labkey/api/study/StudyCache.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index bcc137a3726..0b5661c05e0 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -81,9 +81,6 @@ public static Object getCached(TableInfo tinfo, Container c, Object cacheKey) DatabaseCache cache = getCache(tinfo, false); Object newObj = cache != null ? cache.get(getCacheName(c, cacheKey)) : null; - if (oldObj instanceof Dataset) - return oldObj; - assert Objects.equals(oldObj, newObj); return newObj; @@ -99,9 +96,6 @@ public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoa DatabaseCache cache2 = getCache(tinfo, true); Object newObj = cache2 != null ? cache2.get(getCacheName(c, cacheKey), null, loader) : null; - if (cache2.toString().contains("DataSet")) - return oldObj; - assert Objects.equals(oldObj, newObj); return newObj; From adbc6f2d99a274b635ab8d749b2d124e2eaa00f5 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 May 2024 13:44:56 -0700 Subject: [PATCH 08/32] Add equals()/hashCode() to SpecimenRequest --- .../requirements/SpecimenRequest.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/specimen/src/org/labkey/specimen/requirements/SpecimenRequest.java b/specimen/src/org/labkey/specimen/requirements/SpecimenRequest.java index c59f9fe7a5f..fc6023f0320 100644 --- a/specimen/src/org/labkey/specimen/requirements/SpecimenRequest.java +++ b/specimen/src/org/labkey/specimen/requirements/SpecimenRequest.java @@ -30,12 +30,8 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; +import java.util.Objects; -/** - * User: brittp - * Date: Feb 8, 2006 - * Time: 1:50:53 PM - */ public class SpecimenRequest extends AbstractStudyCachable implements RequirementOwner { private Container _container; @@ -47,7 +43,7 @@ public class SpecimenRequest extends AbstractStudyCachable impl private int _modifiedBy; private long _modified; private String _comments; - private Integer _destinationSiteId; // This is a locationId, but still needs to martch the column in the table + private Integer _destinationSiteId; // This is a locationId, but still needs to match the column in the table private boolean _hidden; @Override @@ -222,4 +218,19 @@ public void setEntityId(String entityId) { _entityId = entityId; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SpecimenRequest that = (SpecimenRequest) o; + return _rowId == that._rowId && _statusId == that._statusId && _createdBy == that._createdBy && _created == that._created && _modifiedBy == that._modifiedBy && _modified == that._modified && _hidden == that._hidden && Objects.equals(_container, that._container) && Objects.equals(_entityId, that._entityId) && Objects.equals(_comments, that._comments) && Objects.equals(_destinationSiteId, that._destinationSiteId); + } + + @Override + public int hashCode() + { + return Objects.hash(_container, _entityId, _rowId, _statusId, _createdBy, _created, _modifiedBy, _modified, _comments, _destinationSiteId, _hidden); + } } From dc11c6350b8451e179b92f2fa1968f6ec013e607 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 12 May 2024 00:04:04 -0700 Subject: [PATCH 09/32] Log more info on mismatches --- .../org/labkey/core/view/TableViewFormTestCase.java | 2 -- study/api-src/org/labkey/api/study/StudyCache.java | 13 +++++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index 6469df1d979..b0cd32940b7 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -19,7 +19,6 @@ import org.junit.Assert; import org.junit.Test; import org.labkey.api.action.NullSafeBindException; -import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.TableViewForm; import org.labkey.api.data.TestSchema; @@ -92,7 +91,6 @@ public void testErrorHandling() Assert.assertTrue("Final form should be valid", tf.isValid()); } - @Test public void testDbOperations() throws SQLException { diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index 0b5661c05e0..1dac79981ca 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -25,6 +25,7 @@ import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.TableInfo; import org.labkey.api.util.Path; +import org.springframework.jdbc.core.metadata.Db2CallMetaDataProvider; import java.util.HashMap; import java.util.Map; @@ -81,7 +82,11 @@ public static Object getCached(TableInfo tinfo, Container c, Object cacheKey) DatabaseCache cache = getCache(tinfo, false); Object newObj = cache != null ? cache.get(getCacheName(c, cacheKey)) : null; - assert Objects.equals(oldObj, newObj); + if (!Objects.equals(oldObj, newObj)) + { + DbCache.logUnmatched(); + throw new IllegalStateException(oldObj + " != " + newObj); + } return newObj; } @@ -96,7 +101,11 @@ public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoa DatabaseCache cache2 = getCache(tinfo, true); Object newObj = cache2 != null ? cache2.get(getCacheName(c, cacheKey), null, loader) : null; - assert Objects.equals(oldObj, newObj); + if (!Objects.equals(oldObj, newObj)) + { + DbCache.logUnmatched(); + throw new IllegalStateException(oldObj + " != " + newObj); + } return newObj; } From a2cf46c2e3a14973d0328b592540cbb69a568f45 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 12 May 2024 06:31:29 -0700 Subject: [PATCH 10/32] Fix some specimen caching mismatches --- .../org/labkey/api/study/SpecimenService.java | 4 +- .../specimen/SpecimenRequestManager.java | 9 ++- .../labkey/specimen/SpecimenServiceImpl.java | 74 ++++++++++++++++++ .../specimen/model/SpecimenRequestEvent.java | 5 -- .../labkey/api/specimen/SpecimenManager.java | 76 +------------------ .../org/labkey/api/study/StudyCache.java | 1 - .../org/labkey/study/model/StudyManager.java | 7 +- 7 files changed, 92 insertions(+), 84 deletions(-) diff --git a/api/src/org/labkey/api/study/SpecimenService.java b/api/src/org/labkey/api/study/SpecimenService.java index ef0f0179c85..4e8661e33ba 100644 --- a/api/src/org/labkey/api/study/SpecimenService.java +++ b/api/src/org/labkey/api/study/SpecimenService.java @@ -119,7 +119,7 @@ static SpecimenService get() /** Hooks to allow other modules to control a few items about how specimens are treated */ interface SpecimenRequestCustomizer { - /** @return whether or not a specimen request must include at least one vial */ + /** @return whether a specimen request must include at least one vial */ boolean allowEmptyRequests(); /** @return null if users should always supply a destination site for a given request, or the site's id if they should all be the same */ @@ -140,4 +140,6 @@ interface SpecimenRequestCustomizer @Migrate // Remove after specimen module refactor (SpecimenImporter should call the impl) void fireSpecimensChanged(Container c, User user, Logger logger); + + void deleteAllSpecimenData(Container c, Set set, User user); } diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index ea24d5ebd1d..6a666ecd6d3 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -303,7 +303,7 @@ public SpecimenRequestInput[] getNewSpecimenRequestInputs(Container container, b String parentObjectLsid = getRequestInputObjectLsid(container); Map resourceProperties = OntologyManager.getPropertyObjects(container, parentObjectLsid); SpecimenRequestInput[] inputs = new SpecimenRequestInput[0]; - if (resourceProperties == null || resourceProperties.size() == 0) + if (resourceProperties == null || resourceProperties.isEmpty()) { if (createIfMissing) { @@ -748,7 +748,7 @@ private void updateVialCounts(Container container, User user, List vials) updateSql.append("\tFROM ").append(tableInfoVialSelectName).append("\n"); - if (vials != null && vials.size() > 0) + if (vials != null && !vials.isEmpty()) { Set specimenIds = new HashSet<>(); for (Vial vial : vials) @@ -789,6 +789,11 @@ public void clearCaches(Container c) clearGroupedValuesForColumn(c); } + void clearRequestStatusHelper(Container c) + { + _requestStatusHelper.clearCache(c); + } + private static class GroupedValueColumnHelper { private final String _viewColumnName; diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index 4f67449180d..d1146e198ce 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -20,27 +20,36 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; +import org.labkey.api.data.DbSchema; import org.labkey.api.data.PropertyManager; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.Lsid; import org.labkey.api.exp.api.ExpMaterial; +import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.exp.property.Domain; import org.labkey.api.pipeline.PipeRoot; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.pipeline.PipelineService; +import org.labkey.api.query.FieldKey; import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.specimen.DefaultSpecimenTablesTemplate; import org.labkey.api.specimen.SpecimenColumns; import org.labkey.api.specimen.SpecimenManagerNew; +import org.labkey.api.specimen.SpecimenMigrationService; import org.labkey.api.specimen.SpecimenSchema; import org.labkey.api.specimen.Vial; import org.labkey.api.specimen.importer.SimpleSpecimenImporter; import org.labkey.api.specimen.importer.SpecimenColumn; +import org.labkey.api.specimen.location.LocationCache; import org.labkey.api.specimen.model.SpecimenTablesProvider; import org.labkey.api.study.ParticipantVisit; import org.labkey.api.study.SpecimenChangeListener; @@ -461,4 +470,69 @@ public void fireSpecimensChanged(Container c, User user, Logger logger) for (SpecimenChangeListener l : _changeListeners) l.specimensChanged(c, user, logger); } + + @Override + public void deleteAllSpecimenData(Container c, Set set, User user) + { + // UNDONE: use transaction? + SimpleFilter containerFilter = SimpleFilter.createContainerFilter(c); + + Table.delete(SpecimenSchema.get().getTableInfoSampleRequestSpecimen(), containerFilter); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequestSpecimen()); + Table.delete(SpecimenSchema.get().getTableInfoSampleRequestEvent(), containerFilter); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequestEvent()); + Table.delete(SpecimenSchema.get().getTableInfoSampleRequest(), containerFilter); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequest()); + Table.delete(SpecimenSchema.get().getTableInfoSampleRequestStatus(), containerFilter); + DbCache.trackRemove(SpecimenSchema.get().getTableInfoSampleRequestStatus()); + SpecimenRequestManager.get().clearRequestStatusHelper(c); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequestStatus()); + + new SpecimenTablesProvider(c, null, null).deleteTables(); + LocationCache.clear(c); + + Table.delete(SpecimenSchema.get().getTableInfoSampleAvailabilityRule(), containerFilter); + assert set.add(SpecimenSchema.get().getTableInfoSampleAvailabilityRule()); + + SpecimenMigrationService SMS = SpecimenMigrationService.get(); + if (null != SMS) + SMS.purgeRequestRequirementsAndActors(c); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequestRequirement()); + assert set.add(SpecimenSchema.get().getTableInfoSampleRequestActor()); + + DbSchema expSchema = ExperimentService.get().getSchema(); + TableInfo tinfoMaterial = expSchema.getTable("Material"); + + ExpSampleType sampleType = SampleTypeService.get().getSampleType(c, SpecimenService.SAMPLE_TYPE_NAME); + + if (sampleType != null) + { + // Check if any of the samples are referenced in an experiment run + SQLFragment sql = new SQLFragment("SELECT m.RowId FROM "); + sql.append(ExperimentService.get().getTinfoMaterial(), "m"); + sql.append(" INNER JOIN "); + sql.append(ExperimentService.get().getTinfoMaterialInput(), "mi"); + sql.append(" ON m.RowId = mi.MaterialId AND m.CpasType = ?"); + sql.add(sampleType.getLSID()); + + if (new SqlSelector(ExperimentService.get().getSchema(), sql).exists()) + { + // If so, do the slow version of the delete that tears down runs + sampleType.delete(user); + } + else + { + // If not, do the quick version that just kills the samples themselves in the exp.Material table + SimpleFilter materialFilter = new SimpleFilter(containerFilter); + materialFilter.addCondition(FieldKey.fromParts("CpasType"), sampleType.getLSID()); + Table.delete(tinfoMaterial, materialFilter); + } + } + + // VIEW: if this view gets removed, remove this line + assert set.add(SpecimenSchema.get().getSchema().getTable("LockedSpecimens")); + + if (null != SMS) + SMS.clearGroupedValuesForColumn(c); + } } diff --git a/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java b/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java index 2549c382d36..32f32912675 100644 --- a/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java +++ b/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java @@ -24,11 +24,6 @@ import java.util.Date; -/** - * User: brittp - * Date: Feb 24, 2006 - * Time: 1:56:09 PM - */ public class SpecimenRequestEvent extends AbstractStudyCachable implements AttachmentParent { private int _rowId; diff --git a/study/api-src/org/labkey/api/specimen/SpecimenManager.java b/study/api-src/org/labkey/api/specimen/SpecimenManager.java index 6fd5177e824..2befed23985 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenManager.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenManager.java @@ -20,27 +20,18 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; -import org.labkey.api.data.DbSchema; import org.labkey.api.data.Filter; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; -import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; -import org.labkey.api.exp.api.ExpSampleType; -import org.labkey.api.exp.api.ExperimentService; -import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.module.Module; import org.labkey.api.module.ModuleLoader; import org.labkey.api.query.FieldKey; -import org.labkey.api.security.User; -import org.labkey.api.specimen.location.LocationCache; import org.labkey.api.specimen.model.SpecimenComment; -import org.labkey.api.specimen.model.SpecimenTablesProvider; -import org.labkey.api.study.SpecimenService; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; import org.labkey.api.study.TimepointType; @@ -52,7 +43,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; public class SpecimenManager { @@ -195,7 +185,7 @@ private SQLFragment getVisitRangeSql(Visit visit, TableInfo tinfoSpecimen, Strin SQLFragment sqlVisitRange = new SQLFragment(); sqlVisitRange.append(tinfoSpecimen.getColumn("VisitValue").getValueSql(specimenAlias)).append(" >= ? AND ") - .append(tinfoSpecimen.getColumn("VisitValue").getValueSql(specimenAlias)).append(" <= ?"); + .append(tinfoSpecimen.getColumn("VisitValue").getValueSql(specimenAlias)).append(" <= ?"); if (TimepointType.VISIT == study.getTimepointType()) { @@ -210,7 +200,7 @@ private SQLFragment getVisitRangeSql(Visit visit, TableInfo tinfoSpecimen, Strin Sort sort = new Sort(); sort.insertSortColumn(FieldKey.fromString("SequenceNum"), Sort.SortDirection.ASC); ArrayList visitValues = new TableSelector(columnInfo, filter, sort).getArrayList(Double.class); - if (0 == visitValues.size()) + if (visitValues.isEmpty()) { // No participant visits for this timepoint; return False return null; @@ -253,68 +243,6 @@ public void deleteSpecimen(@NotNull Vial vial, boolean clearCaches) } } - public void deleteAllSpecimenData(Container c, Set set, User user) - { - // UNDONE: use transaction? - SimpleFilter containerFilter = SimpleFilter.createContainerFilter(c); - - Table.delete(SpecimenSchema.get().getTableInfoSampleRequestSpecimen(), containerFilter); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequestSpecimen()); - Table.delete(SpecimenSchema.get().getTableInfoSampleRequestEvent(), containerFilter); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequestEvent()); - Table.delete(SpecimenSchema.get().getTableInfoSampleRequest(), containerFilter); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequest()); - Table.delete(SpecimenSchema.get().getTableInfoSampleRequestStatus(), containerFilter); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequestStatus()); - - new SpecimenTablesProvider(c, null, null).deleteTables(); - LocationCache.clear(c); - - Table.delete(SpecimenSchema.get().getTableInfoSampleAvailabilityRule(), containerFilter); - assert set.add(SpecimenSchema.get().getTableInfoSampleAvailabilityRule()); - - SpecimenMigrationService SMS = SpecimenMigrationService.get(); - if (null != SMS) - SMS.purgeRequestRequirementsAndActors(c); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequestRequirement()); - assert set.add(SpecimenSchema.get().getTableInfoSampleRequestActor()); - - DbSchema expSchema = ExperimentService.get().getSchema(); - TableInfo tinfoMaterial = expSchema.getTable("Material"); - - ExpSampleType sampleType = SampleTypeService.get().getSampleType(c, SpecimenService.SAMPLE_TYPE_NAME); - - if (sampleType != null) - { - // Check if any of the samples are referenced in an experiment run - SQLFragment sql = new SQLFragment("SELECT m.RowId FROM "); - sql.append(ExperimentService.get().getTinfoMaterial(), "m"); - sql.append(" INNER JOIN "); - sql.append(ExperimentService.get().getTinfoMaterialInput(), "mi"); - sql.append(" ON m.RowId = mi.MaterialId AND m.CpasType = ?"); - sql.add(sampleType.getLSID()); - - if (new SqlSelector(ExperimentService.get().getSchema(), sql).exists()) - { - // If so, do the slow version of the delete that tears down runs - sampleType.delete(user); - } - else - { - // If not, do the quick version that just kills the samples themselves in the exp.Material table - SimpleFilter materialFilter = new SimpleFilter(containerFilter); - materialFilter.addCondition(FieldKey.fromParts("CpasType"), sampleType.getLSID()); - Table.delete(tinfoMaterial, materialFilter); - } - } - - // VIEW: if this view gets removed, remove this line - assert set.add(SpecimenSchema.get().getSchema().getTable("LockedSpecimens")); - - if (null != SMS) - SMS.clearGroupedValuesForColumn(c); - } - public SpecimenComment[] getSpecimenCommentForSpecimen(Container container, String specimenHash) { return getSpecimenCommentForSpecimens(container, Collections.singleton(specimenHash)); diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index 1dac79981ca..b74561f1b21 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -25,7 +25,6 @@ import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.TableInfo; import org.labkey.api.util.Path; -import org.springframework.jdbc.core.metadata.Db2CallMetaDataProvider; import java.util.HashMap; import java.util.Map; diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 4108dc14346..8425047d195 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -147,6 +147,7 @@ import org.labkey.api.study.Dataset; import org.labkey.api.study.DataspaceContainerFilter; import org.labkey.api.study.QueryHelper; +import org.labkey.api.study.SpecimenService; import org.labkey.api.study.Study; import org.labkey.api.study.StudyCache; import org.labkey.api.study.StudyService; @@ -2789,7 +2790,9 @@ public void deleteAllStudyData(Container c, User user) // // specimens // - SpecimenManager.get().deleteAllSpecimenData(c, deletedTables, user); + SpecimenService ss = SpecimenService.get(); + if (null != ss) + ss.deleteAllSpecimenData(c, deletedTables, user); // // assay schedule @@ -2797,6 +2800,8 @@ public void deleteAllStudyData(Container c, User user) Table.delete(SCHEMA.getTableInfoAssaySpecimenVisit(), containerFilter); assert deletedTables.add(SCHEMA.getTableInfoAssaySpecimenVisit()); Table.delete(_assaySpecimenHelper.getTableInfo(), containerFilter); + DbCache.trackRemove(_assaySpecimenHelper.getTableInfo()); + _assaySpecimenHelper.clearCache(c); assert deletedTables.add(_assaySpecimenHelper.getTableInfo()); // From da58dca1efec68d84ca73feae9e87d5e974a1bde Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 12 May 2024 07:56:01 -0700 Subject: [PATCH 11/32] Remove TestTable caching. Move more specimen code into specimen module. Migrate participant cache. Uncache specimen status. --- .../org/labkey/api/cache/CachingTestCase.java | 77 ------------------- .../api/data/AtomicDatabaseInteger.java | 9 --- api/src/org/labkey/api/data/DbSchema.java | 4 - .../org/labkey/api/data/TableViewForm.java | 4 - .../labkey/core/CoreContainerListener.java | 2 - .../org/labkey/specimen/SpecimenManager.java | 62 +++++++++++++++ .../specimen/SpecimenRequestManager.java | 6 +- .../labkey/specimen/SpecimenServiceImpl.java | 4 +- .../actions/ShowUploadSpecimensAction.java | 3 +- .../actions/SpecimenApiController.java | 6 +- .../specimen/actions/SpecimenController.java | 12 +-- .../labkey/api/specimen/SpecimenManager.java | 29 ------- .../api/specimen/SpecimenManagerNew.java | 70 +---------------- .../api/specimen/query/SpecimenQueryView.java | 34 +++++++- study/src/org/labkey/study/StudySchema.java | 5 -- .../study/importer/AssayScheduleImporter.java | 1 + .../importer/DefaultStudyDesignImporter.java | 2 + .../org/labkey/study/model/StudyManager.java | 30 ++++++++ 18 files changed, 147 insertions(+), 213 deletions(-) diff --git a/api/src/org/labkey/api/cache/CachingTestCase.java b/api/src/org/labkey/api/cache/CachingTestCase.java index 7f40ecf937f..ac654316ff3 100644 --- a/api/src/org/labkey/api/cache/CachingTestCase.java +++ b/api/src/org/labkey/api/cache/CachingTestCase.java @@ -2,89 +2,12 @@ import org.junit.Assert; import org.junit.Test; -import org.labkey.api.data.DbSchema; -import org.labkey.api.data.DbScope; -import org.labkey.api.data.Table; -import org.labkey.api.data.TableInfo; -import org.labkey.api.data.TestSchema; -import org.labkey.api.util.JunitUtil; -import org.labkey.api.util.TestContext; - -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; public class CachingTestCase extends Assert { @Test public void testCaching() { - TestSchema test = TestSchema.getInstance(); - DbSchema testSchema = test.getSchema(); - TableInfo testTable = test.getTableInfoTestTable(); - TestContext ctx = TestContext.get(); - - assertNotNull(testTable); - DbCache.clear(testTable); - DbCache.trackRemove(testTable); - - Map mm = new HashMap<>(); // Modifiable map - Map umm = Collections.unmodifiableMap(mm); // Unmodifiable map - mm.put("DatetimeNotNull", new Date()); - mm.put("BitNotNull", Boolean.TRUE); - mm.put("Text", "Added by Caching Test Suite"); - mm.put("IntNotNull", 0); - mm.put("Container", JunitUtil.getTestContainer()); - mm = Table.insert(ctx.getUser(), testTable, mm); - DbCache.trackRemove(testTable); - Integer rowId1 = ((Integer) mm.get("RowId")); - - String key = "RowId" + rowId1; - DbCache.put(testTable, key, umm); - Map m2 = (Map) DbCache.get(testTable, key); - assertEquals(umm, m2); - - //Does cache get cleared on delete - Table.delete(testTable, rowId1); - DbCache.trackRemove(testTable); - m2 = (Map) DbCache.get(testTable, key); - assertNull(m2); - - //Does cache get cleared on insert - mm.remove("RowId"); - mm = Table.insert(ctx.getUser(), testTable, mm); - DbCache.trackRemove(testTable); - int rowId2 = ((Integer) mm.get("RowId")); - key = "RowId" + rowId2; - DbCache.put(testTable, key, umm); - mm.remove("RowId"); - mm = Table.insert(ctx.getUser(), testTable, mm); - DbCache.trackRemove(testTable); - int rowId3 = ((Integer) mm.get("RowId")); - m2 = (Map) DbCache.get(testTable, key); - assertNull(m2); - - //Make sure things are not inserted in transaction - mm.remove("RowId"); - String key2; - try (DbScope.Transaction ignored = testSchema.getScope().beginTransaction()) - { - mm = Table.insert(ctx.getUser(), testTable, mm); - DbCache.trackRemove(testTable); - int rowId4 = ((Integer) mm.get("RowId")); - key2 = "RowId" + rowId4; - DbCache.put(testTable, key2, umm); - } - m2 = (Map) DbCache.get(testTable, key2); - assertNull(m2); - - // Clean up - Table.delete(testTable, rowId2); - DbCache.trackRemove(testTable); - Table.delete(testTable, rowId3); - DbCache.trackRemove(testTable); - DbCache.logUnmatched(); } } diff --git a/api/src/org/labkey/api/data/AtomicDatabaseInteger.java b/api/src/org/labkey/api/data/AtomicDatabaseInteger.java index 143e11604be..49ace3108d5 100644 --- a/api/src/org/labkey/api/data/AtomicDatabaseInteger.java +++ b/api/src/org/labkey/api/data/AtomicDatabaseInteger.java @@ -20,7 +20,6 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.labkey.api.cache.DbCache; import org.labkey.api.security.User; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.TestContext; @@ -30,11 +29,6 @@ import java.util.List; import java.util.Map; -/** - * User: adam - * Date: Oct 22, 2010 - * Time: 7:52:17 AM - */ public class AtomicDatabaseInteger { private final TableInfo _table; @@ -44,7 +38,6 @@ public class AtomicDatabaseInteger // Acts like an AtomicInteger, but uses the database for synchronization. This is convenient for scenarios where // multiple threads (eventually, even different servers) might attempt an update but only one should succeed. - // Currently only implements compareAndSet(), but could add other methods from AtomicInteger. public AtomicDatabaseInteger(ColumnInfo targetColumn, @Nullable Container container, Object rowId) { if (targetColumn.getJavaObjectClass() != Integer.class) @@ -145,7 +138,6 @@ public void setup() map.put("BitNotNull", true); map = Table.insert(user, table, map); - DbCache.trackRemove(table); _rowId = (Integer)map.get("RowId"); _adi = new AtomicDatabaseInteger(table.getColumn("IntNotNull"), c, _rowId); @@ -209,7 +201,6 @@ public void cleanup() { TableInfo table = TestSchema.getInstance().getTableInfoTestTable(); Table.delete(table, _rowId); - DbCache.trackRemove(table); } } } diff --git a/api/src/org/labkey/api/data/DbSchema.java b/api/src/org/labkey/api/data/DbSchema.java index 9f108f335d7..dfd9e1e8ef1 100644 --- a/api/src/org/labkey/api/data/DbSchema.java +++ b/api/src/org/labkey/api/data/DbSchema.java @@ -21,7 +21,6 @@ import org.junit.After; import org.junit.Assert; import org.junit.Test; -import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.dialect.JdbcMetaDataLocator; import org.labkey.api.data.dialect.SqlDialect; @@ -565,7 +564,6 @@ public void testTransactions() throws Exception { assertTrue("Not in transaction when should be.", testSchema.getScope().isTransactionActive()); m = Table.insert(ctx.getUser(), testTable, m); - DbCache.trackRemove(testTable); rowId = ((Integer) m.get("RowId")); assertNotNull("Inserted Row doesn't have Id", rowId); assertTrue(rowId != 0); @@ -585,7 +583,6 @@ public void testTransactions() throws Exception { m.put("IntNotNull", 1); m = Table.update(ctx.getUser(), testTable, m, rowId); - DbCache.trackRemove(testTable); assertEquals("Update is consistent in transaction?", 1, (int)m.get("IntNotNull")); } @@ -597,7 +594,6 @@ public void testTransactions() throws Exception assertEquals("Rollback did not appear to work.", 0, (int)m.get("IntNotNull")); Table.delete(testTable, rowId); - DbCache.trackRemove(testTable); } } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 8b4ea6c998b..cc30d8c8f00 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -33,7 +33,6 @@ import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.ReturnUrlForm; import org.labkey.api.action.SpringActionController; -import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.query.SchemaKey; import org.labkey.api.security.permissions.DeletePermission; @@ -163,7 +162,6 @@ public void doInsert() throws SQLException set("container", _c.getId()); Map newMap = Table.insert(_user, _tinfo, getTypedValues()); - if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); // Temporary hack setTypedValues(newMap, false); } @@ -189,7 +187,6 @@ public void doUpdate() throws SQLException Object[] pkVal = getPkVals(); Map newMap = Table.update(_user, _tinfo, getTypedValues(), pkVal); - if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); setTypedValues(newMap, true); } @@ -220,7 +217,6 @@ public void doDelete() if (null != pkVal && null != pkVal[0]) { Table.delete(_tinfo, pkVal); - if (_tinfo.getName().equals("TestTable")) DbCache.trackRemove(_tinfo); } else //Hmm, throw an exception here???? _log.warn("Nothing to delete for table " + _tinfo.getName() + " on request " + _request.getRequestURI()); diff --git a/core/src/org/labkey/core/CoreContainerListener.java b/core/src/org/labkey/core/CoreContainerListener.java index f001c99ae18..59d1780441a 100644 --- a/core/src/org/labkey/core/CoreContainerListener.java +++ b/core/src/org/labkey/core/CoreContainerListener.java @@ -22,7 +22,6 @@ import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.provider.ContainerAuditProvider; -import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; @@ -66,7 +65,6 @@ public void containerDeleted(Container c, User user) // Delete any rows in test.TestTable associated with this container SimpleFilter containerFilter = SimpleFilter.createContainerFilter(c); Table.delete(TestSchema.getInstance().getTableInfoTestTable(), containerFilter); - DbCache.trackRemove(TestSchema.getInstance().getTableInfoTestTable()); // Data States Table.delete(CoreSchema.getInstance().getTableInfoDataStates(), containerFilter); diff --git a/specimen/src/org/labkey/specimen/SpecimenManager.java b/specimen/src/org/labkey/specimen/SpecimenManager.java index 16463ee9044..8165c3b5298 100644 --- a/specimen/src/org/labkey/specimen/SpecimenManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenManager.java @@ -25,6 +25,7 @@ import org.labkey.api.specimen.Vial; import org.labkey.api.specimen.location.LocationImpl; import org.labkey.api.specimen.location.LocationManager; +import org.labkey.api.specimen.model.PrimaryType; import org.labkey.api.specimen.model.SpecimenComment; import org.labkey.api.study.Cohort; import org.labkey.api.study.Study; @@ -411,4 +412,65 @@ private List getDerivativeTypes(final Container container, @Null forEachMap(map -> derivativeTypes.add(new DerivativeType(container, map))); return derivativeTypes; } + + public boolean isSpecimensEmpty(Container container, User user) + { + TableSelector selector = SpecimenManagerNew.get().getSpecimensSelector(container, user, null); + return !selector.exists(); + } + + public Vial getVial(Container container, User user, String globalUniqueId) + { + SimpleFilter filter = new SimpleFilter(new SimpleFilter.SQLClause("LOWER(GlobalUniqueId) = LOWER(?)", new Object[] { globalUniqueId })); + List matches = SpecimenManagerNew.get().getVials(container, user, filter); + if (matches == null || matches.isEmpty()) + return null; + if (matches.size() > 1) + { + // we apparently have two specimens with IDs that differ only in case; do a case-sensitive check + // here to find the right one: + for (Vial vial : matches) + { + if (vial.getGlobalUniqueId().equals(globalUniqueId)) + return vial; + } + throw new IllegalStateException("Expected at least one vial to exactly match the specified global unique ID: " + globalUniqueId); + } + else + return matches.get(0); + } + + /** Looks for any specimens that have the given id as a globalUniqueId */ + public PrimaryType getPrimaryType(Container c, int rowId) + { + List primaryTypes = SpecimenManagerNew.get().getPrimaryTypes(c, new SimpleFilter(FieldKey.fromParts("RowId"), rowId), null); + if (!primaryTypes.isEmpty()) + return primaryTypes.get(0); + return null; + } + + public List getRequestableVials(Container container, User user, Set vialRowIds) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addInClause(FieldKey.fromParts("RowId"), vialRowIds).addCondition(FieldKey.fromString("available"), true); + return SpecimenManagerNew.get().getVials(container, user, filter); + } + + public Map> getVialsForSpecimenHashes(Container container, User user, Collection hashes, boolean onlyAvailable) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addInClause(FieldKey.fromParts("SpecimenHash"), hashes); + if (onlyAvailable) + filter.addCondition(FieldKey.fromParts("Available"), true); + List vials = SpecimenManagerNew.get().getVials(container, user, filter); + Map> map = new HashMap<>(); + for (Vial vial : vials) + { + String hash = vial.getSpecimenHash(); + List keyVials = map.computeIfAbsent(hash, k -> new ArrayList<>()); + keyVials.add(vial); + } + + return map; + } } diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index 6a666ecd6d3..cd433a3b8d5 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -10,9 +10,11 @@ import org.labkey.api.cache.BlockingCache; import org.labkey.api.cache.CacheLoader; import org.labkey.api.cache.CacheManager; +import org.labkey.api.cache.DbCache; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.DatabaseCache; +import org.labkey.api.data.DbSchemaCache; import org.labkey.api.data.DbScope; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; @@ -113,6 +115,8 @@ public List getRequestStatuses(Container c, User user) try (var ignore = SpringActionController.ignoreSqlUpdates()) { Table.insert(user, _requestStatusHelper.getTableInfo(), notYetSubmittedStatus); + DbCache.trackRemove(_requestStatusHelper.getTableInfo()); + _requestStatusHelper.clearCache(c); } statuses = _requestStatusHelper.get(c, "SortOrder"); } @@ -1247,7 +1251,7 @@ private void deleteRequestEvents(SpecimenRequest request) public RequestedSpecimens getRequestableBySpecimenHash(Container c, User user, Set formValues, Integer preferredLocation) throws AmbiguousLocationException { - Map> vialsByHash = SpecimenManagerNew.get().getVialsForSpecimenHashes(c, user, formValues, true); + Map> vialsByHash = SpecimenManager.get().getVialsForSpecimenHashes(c, user, formValues, true); if (vialsByHash == null || vialsByHash.isEmpty()) return new RequestedSpecimens(Collections.emptyList()); diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index d1146e198ce..ab7c761a2e0 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -220,7 +220,7 @@ public void setDate(Date date) @Override public ParticipantVisit getSampleInfo(Container studyContainer, User user, String sampleId) { - Vial match = SpecimenManagerNew.get().getVial(studyContainer, user, sampleId); + Vial match = SpecimenManager.get().getVial(studyContainer, user, sampleId); if (match != null) return new StudyParticipantVisit(studyContainer, sampleId, match.getPtid(), match.getVisitValue(), match.getDrawTimestamp()); else @@ -233,7 +233,7 @@ public Set getSampleInfo(Container studyContainer, User user, if (null != studyContainer && null != StringUtils.trimToNull(participantId) && null != date) { List matches = SpecimenManager.get().getVials(studyContainer, user, participantId, date); - if (matches.size() > 0) + if (!matches.isEmpty()) { Set result = new HashSet<>(); for (Vial match : matches) diff --git a/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java b/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java index ffe324433d1..9f7a40d8bc2 100644 --- a/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java +++ b/specimen/src/org/labkey/specimen/actions/ShowUploadSpecimensAction.java @@ -48,6 +48,7 @@ import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; +import org.labkey.specimen.SpecimenManager; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; @@ -78,7 +79,7 @@ public ModelAndView getView(UploadSpecimensForm form, boolean reshow, BindExcept if (!settings.isSimple()) return HttpView.redirect(PageFlowUtil. urlProvider(PipelineUrls.class).urlBrowse(container)); - boolean isEmpty = SpecimenManagerNew.get().isSpecimensEmpty(container, getUser()); + boolean isEmpty = SpecimenManager.get().isSpecimensEmpty(container, getUser()); if (isEmpty) { form.setNoSpecimens(true); diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenApiController.java b/specimen/src/org/labkey/specimen/actions/SpecimenApiController.java index c119ce402dd..e091c85752a 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenApiController.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenApiController.java @@ -126,7 +126,7 @@ private List> getSpecimenListResponse(List vials) vialProperties.put("primaryTypeId", vial.getPrimaryTypeId()); if (vial.getPrimaryTypeId() != null) { - PrimaryType primaryType = SpecimenManagerNew.get().getPrimaryType(vial.getContainer(), vial.getPrimaryTypeId()); + PrimaryType primaryType = SpecimenManager.get().getPrimaryType(vial.getContainer(), vial.getPrimaryTypeId()); if (primaryType != null) vialProperties.put("primaryType", primaryType.getPrimaryType()); } @@ -331,7 +331,7 @@ public class GetProvidingLocations extends ReadOnlyApiAction> vialsByHash = SpecimenManagerNew.get().getVialsForSpecimenHashes(getContainer(), getUser(), + Map> vialsByHash = SpecimenManager.get().getVialsForSpecimenHashes(getContainer(), getUser(), PageFlowUtil.set(form.getSpecimenHashes()), true); Collection preferredLocations = StudyUtils.getPreferredProvidingLocations(vialsByHash.values()); final Map response = new HashMap<>(); @@ -486,7 +486,7 @@ private Vial getVial(String vialId, String idType) { Vial vial; if (IdTypes.GlobalUniqueId.name().equals(idType)) - vial = SpecimenManagerNew.get().getVial(getContainer(), getUser(), vialId); + vial = SpecimenManager.get().getVial(getContainer(), getUser(), vialId); else if (IdTypes.RowId.name().equals(idType)) { try diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenController.java b/specimen/src/org/labkey/specimen/actions/SpecimenController.java index e43e6ecc4da..8abd490aec4 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenController.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenController.java @@ -1071,7 +1071,7 @@ public ModelAndView getView(PipelineForm form, BindException bindErrors) } ImportSpecimensBean bean = new ImportSpecimensBean(getContainer(), archives, form.getPath(), form.getFile(), errors); - boolean isEmpty = SpecimenManagerNew.get().isSpecimensEmpty(getContainer(), getUser()); + boolean isEmpty = SpecimenManager.get().isSpecimensEmpty(getContainer(), getUser()); if (isEmpty) { bean.setNoSpecimens(true); @@ -1157,7 +1157,7 @@ public ModelAndView getView(SpecimenEventForm form, BindException errors) { if (form.getId() != null && form.getTargetStudy() != null) { - Vial vial = SpecimenManagerNew.get().getVial(form.getTargetStudy(), getUser(), form.getId()); + Vial vial = SpecimenManager.get().getVial(form.getTargetStudy(), getUser(), form.getId()); if (vial != null) { ActionURL url = new ActionURL(SpecimenEventsAction.class, form.getTargetStudy()).addParameter("id", vial.getRowId()); @@ -2007,7 +2007,7 @@ public List getSpecimensFromPost(boolean fromGroupedView, boolean onlyAvai if (fromGroupedView) { Map> keyToVialMap = - SpecimenManagerNew.get().getVialsForSpecimenHashes(getContainer(), getUser(), formValues, onlyAvailable); + SpecimenManager.get().getVialsForSpecimenHashes(getContainer(), getUser(), formValues, onlyAvailable); List vials = new ArrayList<>(); for (List vialList : keyToVialMap.values()) vials.addAll(vialList); @@ -2216,7 +2216,7 @@ private RequestedSpecimens getRequestableByVialRowIds(Set rowIds, Contai { Set ids = new HashSet<>(); Arrays.stream(toLongArray(rowIds)).forEach(ids::add); - List requestedSpecimens = SpecimenManagerNew.get().getRequestableVials(container, user, ids); + List requestedSpecimens = SpecimenManager.get().getRequestableVials(container, user, ids); return new RequestedSpecimens(requestedSpecimens); } @@ -2229,7 +2229,7 @@ private RequestedSpecimens getRequestableByVialGlobalUniqueIds(Set globa List vials = new ArrayList<>(); for (String globalUniqueId : globalUniqueIds) { - Vial match = SpecimenManagerNew.get().getVial(container, user, globalUniqueId); + Vial match = SpecimenManager.get().getVial(container, user, globalUniqueId); if (match != null) vials.add(match); } @@ -3561,7 +3561,7 @@ protected int importData(DataLoader dl, FileStream file, String originalName, Ba boolean hasSpecimenError = false; for (String id : globalIds) { - Vial vial = SpecimenManagerNew.get().getVial(getContainer(), getUser(), id); + Vial vial = SpecimenManager.get().getVial(getContainer(), getUser(), id); if (vial == null) { errorList.add("Specimen " + id + " not found."); diff --git a/study/api-src/org/labkey/api/specimen/SpecimenManager.java b/study/api-src/org/labkey/api/specimen/SpecimenManager.java index 2befed23985..d0a473499b1 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenManager.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenManager.java @@ -40,9 +40,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; public class SpecimenManager { @@ -71,33 +69,6 @@ public long getMaxExternalId(Container container) return new SqlSelector(tableInfo.getSchema(), sql).getArrayList(Long.class).get(0); } - public Map getSpecimenCounts(Container container, Collection specimenHashes) - { - TableInfo tableInfoSpecimen = SpecimenSchema.get().getTableInfoSpecimen(container); - - SQLFragment extraClause = null; - if (specimenHashes != null) - { - extraClause = new SQLFragment(" WHERE SpecimenHash "); - tableInfoSpecimen.getSqlDialect().appendInClauseSql(extraClause, specimenHashes); - } - - final Map map = new HashMap<>(); - - SQLFragment sql = new SQLFragment("SELECT SpecimenHash, CAST(AvailableCount AS Integer) AS AvailableCount FROM "); - sql.append(tableInfoSpecimen.getFromSQL("")); - if (extraClause != null) - { - sql.append(extraClause); - } - new SqlSelector(SpecimenSchema.get().getSchema(), sql).forEach(rs -> { - String specimenHash = rs.getString("SpecimenHash"); - map.put(specimenHash, rs.getInt("AvailableCount")); - }); - - return map; - } - public int getSpecimenCountForVisit(Visit visit) { Container container = visit.getContainer(); diff --git a/study/api-src/org/labkey/api/specimen/SpecimenManagerNew.java b/study/api-src/org/labkey/api/specimen/SpecimenManagerNew.java index a9809a8b7bd..8c69a10e5e2 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenManagerNew.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenManagerNew.java @@ -20,10 +20,7 @@ import org.labkey.api.view.NotFoundException; import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Set; public class SpecimenManagerNew @@ -39,12 +36,6 @@ public static SpecimenManagerNew get() return INSTANCE; } - public boolean isSpecimensEmpty(Container container, User user) - { - TableSelector selector = getSpecimensSelector(container, user, null); - return !selector.exists(); - } - public List getVials(Container container, User user, Set vialRowIds) { // Take a set to eliminate dups - issue 26940 @@ -70,12 +61,12 @@ public List getVials(final Container container, final User user, SimpleFil final List vials = new ArrayList<>(); getSpecimensSelector(container, user, filter) - .forEachMap(map -> vials.add(new Vial(container, map))); + .forEachMap(map -> vials.add(new Vial(container, map))); return vials; } - private TableSelector getSpecimensSelector(final Container container, final User user, SimpleFilter filter) + public TableSelector getSpecimensSelector(final Container container, final User user, SimpleFilter filter) { Study study = StudyService.get().getStudy(container); if (study == null) @@ -96,42 +87,12 @@ public Vial getVial(Container container, User user, long rowId) return vials.get(0); } - /** Looks for any specimens that have the given id as a globalUniqueId */ - public Vial getVial(Container container, User user, String globalUniqueId) - { - SimpleFilter filter = new SimpleFilter(new SimpleFilter.SQLClause("LOWER(GlobalUniqueId) = LOWER(?)", new Object[] { globalUniqueId })); - List matches = getVials(container, user, filter); - if (matches == null || matches.isEmpty()) - return null; - if (matches.size() > 1) - { - // we apparently have two specimens with IDs that differ only in case; do a case sensitive check - // here to find the right one: - for (Vial vial : matches) - { - if (vial.getGlobalUniqueId().equals(globalUniqueId)) - return vial; - } - throw new IllegalStateException("Expected at least one vial to exactly match the specified global unique ID: " + globalUniqueId); - } - else - return matches.get(0); - } - - public PrimaryType getPrimaryType(Container c, int rowId) - { - List primaryTypes = getPrimaryTypes(c, new SimpleFilter(FieldKey.fromParts("RowId"), rowId), null); - if (!primaryTypes.isEmpty()) - return primaryTypes.get(0); - return null; - } - public List getPrimaryTypes(Container c) { return getPrimaryTypes(c, null, new Sort("ExternalId")); } - private List getPrimaryTypes(final Container container, @Nullable SimpleFilter filter, @Nullable Sort sort) + public List getPrimaryTypes(final Container container, @Nullable SimpleFilter filter, @Nullable Sort sort) { final List primaryTypes = new ArrayList<>(); new TableSelector(SpecimenSchema.get().getTableInfoSpecimenPrimaryType(container), filter, sort). @@ -139,13 +100,6 @@ private List getPrimaryTypes(final Container container, @Nullable S return primaryTypes; } - public List getRequestableVials(Container container, User user, Set vialRowIds) - { - SimpleFilter filter = SimpleFilter.createContainerFilter(container); - filter.addInClause(FieldKey.fromParts("RowId"), vialRowIds).addCondition(FieldKey.fromString("available"), true); - return getVials(container, user, filter); - } - public SpecimenTypeSummary getSpecimenTypeSummary(Container container, @NotNull User user) { UserSchema querySchema = SpecimenQuerySchema.get(StudyService.get().getStudy(container), user); @@ -235,24 +189,6 @@ public SpecimenTypeSummary getSpecimenTypeSummary(Container container, @NotNull return new SpecimenTypeSummary(container, rows); } - - public Map> getVialsForSpecimenHashes(Container container, User user, Collection hashes, boolean onlyAvailable) - { - SimpleFilter filter = SimpleFilter.createContainerFilter(container); - filter.addInClause(FieldKey.fromParts("SpecimenHash"), hashes); - if (onlyAvailable) - filter.addCondition(FieldKey.fromParts("Available"), true); - List vials = getVials(container, user, filter); - Map> map = new HashMap<>(); - for (Vial vial : vials) - { - String hash = vial.getSpecimenHash(); - List keyVials = map.computeIfAbsent(hash, k -> new ArrayList<>()); - keyVials.add(vial); - } - - return map; - } } diff --git a/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java b/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java index eb716f3935f..2b2a77a1ba2 100644 --- a/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java +++ b/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java @@ -36,6 +36,7 @@ import org.labkey.api.data.SimpleDisplayColumn; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.query.DetailsURL; @@ -544,19 +545,19 @@ public Map getSampleCounts(RenderContext ctx) specimenHashes.add(rs.getString("SpecimenHash")); if (specimenHashes.size() > 150) { - _availableSpecimenCounts.putAll(SpecimenManager.get().getSpecimenCounts(ctx.getContainer(), specimenHashes)); + _availableSpecimenCounts.putAll(getSpecimenCounts(ctx.getContainer(), specimenHashes)); specimenHashes = new HashSet<>(); } } while (rs.next()); if (!specimenHashes.isEmpty()) { - _availableSpecimenCounts.putAll(SpecimenManager.get().getSpecimenCounts(ctx.getContainer(), specimenHashes)); + _availableSpecimenCounts.putAll(getSpecimenCounts(ctx.getContainer(), specimenHashes)); } } else { - _availableSpecimenCounts.putAll(SpecimenManager.get().getSpecimenCounts(ctx.getContainer(), null)); + _availableSpecimenCounts.putAll(getSpecimenCounts(ctx.getContainer(), null)); } rs.absolute(originalRow); } @@ -568,6 +569,33 @@ public Map getSampleCounts(RenderContext ctx) return _availableSpecimenCounts; } + private Map getSpecimenCounts(Container container, Collection specimenHashes) + { + TableInfo tableInfoSpecimen = SpecimenSchema.get().getTableInfoSpecimen(container); + + SQLFragment extraClause = null; + if (specimenHashes != null) + { + extraClause = new SQLFragment(" WHERE SpecimenHash "); + tableInfoSpecimen.getSqlDialect().appendInClauseSql(extraClause, specimenHashes); + } + + final Map map = new HashMap<>(); + + SQLFragment sql = new SQLFragment("SELECT SpecimenHash, CAST(AvailableCount AS Integer) AS AvailableCount FROM "); + sql.append(tableInfoSpecimen.getFromSQL("")); + if (extraClause != null) + { + sql.append(extraClause); + } + new SqlSelector(SpecimenSchema.get().getSchema(), sql).forEach(rs -> { + String specimenHash = rs.getString("SpecimenHash"); + map.put(specimenHash, rs.getInt("AvailableCount")); + }); + + return map; + } + private static Sort createDefaultSort(ViewType viewType) { if (viewType.isVialView()) diff --git a/study/src/org/labkey/study/StudySchema.java b/study/src/org/labkey/study/StudySchema.java index a553817bf0e..5c1ce0ff16a 100644 --- a/study/src/org/labkey/study/StudySchema.java +++ b/study/src/org/labkey/study/StudySchema.java @@ -29,11 +29,6 @@ import java.util.Collection; -/** - * User: brittp - * Date: Jan 6, 2006 - * Time: 10:36:43 AM - */ public class StudySchema { private static final StudySchema instance = new StudySchema(); diff --git a/study/src/org/labkey/study/importer/AssayScheduleImporter.java b/study/src/org/labkey/study/importer/AssayScheduleImporter.java index 1e992c00cc0..a2cfab9b8c4 100644 --- a/study/src/org/labkey/study/importer/AssayScheduleImporter.java +++ b/study/src/org/labkey/study/importer/AssayScheduleImporter.java @@ -96,6 +96,7 @@ public void process(StudyImportContext ctx, VirtualFile root, BindException erro // assay specimen table StudyQuerySchema.TablePackage assaySpecimenTablePackage = schema.getTablePackage(ctx, projectSchema, StudyQuerySchema.ASSAY_SPECIMEN_TABLE_NAME, null); importTableData(ctx, vf, assaySpecimenTablePackage, _assaySpecimenTransform, null); + StudyManager.getInstance().clearAssaySpecimenCache(assaySpecimenTablePackage.getContainer()); // assay specimen visit table StudyQuerySchema.TablePackage assaySpecimenVisitTablePackage = schema.getTablePackage(ctx, projectSchema, StudyQuerySchema.ASSAY_SPECIMEN_VISIT_TABLE_NAME, null); diff --git a/study/src/org/labkey/study/importer/DefaultStudyDesignImporter.java b/study/src/org/labkey/study/importer/DefaultStudyDesignImporter.java index 8f05e1fe768..721fd308c9b 100644 --- a/study/src/org/labkey/study/importer/DefaultStudyDesignImporter.java +++ b/study/src/org/labkey/study/importer/DefaultStudyDesignImporter.java @@ -20,6 +20,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.admin.ImportException; import org.labkey.api.admin.InvalidFileException; +import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.AbstractTableInfo; import org.labkey.api.data.Container; @@ -76,6 +77,7 @@ protected void deleteData(Container container, TableInfo tableInfo) throws Impor if (tableInfo instanceof FilteredTable) { Table.delete(((FilteredTable)tableInfo).getRealTable(), SimpleFilter.createContainerFilter(container)); + if (tableInfo.getName().equals("AssaySpecimen")) DbCache.trackRemove(((FilteredTable)tableInfo).getRealTable()); } } catch (RuntimeSQLException e) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 8425047d195..9010637b842 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -26,6 +26,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.Constants; import org.labkey.api.annotations.Migrate; import org.labkey.api.assay.AssayService; import org.labkey.api.attachments.Attachment; @@ -48,6 +49,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.CommitTaskOption; @@ -215,6 +217,7 @@ import java.util.Set; import java.util.TreeMap; import java.util.WeakHashMap; +import java.util.stream.Collectors; import static org.labkey.api.action.SpringActionController.ERROR_MSG; import static org.labkey.study.query.StudyQuerySchema.PERSONNEL_TABLE_NAME; @@ -239,6 +242,20 @@ public class StudyManager private final QueryHelper _cohortHelper; private final BlockingCache> _sharedProperties; + private final BlockingCache> _participantCache = DatabaseCache.get(StudySchema.getInstance().getScope(), Constants.getMaxContainers(), "Participants", new CacheLoader>() + { + @Override + public Map load(@NotNull Container c, @Nullable Object argument) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(c); + return Collections.unmodifiableMap( + new TableSelector(StudySchema.getInstance().getTableInfoParticipant(), filter, new Sort("ParticipantId")) + .stream(Participant.class) + .collect(Collectors.toMap(Participant::getParticipantId, participant -> participant)) + ); + } + }); + private static final String LSID_REQUIRED = "LSID_REQUIRED"; @@ -2748,6 +2765,7 @@ public void clearCaches(Container c, boolean unmaterializeDatasets) _datasetHelper.clearCache(c); DbCache.clear(StudySchema.getInstance().getTableInfoParticipant()); + DbCache.trackRemove(StudySchema.getInstance().getTableInfoParticipant()); for (StudyImpl substudy : StudyManager.getInstance().getAncillaryStudies(c)) clearCaches(substudy.getContainer(), unmaterializeDatasets); @@ -4123,12 +4141,23 @@ private Map getParticipantMap(Study study) participantMap = Collections.unmodifiableMap(participantMap); DbCache.put(StudySchema.getInstance().getTableInfoParticipant(), getParticipantCacheKey(study.getContainer()), participantMap, CacheManager.HOUR); } + Map participantMapNew = _participantCache.get(study.getContainer()); + + assert participantMap.equals(participantMapNew); + return participantMap; } public void clearParticipantCache(Container container) { DbCache.remove(StudySchema.getInstance().getTableInfoParticipant(), getParticipantCacheKey(container)); + DbCache.trackRemove(StudySchema.getInstance().getTableInfoParticipant()); + _participantCache.remove(container); + } + + public void clearAssaySpecimenCache(Container container) + { + _assaySpecimenHelper.clearCache(container); } public Collection getParticipants(Study study) @@ -4848,6 +4877,7 @@ public static class StudyUpgradeCode implements UpgradeCode * Issue : 46986. Move the study design domains to the project folder (if not already there), since * their URI references the project folder already. */ + @SuppressWarnings("unused") public static void moveDesignDomains(ModuleContext ctx) { if (ctx.isNewInstall()) From 36e554f13f99225f26f9ca4353976e6966a2ea4b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 12 May 2024 14:22:16 -0700 Subject: [PATCH 12/32] More specimen fixes --- specimen/src/org/labkey/specimen/SpecimenRequestManager.java | 5 +++++ specimen/src/org/labkey/specimen/SpecimenServiceImpl.java | 3 ++- study/src/org/labkey/study/model/StudyManager.java | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index cd433a3b8d5..05747ef9e74 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -798,6 +798,11 @@ void clearRequestStatusHelper(Container c) _requestStatusHelper.clearCache(c); } + void clearRequestHelper(Container c) + { + _requestHelper.clearCache(c); + } + private static class GroupedValueColumnHelper { private final String _viewColumnName; diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index ab7c761a2e0..8e3c28cad7a 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -43,7 +43,6 @@ import org.labkey.api.security.User; import org.labkey.api.specimen.DefaultSpecimenTablesTemplate; import org.labkey.api.specimen.SpecimenColumns; -import org.labkey.api.specimen.SpecimenManagerNew; import org.labkey.api.specimen.SpecimenMigrationService; import org.labkey.api.specimen.SpecimenSchema; import org.labkey.api.specimen.Vial; @@ -482,6 +481,8 @@ public void deleteAllSpecimenData(Container c, Set set, User user) Table.delete(SpecimenSchema.get().getTableInfoSampleRequestEvent(), containerFilter); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestEvent()); Table.delete(SpecimenSchema.get().getTableInfoSampleRequest(), containerFilter); + DbCache.trackRemove(SpecimenSchema.get().getTableInfoSampleRequest()); + SpecimenRequestManager.get().clearRequestHelper(c); assert set.add(SpecimenSchema.get().getTableInfoSampleRequest()); Table.delete(SpecimenSchema.get().getTableInfoSampleRequestStatus(), containerFilter); DbCache.trackRemove(SpecimenSchema.get().getTableInfoSampleRequestStatus()); diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 9010637b842..78899bd4487 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -1883,6 +1883,8 @@ public void clearParticipantVisitCaches(Study study) _visitHelper.clearCache(visitStudy.getContainer()); DbCache.clear(StudySchema.getInstance().getTableInfoParticipant()); + DbCache.trackRemove(StudySchema.getInstance().getTableInfoParticipant()); + _participantCache.remove(study.getContainer()); for (StudyImpl substudy : StudyManager.getInstance().getAncillaryStudies(study.getContainer())) clearParticipantVisitCaches(substudy); } From c1c8ec7d9b088845a6d0d1e33568c251e7e9fece Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 13 May 2024 00:00:10 -0700 Subject: [PATCH 13/32] Participant equals and mismatch logging --- .../org/labkey/study/model/Participant.java | 20 +++++++++++++++---- .../org/labkey/study/model/StudyManager.java | 11 +++++++--- .../query/StudyPropertiesUpdateService.java | 11 +--------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/study/src/org/labkey/study/model/Participant.java b/study/src/org/labkey/study/model/Participant.java index 00a47cd41f9..ae4f4700927 100644 --- a/study/src/org/labkey/study/model/Participant.java +++ b/study/src/org/labkey/study/model/Participant.java @@ -18,11 +18,8 @@ import org.labkey.api.data.Container; import java.util.Date; +import java.util.Objects; -/** - * User: brittp - * Created: Jan 17, 2008 10:26:32 AM - */ public class Participant { private Container _container; @@ -102,4 +99,19 @@ public void setInitialCohortId(Integer initialCohortId) { _initialCohortId = initialCohortId; } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Participant that = (Participant) o; + return Objects.equals(_container, that._container) && Objects.equals(_participantId, that._participantId) && Objects.equals(_enrollmentSiteId, that._enrollmentSiteId) && Objects.equals(_currentSiteId, that._currentSiteId) && Objects.equals(_startDate, that._startDate) && Objects.equals(_initialCohortId, that._initialCohortId) && Objects.equals(_currentCohortId, that._currentCohortId); + } + + @Override + public int hashCode() + { + return Objects.hash(_container, _participantId, _enrollmentSiteId, _currentSiteId, _startDate, _initialCohortId, _currentCohortId); + } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 78899bd4487..1aa4bb7effd 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -2751,7 +2751,6 @@ private void deleteDatasetType(Study study, User user, DatasetDefinition ds) } } - // Any container can be passed here (whether it contains a study or not). public void clearCaches(Container c, boolean unmaterializeDatasets) { @@ -2863,6 +2862,8 @@ public void deleteAllStudyData(Container c, User user) Table.delete(StudySchema.getInstance().getTableInfoVisitAliases(), containerFilter); assert deletedTables.add(StudySchema.getInstance().getTableInfoVisitAliases()); Table.delete(SCHEMA.getTableInfoParticipant(), containerFilter); + DbCache.trackRemove(SCHEMA.getTableInfoParticipant()); + _participantCache.remove(c); assert deletedTables.add(SCHEMA.getTableInfoParticipant()); Table.delete(_cohortHelper.getTableInfo(), containerFilter); DbCache.trackRemove(_cohortHelper.getTableInfo()); @@ -4145,9 +4146,13 @@ private Map getParticipantMap(Study study) } Map participantMapNew = _participantCache.get(study.getContainer()); - assert participantMap.equals(participantMapNew); + if (!Objects.equals(participantMap, participantMapNew)) + { + DbCache.logUnmatched(); + throw new IllegalStateException(participantMap + " != " + participantMapNew); + } - return participantMap; + return participantMapNew; } public void clearParticipantCache(Container container) diff --git a/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java b/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java index 19c2a012666..03b4113cc03 100644 --- a/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java +++ b/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java @@ -39,10 +39,6 @@ import java.util.Date; import java.util.Map; -/** - * User: jgarms - * Date: Aug 11, 2008 - */ public class StudyPropertiesUpdateService extends AbstractQueryUpdateService { public StudyPropertiesUpdateService(TableInfo table) @@ -50,7 +46,6 @@ public StudyPropertiesUpdateService(TableInfo table) super(table); } - @Override @SuppressWarnings("unchecked") protected Map getRow(User user, Container container, Map keys) throws QueryUpdateServiceException @@ -65,7 +60,6 @@ protected Map getRow(User user, Container container, Map updateRow(User user, Container container, Map row, @Nullable Map oldRow, @Nullable Map configParameters) throws ValidationException, QueryUpdateServiceException { @@ -141,7 +135,6 @@ else if (c.isUserEditable()) if (!updateRow.isEmpty()) Table.update(user, table.getRealTable(), updateRow, study.getContainer().getId()); - StudyManager.getInstance().clearCaches(container, false); // Reload the study object after flushing caches @@ -149,7 +142,7 @@ else if (c.isUserEditable()) if (recalculateTimepoints) { - // Blow away all of the existing visit info and rebuild it + // Blow away all existing visit info and rebuild it VisitManager manager = StudyManager.getInstance().getVisitManager(study); StudySchema schema = StudySchema.getInstance(); @@ -172,14 +165,12 @@ else if (recomputeStartDates && study.getTimepointType() == TimepointType.DATE) return getRow(user, container, null); } - @Override protected Map deleteRow(User user, Container container, Map oldRow) { throw new UnsupportedOperationException("You cannot delete all of a Study's properties"); } - @Override protected Map insertRow(User user, Container container, Map row) throws ValidationException, QueryUpdateServiceException { From 6b5384f25b5b9b0b4529fd05f7df316f49115838 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 13 May 2024 09:06:40 -0700 Subject: [PATCH 14/32] SpecimenRequestEvent equals()/hashCode() --- .../specimen/model/SpecimenRequestEvent.java | 16 +++++++++++++ .../org/labkey/study/model/StudyManager.java | 24 +++++++------------ .../query/StudyPropertiesUpdateService.java | 5 ++++ 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java b/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java index 32f32912675..1f826ee627e 100644 --- a/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java +++ b/specimen/src/org/labkey/specimen/model/SpecimenRequestEvent.java @@ -23,6 +23,7 @@ import org.labkey.api.study.AbstractStudyCachable; import java.util.Date; +import java.util.Objects; public class SpecimenRequestEvent extends AbstractStudyCachable implements AttachmentParent { @@ -155,4 +156,19 @@ public void setRequirementId(Integer requirementId) { return SpecimenRequestEventType.get(); } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SpecimenRequestEvent that = (SpecimenRequestEvent) o; + return _rowId == that._rowId && _createdBy == that._createdBy && _created == that._created && _requestId == that._requestId && Objects.equals(_entityId, that._entityId) && Objects.equals(_container, that._container) && Objects.equals(_requirementId, that._requirementId) && Objects.equals(_comments, that._comments) && Objects.equals(_entryType, that._entryType); + } + + @Override + public int hashCode() + { + return Objects.hash(_rowId, _entityId, _createdBy, _created, _container, _requestId, _requirementId, _comments, _entryType); + } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 1aa4bb7effd..34b1e65147d 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -242,26 +242,19 @@ public class StudyManager private final QueryHelper _cohortHelper; private final BlockingCache> _sharedProperties; - private final BlockingCache> _participantCache = DatabaseCache.get(StudySchema.getInstance().getScope(), Constants.getMaxContainers(), "Participants", new CacheLoader>() - { - @Override - public Map load(@NotNull Container c, @Nullable Object argument) - { - SimpleFilter filter = SimpleFilter.createContainerFilter(c); - return Collections.unmodifiableMap( - new TableSelector(StudySchema.getInstance().getTableInfoParticipant(), filter, new Sort("ParticipantId")) - .stream(Participant.class) - .collect(Collectors.toMap(Participant::getParticipantId, participant -> participant)) - ); - } + private final BlockingCache> _participantCache = DatabaseCache.get(StudySchema.getInstance().getScope(), Constants.getMaxContainers(), CacheManager.HOUR, "Participants", (c, argument) -> { + SimpleFilter filter = SimpleFilter.createContainerFilter(c); + return Collections.unmodifiableMap( + new TableSelector(StudySchema.getInstance().getTableInfoParticipant(), filter, new Sort("ParticipantId")) + .stream(Participant.class) + .collect(Collectors.toMap(Participant::getParticipantId, participant -> participant)) + ); }); private static final String LSID_REQUIRED = "LSID_REQUIRED"; - - protected StudyManager() + private StudyManager() { - // prevent external construction with a private default constructor _studyHelper = new QueryHelper<>(() -> StudySchema.getInstance().getTableInfoStudy(), StudyImpl.class) { @Override @@ -430,7 +423,6 @@ public void clearCache(DatasetDefinition obj) } }; - private DatasetHelper() { } diff --git a/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java b/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java index 03b4113cc03..d3257cc637f 100644 --- a/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java +++ b/study/src/org/labkey/study/query/StudyPropertiesUpdateService.java @@ -16,6 +16,7 @@ package org.labkey.study.query; import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; @@ -134,7 +135,11 @@ else if (c.isUserEditable()) } if (!updateRow.isEmpty()) + { Table.update(user, table.getRealTable(), updateRow, study.getContainer().getId()); + DbCache.trackRemove(table.getRealTable()); + } + StudyManager.getInstance().clearCaches(container, false); // Reload the study object after flushing caches From 27d74ba5e1951edee0e7b7cc7cf61aa283e788bd Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 13 May 2024 10:41:21 -0700 Subject: [PATCH 15/32] Clear request cache and track it --- specimen/src/org/labkey/specimen/SpecimenRequestManager.java | 5 +++++ specimen/src/org/labkey/specimen/SpecimenServiceImpl.java | 2 ++ 2 files changed, 7 insertions(+) diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index 05747ef9e74..ea7ee0f65b1 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -803,6 +803,11 @@ void clearRequestHelper(Container c) _requestHelper.clearCache(c); } + void clearRequestEventHelper(Container c) + { + _requestEventHelper.clearCache(c); + } + private static class GroupedValueColumnHelper { private final String _viewColumnName; diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index 8e3c28cad7a..a391f822aa6 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -479,6 +479,8 @@ public void deleteAllSpecimenData(Container c, Set set, User user) Table.delete(SpecimenSchema.get().getTableInfoSampleRequestSpecimen(), containerFilter); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestSpecimen()); Table.delete(SpecimenSchema.get().getTableInfoSampleRequestEvent(), containerFilter); + DbCache.trackRemove(SpecimenSchema.get().getTableInfoSampleRequestEvent()); + SpecimenRequestManager.get().clearRequestEventHelper(c); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestEvent()); Table.delete(SpecimenSchema.get().getTableInfoSampleRequest(), containerFilter); DbCache.trackRemove(SpecimenSchema.get().getTableInfoSampleRequest()); From 6710ae68ebe73e75bfebe9b536a70c0961e92744 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 14 May 2024 01:47:25 -0700 Subject: [PATCH 16/32] Address some IntelliJ warnings --- .../labkey/study/model/DatasetDomainKind.java | 8 ++-- .../org/labkey/study/model/StudyManager.java | 48 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/study/src/org/labkey/study/model/DatasetDomainKind.java b/study/src/org/labkey/study/model/DatasetDomainKind.java index 24bc0870e14..6cb545f3cfb 100644 --- a/study/src/org/labkey/study/model/DatasetDomainKind.java +++ b/study/src/org/labkey/study/model/DatasetDomainKind.java @@ -410,7 +410,7 @@ public Domain createDomain(GWTDomain domain, DatasetDomainKindProperties argumen if (name == null) throw new IllegalArgumentException("Dataset name cannot be empty."); String description = arguments.getDescription() != null ? arguments.getDescription() : domain.getDescription(); - String label = (arguments.getLabel() == null || arguments.getLabel().length() == 0) ? arguments.getName() : arguments.getLabel(); + String label = (arguments.getLabel() == null || arguments.getLabel().isEmpty()) ? arguments.getName() : arguments.getLabel(); Integer cohortId = arguments.getCohortId(); String tag = arguments.getTag(); Integer datasetId = arguments.getDatasetId(); @@ -556,7 +556,7 @@ private void validateDatasetProperties(DatasetDomainKindProperties datasetProper // Name related exceptions - if (name == null || name.length() == 0) + if (name == null || name.isEmpty()) throw new IllegalArgumentException("Dataset name cannot be empty."); if (name.length() > 200) @@ -587,7 +587,7 @@ private void validateDatasetProperties(DatasetDomainKindProperties datasetProper if ("".equals(keyPropertyName)) throw new IllegalArgumentException("Please select a field name for the additional key."); - if (isManagedField && (keyPropertyName == null || keyPropertyName.length() == 0)) + if (isManagedField && (keyPropertyName == null || keyPropertyName.isEmpty())) throw new IllegalArgumentException("Additional Key Column name must be specified if field is managed."); if (useTimeKeyField && isManagedField) @@ -627,7 +627,7 @@ private void checkCanUpdate(DatasetDefinition def, Container container, User use if (!def.canUpdateDefinition(user)) throw new IllegalArgumentException("Shared dataset can not be edited in this folder."); - if (datasetProperties.getLabel() == null || datasetProperties.getLabel().length() == 0) + if (datasetProperties.getLabel() == null || datasetProperties.getLabel().isEmpty()) throw new IllegalArgumentException("Dataset label cannot be empty."); if (null == PropertyService.get().getDomain(container, update.getDomainURI())) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 34b1e65147d..4446304df05 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -867,10 +867,10 @@ else if (datasetDefinition.getKeyPropertyName() == null) if (!old.getName().equals(datasetDefinition.getName())) { nameChange = new QueryChangeListener.QueryPropertyChange<>( - QueryService.get().getUserSchema(user, datasetDefinition.getContainer(), StudyQuerySchema.SCHEMA_NAME).getQueryDefForTable(datasetDefinition.getName()), - QueryChangeListener.QueryProperty.Name, - old.getName(), - datasetDefinition.getName() + QueryService.get().getUserSchema(user, datasetDefinition.getContainer(), StudyQuerySchema.SCHEMA_NAME).getQueryDefForTable(datasetDefinition.getName()), + QueryChangeListener.QueryProperty.Name, + old.getName(), + datasetDefinition.getName() ); } final QueryChangeListener.QueryPropertyChange change = nameChange; @@ -945,11 +945,11 @@ private boolean updateDatasetPropertyOverrides(User user, final DatasetDefinitio else remove.add("showByDefault"); - // update the override map Container c = datasetDefinition.getContainer(); String category = "dataset-overrides:" + datasetDefinition.getDatasetId(); PropertyManager.PropertyMap map = null; + if (!add.isEmpty()) { map = PropertyManager.getWritableProperties(c, category, true); @@ -2547,17 +2547,17 @@ public Map getRequiredMap(Study study) return map; } - private static final String VISITMAP_JOIN_BY_VISIT = "SELECT d.*, vm.Required\n" + - "FROM study.Visit v, study.DataSet d, study.VisitMap vm\n" + - "WHERE v.RowId = vm.VisitRowId and vm.DataSetId = d.DataSetId and " + - "v.Container = vm.Container and vm.Container = d.Container " + - "and v.Container = ? and v.RowId = ?\n" + - "ORDER BY d.DisplayOrder,d.DataSetId"; + private static final String VISITMAP_JOIN_BY_VISIT = """ + SELECT d.*, vm.Required FROM study.Visit v, study.DataSet d, study.VisitMap vm + WHERE v.RowId = vm.VisitRowId AND vm.DataSetId = d.DataSetId AND v.Container = vm.Container AND + vm.Container = d.Container AND v.Container = ? AND v.RowId = ? + ORDER BY d.DisplayOrder, d.DataSetId"""; - private static final String VISITMAP_JOIN_BY_DATASET = "SELECT vm.VisitRowId, vm.Required\n" + - "FROM study.VisitMap vm JOIN study.Visit v ON vm.VisitRowId = v.RowId\n" + - "WHERE vm.Container = ? AND vm.DataSetId = ?\n" + - "ORDER BY v.DisplayOrder, v.RowId"; + private static final String VISITMAP_JOIN_BY_DATASET = """ + SELECT vm.VisitRowId, vm.Required + FROM study.VisitMap vm JOIN study.Visit v ON vm.VisitRowId = v.RowId + WHERE vm.Container = ? AND vm.DataSetId = ? + ORDER BY v.DisplayOrder, v.RowId"""; List getMapping(final VisitImpl visit) { @@ -2567,11 +2567,11 @@ List getMapping(final VisitImpl visit) final List visitDatasets = new ArrayList<>(); new SqlSelector(StudySchema.getInstance().getSchema(), VISITMAP_JOIN_BY_VISIT, - visit.getContainer(), visit.getRowId()).forEach(rs -> { - int datasetId = rs.getInt("DataSetId"); - boolean isRequired = rs.getBoolean("Required"); - visitDatasets.add(new VisitDataset(visit.getContainer(), datasetId, visit.getRowId(), isRequired)); - }); + visit.getContainer(), visit.getRowId()).forEach(rs -> { + int datasetId = rs.getInt("DataSetId"); + boolean isRequired = rs.getBoolean("Required"); + visitDatasets.add(new VisitDataset(visit.getContainer(), datasetId, visit.getRowId(), isRequired)); + }); return visitDatasets; } @@ -2836,10 +2836,10 @@ public void deleteAllStudyData(Container c, User user) assert deletedTables.add(_studyHelper.getTableInfo()); // participant lists - Table.delete(ParticipantGroupManager.getInstance().getTableInfoParticipantGroupMap(), containerFilter); - assert deletedTables.add(ParticipantGroupManager.getInstance().getTableInfoParticipantGroupMap()); - Table.delete(ParticipantGroupManager.getInstance().getTableInfoParticipantGroup(), containerFilter); - assert deletedTables.add(ParticipantGroupManager.getInstance().getTableInfoParticipantGroup()); + Table.delete(ParticipantGroupManager.getTableInfoParticipantGroupMap(), containerFilter); + assert deletedTables.add(ParticipantGroupManager.getTableInfoParticipantGroupMap()); + Table.delete(ParticipantGroupManager.getTableInfoParticipantGroup(), containerFilter); + assert deletedTables.add(ParticipantGroupManager.getTableInfoParticipantGroup()); Table.delete(StudySchema.getInstance().getTableInfoParticipantCategory(), containerFilter); assert deletedTables.add(StudySchema.getInstance().getTableInfoParticipantCategory()); ParticipantGroupManager.getInstance().clearCache(c); From 12b781e03f96cbe3f32f08f9c0139c11f70d4587 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 14 May 2024 02:08:57 -0700 Subject: [PATCH 17/32] More logging --- api/src/org/labkey/api/cache/DbCache.java | 9 +++- .../org/labkey/study/model/StudyManager.java | 44 +++++++++---------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index 5670064e8ae..e621ad93190 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -174,6 +174,13 @@ public static void trackRemove(TableInfo tinfo) public static void logUnmatched() { - TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched {}", key)); + if (TRACKING_BAG.isEmpty()) + { + LOG.info("No unmatched cache removes"); + } + else + { + TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched {}", key)); + } } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 4446304df05..85d091ee0dd 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -349,42 +349,40 @@ public void clearCache(StudyImpl obj) }; _visitHelper = new QueryHelper<>(() -> StudySchema.getInstance().getTableInfoVisit(), VisitImpl.class); - _assaySpecimenHelper = new QueryHelper<>(() -> StudySchema.getInstance().getTableInfoAssaySpecimen(), AssaySpecimenConfigImpl.class); - _cohortHelper = new QueryHelper<>(() -> StudySchema.getInstance().getTableInfoCohort(), CohortImpl.class); - /* Whenever we explicitly invalidate a dataset, unmaterialize it as well - * this is probably a little overkill, e.g. name change doesn't need to unmaterialize - * however, this is the best choke point + /* + * Whenever we explicitly invalidate a dataset, unmaterialize it as well this is probably a little overkill, + * e.g. name change doesn't need to unmaterialize however, this is the best choke point */ _datasetHelper = new DatasetHelper(); // Cache of PropertyDescriptors found in the Shared container for datasets in the given study Container. // The shared properties cache will be cleared when the _datasetHelper cache is cleared. _sharedProperties = CacheManager.getBlockingCache(1000, CacheManager.UNLIMITED, "Study shared properties", - (key, argument) -> - { - Container sharedContainer = ContainerManager.getSharedContainer(); - assert key != sharedContainer; + (key, argument) -> + { + Container sharedContainer = ContainerManager.getSharedContainer(); + assert key != sharedContainer; - List defs = _datasetHelper.get(key); - if (defs == null) - return Collections.emptySet(); + List defs = _datasetHelper.get(key); + if (defs == null) + return Collections.emptySet(); - Set set = new LinkedHashSet<>(); - for (DatasetDefinition def : defs) - { - Domain domain = def.getDomain(); - if (domain == null) - continue; + Set set = new LinkedHashSet<>(); + for (DatasetDefinition def : defs) + { + Domain domain = def.getDomain(); + if (domain == null) + continue; - for (DomainProperty dp : domain.getProperties()) - if (dp.getContainer().equals(sharedContainer)) - set.add(dp.getPropertyDescriptor()); - } - return Collections.unmodifiableSet(set); + for (DomainProperty dp : domain.getProperties()) + if (dp.getContainer().equals(sharedContainer)) + set.add(dp.getPropertyDescriptor()); } + return Collections.unmodifiableSet(set); + } ); ViewCategoryManager.addCategoryListener(new CategoryListener(this)); From 49d6c6cc68b548f5311fc07257fd1b97e491d353 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 14 May 2024 02:28:56 -0700 Subject: [PATCH 18/32] get() -> getList() --- .../specimen/SpecimenRequestManager.java | 13 ++++----- .../specimen/settings/SettingsManager.java | 2 +- .../org/labkey/api/study/QueryHelper.java | 28 ++++++------------- .../org/labkey/study/model/StudyManager.java | 18 ++++++------ 4 files changed, 25 insertions(+), 36 deletions(-) diff --git a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java index ea7ee0f65b1..2bcfff0f424 100644 --- a/specimen/src/org/labkey/specimen/SpecimenRequestManager.java +++ b/specimen/src/org/labkey/specimen/SpecimenRequestManager.java @@ -14,7 +14,6 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.DatabaseCache; -import org.labkey.api.data.DbSchemaCache; import org.labkey.api.data.DbScope; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; @@ -101,7 +100,7 @@ private SpecimenRequestManager() public List getRequestStatuses(Container c, User user) { - List statuses = _requestStatusHelper.get(c, "SortOrder"); + List statuses = _requestStatusHelper.getList(c, "SortOrder"); // if the 'not-yet-submitted' status doesn't exist, create it here, with sort order -1, // so it's always first. if (statuses == null || statuses.isEmpty() || statuses.get(0).getSortOrder() != -1) @@ -118,7 +117,7 @@ public List getRequestStatuses(Container c, User user) DbCache.trackRemove(_requestStatusHelper.getTableInfo()); _requestStatusHelper.clearCache(c); } - statuses = _requestStatusHelper.get(c, "SortOrder"); + statuses = _requestStatusHelper.getList(c, "SortOrder"); } return statuses; } @@ -161,7 +160,7 @@ public boolean hasEditRequestPermissions(User user, SpecimenRequest request) public Set getRequestStatusIdsInUse(Container c) { - List requests = _requestHelper.get(c); + List requests = _requestHelper.getList(c); Set uniqueStatuses = new HashSet<>(); for (SpecimenRequest request : requests) uniqueStatuses.add(request.getStatusId()); @@ -170,7 +169,7 @@ public Set getRequestStatusIdsInUse(Container c) public List getRequestEvents(Container c) { - return _requestEventHelper.get(c); + return _requestEventHelper.getList(c); } public SpecimenRequestEvent getRequestEvent(Container c, int rowId) @@ -551,7 +550,7 @@ public List getRequests(Container c, User user) SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Hidden"), Boolean.FALSE); if (user != null) filter.addCondition(FieldKey.fromParts("CreatedBy"), user.getUserId()); - return _requestHelper.get(c, filter, "-Created"); + return _requestHelper.getList(c, filter, "-Created"); } public SpecimenRequest getRequest(Container c, int rowId) @@ -1251,7 +1250,7 @@ public List getMissingSpecimens(SpecimenRequest specimenRequest) private void deleteRequestEvents(SpecimenRequest request) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("RequestId"), request.getRowId()); - List events = _requestEventHelper.get(request.getContainer(), filter); + List events = _requestEventHelper.getList(request.getContainer(), filter); for (SpecimenRequestEvent event : events) { AttachmentService.get().deleteAttachments(event); diff --git a/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java b/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java index 56b094a8ba9..aeeba2de7eb 100644 --- a/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java +++ b/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java @@ -116,7 +116,7 @@ public boolean isSpecimenRequestEnabled(Container container, boolean checkExisti { if (!getRepositorySettings(container).isEnableRequests()) return false; - List statuses = _requestStatusHelper.get(container, "SortOrder"); + List statuses = _requestStatusHelper.getList(container, "SortOrder"); return (statuses != null && statuses.size() > 1); } } diff --git a/study/api-src/org/labkey/api/study/QueryHelper.java b/study/api-src/org/labkey/api/study/QueryHelper.java index 92cc9744890..191fc47483e 100644 --- a/study/api-src/org/labkey/api/study/QueryHelper.java +++ b/study/api-src/org/labkey/api/study/QueryHelper.java @@ -44,22 +44,22 @@ public QueryHelper(TableInfoGetter tableInfoGetter, Class objectClass) _objectClass = objectClass; } - public List get(Container c) + public List getList(Container c) { - return get(c, null, null); + return getList(c, null, null); } - public List get(Container c, String sortString) + public List getList(Container c, String sortString) { - return get(c, null, sortString); + return getList(c, null, sortString); } - public List get(Container c, SimpleFilter filter) + public List getList(Container c, SimpleFilter filter) { - return get(c, filter, null); + return getList(c, filter, null); } - public List get(final Container c, @Nullable final SimpleFilter filterArg, @Nullable final String sortString) + public List getList(final Container c, @Nullable final SimpleFilter filterArg, @Nullable final String sortString) { String cacheId = getCacheId(filterArg); if (sortString != null) @@ -93,21 +93,11 @@ else if(null != getTableInfo().getColumn("container")) return (List)StudyCache.get(getTableInfo(), c, cacheId, loader); } - public K get(Container c, double rowId) - { - return get(c, (Object)rowId, "RowId"); - } - public K get(Container c, int rowId) { return get(c, (Object)rowId, "RowId"); } - public K get(Container c, double rowId, String rowIdColumnName) - { - return get(c, (Object)rowId, rowIdColumnName); - } - public K get(Container c, int rowId, String rowIdColumnName) { return get(c, (Object)rowId, rowIdColumnName); @@ -118,7 +108,7 @@ private K get(final Container c, final Object rowId, final String rowIdColumnNam CacheLoader loader = (key, argument) -> { SimpleFilter filter = SimpleFilter.createContainerFilter(c); filter.addCondition(rowIdColumnName, rowId); - StudyCachable obj = new TableSelector(getTableInfo(), filter, null).getObject(_objectClass); + StudyCachable obj = new TableSelector(getTableInfo(), filter, null).getObject(_objectClass); if (obj != null) obj.lock(); return obj; @@ -175,7 +165,7 @@ public void clearCache() StudyCache.clearCache(getTableInfo()); } - protected String getCacheId(Filter filter) + protected String getCacheId(@Nullable Filter filter) { if (filter == null) return "~ALL"; diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 85d091ee0dd..37877ae943e 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -258,7 +258,7 @@ private StudyManager() _studyHelper = new QueryHelper<>(() -> StudySchema.getInstance().getTableInfoStudy(), StudyImpl.class) { @Override - public List get(final Container c, SimpleFilter filterArg, final String sortString) + public List getList(final Container c, SimpleFilter filterArg, final String sortString) { assert filterArg == null && sortString == null; String cacheId = getCacheId(filterArg); @@ -458,17 +458,17 @@ public DatasetDefinition update(User user, DatasetDefinition obj, Object... pk) public List get(Container c) { - return toSharedInstance(helper.get(c)); + return toSharedInstance(helper.getList(c)); } public List get(Container c, SimpleFilter filter) { - return toSharedInstance(helper.get(c, filter)); + return toSharedInstance(helper.getList(c, filter)); } public List get(Container c, @Nullable SimpleFilter filterArg, @Nullable String sortString) { - return toSharedInstance(helper.get(c, filterArg, sortString)); + return toSharedInstance(helper.getList(c, filterArg, sortString)); } public DatasetDefinition get(Container c, int rowId) @@ -523,7 +523,7 @@ public StudyImpl getStudy(@NotNull Container c) while (true) { - List studies = _studyHelper.get(c); + List studies = _studyHelper.getList(c); if (studies == null || studies.isEmpty()) return null; else if (studies.size() > 1) @@ -1752,7 +1752,7 @@ public void updateParticipant(User user, Participant participant) public List getAssaySpecimenConfigs(Container container, String sortCol) { - return _assaySpecimenHelper.get(container, sortCol); + return _assaySpecimenHelper.getList(container, sortCol); } public List getVisitsForAssaySchedule(Container container) @@ -1860,7 +1860,7 @@ public List getVisits(Study study, @Nullable Cohort cohort, @Nullable filter.addWhereClause("(CohortId IS NULL OR CohortId = ?)", new Object[]{cohort.getRowId()}); } - return _visitHelper.get(visitStudy.getContainer(), filter, order.getSortColumns()); + return _visitHelper.getList(visitStudy.getContainer(), filter, order.getSortColumns()); } public void clearParticipantVisitCaches(Study study) @@ -2116,7 +2116,7 @@ public void assertCohortsViewable(Container container, User user) public List getCohorts(Container container, User user) { assertCohortsViewable(container, user); - return _cohortHelper.get(container, "Label"); + return _cohortHelper.getList(container, "Label"); } public CohortImpl getCurrentCohortForParticipant(Container container, User user, String participantId) @@ -2140,7 +2140,7 @@ public CohortImpl getCohortByLabel(Container container, User user, String label) SimpleFilter filter = SimpleFilter.createContainerFilter(container); filter.addCondition(FieldKey.fromParts("Label"), label); - List cohorts = _cohortHelper.get(container, filter); + List cohorts = _cohortHelper.getList(container, filter); if (cohorts != null && cohorts.size() == 1) return cohorts.get(0); From dc0ff11942ab6b63a7cc3da593297e37e7472c5b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 14 May 2024 06:56:59 -0700 Subject: [PATCH 19/32] Clean up --- .../org/labkey/api/study/StudyCache.java | 19 ++----------------- .../org/labkey/study/model/StudyManager.java | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index b74561f1b21..85ad9ea8301 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -36,7 +36,7 @@ public class StudyCache // TODO: Switch to BlockingCache private static final Map> CACHES = new HashMap<>(10); - private static DatabaseCache getCache(TableInfo tinfo, boolean create) + public static DatabaseCache getCache(TableInfo tinfo, boolean create) { Path cacheKey = tinfo.getNotificationKey(); assert null != cacheKey : "StudyCache not supported for " + tinfo; @@ -55,7 +55,7 @@ private static DatabaseCache getCache(TableInfo tinfo, boolean c } } - private static String getCacheName(Container c, @Nullable Object cacheKey) + public static String getCacheName(Container c, @Nullable Object cacheKey) { return c.getId() + "/" + (null != cacheKey ? cacheKey : ""); } @@ -75,21 +75,6 @@ public static void uncache(TableInfo tinfo, Container c, Object cacheKey) clearCache(tinfo, c); // TODO: this method is broken/inconsistent -- the cacheKey passed in doesn't match the put() keys } - public static Object getCached(TableInfo tinfo, Container c, Object cacheKey) - { - Object oldObj = DbCache.get(tinfo, getCacheName(c, cacheKey)); - DatabaseCache cache = getCache(tinfo, false); - Object newObj = cache != null ? cache.get(getCacheName(c, cacheKey)) : null; - - if (!Objects.equals(oldObj, newObj)) - { - DbCache.logUnmatched(); - throw new IllegalStateException(oldObj + " != " + newObj); - } - - return newObj; - } - public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoader loader) { // Don't use a BlockingCache as that can cause deadlocks when needing to do a diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 37877ae943e..a43be9a90f0 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -483,7 +483,7 @@ private List toSharedInstance(List in) ArrayList ret = new ArrayList<>(in.size()); for (DatasetDefinition dsIn : in) { - DatasetDefinition dsRet = (DatasetDefinition) StudyCache.getCached(t, dsIn.getContainer(), dsIn.getEntityId()); + DatasetDefinition dsRet = (DatasetDefinition) getCached(t, dsIn.getContainer(), dsIn.getEntityId()); if (null == dsRet) { dsRet = dsIn; @@ -499,7 +499,7 @@ private DatasetDefinition toSharedInstance(DatasetDefinition dsIn) if (null == dsIn) return null; TableInfo t = getTableInfo(); - DatasetDefinition dsRet = (DatasetDefinition) StudyCache.getCached(t, dsIn.getContainer(), dsIn.getEntityId()); + DatasetDefinition dsRet = (DatasetDefinition) getCached(t, dsIn.getContainer(), dsIn.getEntityId()); if (null == dsRet) { dsRet = dsIn; @@ -507,6 +507,21 @@ private DatasetDefinition toSharedInstance(DatasetDefinition dsIn) } return dsRet; } + + private static Object getCached(TableInfo tinfo, Container c, Object cacheKey) + { + Object oldObj = DbCache.get(tinfo, StudyCache.getCacheName(c, cacheKey)); + DatabaseCache cache = StudyCache.getCache(tinfo, false); + Object newObj = cache != null ? cache.get(StudyCache.getCacheName(c, cacheKey)) : null; + + if (!Objects.equals(oldObj, newObj)) + { + DbCache.logUnmatched(); + throw new IllegalStateException(oldObj + " != " + newObj); + } + + return newObj; + } } From 81535e44844fde348ca30b061d49895df7283c0d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 15 May 2024 01:27:26 -0700 Subject: [PATCH 20/32] Tolerate discrepancies between DbCache and DatabaseCache contents, since DbCache is invalidated more aggressively --- api/src/org/labkey/api/cache/DbCache.java | 1 - .../labkey/pipeline/api/PipelineManager.java | 1 - .../org/labkey/api/study/QueryHelper.java | 3 +++ .../org/labkey/api/study/StudyCache.java | 17 ++++++++++++++++- .../org/labkey/study/model/StudyManager.java | 18 ++++++++++++------ .../visitmanager/RelativeDateVisitManager.java | 2 +- 6 files changed, 32 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index e621ad93190..4324f4b12a8 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -127,7 +127,6 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) */ private static final Bag TRACKING_BAG = new HashBag<>(); - private static final Set INTERESTING = Set.of("TestTable", "ExperimentRun", "Study"); private static boolean isInteresting(TableInfo tinfo) { diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java index 5437d47e165..55d355669ce 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java @@ -28,7 +28,6 @@ import org.labkey.api.cache.DbCache; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; -import org.labkey.api.data.DbSchemaCache; import org.labkey.api.data.DbScope; import org.labkey.api.data.Filter; import org.labkey.api.data.ObjectFactory; diff --git a/study/api-src/org/labkey/api/study/QueryHelper.java b/study/api-src/org/labkey/api/study/QueryHelper.java index 191fc47483e..e9e8c2bff6b 100644 --- a/study/api-src/org/labkey/api/study/QueryHelper.java +++ b/study/api-src/org/labkey/api/study/QueryHelper.java @@ -31,6 +31,7 @@ import org.labkey.api.security.User; import java.util.Collections; +import java.util.Comparator; import java.util.List; public class QueryHelper @@ -84,6 +85,8 @@ else if(null != getTableInfo().getColumn("container")) if (sortString != null) sort = new Sort(sortString); List objs = new TableSelector(getTableInfo(), filter, sort).getArrayList(_objectClass); + if (null == sort) + objs.sort(Comparator.comparingInt(Object::hashCode)); // TODO: temp for comparison purposes -- stable sort if none specified // Make both the objects and the list itself immutable so that we don't end up with a corrupted // version in the cache for (StudyCachable obj : objs) diff --git a/study/api-src/org/labkey/api/study/StudyCache.java b/study/api-src/org/labkey/api/study/StudyCache.java index 85ad9ea8301..6b46362da88 100644 --- a/study/api-src/org/labkey/api/study/StudyCache.java +++ b/study/api-src/org/labkey/api/study/StudyCache.java @@ -16,6 +16,8 @@ package org.labkey.api.study; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheLoader; @@ -67,12 +69,24 @@ public static void cache(TableInfo tinfo, Container c, String objectId, StudyCac DbCache.put(tinfo, getCacheName(c, objectId), cachable, CacheManager.HOUR); DatabaseCache cache = getCache(tinfo, true); cache.put(getCacheName(c, objectId), cachable, CacheManager.HOUR); + track(tinfo, "Put " + getCacheName(c, objectId)); } + private static final Logger LOG = LogManager.getLogger(StudyCache.class); + + public static void track(TableInfo tinfo, String message) + { + if (tinfo.getName().contains("DataSet")) + LOG.info(message); + } + + // TODO: this method is broken/inconsistent -- the cacheKey passed in doesn't match the put() keys public static void uncache(TableInfo tinfo, Container c, Object cacheKey) { DbCache.remove(tinfo, getCacheName(c, cacheKey)); - clearCache(tinfo, c); // TODO: this method is broken/inconsistent -- the cacheKey passed in doesn't match the put() keys + DatabaseCache cache = getCache(tinfo, false); + if (null != cache) {cache.remove(getCacheName(c, cacheKey)); cache.clear();} + track(tinfo, "Remove " + getCacheName(c, cacheKey)); } public static Object get(TableInfo tinfo, Container c, Object cacheKey, CacheLoader loader) @@ -100,6 +114,7 @@ public static void clearCache(TableInfo tinfo, Container c) DatabaseCache cache = getCache(tinfo, false); if (null != cache) cache.removeUsingFilter(new Cache.StringPrefixFilter(getCacheName(c, null))); + track(tinfo, "Remove using prefix " + getCacheName(c, null)); } public static void clearCache(TableInfo tinfo) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index a43be9a90f0..71b60d246df 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -410,14 +410,14 @@ private class DatasetHelper public void clearCache(Container c) { super.clearCache(c); - super.clearCache(); // Big hammer, but we're caching datasets in multiple containers + StudyCache.track(getTableInfo(), "Clear container " + c); } @Override public void clearCache(DatasetDefinition obj) { - super.clearCache(obj); - super.clearCache(); // Big hammer, but we're caching datasets in multiple containers + super.clearCache(obj.getContainer()); + StudyCache.track(getTableInfo(), "Clear container " + obj.getContainer() + " for dataset " + obj); } }; @@ -516,15 +516,21 @@ private static Object getCached(TableInfo tinfo, Container c, Object cacheKey) if (!Objects.equals(oldObj, newObj)) { - DbCache.logUnmatched(); - throw new IllegalStateException(oldObj + " != " + newObj); + if (newObj instanceof DatasetDefinition dd) + { + _log.info("Mismatch for dataset " + dd); + } + else + { + DbCache.logUnmatched(); + throw new IllegalStateException(oldObj + " != " + newObj); + } } return newObj; } } - public static StudyManager getInstance() { return _instance; diff --git a/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java b/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java index a646125a867..89df4f2b634 100644 --- a/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java +++ b/study/src/org/labkey/study/visitmanager/RelativeDateVisitManager.java @@ -321,7 +321,7 @@ private void _updateVisitRowId() ValidationException errors = StudyManager.getInstance().ensureVisits(getStudy(), user, daysToEnsure, null, failForUndefinedVisits); - if (daysToEnsure.size() > 0) + if (!daysToEnsure.isEmpty()) _updateVisitRowId(); return errors; From 06b747b0ed43d3ed6191a568745cce35809945ab Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 15 May 2024 08:56:42 -0700 Subject: [PATCH 21/32] More lenient validation --- study/src/org/labkey/study/model/StudyManager.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 71b60d246df..ff5f2a9d3c8 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -516,11 +516,7 @@ private static Object getCached(TableInfo tinfo, Container c, Object cacheKey) if (!Objects.equals(oldObj, newObj)) { - if (newObj instanceof DatasetDefinition dd) - { - _log.info("Mismatch for dataset " + dd); - } - else + if (newObj instanceof List || oldObj instanceof List) { DbCache.logUnmatched(); throw new IllegalStateException(oldObj + " != " + newObj); From 94ad18256bf16819f881483530de5536a91fdbac Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 15 May 2024 09:03:52 -0700 Subject: [PATCH 22/32] Slight cleanup --- api/src/org/labkey/api/data/TableViewForm.java | 2 -- core/src/org/labkey/core/view/TableViewFormTestCase.java | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index cc30d8c8f00..3d208878168 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -215,9 +215,7 @@ public void doDelete() { Object[] pkVal = getPkVals(); if (null != pkVal && null != pkVal[0]) - { Table.delete(_tinfo, pkVal); - } else //Hmm, throw an exception here???? _log.warn("Nothing to delete for table " + _tinfo.getName() + " on request " + _request.getRequestURI()); } diff --git a/core/src/org/labkey/core/view/TableViewFormTestCase.java b/core/src/org/labkey/core/view/TableViewFormTestCase.java index b0cd32940b7..abed3c1a058 100644 --- a/core/src/org/labkey/core/view/TableViewFormTestCase.java +++ b/core/src/org/labkey/core/view/TableViewFormTestCase.java @@ -91,6 +91,7 @@ public void testErrorHandling() Assert.assertTrue("Final form should be valid", tf.isValid()); } + @Test public void testDbOperations() throws SQLException { From 2e2db2ba737800f0f0efe05cfe1732ebb8fa0a19 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 00:31:52 -0700 Subject: [PATCH 23/32] Experiment run discrepancy logging --- .../org/labkey/experiment/api/ExperimentServiceImpl.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 1122ee16470..80dc245a2ca 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -4166,7 +4166,11 @@ public ExperimentRun getExperimentRun(String LSID) } else { - assert null == runNew; + if (runNew != null) + { + DbCache.logUnmatched(); + throw new IllegalStateException(runOld + " != " + runNew); + } } return runOld; From c7c4567dc7bb373626d93f1c0728364ecebf09d6 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 02:28:07 -0700 Subject: [PATCH 24/32] Track invalidations --- api/src/org/labkey/api/audit/data/RunColumn.java | 2 +- api/src/org/labkey/api/query/DefaultQueryUpdateService.java | 5 ++++- .../org/labkey/experiment/api/ExpIdentifiableBaseImpl.java | 5 +---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/audit/data/RunColumn.java b/api/src/org/labkey/api/audit/data/RunColumn.java index ae5b3935c7d..8884680cc8c 100644 --- a/api/src/org/labkey/api/audit/data/RunColumn.java +++ b/api/src/org/labkey/api/audit/data/RunColumn.java @@ -50,7 +50,7 @@ protected String extractFromKey3(RenderContext ctx) String[] parts = value.toString().split(KEY_SEPARATOR); if (parts.length != 2) return null; - return parts[1].length() > 0 ? parts[1] : null; + return !parts[1].isEmpty() ? parts[1] : null; } @Override diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index 498c02a8529..7219a8c2b1e 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -20,6 +20,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.attachments.AttachmentFile; +import org.labkey.api.cache.DbCache; import org.labkey.api.collections.ArrayListMap; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.ColumnInfo; @@ -339,7 +340,9 @@ protected Map _insert(User user, Container c, Map ret = Table.insert(user, getDbTable(), row); + if (getDbTable().getName().equals("AssaySpecimen")) { DbCache.trackRemove(getDbTable()); } + return ret; } catch (RuntimeValidationException e) { diff --git a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java index 2d8aea10479..5137436a944 100644 --- a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java @@ -33,10 +33,6 @@ import java.util.Objects; import java.util.function.Function; -/** - * User: jeckels - * Date: Sep 26, 2007 - */ public abstract class ExpIdentifiableBaseImpl extends ExpObjectImpl { protected Type _object; @@ -153,6 +149,7 @@ protected void save(User user, TableInfo table, boolean ensureObject, boolean is } _object = Table.update(user, table, _object, getRowId()); + if (table.getName().equals("ExperimentRun")) DbCache.trackRemove(table); if (_prevLsid != null && ensureObject) { From 0123d91d20d3c0f51543ad6bd689303bbd004183 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 03:00:36 -0700 Subject: [PATCH 25/32] Invalidate cohort cache --- study/src/org/labkey/study/model/StudyManager.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index ff5f2a9d3c8..7df0dbbfb79 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -2771,9 +2771,11 @@ public void clearCaches(Container c, boolean unmaterializeDatasets) for (DatasetDefinition def : getDatasetDefinitions(study)) uncache(def); _datasetHelper.clearCache(c); + _cohortHelper.clearCache(c); DbCache.clear(StudySchema.getInstance().getTableInfoParticipant()); DbCache.trackRemove(StudySchema.getInstance().getTableInfoParticipant()); + _participantCache.remove(c); for (StudyImpl substudy : StudyManager.getInstance().getAncillaryStudies(c)) clearCaches(substudy.getContainer(), unmaterializeDatasets); From 1b78ead166792b44ea2eedd69cb63b4c1f75fc9e Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 08:53:44 -0700 Subject: [PATCH 26/32] Invalidate cohort cache --- .../labkey/experiment/api/ExpIdentifiableBaseImpl.java | 2 +- study/src/org/labkey/study/model/StudyManager.java | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java index 5137436a944..4b3de3523a2 100644 --- a/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpIdentifiableBaseImpl.java @@ -149,7 +149,7 @@ protected void save(User user, TableInfo table, boolean ensureObject, boolean is } _object = Table.update(user, table, _object, getRowId()); - if (table.getName().equals("ExperimentRun")) DbCache.trackRemove(table); + if (table.getName().equals("ExperimentRun")) DbCache.trackRemove(table); // Handled in override if (_prevLsid != null && ensureObject) { diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 7df0dbbfb79..ad93d2ff0a3 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -1729,6 +1729,7 @@ public void deleteVisits(StudyImpl study, Collection visits, User use try { Table.delete(schema.getTableInfoVisit(), new Object[]{visitStudy.getContainer(), visit.getRowId()}); + DbCache.trackRemove(schema.getTableInfoVisit()); } finally { @@ -1760,11 +1761,9 @@ public void updateCohort(User user, CohortImpl cohort) public void updateParticipant(User user, Participant participant) { - Table.update(user, - SCHEMA.getTableInfoParticipant(), - participant, - new Object[]{participant.getContainer().getId(), participant.getParticipantId()} - ); + Table.update(user, SCHEMA.getTableInfoParticipant(), participant, new Object[]{participant.getContainer().getId(), participant.getParticipantId()}); + DbCache.trackRemove(SCHEMA.getTableInfoParticipant()); + _participantCache.remove(participant.getContainer()); } public List getAssaySpecimenConfigs(Container container, String sortCol) From 8ac47fba9b07aae441cc0fd72c5fe0a4d315c82b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 13:33:11 -0700 Subject: [PATCH 27/32] Invalidate assay specimen cache --- .../org/labkey/api/query/DefaultQueryUpdateService.java | 8 +++++--- study/src/org/labkey/study/model/TreatmentManager.java | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index 7219a8c2b1e..0af2e056834 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -571,7 +571,9 @@ protected Map _update(User user, Container c, Map ret = Table.update(user, getDbTable(), row, keys); + if (getDbTable().getName().equals("AssaySpecimen")) DbCache.trackRemove(getDbTable()); // Handled in caller (TreatmentManager.saveAssaySpecimen()) + return ret; } // Get value from row map where the keys are column names. @@ -584,7 +586,7 @@ private Object getPropertyValue(Map row, PropertyDescriptor pd) return row.get(pd.getLabel()); Set aliases = pd.getImportAliasSet(); - if (aliases.size() > 0) + if (!aliases.isEmpty()) { for (String alias : aliases) { @@ -606,7 +608,7 @@ private boolean hasProperty(Map row, PropertyDescriptor pd) return true; Set aliases = pd.getImportAliasSet(); - if (aliases.size() > 0) + if (!aliases.isEmpty()) { for (String alias : aliases) { diff --git a/study/src/org/labkey/study/model/TreatmentManager.java b/study/src/org/labkey/study/model/TreatmentManager.java index 6d96a9bab75..a8d039ffc06 100644 --- a/study/src/org/labkey/study/model/TreatmentManager.java +++ b/study/src/org/labkey/study/model/TreatmentManager.java @@ -377,7 +377,9 @@ public DoseAndRoute getDoseAndRoute(Container container, String dose, String rou public Integer saveAssaySpecimen(Container container, User user, AssaySpecimenConfigImpl assaySpecimen) throws Exception { TableInfo assaySpecimenTable = QueryService.get().getUserSchema(user, container, StudyQuerySchema.SCHEMA_NAME).getTable(StudyQuerySchema.ASSAY_SPECIMEN_TABLE_NAME); - return saveStudyDesignRow(container, user, assaySpecimenTable, assaySpecimen.serialize(), assaySpecimen.isNew() ? null : assaySpecimen.getRowId(), "RowId", true); + Integer ret = saveStudyDesignRow(container, user, assaySpecimenTable, assaySpecimen.serialize(), assaySpecimen.isNew() ? null : assaySpecimen.getRowId(), "RowId", true); + StudyManager.getInstance().clearAssaySpecimenCache(container); + return ret; } public Integer saveAssaySpecimenVisit(Container container, User user, AssaySpecimenVisitImpl assaySpecimenVisit) throws Exception From 4ae69ce4f87f53bb3933b2a9a2cfcdd155620102 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 16:59:27 -0700 Subject: [PATCH 28/32] Invalidate run cache --- experiment/src/org/labkey/experiment/api/ExpRunImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index c1d71218024..be29f260750 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -80,6 +80,7 @@ import java.util.Set; import java.util.TreeMap; +import static org.labkey.experiment.api.ExperimentServiceImpl.EXPERIMENT_RUN_CACHE; import static org.labkey.experiment.api.ExperimentServiceImpl.getExpSchema; public class ExpRunImpl extends ExpIdentifiableEntityImpl implements ExpRun @@ -549,6 +550,7 @@ public void deleteProtocolApplications(List datasToDelete, User use final SqlDialect dialect = svc.getSchema().getSqlDialect(); DbCache.remove(svc.getTinfoExperimentRun(), svc.getCacheKey(getLSID())); + EXPERIMENT_RUN_CACHE.remove(getLSID()); deleteProtocolApplicationProvenance(); From 8bbcecdf27f68a670d91ed318f6306da8b26e3ac Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 16 May 2024 20:25:58 -0700 Subject: [PATCH 29/32] Don't track --- api/src/org/labkey/api/data/SQLFragment.java | 2 +- study/src/org/labkey/study/model/StudyManager.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/SQLFragment.java b/api/src/org/labkey/api/data/SQLFragment.java index 81f6103c50e..0a8981fd627 100644 --- a/api/src/org/labkey/api/data/SQLFragment.java +++ b/api/src/org/labkey/api/data/SQLFragment.java @@ -159,7 +159,7 @@ public SQLFragment(SQLFragment other, boolean deep) @Override public boolean isEmpty() { - return (null == sb || sb.length() == 0) && (sql == null || sql.length() == 0); + return (null == sb || sb.isEmpty()) && (sql == null || sql.isEmpty()); } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index ad93d2ff0a3..67805a88443 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -4166,7 +4166,6 @@ private Map getParticipantMap(Study study) public void clearParticipantCache(Container container) { DbCache.remove(StudySchema.getInstance().getTableInfoParticipant(), getParticipantCacheKey(container)); - DbCache.trackRemove(StudySchema.getInstance().getTableInfoParticipant()); _participantCache.remove(container); } From 8f4a1b73b433d88cbd05bf8b1c3b1fc43ede75ee Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 17 May 2024 09:41:55 -0700 Subject: [PATCH 30/32] Linked map to maintain sorted order --- study/src/org/labkey/study/model/StudyManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 67805a88443..6acb314c4a2 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -41,6 +41,7 @@ import org.labkey.api.cache.DbCache; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.compliance.ComplianceService; import org.labkey.api.data.Activity; import org.labkey.api.data.ColumnInfo; @@ -247,7 +248,7 @@ public class StudyManager return Collections.unmodifiableMap( new TableSelector(StudySchema.getInstance().getTableInfoParticipant(), filter, new Sort("ParticipantId")) .stream(Participant.class) - .collect(Collectors.toMap(Participant::getParticipantId, participant -> participant)) + .collect(LabKeyCollectors.toLinkedMap(Participant::getParticipantId, participant -> participant)) ); }); From 06df04dec1fd09e3cf63f2b5510b99b452b0b48f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 17 May 2024 16:33:31 -0700 Subject: [PATCH 31/32] Fix FolderTest --- api/src/org/labkey/api/cache/DbCache.java | 5 ++--- study/src/org/labkey/study/StudyContainerListener.java | 5 ----- study/src/org/labkey/study/model/StudyManager.java | 9 ++++++++- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/cache/DbCache.java b/api/src/org/labkey/api/cache/DbCache.java index 4324f4b12a8..d2e17c6f5d0 100644 --- a/api/src/org/labkey/api/cache/DbCache.java +++ b/api/src/org/labkey/api/cache/DbCache.java @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.Map; -import java.util.Set; /** * Don't use this! Use CacheManager.getCache() or DatabaseCache instead. DbCache associates a DatabaseCache with each @@ -114,8 +113,8 @@ public static void removeUsingPrefix(TableInfo tinfo, String name) } /** - * Everything below is temporary, meant to help irradicate the use of DbCache. If a TableInfo is deemed - * "interesting" (it's in the process of being migrated): + * Everything below is temporary, meant to help eradicate the use of DbCache. If a TableInfo is deemed "interesting" + * (it's in the process of being migrated): * - Each call to invalidateAll() or clear() causes its stack trace to be added to the tracking bag * - Each call to trackRemove() causes its stack trace to be removed from the tracking bag * A remove that's unsuccessful indicates no corresponding invalidateAll(), so log that. Anything left in the bag diff --git a/study/src/org/labkey/study/StudyContainerListener.java b/study/src/org/labkey/study/StudyContainerListener.java index 479f8c00ca4..29454820036 100644 --- a/study/src/org/labkey/study/StudyContainerListener.java +++ b/study/src/org/labkey/study/StudyContainerListener.java @@ -24,11 +24,6 @@ import java.beans.PropertyChangeEvent; -/** - * User: adam - * Date: Nov 3, 2008 - * Time: 3:48:15 PM - */ public class StudyContainerListener extends ContainerManager.AbstractContainerListener { @Override diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 6acb314c4a2..2f4cf753221 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -447,6 +447,11 @@ public void clearCache(DatasetDefinition def) clearProperties(def); } + public void clearCache() + { + helper.clearCache(); + } + public DatasetDefinition create(User user, DatasetDefinition obj) { return helper.create(user, obj); @@ -2770,7 +2775,9 @@ public void clearCaches(Container c, boolean unmaterializeDatasets) if (unmaterializeDatasets && null != study) for (DatasetDefinition def : getDatasetDefinitions(study)) uncache(def); - _datasetHelper.clearCache(c); + // Aggressive, but datasets are cached with container objects that might go stale, for example, when moving a + // folder tree to another parent the datasets in subfolders will be left with invalid paths. See FolderTest. + _datasetHelper.clearCache(); _cohortHelper.clearCache(c); DbCache.clear(StudySchema.getInstance().getTableInfoParticipant()); From 3961b53fafc559dfcd7e027da259295fd077e881 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 20 May 2024 13:45:37 -0700 Subject: [PATCH 32/32] Code review feedback. Move more specimen code into specimen module. --- .../labkey/api/exp/api/ExperimentService.java | 2 + .../org/labkey/experiment/api/ExpRunImpl.java | 5 +- .../experiment/api/ExperimentServiceImpl.java | 23 ++-- .../org/labkey/specimen/SpecimenModule.java | 13 --- .../labkey/specimen/SpecimenServiceImpl.java | 9 +- .../specimen/actions/SpecimenController.java | 68 ++++++++---- .../importer/SpecimenSettingsImporter.java | 14 +-- .../DefaultRequestNotification.java | 13 +-- .../settings/RequestNotificationSettings.java | 10 +- .../specimen/settings/SettingsManager.java | 103 ++++++++++++++++++ ...cimenRequestNotificationEmailTemplate.java | 11 +- .../view/SpecimenToolsWebPartFactory.java | 2 +- .../specimen/view/manageNotifications.jsp | 7 +- .../src/org/labkey/specimen/view/webPart.jsp | 6 +- .../writer/SpecimenSettingsWriter.java | 4 +- .../specimen/SpecimenMigrationService.java | 2 - .../api/specimen/query/SpecimenQueryView.java | 6 - .../specimen/settings/SettingsManager.java | 79 -------------- 18 files changed, 192 insertions(+), 185 deletions(-) rename {study/api-src/org/labkey/api => specimen/src/org/labkey}/specimen/settings/RequestNotificationSettings.java (98%) create mode 100644 specimen/src/org/labkey/specimen/settings/SettingsManager.java diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 1fc73247abf..9ca1973c5e4 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -748,6 +748,8 @@ static void validateParentAlias(Map aliasMap, Set reserv void clearExperimentRunCache(); + void invalidateExperimentRun(String lsid); + List getProtocolApplicationParameters(int rowId); void moveContainer(Container c, Container cOldParent, Container cNewParent) throws ExperimentException; diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index be29f260750..ebeeba70984 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -80,7 +80,6 @@ import java.util.Set; import java.util.TreeMap; -import static org.labkey.experiment.api.ExperimentServiceImpl.EXPERIMENT_RUN_CACHE; import static org.labkey.experiment.api.ExperimentServiceImpl.getExpSchema; public class ExpRunImpl extends ExpIdentifiableEntityImpl implements ExpRun @@ -329,7 +328,7 @@ protected void save(User user, TableInfo table, boolean ensureObject) { assert ensureObject; super.save(user, table, true); - ExperimentServiceImpl.EXPERIMENT_RUN_CACHE.remove(getLSID()); + ExperimentServiceImpl.get().invalidateExperimentRun(getLSID()); } @Override @@ -550,7 +549,7 @@ public void deleteProtocolApplications(List datasToDelete, User use final SqlDialect dialect = svc.getSchema().getSqlDialect(); DbCache.remove(svc.getTinfoExperimentRun(), svc.getCacheKey(getLSID())); - EXPERIMENT_RUN_CACHE.remove(getLSID()); + ExperimentServiceImpl.get().invalidateExperimentRun(getLSID()); deleteProtocolApplicationProvenance(); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 80dc245a2ca..9697ece640b 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -299,7 +299,7 @@ public class ExperimentServiceImpl implements ExperimentService, ObjectReference private final Cache PROTOCOL_LSID_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, CacheManager.HOUR, "Protocol by LSID", (key, argument) -> toExpProtocol(new TableSelector(getTinfoProtocol(), new SimpleFilter(FieldKey.fromParts("LSID"), key), null).getObject(Protocol.class))); - static final Cache EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, "Experiment Runs", new ExperimentRunCacheLoader()); + private final Cache EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), getTinfoExperimentRun().getCacheSize(), "Experiment Run by LSID", new ExperimentRunCacheLoader()); private final Cache> dataClassCache = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Data classes", (containerId, argument) -> { @@ -3882,11 +3882,6 @@ public TableInfo getTinfoExperiment() @Override public TableInfo getTinfoExperimentRun() - { - return getTableInfoExperimentRun(); - } - - private static TableInfo getTableInfoExperimentRun() { return getExpSchema().getTable("ExperimentRun"); } @@ -4147,10 +4142,10 @@ public List getExperiments(Container container, User user, bo return ExpExperimentImpl.fromExperiments(new TableSelector(getTinfoExperiment(), filter, sort).getArray(Experiment.class)); } - public ExperimentRun getExperimentRun(String LSID) + public ExperimentRun getExperimentRun(String lsid) { - ExperimentRun runOld = getExperimentRunOld(LSID); - ExperimentRun runNew = getExperimentRunNew(LSID); + ExperimentRun runOld = getExperimentRunOld(lsid); + ExperimentRun runNew = getExperimentRunNew(lsid); if (null != runOld) { @@ -4196,13 +4191,13 @@ private ExperimentRun getExperimentRunNew(String LSID) return EXPERIMENT_RUN_CACHE.get(LSID); } - private static class ExperimentRunCacheLoader implements CacheLoader + private class ExperimentRunCacheLoader implements CacheLoader { @Override public ExperimentRun load(@NotNull String lsid, @Nullable Object argument) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("LSID"), lsid); - return new TableSelector(getTableInfoExperimentRun(), filter, null).getObject(ExperimentRun.class); + return new TableSelector(getTinfoExperimentRun(), filter, null).getObject(ExperimentRun.class); } } @@ -4223,6 +4218,12 @@ public void clearExperimentRunCache() EXPERIMENT_RUN_CACHE.clear(); } + @Override + public void invalidateExperimentRun(String lsid) + { + EXPERIMENT_RUN_CACHE.remove(lsid); + } + @Override public ExpProtocolApplication getExpProtocolApplication(String lsid) { diff --git a/specimen/src/org/labkey/specimen/SpecimenModule.java b/specimen/src/org/labkey/specimen/SpecimenModule.java index b01650ee23d..01c6f48f9b2 100644 --- a/specimen/src/org/labkey/specimen/SpecimenModule.java +++ b/specimen/src/org/labkey/specimen/SpecimenModule.java @@ -70,7 +70,6 @@ import org.labkey.specimen.pipeline.SampleMindedTransform; import org.labkey.specimen.pipeline.SampleMindedTransformTask; import org.labkey.specimen.pipeline.SpecimenPipeline; -import org.labkey.specimen.requirements.SpecimenRequestRequirementProvider; import org.labkey.specimen.security.roles.SpecimenCoordinatorRole; import org.labkey.specimen.security.roles.SpecimenRequesterRole; import org.labkey.specimen.view.ManageSpecimenView; @@ -201,23 +200,11 @@ public void clearRequestCaches(Container c) SpecimenRequestManager.get().clearCaches(c); } - @Override - public void clearGroupedValuesForColumn(Container container) - { - SpecimenRequestManager.get().clearGroupedValuesForColumn(container); - } - @Override public void updateVialCounts(Container container, User user) { SpecimenRequestManager.get().updateVialCounts(container, user); } - - @Override - public void purgeRequestRequirementsAndActors(Container container) - { - SpecimenRequestRequirementProvider.get().purgeContainer(container); - } }); } diff --git a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java index a391f822aa6..6354073b5a1 100644 --- a/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java +++ b/specimen/src/org/labkey/specimen/SpecimenServiceImpl.java @@ -43,7 +43,6 @@ import org.labkey.api.security.User; import org.labkey.api.specimen.DefaultSpecimenTablesTemplate; import org.labkey.api.specimen.SpecimenColumns; -import org.labkey.api.specimen.SpecimenMigrationService; import org.labkey.api.specimen.SpecimenSchema; import org.labkey.api.specimen.Vial; import org.labkey.api.specimen.importer.SimpleSpecimenImporter; @@ -62,6 +61,7 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.specimen.pipeline.SpecimenReloadJob; +import org.labkey.specimen.requirements.SpecimenRequestRequirementProvider; import java.io.IOException; import java.util.ArrayList; @@ -497,9 +497,7 @@ public void deleteAllSpecimenData(Container c, Set set, User user) Table.delete(SpecimenSchema.get().getTableInfoSampleAvailabilityRule(), containerFilter); assert set.add(SpecimenSchema.get().getTableInfoSampleAvailabilityRule()); - SpecimenMigrationService SMS = SpecimenMigrationService.get(); - if (null != SMS) - SMS.purgeRequestRequirementsAndActors(c); + SpecimenRequestRequirementProvider.get().purgeContainer(c); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestRequirement()); assert set.add(SpecimenSchema.get().getTableInfoSampleRequestActor()); @@ -535,7 +533,6 @@ public void deleteAllSpecimenData(Container c, Set set, User user) // VIEW: if this view gets removed, remove this line assert set.add(SpecimenSchema.get().getSchema().getTable("LockedSpecimens")); - if (null != SMS) - SMS.clearGroupedValuesForColumn(c); + SpecimenRequestManager.get().clearGroupedValuesForColumn(c); } } diff --git a/specimen/src/org/labkey/specimen/actions/SpecimenController.java b/specimen/src/org/labkey/specimen/actions/SpecimenController.java index 8abd490aec4..c8491c6bbcc 100644 --- a/specimen/src/org/labkey/specimen/actions/SpecimenController.java +++ b/specimen/src/org/labkey/specimen/actions/SpecimenController.java @@ -1,5 +1,12 @@ package org.labkey.specimen.actions; +import jakarta.mail.Address; +import jakarta.mail.Message; +import jakarta.mail.internet.AddressException; +import jakarta.mail.internet.InternetAddress; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpSession; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -25,7 +32,30 @@ import org.labkey.api.attachments.BaseDownloadAction; import org.labkey.api.attachments.ByteArrayAttachmentFile; import org.labkey.api.audit.TransactionAuditProvider; -import org.labkey.api.data.*; +import org.labkey.api.data.ActionButton; +import org.labkey.api.data.BaseColumnInfo; +import org.labkey.api.data.BeanViewForm; +import org.labkey.api.data.ButtonBar; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.CompareType; +import org.labkey.api.data.Container; +import org.labkey.api.data.DataColumn; +import org.labkey.api.data.DataRegion; +import org.labkey.api.data.DataRegionSelection; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.DisplayColumn; +import org.labkey.api.data.ExcelWriter; +import org.labkey.api.data.MenuButton; +import org.labkey.api.data.ObjectFactory; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.data.RenderContext; +import org.labkey.api.data.SimpleDisplayColumn; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.Sort; +import org.labkey.api.data.TSVGridWriter; +import org.labkey.api.data.TSVWriter; +import org.labkey.api.data.Table; +import org.labkey.api.data.TableInfo; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.module.FolderType; import org.labkey.api.module.Module; @@ -47,7 +77,6 @@ import org.labkey.api.reader.DataLoader; import org.labkey.api.security.ActionNames; import org.labkey.api.security.RequiresPermission; -import org.labkey.api.security.RequiresSiteAdmin; import org.labkey.api.security.User; import org.labkey.api.security.ValidEmail; import org.labkey.api.security.permissions.AdminPermission; @@ -70,7 +99,6 @@ import org.labkey.api.specimen.security.permissions.RequestSpecimensPermission; import org.labkey.api.specimen.settings.DisplaySettings; import org.labkey.api.specimen.settings.RepositorySettings; -import org.labkey.api.specimen.settings.RequestNotificationSettings; import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.specimen.settings.StatusSettings; import org.labkey.api.study.CohortFilter; @@ -147,6 +175,7 @@ import org.labkey.specimen.security.permissions.ManageRequestsPermission; import org.labkey.specimen.security.permissions.ManageSpecimenActorsPermission; import org.labkey.specimen.security.permissions.SetSpecimenCommentsPermission; +import org.labkey.specimen.settings.RequestNotificationSettings; import org.labkey.specimen.view.NotificationBean; import org.labkey.specimen.view.SpecimenRequestNotificationEmailTemplate; import org.labkey.specimen.view.SpecimenSearchWebPart; @@ -157,13 +186,6 @@ import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.mvc.Controller; -import jakarta.mail.Address; -import jakarta.mail.Message; -import jakarta.mail.internet.AddressException; -import jakarta.mail.internet.InternetAddress; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -335,7 +357,7 @@ private ActionURL getManageStudyURL() public void ensureSpecimenRequestsConfigured(boolean checkExistingStatuses) { - if (!SettingsManager.get().isSpecimenRequestEnabled(getContainer(), checkExistingStatuses)) + if (!org.labkey.specimen.settings.SettingsManager.get().isSpecimenRequestEnabled(getContainer(), checkExistingStatuses)) throw new RedirectException(new ActionURL(SpecimenRequestConfigRequiredAction.class, getContainer())); } @@ -859,7 +881,7 @@ public ApiResponse execute(SpecimenWebPartForm form, BindException errors) groupings.add(form.getGrouping1()); groupings.add(form.getGrouping2()); settings.setSpecimenWebPartGroupings(groupings); - SettingsManager.get().saveRepositorySettings(container, settings); + org.labkey.specimen.settings.SettingsManager.get().saveRepositorySettings(container, settings); response.put("success", true); return response; } @@ -1464,7 +1486,7 @@ public ModelAndView getView(RequestNotificationSettings form, boolean reshow, Bi // try to get the settings from the form, just in case this is a reshow: RequestNotificationSettings settings = form; if (settings == null || settings.getReplyTo() == null) - settings = SettingsManager.get().getRequestNotificationSettings(getContainer()); + settings = org.labkey.specimen.settings.SettingsManager.get().getRequestNotificationSettings(getContainer()); return new JspView<>("/org/labkey/specimen/view/manageNotifications.jsp", settings, errors); } @@ -1491,7 +1513,7 @@ public boolean handlePost(RequestNotificationSettings settings, BindException er if (errors.hasErrors()) return false; - SettingsManager.get().saveRequestNotificationSettings(getContainer(), settings); + org.labkey.specimen.settings.SettingsManager.get().saveRequestNotificationSettings(getContainer(), settings); return true; } @@ -1533,7 +1555,7 @@ public ModelAndView getView(DisplaySettingsForm form, boolean reshow, BindExcept public boolean handlePost(DisplaySettingsForm form, BindException errors) { DisplaySettings settings = form.getBean(); - SettingsManager.get().saveDisplaySettings(getContainer(), settings); + org.labkey.specimen.settings.SettingsManager.get().saveDisplaySettings(getContainer(), settings); return true; } @@ -1654,7 +1676,7 @@ public boolean handlePost(ManageRepositorySettingsForm form, BindException error settings.setSimple(form.isSimple()); settings.setEnableRequests(!form.isSimple() && form.isEnableRequests()); settings.setSpecimenDataEditable(!form.isSimple() && form.isSpecimenDataEditable()); - SettingsManager.get().saveRepositorySettings(getContainer(), settings); + org.labkey.specimen.settings.SettingsManager.get().saveRepositorySettings(getContainer(), settings); return true; } @@ -1852,7 +1874,7 @@ public boolean handlePost(DefaultRequirementsForm form, BindException errors) private void createDefaultRequirement(Integer actorId, String description, SpecimenRequestRequirementType type) { - if (actorId != null && actorId.intValue() > 0 && description != null && description.length() > 0) + if (actorId != null && actorId.intValue() > 0 && description != null && !description.isEmpty()) { SpecimenRequestRequirement requirement = new SpecimenRequestRequirement(); requirement.setContainer(getContainer()); @@ -1866,7 +1888,7 @@ private void createDefaultRequirement(Integer actorId, String description, Speci @Override public ActionURL getSuccessURL(DefaultRequirementsForm form) { - if (form.getNextPage() != null && form.getNextPage().length() > 0) + if (form.getNextPage() != null && !form.getNextPage().isEmpty()) return new ActionURL(form.getNextPage()); else return getManageStudyURL(); @@ -3034,7 +3056,7 @@ public boolean handlePost(StatusEditForm form, BindException errors) if (settings.isUseShoppingCart() != form.isUseShoppingCart()) { settings.setUseShoppingCart(form.isUseShoppingCart()); - SettingsManager.get().saveStatusSettings(getContainer(), settings); + org.labkey.specimen.settings.SettingsManager.get().saveStatusSettings(getContainer(), settings); } return true; } @@ -3042,7 +3064,7 @@ public boolean handlePost(StatusEditForm form, BindException errors) @Override public ActionURL getSuccessURL(StatusEditForm form) { - if (form.getNextPage() != null && form.getNextPage().length() > 0) + if (form.getNextPage() != null && !form.getNextPage().isEmpty()) return new ActionURL(form.getNextPage()); else return getManageStudyURL(); @@ -3672,7 +3694,7 @@ private List getNotifications(SpecimenReques private void sendNewRequestNotifications(SpecimenRequest request, BindException errors) throws Exception { - RequestNotificationSettings settings = SettingsManager.get().getRequestNotificationSettings(request.getContainer()); + RequestNotificationSettings settings = org.labkey.specimen.settings.SettingsManager.get().getRequestNotificationSettings(request.getContainer()); Address[] notify = settings.getNewRequestNotifyAddresses(); if (notify != null && notify.length > 0) { @@ -3686,7 +3708,7 @@ private void sendNewRequestNotifications(SpecimenRequest request, BindException private void sendNotification(DefaultRequestNotification notification, boolean includeInactiveUsers, BindException errors) throws Exception { - RequestNotificationSettings settings = SettingsManager.get().getRequestNotificationSettings(getContainer()); + RequestNotificationSettings settings = org.labkey.specimen.settings.SettingsManager.get().getRequestNotificationSettings(getContainer()); SpecimenRequest specimenRequest = notification.getSpecimenRequest(); String specimenList = null; @@ -4622,7 +4644,7 @@ public ManageRequirementBean(ViewContext context, SpecimenRequest request, Speci public boolean isDefaultNotification(ActorNotificationRecipientSet notification) { - RequestNotificationSettings settings = SettingsManager.get().getRequestNotificationSettings(getContainer()); + RequestNotificationSettings settings = org.labkey.specimen.settings.SettingsManager.get().getRequestNotificationSettings(getContainer()); if (settings.getDefaultEmailNotifyEnum() == RequestNotificationSettings.DefaultEmailNotifyEnum.All) return true; // All should be checked else if (settings.getDefaultEmailNotifyEnum() == RequestNotificationSettings.DefaultEmailNotifyEnum.None) diff --git a/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java b/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java index c2389d4873b..4fd362a0d98 100644 --- a/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java +++ b/specimen/src/org/labkey/specimen/importer/SpecimenSettingsImporter.java @@ -26,7 +26,6 @@ import org.labkey.api.specimen.location.LocationImpl; import org.labkey.api.specimen.settings.DisplaySettings; import org.labkey.api.specimen.settings.RepositorySettings; -import org.labkey.api.specimen.settings.RequestNotificationSettings; import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.specimen.settings.StatusSettings; import org.labkey.api.study.StudyService; @@ -42,6 +41,7 @@ import org.labkey.specimen.requirements.SpecimenRequestRequirement; import org.labkey.specimen.requirements.SpecimenRequestRequirementProvider; import org.labkey.specimen.requirements.SpecimenRequestRequirementType; +import org.labkey.specimen.settings.RequestNotificationSettings; import org.labkey.specimen.writer.SpecimenArchiveDataTypes; import org.labkey.study.xml.DefaultRequirementType; import org.labkey.study.xml.DefaultRequirementsType; @@ -160,7 +160,7 @@ private void importSettings(SimpleStudyImportContext ctx, SpecimenSettingsType x reposSettings.setEnableRequests(xmlSettings.getEnableRequests()); if (xmlSettings.isSetEditableRepository()) reposSettings.setSpecimenDataEditable(xmlSettings.getEditableRepository()); - SettingsManager.get().saveRepositorySettings(c, reposSettings); + org.labkey.specimen.settings.SettingsManager.get().saveRepositorySettings(c, reposSettings); // location types SpecimenSettingsType.LocationTypes xmlLocationTypes = xmlSettings.getLocationTypes(); @@ -234,7 +234,7 @@ else if (newStatusLabel != null) if (settings.isUseShoppingCart() != xmlRequestStatuses.getMultipleSearch()) { settings.setUseShoppingCart(xmlRequestStatuses.getMultipleSearch()); - SettingsManager.get().saveStatusSettings(ctx.getContainer(), settings); + org.labkey.specimen.settings.SettingsManager.get().saveStatusSettings(ctx.getContainer(), settings); } } } @@ -350,12 +350,12 @@ private void importDefaultRequirements(SimpleStudyImportContext ctx, SpecimenSet private void createDefaultRequirement(DefaultRequirementsType xmlReqs, SimpleStudyImportContext ctx, RequirementType type) { - if (xmlReqs != null && xmlReqs.getRequirementArray().length > 0) + if (xmlReqs != null) { for (DefaultRequirementType xmlReq : xmlReqs.getRequirementArray()) { List matchingActors = SpecimenRequestRequirementProvider.get().getActorsByLabel(ctx.getContainer(), xmlReq.getActor()); - if (matchingActors.size() > 0) + if (!matchingActors.isEmpty()) { SpecimenRequestRequirement requirement = new SpecimenRequestRequirement(); requirement.setContainer(ctx.getContainer()); @@ -392,7 +392,7 @@ private void importDisplaySettings(SimpleStudyImportContext ctx, SpecimenSetting display.setZeroVials(warnings.getZeroVials()); } - SettingsManager.get().saveDisplaySettings(ctx.getContainer(), display); + org.labkey.specimen.settings.SettingsManager.get().saveDisplaySettings(ctx.getContainer(), display); } } @@ -461,7 +461,7 @@ private void importNotifications(SimpleStudyImportContext ctx, SpecimenSettingsT if (xmlNotifications.getSpecimensAttachment() != null) notifications.setSpecimensAttachment(xmlNotifications.getSpecimensAttachment()); - SettingsManager.get().saveRequestNotificationSettings(ctx.getContainer(), notifications); + org.labkey.specimen.settings.SettingsManager.get().saveRequestNotificationSettings(ctx.getContainer(), notifications); } } diff --git a/specimen/src/org/labkey/specimen/notifications/DefaultRequestNotification.java b/specimen/src/org/labkey/specimen/notifications/DefaultRequestNotification.java index 6d7ef033b03..b838346ba0a 100644 --- a/specimen/src/org/labkey/specimen/notifications/DefaultRequestNotification.java +++ b/specimen/src/org/labkey/specimen/notifications/DefaultRequestNotification.java @@ -26,23 +26,18 @@ import org.labkey.api.data.TSVWriter; import org.labkey.api.specimen.Vial; import org.labkey.api.specimen.query.SpecimenQueryView; -import org.labkey.api.specimen.settings.RequestNotificationSettings; -import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.view.ViewContext; import org.labkey.specimen.model.SpecimenRequestEvent; import org.labkey.specimen.requirements.SpecimenRequest; import org.labkey.specimen.requirements.SpecimenRequestRequirement; +import org.labkey.specimen.settings.RequestNotificationSettings; +import org.labkey.specimen.settings.SettingsManager; import java.io.IOException; import java.sql.SQLException; import java.util.ArrayList; import java.util.List; -/** - * User: brittp - * Date: May 4, 2007 - * Time: 3:41:44 PM - */ public class DefaultRequestNotification { protected List _recipients; @@ -68,7 +63,7 @@ public DefaultRequestNotification(SpecimenRequest request, List vials = getSpecimenList(); - if (vials != null && vials.size() > 0) + if (vials != null && !vials.isEmpty()) { SpecimenQueryView view = SpecimenQueryView.createView(context, vials, SpecimenQueryView.ViewType.VIALS_EMAIL); view.setDisableLowVialIndicators(true); @@ -86,7 +81,7 @@ private void addSpecimenListFileIfNeeded(ViewContext context) throws Exception { final ByteArrayAttachmentFile specimenListFile; List vials = getSpecimenList(); - if (vials != null && vials.size() > 0) + if (vials != null && !vials.isEmpty()) { SpecimenQueryView view = SpecimenQueryView.createView(context, vials, SpecimenQueryView.ViewType.VIALS_EMAIL); view.getSettings().setMaxRows(-1); diff --git a/study/api-src/org/labkey/api/specimen/settings/RequestNotificationSettings.java b/specimen/src/org/labkey/specimen/settings/RequestNotificationSettings.java similarity index 98% rename from study/api-src/org/labkey/api/specimen/settings/RequestNotificationSettings.java rename to specimen/src/org/labkey/specimen/settings/RequestNotificationSettings.java index d2318592f53..10ed391eacf 100644 --- a/study/api-src/org/labkey/api/specimen/settings/RequestNotificationSettings.java +++ b/specimen/src/org/labkey/specimen/settings/RequestNotificationSettings.java @@ -13,26 +13,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.labkey.api.specimen.settings; +package org.labkey.specimen.settings; +import jakarta.mail.Address; import org.labkey.api.data.Container; import org.labkey.api.security.User; import org.labkey.api.security.ValidEmail; import org.labkey.api.settings.LookAndFeelProperties; import org.labkey.api.util.SafeToRenderEnum; -import jakarta.mail.Address; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.StringTokenizer; -/* - * User: brittp - * Date: May 8, 2009 - * Time: 2:52:06 PM - */ - public class RequestNotificationSettings { public static final String REPLY_TO_CURRENT_USER_VALUE = "[current user]"; diff --git a/specimen/src/org/labkey/specimen/settings/SettingsManager.java b/specimen/src/org/labkey/specimen/settings/SettingsManager.java new file mode 100644 index 00000000000..6adfb2d86eb --- /dev/null +++ b/specimen/src/org/labkey/specimen/settings/SettingsManager.java @@ -0,0 +1,103 @@ +package org.labkey.specimen.settings; + +import org.labkey.api.action.SpringActionController; +import org.labkey.api.data.Container; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.security.UserManager; +import org.labkey.api.specimen.SpecimenRequestStatus; +import org.labkey.api.specimen.SpecimenSchema; +import org.labkey.api.specimen.settings.DisplaySettings; +import org.labkey.api.specimen.settings.RepositorySettings; +import org.labkey.api.specimen.settings.StatusSettings; +import org.labkey.api.study.QueryHelper; +import org.labkey.specimen.SpecimenRequestManager; + +import java.util.List; +import java.util.Map; + +public class SettingsManager +{ + private final QueryHelper _requestStatusHelper; + + private static final SettingsManager INSTANCE = new SettingsManager(); + + public static SettingsManager get() + { + return INSTANCE; + } + + private SettingsManager() + { + _requestStatusHelper = new QueryHelper<>(()-> SpecimenSchema.get().getTableInfoSampleRequestStatus(), SpecimenRequestStatus.class); + } + + public RequestNotificationSettings getRequestNotificationSettings(Container container) + { + Map settingsMap = PropertyManager.getProperties(UserManager.getGuestUser(), + container, "SpecimenRequestNotifications"); + if (settingsMap.get("ReplyTo") == null) + { + try (var ignore = SpringActionController.ignoreSqlUpdates()) + { + RequestNotificationSettings defaults = RequestNotificationSettings.getDefaultSettings(container); + saveRequestNotificationSettings(container, defaults); + return defaults; + } + } + else + return new RequestNotificationSettings(settingsMap); + } + + public void saveRequestNotificationSettings(Container container, RequestNotificationSettings settings) + { + PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), + container, "SpecimenRequestNotifications", true); + settings.populateMap(settingsMap); + settingsMap.save(); + } + + public void saveDisplaySettings(Container container, DisplaySettings settings) + { + PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), + container, "SpecimenRequestDisplay", true); + settings.populateMap(settingsMap); + settingsMap.save(); + } + + public void saveStatusSettings(Container container, StatusSettings settings) + { + PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), + container, "SpecimenRequestStatus", true); + settings.populateMap(settingsMap); + settingsMap.save(); + } + + public void saveRepositorySettings(Container container, RepositorySettings settings) + { + PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), + container, "SpecimenRepositorySettings", true); + settings.populateMap(settingsMap); + settingsMap.save(); + SpecimenRequestManager.get().clearGroupedValuesForColumn(container); // May have changed groupings + } + + public boolean isSpecimenRequestEnabled(Container container) + { + return isSpecimenRequestEnabled(container, true); + } + + public boolean isSpecimenRequestEnabled(Container container, boolean checkExistingStatuses) + { + if (!checkExistingStatuses) + { + return org.labkey.api.specimen.settings.SettingsManager.get().getRepositorySettings(container).isEnableRequests(); + } + else + { + if (!org.labkey.api.specimen.settings.SettingsManager.get().getRepositorySettings(container).isEnableRequests()) + return false; + List statuses = _requestStatusHelper.getList(container, "SortOrder"); + return (statuses != null && statuses.size() > 1); + } + } +} diff --git a/specimen/src/org/labkey/specimen/view/SpecimenRequestNotificationEmailTemplate.java b/specimen/src/org/labkey/specimen/view/SpecimenRequestNotificationEmailTemplate.java index 1109437a1ef..df3de869c9a 100644 --- a/specimen/src/org/labkey/specimen/view/SpecimenRequestNotificationEmailTemplate.java +++ b/specimen/src/org/labkey/specimen/view/SpecimenRequestNotificationEmailTemplate.java @@ -18,22 +18,18 @@ import org.labkey.api.attachments.Attachment; import org.labkey.api.data.Container; import org.labkey.api.security.User; -import org.labkey.api.specimen.settings.RequestNotificationSettings; -import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; import org.labkey.api.util.Link.LinkBuilder; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.emailTemplate.EmailTemplate; import org.labkey.specimen.actions.SpecimenController; +import org.labkey.specimen.settings.RequestNotificationSettings; +import org.labkey.specimen.settings.SettingsManager; import java.util.List; import java.util.function.Function; -/** - * User: jeckels - * Date: 11/11/13 - */ public class SpecimenRequestNotificationEmailTemplate extends EmailTemplate { public static final String NAME = "Specimen request notification"; @@ -133,8 +129,7 @@ public void init(NotificationBean notification) _notification = notification; // For backwards compatibility, use the template specified in the notification management UI - RequestNotificationSettings settings = - SettingsManager.get().getRequestNotificationSettings(notification.getContainer()); + RequestNotificationSettings settings = SettingsManager.get().getRequestNotificationSettings(notification.getContainer()); _subjectSuffix = settings.getSubjectSuffix().replaceAll("%requestId%", "" + notification.getRequestId()); } diff --git a/specimen/src/org/labkey/specimen/view/SpecimenToolsWebPartFactory.java b/specimen/src/org/labkey/specimen/view/SpecimenToolsWebPartFactory.java index e71c081d4cf..a49fedcf82f 100644 --- a/specimen/src/org/labkey/specimen/view/SpecimenToolsWebPartFactory.java +++ b/specimen/src/org/labkey/specimen/view/SpecimenToolsWebPartFactory.java @@ -1,7 +1,6 @@ package org.labkey.specimen.view; import org.labkey.api.specimen.security.permissions.RequestSpecimensPermission; -import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.study.StudyUrls; import org.labkey.api.study.security.permissions.ManageStudyPermission; import org.labkey.api.study.view.StudyToolsWebPart; @@ -13,6 +12,7 @@ import org.labkey.specimen.actions.ShowSearchAction; import org.labkey.specimen.actions.SpecimenController.AutoReportListAction; import org.labkey.specimen.actions.SpecimenController.ShowCreateSpecimenRequestAction; +import org.labkey.specimen.settings.SettingsManager; import java.util.ArrayList; import java.util.List; diff --git a/specimen/src/org/labkey/specimen/view/manageNotifications.jsp b/specimen/src/org/labkey/specimen/view/manageNotifications.jsp index 9199bfe93a6..f1d1aa7a621 100644 --- a/specimen/src/org/labkey/specimen/view/manageNotifications.jsp +++ b/specimen/src/org/labkey/specimen/view/manageNotifications.jsp @@ -20,16 +20,15 @@ <%@ page import="org.labkey.api.data.Container" %> <%@ page import="org.labkey.api.security.SecurityUrls" %> <%@ page import="org.labkey.api.security.permissions.AdminPermission" %> -<%@ page import="org.labkey.api.specimen.settings.RequestNotificationSettings" %> -<%@ page import="org.labkey.api.specimen.settings.RequestNotificationSettings.DefaultEmailNotifyEnum" %> -<%@ page import="org.labkey.api.specimen.settings.RequestNotificationSettings.SpecimensAttachmentEnum" %> <%@ page import="org.labkey.api.study.StudyService" %> <%@ page import="org.labkey.api.study.StudyUrls" %> -<%@ page import="org.labkey.api.util.HtmlString" %> <%@ page import="org.labkey.api.view.ActionURL" %> <%@ page import="org.labkey.api.view.HttpView" %> <%@ page import="org.labkey.api.view.JspView" %> <%@ page import="org.labkey.specimen.actions.SpecimenController.ManageNotificationsAction" %> +<%@ page import="org.labkey.specimen.settings.RequestNotificationSettings" %> +<%@ page import="org.labkey.specimen.settings.RequestNotificationSettings.DefaultEmailNotifyEnum" %> +<%@ page import="org.labkey.specimen.settings.RequestNotificationSettings.SpecimensAttachmentEnum" %> <%@ page import="org.labkey.specimen.view.SpecimenRequestNotificationEmailTemplate" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> diff --git a/specimen/src/org/labkey/specimen/view/webPart.jsp b/specimen/src/org/labkey/specimen/view/webPart.jsp index c55d687834d..625b8ede82f 100644 --- a/specimen/src/org/labkey/specimen/view/webPart.jsp +++ b/specimen/src/org/labkey/specimen/view/webPart.jsp @@ -19,9 +19,8 @@ <%@ page import="org.labkey.api.security.User"%> <%@ page import="org.labkey.api.security.permissions.AdminPermission"%> <%@ page import="org.labkey.api.specimen.security.permissions.RequestSpecimensPermission"%> -<%@ page import="org.labkey.api.specimen.settings.SettingsManager" %> -<%@ page import="org.labkey.specimen.view.SpecimenWebPart" %> <%@ page import="org.labkey.api.study.StudyUrls" %> +<%@ page import="org.labkey.api.util.HtmlString" %> <%@ page import="org.labkey.api.view.ActionURL" %> <%@ page import="org.labkey.api.view.HttpView" %> <%@ page import="org.labkey.specimen.actions.ShowSearchAction" %> @@ -30,7 +29,8 @@ <%@ page import="org.labkey.specimen.actions.SpecimenController.AutoReportListAction" %> <%@ page import="org.labkey.specimen.actions.SpecimenController.ShowCreateSpecimenRequestAction" %> <%@ page import="org.labkey.specimen.actions.SpecimenController.ViewRequestsAction" %> -<%@ page import="org.labkey.api.util.HtmlString" %> +<%@ page import="org.labkey.specimen.settings.SettingsManager" %> +<%@ page import="org.labkey.specimen.view.SpecimenWebPart" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <% SpecimenWebPart.SpecimenWebPartBean bean = (SpecimenWebPart.SpecimenWebPartBean) HttpView.currentView().getModelBean(); diff --git a/specimen/src/org/labkey/specimen/writer/SpecimenSettingsWriter.java b/specimen/src/org/labkey/specimen/writer/SpecimenSettingsWriter.java index 0c8978b6bc7..8e9707f7795 100644 --- a/specimen/src/org/labkey/specimen/writer/SpecimenSettingsWriter.java +++ b/specimen/src/org/labkey/specimen/writer/SpecimenSettingsWriter.java @@ -21,7 +21,6 @@ import org.labkey.api.specimen.importer.RequestabilityManager; import org.labkey.api.specimen.settings.DisplaySettings; import org.labkey.api.specimen.settings.RepositorySettings; -import org.labkey.api.specimen.settings.RequestNotificationSettings; import org.labkey.api.specimen.settings.SettingsManager; import org.labkey.api.specimen.settings.StatusSettings; import org.labkey.api.study.Location; @@ -36,6 +35,7 @@ import org.labkey.specimen.model.SpecimenRequestActor; import org.labkey.specimen.requirements.SpecimenRequestRequirement; import org.labkey.specimen.requirements.SpecimenRequestRequirementProvider; +import org.labkey.specimen.settings.RequestNotificationSettings; import org.labkey.study.xml.DefaultRequirementType; import org.labkey.study.xml.DefaultRequirementsType; import org.labkey.study.xml.SpecimenRepositoryType; @@ -288,7 +288,7 @@ private void writeRequestForm(SpecimenSettingsType specimenSettingsType, SimpleS private void writeNotifications(SpecimenSettingsType specimenSettingsType, SimpleStudyExportContext ctx) { ctx.getLogger().info("Exporting specimen notification settings"); - RequestNotificationSettings notifications = SettingsManager.get().getRequestNotificationSettings(ctx.getContainer()); + RequestNotificationSettings notifications = org.labkey.specimen.settings.SettingsManager.get().getRequestNotificationSettings(ctx.getContainer()); SpecimenSettingsType.Notifications xmlNotifications = specimenSettingsType.addNewNotifications(); if (notifications.getReplyTo() != null) diff --git a/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java b/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java index 3a513b3b873..16fa8589a75 100644 --- a/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java +++ b/study/api-src/org/labkey/api/specimen/SpecimenMigrationService.java @@ -40,7 +40,5 @@ void importSpecimenArchive(@Nullable Path inputFile, PipelineJob job, SimpleStud boolean syncParticipantVisit) throws PipelineJobException, ValidationException; void clearRequestCaches(Container c); - void clearGroupedValuesForColumn(Container container); void updateVialCounts(Container container, User user); - void purgeRequestRequirementsAndActors(Container container); } diff --git a/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java b/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java index 2b2a77a1ba2..1bbb3d1670b 100644 --- a/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java +++ b/study/api-src/org/labkey/api/specimen/query/SpecimenQueryView.java @@ -45,7 +45,6 @@ import org.labkey.api.query.UserSchema; import org.labkey.api.reports.report.ReportUrls; import org.labkey.api.security.User; -import org.labkey.api.specimen.SpecimenManager; import org.labkey.api.specimen.SpecimenMigrationService; import org.labkey.api.specimen.SpecimenQuerySchema; import org.labkey.api.specimen.SpecimenSchema; @@ -83,11 +82,6 @@ import static java.util.Objects.requireNonNull; -/** - * User: brittp - * Date: Aug 23, 2006 - * Time: 3:38:44 PM - */ public class SpecimenQueryView extends BaseSpecimenQueryView { public enum Mode diff --git a/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java b/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java index aeeba2de7eb..ebf261a49b6 100644 --- a/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java +++ b/study/api-src/org/labkey/api/specimen/settings/SettingsManager.java @@ -1,23 +1,15 @@ package org.labkey.api.specimen.settings; -import org.labkey.api.action.SpringActionController; import org.labkey.api.data.Container; import org.labkey.api.data.PropertyManager; import org.labkey.api.security.UserManager; -import org.labkey.api.specimen.SpecimenMigrationService; -import org.labkey.api.specimen.SpecimenRequestStatus; -import org.labkey.api.specimen.SpecimenSchema; -import org.labkey.api.study.QueryHelper; -import java.util.List; import java.util.Map; public class SettingsManager { private static final SettingsManager INSTANCE = new SettingsManager(); - private final QueryHelper _requestStatusHelper; - public static SettingsManager get() { return INSTANCE; @@ -25,32 +17,6 @@ public static SettingsManager get() private SettingsManager() { - _requestStatusHelper = new QueryHelper<>(()->SpecimenSchema.get().getTableInfoSampleRequestStatus(), SpecimenRequestStatus.class); - } - - public RequestNotificationSettings getRequestNotificationSettings(Container container) - { - Map settingsMap = PropertyManager.getProperties(UserManager.getGuestUser(), - container, "SpecimenRequestNotifications"); - if (settingsMap.get("ReplyTo") == null) - { - try (var ignore = SpringActionController.ignoreSqlUpdates()) - { - RequestNotificationSettings defaults = RequestNotificationSettings.getDefaultSettings(container); - saveRequestNotificationSettings(container, defaults); - return defaults; - } - } - else - return new RequestNotificationSettings(settingsMap); - } - - public void saveRequestNotificationSettings(Container container, RequestNotificationSettings settings) - { - PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), - container, "SpecimenRequestNotifications", true); - settings.populateMap(settingsMap); - settingsMap.save(); } public DisplaySettings getDisplaySettings(Container container) @@ -59,28 +25,12 @@ public DisplaySettings getDisplaySettings(Container container) return settingsMap.isEmpty() ? DisplaySettings.getDefaultSettings() : new DisplaySettings(settingsMap); } - public void saveDisplaySettings(Container container, DisplaySettings settings) - { - PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), - container, "SpecimenRequestDisplay", true); - settings.populateMap(settingsMap); - settingsMap.save(); - } - public StatusSettings getStatusSettings(Container container) { Map settingsMap = PropertyManager.getProperties(UserManager.getGuestUser(), container, "SpecimenRequestStatus"); return settingsMap.get(StatusSettings.KEY_USE_SHOPPING_CART) == null ? StatusSettings.getDefaultSettings() : new StatusSettings(settingsMap); } - public void saveStatusSettings(Container container, StatusSettings settings) - { - PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), - container, "SpecimenRequestStatus", true); - settings.populateMap(settingsMap); - settingsMap.save(); - } - public boolean isSpecimenShoppingCartEnabled(Container container) { return getStatusSettings(container).isUseShoppingCart(); @@ -91,33 +41,4 @@ public RepositorySettings getRepositorySettings(Container container) Map settingsMap = PropertyManager.getProperties(UserManager.getGuestUser(), container, "SpecimenRepositorySettings"); return settingsMap.isEmpty() ? RepositorySettings.getDefaultSettings(container) : new RepositorySettings(container, settingsMap); } - - public void saveRepositorySettings(Container container, RepositorySettings settings) - { - PropertyManager.PropertyMap settingsMap = PropertyManager.getWritableProperties(UserManager.getGuestUser(), - container, "SpecimenRepositorySettings", true); - settings.populateMap(settingsMap); - settingsMap.save(); - SpecimenMigrationService.get().clearGroupedValuesForColumn(container); // May have changed groupings - } - - public boolean isSpecimenRequestEnabled(Container container) - { - return isSpecimenRequestEnabled(container, true); - } - - public boolean isSpecimenRequestEnabled(Container container, boolean checkExistingStatuses) - { - if (!checkExistingStatuses) - { - return getRepositorySettings(container).isEnableRequests(); - } - else - { - if (!getRepositorySettings(container).isEnableRequests()) - return false; - List statuses = _requestStatusHelper.getList(container, "SortOrder"); - return (statuses != null && statuses.size() > 1); - } - } }