Inspect cached objects for questionable members

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

@@ -20,20 +20,28 @@
 import org.jetbrains.annotations.Nullable;
 import org.labkey.api.cache.ehcache.EhCacheProvider;
 import org.labkey.api.collections.CollectionUtils;
+import org.labkey.api.data.Container;
+import org.labkey.api.data.Project;
 import org.labkey.api.mbean.LabKeyManagement;
+import org.labkey.api.security.User;
 import org.labkey.api.util.logging.LogHelper;
 
+import java.lang.ref.Reference;
+import java.lang.reflect.Array;
+import java.lang.reflect.Field;
+import java.lang.reflect.InaccessibleObjectException;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 
-/**
- * User: adam
- * Date: Jun 20, 2010
- * Time: 10:05:07 AM
- */
-
 public class CacheManager
 {
     private static final Logger LOG = LogHelper.getLogger(CacheManager.class, "General cache information");
@@ -112,7 +120,7 @@ private static void addToKnownCaches(TrackingCache<?, ?> cache)
         {
             KNOWN_CACHES.add(cache);
 
-            LOG.debug("Known caches: " + KNOWN_CACHES.size());
+            LOG.trace("Known caches: {}", KNOWN_CACHES.size());
         }
     }
 
@@ -165,6 +173,8 @@ public static void shutdown()
         PROVIDER.shutdown();
     }
 
+    private static final Set<Class<?>> CLASSES = new HashSet<>();
+
     // Validate a cached value. For now, just log warnings for mutable collections.
     public static <V> void validate(String debugName, @Nullable V value)
     {
@@ -175,10 +185,95 @@ public static <V> void validate(String debugName, @Nullable V value)
 
         if (null != description)
         {
-            LOG.warn(debugName + " attempted to cache " + description + ", which could be mutated by callers!");
+            LOG.warn("{} attempted to cache {}, which could be mutated by callers!", debugName, description);
+        }
+
+        // Log questionable members, but don't do the work if we're not going to log it
+        if (LOG.isDebugEnabled())
+        {
+            analyzeValue(value, debugName, null, 1);
         }
     }
 
+    private static final int MAX_DEPTH = 4;
+
+    private static void analyzeValue(@Nullable Object value, String cacheName, @Nullable String fields, int depth)
+    {
+        if (value != null && depth < MAX_DEPTH)
+        {
+            Class<?> clazz = value.getClass();
+
+            if (value instanceof Map<?, ?> m)
+            {
+                m.entrySet().stream()
+                    .findFirst()
+                    .ifPresent(entry -> {
+                        analyzeValue(entry.getKey(), cacheName, fields, depth + 1);
+                        analyzeValue(entry.getValue(), cacheName, fields, depth + 1);
+                    });
+            }
+            else if (value instanceof Collection<?> coll)
+            {
+                coll.stream()
+                    .findFirst()
+                    .ifPresent(element -> analyzeValue(element, cacheName, fields, depth + 1));
+            }
+            else if (clazz.isArray() && Array.getLength(value) > 0)
+            {
+                analyzeValue(Array.get(value, 0), cacheName, fields, depth + 1);
+            }
+            else if (value instanceof Reference<?> ref)
+            {
+                analyzeValue(ref, cacheName, fields, depth + 1);
+            }
+            else if (CLASSES.add(clazz))
+            {
+                for (Field field : getAllInstanceFields(clazz))
+                {
+                    String fieldPath = (fields != null ? fields + "." : "") + field.getName();
+                    Class<?> type = field.getType();
+
+                    if (Container.class.isAssignableFrom(type) || User.class.isAssignableFrom(type) || Project.class.isAssignableFrom(type))
+                    {
+                        LOG.debug("{}: {} field {} ({})", cacheName, clazz.getName(), fieldPath, field.getType().getName());
+                    }
+                    else
+                    {
+                        try
+                        {
+                            field.setAccessible(true);
+                            Object val = field.get(value);
+                            analyzeValue(val, cacheName, fieldPath, depth + 1);
+                        }
+                        catch (InaccessibleObjectException e)
+                        {
+                            LOG.debug("{}: {} field {} ({}): inaccessible member", cacheName, clazz.getName(), fieldPath, field.getType().getName());
+                        }
+                        catch (IllegalAccessException e)
+                        {
+                            LOG.debug("{}: {} field {} ({}): exception attempting to access", cacheName, clazz.getName(), fieldPath, field.getType().getName(), e);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    // Recurse up the class hierarchy
+    private static List<Field> getAllInstanceFields(Class<?> clazz)
+    {
+        if (clazz == null)
+        {
+            return Collections.emptyList();
+        }
+
+        List<Field> result = new ArrayList<>(getAllInstanceFields(clazz.getSuperclass()));
+        List<Field> filteredFields = Arrays.stream(clazz.getDeclaredFields())
+            .filter(field -> !Modifier.isStatic(field.getModifiers()))
+            .toList();
+        result.addAll(filteredFields);
+        return result;
+    }
 
     /* This interface allows a Collection to declare itself immutable when being added to a cache */
     public interface Sealable

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

@@ -200,7 +200,7 @@ public String getDomainURI()
     public Container getProject()
     {
         var c = getContainer();
-        return null==c ? null : c.getProject();
+        return null == c ? null : c.getProject();
     }
 
     public int getTitlePropertyId()

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

@@ -1455,7 +1455,6 @@ private static void copyDescriptors(final Container c, final Container project)
                     PROP_DESCRIPTOR_CACHE.remove(getCacheKey(pd));
                     DOMAIN_PROPERTIES_CACHE.clear();
                     pd.setContainer(project);
-                    pd.setProject(project);
                     pd.setPropertyId(0);
                     pd = ensurePropertyDescriptor(pd);
                 }
@@ -1640,7 +1639,6 @@ public static void moveContainer(@NotNull final Container c, @NotNull Container
                         if (!pd.getContainer().equals(c) || !pd.getProject().equals(fNewProject))
                         {
                             pd.setContainer(c);
-                            pd.setProject(fNewProject);
                             pd.setPropertyId(0);
                         }
 

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

@@ -15,7 +15,6 @@
  */
 package org.labkey.api.exp;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -42,9 +41,11 @@
 import org.labkey.api.query.PdLookupForeignKey;
 import org.labkey.api.query.UserSchema;
 import org.labkey.api.security.User;
+import org.labkey.api.util.GUID;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.util.StringExpression;
 import org.labkey.api.util.UnexpectedException;
+import org.labkey.api.util.logging.LogHelper;
 
 import java.io.Serializable;
 import java.util.Collection;
@@ -64,16 +65,15 @@ public class PropertyDescriptor extends ColumnRenderPropertiesImpl implements Pa
     private String _name;
     private String _storageColumnName;
     private int _propertyId;
-    private Container _container;
-    private Container _project;
+    private GUID _containerId;
     private String _lookupContainer;
     private String _lookupSchema;
     private String _lookupQuery;
     private boolean _mvEnabled;
     private String _mvIndicatorStorageColumnName;        // only valid if mvEnabled
     private Object _databaseDefaultValue;
 
-    private static final Logger LOG = LogManager.getLogger(PropertyDescriptor.class);
+    private static final Logger LOG = LogHelper.getLogger(PropertyDescriptor.class, "Property type warnings");
 
     @Override
     public void checkLocked()
@@ -289,31 +289,28 @@ public void setDefaultValueType(String defaultValueTypeName)
     @Override
     public String toString()
     {
-        return _propertyURI + " name=" + _name + " project="+  (_project == null ? "null" : _project.getPath()) + " container="+  (_container ==null ? "null" : _container.getPath()) + " label=" + _label + " range=" + _rangeURI + " concept=" + _conceptURI;
+        return _propertyURI + " name=" + _name + " project="+  (getProject() == null ? "null" : getProject().getPath()) + " container="+  (getContainer() == null ? "null" : getContainer().getPath()) + " label=" + _label + " range=" + _rangeURI + " concept=" + _conceptURI;
     }
 
     public Container getContainer()
     {
-        return _container;
+        return ContainerManager.getForId(_containerId);
     }
 
     public void setContainer(Container container)
     {
-        _container = container;
-        if (null== _project)
-            _project =container.getProject();
-        if (null== _project)
-            _project =container;
+        _containerId = container.getEntityId();
     }
 
     public Container getProject()
     {
-        return _project;
+        var c = getContainer();
+        return null == c ? null : c.getProject();
     }
 
     public void setProject(Container proj)
     {
-        _project = proj;
+        // No-op - project is determined by _containerId
     }
 
     @NotNull
@@ -437,8 +434,7 @@ public void copyTo(ColumnRenderPropertiesImpl to)
         if (to instanceof PropertyDescriptor)
         {
             PropertyDescriptor toPD = (PropertyDescriptor)to;
-            toPD._container = _container; // ?
-            toPD._project = _project; // ?
+            toPD._containerId = _containerId; // ?
             toPD._lookupContainer = _lookupContainer;
             toPD._lookupSchema = _lookupSchema;
             toPD._lookupQuery = _lookupQuery;

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

@@ -24,11 +24,8 @@
 import java.util.Map;
 
 /**
- * User: matthewb
- * Date: Sep 27, 2009
- *
- * This is a helper class for DetailsURL.  Rather than needing to subclass DetailsURL to provide a
- * container value, you may provide a ContainerContext instead.
+ * This is a helper class for DetailsURL. Rather than needing to subclass DetailsURL to provide a container value,
+ * you may provide a ContainerContext instead.
  */
 public interface ContainerContext
 {

assay/api-src/org/labkey/api/assay/dilution/DilutionAssayRun.java

@@ -145,7 +145,6 @@ private Map<FieldKey, PropertyDescriptor> getFieldKeys()
                 pd.setLabel(runColumn.getLabel());
                 pd.setPropertyURI(runColumn.getPropertyURI());
                 pd.setContainer(_protocol.getContainer());
-                pd.setProject(_protocol.getContainer().getProject());
                 fieldKeys.put(FieldKey.fromParts(runColumn.getName()), pd);
             }
         }

study/src/org/labkey/study/assay/StudyPublishManager.java

@@ -221,7 +221,6 @@ private List<PropertyDescriptor> createTargetPropertyDescriptors(Dataset dataset
 
             targetPd.setPropertyURI(dataset.getTypeURI() + "#" + sourcePd.getName());
             targetPd.setContainer(dataset.getContainer());
-            targetPd.setProject(dataset.getContainer().getProject());
             if (targetPd.getLookupQuery() != null)
                 targetPd.setLookupContainer(sourcePd.getLookupContainer());
             // set the ID to zero so it's clear that this is a new property descriptor:

study/src/org/labkey/study/model/StudyManager.java

@@ -3776,7 +3776,6 @@ private void buildPropertySaveAndDeleteLists(Map<String, DatasetDefinitionEntry>
                 {
                     p.getPropertyDescriptor().setPropertyId(0);
                     p.getPropertyDescriptor().setContainer(ipd.pd.getContainer());
-                    p.getPropertyDescriptor().setProject(ipd.pd.getProject());
                 }
             }
             else