diff --git a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java index 087373e5bce..66905fe5fda 100644 --- a/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java +++ b/api/src/org/labkey/api/audit/data/ExperimentAuditColumn.java @@ -24,19 +24,27 @@ 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; +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; + // 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); @@ -61,15 +69,23 @@ protected Container getContainer(RenderContext ctx) } @Nullable - protected abstract Pair getExpValue(RenderContext ctx); + protected abstract ExpLink getExpValue(RenderContext ctx); + + @Nullable + private ExpLink 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); + } @Override public Object getDisplayValue(RenderContext ctx) { - Pair value = getExpValue(ctx); + ExpLink value = getCachedExpValue(ctx); if (value != null) { - return value.first.getName(); + return value.object().getName(); } if (_defaultName != null) @@ -101,10 +117,10 @@ public boolean isFilterable() @Override public void renderGridCellContents(RenderContext ctx, HtmlWriter out) { - Pair value = getExpValue(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/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..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 { - PROPERTY_MAP_CACHE.clear(); + // 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) { - PROPERTY_MAP_CACHE.clear(); - 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(); + } } } @@ -2916,8 +2933,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 +2968,35 @@ public static void clearPropertyCache(String parentObjectURI) } + /** + * 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) + { + // 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() { PROPERTY_MAP_CACHE.clear(); @@ -3128,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/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index 3c2a4fcd71e..d71b85e076f 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,18 @@ import org.labkey.api.query.UserSchema; import org.labkey.api.util.HtmlString; import org.labkey.api.util.LinkBuilder; +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 +225,12 @@ public static class DomainColumn extends DataColumn private final String _containerColumnName; @NotNull private final String _defaultNameColumnName; + // 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) {} public DomainColumn(@NotNull ColumnInfo col, @NotNull String containerColumnName, @NotNull String defaultNameColumnName) { @@ -255,19 +265,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(new DomainKey(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 +283,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..d1b0b2518ee 100644 --- a/api/src/org/labkey/api/pipeline/PipelineService.java +++ b/api/src/org/labkey/api/pipeline/PipelineService.java @@ -266,6 +266,10 @@ public AbstractFileAnalysisProtocolFactory getFactory() Collection> getActivePipelineJobs(User u, Container c, String providerName, @Nullable ContainerFilter cf); + record ActiveJob(String provider, String description) {} + + List 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/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/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..84ba7809763 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,10 @@ public class PlateImpl extends PropertySetImpl implements Plate, Cloneable private Map _wellMap; private Integer _metadataDomainId; private transient Long _sourcePlateRowId; + // 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; // no-param constructor for reflection public PlateImpl() @@ -115,6 +123,12 @@ public PlateImpl(Container container, String name, @Nullable String barcode, @No this(container, name, barcode, null, plateType); } + 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) { @@ -700,6 +714,30 @@ 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 = _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)) + { + 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/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 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..1de1eed4e3b 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,19 @@ private void removeDataTypeExclusion(Collection rowIds, DataTypeForExclusi } @Override - public @NotNull Map> getContainerDataTypeExclusions(@NotNull String excludedContainerId) + 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); + if (container == null) + return Collections.emptyMap(); + + return DATA_TYPE_EXCLUSION_CACHE.get(container.getId()); + } + + private @NotNull Map> loadContainerDataTypeExclusions(@NotNull String excludedContainerId) { List> exclusions = _getContainerDataTypeExclusions(null, excludedContainerId, null); @@ -9043,12 +9066,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..88d8a3439f5 100644 --- a/list/src/org/labkey/list/model/ListServiceImpl.java +++ b/list/src/org/labkey/list/model/ListServiceImpl.java @@ -153,6 +153,17 @@ 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) + { + 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..a8002a045e1 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 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).getArrayList(ActiveJob.class); + } + public static class TestCase extends Assert { private static final String PROJECT_NAME = "__PipelineRootTestProject";