Migrate DbCache use to DatabaseCache

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

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

@@ -2,79 +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);
-
-        Map<String, Object> mm = new HashMap<>();                  // Modifiable map
-        Map<String, Object> 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);
-        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);
-        m2 = (Map) DbCache.get(testTable, key);
-        assertNull(m2);
-
-        //Does cache get cleared on insert
-        mm.remove("RowId");
-        mm = Table.insert(ctx.getUser(), testTable, mm);
-        int rowId2 = ((Integer) mm.get("RowId"));
-        key = "RowId" + rowId2;
-        DbCache.put(testTable, key, umm);
-        mm.remove("RowId");
-        mm = Table.insert(ctx.getUser(), testTable, mm);
-        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);
-            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);
-        Table.delete(testTable, rowId3);
+        DbCache.logUnmatched();
     }
 }

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

@@ -16,15 +16,19 @@
 
 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;
 
 /**
- *
  * 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
@@ -33,7 +37,8 @@
 @Deprecated
 public class DbCache
 {
-    private static final Map<Path, DatabaseCache<String, Object>> CACHES = new HashMap<>(100);
+    private static final Logger LOG = LogHelper.getLogger(DbCache.class, "DbCache invalidations");
+    private static final Map<Path, DatabaseCache<String, Object>> CACHES = new HashMap<>(10);
 
     public static DatabaseCache<String, Object> getCache(TableInfo tinfo, boolean create)
     {
@@ -84,14 +89,20 @@ public static void invalidateAll(TableInfo tinfo)
     {
         DatabaseCache<String, Object> cache = CACHES.get(tinfo.getNotificationKey());
         if (null != cache)
+        {
+            trackInvalidate(tinfo);
             cache.clear();
+        }
     }
 
     public static void clear(TableInfo tinfo)
     {
         DatabaseCache<String, Object> cache = getCache(tinfo, false);
         if (null != cache)
+        {
+            trackInvalidate(tinfo);
             cache.clear();
+        }
     }
 
     public static void removeUsingPrefix(TableInfo tinfo, String name)
@@ -100,4 +111,74 @@ public static void removeUsingPrefix(TableInfo tinfo, String name)
         if (null != cache)
             cache.removeUsingFilter(new Cache.StringPrefixFilter(name));
     }
+
+    /**
+     * 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
+     * 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<String> 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()
+    {
+        if (TRACKING_BAG.isEmpty())
+        {
+            LOG.info("No unmatched cache removes");
+        }
+        else
+        {
+            TRACKING_BAG.uniqueSet().forEach(key -> LOG.error("Unmatched {}", key));
+        }
+    }
 }

api/src/org/labkey/api/data/AtomicDatabaseInteger.java

@@ -29,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;
@@ -43,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)

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());
     }
 
 

api/src/org/labkey/api/data/Table.java

@@ -632,7 +632,7 @@ protected static <K> Map<String, Object> _getTableData(TableInfo table, K from,
         //noinspection unchecked
         ObjectFactory<K> f = ObjectFactory.Registry.getFactory((Class<K>)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);
     }

api/src/org/labkey/api/exp/api/ExperimentService.java

@@ -746,6 +746,10 @@ static void validateParentAlias(Map<String, String> aliasMap, Set<String> reserv
 
     void clearCaches();
 
+    void clearExperimentRunCache();
+
+    void invalidateExperimentRun(String lsid);
+
     List<ProtocolApplicationParameter> getProtocolApplicationParameters(int rowId);
 
     void moveContainer(Container c, Container cOldParent, Container cNewParent) throws ExperimentException;

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;
@@ -334,7 +335,9 @@ protected Map<String, Object> _insert(User user, Container c, Map<String, Object
 
         try
         {
-            return Table.insert(user, getDbTable(), row);
+            Map<String, Object> ret = Table.insert(user, getDbTable(), row);
+            if (getDbTable().getName().equals("AssaySpecimen")) { DbCache.trackRemove(getDbTable()); }
+            return ret;
         }
         catch (RuntimeValidationException e)
         {
@@ -563,7 +566,9 @@ protected Map<String, Object> _update(User user, Container c, Map<String, Object
             }
         }
 
-        return Table.update(user, getDbTable(), row, keys);
+        Map<String, Object> 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.

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<TableInfo> set, User user);
 }

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<T>
 {

api/src/org/labkey/api/util/ExceptionUtil.java

@@ -119,16 +119,20 @@ 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)
         {
             return MiniProfiler.NO_STACK_TRACE_AVAILABLE;
         }
         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("<pre class='exception-stacktrace'>\n" + s + "</pre>\n");
     }
 
-
     public static HtmlString getUnauthorizedMessage(ViewContext context)
     {
         return HtmlString.unsafe("<table width=\"100%\"><tr><td align=left>" +

core/src/org/labkey/core/CoreContainerListener.java

@@ -38,11 +38,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);