From c0522ebbadfe01760f4faf510c59799b05a02f4c Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 28 Sep 2026 09:42:41 -0700 Subject: [PATCH 1/6] Reduce connection pool borrows in menu sections, metadata loading, lineage, NAb, and audit grids Cache data type exclusions, batch per-section and per-well-group queries, share one held connection across repeated statements, narrow invalidateDomain and object-delete property cache clears, and memoize per-row audit lookups. --- .../api/audit/data/ExperimentAuditColumn.java | 17 ++++++- .../labkey/api/data/SchemaColumnMetaData.java | 13 ++++-- .../org/labkey/api/exp/OntologyManager.java | 25 +++++++++-- .../api/exp/property/DomainAuditProvider.java | 45 ++++++++++++++----- .../labkey/api/pipeline/PipelineService.java | 3 ++ .../assay/dilution/DilutionDataHandler.java | 16 +++++-- .../api/assay/dilution/DilutionManager.java | 8 ++++ .../labkey/api/assay/plate/PlateService.java | 6 +++ .../src/org/labkey/assay/plate/PlateImpl.java | 35 +++++++++++++++ .../org/labkey/assay/plate/PlateManager.java | 12 +++++ .../org/labkey/assay/plate/WellGroupImpl.java | 8 +--- .../core/products/ProductController.java | 14 +++++- .../experiment/api/ClosureQueryHelper.java | 16 +++++++ .../experiment/api/ExperimentServiceImpl.java | 34 +++++++++++--- .../labkey/list/model/ListServiceImpl.java | 10 +++++ .../pipeline/api/PipelineServiceImpl.java | 10 +++++ 16 files changed, 233 insertions(+), 39 deletions(-) diff --git a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java index 087373e5bce..0bc04e847d6 100644 --- a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java +++ b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java @@ -28,12 +28,17 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.writer.HtmlWriter; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; import java.util.Set; public abstract class ExperimentAuditColumn extends DataColumn { protected ColumnInfo _containerId; protected ColumnInfo _defaultName; + // Keyed by bound value + container id, since the same object typically repeats across rows + private final Map, Optional>> _expValues = new HashMap<>(); public static final String KEY_SEPARATOR = "~~KEYSEP~~"; @@ -63,10 +68,18 @@ protected Container getContainer(RenderContext ctx) @Nullable protected abstract Pair getExpValue(RenderContext ctx); + @Nullable + private Pair getCachedExpValue(RenderContext ctx) + { + Container c = getContainer(ctx); + Pair key = Pair.of(getBoundColumn().getValue(ctx), c == null ? null : c.getId()); + return _expValues.computeIfAbsent(key, _ -> Optional.ofNullable(getExpValue(ctx))).orElse(null); + } + @Override public Object getDisplayValue(RenderContext ctx) { - Pair value = getExpValue(ctx); + Pair value = getCachedExpValue(ctx); if (value != null) { return value.first.getName(); @@ -101,7 +114,7 @@ public boolean isFilterable() @Override public void renderGridCellContents(RenderContext ctx, HtmlWriter out) { - Pair value = getExpValue(ctx); + Pair value = getCachedExpValue(ctx); if (value != null && value.second != null) { out.write(LinkBuilder.simpleLink(value.first.getName(), value.second)); diff --git a/api/src/org/labkey/api/data/SchemaColumnMetaData.java b/api/src/org/labkey/api/data/SchemaColumnMetaData.java index 67590fd1768..753f293729f 100644 --- a/api/src/org/labkey/api/data/SchemaColumnMetaData.java +++ b/api/src/org/labkey/api/data/SchemaColumnMetaData.java @@ -38,6 +38,7 @@ import org.labkey.data.xml.ColumnType; import org.labkey.data.xml.TableType; +import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; @@ -254,13 +255,17 @@ private DbScope.RetryFn createRetryWrapper(RetrySqlException retry) private void loadFromMetaData(SchemaTableInfo ti) throws SQLException { - try (var ignore = DebugInfoDumper.pushThreadDumpContext("SchemaColumnMetaData.loadFromMetaData(" + ti.getSelectName() + ")")) + DbScope scope = ti.getSchema().getScope(); + + // Hold the thread connection so the three metadata passes share one pool borrow + try (var ignore = DebugInfoDumper.pushThreadDumpContext("SchemaColumnMetaData.loadFromMetaData(" + ti.getSelectName() + ")"); + Connection ignored = scope.getConnection()) { // With the Microsoft JDBC driver we're seeing more deadlocks loading schema metadata so try multiple // times when possible - ti.getSchema().getScope().executeWithRetryReadOnly(createRetryWrapper((tx) -> loadColumnsFromMetaData(ti))); - ti.getSchema().getScope().executeWithRetryReadOnly(createRetryWrapper((tx) -> loadPkColumns(ti))); - ti.getSchema().getScope().executeWithRetryReadOnly(createRetryWrapper((tx) -> loadIndices(ti))); + scope.executeWithRetryReadOnly(createRetryWrapper((tx) -> loadColumnsFromMetaData(ti))); + scope.executeWithRetryReadOnly(createRetryWrapper((tx) -> loadPkColumns(ti))); + scope.executeWithRetryReadOnly(createRetryWrapper((tx) -> loadIndices(ti))); } catch (RuntimeSQLException e) { diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index d0bfbd2bb94..b2ff0bfa4dc 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -1053,7 +1053,7 @@ public static void deleteOntologyObjects(Container c, String... uris) } finally { - PROPERTY_MAP_CACHE.clear(); + clearPropertyCache(c); OBJECT_ID_CACHE.clear(); } } @@ -1166,7 +1166,7 @@ public static void deleteOntologyObjects(Container c, boolean deleteOwnedObjects } finally { - PROPERTY_MAP_CACHE.clear(); + clearPropertyCache(c); OBJECT_ID_CACHE.clear(); } } @@ -2916,8 +2916,19 @@ public PropertyUsages(int propertyId, String propertyURI, int usageCount, List propertyURIs = d.getProperties().stream().map(DomainProperty::getPropertyURI).collect(Collectors.toSet()); + + // Keys are per lookup project and nulls are cached, so drop this domain's entries under every project + 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)); + 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 + PROPERTY_MAP_CACHE.clear(); + ExperimentService.get().clearCaches(); } @@ -2940,6 +2951,12 @@ public static void clearPropertyCache(String parentObjectURI) } + /** Owned objects share their owner's container, so this also covers deleted children whose URIs aren't known */ + private static void clearPropertyCache(Container c) + { + PROPERTY_MAP_CACHE.removeUsingFilter(key -> key.first == null || key.first.equals(c)); + } + public static void clearPropertyCache() { PROPERTY_MAP_CACHE.clear(); diff --git a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index 3c2a4fcd71e..17861b283cc 100644 --- a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java @@ -16,6 +16,7 @@ package org.labkey.api.exp.property; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; @@ -37,15 +38,19 @@ import org.labkey.api.query.UserSchema; import org.labkey.api.util.HtmlString; import org.labkey.api.util.LinkBuilder; +import org.labkey.api.util.Pair; +import org.labkey.api.view.ActionURL; import org.labkey.api.writer.DefaultContainerUser; import org.labkey.api.writer.HtmlWriter; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; public class DomainAuditProvider extends AbstractAuditTypeProvider implements AuditTypeProvider @@ -221,6 +226,10 @@ public static class DomainColumn extends DataColumn private final String _containerColumnName; @NotNull private final String _defaultNameColumnName; + // Keyed by container id + domain URI; empty when the domain no longer exists + private final Map, Optional> _domainLinks = new HashMap<>(); + + private record DomainLink(String name, @Nullable ActionURL url, boolean hasKind) {} public DomainColumn(@NotNull ColumnInfo col, @NotNull String containerColumnName, @NotNull String defaultNameColumnName) { @@ -255,19 +264,14 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out) if (uri != null && cId != null) { - Container c = ContainerManager.getForId(cId); - if (c != null) + Optional link = _domainLinks.computeIfAbsent(Pair.of(cId, uri), _ -> getDomainLink(ctx, cId, uri)); + if (link.isPresent()) { - Domain domain = PropertyService.get().getDomain(c, uri); - if (domain != null) - { - DomainKind kind = PropertyService.get().getDomainKind(domain.getTypeURI()); - if (kind != null) - out.write(LinkBuilder.simpleLink(domain.getName(), kind.urlShowData(domain, new DefaultContainerUser(c, ctx.getViewContext().getUser())))); - else - out.write(domain.getName()); - return; - } + if (link.get().hasKind()) + out.write(LinkBuilder.simpleLink(link.get().name(), link.get().url())); + else + out.write(link.get().name()); + return; } } @@ -278,6 +282,23 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out) out.write(HtmlString.NBSP); } + private Optional getDomainLink(RenderContext ctx, String cId, String uri) + { + Container c = ContainerManager.getForId(cId); + if (c == null) + return Optional.empty(); + + Domain domain = PropertyService.get().getDomain(c, uri); + if (domain == null) + return Optional.empty(); + + DomainKind kind = PropertyService.get().getDomainKind(domain.getTypeURI()); + if (kind == null) + return Optional.of(new DomainLink(domain.getName(), null, false)); + + return Optional.of(new DomainLink(domain.getName(), kind.urlShowData(domain, new DefaultContainerUser(c, ctx.getViewContext().getUser())), true)); + } + @Override public void addQueryFieldKeys(Set keys) { diff --git a/api/src/org/labkey/api/pipeline/PipelineService.java b/api/src/org/labkey/api/pipeline/PipelineService.java index 16678921d56..1fde0286c5e 100644 --- a/api/src/org/labkey/api/pipeline/PipelineService.java +++ b/api/src/org/labkey/api/pipeline/PipelineService.java @@ -266,6 +266,9 @@ public AbstractFileAnalysisProtocolFactory getFactory() Collection> getActivePipelineJobs(User u, Container c, String providerName, @Nullable ContainerFilter cf); + /** Provider and Description of the active jobs for any of the given providers */ + Collection> getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf); + interface PipelineProviderSupplier { @NotNull Collection getAll(); diff --git a/assay/api-src/org/labkey/api/assay/dilution/DilutionDataHandler.java b/assay/api-src/org/labkey/api/assay/dilution/DilutionDataHandler.java index 2660f47fa9c..b5986487789 100644 --- a/assay/api-src/org/labkey/api/assay/dilution/DilutionDataHandler.java +++ b/assay/api-src/org/labkey/api/assay/dilution/DilutionDataHandler.java @@ -329,7 +329,9 @@ protected List createPlates(ExpRun run, Plate template, boolean recalcSta excluded[wellDataRow.getRow()][wellDataRow.getColumn()] = wellDataRow.isExcluded(); } - Plate plate = PlateService.get().createPlate(template, cellValues, excluded, recalcStats ? PlateService.NO_RUNID : run.getRowId(), 1); + Plate plate = recalcStats + ? PlateService.get().createPlate(template, cellValues, excluded, PlateService.NO_RUNID, 1) + : PlateService.get().createPlate(template, cellValues, excluded, run, 1); return Collections.singletonList(plate); } @@ -353,16 +355,22 @@ protected DilutionAssayRun getAssayResults(ExpRun run, User user, @Nullable File // Attempt to populate the well data for dataFileUrls that may have been fixed since the new // WellData and DilutionData tables were added. + boolean wellDataPopulated = false; synchronized (WELL_DATA_LOCK_OBJECT) { - if (useRunForPlates && !isWellDataPopulated(run) && getDataFile(run) != null) + if (useRunForPlates) { - populateWellData(protocol, run, user); + wellDataPopulated = isWellDataPopulated(run); + if (!wellDataPopulated && getDataFile(run) != null) + { + populateWellData(protocol, run, user); + wellDataPopulated = isWellDataPopulated(run); + } } } List plates; - if (useRunForPlates && isWellDataPopulated(run)) + if (wellDataPopulated) { plates = createPlates(run, nabTemplate, recalcStats); } diff --git a/assay/api-src/org/labkey/api/assay/dilution/DilutionManager.java b/assay/api-src/org/labkey/api/assay/dilution/DilutionManager.java index 2612a753850..81f8dfdfb18 100644 --- a/assay/api-src/org/labkey/api/assay/dilution/DilutionManager.java +++ b/assay/api-src/org/labkey/api/assay/dilution/DilutionManager.java @@ -153,6 +153,14 @@ public static List getDilutionDataRows(long runId, long plateNu return new TableSelector(getSchema().getTable(DILUTION_DATA_TABLE_NAME), filter, null).getArrayList(DilutionDataRow.class); } + public static List getDilutionDataRows(long runId, long plateNumber, Container container) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromString("runId"), runId); + filter.addCondition(FieldKey.fromString("plateNumber"), plateNumber); + return new TableSelector(getSchema().getTable(DILUTION_DATA_TABLE_NAME), filter, null).getArrayList(DilutionDataRow.class); + } + public static int insertWellDataRow(User user, Map fields) { TableInfo tableInfo = getSchema().getTable(WELL_DATA_TABLE_NAME); diff --git a/assay/api-src/org/labkey/api/assay/plate/PlateService.java b/assay/api-src/org/labkey/api/assay/plate/PlateService.java index 2633b918c96..5bd2429bd24 100644 --- a/assay/api-src/org/labkey/api/assay/plate/PlateService.java +++ b/assay/api-src/org/labkey/api/assay/plate/PlateService.java @@ -71,6 +71,12 @@ static PlateService get() */ @Nullable Plate createPlate(Plate plate, double[][] wellValues, boolean[][] excludedWells, long runId, int plateNumber); + /** + * Instantiates a new plate instance whose well group statistics come from the run's stored DilutionData. + * This plate is not persisted to the database. + */ + @Nullable Plate createPlate(Plate plate, double[][] wellValues, boolean[][] excludedWells, @NotNull ExpRun run, int plateNumber); + /** * Instantiates a new plate instance based on the specified plate and well data. * This plate is not persisted to the database. diff --git a/assay/src/org/labkey/assay/plate/PlateImpl.java b/assay/src/org/labkey/assay/plate/PlateImpl.java index 740f6774d14..78ecb74883b 100644 --- a/assay/src/org/labkey/assay/plate/PlateImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateImpl.java @@ -28,6 +28,8 @@ import org.junit.Test; import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; +import org.labkey.api.assay.dilution.DilutionDataRow; +import org.labkey.api.assay.dilution.DilutionManager; import org.labkey.api.assay.plate.Plate; import org.labkey.api.assay.plate.PlateCustomField; import org.labkey.api.assay.plate.PlateService; @@ -40,6 +42,8 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.Transient; +import org.labkey.api.exp.api.ExpRun; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryRowReference; import org.labkey.api.query.SchemaKey; @@ -93,6 +97,9 @@ public class PlateImpl extends PropertySetImpl implements Plate, Cloneable private Map _wellMap; private Integer _metadataDomainId; private transient Long _sourcePlateRowId; + private transient @Nullable Container _runContainer; + private transient Map> _replicateDilutionData; + private transient Map> _wellGroupDilutionData; // no-param constructor for reflection public PlateImpl() @@ -116,6 +123,12 @@ public PlateImpl(Container container, String name, @Nullable String barcode, @No } // Note that barcode values will be auto-generated + public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, @Nullable ExpRun run, int plateNumber) + { + this(plate, wellValues, excluded, run == null ? PlateService.NO_RUNID : run.getRowId(), plateNumber); + _runContainer = run == null ? null : run.getContainer(); + } + public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, long runId, int plateNumber) { this(plate.getContainer(), plate.getName(), null, plate.getAssayType(), plate.getPlateType()); @@ -700,6 +713,28 @@ public boolean mustCalculateStats() return _runId == PlateService.NO_RUNID; } + /** Stored DilutionData rows for one group; the first call loads the whole plate's rows */ + synchronized List getDilutionDataRows(String groupName, boolean replicate) + { + if (_wellGroupDilutionData == null) + { + Container runContainer = _runContainer != null ? _runContainer : ExperimentService.get().getExpRun(_runId).getContainer(); + Map> replicateRows = new HashMap<>(); + Map> wellGroupRows = new HashMap<>(); + for (DilutionDataRow row : DilutionManager.getDilutionDataRows(_runId, _plateNumber, runContainer)) + { + if (row.getReplicateName() != null) + replicateRows.computeIfAbsent(row.getReplicateName(), _ -> new ArrayList<>()).add(row); + if (row.getWellgroupName() != null) + wellGroupRows.computeIfAbsent(row.getWellgroupName(), _ -> new ArrayList<>()).add(row); + } + _replicateDilutionData = replicateRows; + _wellGroupDilutionData = wellGroupRows; + } + + return (replicate ? _replicateDilutionData : _wellGroupDilutionData).getOrDefault(groupName, List.of()); + } + @JsonIgnore @Override public int getPlateNumber() diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index a26f8e661b5..fbd279312e6 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -287,6 +287,18 @@ public List getWellGroupTypes() throw new IllegalArgumentException("Only plates retrieved from the plate service can be used to create plate instances."); } + @Override + public @Nullable Plate createPlate(Plate plate, double[][] wellValues, boolean[][] excluded, @NotNull ExpRun run, int plateNumber) + { + if (plate == null) + return null; + + if (plate instanceof PlateImpl plateImpl) + return new PlateImpl(plateImpl, wellValues, excluded, run, plateNumber); + + throw new IllegalArgumentException("Only plates retrieved from the plate service can be used to create plate instances."); + } + @Override public @NotNull PlateImpl createPlate(Container container, String assayType, @NotNull PlateType plateType) { diff --git a/assay/src/org/labkey/assay/plate/WellGroupImpl.java b/assay/src/org/labkey/assay/plate/WellGroupImpl.java index ffcaeddaa1e..f51586c94b2 100644 --- a/assay/src/org/labkey/assay/plate/WellGroupImpl.java +++ b/assay/src/org/labkey/assay/plate/WellGroupImpl.java @@ -19,7 +19,6 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.dilution.DilutionCurve; import org.labkey.api.assay.dilution.DilutionDataRow; -import org.labkey.api.assay.dilution.DilutionManager; import org.labkey.api.assay.plate.Plate; import org.labkey.api.assay.plate.PlateService; import org.labkey.api.assay.plate.Position; @@ -28,8 +27,6 @@ import org.labkey.api.assay.plate.WellGroup; import org.labkey.api.data.statistics.FitFailedException; import org.labkey.api.data.statistics.StatsService; -import org.labkey.api.exp.api.ExpRun; -import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.view.ActionURL; import java.util.ArrayList; @@ -462,10 +459,7 @@ private synchronized void computeStats(Double[] data) private void populateStatsFromTable() { - ExpRun run = ExperimentService.get().getExpRun(_plate.getRunId()); - List dilutionDataRows = DilutionManager.getDilutionDataRows(_plate.getRunId(), - _plate.getPlateNumber(), getName(), run.getContainer(), - Type.REPLICATE.equals(getType())); + List dilutionDataRows = _plate.getDilutionDataRows(getName(), Type.REPLICATE.equals(getType())); if (1 != dilutionDataRows.size()) throw new IllegalStateException("Expected a single DilutionData row to calculate wellgroup stats, but found " + dilutionDataRows.size() + " rows"); _dilutionDataRow = dilutionDataRows.getFirst(); diff --git a/core/src/org/labkey/core/products/ProductController.java b/core/src/org/labkey/core/products/ProductController.java index 9372edabef4..3e82f6068c3 100644 --- a/core/src/org/labkey/core/products/ProductController.java +++ b/core/src/org/labkey/core/products/ProductController.java @@ -21,6 +21,8 @@ import org.labkey.api.action.Marshaller; import org.labkey.api.action.ReadOnlyApiAction; import org.labkey.api.action.SpringActionController; +import org.labkey.api.data.DbScope; +import org.labkey.api.products.MenuSection; import org.labkey.api.products.ProductRegistry; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.ReadPermission; @@ -28,6 +30,7 @@ import org.springframework.validation.BindException; import org.springframework.validation.Errors; +import java.sql.Connection; import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -99,7 +102,16 @@ public void validateForm(MenuItemsForm menuItemsForm, Errors errors) @Override public Object execute(MenuItemsForm menuItemsForm, BindException errors) throws Exception { - return success(ProductRegistry.get().getProductMenuSections(getViewContext(), _productIds)); + List sections; + + // Items otherwise load lazily during serialization, one borrow per query + try (Connection ignored = DbScope.getLabKeyScope().getConnection()) + { + sections = ProductRegistry.get().getProductMenuSections(getViewContext(), _productIds); + sections.forEach(MenuSection::getItems); + } + + return success(sections); } } diff --git a/experiment/src/org/labkey/experiment/api/ClosureQueryHelper.java b/experiment/src/org/labkey/experiment/api/ClosureQueryHelper.java index 2354b504469..4e922068a12 100644 --- a/experiment/src/org/labkey/experiment/api/ClosureQueryHelper.java +++ b/experiment/src/org/labkey/experiment/api/ClosureQueryHelper.java @@ -30,6 +30,7 @@ import org.labkey.api.data.DisplayColumnFactory; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MutableColumnInfo; +import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.Table; @@ -53,6 +54,8 @@ import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.NotFoundException; +import java.sql.Connection; +import java.sql.SQLException; import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -387,6 +390,19 @@ public static void truncateAndRecreate(Logger logger) } public static void recomputeFromSeeds(SQLFragment selectSeedsSql, boolean isSampleType) + { + // Outside a transaction (e.g. as a post-commit task) each statement would otherwise borrow its own connection + try (Connection ignored = getScope().getConnection()) + { + _recomputeFromSeeds(selectSeedsSql, isSampleType); + } + catch (SQLException e) + { + throw new RuntimeSQLException(e); + } + } + + private static void _recomputeFromSeeds(SQLFragment selectSeedsSql, boolean isSampleType) { TempTableTracker ttt = null; try diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 081c99a3c80..6e1b39d2e81 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -337,6 +337,10 @@ public class ExperimentServiceImpl implements ExperimentService, ObjectReference private final Cache EXPERIMENT_RUN_CACHE = DatabaseCache.get(getExpSchema().getScope(), getTinfoExperimentRun().getCacheSize(), "Experiment Run by LSID", new ExperimentRunCacheLoader()); + /** ExcludedContainer id -> excluded data type row ids, by data type */ + private final Cache>> DATA_TYPE_EXCLUSION_CACHE = DatabaseCache.get(getExpSchema().getScope(), CacheManager.UNLIMITED, CacheManager.DAY, "Data type exclusions", + (containerId, _) -> loadContainerDataTypeExclusions(containerId)); + /** DataClass LSID -> Container */ private final Cache dataClassLsidCache = CacheManager.getStringKeyCache(CacheManager.UNLIMITED, CacheManager.DAY, "DataClass to container"); @@ -8954,6 +8958,7 @@ private void addDataTypeExclusion(long rowId, DataTypeForExclusion dataType, Str fields.put("DataType", dataType.name()); fields.put("ExcludedContainer", excludedContainerId); Table.insert(user, getTinfoDataTypeExclusion(), fields); + DATA_TYPE_EXCLUSION_CACHE.remove(excludedContainerId); } @Override @@ -8964,6 +8969,7 @@ public void removeContainerDataTypeExclusions(String containerId) .append(" WHERE excludedContainer = ? "); sql.add(containerId); new SqlExecutor(getExpSchema()).execute(sql); + DATA_TYPE_EXCLUSION_CACHE.remove(containerId); } @Override @@ -8987,6 +8993,11 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi } new SqlExecutor(getExpSchema()).execute(sql); + + if (StringUtils.isEmpty(excludedContainerId)) + DATA_TYPE_EXCLUSION_CACHE.clear(); + else + DATA_TYPE_EXCLUSION_CACHE.remove(excludedContainerId); } @NotNull private List> _getContainerDataTypeExclusions(@Nullable DataTypeForExclusion dataType, @Nullable String excludedContainerIdOrPath, @Nullable Long dataTypeRowId) @@ -9034,7 +9045,21 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi } @Override - public @NotNull Map> getContainerDataTypeExclusions(@NotNull String excludedContainerId) + public @NotNull Map> getContainerDataTypeExclusions(@NotNull String excludedContainerIdOrPath) + { + String excludedContainerId = excludedContainerIdOrPath; + if (!GUID.isGUID(excludedContainerIdOrPath)) + { + Container container = ContainerManager.getForPath(excludedContainerIdOrPath); + if (container == null) + return Collections.emptyMap(); + excludedContainerId = container.getId(); + } + + return DATA_TYPE_EXCLUSION_CACHE.get(excludedContainerId); + } + + private @NotNull Map> loadContainerDataTypeExclusions(@NotNull String excludedContainerId) { List> exclusions = _getContainerDataTypeExclusions(null, excludedContainerId, null); @@ -9043,12 +9068,11 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi { String dataTypeStr = (String) exclusion.get("DataType"); DataTypeForExclusion dataType = DataTypeForExclusion.valueOf(dataTypeStr); - if (!typeExclusions.containsKey(dataType)) - typeExclusions.put(dataType, new HashSet<>()); - typeExclusions.get(dataType).add(asLong(exclusion.get("DataTypeRowId"))); + typeExclusions.computeIfAbsent(dataType, _ -> new HashSet<>()).add(asLong(exclusion.get("DataTypeRowId"))); } - return typeExclusions; + typeExclusions.replaceAll((_, rowIds) -> Collections.unmodifiableSet(rowIds)); + return Collections.unmodifiableMap(typeExclusions); } @Override diff --git a/list/src/org/labkey/list/model/ListServiceImpl.java b/list/src/org/labkey/list/model/ListServiceImpl.java index 43577d7ad35..85e476f4830 100644 --- a/list/src/org/labkey/list/model/ListServiceImpl.java +++ b/list/src/org/labkey/list/model/ListServiceImpl.java @@ -153,6 +153,16 @@ public ListDefinition getList(Container container, String name, boolean includeP @Override public ListDefinition getList(Domain domain) { + Container c = domain.getContainer(); + if (c != null) + { + for (ListDef def : ListManager.get().getLists(c)) + { + if (def.getDomainId() == domain.getTypeId()) + return new ListDefinitionImpl(def); + } + } + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("domainid"), domain.getTypeId()); ListDef def = new TableSelector(ListManager.get().getListMetadataTable(), filter, null).getObject(ListDef.class); return ListDefinitionImpl.of(def); diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java index 5b19e0eec76..30e24134775 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java @@ -118,6 +118,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -1048,6 +1049,15 @@ public Collection> getActivePipelineJobs(User u, Container c return new TableSelector(PipelineService.get().getJobsTable(u, c, cf), Collections.singleton("Description"), filter, null).getMapCollection(); } + @Override + public Collection> getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Provider"), providerNames, CompareType.IN); + filter.addCondition(FieldKey.fromParts("Status"), INACTIVE_JOB_STATUSES, CompareType.NOT_IN); + + return new TableSelector(PipelineService.get().getJobsTable(u, c, cf), Set.of("Provider", "Description"), filter, null).getMapCollection(); + } + public static class TestCase extends Assert { private static final String PROJECT_NAME = "__PipelineRootTestProject"; From 55a4d39fe9e8de99db1f59f3ac43c4070928825d Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 28 Sep 2026 12:50:10 -0700 Subject: [PATCH 2/6] Clear ontology caches after rewriting lookups on query rename --- .../org/labkey/experiment/PropertyQueryChangeListener.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java b/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java index 00bc2122399..fa6d13989f6 100644 --- a/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java +++ b/experiment/src/org/labkey/experiment/PropertyQueryChangeListener.java @@ -101,6 +101,10 @@ public void queryChanged(User user, Container container, ContainerFilter scope, else updateLookupQuery(newValue, schema, oldValue, container); } + + // Updated lookups can belong to any domain, so a targeted invalidateDomain() won't reach them + if (!queryNameChangeMap.isEmpty()) + OntologyManager.clearCaches(); } @Override From 27f014caedea5e7d689a6149e45a6417c70bd437 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Wed, 30 Sep 2026 21:03:35 -0700 Subject: [PATCH 3/6] Hold plate run container by id so Plate Cache entries pass the cache safety check --- assay/src/org/labkey/assay/plate/PlateImpl.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/PlateImpl.java b/assay/src/org/labkey/assay/plate/PlateImpl.java index 78ecb74883b..390e9fb7dfb 100644 --- a/assay/src/org/labkey/assay/plate/PlateImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateImpl.java @@ -97,7 +97,8 @@ public class PlateImpl extends PropertySetImpl implements Plate, Cloneable private Map _wellMap; private Integer _metadataDomainId; private transient Long _sourcePlateRowId; - private transient @Nullable Container _runContainer; + // Container's id rather than the Container itself, since CacheManager rejects cached values with Container fields + private transient @Nullable String _runContainerId; private transient Map> _replicateDilutionData; private transient Map> _wellGroupDilutionData; @@ -126,7 +127,7 @@ public PlateImpl(Container container, String name, @Nullable String barcode, @No public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, @Nullable ExpRun run, int plateNumber) { this(plate, wellValues, excluded, run == null ? PlateService.NO_RUNID : run.getRowId(), plateNumber); - _runContainer = run == null ? null : run.getContainer(); + _runContainerId = run == null ? null : run.getContainer().getId(); } public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, long runId, int plateNumber) @@ -718,7 +719,9 @@ synchronized List getDilutionDataRows(String groupName, boolean { if (_wellGroupDilutionData == null) { - Container runContainer = _runContainer != null ? _runContainer : ExperimentService.get().getExpRun(_runId).getContainer(); + Container runContainer = _runContainerId != null ? ContainerManager.getForId(_runContainerId) : null; + if (runContainer == null) + runContainer = ExperimentService.get().getExpRun(_runId).getContainer(); Map> replicateRows = new HashMap<>(); Map> wellGroupRows = new HashMap<>(); for (DilutionDataRow row : DilutionManager.getDilutionDataRows(_runId, _plateNumber, runContainer)) From 9d6178854b8272b7b5db7031231a51576aac8b56 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sat, 3 Oct 2026 16:01:50 -0700 Subject: [PATCH 4/6] Address review findings on ontology cache clearing and shared caches Object deletes remove exact property map keys and run one deduped container filter only when unknown URIs were deleted, exclusion lookups cache only real containers, and NAb sample maps are published fully built. --- .../org/labkey/api/exp/OntologyManager.java | 199 +++++++++++++----- .../api/assay/dilution/DilutionSummary.java | 13 +- .../src/org/labkey/assay/plate/PlateImpl.java | 2 +- .../experiment/api/ExperimentServiceImpl.java | 16 +- 4 files changed, 160 insertions(+), 70 deletions(-) diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index b2ff0bfa4dc..a1714b0a73e 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -117,6 +117,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; import java.util.stream.Collectors; import static java.util.Collections.emptySet; @@ -1053,7 +1055,8 @@ public static void deleteOntologyObjects(Container c, String... uris) } finally { - clearPropertyCache(c); + // deleteObject also deletes owned objects, whose URIs aren't known here + clearPropertyCache(c, Arrays.asList(uris), true); OBJECT_ID_CACHE.clear(); } } @@ -1106,68 +1109,74 @@ public static void deleteOntologyObjects(Container c, boolean deleteOwnedObjects try { - // if it's a long list, split it up - if (objectIds.length > 1000) - { - int countBatches = objectIds.length / 1000; - int lenBatch = 1 + objectIds.length / (countBatches + 1); + deleteOntologyObjectsById(c, deleteOwnedObjects, deleteObjectProperties, deleteObjects, objectIds); + } + finally + { + clearPropertyCache(c, List.of(), true); + OBJECT_ID_CACHE.clear(); + } + } - for (int s = 0; s < objectIds.length; s += lenBatch) - { - long[] sub = new long[Math.min(lenBatch, objectIds.length - s)]; - System.arraycopy(objectIds, s, sub, 0, sub.length); - deleteOntologyObjects(c, deleteOwnedObjects, deleteObjectProperties, deleteObjects, sub); - } + /** Deletes without clearing caches, so callers that know the deleted URIs can clear just those */ + private static void deleteOntologyObjectsById(Container c, boolean deleteOwnedObjects, boolean deleteObjectProperties, boolean deleteObjects, long... objectIds) + { + // if it's a long list, split it up + if (objectIds.length > 1000) + { + int countBatches = objectIds.length / 1000; + int lenBatch = 1 + objectIds.length / (countBatches + 1); - return; + for (int s = 0; s < objectIds.length; s += lenBatch) + { + long[] sub = new long[Math.min(lenBatch, objectIds.length - s)]; + System.arraycopy(objectIds, s, sub, 0, sub.length); + deleteOntologyObjectsById(c, deleteOwnedObjects, deleteObjectProperties, deleteObjects, sub); } - SQLFragment objectIdInClause = new SQLFragment(); - getExpSchema().getSqlDialect().appendInClauseSql(objectIdInClause, Arrays.stream(objectIds).boxed().toList()); - - if (deleteOwnedObjects) - { - // NOTE: owned objects should never be in a different container than the owner, that would be a problem - SQLFragment sqlDeleteOwnedProperties = new SQLFragment("DELETE FROM ") - .append(getTinfoObjectProperty()) - .append(" WHERE ObjectId IN (SELECT ObjectId FROM ") - .append(getTinfoObject()) - .append(" WHERE Container = ? AND OwnerObjectId ") - .add(c) - .append(objectIdInClause) - .append(")"); + return; + } - new SqlExecutor(getExpSchema()).execute(sqlDeleteOwnedProperties); + SQLFragment objectIdInClause = new SQLFragment(); + getExpSchema().getSqlDialect().appendInClauseSql(objectIdInClause, Arrays.stream(objectIds).boxed().toList()); - SQLFragment sqlDeleteOwnedObjects = new SQLFragment("DELETE FROM ") - .append(getTinfoObject()) - .append(" WHERE Container = ? AND OwnerObjectId ") - .add(c) - .append(objectIdInClause); + if (deleteOwnedObjects) + { + // NOTE: owned objects should never be in a different container than the owner, that would be a problem + SQLFragment sqlDeleteOwnedProperties = new SQLFragment("DELETE FROM ") + .append(getTinfoObjectProperty()) + .append(" WHERE ObjectId IN (SELECT ObjectId FROM ") + .append(getTinfoObject()) + .append(" WHERE Container = ? AND OwnerObjectId ") + .add(c) + .append(objectIdInClause) + .append(")"); - new SqlExecutor(getExpSchema()).execute(sqlDeleteOwnedObjects); - } + new SqlExecutor(getExpSchema()).execute(sqlDeleteOwnedProperties); - if (deleteObjectProperties) - { - deleteProperties(c, objectIdInClause); - } + SQLFragment sqlDeleteOwnedObjects = new SQLFragment("DELETE FROM ") + .append(getTinfoObject()) + .append(" WHERE Container = ? AND OwnerObjectId ") + .add(c) + .append(objectIdInClause); - if (deleteObjects) - { - SQLFragment sqlDeleteObjects = new SQLFragment("DELETE FROM ") - .append(getTinfoObject()) - .append(" WHERE Container = ? AND ObjectId ") - .add(c) - .append(objectIdInClause); + new SqlExecutor(getExpSchema()).execute(sqlDeleteOwnedObjects); + } - new SqlExecutor(getExpSchema()).execute(sqlDeleteObjects); - } + if (deleteObjectProperties) + { + deleteProperties(c, objectIdInClause); } - finally + + if (deleteObjects) { - clearPropertyCache(c); - OBJECT_ID_CACHE.clear(); + SQLFragment sqlDeleteObjects = new SQLFragment("DELETE FROM ") + .append(getTinfoObject()) + .append(" WHERE Container = ? AND ObjectId ") + .add(c) + .append(objectIdInClause); + + new SqlExecutor(getExpSchema()).execute(sqlDeleteObjects); } } @@ -1178,7 +1187,15 @@ public static void deleteOntologyObject(String objectURI, Container container, b if (null != ontologyObject) { - deleteOntologyObjects(container, deleteOwnedObjects, true, true, ontologyObject.getObjectId()); + try + { + deleteOntologyObjectsById(container, deleteOwnedObjects, true, true, ontologyObject.getObjectId()); + } + finally + { + clearPropertyCache(container, List.of(objectURI), deleteOwnedObjects); + OBJECT_ID_CACHE.clear(); + } } } @@ -2951,10 +2968,33 @@ public static void clearPropertyCache(String parentObjectURI) } - /** Owned objects share their owner's container, so this also covers deleted children whose URIs aren't known */ - private static void clearPropertyCache(Container c) + /** + * Removes deleted objects' property maps. Pass deletedUnknownUris when objects outside deletedUris were also + * deleted (owned children, or deletes by object id). + */ + private static void clearPropertyCache(Container c, Collection deletedUris, boolean deletedUnknownUris) { - PROPERTY_MAP_CACHE.removeUsingFilter(key -> key.first == null || key.first.equals(c)); + // Unlike a filter, remove() marks the key even when it isn't cached, so a deleting transaction never reads + // a pre-delete map that another thread caches later + for (String uri : deletedUris) + { + PROPERTY_MAP_CACHE.remove(getPropertyMapCacheKey(c, uri)); + PROPERTY_MAP_CACHE.remove(getPropertyMapCacheKey(null, uri)); + } + + // Owned objects share their owner's container, so this covers deleted objects whose URIs aren't known + if (deletedUnknownUris) + PROPERTY_MAP_CACHE.removeUsingFilter(new ContainerPropertyMapKeys(c)); + } + + /** A record rather than a lambda so a transaction's equal post-commit removal tasks dedupe */ + private record ContainerPropertyMapKeys(Container c) implements Predicate> + { + @Override + public boolean test(Pair key) + { + return key.first == null || c.equals(key.first); + } } public static void clearPropertyCache() @@ -3145,6 +3185,55 @@ public void testBasicPropertiesObject() throws ValidationException assertEquals(0, m.size()); } + @Test + public void testDeleteClearsPropertyMapCacheInTransaction() throws Exception + { + User user = TestContext.get().getUser(); + Container c = ContainerManager.ensureContainer("/_ontologyManagerTest", user); + String parentObjectLsid = new Lsid("Junit", "OntologyManager", "cacheParent").toString(); + String childObjectLsid = new Lsid("Junit", "OntologyManager", "cacheChild").toString(); + String strProp = new Lsid("Junit", "OntologyManager", "cacheStringProp").toString(); + DbScope scope = getExpSchema().getScope(); + + deleteOntologyObjects(c, parentObjectLsid); + try + { + ensureObject(c, childObjectLsid, parentObjectLsid); + insertProperties(c, user, parentObjectLsid, new ObjectProperty(childObjectLsid, c, strProp, "Cached")); + assertEquals(1, getPropertyObjects(c, childObjectLsid).size()); + + // Deleting the owner must also drop the cached map of its child, whose URI isn't passed in + try (Transaction ignored = scope.ensureTransaction()) + { + deleteOntologyObject(parentObjectLsid, c, true); + assertTrue(getPropertyObjects(c, childObjectLsid).isEmpty()); + } + + // Uncached at delete time, then cached by another thread that still sees the committed rows + clearPropertyCache(childObjectLsid); + try (Transaction tx = scope.ensureTransaction()) + { + deleteOntologyObject(childObjectLsid, c, false); + + AtomicReference> otherThreadProps = new AtomicReference<>(); + Thread otherThread = new Thread(() -> otherThreadProps.set(getPropertyObjects(c, childObjectLsid))); + otherThread.start(); + otherThread.join(); + assertNotNull(otherThreadProps.get()); + assertEquals(1, otherThreadProps.get().size()); + + assertTrue(getPropertyObjects(c, childObjectLsid).isEmpty()); + tx.commit(); + } + + assertTrue(getPropertyObjects(c, childObjectLsid).isEmpty()); + } + finally + { + deleteOntologyObjects(c, parentObjectLsid); + } + } + @Test public void testContainerDelete() throws ValidationException { diff --git a/assay/api-src/org/labkey/api/assay/dilution/DilutionSummary.java b/assay/api-src/org/labkey/api/assay/dilution/DilutionSummary.java index 7d724a9b8f4..b5db682040d 100644 --- a/assay/api-src/org/labkey/api/assay/dilution/DilutionSummary.java +++ b/assay/api-src/org/labkey/api/assay/dilution/DilutionSummary.java @@ -148,21 +148,24 @@ public String getSampleDescription() return (String) _firstGroup.getProperty(SampleProperty.SampleDescription.name()); } - private Map _dataToSample; + private volatile Map _dataToSample; private Map getDataToSampleMap() { - if (_dataToSample == null) + Map dataToSample = _dataToSample; + if (dataToSample == null) { - _dataToSample = new HashMap<>(); + // Populate before publishing, since cached runs are shared by concurrent graph requests + dataToSample = new HashMap<>(); for (WellGroup sampleGroup : _sampleGroups) { for (WellData data : sampleGroup.getWellData(true)) { - _dataToSample.put(data, sampleGroup); + dataToSample.put(data, sampleGroup); } } + _dataToSample = dataToSample; } - return _dataToSample; + return dataToSample; } public double getPercent(WellData data) throws FitFailedException diff --git a/assay/src/org/labkey/assay/plate/PlateImpl.java b/assay/src/org/labkey/assay/plate/PlateImpl.java index 390e9fb7dfb..84ba7809763 100644 --- a/assay/src/org/labkey/assay/plate/PlateImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateImpl.java @@ -123,13 +123,13 @@ public PlateImpl(Container container, String name, @Nullable String barcode, @No this(container, name, barcode, null, plateType); } - // Note that barcode values will be auto-generated public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, @Nullable ExpRun run, int plateNumber) { this(plate, wellValues, excluded, run == null ? PlateService.NO_RUNID : run.getRowId(), plateNumber); _runContainerId = run == null ? null : run.getContainer().getId(); } + // Note that barcode values will be auto-generated public PlateImpl(@NotNull PlateImpl plate, double[][] wellValues, boolean[][] excluded, long runId, int plateNumber) { this(plate.getContainer(), plate.getName(), null, plate.getAssayType(), plate.getPlateType()); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 6e1b39d2e81..1de1eed4e3b 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9047,16 +9047,14 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi @Override public @NotNull Map> getContainerDataTypeExclusions(@NotNull String excludedContainerIdOrPath) { - String excludedContainerId = excludedContainerIdOrPath; - if (!GUID.isGUID(excludedContainerIdOrPath)) - { - Container container = ContainerManager.getForPath(excludedContainerIdOrPath); - if (container == null) - return Collections.emptyMap(); - excludedContainerId = container.getId(); - } + // Resolve to a real container so arbitrary client-supplied ids can't add cache entries + Container container = GUID.isGUID(excludedContainerIdOrPath) + ? ContainerManager.getForId(excludedContainerIdOrPath) + : ContainerManager.getForPath(excludedContainerIdOrPath); + if (container == null) + return Collections.emptyMap(); - return DATA_TYPE_EXCLUSION_CACHE.get(excludedContainerId); + return DATA_TYPE_EXCLUSION_CACHE.get(container.getId()); } private @NotNull Map> loadContainerDataTypeExclusions(@NotNull String excludedContainerId) From 81d62c7783f1e47b176ebc1965165f1a2b8c4b27 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 4 Oct 2026 15:00:04 -0700 Subject: [PATCH 5/6] Use records, improve comments --- .../api/audit/data/ExperimentAuditColumn.java | 25 +++++++++++-------- .../labkey/api/audit/data/ProtocolColumn.java | 5 ++-- .../org/labkey/api/audit/data/RunColumn.java | 5 ++-- .../labkey/api/audit/data/RunGroupColumn.java | 7 +++--- .../api/exp/property/DomainAuditProvider.java | 9 ++++--- .../labkey/api/pipeline/PipelineService.java | 5 ++-- .../labkey/list/model/ListServiceImpl.java | 1 + .../pipeline/api/PipelineServiceImpl.java | 4 +-- 8 files changed, 32 insertions(+), 29 deletions(-) diff --git a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java index 0bc04e847d6..66905fe5fda 100644 --- a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java +++ b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java @@ -24,7 +24,6 @@ import org.labkey.api.exp.api.ExpObject; import org.labkey.api.util.HtmlString; import org.labkey.api.util.LinkBuilder; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import org.labkey.api.writer.HtmlWriter; @@ -37,11 +36,15 @@ public abstract class ExperimentAuditColumn extend { protected ColumnInfo _containerId; protected ColumnInfo _defaultName; - // Keyed by bound value + container id, since the same object typically repeats across rows - private final Map, Optional>> _expValues = new HashMap<>(); + // The same object typically repeats across rows + private final Map>> _expValues = new HashMap<>(); public static final String KEY_SEPARATOR = "~~KEYSEP~~"; + protected record ExpLink(T object, @Nullable ActionURL url) {} + + private record CacheKey(Object boundValue, @Nullable String containerId) {} + public ExperimentAuditColumn(ColumnInfo col, ColumnInfo containerId, ColumnInfo defaultName) { super(col); @@ -66,23 +69,23 @@ protected Container getContainer(RenderContext ctx) } @Nullable - protected abstract Pair getExpValue(RenderContext ctx); + protected abstract ExpLink getExpValue(RenderContext ctx); @Nullable - private Pair getCachedExpValue(RenderContext ctx) + private ExpLink getCachedExpValue(RenderContext ctx) { Container c = getContainer(ctx); - Pair key = Pair.of(getBoundColumn().getValue(ctx), c == null ? null : c.getId()); + CacheKey key = new CacheKey(getBoundColumn().getValue(ctx), c == null ? null : c.getId()); return _expValues.computeIfAbsent(key, _ -> Optional.ofNullable(getExpValue(ctx))).orElse(null); } @Override public Object getDisplayValue(RenderContext ctx) { - Pair value = getCachedExpValue(ctx); + ExpLink value = getCachedExpValue(ctx); if (value != null) { - return value.first.getName(); + return value.object().getName(); } if (_defaultName != null) @@ -114,10 +117,10 @@ public boolean isFilterable() @Override public void renderGridCellContents(RenderContext ctx, HtmlWriter out) { - Pair value = getCachedExpValue(ctx); - if (value != null && value.second != null) + ExpLink value = getCachedExpValue(ctx); + if (value != null && value.url() != null) { - out.write(LinkBuilder.simpleLink(value.first.getName(), value.second)); + out.write(LinkBuilder.simpleLink(value.object().getName(), value.url())); return; } diff --git a/api/src/org/labkey/api/audit/data/ProtocolColumn.java b/api/src/org/labkey/api/audit/data/ProtocolColumn.java index 2f78d85fc6c..250addff882 100644 --- a/api/src/org/labkey/api/audit/data/ProtocolColumn.java +++ b/api/src/org/labkey/api/audit/data/ProtocolColumn.java @@ -26,7 +26,6 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.ExperimentUrls; import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import static org.labkey.api.util.IntegerUtils.asLongElseNull; @@ -44,7 +43,7 @@ public ProtocolColumn(ColumnInfo col, ColumnInfo containerId, @Nullable ColumnIn @Nullable @Override - protected Pair getExpValue(RenderContext ctx) + protected ExpLink getExpValue(RenderContext ctx) { Object protocolId = getBoundColumn().getValue(ctx); @@ -69,7 +68,7 @@ protected Pair getExpValue(RenderContext ctx) url = PageFlowUtil.urlProvider(AssayUrls.class).getAssayRunsURL(c, protocol); else if (protocol != null) url = PageFlowUtil.urlProvider(ExperimentUrls.class).getProtocolDetailsURL(protocol); - return protocol == null ? null : new Pair<>(protocol, url); + return protocol == null ? null : new ExpLink<>(protocol, url); } } return null; diff --git a/api/src/org/labkey/api/audit/data/RunColumn.java b/api/src/org/labkey/api/audit/data/RunColumn.java index 19861d5012f..5f788de98cb 100644 --- a/api/src/org/labkey/api/audit/data/RunColumn.java +++ b/api/src/org/labkey/api/audit/data/RunColumn.java @@ -27,7 +27,6 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.ExperimentUrls; import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; /** @@ -55,7 +54,7 @@ protected String extractFromKey3(RenderContext ctx) @Override @Nullable - protected Pair getExpValue(RenderContext ctx) + protected ExpLink getExpValue(RenderContext ctx) { String runLsid = (String) getBoundColumn().getValue(ctx); if (runLsid != null) @@ -77,7 +76,7 @@ protected Pair getExpValue(RenderContext ctx) else if (run != null) url = PageFlowUtil.urlProvider(ExperimentUrls.class).getRunGraphURL(run); - return run == null ? null : new Pair<>(run, url); + return run == null ? null : new ExpLink<>(run, url); } } return null; diff --git a/api/src/org/labkey/api/audit/data/RunGroupColumn.java b/api/src/org/labkey/api/audit/data/RunGroupColumn.java index e44660a686b..8dc05fdef53 100644 --- a/api/src/org/labkey/api/audit/data/RunGroupColumn.java +++ b/api/src/org/labkey/api/audit/data/RunGroupColumn.java @@ -23,14 +23,13 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.ExperimentUrls; import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; /** * User: klum * Date: Mar 15, 2012 */ -public class RunGroupColumn extends ExperimentAuditColumn +public class RunGroupColumn extends ExperimentAuditColumn { public RunGroupColumn(ColumnInfo col, ColumnInfo containerId, @Nullable ColumnInfo defaultName) { @@ -39,7 +38,7 @@ public RunGroupColumn(ColumnInfo col, ColumnInfo containerId, @Nullable ColumnIn @Nullable @Override - protected Pair getExpValue(RenderContext ctx) + protected ExpLink getExpValue(RenderContext ctx) { Object rowId = getBoundColumn().getValue(ctx); if (rowId != null) @@ -53,7 +52,7 @@ protected Pair getExpValue(RenderContext ctx) if (runGroup != null) url = PageFlowUtil.urlProvider(ExperimentUrls.class).getExperimentDetailsURL(c, runGroup); - return runGroup == null ? null : new Pair<>(runGroup, url); + return runGroup == null ? null : new ExpLink<>(runGroup, url); } } return null; diff --git a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index 17861b283cc..d71b85e076f 100644 --- a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java @@ -38,7 +38,6 @@ import org.labkey.api.query.UserSchema; import org.labkey.api.util.HtmlString; import org.labkey.api.util.LinkBuilder; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import org.labkey.api.writer.DefaultContainerUser; import org.labkey.api.writer.HtmlWriter; @@ -226,8 +225,10 @@ public static class DomainColumn extends DataColumn private final String _containerColumnName; @NotNull private final String _defaultNameColumnName; - // Keyed by container id + domain URI; empty when the domain no longer exists - private final Map, Optional> _domainLinks = new HashMap<>(); + // Empty when the domain no longer exists + private final Map> _domainLinks = new HashMap<>(); + + private record DomainKey(String containerId, String domainURI) {} private record DomainLink(String name, @Nullable ActionURL url, boolean hasKind) {} @@ -264,7 +265,7 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out) if (uri != null && cId != null) { - Optional link = _domainLinks.computeIfAbsent(Pair.of(cId, uri), _ -> getDomainLink(ctx, cId, uri)); + Optional link = _domainLinks.computeIfAbsent(new DomainKey(cId, uri), _ -> getDomainLink(ctx, cId, uri)); if (link.isPresent()) { if (link.get().hasKind()) diff --git a/api/src/org/labkey/api/pipeline/PipelineService.java b/api/src/org/labkey/api/pipeline/PipelineService.java index 1fde0286c5e..d1b0b2518ee 100644 --- a/api/src/org/labkey/api/pipeline/PipelineService.java +++ b/api/src/org/labkey/api/pipeline/PipelineService.java @@ -266,8 +266,9 @@ public AbstractFileAnalysisProtocolFactory getFactory() Collection> getActivePipelineJobs(User u, Container c, String providerName, @Nullable ContainerFilter cf); - /** Provider and Description of the active jobs for any of the given providers */ - Collection> getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf); + record ActiveJob(String provider, String description) {} + + List getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf); interface PipelineProviderSupplier { diff --git a/list/src/org/labkey/list/model/ListServiceImpl.java b/list/src/org/labkey/list/model/ListServiceImpl.java index 85e476f4830..88d8a3439f5 100644 --- a/list/src/org/labkey/list/model/ListServiceImpl.java +++ b/list/src/org/labkey/list/model/ListServiceImpl.java @@ -153,6 +153,7 @@ public ListDefinition getList(Container container, String name, boolean includeP @Override public ListDefinition getList(Domain domain) { + // Check the container's cached list definitions first to avoid a DB query Container c = domain.getContainer(); if (c != null) { diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java index 30e24134775..a8002a045e1 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java @@ -1050,12 +1050,12 @@ public Collection> getActivePipelineJobs(User u, Container c } @Override - public Collection> getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf) + public List getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Provider"), providerNames, CompareType.IN); filter.addCondition(FieldKey.fromParts("Status"), INACTIVE_JOB_STATUSES, CompareType.NOT_IN); - return new TableSelector(PipelineService.get().getJobsTable(u, c, cf), Set.of("Provider", "Description"), filter, null).getMapCollection(); + return new TableSelector(PipelineService.get().getJobsTable(u, c, cf), Set.of("Provider", "Description"), filter, null).getArrayList(ActiveJob.class); } public static class TestCase extends Assert From 0f83668592189e3c5e228f7f7370aa61603216e9 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Tue, 6 Oct 2026 17:41:23 -0700 Subject: [PATCH 6/6] Address PR #8130 review feedback Use ContainerManager.getForPath for id-or-path resolution, short-circuit empty provider lists, bound and lighten the audit column memo, and evict the stale old-URI property descriptor cache entry when a property URI changes. --- .../api/audit/data/ExperimentAuditColumn.java | 32 +++++++++++++------ .../org/labkey/api/exp/OntologyManager.java | 6 ++++ .../experiment/api/ExperimentServiceImpl.java | 4 +-- .../api/property/DomainPropertyImpl.java | 3 ++ .../pipeline/api/PipelineServiceImpl.java | 3 ++ 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java index 66905fe5fda..464b5a3c174 100644 --- a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java +++ b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java @@ -27,22 +27,33 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.writer.HtmlWriter; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.Set; public abstract class ExperimentAuditColumn extends DataColumn { + // The same object typically repeats across rows, usually on adjacent ones; cache only its name and URL, and cap the memo, so a large export can't pin one ExpObject per distinct row + private static final int MAX_CACHED_VALUES = 1000; + protected ColumnInfo _containerId; protected ColumnInfo _defaultName; - // The same object typically repeats across rows - private final Map>> _expValues = new HashMap<>(); + private final Map> _expValues = new LinkedHashMap<>(16, 0.75f, true) + { + @Override + protected boolean removeEldestEntry(Map.Entry> eldest) + { + return size() > MAX_CACHED_VALUES; + } + }; public static final String KEY_SEPARATOR = "~~KEYSEP~~"; protected record ExpLink(T object, @Nullable ActionURL url) {} + private record ExpLinkDisplay(String name, @Nullable ActionURL url) {} + private record CacheKey(Object boundValue, @Nullable String containerId) {} public ExperimentAuditColumn(ColumnInfo col, ColumnInfo containerId, ColumnInfo defaultName) @@ -72,20 +83,23 @@ protected Container getContainer(RenderContext ctx) protected abstract ExpLink getExpValue(RenderContext ctx); @Nullable - private ExpLink getCachedExpValue(RenderContext ctx) + private ExpLinkDisplay getCachedExpValue(RenderContext ctx) { Container c = getContainer(ctx); CacheKey key = new CacheKey(getBoundColumn().getValue(ctx), c == null ? null : c.getId()); - return _expValues.computeIfAbsent(key, _ -> Optional.ofNullable(getExpValue(ctx))).orElse(null); + return _expValues.computeIfAbsent(key, _ -> { + ExpLink link = getExpValue(ctx); + return Optional.ofNullable(link == null ? null : new ExpLinkDisplay(link.object().getName(), link.url())); + }).orElse(null); } @Override public Object getDisplayValue(RenderContext ctx) { - ExpLink value = getCachedExpValue(ctx); + ExpLinkDisplay value = getCachedExpValue(ctx); if (value != null) { - return value.object().getName(); + return value.name(); } if (_defaultName != null) @@ -117,10 +131,10 @@ public boolean isFilterable() @Override public void renderGridCellContents(RenderContext ctx, HtmlWriter out) { - ExpLink value = getCachedExpValue(ctx); + ExpLinkDisplay value = getCachedExpValue(ctx); if (value != null && value.url() != null) { - out.write(LinkBuilder.simpleLink(value.object().getName(), value.url())); + out.write(LinkBuilder.simpleLink(value.name(), value.url())); return; } diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index a1714b0a73e..d5de44e5afe 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -2753,6 +2753,12 @@ public static PropertyDescriptor insertPropertyDescriptor(PropertyDescriptor pd) return pd; } + // For callers that write a PropertyDescriptor with a raw Table.update and so bypass the cache eviction here + public static void clearPropertyDescriptorCache(PropertyDescriptor pd) + { + PROP_DESCRIPTOR_CACHE.remove(getCacheKey(pd)); + } + //todo: we automatically update a pd to the last one in? public static PropertyDescriptor updatePropertyDescriptor(PropertyDescriptor pd) { diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 1de1eed4e3b..34afbac8177 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9048,9 +9048,7 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi public @NotNull Map> getContainerDataTypeExclusions(@NotNull String excludedContainerIdOrPath) { // Resolve to a real container so arbitrary client-supplied ids can't add cache entries - Container container = GUID.isGUID(excludedContainerIdOrPath) - ? ContainerManager.getForId(excludedContainerIdOrPath) - : ContainerManager.getForPath(excludedContainerIdOrPath); + Container container = ContainerManager.getForPath(excludedContainerIdOrPath); if (container == null) return Collections.emptyMap(); diff --git a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java index e2467a0097c..fd9b5403892 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java @@ -870,6 +870,9 @@ else if (newType == PropertyType.MULTI_CHOICE || oldType == PropertyType.MULTI_C OntologyManager.validatePropertyDescriptor(_pd); Table.update(user, OntologyManager.getTinfoPropertyDescriptor(), _pd, _pdOld.getPropertyId()); + // The raw update above bypasses the cache; a changed URI leaves a stale entry under the old key that invalidateDomain() can't find + if (!_pdOld.getPropertyURI().equals(_pd.getPropertyURI())) + OntologyManager.clearPropertyDescriptorCache(_pdOld); OntologyManager.ensurePropertyDomain(_pd, dd, sortOrder); boolean hasProvisioner = null != getDomain().getDomainKind() && null != getDomain().getDomainKind().getStorageSchemaName() && dd.getStorageTableName() != null; diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java index a8002a045e1..e98af5ad163 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineServiceImpl.java @@ -1052,6 +1052,9 @@ public Collection> getActivePipelineJobs(User u, Container c @Override public List getActivePipelineJobs(User u, Container c, Collection providerNames, @Nullable ContainerFilter cf) { + if (providerNames.isEmpty()) + return List.of(); + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Provider"), providerNames, CompareType.IN); filter.addCondition(FieldKey.fromParts("Status"), INACTIVE_JOB_STATUSES, CompareType.NOT_IN);