diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index ce7c9b7bc82..88f5327c974 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -30,6 +30,7 @@ import org.labkey.api.assay.AssayResultsFileWriter; import org.labkey.api.assay.ReplacedRunFilter; import org.labkey.api.assay.sample.MaterialInputRoleComparator; +import org.labkey.api.assay.transform.PreparedTransform; import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.ImageServlet; import org.labkey.api.attachments.LookAndFeelResourceType; @@ -589,6 +590,7 @@ public void registerServlets(ServletContext servletCtx) ParameterSubstitutionTest.class, Portal.TestCase.class, PostgresSnapshot.TestCase.class, + PreparedTransform.TestCase.class, PropertyManager.TestCase.class, QueryDataIteratorBuilder.TestCase.class, QueryProfiler.ExecutionPlanUnavailableTestCase.class, diff --git a/api/src/org/labkey/api/assay/AssayFileWriter.java b/api/src/org/labkey/api/assay/AssayFileWriter.java index 087a2378f05..c15f72e257e 100644 --- a/api/src/org/labkey/api/assay/AssayFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayFileWriter.java @@ -20,6 +20,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.labkey.api.assay.transform.PendingFileAudit; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.collections.CollectionUtils; @@ -262,7 +263,11 @@ public Map savePostedFiles(ContextType context, @NotNull Set pending = context.getPendingFileAudits(); + if (pending != null) + pending.add(new PendingFileAudit(event, file)); + else + AuditLogService.get().addEvent(context.getUser(), event); } if (!isAfterFirstFile) // first file gets stored with multipartFile's name { diff --git a/api/src/org/labkey/api/assay/AssayRunCreator.java b/api/src/org/labkey/api/assay/AssayRunCreator.java index 01b108ca878..ef8706b3739 100644 --- a/api/src/org/labkey/api/assay/AssayRunCreator.java +++ b/api/src/org/labkey/api/assay/AssayRunCreator.java @@ -55,10 +55,10 @@ Pair saveExperimentRun(AssayRunUploadContext context, boolean forceAsync) + default @Nullable PreparedTransform prepareTransform(AssayRunUploadContext context) throws ExperimentException, ValidationException { return null; diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContext.java b/api/src/org/labkey/api/assay/AssayRunUploadContext.java index 4ac118c772c..3fa23381de1 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContext.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContext.java @@ -28,6 +28,7 @@ import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.property.DomainProperty; +import org.labkey.api.assay.transform.PendingFileAudit; import org.labkey.api.assay.transform.PreparedTransform; import org.labkey.api.assay.transform.TransformResult; import org.labkey.api.security.User; @@ -37,6 +38,7 @@ import org.labkey.api.writer.ContainerUser; import org.labkey.vfs.FileLike; +import java.util.List; import java.util.Map; import java.util.Set; @@ -188,6 +190,17 @@ default void setPreparedTransform(@Nullable PreparedTransform prepared) throw new UnsupportedOperationException(getClass().getName() + " can't carry a prepared transform"); } + /** Audits of files saved while a transform is prepared, or null to add each event as its file is saved. */ + default @Nullable List getPendingFileAudits() + { + return null; + } + + /** Contexts that can't hold audits ignore this and add each event as its file is saved. */ + default void setPendingFileAudits(@Nullable List audits) + { + } + /** The RowId for the run that is being deleted and reuploaded, or null if this is a new run */ Long getReRunId(); diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java index 35351036bfe..0dce81f7ee7 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java @@ -32,6 +32,7 @@ import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.assay.transform.DefaultTransformResult; +import org.labkey.api.assay.transform.PendingFileAudit; import org.labkey.api.assay.transform.PreparedTransform; import org.labkey.api.assay.transform.TransformResult; import org.labkey.api.security.User; @@ -95,6 +96,7 @@ public class AssayRunUploadContextImpl imple // Mutable fields private TransformResult _transformResult; private @Nullable PreparedTransform _preparedTransform; + private @Nullable List _pendingFileAudits; // async fields protected String _jobDescription; @@ -453,6 +455,18 @@ public void setPreparedTransform(@Nullable PreparedTransform prepared) _preparedTransform = prepared; } + @Override + public @Nullable List getPendingFileAudits() + { + return _pendingFileAudits; + } + + @Override + public void setPendingFileAudits(@Nullable List audits) + { + _pendingFileAudits = audits; + } + @Override public void uploadComplete(ExpRun run) { diff --git a/api/src/org/labkey/api/assay/AssaySaveHandler.java b/api/src/org/labkey/api/assay/AssaySaveHandler.java index 6102e101a2c..091ddf2da56 100644 --- a/api/src/org/labkey/api/assay/AssaySaveHandler.java +++ b/api/src/org/labkey/api/assay/AssaySaveHandler.java @@ -16,7 +16,16 @@ package org.labkey.api.assay; +import org.jetbrains.annotations.NotNull; +import org.json.JSONObject; +import org.labkey.api.assay.transform.PreparedTransforms; +import org.labkey.api.exp.ExperimentException; +import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExperimentSaveHandler; +import org.labkey.api.query.ValidationException; +import org.labkey.api.view.ViewContext; + +import java.util.List; /** * User: dax @@ -64,4 +73,14 @@ public interface AssaySaveHandler extends ExperimentSaveHandler */ void setProvider(AssayProvider provider); AssayProvider getProvider(); + + /** + * Runs the transform scripts of the given runs before the save's transaction opens, so a script's callbacks into + * the server can't deadlock with it. The handler records the outputs as it saves each run; runs it doesn't prepare + * run their scripts inside the transaction. Don't rely on beforeSave having run: saveAssayRuns never calls it. + */ + default @NotNull PreparedTransforms prepareTransforms(ViewContext context, ExpProtocol protocol, List runJsons) throws ExperimentException, ValidationException + { + return new PreparedTransforms(); + } } diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index bda56e87bdd..2889e5c849f 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -29,6 +29,7 @@ import org.labkey.api.assay.pipeline.AssayUploadPipelineJob; import org.labkey.api.assay.sample.AssaySampleLookupContext; import org.labkey.api.assay.transform.DataTransformService; +import org.labkey.api.assay.transform.PendingFileAudit; import org.labkey.api.assay.transform.PreparedTransform; import org.labkey.api.assay.transform.TransformDataHandler; import org.labkey.api.assay.transform.TransformResult; @@ -134,9 +135,9 @@ public TransformResult transform(AssayRunUploadContext context, Ex } @Override - public @Nullable PreparedTransform prepareTransform(AssayRunUploadContext context, boolean forceAsync) throws ExperimentException, ValidationException + public @Nullable PreparedTransform prepareTransform(AssayRunUploadContext context) throws ExperimentException, ValidationException { - if (isBackgroundImport(context.getProvider(), context.getProtocol(), forceAsync) || !hasTransformBeforeRun(context)) + if (!hasTransformBeforeRun(context)) return null; context.init(); @@ -148,11 +149,32 @@ private PreparedTransform prepareTransformAfterInit(AssayRunUploadContext allProperties = new HashMap<>(context.getRunProperties()); allProperties.putAll(context.getBatchProperties()); - Map inputDatas = new HashMap<>(); - addRunDataInputs(context, null, findResolverType(allProperties), inputDatas); - PreparedTransform prepared = runTransformBeforeRun(context, inputDatas.keySet()); - // Run creation reuses these rather than collecting again, since a thaw list resolver writes a new file each time - prepared.setRunDataInputs(inputDatas); + // Collecting the inputs saves posted files, whose audit events belong to the import transaction that isn't open yet + List fileAudits = new ArrayList<>(); + context.setPendingFileAudits(fileAudits); + Map resolverInputs = new HashMap<>(); + PreparedTransform prepared; + try + { + ParticipantVisitResolverType resolverType = findResolverType(allProperties); + if (resolverType != null) + resolverType.configureRun(context, null, resolverInputs); + Map inputDatas = new HashMap<>(resolverInputs); + addInputDatas(context, inputDatas); + prepared = runTransformBeforeRun(context, inputDatas.keySet()); + } + catch (Exception e) + { + PreparedTransform.addFileAuditEventsForRemainingFiles(context.getUser(), fileAudits); + throw e; + } + finally + { + context.setPendingFileAudits(null); + } + prepared.holdFileAudits(context.getUser(), fileAudits); + // Run creation reuses these rather than resolving again, since a thaw list resolver writes a new file each time + prepared.setResolverDataInputs(resolverInputs); return prepared; } @@ -165,18 +187,6 @@ private PreparedTransform prepareTransformAfterInit(AssayRunUploadContext context, - @Nullable ExpRun run, - @Nullable ParticipantVisitResolverType resolverType, - Map inputDatas - ) throws ExperimentException, ValidationException - { - if (resolverType != null) - resolverType.configureRun(context, run, inputDatas); - addInputDatas(context, inputDatas); - } - private boolean hasTransformBeforeRun(AssayRunUploadContext context) { return context.getProvider().isTransformBeforeRunSupported(context.getProtocol()) @@ -396,10 +406,11 @@ public ExpExperiment saveExperimentRun( ParticipantVisitResolverType resolverType = findResolverType(allProperties); PreparedTransform preparedBeforeRun = context.getPreparedTransform(); - if (preparedBeforeRun != null && preparedBeforeRun.getRunDataInputs() != null) - inputDatas.putAll(preparedBeforeRun.getRunDataInputs()); - else - addRunDataInputs(context, run, resolverType, inputDatas); + if (preparedBeforeRun != null && preparedBeforeRun.getResolverDataInputs() != null) + inputDatas.putAll(preparedBeforeRun.getResolverDataInputs()); + else if (resolverType != null) + resolverType.configureRun(context, run, inputDatas); + addInputDatas(context, inputDatas); // TODO: Share these RemapCache and materialCache instances with AbstractAssayTsvDataHandler.checkData and ExpressionMatrixDataHandler.importFile // Cache of resolved alternate lookup keys -> rowId @@ -433,6 +444,8 @@ public ExpExperiment saveExperimentRun( AbstractQueryUpdateService.addTransactionAuditEvent(transaction, context.getUser(), auditEvent); } } + if (preparedBeforeRun != null) + preparedBeforeRun.recordFileAuditEvents(transaction); boolean saveBatchProps = forceSaveBatchProps; // Add any material/data inputs related to the specimen IDs, etc in the incoming data. diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index f0cb8551789..3ffbf3b5410 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -49,6 +49,7 @@ import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.assay.transform.DefaultTransformResult; +import org.labkey.api.assay.transform.PendingFileAudit; import org.labkey.api.assay.transform.PreparedTransform; import org.labkey.api.assay.transform.TransformResult; import org.labkey.api.qc.TsvDataExchangeHandler; @@ -108,6 +109,7 @@ public class AssayRunUploadForm extends Prot private List _collectors; private TransformResult _transformResult = DefaultTransformResult.createEmptyResult(); private @Nullable PreparedTransform _preparedTransform; + private @Nullable List _pendingFileAudits; private ExpRun _reRun; private boolean _allowCrossRunFileInputs; @@ -727,6 +729,18 @@ public void setPreparedTransform(@Nullable PreparedTransform prepared) _preparedTransform = prepared; } + @Override + public @Nullable List getPendingFileAudits() + { + return _pendingFileAudits; + } + + @Override + public void setPendingFileAudits(@Nullable List audits) + { + _pendingFileAudits = audits; + } + @Override public Long getReRunId() { diff --git a/api/src/org/labkey/api/assay/transform/DataTransformService.java b/api/src/org/labkey/api/assay/transform/DataTransformService.java index 1256a97c76e..08a707de979 100644 --- a/api/src/org/labkey/api/assay/transform/DataTransformService.java +++ b/api/src/org/labkey/api/assay/transform/DataTransformService.java @@ -27,7 +27,6 @@ import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExpRun; -import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.query.ValidationException; import org.labkey.api.reader.Readers; @@ -155,8 +154,6 @@ public TransformResult transformAndValidate( "the configuration for this assay design."); } - logIfInTransaction(context, operation); - if (operation == TransformOperation.UPDATE) SimpleMetricsService.get().increment(AssayMetricArea.TRANSFORM_SCRIPT, "executionCountForUpdate"); if (operation == TransformOperation.INSERT) @@ -298,17 +295,6 @@ public TransformResult recordTransform(AssayRunUploadContext context, @Nullab return prepared.getResult(); } - private void logIfInTransaction(AssayRunUploadContext context, TransformOperation operation) - { - if (LOG.isDebugEnabled() - && operation == TransformOperation.INSERT - && context.getProvider().isTransformBeforeRunSupported(context.getProtocol()) - && ExperimentService.get().getSchema().getScope().isTransactionActive()) - { - LOG.debug("Transform script for assay '{}' is running inside a database transaction; callbacks from the script into the server can deadlock", context.getProtocol().getName(), new Throwable()); - } - } - public void addStandardParameters(@Nullable HttpServletRequest request, @Nullable Container container, @Nullable FileLike scriptFile, @Nullable String apiKey, @NotNull Map paramMap) { if (scriptFile != null) diff --git a/api/src/org/labkey/api/assay/transform/PendingFileAudit.java b/api/src/org/labkey/api/assay/transform/PendingFileAudit.java new file mode 100644 index 00000000000..20f45be1ce2 --- /dev/null +++ b/api/src/org/labkey/api/assay/transform/PendingFileAudit.java @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.assay.transform; + +import org.jetbrains.annotations.NotNull; +import org.labkey.api.audit.provider.FileSystemAuditProvider; +import org.labkey.vfs.FileLike; + +/** The audit event of a file saved while preparing a transform, held until the import transaction opens. */ +public record PendingFileAudit(@NotNull FileSystemAuditProvider.FileSystemAuditEvent event, @NotNull FileLike file) {} diff --git a/api/src/org/labkey/api/assay/transform/PreparedTransform.java b/api/src/org/labkey/api/assay/transform/PreparedTransform.java index 13a7cb43f50..9848cf76883 100644 --- a/api/src/org/labkey/api/assay/transform/PreparedTransform.java +++ b/api/src/org/labkey/api/assay/transform/PreparedTransform.java @@ -18,14 +18,32 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.json.JSONObject; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.TransactionAuditProvider; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.labkey.api.audit.provider.FileSystemAuditProvider; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.exp.api.ExpData; +import org.labkey.api.query.AbstractQueryUpdateService; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.QueryService; +import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.GUID; +import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.TestContext; import org.labkey.api.util.logging.LogHelper; import org.labkey.vfs.FileLike; import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -34,7 +52,8 @@ /** * The outcome of running an assay's transform scripts, before any of it is written to the database. * {@link DataTransformService#recordTransform} writes it against the saved run; closing it deletes the scripts' - * working directories, so close it only after the transaction that recorded it has committed or rolled back. + * working directories and adds unrecorded file audit events for files still on disk, so close it only after the transaction that + * recorded it has committed or rolled back. */ public class PreparedTransform implements AutoCloseable { @@ -45,7 +64,10 @@ public class PreparedTransform implements AutoCloseable private final @NotNull List _scriptOutputs; private final @NotNull List _scriptDirs; private boolean _recorded; - private @Nullable Map _runDataInputs; + private @Nullable Map _resolverDataInputs; + private @Nullable JSONObject _scriptInputs; + private final @NotNull List _fileAudits = new ArrayList<>(); + private @Nullable User _fileAuditUser; PreparedTransform(@Nullable DataExchangeHandler handler, @NotNull TransformResult result, @NotNull List scriptOutputs, @NotNull List scriptDirs) { @@ -93,15 +115,60 @@ static PreparedTransform empty() return files; } - /** The run's data inputs, when collected before the scripts ran; null otherwise. */ - public @Nullable Map getRunDataInputs() + /** The data inputs the participant visit resolver added before the scripts ran; null when it didn't run then. */ + public @Nullable Map getResolverDataInputs() { - return _runDataInputs; + return _resolverDataInputs; } - public void setRunDataInputs(@NotNull Map runDataInputs) + public void setResolverDataInputs(@NotNull Map resolverDataInputs) { - _runDataInputs = runDataInputs; + _resolverDataInputs = resolverDataInputs; + } + + /** What the scripts read from the existing run, for the save to compare once its transaction reads the run again. */ + public void setScriptInputs(@NotNull JSONObject scriptInputs) + { + _scriptInputs = scriptInputs; + } + + /** Whether the run, read again inside the transaction, still holds what the scripts read; true when they read nothing from it. */ + public boolean scriptInputsMatch(@Nullable JSONObject current) + { + return _scriptInputs == null || (current != null && _scriptInputs.similar(current)); + } + + /** Holds the audits of files saved while preparing, to record against the import transaction once it opens. */ + public void holdFileAudits(@Nullable User user, @NotNull List audits) + { + _fileAuditUser = user; + _fileAudits.addAll(audits); + } + + /** Records the held audits in the import transaction; a rollback discards them too, so they stay held for close() until it commits. */ + public void recordFileAuditEvents(@NotNull DbScope.Transaction transaction) + { + List audits = List.copyOf(_fileAudits); + if (audits.isEmpty()) + return; + List events = audits.stream().map(PendingFileAudit::event).toList(); + TransactionAuditProvider.TransactionAuditEvent transactionEvent = transaction.getAuditEvent(); + if (transactionEvent != null) + events.forEach(event -> event.setTransactionEvent(transactionEvent, FileSystemAuditProvider.EVENT_TYPE)); + AuditLogService.get().addEvents(_fileAuditUser, events); + transaction.addCommitTask(() -> _fileAudits.removeAll(audits), DbScope.CommitTaskOption.POSTCOMMIT); + transaction.addCommitTask(() -> events.forEach(event -> event.setTransactionId(null)), DbScope.CommitTaskOption.POSTROLLBACK); + } + + /** For an import that failed before recording its held audits; cleanup may already have deleted some of the files. */ + public static void addFileAuditEventsForRemainingFiles(@Nullable User user, @NotNull List audits) + { + List events = audits.stream() + .filter(audit -> audit.file().exists()) + .map(PendingFileAudit::event) + .toList(); + if (!events.isEmpty()) + AuditLogService.get().addEvents(user, events); } public static @NotNull Path normalize(@NotNull File file) @@ -122,7 +189,15 @@ void setRecorded() @Override public void close() { - deleteScriptDirs(_scriptDirs); + try + { + addFileAuditEventsForRemainingFiles(_fileAuditUser, _fileAudits); + _fileAudits.clear(); + } + finally + { + deleteScriptDirs(_scriptDirs); + } } static void deleteScriptDirs(List scriptDirs) @@ -142,4 +217,126 @@ static void deleteScriptDirs(List scriptDirs) } } } + + public static class TestCase extends Assert + { + private FileLike _dir; + + @Before + public void createDirectory() throws IOException + { + _dir = FileUtil.createTempDirectoryFileLike("preparedTransformTest"); + } + + @After + public void deleteDirectory() throws IOException + { + FileUtil.deleteDir(_dir.toNioPathForWrite().toFile()); + } + + @Test + public void testCloseAddsEventsOnlyForRemainingFiles() throws IOException + { + String comment = "PreparedTransform close " + GUID.makeGUID(); + FileLike kept = FileUtil.createTempFile("kept", ".tsv", _dir); + FileLike deleted = FileUtil.createTempFile("deleted", ".tsv", _dir); + + try (PreparedTransform prepared = PreparedTransform.empty()) + { + prepared.holdFileAudits(TestContext.get().getUser(), List.of(audit(comment, kept), audit(comment, deleted))); + deleted.delete(); + } + + List events = selectEvents(comment); + assertEquals("Events added on close", List.of(kept.getName()), events.stream().map(FileSystemAuditProvider.FileSystemAuditEvent::getFile).toList()); + assertNull("Transaction of an event added on close", events.getFirst().getTransactionId()); + } + + @Test + public void testRecordLinksTransactionAndCloseAddsNothingMore() throws IOException + { + String comment = "PreparedTransform record " + GUID.makeGUID(); + FileLike file = FileUtil.createTempFile("recorded", ".tsv", _dir); + TransactionAuditProvider.TransactionAuditEvent transactionEvent = AbstractQueryUpdateService.createTransactionAuditEvent(JunitUtil.getTestContainer(), QueryService.AuditAction.INSERT); + + try (PreparedTransform prepared = PreparedTransform.empty()) + { + prepared.holdFileAudits(TestContext.get().getUser(), List.of(audit(comment, file))); + try (DbScope.Transaction transaction = DbScope.getLabKeyScope().ensureTransaction()) + { + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, TestContext.get().getUser(), transactionEvent); + prepared.recordFileAuditEvents(transaction); + transaction.commit(); + } + } + + List events = selectEvents(comment); + assertEquals("Events after recording and closing", 1, events.size()); + assertEquals("Transaction of the recorded event", Long.valueOf(transactionEvent.getRowId()), events.getFirst().getTransactionId()); + } + + @Test + public void testRolledBackRecordLeavesEventsForClose() throws IOException + { + String comment = "PreparedTransform rollback " + GUID.makeGUID(); + FileLike file = FileUtil.createTempFile("rolledBack", ".tsv", _dir); + TransactionAuditProvider.TransactionAuditEvent transactionEvent = AbstractQueryUpdateService.createTransactionAuditEvent(JunitUtil.getTestContainer(), QueryService.AuditAction.INSERT); + + try (PreparedTransform prepared = PreparedTransform.empty()) + { + prepared.holdFileAudits(TestContext.get().getUser(), List.of(audit(comment, file))); + try (DbScope.Transaction transaction = DbScope.getLabKeyScope().ensureTransaction()) + { + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, TestContext.get().getUser(), transactionEvent); + prepared.recordFileAuditEvents(transaction); + } + } + + List events = selectEvents(comment); + assertEquals("Events after a rolled back record and closing", 1, events.size()); + assertNull("Transaction of an event added on close", events.getFirst().getTransactionId()); + } + + @Test + public void testCloseAllContinuesPastFailure() throws IOException + { + FileLike scriptDir = _dir.resolveChild("run").resolveChild("work"); + FileUtil.mkdirs(scriptDir); + // Closing order isn't defined, so two failures guarantee one comes before another close + PreparedTransforms preparedTransforms = new PreparedTransforms(); + preparedTransforms.put(new JSONObject(), failingTransform()); + preparedTransforms.put(new JSONObject(), failingTransform()); + preparedTransforms.put(new JSONObject(), new PreparedTransform(null, DefaultTransformResult.createEmptyResult(), List.of(), List.of(scriptDir))); + + IllegalStateException e = assertThrows(IllegalStateException.class, preparedTransforms::close); + assertEquals("Failures suppressed into the first", 1, e.getSuppressed().length); + assertFalse("Working directory of the run that closed cleanly", scriptDir.exists()); + } + + private static PreparedTransform failingTransform() + { + return new PreparedTransform(null, DefaultTransformResult.createEmptyResult(), List.of(), List.of()) + { + @Override + public void close() + { + throw new IllegalStateException("close failed"); + } + }; + } + + private static PendingFileAudit audit(String comment, FileLike file) + { + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(JunitUtil.getTestContainer(), comment); + event.setFile(file.getName()); + event.setProvidedFileName(file.getName()); + return new PendingFileAudit(event, file); + } + + private static List selectEvents(String comment) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Comment"), comment); + return AuditLogService.get().getAuditEvents(JunitUtil.getTestContainer(), TestContext.get().getUser(), FileSystemAuditProvider.EVENT_TYPE, filter, null); + } + } } diff --git a/api/src/org/labkey/api/assay/transform/PreparedTransforms.java b/api/src/org/labkey/api/assay/transform/PreparedTransforms.java new file mode 100644 index 00000000000..2e61785454a --- /dev/null +++ b/api/src/org/labkey/api/assay/transform/PreparedTransforms.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.assay.transform; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.json.JSONObject; + +import java.util.IdentityHashMap; +import java.util.Map; + +/** + * The {@link PreparedTransform}s for the runs of one save request, keyed by each run's JSON object. + * Close it only after the transaction that recorded them has committed or rolled back. + */ +public class PreparedTransforms implements AutoCloseable +{ + private final Map _byRunJson = new IdentityHashMap<>(); + + public void put(@NotNull JSONObject runJson, @NotNull PreparedTransform prepared) + { + _byRunJson.put(runJson, prepared); + } + + public @Nullable PreparedTransform get(@NotNull JSONObject runJson) + { + return _byRunJson.get(runJson); + } + + @Override + public void close() + { + // Closing writes audit events, so one failure must not leave the other runs' working directories behind + RuntimeException failure = null; + for (PreparedTransform prepared : _byRunJson.values()) + { + try + { + prepared.close(); + } + catch (RuntimeException e) + { + if (failure == null) + failure = e; + else + failure.addSuppressed(e); + } + } + if (failure != null) + throw failure; + } +} diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index b5fb6e57969..2d5feedeb2c 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -63,6 +63,17 @@ public class ExpDataFileConverter public static final Converter FILE_CONVERTER = new FileConverter(); public static ExpData resolveExpData(JSONObject dataObject, @NotNull Container container, @NotNull User user, @NotNull Collection knownTypes) + { + return resolveExpData(dataObject, container, user, knownTypes, true); + } + + /** Like resolveExpData, but data for a file that has none yet comes back unsaved. */ + public static ExpData resolveExpDataWithoutSaving(JSONObject dataObject, @NotNull Container container, @NotNull User user, @NotNull Collection knownTypes) + { + return resolveExpData(dataObject, container, user, knownTypes, false); + } + + private static ExpData resolveExpData(JSONObject dataObject, @NotNull Container container, @NotNull User user, @NotNull Collection knownTypes, boolean save) { ExperimentService expSvc = ExperimentService.get(); @@ -122,7 +133,8 @@ else if (dataObject.has(ExperimentJSONConverter.PIPELINE_PATH) && dataObject.get String lsid = expSvc.generateLSID(container, type, name); data = expSvc.createData(container, name, lsid); data.setDataFileURI(file.toPath().toUri()); - data.save(user); + if (save) + data.save(user); } return data; } @@ -170,7 +182,8 @@ else if (dataObject.has(ExperimentJSONConverter.ABSOLUTE_PATH) && dataObject.get String lsid = expSvc.generateLSID(container, type, name); data = expSvc.createData(container, name, lsid); data.setDataFileURI(f.toURI()); - data.save(user); + if (save) + data.save(user); } return data; } diff --git a/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java b/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java index c2704721e6c..ea534b4bb56 100644 --- a/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java +++ b/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java @@ -25,6 +25,7 @@ import org.json.JSONException; import org.json.JSONObject; import org.labkey.api.action.ApiUsageException; +import org.labkey.api.assay.AssayDataType; import org.labkey.api.assay.AssayService; import org.labkey.api.data.Container; import org.labkey.api.data.ExpDataFileConverter; @@ -61,6 +62,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import static org.labkey.api.exp.api.ExperimentService.SAMPLE_ALIQUOT_PROTOCOL_LSID; @@ -281,26 +283,8 @@ protected JSONArray serializeMaterialOutputs(ViewContext context, ExpRun run, JS @Override public ExpRun handleRun(ViewContext context, JSONObject runJsonObject, ExpProtocol protocol, @Nullable ExpExperiment batch) throws JSONException, ValidationException, ExperimentException { - ExpRun run; - - if (!runJsonObject.isNull(ExperimentJSONConverter.ID)) - { - int runId = runJsonObject.getInt(ExperimentJSONConverter.ID); - run = ExperimentService.get().getExpRun(runId); - if (run == null) - { - throw new NotFoundException("Could not find experiment run " + runId); - } - if (!run.getContainer().equals(context.getContainer())) - { - throw new NotFoundException("Could not find experiment run " + runId + " in folder " + context.getContainer()); - } - if (null != batch && !batch.getContainer().equals(context.getContainer())) - { - throw new NotFoundException("Could not find experiment run " + runId + " in folder " + context.getContainer()); - } - } - else + ExpRun run = lookupRun(context, runJsonObject, batch); + if (run == null) { String name = runJsonObject.optString(ExperimentJSONConverter.NAME, null); run = createRun(name, context.getContainer(), protocol); @@ -315,10 +299,7 @@ public ExpRun handleRun(ViewContext context, JSONObject runJsonObject, ExpProtoc { handleStandardProperties(context, runJsonObject, run, getRunDomainProperties(protocol)); - if (runJsonObject.has(AssayJSONConverter.DATA_ROWS) || - runJsonObject.has(ExperimentJSONConverter.DATA_INPUTS) || - runJsonObject.has(ExperimentJSONConverter.MATERIAL_INPUTS) || - runJsonObject.has(ExperimentJSONConverter.DATA_OUTPUTS)) + if (hasRunContent(runJsonObject)) { JSONArray dataRows = serializeRunData(context, run, runJsonObject, protocol, batch); JSONArray dataInputs = serializeDataInputs(context, run, runJsonObject, protocol, batch); @@ -339,6 +320,38 @@ public ExpRun handleRun(ViewContext context, JSONObject runJsonObject, ExpProtoc return run; } + /** The existing run the post identifies, or null when it posts a new run. */ + protected @Nullable ExpRun lookupRun(ViewContext context, JSONObject runJsonObject, @Nullable ExpExperiment batch) + { + if (runJsonObject.isNull(ExperimentJSONConverter.ID)) + return null; + + int runId = runJsonObject.getInt(ExperimentJSONConverter.ID); + ExpRun run = ExperimentService.get().getExpRun(runId); + if (run == null) + { + throw new NotFoundException("Could not find experiment run " + runId); + } + if (!run.getContainer().equals(context.getContainer())) + { + throw new NotFoundException("Could not find experiment run " + runId + " in folder " + context.getContainer()); + } + if (null != batch && !batch.getContainer().equals(context.getContainer())) + { + throw new NotFoundException("Could not find experiment run " + runId + " in folder " + context.getContainer()); + } + return run; + } + + /** Whether the post replaces the run's data and protocol applications rather than only its properties. */ + protected static boolean hasRunContent(JSONObject runJsonObject) + { + return runJsonObject.has(AssayJSONConverter.DATA_ROWS) || + runJsonObject.has(ExperimentJSONConverter.DATA_INPUTS) || + runJsonObject.has(ExperimentJSONConverter.MATERIAL_INPUTS) || + runJsonObject.has(ExperimentJSONConverter.DATA_OUTPUTS); + } + @Override public void handleProperties(ViewContext context, ExpObject object, List dps, JSONObject propertiesJsonObject) throws ValidationException, JSONException { @@ -410,14 +423,14 @@ protected void clearOutputDatas(ViewContext context, ExpRun run) } // Disallow creating a run with inputs which are also outputs - private void checkForCycles(Map inputs, Map outputs) throws ExperimentException + private void checkForCycles(Map inputs, Map outputs) throws ValidationException { for (ExpRunItem input : inputs.keySet()) { if (outputs.containsKey(input)) { String role = outputs.get(input); - throw new ExperimentException("Circular input/output '" + input.getName() + "' with role '" + role + "'"); + throw new ValidationException("Circular input/output '" + input.getName() + "' with role '" + role + "'"); } } } @@ -460,30 +473,7 @@ protected void handleProtocolApplications( aliquotRootRowId = parent.getRootMaterialRowId() == null ? parent.getRowId() : parent.getRootMaterialRowId(); } - Map existingOutputs = new HashMap<>(); - ExpProtocolApplication existingOutputApp = run.getOutputProtocolApplication(); - if (existingOutputApp != null) - { - for (ExpMaterialRunInput input : existingOutputApp.getMaterialInputs()) - existingOutputs.put(input.getMaterial().getRowId(), input.getMaterial()); - } - - Set existingMaterialInputRowIds = new HashSet<>(); - for (ExpMaterial in : run.getMaterialInputs().keySet()) - existingMaterialInputRowIds.add(in.getRowId()); - Set postedMaterialInputRowIds = new HashSet<>(); - for (ExpMaterial in : inputMaterial.keySet()) - postedMaterialInputRowIds.add(in.getRowId()); - - Set existingDataInputRowIds = new HashSet<>(); - for (ExpData in : run.getDataInputs().keySet()) - existingDataInputRowIds.add(in.getRowId()); - Set postedDataInputRowIds = new HashSet<>(); - for (ExpData in : inputData.keySet()) - postedDataInputRowIds.add(in.getRowId()); - - boolean inputsUnchanged = existingMaterialInputRowIds.equals(postedMaterialInputRowIds) - && existingDataInputRowIds.equals(postedDataInputRowIds); + ExistingOutputs existingOutputs = ExistingOutputs.read(run, rowIds(inputData.keySet()), rowIds(inputMaterial.keySet())); run.deleteProtocolApplications(context.getUser()); @@ -507,12 +497,7 @@ protected void handleProtocolApplications( if (material != null) { keptOutputRowIds.add(material.getRowId()); - - // Attaching an output rewrites the material's lineage, so require write access - but skip - // materials that were already outputs of this run, since a re-save isn't changing them. - // A sample created in this request only needs insert rights. - if (!existingOutputs.containsKey(material.getRowId()) || !inputsUnchanged) - assertCanEditLineage(context.getUser(), material.getLSID(), material, createdMaterialLsids.contains(material.getLSID())); + existingOutputs.assertCanAttach(context.getUser(), material, createdMaterialLsids.contains(material.getLSID())); if (isAliquotProtocol) { @@ -524,13 +509,8 @@ protected void handleProtocolApplications( } } - // Detaching an output also rewrites its lineage: authorize any prior output the post didn't keep. // This runs inside the save's transaction, so a denial rolls the detach above back. - for (Map.Entry entry : existingOutputs.entrySet()) - { - if (!keptOutputRowIds.contains(entry.getKey())) - assertCanEditLineage(context.getUser(), entry.getValue().getLSID(), entry.getValue()); - } + existingOutputs.assertCanDetachOthers(context.getUser(), keptOutputRowIds); checkForCycles(inputData, outputData); checkForCycles(inputMaterial, outputMaterial); @@ -538,6 +518,109 @@ protected void handleProtocolApplications( saveExperimentRun(context, protocol, batch, run, runJsonObject, dataArray, inputData, outputData, inputMaterial, outputMaterial); } + /** + * Runs handleProtocolApplications' permission and cycle checks without writing anything, to refuse a post + * before work a rollback can't undo, like running transform scripts. The save pass still runs every check. + */ + protected void checkProtocolApplications(ViewContext context, @Nullable ExpRun run, JSONArray inputDataArray, JSONArray inputMaterialArray, JSONArray outputDataArray, JSONArray outputMaterialArray) throws ExperimentException, ValidationException + { + Map inputData = lookupDatas(context, inputDataArray); + Map outputData = lookupDatas(context, outputDataArray); + checkForCycles(inputData, outputData); + + Map inputMaterial = new HashMap<>(); + Set inputMaterialRowIds = new HashSet<>(); + for (JSONObject materialObject : JsonUtil.toJSONObjectList(inputMaterialArray)) + { + MaterialLookup lookup = checkMaterial(context, materialObject); + if (lookup.material() != null) + { + inputMaterial.put(lookup.material(), materialObject.optString(ExperimentJSONConverter.ROLE, ExpMaterialRunInput.DEFAULT_ROLE)); + inputMaterialRowIds.add(lookup.material().getRowId()); + } + else if (lookup.newName() != null) + inputMaterialRowIds.add(0L); // Not created yet, but still a changed input + } + + ExistingOutputs existingOutputs = ExistingOutputs.read(run, rowIds(inputData.keySet()), inputMaterialRowIds); + Map outputMaterial = new HashMap<>(); + for (JSONObject materialObject : JsonUtil.toJSONObjectList(outputMaterialArray)) + { + // A material the save would create needs only insert rights, which the save actions already require + ExpMaterial material = checkMaterial(context, materialObject).material(); + if (material != null) + { + existingOutputs.assertCanAttach(context.getUser(), material, false); + outputMaterial.put(material, materialObject.optString(ExperimentJSONConverter.ROLE, ExpMaterial.DEFAULT_CPAS_TYPE)); + } + } + existingOutputs.assertCanDetachOthers(context.getUser(), rowIds(outputMaterial.keySet())); + checkForCycles(inputMaterial, outputMaterial); + } + + private Map lookupDatas(ViewContext context, JSONArray dataArray) + { + Map datas = new HashMap<>(); + for (JSONObject dataObject : JsonUtil.toJSONObjectList(dataArray)) + datas.put(lookupData(context, dataObject), dataObject.optString(ExperimentJSONConverter.ROLE, ExpDataRunInput.DEFAULT_ROLE)); + return datas; + } + + private MaterialLookup checkMaterial(ViewContext context, JSONObject materialObject) throws ValidationException + { + MaterialLookup lookup = lookupMaterial(context, materialObject); + JSONObject materialProperties = materialObject.optJSONObject(ExperimentJSONConverter.PROPERTIES); + if (lookup.material() != null && materialProperties != null && !materialProperties.isEmpty()) + assertCanEditProperties(context, lookup.material(), lookup.domainProperties(), materialProperties, false); + return lookup; + } + + private static Set rowIds(Collection items) + { + return items.stream().map(ExpRunItem::getRowId).collect(Collectors.toSet()); + } + + /** A run's output materials before the save replaces its protocol applications, and whether the post keeps its inputs. */ + private record ExistingOutputs(Map materials, boolean inputsUnchanged) + { + static ExistingOutputs read(@Nullable ExpRun run, Set postedDataInputRowIds, Set postedMaterialInputRowIds) + { + if (run == null) + return new ExistingOutputs(Map.of(), false); + + Map materials = new HashMap<>(); + ExpProtocolApplication outputApp = run.getOutputProtocolApplication(); + if (outputApp != null) + { + for (ExpMaterialRunInput input : outputApp.getMaterialInputs()) + materials.put(input.getMaterial().getRowId(), input.getMaterial()); + } + + boolean inputsUnchanged = rowIds(run.getMaterialInputs().keySet()).equals(postedMaterialInputRowIds) + && rowIds(run.getDataInputs().keySet()).equals(postedDataInputRowIds); + return new ExistingOutputs(materials, inputsUnchanged); + } + + // Attaching an output rewrites the material's lineage, so require write access - but skip + // materials that were already outputs of this run, since a re-save isn't changing them. + // A sample created in this request only needs insert rights. + void assertCanAttach(User user, ExpMaterial material, boolean createdHere) + { + if (!materials.containsKey(material.getRowId()) || !inputsUnchanged) + assertCanEditLineage(user, material.getLSID(), material, createdHere); + } + + /** Detaching an output also rewrites its lineage: authorize any prior output the post didn't keep. */ + void assertCanDetachOthers(User user, Set keptRowIds) + { + for (Map.Entry entry : materials.entrySet()) + { + if (!keptRowIds.contains(entry.getKey())) + assertCanEditLineage(user, entry.getValue().getLSID(), entry.getValue()); + } + } + } + protected ExpExperiment saveExperimentRun(ViewContext context, ExpProtocol protocol, ExpExperiment batch, ExpRun run, JSONObject runJson, JSONArray dataArray, @@ -554,10 +637,49 @@ protected ExpExperiment saveExperimentRun(ViewContext context, ExpProtocol proto @Override public ExpMaterial handleMaterial(ViewContext context, JSONObject materialObject) throws ValidationException + { + MaterialLookup lookup = lookupMaterial(context, materialObject); + ExpMaterial material = lookup.material(); + boolean createdHere = false; + if (material == null && lookup.newName() != null) + { + material = createMaterial(context, lookup.sampleType(), lookup.newName()); + createdHere = true; + createdMaterialLsids.add(material.getLSID()); + } + + if (material == null) + return null; + + if (materialObject.has(ExperimentJSONConverter.PROPERTIES)) + { + JSONObject materialProperties = materialObject.getJSONObject(ExperimentJSONConverter.PROPERTIES); + // Treat an empty properties collection as if there were no property map at all. + // To delete a property, include a property map with that property and set its value to null. + if (!materialProperties.isEmpty()) + { + List dps = lookup.domainProperties(); + assertCanEditProperties(context, material, dps, materialProperties, createdHere); + handleProperties(context, material, dps, materialProperties); + } + } + + return material; + } + + /** A posted material that exists, or the sample type and name of the one handleMaterial would create. */ + private record MaterialLookup(@Nullable ExpMaterial material, @Nullable ExpSampleType sampleType, @Nullable String newName) + { + List domainProperties() + { + return sampleType != null ? sampleType.getDomain().getProperties() : Collections.emptyList(); + } + } + + private MaterialLookup lookupMaterial(ViewContext context, JSONObject materialObject) throws ValidationException { ExpSampleType sampleType = null; ExpMaterial material = null; - boolean createdHere = false; if (materialObject.has(ExperimentJSONConverter.ID)) { int materialRowId = materialObject.getInt(ExperimentJSONConverter.ID); @@ -575,6 +697,7 @@ else if (materialObject.has(ExperimentJSONConverter.LSID)) sampleType = material.getSampleType(); } + String newName = null; if (material == null) { // Get SampleType by id, name, or lsid @@ -641,45 +764,27 @@ else if (sampleType != null && !sampleType.hasNameAsIdCol() && materialObject.ha throw new NotFoundException("Could not find material '" + materialName + "'"); if (material == null) - { - material = createMaterial(context, sampleType, materialName); - createdHere = true; - createdMaterialLsids.add(material.getLSID()); - } + newName = materialName; } } + return new MaterialLookup(material, sampleType, newName); + } - if (material == null) - return null; - - if (materialObject.has(ExperimentJSONConverter.PROPERTIES)) - { - JSONObject materialProperties = materialObject.getJSONObject(ExperimentJSONConverter.PROPERTIES); - // Treat an empty properties collection as if there were no property map at all. - // To delete a property, include a property map with that property and set its value to null. - if (!materialProperties.isEmpty()) - { - List dps = sampleType != null ? sampleType.getDomain().getProperties() : Collections.emptyList(); - - if (propertiesWouldChange(context, material, dps, materialProperties)) - { - // A sample created by this request needs only insert rights; an existing one needs edit rights. - // The status check below applies either way. - Class editPerm = createdHere - ? InsertPermission.class - : SampleTypeService.SampleOperations.EditMetadata.getPermissionClass(); - if (editPerm == null || !material.getContainer().hasPermission(context.getUser(), editPerm)) - throw new UnauthorizedException("User does not have permission to edit sample '" + material.getName() + "'"); - if (!material.isOperationPermitted(SampleTypeService.SampleOperations.EditMetadata)) - throw new UnauthorizedException(SampleTypeService.get().getOperationNotPermittedMessage( - List.of(material), SampleTypeService.SampleOperations.EditMetadata)); - } - - handleProperties(context, material, dps, materialProperties); - } - } + // A sample created by this request needs only insert rights; an existing one needs edit rights. + // The status check below applies either way. + private void assertCanEditProperties(ViewContext context, ExpMaterial material, List dps, JSONObject materialProperties, boolean createdHere) throws ValidationException + { + if (!propertiesWouldChange(context, material, dps, materialProperties)) + return; - return material; + Class editPerm = createdHere + ? InsertPermission.class + : SampleTypeService.SampleOperations.EditMetadata.getPermissionClass(); + if (editPerm == null || !material.getContainer().hasPermission(context.getUser(), editPerm)) + throw new UnauthorizedException("User does not have permission to edit sample '" + material.getName() + "'"); + if (!material.isOperationPermitted(SampleTypeService.SampleOperations.EditMetadata)) + throw new UnauthorizedException(SampleTypeService.get().getOperationNotPermittedMessage( + List.of(material), SampleTypeService.SampleOperations.EditMetadata)); } private boolean propertiesWouldChange(ViewContext context, ExpMaterial material, List dps, JSONObject submitted) throws ValidationException @@ -752,7 +857,19 @@ private static boolean valuesEqual(Object stored, Object submitted) @Override public ExpData handleData(ViewContext context, JSONObject dataObject) throws ValidationException { - return ExpDataFileConverter.resolveExpData(dataObject, context.getContainer(), context.getUser(), Collections.emptyList()); + return ExpDataFileConverter.resolveExpData(dataObject, context.getContainer(), context.getUser(), getKnownDataTypes()); + } + + /** Resolves the data handleData would use, without writing anything. Data that doesn't exist yet comes back unsaved. */ + protected ExpData lookupData(ViewContext context, JSONObject dataObject) + { + return ExpDataFileConverter.resolveExpDataWithoutSaving(dataObject, context.getContainer(), context.getUser(), getKnownDataTypes()); + } + + /** Data types used to pick the type of data created for a new file. */ + protected @NotNull Collection getKnownDataTypes() + { + return Collections.emptyList(); } // XXX: doesn't handle SampleType parent property magic diff --git a/assay/api-src/org/labkey/api/assay/DefaultAssaySaveHandler.java b/assay/api-src/org/labkey/api/assay/DefaultAssaySaveHandler.java index 5882233b6de..800b65f4e2e 100644 --- a/assay/api-src/org/labkey/api/assay/DefaultAssaySaveHandler.java +++ b/assay/api-src/org/labkey/api/assay/DefaultAssaySaveHandler.java @@ -22,10 +22,12 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; +import org.labkey.api.action.ApiUsageException; import org.labkey.api.action.BaseViewAction; +import org.labkey.api.assay.transform.PreparedTransform; +import org.labkey.api.assay.transform.PreparedTransforms; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.data.Container; -import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.MvUtil; import org.labkey.api.dataiterator.MapDataIterator; import org.labkey.api.exp.ExperimentException; @@ -40,6 +42,7 @@ import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.api.ExperimentJSONConverter; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.ProvenanceService; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; @@ -50,14 +53,22 @@ import org.labkey.remoteapi.collections.CaseInsensitiveHashMap; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.Collections; +import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.Set; public class DefaultAssaySaveHandler extends DefaultExperimentSaveHandler implements AssaySaveHandler { protected static final Logger LOG = LogManager.getLogger(DefaultAssaySaveHandler.class); protected AssayProvider _provider; + private PreparedTransforms _preparedTransforms = new PreparedTransforms(); + private final Map _serializedRuns = new HashMap<>(); + private static final List SCRIPT_INPUT_KEYS = List.of(AssayJSONConverter.DATA_ROWS, ExperimentJSONConverter.DATA_INPUTS, ExperimentJSONConverter.DATA_OUTPUTS); @Override public AssayProvider getProvider() @@ -97,7 +108,7 @@ protected JSONArray serializeRunData(ViewContext context, ExpRun run, JSONObject { // Client didn't post the rows so reuse the values that are currently attached to the run // Inefficient but easy - JSONObject serializedRun = AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS); + JSONObject serializedRun = serializeExistingRun(context, run, protocol); dataRows = serializedRun.getJSONArray(AssayJSONConverter.DATA_ROWS); } else @@ -113,7 +124,7 @@ protected JSONArray serializeDataInputs(ViewContext context, ExpRun run, JSONObj JSONArray dataInputs; if (!runJson.has(ExperimentJSONConverter.DATA_INPUTS)) { - JSONObject serializedRun = AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS); + JSONObject serializedRun = serializeExistingRun(context, run, protocol); dataInputs = serializedRun.getJSONArray(ExperimentJSONConverter.DATA_INPUTS); } else @@ -129,7 +140,7 @@ protected JSONArray serializeMaterialInputs(ViewContext context, ExpRun run, JSO JSONArray materialInputs; if (!runJson.has(ExperimentJSONConverter.MATERIAL_INPUTS)) { - JSONObject serializedRun = AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS); + JSONObject serializedRun = serializeExistingRun(context, run, protocol); materialInputs = serializedRun.getJSONArray(ExperimentJSONConverter.MATERIAL_INPUTS); } else @@ -145,7 +156,7 @@ protected JSONArray serializeDataOutputs(ViewContext context, ExpRun run, JSONOb JSONArray dataOutputs; if (!runJson.has(ExperimentJSONConverter.DATA_OUTPUTS)) { - JSONObject serializedRun = AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS); + JSONObject serializedRun = serializeExistingRun(context, run, protocol); dataOutputs = serializedRun.getJSONArray(ExperimentJSONConverter.DATA_OUTPUTS); } else @@ -161,7 +172,7 @@ protected JSONArray serializeMaterialOutputs(ViewContext context, ExpRun run, JS JSONArray materialOutputs; if (!runJson.has(ExperimentJSONConverter.MATERIAL_OUTPUTS)) { - JSONObject serializedRun = AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS); + JSONObject serializedRun = serializeExistingRun(context, run, protocol); materialOutputs = serializedRun.getJSONArray(ExperimentJSONConverter.MATERIAL_OUTPUTS); } else @@ -171,6 +182,13 @@ protected JSONArray serializeMaterialOutputs(ViewContext context, ExpRun run, JS return materialOutputs; } + /** Serializes a run once for all the serialize methods within one pass of a save. */ + private JSONObject serializeExistingRun(ViewContext context, ExpRun run, ExpProtocol protocol) + { + return _serializedRuns.computeIfAbsent(run.getLSID(), lsid -> + AssayJSONConverter.serializeRun(run, _provider, protocol, context.getUser(), ExperimentJSONConverter.DEFAULT_SETTINGS)); + } + @Override protected void clearOutputDatas(ViewContext context, ExpRun run) { @@ -197,33 +215,178 @@ protected ExpExperiment saveExperimentRun( Map outputMaterial ) throws ExperimentException, ValidationException { + // This run's read inside the transaction, to check against what its scripts saw + JSONObject serializedRun = _serializedRuns.remove(run.getLSID()); + List> dataRows = convertRunData(dataArray, run.getContainer(), protocol); - ExpData tsvData = DefaultAssayRunCreator.generateResultData(context.getUser(), run.getContainer(), getProvider(), dataRows, (Map) outputData, null); + if (skipsAssayRun(dataRows, !outputData.isEmpty())) + return null; + + DefaultAssayRunCreator.generateResultData(context.getUser(), run.getContainer(), getProvider(), dataRows, (Map) outputData, null); + AssayRunUploadContext uploadContext = createRunUploadContext(context, protocol, runJson, dataRows, + inputData, outputData, inputMaterial, outputMaterial); + + PreparedTransform prepared = _preparedTransforms.get(runJson); + if (prepared != null && uploadContext != null) + { + // Rerunning the scripts inside the transaction would reopen the deadlock they ran early to avoid + if (!prepared.scriptInputsMatch(serializedRun == null ? null : readFromSerialization(runJson, serializedRun))) + throw new ValidationException("Run " + run.getRowId() + " changed while its transform scripts ran. Retry the save."); + + uploadContext.setPreparedTransform(prepared); + } - // CONSIDER: Is this block still needed? - // Try to find a data object to attach our data rows to - if (tsvData == null && !outputData.isEmpty()) + return saveAssayRun(uploadContext, batch, run, BaseViewAction.getTransactionAuditDetails(context)); + } + + /** The arrays read from an existing run's serialization because the client didn't post them. */ + private static JSONObject readFromSerialization(JSONObject runJson, JSONObject serializedRun) + { + JSONObject read = new JSONObject(); + for (String key : SCRIPT_INPUT_KEYS) { - tsvData = outputData.keySet().iterator().next(); - for (Map.Entry entry : outputData.entrySet()) + if (!runJson.has(key)) + read.put(key, serializedRun.getJSONArray(key)); + } + return read; + } + + /** Whether saveExperimentRun skips the assay save, and with it the transform scripts. */ + private static boolean skipsAssayRun(List> dataRows, boolean hasOutputData) + { + return dataRows.isEmpty() && !hasOutputData; + } + + /** Whether runs are saved exactly as this class saves them, which the first pass of prepareTransforms assumes. */ + protected boolean isPrepareTransformsSupported() + { + return false; + } + + @Override + public @NotNull PreparedTransforms prepareTransforms(ViewContext context, ExpProtocol protocol, List runJsons) throws ExperimentException, ValidationException + { + PreparedTransforms preparedTransforms = new PreparedTransforms(); + // Inside a caller's transaction the scripts can't run outside it, so leave them to the save + if (!isPrepareTransformsSupported() + || ExperimentService.get().getSchema().getScope().isTransactionActive() + || !getProvider().isTransformBeforeRunSupported(protocol) + || getProvider().getValidationAndAnalysisScripts(protocol, AssayProvider.Scope.ALL).isEmpty()) + return preparedTransforms; + + // A repeated run's later posts would save over what its scripts saw before the transaction + Set runIds = new HashSet<>(); + for (JSONObject runJson : runJsons) + { + if (!runJson.isNull(ExperimentJSONConverter.ID) && !runIds.add(runJson.getInt(ExperimentJSONConverter.ID))) + throw new ApiUsageException("Run " + runJson.getInt(ExperimentJSONConverter.ID) + " appears more than once in the request"); + } + + // A script's side effects can't be rolled back, so refuse the request before any of them runs + List postedRuns = new ArrayList<>(); + for (JSONObject runJson : runJsons) + { + PostedRun posted = readPostedRun(context, protocol, runJson); + if (posted != null) { - // Prefer the generic "Data" role if we have one - if (ExpDataRunInput.DEFAULT_ROLE.equals(entry.getValue())) - { - tsvData = entry.getKey(); - break; - } + checkProtocolApplications(context, posted.existingRun(), posted.dataInputs(), posted.materialInputs(), posted.dataOutputs(), posted.materialOutputs()); + postedRuns.add(posted); + } + } + + boolean success = false; + try + { + for (PostedRun posted : postedRuns) + { + PreparedTransform prepared = runTransformScripts(context, protocol, posted); + if (prepared != null) + preparedTransforms.put(posted.json(), prepared); } + success = true; } + finally + { + if (!success) + preparedTransforms.close(); + } + _preparedTransforms = preparedTransforms; + return preparedTransforms; + } - if (tsvData != null) + /** A posted run with the arrays handleRun would save, read from the existing run where the client didn't post them. */ + private record PostedRun(JSONObject json, @Nullable ExpRun existingRun, JSONArray dataRows, JSONArray dataInputs, JSONArray materialInputs, + JSONArray dataOutputs, JSONArray materialOutputs, @Nullable JSONObject scriptInputs) {} + + /** Null when the post changes only the run's properties, which runs no scripts. */ + private @Nullable PostedRun readPostedRun(ViewContext context, ExpProtocol protocol, JSONObject runJson) + { + ExpRun existingRun = lookupRun(context, runJson, null); + if (!hasRunContent(runJson)) + return null; + + if (existingRun == null) { - AssayRunUploadContext uploadContext = createRunUploadContext(context, protocol, runJson, dataRows, - inputData, outputData, inputMaterial, outputMaterial); + return new PostedRun(runJson, null, postedArray(runJson, AssayJSONConverter.DATA_ROWS), + postedArray(runJson, ExperimentJSONConverter.DATA_INPUTS), postedArray(runJson, ExperimentJSONConverter.MATERIAL_INPUTS), + postedArray(runJson, ExperimentJSONConverter.DATA_OUTPUTS), postedArray(runJson, ExperimentJSONConverter.MATERIAL_OUTPUTS), null); + } - return saveAssayRun(uploadContext, batch, run, BaseViewAction.getTransactionAuditDetails(context)); + try + { + JSONArray dataRows = serializeRunData(context, existingRun, runJson, protocol, null); + JSONArray dataInputs = serializeDataInputs(context, existingRun, runJson, protocol, null); + JSONArray materialInputs = serializeMaterialInputs(context, existingRun, runJson, protocol, null); + JSONArray dataOutputs = serializeDataOutputs(context, existingRun, runJson, protocol, null); + JSONArray materialOutputs = serializeMaterialOutputs(context, existingRun, runJson, protocol, null); + JSONObject serializedRun = _serializedRuns.get(existingRun.getLSID()); + return new PostedRun(runJson, existingRun, dataRows, dataInputs, materialInputs, dataOutputs, materialOutputs, + serializedRun == null ? null : readFromSerialization(runJson, serializedRun)); + } + finally + { + // The save pass reads the run again inside its transaction + _serializedRuns.remove(existingRun.getLSID()); } - return null; + } + + /** Builds the upload context handleRun would build for this run, without writing anything, and runs its transform scripts. */ + private @Nullable PreparedTransform runTransformScripts(ViewContext context, ExpProtocol protocol, PostedRun posted) throws ExperimentException, ValidationException + { + // Unsaved data all have rowId 0 and can share an LSID, so key by file instead of ExpData equality + Map dataByKey = new HashMap<>(); + Map inputData = new IdentityHashMap<>(); + for (JSONObject dataObject : JsonUtil.toJSONObjectList(posted.dataInputs())) + inputData.put(lookupData(context, dataObject, dataByKey), dataObject.optString(ExperimentJSONConverter.ROLE, ExpDataRunInput.DEFAULT_ROLE)); + + List> dataRows = convertRunData(posted.dataRows(), context.getContainer(), protocol); + if (skipsAssayRun(dataRows, !posted.dataOutputs().isEmpty())) + return null; + + // Scripts never see output data, so the outputs are only counted, not resolved + AssayRunUploadContext uploadContext = createRunUploadContext(context, protocol, posted.json(), dataRows, + inputData, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()); + PreparedTransform prepared = getProvider().getRunCreator().prepareTransform(uploadContext); + if (prepared != null && posted.scriptInputs() != null) + prepared.setScriptInputs(posted.scriptInputs()); + return prepared; + } + + /** Each lookup of a not-yet-saved file returns a new data object, but handleData saves the first and reuses it for the rest. */ + private ExpData lookupData(ViewContext context, JSONObject dataObject, Map dataByKey) + { + ExpData data = lookupData(context, dataObject); + return dataByKey.computeIfAbsent(dataKey(data), key -> data); + } + + private static Object dataKey(ExpData data) + { + return data.getDataFileUrl() != null ? data.getDataFileUrl() : data.getRowId(); + } + + private static JSONArray postedArray(JSONObject runJson, String key) + { + return runJson.has(key) ? runJson.getJSONArray(key) : new JSONArray(); } /** @@ -328,6 +491,15 @@ protected ExpExperiment saveAssayRun(AssayRunUploadContext context, ExpExperi @Override @Deprecated // Use new JSONObject variant above public ExpData handleData(ViewContext context, JSONObject dataObject) throws ValidationException + { + ExpData data = super.handleData(context, dataObject); + + handleProperties(context, data, Collections.emptyList(), dataObject); + return data; + } + + @Override + protected @NotNull Collection getKnownDataTypes() { List knownTypes = new ArrayList<>(); if (getProvider().getDataType() != null) @@ -335,9 +507,6 @@ public ExpData handleData(ViewContext context, JSONObject dataObject) throws Val knownTypes.add(getProvider().getDataType()); } knownTypes.addAll(getProvider().getRelatedDataTypes()); - ExpData data = ExpDataFileConverter.resolveExpData(dataObject, context.getContainer(), context.getUser(), knownTypes); - - handleProperties(context, data, Collections.emptyList(), dataObject); - return data; + return knownTypes; } } diff --git a/assay/src/org/labkey/assay/TsvAssaySaveHandler.java b/assay/src/org/labkey/assay/TsvAssaySaveHandler.java index 46c10e02db0..f8a888e0429 100644 --- a/assay/src/org/labkey/assay/TsvAssaySaveHandler.java +++ b/assay/src/org/labkey/assay/TsvAssaySaveHandler.java @@ -22,8 +22,14 @@ * User: kevink * Date: 8/10/14 */ -public class TsvAssaySaveHandler extends DefaultAssaySaveHandler +public final class TsvAssaySaveHandler extends DefaultAssaySaveHandler { + @Override + protected boolean isPrepareTransformsSupported() + { + return true; + } + @Override public void setProvider(AssayProvider provider) { diff --git a/assay/src/org/labkey/assay/actions/BaseProtocolAPIAction.java b/assay/src/org/labkey/assay/actions/BaseProtocolAPIAction.java index 96a9f831760..c6ce1c8ab54 100644 --- a/assay/src/org/labkey/assay/actions/BaseProtocolAPIAction.java +++ b/assay/src/org/labkey/assay/actions/BaseProtocolAPIAction.java @@ -22,14 +22,18 @@ import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.SimpleApiJsonForm; import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.AssaySaveHandler; import org.labkey.api.assay.AssayService; +import org.labkey.api.assay.transform.PreparedTransforms; import org.labkey.api.data.Container; +import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.api.AssayJSONConverter; import org.labkey.api.exp.api.DefaultExperimentSaveHandler; import org.labkey.api.exp.api.ExpProtocol; import org.labkey.api.exp.api.ExperimentJSONConverter; import org.labkey.api.exp.api.ExperimentSaveHandler; import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.query.ValidationException; import org.labkey.api.util.Pair; import org.labkey.api.view.NotFoundException; import org.springframework.validation.BindException; @@ -152,4 +156,12 @@ public ExperimentSaveHandler getExperimentSaveHandler(@Nullable AssayProvider pr return saveHandler; } + + /** GH Issue 1720: run transform scripts before the save's transaction so their callbacks into the server can't deadlock with it */ + protected PreparedTransforms prepareTransforms(ExperimentSaveHandler saveHandler, ExpProtocol protocol, List runJsons) throws ExperimentException, ValidationException + { + if (saveHandler instanceof AssaySaveHandler assaySaveHandler) + return assaySaveHandler.prepareTransforms(getViewContext(), protocol, runJsons); + return new PreparedTransforms(); + } } diff --git a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java index 30aac021156..c9238782d50 100644 --- a/assay/src/org/labkey/assay/actions/ImportRunApiAction.java +++ b/assay/src/org/labkey/assay/actions/ImportRunApiAction.java @@ -352,7 +352,7 @@ else if (rawData != null && !rawData.isEmpty()) .create(); // GH Issue 1707: run transform scripts before the transaction so their callbacks into the server can't deadlock with it - try (PreparedTransform ignored = provider.getRunCreator().prepareTransform(uploadContext, forceAsync); + try (PreparedTransform ignored = importInBackground ? null : provider.getRunCreator().prepareTransform(uploadContext); DbScope.Transaction transaction = ExperimentService.get().getSchema().getScope().ensureTransaction(DefaultAssayRunCreator.protocolImportLocks(importInBackground))) { Map transactionDetails = getTransactionAuditDetails(); diff --git a/assay/src/org/labkey/assay/actions/SaveAssayBatchAction.java b/assay/src/org/labkey/assay/actions/SaveAssayBatchAction.java index 8c15c890678..6b06d32b82e 100644 --- a/assay/src/org/labkey/assay/actions/SaveAssayBatchAction.java +++ b/assay/src/org/labkey/assay/actions/SaveAssayBatchAction.java @@ -21,14 +21,18 @@ import org.labkey.api.action.ApiVersion; import org.labkey.api.action.SimpleApiJsonForm; import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.transform.PreparedTransforms; import org.labkey.api.data.DbScope; import org.labkey.api.exp.api.AssayJSONConverter; +import org.labkey.api.exp.api.DefaultExperimentSaveHandler; import org.labkey.api.exp.api.ExpExperiment; import org.labkey.api.exp.api.ExpProtocol; +import org.labkey.api.exp.api.ExperimentJSONConverter; import org.labkey.api.exp.api.ExperimentSaveHandler; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.InsertPermission; +import org.labkey.api.util.JsonUtil; import org.springframework.validation.BindException; import java.util.ArrayList; @@ -81,7 +85,14 @@ private ApiResponse executeAction(ExperimentSaveHandler saveHandler, ExpProtocol { saveHandler.beforeSave(getViewContext(), rootJsonObject, protocol); List batches = new ArrayList<>(batchesJsonArray.length()); - try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) + List runJsons = new ArrayList<>(); + for (JSONObject batchJsonObject : JsonUtil.toJSONObjectList(batchesJsonArray)) + { + checkBatch(batchJsonObject); + runJsons.addAll(getRunJsons(batchJsonObject)); + } + try (PreparedTransforms ignored = prepareTransforms(saveHandler, protocol, runJsons); + DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) { for (int i = 0; i < batchesJsonArray.length(); i++) { @@ -102,7 +113,9 @@ private ApiResponse executeAction(ExperimentSaveHandler saveHandler, ExpProtocol List batches = new ArrayList<>(1); saveHandler.beforeSave(getViewContext(), rootJsonObject, protocol); - try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) + checkBatch(batchJsonObject); + try (PreparedTransforms ignored = prepareTransforms(saveHandler, protocol, getRunJsons(batchJsonObject)); + DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) { batches.add(saveHandler.handleBatch(getViewContext(), batchJsonObject, protocol)); transaction.commit(); @@ -110,5 +123,18 @@ private ApiResponse executeAction(ExperimentSaveHandler saveHandler, ExpProtocol saveHandler.afterSave(getViewContext(), batches, protocol); return AssayJSONConverter.serializeResult(provider, protocol, batches.getFirst(), getUser()); } + + /** Rejects a missing batch before any transform script runs. */ + private void checkBatch(JSONObject batchJsonObject) + { + if (!batchJsonObject.isNull(ExperimentJSONConverter.ID)) + DefaultExperimentSaveHandler.lookupBatch(getContainer(), batchJsonObject.getInt(ExperimentJSONConverter.ID)); + } + + private static List getRunJsons(JSONObject batchJsonObject) + { + JSONArray runsArray = batchJsonObject.optJSONArray(AssayJSONConverter.RUNS); + return runsArray == null ? List.of() : JsonUtil.toJSONObjectList(runsArray); + } } diff --git a/assay/src/org/labkey/assay/actions/SaveAssayRunsAction.java b/assay/src/org/labkey/assay/actions/SaveAssayRunsAction.java index 05fb6a1c0dd..62c4d68427d 100644 --- a/assay/src/org/labkey/assay/actions/SaveAssayRunsAction.java +++ b/assay/src/org/labkey/assay/actions/SaveAssayRunsAction.java @@ -20,6 +20,7 @@ import org.labkey.api.action.ApiResponse; import org.labkey.api.action.SimpleApiJsonForm; import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.transform.PreparedTransforms; import org.labkey.api.data.DbScope; import org.labkey.api.exp.api.AssayJSONConverter; import org.labkey.api.exp.api.ExpProtocol; @@ -29,6 +30,7 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.InsertPermission; +import org.labkey.api.util.JsonUtil; import org.springframework.validation.BindException; import java.util.ArrayList; @@ -61,7 +63,8 @@ protected ApiResponse executeAction(ExpProtocol protocol, SimpleApiJsonForm form private ApiResponse executeAction(ExperimentSaveHandler saveHandler, ExpProtocol protocol, AssayProvider provider, JSONArray runsJsonArray) throws Exception { List runs = new ArrayList<>(); - try (DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) + try (PreparedTransforms ignored = prepareTransforms(saveHandler, protocol, JsonUtil.toJSONObjectList(runsJsonArray)); + DbScope.Transaction transaction = ExperimentService.get().ensureTransaction()) { for (int i = 0; i < runsJsonArray.length(); i++) { diff --git a/assay/src/org/labkey/assay/pipeline/AssayImportRunTask.java b/assay/src/org/labkey/assay/pipeline/AssayImportRunTask.java index 845f6c49dd7..7bdb69fa7d5 100644 --- a/assay/src/org/labkey/assay/pipeline/AssayImportRunTask.java +++ b/assay/src/org/labkey/assay/pipeline/AssayImportRunTask.java @@ -808,7 +808,7 @@ public RecordedActionSet run() throws PipelineJobException AssayRunUploadContext uploadContext = factory.create(); // GH Issue 1707: run transform scripts before the transaction so their callbacks into the server can't deadlock with it - try (PreparedTransform ignored = provider.getRunCreator().prepareTransform(uploadContext, false); + try (PreparedTransform ignored = provider.getRunCreator().prepareTransform(uploadContext); DbScope.Transaction tx = scope.ensureTransaction()) { String fileWatcherDescription = getJob().getDescription();