diff --git a/api/src/org/labkey/api/cache/CacheManager.java b/api/src/org/labkey/api/cache/CacheManager.java index 4a0107dd0fb..53486329c38 100644 --- a/api/src/org/labkey/api/cache/CacheManager.java +++ b/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> CLASSES = new HashSet<>(); + // Validate a cached value. For now, just log warnings for mutable collections. public static void validate(String debugName, @Nullable V value) { @@ -175,10 +185,95 @@ public static 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 getAllInstanceFields(Class clazz) + { + if (clazz == null) + { + return Collections.emptyList(); + } + + List result = new ArrayList<>(getAllInstanceFields(clazz.getSuperclass())); + List 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 diff --git a/api/src/org/labkey/api/exp/DomainDescriptor.java b/api/src/org/labkey/api/exp/DomainDescriptor.java index d7f3aa5ac64..2a9e0b0aca6 100644 --- a/api/src/org/labkey/api/exp/DomainDescriptor.java +++ b/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() diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index 9bb15813220..152597bb6b2 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/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); } diff --git a/api/src/org/labkey/api/exp/PropertyDescriptor.java b/api/src/org/labkey/api/exp/PropertyDescriptor.java index 6db6245f25b..20d55c932a9 100644 --- a/api/src/org/labkey/api/exp/PropertyDescriptor.java +++ b/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,8 +65,7 @@ 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; @@ -73,7 +73,7 @@ public class PropertyDescriptor extends ColumnRenderPropertiesImpl implements Pa 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; diff --git a/api/src/org/labkey/api/util/ContainerContext.java b/api/src/org/labkey/api/util/ContainerContext.java index d500779f359..b80525eb1c7 100644 --- a/api/src/org/labkey/api/util/ContainerContext.java +++ b/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 { diff --git a/assay/api-src/org/labkey/api/assay/dilution/DilutionAssayRun.java b/assay/api-src/org/labkey/api/assay/dilution/DilutionAssayRun.java index 6e33258c921..33267a27ce1 100644 --- a/assay/api-src/org/labkey/api/assay/dilution/DilutionAssayRun.java +++ b/assay/api-src/org/labkey/api/assay/dilution/DilutionAssayRun.java @@ -145,7 +145,6 @@ private Map 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); } } diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 41818beb073..694d2e80881 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -221,7 +221,6 @@ private List 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: diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 6f7e1094c5b..54aea179175 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -3776,7 +3776,6 @@ private void buildPropertySaveAndDeleteLists(Map { p.getPropertyDescriptor().setPropertyId(0); p.getPropertyDescriptor().setContainer(ipd.pd.getContainer()); - p.getPropertyDescriptor().setProject(ipd.pd.getProject()); } } else