Issue 53472: OutOfMemoryError during giant deletes from lists

api/src/org/labkey/api/assay/AssayService.java

@@ -87,7 +87,8 @@ static void setInstance(AssayService impl)
     AssaySchema createSchema(User user, Container container, @Nullable Container targetStudy);
 
     /**
-     * @return all the assay protocols that are in scope in the given container
+     * @return all the assay protocols that are in scope in the given container, which may include those
+     * defined in other containers
      */
     @NotNull List<ExpProtocol> getAssayProtocols(Container container);
 

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

@@ -37,7 +37,7 @@ public interface ExpObject extends Identifiable, Comparable<ExpObject>
     String DEFAULT_CPAS_TYPE = "Object";
 
     /** Prevent edits to this object. Subsequent calls to setters will throw an IllegalStateException */
-    void lock();
+    ExpObject lock();
 
     int getRowId();
     /** Get the exp.object objectId */

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

@@ -24,13 +24,15 @@
 import com.fasterxml.jackson.core.json.JsonWriteFeature;
 import com.fasterxml.jackson.databind.*;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.mutable.MutableBoolean;
 import org.apache.logging.log4j.Logger;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.junit.Assert;
 import org.junit.Test;
 import org.labkey.api.action.ApiUsageException;
+import org.labkey.api.collections.CaseInsensitiveHashMap;
 import org.labkey.api.data.Container;
 import org.labkey.api.security.User;
 import org.labkey.api.settings.AppProps;
@@ -202,15 +204,20 @@ public static List<JSONObject> toJSONObjectList(JSONArray array)
         return result;
     }
 
-    // The new JSONObject.toMap() translates all JSONObjects into Maps and JSONArrays into Lists. In many cases, this is
-    // fine, but some existing code paths want to maintain the contained JSONObjects and JSONArrays. This method does
-    // that, acting more like the old JSONObject.toMap().
-    public static void fillMapShallow(JSONObject json, Map<String, Object> map)
+    /**
+     * The new JSONObject.toMap() translates all JSONObjects into Maps and JSONArrays into Lists. In many cases, this is
+     * fine, but some existing code paths want to maintain the contained JSONObjects and JSONArrays. This method does
+     * that, acting more like the old JSONObject.toMap().
+     * @return whether there were duplicate values in the map (useful when filling a case-insensitive map)
+      */
+    public static boolean fillMapShallow(JSONObject json, Map<String, Object> map)
     {
+        MutableBoolean hadDuplicates = new MutableBoolean(false);
         json.keySet().forEach(key -> {
             Object value = json.get(key);
-            map.put(key, JSONObject.NULL == value ? null : value);
+            hadDuplicates.setValue(map.put(key, JSONObject.NULL == value ? null : value) != null || hadDuplicates.booleanValue());
         });
+        return hadDuplicates.booleanValue();
     }
 
     // New JSONObject discards all properties with null values. This returns a JSONObject containing all Map values,
@@ -461,6 +468,20 @@ public void testJsonString()
             assertEquals(u.getUserId(), roundTripJson.get("user"));
         }
 
+        @Test
+        public void testConflictingCase()
+        {
+            JSONObject json = new JSONObject();
+            json.put("foo", "bar");
+            json.put("x", "y");
+            assertFalse(fillMapShallow(json, new HashMap<>()));
+            assertFalse(fillMapShallow(json, new CaseInsensitiveHashMap<>()));
+
+            json.put("FOO", "BAR");
+            assertFalse(fillMapShallow(json, new HashMap<>()));
+            assertTrue(fillMapShallow(json, new CaseInsensitiveHashMap<>()));
+        }
+
         @Test
         public void testSanitize()
         {

assay/src/org/labkey/assay/AssayManager.java

@@ -138,6 +138,7 @@ public Set<String> getPermittedContainerIds(User user, Map<String, Container> co
         }
     };
 
+    /** Cache the protocols defined in a given container, which we can quickly compose to get the protocols in scope */
     private static final Cache<Container, List<ExpProtocol>> PROTOCOL_CACHE = CacheManager.getCache(CacheManager.UNLIMITED, TimeUnit.HOURS.toMillis(1), "Assay protocols");
 
     private static final List<AssayListener> _listeners = new CopyOnWriteArrayList<>();
@@ -287,7 +288,7 @@ private class ModuleAssayLsidHandlerFinder implements LsidHandlerFinder
 
         @Nullable
         @Override
-        public LsidHandler findHandler(String authority, String namespacePrefix)
+        public LsidHandler<?> findHandler(String authority, String namespacePrefix)
         {
             if (AppProps.getInstance().getDefaultLsidAuthority().equals(authority))
             {
@@ -360,29 +361,32 @@ public AssaySchema createSchema(User user, Container container, @Nullable Contai
     @Override
     public @NotNull List<ExpProtocol> getAssayProtocols(Container container)
     {
-        return PROTOCOL_CACHE.get(container, null, (c, argument) ->
-        {
-            // Build up a set of containers so that we can query them all at once
-            Set<Container> containers = c.getContainersFor(ContainerType.DataType.protocol);
-
-            List<? extends ExpProtocol> protocols = ExperimentService.get().getExpProtocols(containers.toArray(new Container[0]));
-            List<ExpProtocol> result = new ArrayList<>();
+        List<ExpProtocol> allProtocols = new ArrayList<>();
 
-            // Filter to just the ones that have an AssayProvider associated with them
-            for (ExpProtocol protocol : protocols)
+        for (Container containerInScope : container.getContainersFor(ContainerType.DataType.protocol))
+        {
+            List<ExpProtocol> ids = PROTOCOL_CACHE.get(containerInScope, null, (c, argument) ->
             {
-                AssayProvider p = getProvider(protocol);
-                if (p != null)
+                List<ExpProtocol> result = new ArrayList<>();
+
+                // Filter to just the ones that have an AssayProvider associated with them
+                for (ExpProtocol protocol : ExperimentService.get().getExpProtocols(c))
                 {
-                    // We don't want anyone editing our cached object
-                    protocol.lock();
-                    result.add(protocol);
+                    AssayProvider p = getProvider(protocol);
+                    if (p != null)
+                    {
+                        // We don't want anyone editing our cached object
+                        protocol.lock();
+                        result.add(protocol);
+                    }
                 }
-            }
-            // Sort them, just to be nice
-            Collections.sort(result);
-            return Collections.unmodifiableList(result);
-        });
+                return Collections.unmodifiableList(result);
+            });
+            allProtocols.addAll(ids);
+        }
+
+        Collections.sort(allProtocols);
+        return Collections.unmodifiableList(allProtocols);
     }
 
     @Override

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

@@ -43,9 +43,10 @@ abstract public class ExpObjectImpl implements ExpObject, Serializable
     protected ExpObjectImpl() {}
 
     @Override
-    public void lock()
+    public ExpObjectImpl lock()
     {
         _locked = true;
+        return this;
     }
 
     protected void ensureUnlocked()

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

@@ -322,6 +322,7 @@ public class ExperimentServiceImpl implements ExperimentService, ObjectReference
 
     private final Cache<String, ExpProtocolImpl> PROTOCOL_LSID_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, CacheManager.HOUR, "Protocol by LSID",
         (key, argument) -> getExpProtocol(new SimpleFilter(FieldKey.fromParts("LSID"), key)));
+
     private final Cache<String, ExperimentRun> EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), getTinfoExperimentRun().getCacheSize(), "Experiment Run by LSID", new ExperimentRunCacheLoader());
 
     private final Cache<String, SortedSet<DataClass>> dataClassCache = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Data classes", (containerId, argument) ->

query/src/org/labkey/query/controllers/QueryController.java

@@ -4580,8 +4580,9 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole
             // NOTE RowMapFactory is faster, but for update it's important to preserve missing v explicit NULL values
             // Do we need to support some sort of UNDEFINED and NULL instance of MvFieldWrapper?
             RowMapFactory<Object> f = null;
-            if (commandType == CommandType.insert || commandType == CommandType.insertWithKeys)
+            if (commandType == CommandType.insert || commandType == CommandType.insertWithKeys || commandType == CommandType.delete)
                 f = new RowMapFactory<>();
+            CaseInsensitiveHashMap<Object> referenceCasing = new CaseInsensitiveHashMap<>();
 
             for (int idx = 0; idx < rows.length(); ++idx)
             {
@@ -4596,10 +4597,14 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole
                 }
                 if (null != jsonObj)
                 {
-                    // TODO: validate duplicate keys with different cases don't exist in data ('Name' vs 'name') (Issue 52616)
-                    Map<String, Object> rowMap = null == f ? new CaseInsensitiveHashMap<>() : f.getRowMap();
+                    Map<String, Object> rowMap = null == f ? new CaseInsensitiveHashMap<>(new HashMap<>(), referenceCasing) : f.getRowMap();
                     // Use shallow copy since jsonObj.toMap() will translate contained JSONObjects into Maps, which we don't want
-                    JsonUtil.fillMapShallow(jsonObj, rowMap);
+                    boolean conflictingCasing = JsonUtil.fillMapShallow(jsonObj, rowMap);
+                    if (conflictingCasing)
+                    {
+                        // Issue 52616
+                        LOG.error("Row contained conflicting casing for key names in the incoming row: {}", jsonObj);
+                    }
                     if (allowRowAttachments())
                         addRowAttachments(table, rowMap, idx, commandIndex);