Merge remote-tracking branch 'origin/develop' into fb_miscAmountsAndUnits

# Conflicts:
#	experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java

Author: labkey-susanh

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

@@ -96,9 +96,12 @@ public V get(@NotNull K key, @Nullable Object argument, CacheLoader<K, V> loader
     {
         Wrapper<V> w;
 
+        if (null == loader)
+            loader = _loader;
+
         try (var ignored = DebugInfoDumper.pushThreadDumpContext(this.getClass().getSimpleName() + ".get(" + key + ")"))
         {
-            synchronized(_cache)
+            synchronized (loader == null ? _cache : loader.getSyncObject(_cache))
             {
                 w = _cache.get(key);
                 if (null == w)
@@ -153,9 +156,6 @@ public V get(@NotNull K key, @Nullable Object argument, CacheLoader<K, V> loader
 
             try
             {
-                if (null == loader)
-                    loader = _loader;
-
                 if (null == loader)
                     throw new IllegalStateException("cache loader was not provided");
 

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

@@ -42,21 +42,14 @@ public interface Cache<K, V>
      */
     int removeUsingFilter(Predicate<K> filter);
 
-    class StringPrefixFilter implements Predicate<String>
-    {
-        private final String _prefix;
-
-        public StringPrefixFilter(String prefix)
-        {
-            _prefix = prefix;
-        }
-
-        @Override
-        public boolean test(String s)
+    record StringPrefixFilter(String prefix) implements Predicate<String>
         {
-            return s.startsWith(_prefix);
+            @Override
+            public boolean test(String s)
+            {
+                return s.startsWith(prefix);
+            }
         }
-    }
 
     Set<K> getKeys();
 

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

@@ -25,4 +25,13 @@
 public interface CacheLoader<K, V>
 {
     V load(@NotNull K key, @Nullable Object argument);
+
+    /**
+     * Returns an object that can be used to synchronize access to the cache. Typically, the cache itself but it's
+     * OK to swap with another lock object when running inside of a transaction to reduce the likelihood of deadlocks.
+     */
+    default Object getSyncObject(Cache<?, ?> cache)
+    {
+        return cache;
+    }
 }

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

@@ -69,6 +69,23 @@ public V get(@NotNull K key)
         return get(key, null, null);
     }
 
+
+    /** A dummy loader that never fetches a value. Lets us see if the shared cache already has a value for this key. */
+    private final CacheLoader<K, V> _nonLoader = new CacheLoader<K, V>()
+    {
+        @Override
+        public V load(@NotNull K key, @Nullable Object argument)
+        {
+            throw new MissingCacheEntryException();
+        }
+
+        @Override
+        public Object getSyncObject(Cache<?, ?> cache)
+        {
+            return TransactionCache.this;
+        }
+    };
+
     @Override
     public V get(@NotNull K key, Object arg, @Nullable CacheLoader<K, V> loader)
     {
@@ -85,9 +102,7 @@ else if (null == v && !_hasBeenCleared)
             try
             {
                 // Entry has not been modified in the private cache, so read through to shared cache
-                v = _sharedCache.get(key, null, (key1, argument) -> {
-                    throw new MissingCacheEntryException();
-                });
+                v = _sharedCache.get(key, null, _nonLoader);
                 // Shared cache has an entry for this key, so return it and don't invoke the loader
                 if (null == v)
                     v = NULL_MARKER; // Cached miss in shared cache; use null marker to skip loading. Issue 47234

api/src/org/labkey/api/query/UserSchema.java

@@ -273,16 +273,15 @@ public Object _getTableOrQuery(String name, ContainerFilter cf, boolean includeE
         if (!canReadSchema())
             return null; // See #21014
 
-        TableInfo table = null;
+        TableInfo table;
         String cacheKey = cacheKey(name, cf, includeExtraMetadata, forWrite);
         if (null != cacheKey)
         {
             table = tableInfoCache.get(cacheKey);
             if (null != table)
                 return table;
         }
-        if (null == table)
-            table = createTable(name, cf, includeExtraMetadata);
+        table = createTable(name, cf, includeExtraMetadata);
         Object torq;
 
         if (table != null)

experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java

@@ -931,8 +931,6 @@ public void save(User user)
             }
         }
 
-        // NOTE cacheMaterialSource() of course calls transactioncache.put(), which does not alter the shared cache! (BUG?)
-        // Just call uncache(), and let normal cache loading do its thing
         SampleTypeServiceImpl.get().clearMaterialSourceCache(getContainer());
     }
 

experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java

@@ -45,6 +45,7 @@
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.data.ConversionExceptionWithMessage;
+import org.labkey.api.data.DatabaseCache;
 import org.labkey.api.data.DbSchema;
 import org.labkey.api.data.DbScope;
 import org.labkey.api.data.DbSequence;
@@ -202,10 +203,11 @@ public static SampleTypeServiceImpl get()
 
     private static final Logger LOG = LogHelper.getLogger(SampleTypeServiceImpl.class, "Info about sample type operations");
 
-    // SampleType -> Container cache
+    /** SampleType LSID -> Container cache */
     private final Cache<String, String> sampleTypeCache = CacheManager.getStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "SampleType to container");
 
-    private final Cache<String, SortedSet<MaterialSource>> materialSourceCache = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Material sources", (container, argument) ->
+    /** ContainerId -> MaterialSources */
+    private final Cache<String, SortedSet<MaterialSource>> materialSourceCache = DatabaseCache.get(ExperimentServiceImpl.get().getSchema().getScope(), CacheManager.UNLIMITED, CacheManager.DAY, "Material sources", (container, argument) ->
     {
         Container c = ContainerManager.getForId(container);
         if (c == null)
@@ -215,7 +217,6 @@ public static SampleTypeServiceImpl get()
         return Collections.unmodifiableSortedSet(new TreeSet<>(new TableSelector(getTinfoMaterialSource(), filter, null).getCollection(MaterialSource.class)));
     });
 
-
     Cache<String, SortedSet<MaterialSource>> getMaterialSourceCache()
     {
         return materialSourceCache;
@@ -954,9 +955,7 @@ public ExpSampleTypeImpl createSampleType(Container c, User u, String name, Stri
                     else
                         ExperimentService.get().ensureDataTypeContainerExclusionsNonAdmin(ExperimentService.DataTypeForExclusion.DashboardSampleType, st.getRowId(), c, u);
                     transaction.addCommitTask(() -> clearMaterialSourceCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK);
-                    transaction.addCommitTask(() -> {
-                        indexSampleType(SampleTypeService.get().getSampleType(domain.getTypeURI()), SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified));
-                    }, POSTCOMMIT);
+                    transaction.addCommitTask(() -> indexSampleType(SampleTypeService.get().getSampleType(domain.getTypeURI()), SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified)), POSTCOMMIT);
 
                     return st;
                 }
@@ -1012,8 +1011,7 @@ public long next(Date date)
         public DbSequence getDbSequence(Date date)
         {
             Pair<String,Integer> seqName = getSequenceName(date);
-            final DbSequence seq = DbSequenceManager.getPreallocatingSequence(ContainerManager.getRoot(), seqName.first, seqName.second, 100);
-            return seq;
+            return DbSequenceManager.getPreallocatingSequence(ContainerManager.getRoot(), seqName.first, seqName.second, 100);
         }
     }
 
@@ -1083,7 +1081,7 @@ public ValidationException updateSampleType(GWTDomain<? extends GWTPropertyDescr
             validateSampleTypeName(container, user, newName, oldSampleTypeName.equalsIgnoreCase(newName));
             hasNameChange = true;
             st.setName(newName);
-            changeDetails.append("The name of the sample type '" + oldSampleTypeName + "' was changed to '" + newName + "'.");
+            changeDetails.append("The name of the sample type '").append(oldSampleTypeName).append("' was changed to '").append(newName).append("'.");
         }
 
         String newDescription = StringUtils.trimToNull(update.getDescription());
@@ -1595,7 +1593,7 @@ private AliquotAvailableAmountUnit computeAliquotTotalAmounts(List<AliquotAmount
             return null;
 
         Unit totalUnit = null;
-        String totalUnitsStr = null;
+        String totalUnitsStr;
         if (!StringUtils.isEmpty(sampleTypeUnitsStr))
             totalUnitsStr = sampleTypeUnitsStr;
         else if (!StringUtils.isEmpty(sampleItemUnitsStr))
@@ -1915,9 +1913,7 @@ public Map<String, Integer> moveSamples(Collection<? extends ExpMaterial> sample
                 if (inventoryService != null)
                 {
                     Map<String, Integer> inventoryCounts = inventoryService.moveSamples(sampleIds, targetContainer, user);
-                    inventoryCounts.forEach((key, count) -> {
-                        updateCounts.compute(key, (k, c) -> c == null ? count : c + count);
-                    });
+                    inventoryCounts.forEach((key, count) -> updateCounts.compute(key, (k, c) -> c == null ? count : c + count));
                 }
 
                 // create summary audit entries for the source and target containers
@@ -2264,7 +2260,6 @@ private void deleteSampleCounterSequence(Container container, boolean isRootOnly
         Container seqContainer = container.getProject();
         DbSequenceManager.delete(seqContainer, seqName);
         DbSequenceManager.invalidatePreallocatingSequence(container, seqName, 0);
-        return;
     }
 
     @Override