From 308acb95f57a6390c363759edd952e7213f422ed Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 20 Aug 2025 12:20:35 -0700 Subject: [PATCH] Revise file/attachment exception handling --- .../assay/AbstractAssayTsvDataHandler.java | 2 +- .../api/assay/DefaultAssayRunCreator.java | 22 ++++++----- .../dataiterator/AttachmentDataIterator.java | 38 +++++++++++-------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 8715399dc35..072e536c44e 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -943,7 +943,7 @@ else if (o instanceof MvFieldWrapper mvWrapper) } catch (ConvertHelper.FileConversionException e) { - throw new ApiUsageException(e); + errors.add(new PropertyValidationError(e.getMessage(), pd.getName())); } } } diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 7f93c434737..98f25ad7510 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -482,23 +482,25 @@ public ExpExperiment saveExperimentRun( return batch; } - catch (ExperimentException | IOException | ConvertHelper.FileConversionException e) + catch (ExperimentException | IOException | ConvertHelper.FileConversionException | BatchValidationException e) { + // TODO: This is better done as a post-rollback task on the transaction // clean up the run results file dir here if it was created, for non-async imports AssayResultsFileWriter resultsFileWriter = new AssayResultsFileWriter<>(context.getProtocol(), run, null); resultsFileWriter.cleanupPostedFiles(context.getContainer(), false); cleanPrimaryFile(context); - if (e instanceof ExperimentException) - throw (ExperimentException)e; - else if (e instanceof ConvertHelper.FileConversionException) - throw new ApiUsageException(e.getMessage(), e); - else - throw new ExperimentException(e); - } - catch (BatchValidationException e) - { + if (e instanceof ExperimentException ee) + throw ee; + + // HACK: Rethrowing these as ApiUsageException avoids any upstream consequences of wrapping them in ExperimentException. + // Namely, that they are logged to the server/mothership. There has to be a better way. + if (e instanceof ConvertHelper.FileConversionException fce) + throw new ApiUsageException(fce.getMessage(), fce); + else if (e instanceof BatchValidationException bve) + throw new ApiUsageException(bve.getMessage(), bve); + throw new ExperimentException(e); } } diff --git a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java b/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java index 8b63482792c..4dc5b12e776 100644 --- a/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/AttachmentDataIterator.java @@ -43,10 +43,6 @@ import java.io.InputStream; import java.util.ArrayList; -/** - * Moved from ListQueryUpdateService.java - * by iansigmon on 2/16/16. - */ public class AttachmentDataIterator extends WrapperDataIterator { final VirtualFile attachmentDir; @@ -92,19 +88,21 @@ public boolean next() throws BatchValidationException for (_AttachmentUploadHelper p : attachmentColumns) { Object attachmentValue = get(p.index); + if (null == attachmentValue) + continue; + String filename; AttachmentFile attachmentFile; - if (null == attachmentValue) - continue; - else if (attachmentValue instanceof String str) + if (attachmentValue instanceof String str) { if (null == attachmentDir) { - errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Can't upload '" + str + "' to field " + p.domainProperty.getName() + " with type " + p.domainProperty.getType().getLabel() + ".")); + errors.addRowError(propertyValidationException(p.domainProperty, attachmentValue)); return false; } - filename = (String) attachmentValue; + + filename = str; InputStream aIS = attachmentDir.getDir(p.domainProperty.getName()).getInputStream(p.uniquifier.uniquify(filename)); if (aIS == null) { @@ -113,25 +111,25 @@ else if (attachmentValue instanceof String str) } attachmentFile = new InputStreamAttachmentFile(aIS, filename); } - else if (attachmentValue instanceof AttachmentFile) + else if (attachmentValue instanceof AttachmentFile file) { - attachmentFile = (AttachmentFile) attachmentValue; + attachmentFile = file; filename = attachmentFile.getFilename(); } - else if (attachmentValue instanceof File) + else if (attachmentValue instanceof File file) { - attachmentFile = new FileAttachmentFile((File) attachmentValue); + attachmentFile = new FileAttachmentFile(file); filename = attachmentFile.getFilename(); } else { - errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file.")); + errors.addRowError(propertyValidationException(p.domainProperty, attachmentValue)); return false; } if (entityIdIndex == 0) { - errors.addRowError(new ValidationException("Row " + get(0) + ": " + "Unable to create attachment file.")); + errors.addRowError(rowValidationException("Unable to create attachment file.")); return false; } @@ -172,6 +170,16 @@ else if (attachmentValue instanceof File) } } + private ValidationException propertyValidationException(DomainProperty property, Object value) + { + return rowValidationException(String.format("Can't upload '%s' to field %s with type %s.", value, property.getName(), property.getType().getLabel())); + } + + private ValidationException rowValidationException(String message) + { + return new ValidationException("Row " + get(0) + ": " + message); + } + public static DataIteratorBuilder getAttachmentDataIteratorBuilder(TableInfo ti, @NotNull final DataIteratorBuilder builder, final User user, @Nullable final VirtualFile attachmentDir,