diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index fee93e64036..783a71692a0 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -39,6 +39,7 @@ import org.labkey.api.cache.BlockingCache; import org.labkey.api.cache.ContainerKeySimpleCache; import org.labkey.api.cache.SimpleKeyMappingCache; +import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.ArrayListMap; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; @@ -179,6 +180,7 @@ import org.labkey.api.util.JobRunner; import org.labkey.api.util.JsonUtil; import org.labkey.api.util.JspTestCase; +import org.labkey.api.util.KeySharingJSONTokener; import org.labkey.api.util.MailHelper; import org.labkey.api.util.MemTracker; import org.labkey.api.util.MimeMap; @@ -427,6 +429,7 @@ public void registerServlets(ServletContext servletCtx) BaseServerProperties.TestCase.class, BooleanFormat.TestCase.class, BuilderObjectFactory.TestCase.class, + CacheManager.TestCase.class, CachingDataIterator.ScrollTestCase.class, CaseInsensitiveHashMap.TestCase.class, CaseInsensitiveHashSet.TestCase.class, @@ -470,6 +473,7 @@ public void registerServlets(ServletContext servletCtx) JobRunner.TestCase.class, JsonTest.class, JsonUtil.TestCase.class, + KeySharingJSONTokener.TestCase.class, LimitedUser.TestCase.class, MarkableIterator.TestCase.class, MaterializedQueryHelper.TestCase.class, diff --git a/api/src/org/labkey/api/action/BaseApiAction.java b/api/src/org/labkey/api/action/BaseApiAction.java index 525de642ab4..8f8279f2cee 100644 --- a/api/src/org/labkey/api/action/BaseApiAction.java +++ b/api/src/org/labkey/api/action/BaseApiAction.java @@ -38,6 +38,7 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.HttpUtil; import org.labkey.api.util.JsonUtil; +import org.labkey.api.util.KeySharingJSONTokener; import org.labkey.api.util.MimeMap; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.view.BadRequestException; @@ -500,7 +501,7 @@ protected long getMaximumJsonInputLength() try (Reader reader = openRequestReader()) { - JSONTokener tokener = new JSONTokener(reader); + JSONTokener tokener = new KeySharingJSONTokener(reader); return tokener.more() ? new JSONObject(tokener) : null; } } diff --git a/api/src/org/labkey/api/cache/CacheManager.java b/api/src/org/labkey/api/cache/CacheManager.java index e07b87b9901..dab05e9360b 100644 --- a/api/src/org/labkey/api/cache/CacheManager.java +++ b/api/src/org/labkey/api/cache/CacheManager.java @@ -19,6 +19,8 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.cache.ehcache.EhCacheProvider; import org.labkey.api.collections.CollectionUtils; import org.labkey.api.data.Container; @@ -309,4 +311,23 @@ public interface Sealable { boolean isSealed(); } + + public static class TestCase extends Assert + { + @Test + public void testContainerKeyRejected() + { + try (Cache cache = getTemporaryCache(10, MINUTE, "Container key test", null)) + { + Container c = new Container(null, "", GUID.makeGUID(), 1, 1, null, 0, false); + assertThrows(IllegalArgumentException.class, () -> cache.put(c, "value")); + assertThrows(IllegalArgumentException.class, () -> cache.put(c, "value", MINUTE)); + assertThrows(IllegalArgumentException.class, () -> cache.get(c, null, (key, argument) -> "loaded")); + assertTrue(cache.getKeys().isEmpty()); + + cache.put(c.getEntityId(), "value"); + assertEquals("value", cache.get(c.getEntityId())); + } + } + } } diff --git a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java index 74b2aae35c2..27034f57eea 100644 --- a/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java +++ b/api/src/org/labkey/api/cache/ContainerKeySimpleCache.java @@ -16,7 +16,8 @@ public class ContainerKeySimpleCache extends SimpleKeyMappingCache delegate) { - super(delegate, Container::getEntityId, ContainerManager::getForId); + // Some caches use a null key for cross-container data (e.g., ExternalSchemaDefCache) + super(delegate, c -> null == c ? null : c.getEntityId(), ContainerManager::getForId); } public static class TestCase extends Assert diff --git a/api/src/org/labkey/api/cache/ehcache/EhCacheProvider.java b/api/src/org/labkey/api/cache/ehcache/EhCacheProvider.java index 78d76a92007..39fa62a7102 100644 --- a/api/src/org/labkey/api/cache/ehcache/EhCacheProvider.java +++ b/api/src/org/labkey/api/cache/ehcache/EhCacheProvider.java @@ -97,7 +97,7 @@ public SimpleCache getSimpleCache(String debugName, int limit, long if (temporary) MemTracker.getInstance().put(ehCache); - return new EhSimpleCache<>(ehCache); + return new EhSimpleCache<>(ehCache, debugName); } void closeCache(Cache cache) diff --git a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java index 155dae99e60..f1ffceede74 100644 --- a/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java +++ b/api/src/org/labkey/api/cache/ehcache/EhSimpleCache.java @@ -23,6 +23,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.CacheType; import org.labkey.api.cache.SimpleCache; +import org.labkey.api.data.Container; import org.labkey.api.util.IntegerUtils; import java.util.List; @@ -33,15 +34,25 @@ class EhSimpleCache implements SimpleCache private static final Logger LOG = LogManager.getLogger(EhSimpleCache.class); private final Cache _cache; + private final String _debugName; - EhSimpleCache(Cache cache) + EhSimpleCache(Cache cache, String debugName) { _cache = cache; + _debugName = debugName; + } + + private void validateKey(K key) + { + // Ehcache keeps the first key object stored for an entry, so a Container key pins a stale copy + if (key instanceof Container) + throw new IllegalArgumentException("Container used as a key in cache \"" + _debugName + "\". Pass Container.class (keyClass) to the Cache factory method to allow this."); } @Override public void put(@NotNull K key, V value) { + validateKey(key); Element element = new Element(key, value); _cache.put(element); } @@ -49,6 +60,7 @@ public void put(@NotNull K key, V value) @Override public void put(@NotNull K key, V value, long timeToLive) { + validateKey(key); Element element = new Element(key, value); element.setTimeToLive(IntegerUtils.asInteger(timeToLive / 1000)); // Convert from ms to sec _cache.put(element); diff --git a/api/src/org/labkey/api/data/Container.java b/api/src/org/labkey/api/data/Container.java index bb9839dd30c..48048b95ff1 100644 --- a/api/src/org/labkey/api/data/Container.java +++ b/api/src/org/labkey/api/data/Container.java @@ -141,25 +141,20 @@ public class Container implements Serializable, Comparable, Securable private Long _fileRootSize = null; private LocalDateTime _fileRootLastCrawled = null; - private final static BlockingCache> REQUIRED_MODULES_CACHE = new BlockingCache<>( - CacheManager.getCache( - Constants.getMaxContainers(), - CacheManager.DAY, - "Required modules per container"), - (key, argument) -> { - if (!(argument instanceof Container c)) - { - throw new IllegalStateException("Expected usage pattern is to include the container instance as the argument. Key: " + key); - } + private final static BlockingCache> REQUIRED_MODULES_CACHE = DatabaseCache.get( + Container.class, + CoreSchema.getInstance().getScope(), + Constants.getMaxContainers(), + CacheManager.DAY, + "Required modules per container", + (c, _) -> { Set requiredModules = new HashSet<>(c.getRequiredModulesForFolderType(c.getFolderType())); requiredModules.add(ModuleLoader.getInstance().getModule("API")); - for (Container child: c.getChildren()) + if (c.canHaveChildren()) { - if (child.isWorkbook()) - { - requiredModules.addAll(c.getRequiredModulesForFolderType(child.getFolderType())); - } + for (FolderType workbookFolderType : ContainerManager.getWorkbookChildFolderTypes(c)) + requiredModules.addAll(c.getRequiredModulesForFolderType(workbookFolderType)); } return Collections.unmodifiableSet(requiredModules); @@ -167,19 +162,20 @@ public class Container implements Serializable, Comparable, Securable static { - // Clear the required modules cache on any change to the container tree or container properties + // An entry depends on its container's folder type, its workbook children's, and (via module path patterns) its + // path. Uncache just the container and its parent, except renames and moves, which change descendants' paths. ContainerManager.addContainerListener(new ContainerManager.ContainerListener() { @Override public void containerCreated(Container c, User user) { - REQUIRED_MODULES_CACHE.clear(); + uncacheRequiredModules(c, c.getParent()); } @Override public void containerDeleted(Container c, User user) { - REQUIRED_MODULES_CACHE.clear(); + uncacheRequiredModules(c, c.getParent()); } @Override @@ -191,11 +187,24 @@ public void containerMoved(Container c, Container oldParent, User user) @Override public void propertyChange(PropertyChangeEvent evt) { - REQUIRED_MODULES_CACHE.clear(); + if (evt instanceof ContainerManager.ContainerPropertyChangeEvent cpce && null != cpce.container + && cpce.property != ContainerManager.Property.Name && cpce.property != ContainerManager.Property.Parent) + uncacheRequiredModules(cpce.container, cpce.container.getParent()); + else + REQUIRED_MODULES_CACHE.clear(); } }); } + private static void uncacheRequiredModules(Container... containers) + { + for (Container c : containers) + { + if (null != c) + REQUIRED_MODULES_CACHE.remove(c); + } + } + // Might add others in the future (e.g., ReadOnly) public enum LockState { @@ -1007,7 +1016,7 @@ public void setFolderType(FolderType folderType, User user, BindException errors public Set getRequiredModules() { - return REQUIRED_MODULES_CACHE.get(getEntityId(), this); + return REQUIRED_MODULES_CACHE.get(this); } public Set getRequiredModulesForFolderType(FolderType folderType) diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index f6e3b553fe4..3441e3c9b5a 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -648,22 +648,44 @@ public static String getFolderTypeName(Container c) } + /** Left-joins each container (alias "c") to its stored folder type name, exposed as pe.Value; null when none is stored */ + private static SQLFragment getFolderTypeNameJoin() + { + return new SQLFragment(" LEFT OUTER JOIN ").append(PropertySchema.getInstance().getTableInfoPropertyEntries(), "pe") + .append(" ON pe.ObjectId = c.EntityId AND pe.Category = ? AND pe.Name = ? AND pe.UserId = ?") + .add(FOLDER_TYPE_PROPERTY_SET_NAME).add(FOLDER_TYPE_PROPERTY_NAME).add(PropertyManager.SHARED_USER.getUserId()); + } + + /** Same resolution as getFolderType(), minus its bad-name logging */ + private static @NotNull FolderType resolveFolderType(@Nullable String name) + { + FolderType folderType = null == name ? null : FolderTypeManager.get().getFolderType(name); + return null == folderType ? FolderType.NONE : folderType; + } + @NotNull - public static Map getFolderTypeNameContainerCounts(Container root) + public static Map getFolderTypeNameContainerCounts() { + SQLFragment sql = new SQLFragment("SELECT pe.Value, COUNT(*) FROM ").append(CORE.getTableInfoContainers(), "c") + .append(getFolderTypeNameJoin()) + .append(" GROUP BY pe.Value"); Map nameCounts = new TreeMap<>(); - for (Container c : getAllChildren(root)) - { - Integer count = nameCounts.get(c.getFolderType().getName()); - if (null == count) - { - count = Integer.valueOf(0); - } - nameCounts.put(c.getFolderType().getName(), ++count); - } + new SqlSelector(CORE.getSchema(), sql).forEach(rs -> nameCounts.merge(resolveFolderType(rs.getString(1)).getName(), rs.getInt(2), Integer::sum)); return nameCounts; } + /** Distinct folder types of the parent's workbook children, without loading each workbook's property map */ + @NotNull + public static Set getWorkbookChildFolderTypes(Container parent) + { + SQLFragment sql = new SQLFragment("SELECT DISTINCT pe.Value FROM ").append(CORE.getTableInfoContainers(), "c") + .append(getFolderTypeNameJoin()) + .append(" WHERE c.Parent = ? AND c.Type = ?").add(parent.getEntityId()).add(WorkbookContainerType.NAME); + return new SqlSelector(CORE.getSchema(), sql).stream(String.class) + .map(ContainerManager::resolveFolderType) + .collect(Collectors.toSet()); + } + @NotNull public static Map getProductFoldersMetrics(@NotNull FolderType folderType) { @@ -994,7 +1016,7 @@ public static void updateTitle(Container container, String title, User user) public static void uncache(Container c) { - _removeFromCache(c, true); + _removeFromCache(c, false); } public static final String SHARED_CONTAINER_PATH = "/Shared"; @@ -1122,9 +1144,11 @@ private static Map getChildrenMap(Container parent) new SqlSelector(CORE.getSchema(), "SELECT * FROM " + CORE.getTableInfoContainers() + " WHERE Parent = ? ORDER BY SortOrder, LOWER(Name)", parent.getId()).forEach(rs -> { - Container c = factory.handle(rs); - ids.add(c.getEntityId()); - _addToCache(c); + GUID id = new GUID(rs.getString("EntityId")); + // Reuse the cached instance; a fresh copy would replace it while Container-keyed caches keep pinning the old one + Container c = CACHE_ENTITY_ID.get(id); + _addToCache(null != c ? c : factory.handle(rs)); + ids.add(id); }); childIds = Collections.unmodifiableList(ids); @@ -2150,9 +2174,11 @@ private static void _removeFromCache(Container c, boolean hierarchyChange) if (hierarchyChange) { - // This is strictly keeping track of the parent/child relationships themselves so it only needs to be - // cleared when the tree changes - CACHE_CHILDREN.clear(); + // Creating or deleting c only changes its own and its parent's child lists; moves and renames use clearCache() + CACHE_CHILDREN.remove(c.getEntityId()); + Container parent = c.getParent(); + if (null != parent) + CACHE_CHILDREN.remove(parent.getEntityId()); } navTreeManageUncache(c); @@ -3069,6 +3095,32 @@ private static void createContainers(MultiValuedMap mm, String n } } + @Test + public void testFolderTypeQueries() + { + User user = TestContext.get().getUser(); + Container parent = createContainer(_testRoot, "folderTypes", user); + Container untyped = createContainer(parent, null, null, null, WorkbookContainerType.NAME, user); + Container typed = createContainer(parent, null, null, null, WorkbookContainerType.NAME, user); + FolderType collaboration = FolderTypeManager.get().getFolderType("Collaboration"); + assertNotNull(collaboration); + + Set collaborationModules = parent.getRequiredModulesForFolderType(collaboration); + assertFalse(collaborationModules.isEmpty()); + assertFalse(parent.getRequiredModules().containsAll(collaborationModules)); + + typed.setFolderType(collaboration, user); + + // A workbook's folder type change must uncache its parent's entry + assertTrue(parent.getRequiredModules().containsAll(collaborationModules)); + assertEquals(Set.of(FolderType.NONE, collaboration), getWorkbookChildFolderTypes(parent)); + assertEquals(FolderType.NONE, untyped.getFolderType()); + + Map expected = new TreeMap<>(); + getAllChildren(getRoot()).forEach(c -> expected.merge(c.getFolderType().getName(), 1, Integer::sum)); + assertEquals(expected, getFolderTypeNameContainerCounts()); + } + private static void cleanUpChildren(MultiValuedMap mm, String name, Container parent) { Collection nodes = mm.get(name); diff --git a/api/src/org/labkey/api/data/MvUtil.java b/api/src/org/labkey/api/data/MvUtil.java index b281a94c75d..ac1817340d1 100644 --- a/api/src/org/labkey/api/data/MvUtil.java +++ b/api/src/org/labkey/api/data/MvUtil.java @@ -35,7 +35,7 @@ */ public class MvUtil { - private static final Cache> CACHE = CacheManager.getBlockingCache(Constants.getMaxContainers(), CacheManager.YEAR, "Missing value indicators", (c, argument) -> getFromDb(c)); + private static final Cache> CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxContainers(), CacheManager.YEAR, "Missing value indicators", (c, argument) -> getFromDb(c)); private MvUtil() {} diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index d5de44e5afe..cec592f6b9e 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -137,7 +137,7 @@ public class OntologyManager { private static final Logger _log = LogManager.getLogger(OntologyManager.class); - private static final Cache, Map> PROPERTY_MAP_CACHE = DatabaseCache.get(getExpSchema().getScope(), 100000, "Property maps", new PropertyMapCacheLoader()); + private static final Cache, Map> PROPERTY_MAP_CACHE = DatabaseCache.get(getExpSchema().getScope(), 100000, "Property maps", new PropertyMapCacheLoader()); private static final BlockingCache OBJECT_ID_CACHE = DatabaseCache.get(getExpSchema().getScope(), 2000, "ObjectIds", new ObjectIdCacheLoader()); private static final Cache, PropertyDescriptor> PROP_DESCRIPTOR_CACHE = DatabaseCache.get(getExpSchema().getScope(), 40000, "Property descriptors", new CacheLoader<>() { @@ -247,7 +247,7 @@ public List> load(@NotNull Pair key, @Nullab .toList(); } }); - private static final Cache> DOMAIN_DESCRIPTORS_BY_CONTAINER_CACHE = DatabaseCache.get(getExpSchema().getScope(), 2000, "Domain descriptors by container", (c, argument) -> { + private static final Cache> DOMAIN_DESCRIPTORS_BY_CONTAINER_CACHE = DatabaseCache.get(Container.class, getExpSchema().getScope(), 2000, "Domain descriptors by container", (c, argument) -> { String sql = "SELECT * FROM " + getTinfoDomainDescriptor() + " WHERE Container = ?"; Map dds = new LinkedHashMap<>(); @@ -853,9 +853,9 @@ public interface UpdateableTableImportHelper extends ImportHelper } @NotNull - private static Pair getPropertyMapCacheKey(@Nullable Container container, @NotNull String objectLSID) + private static Pair getPropertyMapCacheKey(@Nullable Container container, @NotNull String objectLSID) { - return Pair.of(container, objectLSID); + return Pair.of(null == container ? null : container.getEntityId(), objectLSID); } /** @@ -866,22 +866,22 @@ private static Pair getPropertyMapCacheKey(@Nullable Containe */ public static Map getPropertyObjects(@Nullable Container container, @NotNull String objectLSID) { - Pair cacheKey = getPropertyMapCacheKey(container, objectLSID); + Pair cacheKey = getPropertyMapCacheKey(container, objectLSID); return PROPERTY_MAP_CACHE.get(cacheKey); } - public static class PropertyMapCacheLoader implements CacheLoader, Map> + public static class PropertyMapCacheLoader implements CacheLoader, Map> { @Override - public Map load(@NotNull Pair key, @Nullable Object argument) + public Map load(@NotNull Pair key, @Nullable Object argument) { - Container container = key.first; + GUID containerId = key.first; String objectLSID = key.second; SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ObjectURI"), objectLSID); - if (container != null) + if (containerId != null) { - filter.addCondition(FieldKey.fromParts("Container"), container); + filter.addCondition(FieldKey.fromParts("Container"), containerId.toString()); } if (_log.isDebugEnabled()) @@ -2946,7 +2946,7 @@ public static void invalidateDomain(Domain d) DOMAIN_DESCRIPTORS_BY_URI_CACHE.removeUsingFilter(key -> domainURI.equals(key.first)); DOMAIN_DESC_BY_ID_CACHE.remove(d.getTypeId()); DOMAIN_PROPERTIES_CACHE.removeUsingFilter(key -> domainURI.equals(key.first)); - PROP_DESCRIPTOR_CACHE.removeUsingFilter(key -> propertyURIs.contains(key.first)); + uncachePropertyDescriptors(propertyURIs); DOMAIN_DESCRIPTORS_BY_CONTAINER_CACHE.remove(d.getContainer()); // Cached property values embed property metadata (name, type) from any domain, so these can't be narrowed @@ -2954,6 +2954,16 @@ public static void invalidateDomain(Domain d) ExperimentService.get().clearCaches(); } + /** For callers that update exp.PropertyDescriptor rows directly; invalidateDomain() only covers the saved domain's own properties */ + public static void uncachePropertyDescriptors(Collection propertyURIs) + { + if (!propertyURIs.isEmpty()) + { + Set uris = propertyURIs instanceof Set set ? set : new HashSet<>(propertyURIs); + PROP_DESCRIPTOR_CACHE.removeUsingFilter(key -> uris.contains(key.first)); + } + } + public static void clearCaches() { @@ -2973,7 +2983,6 @@ public static void clearPropertyCache(String parentObjectURI) PROPERTY_MAP_CACHE.removeUsingFilter(key -> Objects.equals(key.second, parentObjectURI)); } - /** * Removes deleted objects' property maps. Pass deletedUnknownUris when objects outside deletedUris were also * deleted (owned children, or deletes by object id). @@ -2994,12 +3003,12 @@ private static void clearPropertyCache(Container c, Collection deletedUr } /** A record rather than a lambda so a transaction's equal post-commit removal tasks dedupe */ - private record ContainerPropertyMapKeys(Container c) implements Predicate> + private record ContainerPropertyMapKeys(Container c) implements Predicate> { @Override - public boolean test(Pair key) + public boolean test(Pair key) { - return key.first == null || c.equals(key.first); + return key.first == null || key.first.equals(c.getEntityId()); } } diff --git a/api/src/org/labkey/api/qc/DataStateManager.java b/api/src/org/labkey/api/qc/DataStateManager.java index 17556099fbe..e066be53188 100644 --- a/api/src/org/labkey/api/qc/DataStateManager.java +++ b/api/src/org/labkey/api/qc/DataStateManager.java @@ -42,7 +42,7 @@ public class DataStateManager private static final Logger LOG = LogHelper.getLogger(DataStateManager.class, "Data state persistence issues"); private static final DataStateManager _instance = new DataStateManager(); private static final Map> _DataStateHandlers = new HashMap<>(); - private static final Cache DATA_STATE_DB_CACHE = CacheManager.getBlockingCache(CacheManager.UNLIMITED, CacheManager.DAY, "Data states", + private static final Cache DATA_STATE_DB_CACHE = CacheManager.getBlockingCache(Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Data states", (c, argument) -> new DataStateCollections(c) ); diff --git a/api/src/org/labkey/api/security/ProjectAndSiteGroupsCache.java b/api/src/org/labkey/api/security/ProjectAndSiteGroupsCache.java index 331630cf330..0b9139db804 100644 --- a/api/src/org/labkey/api/security/ProjectAndSiteGroupsCache.java +++ b/api/src/org/labkey/api/security/ProjectAndSiteGroupsCache.java @@ -35,7 +35,7 @@ public class ProjectAndSiteGroupsCache { private static final CoreSchema CORE = CoreSchema.getInstance(); - private static final BlockingCache> CACHE = CacheManager.getBlockingCache(Constants.getMaxProjects(), CacheManager.DAY, "Project Groups", null); + private static final BlockingCache> CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxProjects(), CacheManager.DAY, "Project Groups", null); private static final CacheLoader> GROUP_LIST_LOADER = (c, argument) -> { String containerClause = c.isRoot() ? "IS NULL" : "= ?"; diff --git a/api/src/org/labkey/api/settings/LookAndFeelProperties.java b/api/src/org/labkey/api/settings/LookAndFeelProperties.java index 131e55f87ed..ca9b3a45821 100644 --- a/api/src/org/labkey/api/settings/LookAndFeelProperties.java +++ b/api/src/org/labkey/api/settings/LookAndFeelProperties.java @@ -51,7 +51,7 @@ */ public class LookAndFeelProperties extends LookAndFeelFolderProperties { - private static final Cache SHORT_NAME_CACHE = CacheManager.getBlockingCache(Constants.getMaxProjects(), CacheManager.YEAR, "Short name", null); + private static final Cache SHORT_NAME_CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxProjects(), CacheManager.YEAR, "Short name", null); private static final Logger LOG = LogHelper.getLogger(LookAndFeelProperties.class, "Manages site-wide and project-scoped look and feel settings"); public static void clearCaches() diff --git a/api/src/org/labkey/api/util/KeySharingJSONTokener.java b/api/src/org/labkey/api/util/KeySharingJSONTokener.java new file mode 100644 index 00000000000..b73e028bcdc --- /dev/null +++ b/api/src/org/labkey/api/util/KeySharingJSONTokener.java @@ -0,0 +1,166 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.util; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONTokener; +import org.junit.Assert; +import org.junit.Test; + +import java.io.Reader; +import java.math.BigDecimal; +import java.util.HashMap; +import java.util.Map; + +/** + * JSONTokener that returns one shared String instance per distinct object key, so a bulk payload's rows don't each + * hold their own copy of every column name. + */ +public class KeySharingJSONTokener extends JSONTokener +{ + // Bounds the map when a request sends many unique keys; keys past the cap are not shared + static final int MAX_SHARED_KEYS = 1000; + + private final Map _keys = new HashMap<>(); + + public KeySharingJSONTokener(Reader reader) + { + super(reader); + } + + public KeySharingJSONTokener(String source) + { + super(source); + } + + // org.json reads keys via nextString() but values via nextValue(), so read quoted values here to keep them out of the key map + @Override + public Object nextValue() throws JSONException + { + char c = nextClean(); + if (c == '"' || (c == '\'' && !getJsonParserConfiguration().isStrictMode())) + return super.nextString(c); + if (c != 0) + back(); + return super.nextValue(); + } + + @Override + public String nextString(char quote) throws JSONException + { + String key = super.nextString(quote); + String shared = _keys.get(key); + if (shared != null) + return shared; + if (_keys.size() < MAX_SHARED_KEYS) + _keys.put(key, key); + return key; + } + + public static class TestCase extends Assert + { + private static final String ROWS = """ + { + "schemaName": "lists", + "rows": [ + {"name": "a", "count": 1, "big": 12345678901, "ratio": 1.5, "flag": true, "missing": null, "nested": {"name": "inner", "list": [1, "name", null]}}, + {"name": "name", "count": 2, "big": 12345678902, "ratio": 2.5, "flag": false, "missing": null, "nested": {"name": "inner2", "list": []}}, + {'name': 'single', "count": -3} + ] + } + """; + + @Test + public void testKeysShared() + { + JSONArray rows = new JSONObject(new KeySharingJSONTokener(ROWS)).getJSONArray("rows"); + String nameKey = key(rows.getJSONObject(0), "name"); + for (int i = 1; i < rows.length(); i++) + assertSame(nameKey, key(rows.getJSONObject(i), "name")); + assertSame(nameKey, key(rows.getJSONObject(0).getJSONObject("nested"), "name")); + assertSame(key(rows.getJSONObject(0), "count"), key(rows.getJSONObject(1), "count")); + assertSame(key(rows.getJSONObject(0).getJSONObject("nested"), "list"), key(rows.getJSONObject(1).getJSONObject("nested"), "list")); + + // String values equal to a key must not come from the key map + assertNotSame(nameKey, rows.getJSONObject(1).get("name")); + assertNotSame(nameKey, rows.getJSONObject(0).getJSONObject("nested").getJSONArray("list").get(1)); + } + + @Test + public void testParseUnchanged() + { + JSONObject expected = new JSONObject(new JSONTokener(ROWS)); + JSONObject actual = new JSONObject(new KeySharingJSONTokener(ROWS)); + assertTrue(expected.similar(actual)); + assertEquals(expected.toString(), actual.toString()); + + JSONObject row = actual.getJSONArray("rows").getJSONObject(0); + assertEquals(Integer.class, row.get("count").getClass()); + assertEquals(Long.class, row.get("big").getClass()); + assertEquals(BigDecimal.class, row.get("ratio").getClass()); + assertEquals(Boolean.TRUE, row.get("flag")); + assertTrue(row.has("missing")); + assertSame(JSONObject.NULL, row.get("missing")); + assertSame(JSONObject.NULL, row.getJSONObject("nested").getJSONArray("list").get(2)); + assertEquals("single", actual.getJSONArray("rows").getJSONObject(2).get("name")); + } + + @Test + public void testErrorsUnchanged() + { + for (String bad : new String[]{"{\"a\":", "{\"a\": }", "{\"a\": \"unterminated", "{\"a\": 1,, }", "{\"a\": [1, }"}) + assertEquals(bad, parseError(new JSONTokener(bad)), parseError(new KeySharingJSONTokener(bad))); + } + + @Test + public void testCap() + { + StringBuilder sb = new StringBuilder("["); + for (int row = 0; row < 2; row++) + { + sb.append(row == 0 ? "{" : ",{"); + for (int i = 0; i < MAX_SHARED_KEYS + 10; i++) + sb.append(i == 0 ? "" : ",").append("\"k").append(i).append("\":").append(i); + sb.append("}"); + } + JSONArray rows = new JSONArray(new KeySharingJSONTokener(sb.append("]").toString())); + assertSame(key(rows.getJSONObject(0), "k0"), key(rows.getJSONObject(1), "k0")); + String lastKey = "k" + (MAX_SHARED_KEYS + 9); + assertNotSame(key(rows.getJSONObject(0), lastKey), key(rows.getJSONObject(1), lastKey)); + assertEquals(MAX_SHARED_KEYS + 9, rows.getJSONObject(1).getInt(lastKey)); + } + + private static String key(JSONObject obj, String name) + { + return obj.keySet().stream().filter(name::equals).findFirst().orElseThrow(); + } + + private static String parseError(JSONTokener tokener) + { + try + { + new JSONObject(tokener); + return null; + } + catch (JSONException e) + { + return e.getMessage(); + } + } + } +} diff --git a/api/src/org/labkey/api/util/UsageReportingLevel.java b/api/src/org/labkey/api/util/UsageReportingLevel.java index da7189749cf..1132a14c614 100644 --- a/api/src/org/labkey/api/util/UsageReportingLevel.java +++ b/api/src/org/labkey/api/util/UsageReportingLevel.java @@ -121,7 +121,7 @@ protected void addExtraParams(MothershipReport report, Map metri putModulesMetrics(modulesMap); putModulesBuildInfo(modulesMap); - metrics.put("folderTypeCounts", ContainerManager.getFolderTypeNameContainerCounts(ContainerManager.getRoot())); + metrics.put("folderTypeCounts", ContainerManager.getFolderTypeNameContainerCounts()); metrics.put("auditCommentsRequiredContainerCount", ContainerManager.getAuditCommentRequiredCount()); report.addHostName(); diff --git a/api/src/org/labkey/api/webdav/WebdavResolverImpl.java b/api/src/org/labkey/api/webdav/WebdavResolverImpl.java index c21f8beadf9..bc5420043e1 100644 --- a/api/src/org/labkey/api/webdav/WebdavResolverImpl.java +++ b/api/src/org/labkey/api/webdav/WebdavResolverImpl.java @@ -134,8 +134,9 @@ private class CacheArgument } // Cache with short-lived entries to make webdav perform reasonably. WebdavResolverImpl is a singleton, so we - // end up with just one of these. - private final Cache _folderCache = CacheManager.getBlockingCache(CacheManager.UNLIMITED, 5 * CacheManager.MINUTE, "WebDAV folders", null); + // end up with just one of these. Bounded because Ehcache drops an expired entry only when it's read again, so + // crawling every container would otherwise keep one folder per container. + private final Cache _folderCache = CacheManager.getBlockingCache(CacheManager.DEFAULT_CACHE_SIZE, 5 * CacheManager.MINUTE, "WebDAV folders", null); public class WebFolderResource extends AbstractWebFolderResource { diff --git a/assay/src/org/labkey/assay/AssayManager.java b/assay/src/org/labkey/assay/AssayManager.java index 9d3eb6ba5a2..ad923586544 100644 --- a/assay/src/org/labkey/assay/AssayManager.java +++ b/assay/src/org/labkey/assay/AssayManager.java @@ -142,7 +142,7 @@ public Class getRequiredPermission() }; /** Cache the protocols defined in a given container, which we can quickly compose to get the protocols in scope */ - private static final Cache> PROTOCOL_CACHE = DatabaseCache.get(ExperimentService.get().getSchema().getScope(), CacheManager.UNLIMITED, TimeUnit.HOURS.toMillis(1), "Assay protocols", (c, _) -> + private static final Cache> PROTOCOL_CACHE = DatabaseCache.get(Container.class, ExperimentService.get().getSchema().getScope(), CacheManager.UNLIMITED, TimeUnit.HOURS.toMillis(1), "Assay protocols", (c, _) -> { List result = new ArrayList<>(); diff --git a/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java b/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java index 586e48456ca..8a54362f80e 100644 --- a/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java +++ b/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java @@ -21,6 +21,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.OntologyManager; import org.labkey.api.query.QueryChangeListener; @@ -40,37 +41,42 @@ public void queryCreated(User user, Container container, ContainerFilter scope, private int updateLookupQuery(String newValue, SchemaKey schema, String oldQuery, Container container) { - String fieldName = "lookupquery"; - TableInfo pdTable = OntologyManager.getTinfoPropertyDescriptor(); - SQLFragment updateSql = new SQLFragment("UPDATE ").append(pdTable) - .append(" SET ") - .append(fieldName) - .append(" = ? WHERE lookupschema = ? AND lookupquery = ? AND ") + SQLFragment where = new SQLFragment("lookupschema = ? AND lookupquery = ? AND ") .append("(lookupcontainer = ? OR (lookupcontainer IS NULL AND container = ?))") - .add(newValue) .add(schema.toString()) .add(oldQuery) .add(container) .add(container); - return new SqlExecutor(pdTable.getSchema()).execute(updateSql); + return updateLookups("lookupquery", newValue, where); } private int updateLookupSchema(String newValue, String oldSchema, Container container) { - String fieldName = "lookupschema"; - TableInfo pdTable = OntologyManager.getTinfoPropertyDescriptor(); - SQLFragment updateSql = new SQLFragment("UPDATE ").append(pdTable) - .append(" SET ") - .append(fieldName) - .append(" = ? WHERE lookupschema = ? AND ") + SQLFragment where = new SQLFragment("lookupschema = ? AND ") .append("(lookupcontainer = ? OR (lookupcontainer IS NULL AND container = ?))") - .add(newValue) .add(oldSchema) .add(container) .add(container); - return new SqlExecutor(pdTable.getSchema()).execute(updateSql); + return updateLookups("lookupschema", newValue, where); + } + + private int updateLookups(String fieldName, String newValue, SQLFragment where) + { + TableInfo pdTable = OntologyManager.getTinfoPropertyDescriptor(); + List propertyURIs = new SqlSelector(pdTable.getSchema(), new SQLFragment("SELECT PropertyURI FROM ").append(pdTable).append(" WHERE ").append(where)).getArrayList(String.class); + if (propertyURIs.isEmpty()) + return 0; + + SQLFragment updateSql = new SQLFragment("UPDATE ").append(pdTable) + .append(" SET ").append(fieldName).append(" = ? WHERE ") + .add(newValue) + .append(where); + + int updated = new SqlExecutor(pdTable.getSchema()).execute(updateSql); + OntologyManager.uncachePropertyDescriptors(propertyURIs); + return updated; } @Override diff --git a/experiment/src/org/labkey/experiment/api/DataColorManager.java b/experiment/src/org/labkey/experiment/api/DataColorManager.java index dcc10500385..1371b116451 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorManager.java +++ b/experiment/src/org/labkey/experiment/api/DataColorManager.java @@ -30,7 +30,7 @@ public class DataColorManager private static final DataColorManager _instance = new DataColorManager(); private static final Cache CACHE = CacheManager.getBlockingCache( - CacheManager.UNLIMITED, CacheManager.DAY, "Data colors", (c, argument) -> new DataColorCollections(c)); + Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Data colors", (c, argument) -> new DataColorCollections(c)); private static final Map _handlers = new HashMap<>(); private static class DataColorCollections diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index 2fc3f5dd8f5..f6f1d2581d1 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -1765,7 +1765,7 @@ else if (showAllWarnings) } // Cache with short-lived entries so that exp.files can perform reasonably - private static final Cache _fileDataUpToDateCache = CacheManager.getCache(CacheManager.UNLIMITED, 5 * CacheManager.MINUTE, "Files"); + private static final Cache _fileDataUpToDateCache = CacheManager.getCache(Container.class, CacheManager.UNLIMITED, 5 * CacheManager.MINUTE, "Files"); @TestWhen(TestWhen.When.BVT) public static class TestCase extends AssertionError diff --git a/filecontent/src/org/labkey/filecontent/FileRootManager.java b/filecontent/src/org/labkey/filecontent/FileRootManager.java index 6a103074311..b19dd30d49f 100644 --- a/filecontent/src/org/labkey/filecontent/FileRootManager.java +++ b/filecontent/src/org/labkey/filecontent/FileRootManager.java @@ -26,18 +26,23 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.files.FileRoot; -import org.labkey.api.query.FieldKey; import org.labkey.api.security.User; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + public class FileRootManager { public static final String FILE_CONTENT_SCHEMA_NAME = "filecontent"; private static final FileRootManager _instance = new FileRootManager(); - private static final BlockingCache CACHE = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "FileRoots", (key, c) -> { - SimpleFilter filter = new SimpleFilter(FieldKey.fromString("Container"), c); - - return new TableSelector(getTinfoFileRoots(), filter, null).getObject(FileRoot.class); + // The table is sparse, so cache every row in one entry (container id -> root) rather than a hit or miss per container + private static final String CACHE_KEY = "AllRoots"; + private static final BlockingCache> CACHE = CacheManager.getBlockingStringKeyCache(1, CacheManager.DAY, "FileRoots", (key, argument) -> { + Map roots = new HashMap<>(); + new TableSelector(getTinfoFileRoots()).forEach(FileRoot.class, root -> roots.put(root.getContainer(), root)); + return Collections.unmodifiableMap(roots); }); private FileRootManager(){} @@ -57,17 +62,12 @@ public static TableInfo getTinfoFileRoots() return getFileContentSchema().getTable("FileRoots"); } - private String getCacheKey(Container c) - { - return c.getId(); - } - public FileRoot getFileRoot(Container c) { if (c == null) throw new IllegalArgumentException("getFileRoot: Container cannot be null"); - - FileRoot root = CACHE.get(getCacheKey(c), c); + + FileRoot root = CACHE.get(CACHE_KEY).get(c.getId()); return null == root ? new FileRoot(c) : root; } @@ -77,7 +77,7 @@ public void deleteFileRoot(Container c) SimpleFilter filter = SimpleFilter.createContainerFilter(c); Table.delete(getTinfoFileRoots(), filter); - CACHE.remove(getCacheKey(c)); + clearCache(); } public void saveFileRoot(User user, FileRoot root) @@ -95,13 +95,12 @@ public void saveFileRoot(User user, FileRoot root) } finally { - // Must clear cache in both insert & update, otherwise NULL marker remains in the cache - CACHE.remove(root.getContainer()); + clearCache(); } } public void clearCache() { - CACHE.clear(); + CACHE.remove(CACHE_KEY); } } diff --git a/issues/src/org/labkey/issue/model/IssueListDefCache.java b/issues/src/org/labkey/issue/model/IssueListDefCache.java index bfb0d7bac0c..187b9fd4044 100644 --- a/issues/src/org/labkey/issue/model/IssueListDefCache.java +++ b/issues/src/org/labkey/issue/model/IssueListDefCache.java @@ -37,7 +37,7 @@ */ public class IssueListDefCache { - private static final Cache ISSUE_DEF_DB_CACHE = CacheManager.getBlockingCache(CacheManager.UNLIMITED, CacheManager.DAY, "Issue list definitions", (c, argument) -> new IssueDefCollections(c)); + private static final Cache ISSUE_DEF_DB_CACHE = CacheManager.getBlockingCache(Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Issue list definitions", (c, argument) -> new IssueDefCollections(c)); private static class IssueDefCollections { diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java index 9ee77282f1f..bde9beca07c 100644 --- a/list/src/org/labkey/list/model/ListManager.java +++ b/list/src/org/labkey/list/model/ListManager.java @@ -113,12 +113,17 @@ private class ListDefCacheLoader implements CacheLoader> @Override public List load(@NotNull String entityId, @Nullable Object argument) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Container"), entityId); - ArrayList ownLists = new TableSelector(getListMetadataTable(), filter, null).getArrayList(ListDef.class); - return ownLists.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(ownLists); + return loadOwnLists(entityId); } } + private List loadOwnLists(String containerId) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Container"), containerId); + ArrayList ownLists = new TableSelector(getListMetadataTable(), filter, null).getArrayList(ListDef.class); + return ownLists.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(ownLists); + } + public static ListManager get() { return INSTANCE; @@ -538,14 +543,17 @@ public void deleteLists(Container c, User user, @Nullable String auditUserCommen public void enumerateDocuments(SearchService.TaskIndexingQueue queue, @Nullable Date since) { Consumer r = (q) -> { - Map lists = ListService.get().getLists(q.getContainer(), null, false); + // Only this container's own lists: getLists() adds a workbook's parent lists, which each workbook crawl would + // then re-index. Uncached, so the crawl doesn't leave a cache entry for every container. + List lists = loadOwnLists(q.getContainer().getId()); try { QueryService.get().setEnvironment(QueryService.Environment.USER, User.getSearchUser()); QueryService.get().setEnvironment(QueryService.Environment.CONTAINER, q.getContainer()); - for (ListDefinition list : lists.values()) + for (ListDef def : lists) { + ListDefinition list = new ListDefinitionImpl(def); try { boolean reindex = since == null; diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java index 7c524e1439f..13d0fa939c2 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineManager.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineManager.java @@ -97,6 +97,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.LinkedList; @@ -114,20 +115,23 @@ public class PipelineManager { private static final Logger _log = LogManager.getLogger(PipelineManager.class); private static final PipelineSchema pipeline = PipelineSchema.getInstance(); - private static final BlockingCache CACHE = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "Pipeline roots", - (key, argument) -> new TableSelector(pipeline.getTableInfoPipelineRoots(), (Filter)argument, null).getObject(PipelineRoot.class)); + // The table is sparse, so cache every row in one entry (container id -> type -> root) rather than a hit or miss per container + private static final String CACHE_KEY = "AllRoots"; + private static final BlockingCache>> CACHE = CacheManager.getBlockingStringKeyCache(1, CacheManager.DAY, "Pipeline roots", + (key, argument) -> { + Map> roots = new HashMap<>(); + new TableSelector(pipeline.getTableInfoPipelineRoots()).forEach(PipelineRoot.class, root -> roots.computeIfAbsent(root.getContainerId(), id -> new HashMap<>()).put(root.getType(), root)); + return Collections.unmodifiableMap(roots); + }); protected static PipelineRoot getPipelineRootObject(Container container, String type) { - SimpleFilter filter = SimpleFilter.createContainerFilter(container); - filter.addCondition(FieldKey.fromParts("Type"), type); - - return CACHE.get(getCacheKey(container, type), filter); + return CACHE.get(CACHE_KEY).getOrDefault(container.getId(), Collections.emptyMap()).get(type); } - private static String getCacheKey(Container c, @Nullable String type) + private static void uncache() { - return c.getId() + "/" + StringUtils.trimToEmpty(type); + CACHE.remove(CACHE_KEY); } @Nullable @@ -202,7 +206,7 @@ static public void setPipelineRoot(User user, Container container, URI[] roots, } finally { - CACHE.remove(getCacheKey(container, type)); + uncache(); } ContainerManager.firePropertyChangeEvent(new ContainerManager.ContainerPropertyChangeEvent( @@ -234,7 +238,7 @@ static public void purge(Container container, User user) } finally { - CACHE.remove(getCacheKey(container, null)); + uncache(); } // Delete trigger configurations through the UserSchema so that we stop any associated listeners. See issue 33986 diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 6783b906a7f..d641282ea38 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -235,6 +235,7 @@ import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.JavaScriptFragment; import org.labkey.api.util.JsonUtil; +import org.labkey.api.util.KeySharingJSONTokener; import org.labkey.api.util.LinkBuilder; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; @@ -4659,7 +4660,7 @@ public void validateForm(FORM apiSaveRowsForm, Errors errors) // if the POST was done using FormData, the apiSaveRowsForm would not have bound the json data, so // we'll instead look for that data in the request param directly if (_json == null && getViewContext().getRequest() != null && getViewContext().getRequest().getParameter("json") != null) - _json = new JSONObject(getViewContext().getRequest().getParameter("json")); + _json = new JSONObject(new KeySharingJSONTokener(getViewContext().getRequest().getParameter("json"))); } protected JSONObject getJsonObject() @@ -4814,6 +4815,11 @@ private JSONObject executeJson(TracedOperation op, JSONObject json, CommandType } } + // The form and action share this JSONObject for the whole request, so drop the converted rows to free the parse tree; provenance rereads them after the save + if (!json.has("provenance")) + json.remove(PROP_ROWS); + rows = null; + Map extraContext = json.has("extraContext") ? new CaseInsensitiveHashMap<>(json.getJSONObject("extraContext").toMap()) : new CaseInsensitiveHashMap<>(); Map auditDetails = json.has("auditDetails") ? json.getJSONObject("auditDetails").toMap() : new CaseInsensitiveHashMap<>(); diff --git a/query/src/org/labkey/query/olap/ServerManager.java b/query/src/org/labkey/query/olap/ServerManager.java index 6badfbe052d..c4e60205134 100644 --- a/query/src/org/labkey/query/olap/ServerManager.java +++ b/query/src/org/labkey/query/olap/ServerManager.java @@ -118,7 +118,7 @@ public class ServerManager private static final String DIR_NAME = "olap"; private static final ModuleResourceCache> MODULE_DESCRIPTOR_CACHE = ModuleResourceCaches.create("Olap cube definitions (module)", new OlapSchemaCacheHandler(), ResourceRootProvider.getStandard(new Path(DIR_NAME))); - private static final BlockingCache> DB_DESCRIPTOR_CACHE = CacheManager.getBlockingCache(Constants.getMaxContainers(), CacheManager.DAY, "Olap cube definitions (db)", new OlapCacheLoader()); + private static final BlockingCache> DB_DESCRIPTOR_CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxContainers(), CacheManager.DAY, "Olap cube definitions (db)", new OlapCacheLoader()); private static final BlockingCache CUBES = CacheManager.getBlockingStringKeyCache(CacheManager.UNLIMITED, 2 * CacheManager.DAY, "Olap cubes", null); diff --git a/query/src/org/labkey/query/persist/CustomViewCache.java b/query/src/org/labkey/query/persist/CustomViewCache.java index 0d13bbce8b7..0587cd2e00f 100644 --- a/query/src/org/labkey/query/persist/CustomViewCache.java +++ b/query/src/org/labkey/query/persist/CustomViewCache.java @@ -43,7 +43,7 @@ public class CustomViewCache { private static final Cache CUSTOM_VIEW_DB_CACHE = CacheManager.getBlockingCache( - CacheManager.UNLIMITED, CacheManager.DAY, "Database custom views", + Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Database custom views", (c, argument) -> new CustomViewCollections(c) ); diff --git a/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java b/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java index f29758f78c6..fe2185c29d2 100644 --- a/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java +++ b/query/src/org/labkey/query/persist/ExternalSchemaDefCache.java @@ -37,7 +37,7 @@ */ public class ExternalSchemaDefCache { - private static final Cache EXTERNAL_SCHEMA_DEF_CACHE = CacheManager.getBlockingCache(CacheManager.UNLIMITED, CacheManager.DAY, "External/linked schema definitions", (c, argument) -> getCollectionsToCache(c)); + private static final Cache EXTERNAL_SCHEMA_DEF_CACHE = CacheManager.getBlockingCache(Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "External/linked schema definitions", (c, argument) -> getCollectionsToCache(c)); @Nullable public static T getSchemaDef(Container c, @Nullable String userSchemaName, Class clazz) diff --git a/query/src/org/labkey/query/persist/QueryDefCache.java b/query/src/org/labkey/query/persist/QueryDefCache.java index 21ac9651711..8efc33ef413 100644 --- a/query/src/org/labkey/query/persist/QueryDefCache.java +++ b/query/src/org/labkey/query/persist/QueryDefCache.java @@ -39,7 +39,7 @@ */ public class QueryDefCache { - private static final Cache QUERY_DEF_DB_CACHE = CacheManager.getBlockingCache(CacheManager.UNLIMITED, CacheManager.DAY, "Database QueryDefs", (c, argument) -> new QueryDefCollections(c)); + private static final Cache QUERY_DEF_DB_CACHE = CacheManager.getBlockingCache(Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Database QueryDefs", (c, argument) -> new QueryDefCollections(c)); private static class QueryDefCollections { diff --git a/query/src/org/labkey/query/persist/QuerySnapshotCache.java b/query/src/org/labkey/query/persist/QuerySnapshotCache.java index f47d596392c..8e765d2d0a7 100644 --- a/query/src/org/labkey/query/persist/QuerySnapshotCache.java +++ b/query/src/org/labkey/query/persist/QuerySnapshotCache.java @@ -37,7 +37,7 @@ */ public class QuerySnapshotCache { - private static final Cache QUERY_SNAPSHOT_DEF_CACHE = CacheManager.getBlockingCache(Constants.getMaxContainers(), CacheManager.DAY, "Query snapshots", (c, argument) -> new QuerySnapshotCollections(c)); + private static final Cache QUERY_SNAPSHOT_DEF_CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxContainers(), CacheManager.DAY, "Query snapshots", (c, argument) -> new QuerySnapshotCollections(c)); static @NotNull Collection getQuerySnapshotDefs(@Nullable Container c, @Nullable String schemaName) { diff --git a/query/src/org/labkey/query/reports/DatabaseReportCache.java b/query/src/org/labkey/query/reports/DatabaseReportCache.java index e54f9297507..ce465690876 100644 --- a/query/src/org/labkey/query/reports/DatabaseReportCache.java +++ b/query/src/org/labkey/query/reports/DatabaseReportCache.java @@ -43,71 +43,75 @@ */ public class DatabaseReportCache { - private static final Cache REPORT_DB_CACHE = CacheManager.getBlockingCache(CacheManager.UNLIMITED, CacheManager.DAY, "Database reports", (c, argument) -> new ReportCollections(c)); + private static final Cache REPORT_DB_CACHE = CacheManager.getBlockingCache(Container.class, CacheManager.UNLIMITED, CacheManager.DAY, "Database reports", (c, argument) -> ReportCollections.load(c)); - private static class ReportCollections - { - private final Map _rowIdMap; - private final Map _entityIdMap; - private final MultiValuedMap _reportKeyMap; - private final Collection _inheritableReports; - - private ReportCollections(Container c) - { - ReportServiceImpl svc = ReportServiceImpl.getInstance(); - Map rowIdMap = new IntHashMap<>(); - Map entityIdMap = new HashMap<>(); - MultiValuedMap reportKeyMap = new CaseInsensitiveArrayListValuedMap<>(); // Issue 36199: change map to by case insensitive - List inheritableReports = new LinkedList<>(); - - new TableSelector(CoreSchema.getInstance().getTableInfoReport(), SimpleFilter.createContainerFilter(c, "ContainerId"), null).forEach(ReportDB.class, reportDB -> { - Report report = svc._getInstance(reportDB); - - // Reports can be null if type is unknown (e.g., defining module disappears) - if (null != report) - { - rowIdMap.put(reportDB.getRowId(), report); - entityIdMap.put(reportDB.getEntityId(), report); - reportKeyMap.put(reportDB.getReportKey(), report); - - if ((reportDB.getFlags() & ReportDescriptor.FLAG_INHERITABLE) != 0) - inheritableReports.add(report); - } - }); - - _rowIdMap = rowIdMap.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(rowIdMap); - _entityIdMap = entityIdMap.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(entityIdMap); - _reportKeyMap = reportKeyMap.isEmpty() ? MultiMapUtils.emptyMultiValuedMap() : MultiMapUtils.unmodifiableMultiValuedMap(reportKeyMap); - _inheritableReports = inheritableReports.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(inheritableReports); - } - - private @Nullable Report getForRowId(int rowId) - { - return _rowIdMap.get(rowId); - } - - private @Nullable Report getForEntityId(String entityId) - { - return _entityIdMap.get(entityId); - } - - private @NotNull Collection getForReportKey(String reportKey) - { - Collection reports = _reportKeyMap.get(reportKey); - return null != reports ? reports : Collections.emptyList(); - } - - private @NotNull Collection getReports() + private record ReportCollections(Map _rowIdMap, Map _entityIdMap, + MultiValuedMap _reportKeyMap, + Collection _inheritableReports) { - return _rowIdMap.values(); + // Issue 53472: most containers have no reports, so share one instance across them + private static final ReportCollections EMPTY = new ReportCollections(Collections.emptyMap(), Collections.emptyMap(), MultiMapUtils.emptyMultiValuedMap(), Collections.emptyList()); + + private static ReportCollections load(Container c) + { + ReportServiceImpl svc = ReportServiceImpl.getInstance(); + Map rowIdMap = new IntHashMap<>(); + Map entityIdMap = new HashMap<>(); + MultiValuedMap reportKeyMap = new CaseInsensitiveArrayListValuedMap<>(); // Issue 36199: change map to by case insensitive + List inheritableReports = new LinkedList<>(); + + new TableSelector(CoreSchema.getInstance().getTableInfoReport(), SimpleFilter.createContainerFilter(c, "ContainerId"), null).forEach(ReportDB.class, reportDB -> { + Report report = svc._getInstance(reportDB); + + // Reports can be null if type is unknown (e.g., defining module disappears) + if (null != report) + { + rowIdMap.put(reportDB.getRowId(), report); + entityIdMap.put(reportDB.getEntityId(), report); + reportKeyMap.put(reportDB.getReportKey(), report); + + if ((reportDB.getFlags() & ReportDescriptor.FLAG_INHERITABLE) != 0) + inheritableReports.add(report); + } + }); + + if (rowIdMap.isEmpty()) + return EMPTY; + + return new ReportCollections( + Collections.unmodifiableMap(rowIdMap), + Collections.unmodifiableMap(entityIdMap), + MultiMapUtils.unmodifiableMultiValuedMap(reportKeyMap), + inheritableReports.isEmpty() ? Collections.emptyList() : Collections.unmodifiableList(inheritableReports)); + } + + private @Nullable Report getForRowId(int rowId) + { + return _rowIdMap.get(rowId); + } + + private @Nullable Report getForEntityId(String entityId) + { + return _entityIdMap.get(entityId); + } + + private @NotNull Collection getForReportKey(String reportKey) + { + Collection reports = _reportKeyMap.get(reportKey); + return null != reports ? reports : Collections.emptyList(); + } + + private @NotNull Collection getReports() + { + return _rowIdMap.values(); + } + + private @NotNull Collection getInheritableReports() + { + return _inheritableReports; + } } - private @NotNull Collection getInheritableReports() - { - return _inheritableReports; - } - } - static @Nullable Report getReport(Container c, int rowId) { return REPORT_DB_CACHE.get(c).getForRowId(rowId); diff --git a/query/src/org/labkey/query/reports/ReportServiceImpl.java b/query/src/org/labkey/query/reports/ReportServiceImpl.java index 98cefa57609..3ac49d30851 100644 --- a/query/src/org/labkey/query/reports/ReportServiceImpl.java +++ b/query/src/org/labkey/query/reports/ReportServiceImpl.java @@ -40,6 +40,7 @@ import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; @@ -111,10 +112,12 @@ import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.labkey.api.reports.report.ScriptReportDescriptor.REPORT_METADATA_EXTENSION; @@ -1092,23 +1095,33 @@ public static void registerUsageMetrics(String moduleName) MultiSet chartCountsByRenderType = new HashMultiSet<>(); AtomicInteger genericChartWithTrendlineTypeCount = new AtomicInteger(); AtomicInteger genericChartWithErrorBarsCount = new AtomicInteger(); - Map countsByType = ContainerManager.getAllChildren(ContainerManager.getRoot()).stream() - .flatMap(c -> ReportService.get().getReports(null, c).stream()) - .peek(report -> { - if (report instanceof GenericChartReport chart) - { - chartCountsByRenderType.add(chart.getRenderType()); - if (chart.getDescriptor() instanceof VisualizationReportDescriptor descriptor) + // Type comes from the descriptor XML, so instantiate each report, but stream rows rather than load every container's report cache + TableInfo table = getTable(); + SQLFragment sql = new SQLFragment("SELECT * FROM ").append(table, "r") + .append(" WHERE r.ContainerId IN (SELECT EntityId FROM ").append(CoreSchema.getInstance().getTableInfoContainers(), "c").append(")"); + ReportServiceImpl reportService = getInstance(); + Map countsByType; + try (Stream rows = new SqlSelector(table.getSchema(), sql).uncachedStream(ReportDB.class)) + { + countsByType = rows + .map(reportService::_getInstance) + .filter(Objects::nonNull) + .peek(report -> { + if (report instanceof GenericChartReport chart) { - String configJson = descriptor.getJSON(); - if (configJson.contains("\"trendlineType\":") && !configJson.contains("\"trendlineType\":\"\"")) - genericChartWithTrendlineTypeCount.getAndIncrement(); - if (configJson.contains("\"errorBars\":\"SD\"") || configJson.contains("\"errorBars\":\"SEM\"")) - genericChartWithErrorBarsCount.getAndIncrement(); + chartCountsByRenderType.add(chart.getRenderType()); + if (chart.getDescriptor() instanceof VisualizationReportDescriptor descriptor) + { + String configJson = descriptor.getJSON(); + if (configJson.contains("\"trendlineType\":") && !configJson.contains("\"trendlineType\":\"\"")) + genericChartWithTrendlineTypeCount.getAndIncrement(); + if (configJson.contains("\"errorBars\":\"SD\"") || configJson.contains("\"errorBars\":\"SEM\"")) + genericChartWithErrorBarsCount.getAndIncrement(); + } } - } - }) - .collect(Collectors.groupingBy(Report::getType, Collectors.counting())); + }) + .collect(Collectors.groupingBy(Report::getType, Collectors.counting())); + } return Map.of( "reportCountsByType", countsByType, diff --git a/study/api-src/org/labkey/api/specimen/location/LocationCache.java b/study/api-src/org/labkey/api/specimen/location/LocationCache.java index af3f99ba5d9..386d36162b4 100644 --- a/study/api-src/org/labkey/api/specimen/location/LocationCache.java +++ b/study/api-src/org/labkey/api/specimen/location/LocationCache.java @@ -34,7 +34,7 @@ public class LocationCache { - private static final Cache CACHE = CacheManager.getBlockingCache(Constants.getMaxContainers(), CacheManager.DAY, "Study locations", (c, argument) -> new LocationCollections(c)); + private static final Cache CACHE = CacheManager.getBlockingCache(Container.class, Constants.getMaxContainers(), CacheManager.DAY, "Study locations", (c, argument) -> new LocationCollections(c)); private static class LocationCollections { diff --git a/study/api-src/org/labkey/api/study/QueryHelper.java b/study/api-src/org/labkey/api/study/QueryHelper.java index 2816726a9f5..c680cee8deb 100644 --- a/study/api-src/org/labkey/api/study/QueryHelper.java +++ b/study/api-src/org/labkey/api/study/QueryHelper.java @@ -54,7 +54,7 @@ public QueryHelper(TableInfoGetter tableInfoGetter, Class objectClass, @Nulla _objectClass = objectClass; _defaultSortString = defaultSortString; TableInfo tableInfo = _tableInfoGetter.getTableInfo(); - _cache = DatabaseCache.get(tableInfo.getSchema().getScope(), tableInfo.getCacheSize(), "StudyCache: " + tableInfo.getName(), (key, _) -> + _cache = DatabaseCache.get(Container.class, tableInfo.getSchema().getScope(), tableInfo.getCacheSize(), "StudyCache: " + tableInfo.getName(), (key, _) -> { final Map map; try (Stream stream = getTableSelector(key).uncachedStream(_objectClass)) diff --git a/study/src/org/labkey/study/model/ParticipantGroupCache.java b/study/src/org/labkey/study/model/ParticipantGroupCache.java index 5c6c6922b2f..a21d1c7f41c 100644 --- a/study/src/org/labkey/study/model/ParticipantGroupCache.java +++ b/study/src/org/labkey/study/model/ParticipantGroupCache.java @@ -41,10 +41,11 @@ public class ParticipantGroupCache { private static final Cache PARTICIPANT_GROUP_CACHE = CacheManager.getBlockingCache( + Container.class, Constants.getMaxContainers(), CacheManager.DAY, "Participant groups", - (c, argument) -> new ParticipantGroupCollections(c)); + (c, _) -> new ParticipantGroupCollections(c)); private static class ParticipantGroupCollections { diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 48595f02f33..7bee95b5ae1 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -247,7 +247,7 @@ public class StudyManager private final DatasetHelper _datasetHelper; private final QueryHelper> _cohortHelper; private final BlockingCache> _sharedProperties; - private final BlockingCache> _participantCache = DatabaseCache.get(StudySchema.getInstance().getScope(), Constants.getMaxContainers(), CacheManager.HOUR, "Participants", (c, argument) -> { + private final BlockingCache> _participantCache = DatabaseCache.get(Container.class, StudySchema.getInstance().getScope(), Constants.getMaxContainers(), CacheManager.HOUR, "Participants", (c, argument) -> { SimpleFilter filter = SimpleFilter.createContainerFilter(c); return Collections.unmodifiableMap( new TableSelector(StudySchema.getInstance().getTableInfoParticipant(), filter, new Sort("ParticipantId")) @@ -270,7 +270,7 @@ private StudyManager() // Cache of PropertyDescriptors found in the Shared container for datasets in the given study Container. // The shared properties cache will be cleared when the _datasetHelper cache is cleared. - _sharedProperties = CacheManager.getBlockingCache(1000, CacheManager.UNLIMITED, "Study shared properties", + _sharedProperties = CacheManager.getBlockingCache(Container.class, 1000, CacheManager.UNLIMITED, "Study shared properties", (key, argument) -> { Container sharedContainer = ContainerManager.getSharedContainer();