From a39b7b6c7541fdce62a22b84e3a4a7918812e7d5 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 15 Jul 2025 13:29:30 -0700 Subject: [PATCH 01/25] File path related bugs --- .../api/data/AbstractFileDisplayColumn.java | 30 ++++---- .../labkey/api/data/AbstractTableInfo.java | 3 +- .../org/labkey/api/data/ConvertHelper.java | 8 +++ .../labkey/api/data/ExpDataFileConverter.java | 13 ++++ .../org/labkey/api/data/URLDisplayColumn.java | 6 +- .../api/query/AbstractQueryUpdateService.java | 16 ++++- api/src/org/labkey/api/reader/DataLoader.java | 29 +++++++- .../study/assay/FileLinkDisplayColumn.java | 72 +++++++++++++------ .../query/UserAvatarDisplayColumnFactory.java | 2 +- .../DefaultCustomPropertyRenderer.java | 4 +- .../labkey/experiment/ExpDataIterators.java | 2 +- .../src/org/labkey/experiment/XarReader.java | 2 +- .../experiment/api/ExpRunTableImpl.java | 2 +- .../experiment/api/SampleTypeServiceImpl.java | 2 +- .../api/SampleTypeUpdateServiceDI.java | 1 + .../samples/AbstractExpFolderImporter.java | 2 +- .../org/labkey/study/model/StudyManager.java | 4 +- .../study/query/DatasetUpdateService.java | 1 + 18 files changed, 147 insertions(+), 52 deletions(-) diff --git a/api/src/org/labkey/api/data/AbstractFileDisplayColumn.java b/api/src/org/labkey/api/data/AbstractFileDisplayColumn.java index 591b8b5c4be..74bb606c48c 100644 --- a/api/src/org/labkey/api/data/AbstractFileDisplayColumn.java +++ b/api/src/org/labkey/api/data/AbstractFileDisplayColumn.java @@ -55,6 +55,8 @@ public abstract class AbstractFileDisplayColumn extends DataColumn protected String _thumbnailWidth; protected String _popupWidth; + public static final String UNAVAILABLE_FILE_SUFFIX = " (unavailable)"; + public AbstractFileDisplayColumn(ColumnInfo col) { super(col); @@ -75,16 +77,11 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out) /** @return the short name of the file (not including full path) */ protected abstract String getFileName(RenderContext ctx, Object value); - protected String getFileName(RenderContext ctx, Object value, boolean isDisplay) - { - return getFileName(ctx, value); - } - protected abstract InputStream getFileContents(RenderContext ctx, Object value) throws FileNotFoundException; - protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String filename, boolean link, boolean thumbnail) + protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String fileValue, boolean link, boolean thumbnail) { - renderIconAndFilename(ctx, out, filename, null, null, link, thumbnail); + renderIconAndFilename(ctx, out, fileValue, null, null, link, thumbnail); } protected boolean isImage(String filename) @@ -95,34 +92,35 @@ protected boolean isImage(String filename) || filename.toLowerCase().endsWith(".gif"); } - protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String filename, @Nullable String fileIconUrl, @Nullable String popupIconUrl, boolean link, boolean thumbnail) + protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String fileValue, @Nullable String fileIconUrl, @Nullable String popupIconUrl, boolean link, boolean thumbnail) { - if (null != filename && !StringUtils.isEmpty(filename)) + if (null != fileValue && !StringUtils.isEmpty(fileValue)) { // equivalent of DisplayColumn.renderURL. // Don't want to call renderUrl (DataColumn.renderUrl) to skip unnecessary displayValue check StringExpression s = compileExpression(ctx.getViewContext()); - String url = null == s ? null : s.eval(ctx); - String displayName = getFileName(ctx, filename, true); - boolean isImage = isImage(filename); - FileImageRenderHelper renderHelper = createRenderHelper(ctx, url, filename, displayName, fileIconUrl, popupIconUrl, thumbnail, isImage); + String displayName = getFileName(ctx, fileValue); + boolean unavailable = displayName.endsWith(UNAVAILABLE_FILE_SUFFIX); + String url = null == s || unavailable ? null : s.eval(ctx); + boolean isImage = isImage(fileValue); + FileImageRenderHelper renderHelper = createRenderHelper(ctx, url, fileValue, displayName, fileIconUrl, popupIconUrl, thumbnail, isImage); if (link && null != url) { A( at(title, "Download attached file") - .at(getLinkTarget() != null && MimeMap.DEFAULT.canInlineFor(filename), target, getLinkTarget()) + .at(getLinkTarget() != null && MimeMap.DEFAULT.canInlineFor(fileValue), target, getLinkTarget()) .at(href, url), (Renderable) ret -> { - renderPopup(renderHelper, url, fileIconUrl, displayName, filename, thumbnail, isImage, out); + renderPopup(renderHelper, url, fileIconUrl, displayName, fileValue, thumbnail, isImage, out); return ret; } ).appendTo(out); } else { - renderPopup(renderHelper, url, fileIconUrl, displayName, filename, thumbnail, isImage, out); + renderPopup(renderHelper, url, fileIconUrl, displayName, fileValue, thumbnail, isImage, out); } } else diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index 16b03db90a6..7b70be7085b 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -104,6 +104,7 @@ import static java.util.Collections.unmodifiableCollection; import static org.apache.poi.util.StringUtil.isNotBlank; +import static org.labkey.api.data.AbstractFileDisplayColumn.UNAVAILABLE_FILE_SUFFIX; abstract public class AbstractTableInfo implements TableInfo, AuditConfigurable, MemTrackable { @@ -2094,7 +2095,7 @@ public List> getValidatedImportTemplates(ViewContext ctx) else { boolean fileExist = FileLinkDisplayColumn.filePathExist(template.second, ctx.getContainer(), ctx.getUser()); - templates.add(new Pair<>(template.first, template.second + (fileExist ? "" : " (unavailable)"))); + templates.add(new Pair<>(template.first, template.second + (fileExist ? "" : UNAVAILABLE_FILE_SUFFIX))); } } return templates; diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index e8ae8d0094b..74c188a30fb 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -61,6 +61,7 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.ReturnURLString; import org.labkey.api.util.SimpleTime; +import org.labkey.api.util.SkipMothershipLogging; import org.labkey.api.util.StringExpression; import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.util.TimeOnlyDate; @@ -450,6 +451,13 @@ public ContainerConversionException(String msg) } } + public static class FileLinkConversionException extends ConversionException implements SkipMothershipLogging + { + public FileLinkConversionException(String msg) + { + super(msg); + } + } public static class ContainerConverter implements Converter { diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 610c52b1806..88f55935409 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -227,6 +227,14 @@ public Object convert(Class type, Object value) // If we have a file path, make sure it's supposed to be visible in the current container if (f != null) { + if (!f.isFile()) + { + if (value instanceof String) + throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value); + else + throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + f.getPath()); + } + // Strip out ".." and "." f = FileUtil.resolveFile(f); @@ -255,6 +263,11 @@ public Object convert(Class type, Object value) return f; } } + + if (value instanceof String) + throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value); + else + throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + f.getPath()); } } return null; diff --git a/api/src/org/labkey/api/data/URLDisplayColumn.java b/api/src/org/labkey/api/data/URLDisplayColumn.java index 327e33d8bdf..55bb96d703d 100644 --- a/api/src/org/labkey/api/data/URLDisplayColumn.java +++ b/api/src/org/labkey/api/data/URLDisplayColumn.java @@ -115,7 +115,7 @@ protected InputStream getFileContents(RenderContext ctx, Object value) } @Override - protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String filename, boolean link, boolean thumbnail) + protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String fileValue, boolean link, boolean thumbnail) { Object value = getValue(ctx); String url = renderURL(ctx); @@ -125,11 +125,11 @@ protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String f if (value != null && (url != null || imageUrl != null || popupImageUrl != null)) { // custom image URLs through the column metadata - super.renderIconAndFilename(ctx, out, filename, imageUrl, popupImageUrl, false, thumbnail); + super.renderIconAndFilename(ctx, out, fileValue, imageUrl, popupImageUrl, false, thumbnail); } else { - super.renderIconAndFilename(ctx, out, filename, link, thumbnail); + super.renderIconAndFilename(ctx, out, fileValue, link, thumbnail); } } diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 3dbaa08b6cb..61115f5a333 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -39,6 +39,7 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbSequenceManager; import org.labkey.api.data.ImportAliasable; @@ -294,6 +295,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI dib = ((UpdateableTableInfo) getQueryTable()).persistRows(dib, context); dib = AttachmentDataIterator.getAttachmentDataIteratorBuilder(getQueryTable(), dib, user, context.getInsertOption().batch ? getAttachmentDirectory() : null, container, getAttachmentParentFactory()); + // file link data iterator? dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container); return dib; } @@ -553,7 +555,15 @@ protected DataIteratorBuilder _toDataIteratorBuilder(String debugName, List> convertedRows = new ArrayList<>(); + for (int i = 0; i < rows.size(); i++) + { + Map row = rows.get(i); + row = coerceTypes(row); + convertedRows.add(row); + } + return MapDataIterator.of(colNames, convertedRows, debugName); } @@ -713,6 +723,10 @@ protected Map coerceTypes(Map row) { value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass()); } + catch (ConvertHelper.FileLinkConversionException e) + { + throw e; + } catch (ConversionException e) { // That's OK, the transformation script may be able to fix up the value before it gets inserted diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index ea020b9ec6a..f0f2f9b96dd 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -31,6 +31,7 @@ import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; +import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MvUtil; @@ -458,7 +459,8 @@ else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedC Set columnNames = new HashSet<>(); for (ColumnDescriptor colDesc : colDescs) { - if (!columnNames.add(colDesc.name) && isThrowOnErrors()) + String name = colDesc.name; + if (!columnNames.add(name) && isThrowOnErrors()) { // TODO: This should be refactored to not throw this here, but rather, have the callers check themselves. It // is not in the interest of inferring columns that we validate duplicate columns. @@ -468,6 +470,23 @@ else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedC ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.SkipMothershipLogging, "true", true); throw e; } + + // use File converter for known file fields even if inferTypes = false. If inferTypes, this is already done. + if (!getInferTypes()) + { + Class knownColumnClass = null; + if (_columnInfoMap.containsKey(name)) + { + knownColumnClass = _columnInfoMap.get(name).getJavaClass(); + } + else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) + { + knownColumnClass = _columnInfoMap.get(renamedColumns.get(name)).getJavaClass(); + } + + if (File.class.equals(knownColumnClass) && String.class.equals(colDesc.clazz)) + colDesc.clazz = knownColumnClass; + } } _columns = colDescs; @@ -866,6 +885,10 @@ else if (column.isMvIndicator()) throw new ConversionException(sb.toString(), x); } + else if (x instanceof ConvertHelper.FileLinkConversionException) + { + throw x; + } else if (ERROR_VALUE_USE_ORIGINAL.equals(column.errorValues)) values[i] = fld; else @@ -899,6 +922,10 @@ else if (ERROR_VALUE_USE_ORIGINAL.equals(column.errorValues)) else throw new RuntimeException(e); } + if (e instanceof ConvertHelper.FileLinkConversionException) + { + throw e; + } if (null != _file) _log.error("failed loading file " + _file.getName() + " at line: " + _lineNum + " " + e, e); diff --git a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java index 3880c267301..fd7a1ef8f4f 100644 --- a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java +++ b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java @@ -246,12 +246,6 @@ public void addQueryFieldKeys(Set keys) keys.add(_objectURIFieldKey); } - @Override - protected String getFileName(RenderContext ctx, Object value) - { - return getFileName(ctx, value, false); - } - public static boolean filePathExist(String path, Container container, User user) { String davPath = path; @@ -292,7 +286,7 @@ public static boolean filePathExist(String path, Container container, User user) } @Override - protected String getFileName(RenderContext ctx, Object value, boolean isDisplay) + protected String getFileName(RenderContext ctx, Object value) { String result = value == null ? null : StringUtils.trimToNull(value.toString()); if (result != null) @@ -314,19 +308,23 @@ protected String getFileName(RenderContext ctx, Object value, boolean isDisplay) f = FileUtil.getAbsoluteCaseSensitiveFile(new File(result)); NetworkDrive.ensureDrive(f.getPath()); List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); + boolean valid = false; for (FileContentService.ContentType fileRootType : fileRootTypes) { result = relativize(f, FileContentService.get().getFileRoot(_container, fileRootType)); if (result != null) + { + valid = true; break; + } } if (result == null) { result = f.getName(); } - if (isDisplay && !f.exists() && !result.endsWith("(unavailable)")) - result += " (unavailable)"; + if ((!valid || !f.exists()) && !result.endsWith(UNAVAILABLE_FILE_SUFFIX)) + result += UNAVAILABLE_FILE_SUFFIX; } return result; } @@ -364,48 +362,78 @@ protected InputStream getFileContents(RenderContext ctx, Object ignore) throws F } @Override - protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String filename, boolean link, boolean thumbnail) + protected void renderIconAndFilename( + RenderContext ctx, + HtmlWriter out, + String fileValue /*Could be raw path value, or processed filename by `getFileName`*/, + boolean link, + boolean thumbnail) { Object value = getValue(ctx); - String s = value == null ? null : StringUtils.trimToNull(value.toString()); - if (s != null) + String strValue = value == null ? null : StringUtils.trimToNull(value.toString()); + if (strValue != null && !fileValue.endsWith(UNAVAILABLE_FILE_SUFFIX)) { File f; - if (s.startsWith("file:")) - f = new File(URI.create(s)); + if (strValue.startsWith("file:")) + f = new File(URI.create(strValue)); else - f = new File(s); + f = new File(strValue); if (!f.exists()) { - String fullPath = PipelineService.get().findPipelineRoot(_container).getRootPath().getAbsolutePath() + File.separator + AssayFileWriter.DIR_NAME + File.separator + value; - f = new File(fullPath); + // try all file root + List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); + for (FileContentService.ContentType fileRootType : fileRootTypes) + { + String fullPath = FileContentService.get().getFileRoot(_container, fileRootType).getAbsolutePath()+ File.separator + value; + f = new File(fullPath); + if (f.exists()) + break; + } } // It's probably a file, so check that first if (f.isFile()) { - super.renderIconAndFilename(ctx, out, filename, link, thumbnail); + super.renderIconAndFilename(ctx, out, strValue, link, thumbnail); } else if (f.isDirectory()) { - super.renderIconAndFilename(ctx, out, filename, Attachment.getFileIcon(".folder"), null, link, false); + super.renderIconAndFilename(ctx, out, strValue, Attachment.getFileIcon(".folder"), null, link, false); } else { // It's not on the file system anymore, so don't offer a link and tell the user it's unavailable - super.renderIconAndFilename(ctx, out, filename, Attachment.getFileIcon(filename), null, false, false); + super.renderIconAndFilename(ctx, out, strValue, Attachment.getFileIcon(fileValue), null, false, false); } } else { - super.renderIconAndFilename(ctx, out, filename, link, thumbnail); + super.renderIconAndFilename(ctx, out, fileValue, link, thumbnail); } } @Override public Object getDisplayValue(RenderContext ctx) { - return getFileName(ctx, super.getDisplayValue(ctx), true); + return getFileName(ctx, super.getDisplayValue(ctx)); + } + + @Override + public Object getJsonValue(RenderContext ctx) + { + return getDisplayValue(ctx); } + + @Override + public boolean isFilterable() + { + return false; + } + @Override + public boolean isSortable() + { + return false; + } + } diff --git a/core/src/org/labkey/core/query/UserAvatarDisplayColumnFactory.java b/core/src/org/labkey/core/query/UserAvatarDisplayColumnFactory.java index fdae14dc26a..96f51fa6eff 100644 --- a/core/src/org/labkey/core/query/UserAvatarDisplayColumnFactory.java +++ b/core/src/org/labkey/core/query/UserAvatarDisplayColumnFactory.java @@ -74,7 +74,7 @@ public void renderDetailsCellContents(RenderContext ctx, HtmlWriter out) } @Override - protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String filename, @Nullable String fileIconUrl, @Nullable String popupIconUrl, boolean link, boolean thumbnail) + protected void renderIconAndFilename(RenderContext ctx, HtmlWriter out, String fileValue, @Nullable String fileIconUrl, @Nullable String popupIconUrl, boolean link, boolean thumbnail) { renderDetailsCellContents(ctx, out); } diff --git a/experiment/src/org/labkey/experiment/DefaultCustomPropertyRenderer.java b/experiment/src/org/labkey/experiment/DefaultCustomPropertyRenderer.java index 0186d9b1e72..51c60e04d86 100644 --- a/experiment/src/org/labkey/experiment/DefaultCustomPropertyRenderer.java +++ b/experiment/src/org/labkey/experiment/DefaultCustomPropertyRenderer.java @@ -32,6 +32,8 @@ import java.util.Date; import java.util.List; +import static org.labkey.api.data.AbstractFileDisplayColumn.UNAVAILABLE_FILE_SUFFIX; + /** * Responsible for showing custom field values (like assay run properties or sample type columns) in experiment module detail pages. * User: jeckels @@ -57,7 +59,7 @@ public String getValue(ObjectProperty prop, List siblingProperti } if (o == null) { - o = f.toString(); + o = f.getName() + UNAVAILABLE_FILE_SUFFIX; } } diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index b5c8ad6509f..aa67ea0896f 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2063,7 +2063,7 @@ public static class FileLinkDataIterator extends WrapperDataIterator Object file = fileColumnValueMapping.saveFileColumnValue(user, c, path, col.getName(), value); assert file instanceof FileLike; value = ((FileLike)file).toNioPathForRead().toString(); - savedFileName[index] = (String)value; + savedFileName[index] = (String)value;// } catch (QueryUpdateServiceException ex) { diff --git a/experiment/src/org/labkey/experiment/XarReader.java b/experiment/src/org/labkey/experiment/XarReader.java index 341ea7a210e..8bee215f1f4 100644 --- a/experiment/src/org/labkey/experiment/XarReader.java +++ b/experiment/src/org/labkey/experiment/XarReader.java @@ -1970,7 +1970,7 @@ private Map readPropertyCollection(PropertyCollectionTyp } else if (propType == PropertyType.FILE_LINK && !StringUtils.isEmpty(value) && value.startsWith(FILE_ROOT_SUBSTITUTION) && _fileRootPath != null) { - value = getFileRootSubstitutedFilePath(value, _fileRootPath); + value = getFileRootSubstitutedFilePath(value, _fileRootPath);// } if (StudyPublishService.AUTO_LINK_TARGET_PROPERTY_URI.equals(simpleProp.getOntologyEntryURI()) && value != null) diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index c2794b0db88..7a76a7fa4a9 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -1033,7 +1033,7 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) Object oldValue = run.getProperty(propertyDescriptor); if (propertyDescriptor.getPropertyType() == PropertyType.FILE_LINK && (value instanceof MultipartFile || value instanceof SpringAttachmentFile)) { - value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME); + value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME);// } ForeignKey fk = col.getFk(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 10abe5ad1e6..3c22eb85502 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -2092,7 +2092,7 @@ private Map> updateSampleFilePaths(ExpSampleT List fileDomainProps = sampleType.getDomain() .getProperties().stream() - .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList(); + .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList();// if (fileDomainProps.isEmpty()) return sampleFileRenames; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index c80f7c0133a..38cfbec6c6d 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -462,6 +462,7 @@ public List> insertRows(User user, Container container, List assert _sampleType != null : "SampleType required for insert/update, but not required for read/delete"; // insertRows with lineage is pretty good at deadlocking against itself, so use retry loop + // convert types DbScope scope = getSchema().getDbSchema().getScope(); List> results = scope.executeWithRetry(transaction -> super._insertRowsUsingDIB(user, container, rows, getDataIteratorContext(errors, InsertOption.INSERT, configParameters), extraScriptContext)); diff --git a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java index 7003cd272be..0a43656a7eb 100644 --- a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java +++ b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java @@ -424,7 +424,7 @@ public Object get(int i) if (value instanceof String filePath) return getFileRootSubstitutedFilePath(filePath, _fileRootPath); } - return value; + return value;// } } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index c3b6f37a4e6..92f997c63d5 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -198,6 +198,7 @@ import org.springframework.dao.DataIntegrityViolationException; import org.springframework.validation.BindException; +import java.io.File; import java.io.IOException; import java.math.BigDecimal; import java.sql.ResultSet; @@ -3338,7 +3339,8 @@ private void parseData(User user, } // let DataIterator do conversions - col.clazz = String.class; + if (!File.class.equals(col.clazz)) + col.clazz = String.class; if (columnMap.containsKey(name)) name = columnMap.get(name); diff --git a/study/src/org/labkey/study/query/DatasetUpdateService.java b/study/src/org/labkey/study/query/DatasetUpdateService.java index 74087264a23..7d9fea30d4b 100644 --- a/study/src/org/labkey/study/query/DatasetUpdateService.java +++ b/study/src/org/labkey/study/query/DatasetUpdateService.java @@ -248,6 +248,7 @@ public int mergeRows(User user, Container container, DataIteratorBuilder rows, B @Override public int loadRows(User user, Container container, DataIteratorBuilder rows, DataIteratorContext context, @Nullable Map extraScriptContext) { + // rows convert? int count = _importRowsUsingDIB(user, container, rows, null, context, extraScriptContext); if (count > 0 && !Boolean.TRUE.equals(context.getConfigParameterBoolean(Config.SkipResyncStudy))) { From 4f7a2606076a0115e98b9b21f27ba9e541815d51 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 16 Jul 2025 14:37:56 -0700 Subject: [PATCH 02/25] fix relative path import --- api/src/org/labkey/api/data/ExpDataFileConverter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 88f55935409..a142bfe7057 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -227,7 +227,7 @@ public Object convert(Class type, Object value) // If we have a file path, make sure it's supposed to be visible in the current container if (f != null) { - if (!f.isFile()) + if (f.isDirectory()) { if (value instanceof String) throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value); From a272233d48025bad91a0b3d28ed80043e7161caa Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 16 Jul 2025 16:23:03 -0700 Subject: [PATCH 03/25] fix invalid path that starts with file scheme, or if under root but doesn't exist --- .../labkey/api/data/ExpDataFileConverter.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index a142bfe7057..5136be87b18 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -247,20 +247,23 @@ public Object convert(Class type, Object value) f = new File(root.getRootPath(), f.getPath()); } - if (root.isUnderRoot(f)) + if (root.isUnderRoot(f) && f.isFile()) { return f; } } // It's possible to have the file root and pipeline root pointed at different paths - FileContentService fileContent = FileContentService.get(); - if (fileContent != null) + if (f.isFile()) { - File fileRoot = fileContent.getFileRoot(container); - if (fileRoot != null && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + FileContentService fileContent = FileContentService.get(); + if (fileContent != null) { - return f; + File fileRoot = fileContent.getFileRoot(container); + if (fileRoot != null && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + { + return f; + } } } @@ -354,7 +357,16 @@ private File convertToFile(Object value, @NotNull Container container, @NotNull return result; } } + + String filePath = value.toString(); + if (filePath.startsWith("file:")) + { + URI uri = FileUtil.createUri(filePath); + if (FileUtil.FILE_SCHEME.equals(uri.getScheme())) + return new File(uri); + } + // Otherwise, treat it as a plain path - return FILE_CONVERTER.convert(File.class, webdav); + return FILE_CONVERTER.convert(File.class, filePath); } } From 333ab4f10a44504061aeabad5927c9b5b059794d Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 21 Jul 2025 09:58:46 -0700 Subject: [PATCH 04/25] fix assay import exception handling, disable sort filter for attachment columns --- .../org/labkey/api/assay/DefaultAssayRunCreator.java | 4 +++- .../org/labkey/api/data/AttachmentDisplayColumn.java | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 82c29ade74b..de38bad5128 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -444,7 +444,7 @@ public ExpExperiment saveExperimentRun( return batch; } - catch (ExperimentException | IOException e) + catch (ExperimentException | IOException | ConvertHelper.FileLinkConversionException e) { // clean up the run results file dir here if it was created, for non-async imports AssayResultsFileWriter resultsFileWriter = new AssayResultsFileWriter<>(context.getProtocol(), run, null); @@ -454,6 +454,8 @@ public ExpExperiment saveExperimentRun( if (e instanceof ExperimentException) throw (ExperimentException)e; + else if (e instanceof ConvertHelper.FileLinkConversionException) + throw new ExperimentException(e.getMessage()); else throw new ExperimentException(e); } diff --git a/api/src/org/labkey/api/data/AttachmentDisplayColumn.java b/api/src/org/labkey/api/data/AttachmentDisplayColumn.java index a27a6c60f78..a75bef30ada 100644 --- a/api/src/org/labkey/api/data/AttachmentDisplayColumn.java +++ b/api/src/org/labkey/api/data/AttachmentDisplayColumn.java @@ -77,6 +77,16 @@ protected InputStream getFileContents(RenderContext ctx, Object value) throws Fi return AttachmentService.get().getInputStream(parent, filename); } + @Override + public boolean isFilterable() + { + return false; + } + @Override + public boolean isSortable() + { + return false; + } /** * Created by xingyang on 12/8/15. From d27a817dbb10ff80cfddc79140308f189a9bdc4f Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 21 Jul 2025 11:17:39 -0700 Subject: [PATCH 05/25] allow sort filter for attachment columns --- .../org/labkey/api/data/AttachmentDisplayColumn.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/api/src/org/labkey/api/data/AttachmentDisplayColumn.java b/api/src/org/labkey/api/data/AttachmentDisplayColumn.java index a75bef30ada..a27a6c60f78 100644 --- a/api/src/org/labkey/api/data/AttachmentDisplayColumn.java +++ b/api/src/org/labkey/api/data/AttachmentDisplayColumn.java @@ -77,16 +77,6 @@ protected InputStream getFileContents(RenderContext ctx, Object value) throws Fi return AttachmentService.get().getInputStream(parent, filename); } - @Override - public boolean isFilterable() - { - return false; - } - @Override - public boolean isSortable() - { - return false; - } /** * Created by xingyang on 12/8/15. From 8d8c1dd3dc9f7b634942ea72f7811175cc8078f0 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 21 Jul 2025 19:40:27 -0700 Subject: [PATCH 06/25] Bug fixes for cross folder import --- .../labkey/api/data/ExpDataFileConverter.java | 9 ++++-- api/src/org/labkey/api/reader/DataLoader.java | 11 ++++++- .../study/assay/FileLinkDisplayColumn.java | 32 ++++++++++++++++--- .../labkey/experiment/ExpDataIterators.java | 1 + 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 5136be87b18..69fa1f1f7a3 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -48,6 +48,7 @@ import java.net.URISyntaxException; import java.util.Collection; import java.util.Collections; +import java.util.List; import static org.labkey.api.dataiterator.SimpleTranslator.getFileRootSubstitutedFilePath; @@ -259,10 +260,12 @@ public Object convert(Class type, Object value) FileContentService fileContent = FileContentService.get(); if (fileContent != null) { - File fileRoot = fileContent.getFileRoot(container); - if (fileRoot != null && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); + for (FileContentService.ContentType fileRootType : fileRootTypes) { - return f; + File fileRoot = FileContentService.get().getFileRoot(container, fileRootType); + if (fileRoot != null && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + return f; } } } diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index f0f2f9b96dd..cde838d21ca 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -982,7 +982,16 @@ public DataIterator getDataIterator(final DataIteratorContext context) /** Actually create an instance of DataIterator to use, which might be subclass-specific */ protected DataIterator createDataIterator(DataIteratorContext context) throws IOException { - return new _DataIterator(context, getActiveColumns(), isScrollable()); + ColumnDescriptor[] columnDescriptors = getActiveColumns(); + if (context.isCrossFolderImport() || context.isCrossTypeImport()) + { + for (ColumnDescriptor columnDescriptor : columnDescriptors) + { + if (columnDescriptor.clazz.equals(File.class)) + columnDescriptor.clazz = String.class; // defer file path validation for cross type/folder import + } + } + return new _DataIterator(context, columnDescriptors, isScrollable()); } protected class _DataIterator implements ScrollableDataIterator, MapDataIterator diff --git a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java index fd7a1ef8f4f..7e23d8cb357 100644 --- a/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java +++ b/api/src/org/labkey/api/study/assay/FileLinkDisplayColumn.java @@ -20,11 +20,11 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.admin.CoreUrls; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.attachments.Attachment; import org.labkey.api.data.AbstractFileDisplayColumn; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.RemappingDisplayColumnFactory; import org.labkey.api.data.RenderContext; @@ -57,6 +57,7 @@ import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -309,13 +310,34 @@ protected String getFileName(RenderContext ctx, Object value) NetworkDrive.ensureDrive(f.getPath()); List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); boolean valid = false; - for (FileContentService.ContentType fileRootType : fileRootTypes) + List containers = new ArrayList<>(); + containers.add(_container); + // Not ideal, but needed in case data is queried from cross folder context + if (ctx.get("folder") != null || ctx.get("container") != null) { - result = relativize(f, FileContentService.get().getFileRoot(_container, fileRootType)); - if (result != null) + Object folderObj = ctx.get("folder"); + if (folderObj == null) + folderObj = ctx.get("container"); + if (folderObj instanceof String containerId) { - valid = true; + Container dataContainer = ContainerManager.getForId(containerId); + if (dataContainer != null && !dataContainer.equals(_container)) + containers.add(dataContainer); + } + } + for (Container container : containers) + { + if (valid) break; + + for (FileContentService.ContentType fileRootType : fileRootTypes) + { + result = relativize(f, FileContentService.get().getFileRoot(container, fileRootType)); + if (result != null) + { + valid = true; + break; + } } } if (result == null) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index aa67ea0896f..f6d83233fdc 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2458,6 +2458,7 @@ private int _importSplitFile(TypeData typeData, File splitFile, Container dataCo configureLoader(loader, dataTable, null, true, aliasNames); if (loader instanceof TabLoader tabLoader) tabLoader.setIncludeComments(true); // don't skip lines that starts with "#" (if the original file is Excel) + QueryService.get().setEnvironment(QueryService.Environment.CONTAINER, dataContainer); return updateService.loadRows(_user, dataContainer, loader, _context, null); } catch (SQLException | IOException e) From 618ab5a3401059cf5dae21ea287d856f7c4eb1ac Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 24 Jul 2025 12:46:17 -0700 Subject: [PATCH 07/25] Selenium tests --- .../test/integration/MoveSamplesAction.ispec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts b/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts index 9a84b590b82..7a2ee57bc98 100644 --- a/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts +++ b/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts @@ -712,7 +712,7 @@ describe('Move Samples', () => { const sampleData = await _getSampleData(sampleRowId1, subfolder1Options, SAMPLE_TYPE_NAME_2, "RowId," + FILE_FIELD_1_NAME); expect(sampleData.length).toBe(1); - expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith(subfolder1Options.containerPath + "/@files/sampletype/fileA.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith("sampletype/fileA.txt")).toBe(true); await verifyDetailedAuditLogs(topFolderOptions, subfolder1Options, [sampleRowId1], undefined, userComment, [{ @@ -745,8 +745,8 @@ describe('Move Samples', () => { expect(updateCounts.sampleFiles).toBe(2); const sampleData = await _getSampleData(sampleRowId1, topFolderOptions, SAMPLE_TYPE_NAME_2, "RowId," + FILE_FIELD_1_NAME + "," + FILE_FIELD_2_NAME); expect(sampleData.length).toBe(1); - expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith(topFolderOptions.containerPath + "/@files/sampletype/fileB.txt")).toBe(true); - expect(getSlashedPath(sampleData[0][FILE_FIELD_2_NAME]).endsWith(topFolderOptions.containerPath + "/@files/sampletype/fileC.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith("sampletype/fileB.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_2_NAME]).endsWith("sampletype/fileC.txt")).toBe(true); await verifyDetailedAuditLogs(subfolder1Options, topFolderOptions, [sampleRowId1], undefined, undefined, [{ oldValue: getSlashedPath(subfolder1Options.containerPath + "/@files/sampletype/fileB.txt"), @@ -783,8 +783,8 @@ describe('Move Samples', () => { expect(updateCounts.sampleFiles).toBe(2); const sampleData = await _getSampleData(sampleRowId1, topFolderOptions, SAMPLE_TYPE_NAME_2, "RowId," + FILE_FIELD_1_NAME + "," + FILE_FIELD_2_NAME); expect(sampleData.length).toBe(1); - expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith(topFolderOptions.containerPath + "/@files/sampletype/fileD-1.txt")).toBe(true); - expect(getSlashedPath(sampleData[0][FILE_FIELD_2_NAME]).endsWith(topFolderOptions.containerPath + "/@files/sampletype/fileE.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith("sampletype/fileD-1.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_2_NAME]).endsWith("sampletype/fileE.txt")).toBe(true); await verifyDetailedAuditLogs(subfolder1Options, topFolderOptions, [sampleRowId1], undefined, undefined, [{ oldValue: getSlashedPath(subfolder1Options.containerPath + "/@files/sampletype/fileD.txt"), @@ -815,7 +815,7 @@ describe('Move Samples', () => { expect(updateCounts.sampleFiles).toBe(1); const sampleData = await _getSampleData(sampleRowId1, subfolder2Options, SAMPLE_TYPE_NAME_2, "RowId," + FILE_FIELD_1_NAME); expect(sampleData.length).toBe(1); - expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith(subfolder2Options.containerPath + "/@files/sampletype/fileF.txt")).toBe(true); + expect(getSlashedPath(sampleData[0][FILE_FIELD_1_NAME]).endsWith("sampletype/fileF.txt")).toBe(true); await verifyDetailedAuditLogs(subfolder1Options, subfolder2Options, [sampleRowId1], undefined, undefined, [{ oldValue: getSlashedPath(subfolder1Options.containerPath + "/@files/sampletype/fileF.txt"), From b7f2c1f1bb4a94c8290e21a56d3f11a47de1f8a6 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 24 Jul 2025 18:52:48 -0700 Subject: [PATCH 08/25] Fix assayfile import --- .../api/assay/DefaultAssayRunCreator.java | 10 +++ .../labkey/api/data/ExpDataFileConverter.java | 76 +++++++++++++++---- .../org/labkey/api/query/QueryService.java | 3 +- 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index de38bad5128..4eb84569fe4 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -69,6 +69,7 @@ import org.labkey.api.pipeline.PipelineValidationException; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.PropertyValidationError; +import org.labkey.api.query.QueryService; import org.labkey.api.query.SimpleValidationError; import org.labkey.api.query.ValidationError; import org.labkey.api.query.ValidationException; @@ -83,10 +84,12 @@ import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.api.writer.ContainerUser; import org.labkey.vfs.FileLike; +import org.labkey.vfs.FileSystemLike; import java.io.File; import java.io.FileFilter; import java.io.IOException; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -381,6 +384,13 @@ public ExpExperiment saveExperimentRun( AssayResultsFileWriter resultsFileWriter = new AssayResultsFileWriter(context.getProtocol(), run, null); resultsFileWriter.savePostedFiles(context); + Path assayResultsRunDir = AssayResultsFileWriter.getAssayFilesDirectoryPath(run); + if (null != assayResultsRunDir && !FileUtil.hasCloudScheme(assayResultsRunDir)) + { + FileLike assayResultFileRoot = FileSystemLike.wrapFile(assayResultsRunDir); + if (assayResultFileRoot != null) + QueryService.get().setEnvironment(QueryService.Environment.ASSAYFILESPATH, assayResultFileRoot); + } importResultData(context, run, inputDatas, outputDatas, info, xarContext, transformResult, insertedDatas); Integer reRunId = context.getReRunId(); diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 69fa1f1f7a3..0be5ddf4e38 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -42,6 +42,7 @@ import org.labkey.api.view.NotFoundException; import org.labkey.api.webdav.WebdavResource; import org.labkey.api.webdav.WebdavService; +import org.labkey.vfs.FileLike; import java.io.File; import java.net.URI; @@ -207,8 +208,7 @@ private static DataType getDataType(@NotNull File file, @NotNull Collection fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); + for (FileContentService.ContentType fileRootType : fileRootTypes) { - List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); - for (FileContentService.ContentType fileRootType : fileRootTypes) + File fileRoot = fileContent.getFileRoot(container, fileRootType); + if (fileRoot != null) { - File fileRoot = FileContentService.get().getFileRoot(container, fileRootType); - if (fileRoot != null && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + if (f.isFile() && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) return f; + + if (!f.isAbsolute()) + { + try + { + File fileWithRoot = FileUtil.appendPath(fileRoot, Path.parse(f.getPath())); + if (fileWithRoot.isFile() && URIUtil.isDescendant(fileRoot.toURI(), fileWithRoot.toURI())) + return fileWithRoot; + } + catch (IllegalArgumentException ignore) + { + } + } } } } @@ -279,7 +296,24 @@ public Object convert(Class type, Object value) return null; } - private File convertToFile(Object value, @NotNull Container container, @NotNull User user, @Nullable String fileRootPath) + @Override + public Object convert(Class type, Object value) + { + try + { + return _convert(type, value); + } + catch (ConvertHelper.FileLinkConversionException e) + { + throw e; + } + catch (Exception e) + { + throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value.toString()); + } + } + + private File convertToFile(Object value, @NotNull Container container, @NotNull User user, @Nullable String fileRootPath, @Nullable FileLike assayResultFileRoot) { if (value instanceof File f) { @@ -320,6 +354,20 @@ private File convertToFile(Object value, @NotNull Container container, @NotNull String webdav = value.toString(); if (null != StringUtils.trimToNull(webdav)) { + if (assayResultFileRoot != null) + { + try + { + FileLike assayResultFile = assayResultFileRoot.resolveChild(webdav); + if (assayResultFile.isFile()) + return FileUtil.toFileForRead(assayResultFile); + } + catch (Exception ignore) + { + } + + } + webdav = getFileRootSubstitutedFilePath(webdav, fileRootPath); Path path = Path.decode(FileUtil.encodeForURL(webdav, true /*Issue 51207*/).replace(AppProps.getInstance().getBaseServerUrl() + AppProps.getInstance().getContextPath(), "")); WebdavResource resource = WebdavService.get().getResolver().lookup(path); diff --git a/api/src/org/labkey/api/query/QueryService.java b/api/src/org/labkey/api/query/QueryService.java index efc0abd2594..0e6a8d6301e 100644 --- a/api/src/org/labkey/api/query/QueryService.java +++ b/api/src/org/labkey/api/query/QueryService.java @@ -319,7 +319,8 @@ enum Environment USER(JdbcType.OTHER), CONTAINER(JdbcType.OTHER), ACTION(JdbcType.OTHER), - FILEROOTPATH(JdbcType.VARCHAR); + FILEROOTPATH(JdbcType.VARCHAR), + ASSAYFILESPATH(JdbcType.OTHER); public final JdbcType type; From 7f0bfbcc88d7a35bcf3a3db4d7f473f48737e579 Mon Sep 17 00:00:00 2001 From: XingY Date: Sat, 26 Jul 2025 21:20:03 -0700 Subject: [PATCH 09/25] add test coverage for study dataset --- .../study/StudyDatasetFileFieldTest.java | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index a5a6f7ba5ba..0ae64b2f73e 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -14,9 +14,13 @@ import org.labkey.test.components.domain.DomainFormPanel; import org.labkey.test.components.ext4.Checkbox; import org.labkey.test.pages.DatasetInsertPage; +import org.labkey.test.pages.ImportDataPage; +import org.labkey.test.pages.ViewDatasetDataPage; import org.labkey.test.pages.study.DatasetDesignerPage; import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.DataRegionTable; +import org.labkey.test.util.FileBrowserHelper; +import org.openqa.selenium.NoSuchElementException; import java.io.File; import java.io.IOException; @@ -105,6 +109,8 @@ public void testFileField() throws IOException File downloadedFile = doAndWaitForDownload(() -> waitAndClick(WAIT_FOR_JAVASCRIPT, Locator.tagWithAttribute("a", "title", "Download attached file"), 0)); checker().verifyTrue("Incorrect file name ", FileUtils.contentEquals(downloadedFile, inputFile)); + FileBrowserHelper.FileDetailInfo fileInfoOriginalFile = _fileBrowserHelper.getFileDetailInfo(getProjectName(), "sample.txt"); + goToFolderManagement().goToExportTab(); new Checkbox(Locator.tagWithText("label", "Files").precedingSibling("input").findElement(getDriver())).check(); File exportedFolderFile = doAndWaitForDownload(()->findButton("Export").click()); @@ -158,6 +164,76 @@ public void testFileField() throws IOException setFormElement(Locator.name("quf_intField"), "2"); setFormElement(Locator.name("quf_fileField"), updateFile.toString()); clickButton("Submit"); + + FileBrowserHelper.FileDetailInfo fileInfoImportedFile = _fileBrowserHelper.getFileDetailInfo(IMPORT_PROJECT, "sample.txt"); + + // error case: import, update, merge with invalid file path + ViewDatasetDataPage datasetDataPage = new ViewDatasetDataPage(getDriver()); + ImportDataPage importDataPage = datasetDataPage.importBulkData(); + importDataPage.setCopyPasteMerge(false, false); + importFilePathError("badNew", "101", fileInfoOriginalFile.absoluteFilePath()); + importFilePathError("badNew", "101", fileInfoOriginalFile.dataFileUrl()); + importFilePathError("badNew", "101", fileInfoOriginalFile.webDavUrl()); + importFilePathError("badNew", "101", "bad.txt"); + importFilePathError("badNew", "101", fileInfoOriginalFile.absoluteFilePath().replace("sample.txt", "")); + importFilePathError("badNew", "101", "."); + importFilePathError("badNew", "101", "../.."); + importDataPage.setCopyPasteMerge(true, true); + importFilePathError("badNew", "101", fileInfoOriginalFile.absoluteFilePath()); + importFilePathError("1", "2", fileInfoOriginalFile.dataFileUrl()); + importFilePathError("badNew", "101", fileInfoOriginalFile.webDavUrl()); + importFilePathError("1", "2", "bad.txt"); + importFilePathError("badNew", "101", fileInfoOriginalFile.absoluteFilePath().replace("sample.txt", "")); + importFilePathError("1", "2", "."); + importFilePathError("badNew", "101", "../.."); + importDataPage.setCopyPasteMerge(false, true); + importFilePathError("1", "2", fileInfoOriginalFile.absoluteFilePath()); + importFilePathError("1", "2", fileInfoOriginalFile.dataFileUrl()); + importFilePathError("1", "2", fileInfoOriginalFile.webDavUrl()); + importFilePathError("1", "2", "bad.txt"); + importFilePathError("1", "2", fileInfoOriginalFile.absoluteFilePath().replace("sample.txt", "")); + importFilePathError("1", "2", "."); + importFilePathError("1", "2", "../.."); + // happy case, import/update/merge with valid file path + importDataPage.setCopyPasteMerge(false, false); + String header = "ParticipantId\tSequenceNum\tfileField\n"; + String data = "2\t3\t" + fileInfoImportedFile.absoluteFilePath() + "\n" + + "3\t4\t" + fileInfoImportedFile.dataFileUrl() + "\n" + + "4\t5\t" + fileInfoImportedFile.webDavUrl() + "\n" + + "5\t6\t" + fileInfoImportedFile.webDavUrlRelative() + "\n"; + setFormElement(Locator.name("text"), header + data); + new ImportDataPage(getDriver()).submit(); + datasetDataPage = new ViewDatasetDataPage(getDriver()); + importDataPage = datasetDataPage.importBulkData(); + importDataPage.setCopyPasteMerge(false, true); + data = "2\t3\t" + fileInfoImportedFile.dataFileUrl() + "\n" + + "3\t4\t" + fileInfoImportedFile.webDavUrl() + "\n" + + "4\t5\t" + fileInfoImportedFile.webDavUrlRelative() + "\n" + + "5\t6\t" + fileInfoImportedFile.absoluteFilePath() + "\n"; + setFormElement(Locator.name("text"), header + data); + new ImportDataPage(getDriver()).submit(); + datasetDataPage = new ViewDatasetDataPage(getDriver()); + importDataPage = datasetDataPage.importBulkData(); + importDataPage.setCopyPasteMerge(true, true); + data += "6\t7\t" + fileInfoImportedFile.webDavUrlRelative() + "\n"; + setFormElement(Locator.name("text"), header + data); + new ImportDataPage(getDriver()).submit(); + } + + private void importFilePathError(String participantId, String sequenceNum, String filePath) + { + String header = "ParticipantId\tSequenceNum\tfileField\n"; + String data = participantId + "\t" + sequenceNum + "\t" + filePath + "\n"; + setFormElement(Locator.name("text"), header + data); + new ImportDataPage(getDriver()).submitExpectingError(); + try + { + waitForElementToBeVisible(Locator.xpath("//div[contains(@class, 'labkey-error')][contains(text(),'Invalid file path: " + filePath + "')]")); + } + catch(NoSuchElementException nse) + { + checker().fatal().error("Invalid file path error not present."); + } } protected void createDataset(String name) From 60a2b2a5af72c672ef1cb7b29f3e885187e800c8 Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 27 Jul 2025 17:47:44 -0700 Subject: [PATCH 10/25] fix various assay api, add tests for assay api --- .../api/assay/AbstractAssayTsvDataHandler.java | 9 ++++++++- .../api/assay/DefaultAssayRunCreator.java | 10 ++++++++-- .../api/assay/actions/AssayRunUploadForm.java | 7 ++++++- .../labkey/api/data/ExpDataFileConverter.java | 4 ++-- api/src/org/labkey/api/reader/DataLoader.java | 17 ++++++++++------- .../tests/study/StudyDatasetFileFieldTest.java | 5 +++++ 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 182d9eb0db6..02c00b3ee8b 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -34,6 +34,7 @@ import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.ForeignKey; import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.MvUtil; @@ -797,6 +798,8 @@ else if (sampleLookup.isLookup()) DomainProperty datePD = datePropFinder; DomainProperty targetStudyPD = targetStudyPropFinder; + ExpDataFileConverter expDataFileConverter = new ExpDataFileConverter(); + return DataIteratorUtil.mapTransformer(rawData, inputCols -> { List result = new ArrayList<>(inputCols); @@ -932,7 +935,11 @@ else if (o instanceof MvFieldWrapper mvWrapper) // File column values are stored as the absolute resolved path try { - File resolvedFile = AssayUploadFileResolver.resolve(o, container, pd); + File resolvedFile; + if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri()) && o instanceof String) + resolvedFile = (File) expDataFileConverter.convert(File.class, o); + else + resolvedFile = AssayUploadFileResolver.resolve(o, container, pd); if (resolvedFile != null) { o = resolvedFile; diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 4eb84569fe4..9b5150ff527 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -23,6 +23,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; +import org.labkey.api.action.ApiUsageException; import org.labkey.api.assay.actions.AssayRunUploadForm; import org.labkey.api.assay.pipeline.AssayRunAsyncContext; import org.labkey.api.assay.pipeline.AssayUploadPipelineJob; @@ -465,7 +466,7 @@ public ExpExperiment saveExperimentRun( if (e instanceof ExperimentException) throw (ExperimentException)e; else if (e instanceof ConvertHelper.FileLinkConversionException) - throw new ExperimentException(e.getMessage()); + throw new ApiUsageException(e.getMessage(), e); else throw new ExperimentException(e); } @@ -1083,13 +1084,18 @@ protected void savePropertyObject(ExpObject object, Container container, Map entry : properties.entrySet()) { DomainProperty pd = entry.getKey(); String value = entry.getValue(); // resolve any file links for batch or run properties - File resolvedFile = AssayUploadFileResolver.resolve(value, container, entry.getKey()); + File resolvedFile; + if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri()) && value instanceof String) + resolvedFile = (File) expDataFileConverter.convert(File.class, value); + else + resolvedFile = AssayUploadFileResolver.resolve(value, container, pd); if (resolvedFile != null) { value = resolvedFile.getAbsolutePath(); diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index b1930453c2d..edaec079e62 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -165,7 +165,12 @@ protected Map getPropertyMapFromRequest(List Date: Sun, 27 Jul 2025 17:51:21 -0700 Subject: [PATCH 11/25] clean --- experiment/src/org/labkey/experiment/ExpDataIterators.java | 2 +- experiment/src/org/labkey/experiment/XarReader.java | 2 +- experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java | 2 +- .../src/org/labkey/experiment/api/SampleTypeServiceImpl.java | 2 +- .../labkey/experiment/samples/AbstractExpFolderImporter.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index b6ccd08754a..fa8a67873ac 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2086,7 +2086,7 @@ public static class FileLinkDataIterator extends WrapperDataIterator Object file = fileColumnValueMapping.saveFileColumnValue(user, c, path, col.getName(), value); assert file instanceof FileLike; value = ((FileLike)file).toNioPathForRead().toString(); - savedFileName[index] = (String)value;// + savedFileName[index] = (String)value; } catch (QueryUpdateServiceException ex) { diff --git a/experiment/src/org/labkey/experiment/XarReader.java b/experiment/src/org/labkey/experiment/XarReader.java index 8bee215f1f4..341ea7a210e 100644 --- a/experiment/src/org/labkey/experiment/XarReader.java +++ b/experiment/src/org/labkey/experiment/XarReader.java @@ -1970,7 +1970,7 @@ private Map readPropertyCollection(PropertyCollectionTyp } else if (propType == PropertyType.FILE_LINK && !StringUtils.isEmpty(value) && value.startsWith(FILE_ROOT_SUBSTITUTION) && _fileRootPath != null) { - value = getFileRootSubstitutedFilePath(value, _fileRootPath);// + value = getFileRootSubstitutedFilePath(value, _fileRootPath); } if (StudyPublishService.AUTO_LINK_TARGET_PROPERTY_URI.equals(simpleProp.getOntologyEntryURI()) && value != null) diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index 7a76a7fa4a9..c2794b0db88 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -1033,7 +1033,7 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) Object oldValue = run.getProperty(propertyDescriptor); if (propertyDescriptor.getPropertyType() == PropertyType.FILE_LINK && (value instanceof MultipartFile || value instanceof SpringAttachmentFile)) { - value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME);// + value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME); } ForeignKey fk = col.getFk(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index d67618933bb..03ccd6a1e5e 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -2093,7 +2093,7 @@ private Map> updateSampleFilePaths(ExpSampleT List fileDomainProps = sampleType.getDomain() .getProperties().stream() - .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList();// + .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList(); if (fileDomainProps.isEmpty()) return sampleFileRenames; diff --git a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java index 0a43656a7eb..7003cd272be 100644 --- a/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java +++ b/experiment/src/org/labkey/experiment/samples/AbstractExpFolderImporter.java @@ -424,7 +424,7 @@ public Object get(int i) if (value instanceof String filePath) return getFileRootSubstitutedFilePath(filePath, _fileRootPath); } - return value;// + return value; } } } From 734f58d87d7849c76d1bfc3fb6c81047f5f3babb Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 27 Jul 2025 17:55:21 -0700 Subject: [PATCH 12/25] clean --- api/src/org/labkey/api/query/AbstractQueryUpdateService.java | 1 - .../src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java | 1 - study/src/org/labkey/study/query/DatasetUpdateService.java | 1 - 3 files changed, 3 deletions(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 81de44df3e7..0ab0a66d493 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -289,7 +289,6 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI dib = ((UpdateableTableInfo) getQueryTable()).persistRows(dib, context); dib = AttachmentDataIterator.getAttachmentDataIteratorBuilder(getQueryTable(), dib, user, context.getInsertOption().batch ? getAttachmentDirectory() : null, container, getAttachmentParentFactory()); - // file link data iterator? dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container); return dib; } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 975a6cabf02..36edcf87a17 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -462,7 +462,6 @@ public List> insertRows(User user, Container container, List assert _sampleType != null : "SampleType required for insert/update, but not required for read/delete"; // insertRows with lineage is pretty good at deadlocking against itself, so use retry loop - // convert types DbScope scope = getSchema().getDbSchema().getScope(); List> results = scope.executeWithRetry(transaction -> super._insertRowsUsingDIB(user, container, rows, getDataIteratorContext(errors, InsertOption.INSERT, configParameters), extraScriptContext)); diff --git a/study/src/org/labkey/study/query/DatasetUpdateService.java b/study/src/org/labkey/study/query/DatasetUpdateService.java index 7f088484909..4dfe33e5306 100644 --- a/study/src/org/labkey/study/query/DatasetUpdateService.java +++ b/study/src/org/labkey/study/query/DatasetUpdateService.java @@ -254,7 +254,6 @@ public int mergeRows(User user, Container container, DataIteratorBuilder rows, B @Override public int loadRows(User user, Container container, DataIteratorBuilder rows, DataIteratorContext context, @Nullable Map extraScriptContext) { - // rows convert? int count = _importRowsUsingDIB(user, container, rows, null, context, extraScriptContext); if (count > 0 && !Boolean.TRUE.equals(context.getConfigParameterBoolean(Config.SkipResyncStudy))) { From bdb93d69421e082dbb91ddd5486392516e5c8bdd Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 29 Jul 2025 16:36:01 -0700 Subject: [PATCH 13/25] rename --- .../org/labkey/api/assay/DefaultAssayRunCreator.java | 4 ++-- api/src/org/labkey/api/data/ConvertHelper.java | 4 ++-- .../org/labkey/api/data/ExpDataFileConverter.java | 12 ++++++------ .../labkey/api/query/AbstractQueryUpdateService.java | 2 +- api/src/org/labkey/api/reader/DataLoader.java | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 9b5150ff527..e9b2ecf9029 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -455,7 +455,7 @@ public ExpExperiment saveExperimentRun( return batch; } - catch (ExperimentException | IOException | ConvertHelper.FileLinkConversionException e) + catch (ExperimentException | IOException | ConvertHelper.FileConversionException e) { // clean up the run results file dir here if it was created, for non-async imports AssayResultsFileWriter resultsFileWriter = new AssayResultsFileWriter<>(context.getProtocol(), run, null); @@ -465,7 +465,7 @@ public ExpExperiment saveExperimentRun( if (e instanceof ExperimentException) throw (ExperimentException)e; - else if (e instanceof ConvertHelper.FileLinkConversionException) + else if (e instanceof ConvertHelper.FileConversionException) throw new ApiUsageException(e.getMessage(), e); else throw new ExperimentException(e); diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index 74c188a30fb..4773bca07af 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -451,9 +451,9 @@ public ContainerConversionException(String msg) } } - public static class FileLinkConversionException extends ConversionException implements SkipMothershipLogging + public static class FileConversionException extends ConversionException implements SkipMothershipLogging { - public FileLinkConversionException(String msg) + public FileConversionException(String msg) { super(msg); } diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 2baed328291..293db916b5f 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -233,9 +233,9 @@ private Object _convert(Class type, Object value) if (f.isDirectory()) { if (value instanceof String) - throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value); + throw new ConvertHelper.FileConversionException("Invalid file path: " + value); else - throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + f.getPath()); + throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); } // Strip out ".." and "." @@ -288,9 +288,9 @@ private Object _convert(Class type, Object value) } if (value instanceof String) - throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value); + throw new ConvertHelper.FileConversionException("Invalid file path: " + value); else - throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + f.getPath()); + throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); } } return null; @@ -303,13 +303,13 @@ public Object convert(Class type, Object value) { return _convert(type, value); } - catch (ConvertHelper.FileLinkConversionException e) + catch (ConvertHelper.FileConversionException e) { throw e; } catch (Exception e) { - throw new ConvertHelper.FileLinkConversionException("Invalid file path: " + value.toString()); + throw new ConvertHelper.FileConversionException("Invalid file path: " + value.toString()); } } diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 0ab0a66d493..61f04e8eb8b 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -716,7 +716,7 @@ protected Map coerceTypes(Map row) { value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass()); } - catch (ConvertHelper.FileLinkConversionException e) + catch (ConvertHelper.FileConversionException e) { throw e; } diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index 6c57c34b1d3..296fb103f98 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -888,7 +888,7 @@ else if (column.isMvIndicator()) throw new ConversionException(sb.toString(), x); } - else if (x instanceof ConvertHelper.FileLinkConversionException) + else if (x instanceof ConvertHelper.FileConversionException) { throw x; } @@ -925,7 +925,7 @@ else if (ERROR_VALUE_USE_ORIGINAL.equals(column.errorValues)) else throw new RuntimeException(e); } - if (e instanceof ConvertHelper.FileLinkConversionException) + if (e instanceof ConvertHelper.FileConversionException) { throw e; } From a2b48a8bfa4bfa000d3f623ffc419a7d02a29c71 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 29 Jul 2025 18:09:00 -0700 Subject: [PATCH 14/25] refactor for attachment --- api/src/org/labkey/api/reader/DataLoader.java | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index 296fb103f98..95daaff397b 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -329,7 +329,7 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws for (int f = 0; f < nCols; f++) { List classesToTest = new ArrayList<>(Arrays.asList(CONVERT_CLASSES)); - Class knownColumnClass = null; + Class knownColumnClass; int classIndex = -1; //NOTE: this means we have a header row @@ -339,27 +339,21 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws { String name = lineFields[0][f]; name = StringUtilsLabKey.sanitizeSeparatorsAndTrim(name); - if (_columnInfoMap.containsKey(name)) - { - //preferentially use this class if it matches - knownColumnClass = _columnInfoMap.get(name).getJavaClass(); - classesToTest.add(0, knownColumnClass); - } - else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) + + ColumnInfo knownColumn = getKnownColumn(name, renamedColumns); + + if (knownColumn != null) { - knownColumnClass = _columnInfoMap.get(renamedColumns.get(name)).getJavaClass(); + knownColumnClass = knownColumn.getJavaClass(); classesToTest.add(0, knownColumnClass); + + // Issue 49830: if we know the column class is File based on the columnInfoMap, use it instead of trying to infer the class based on the data + if (setFileColDescriptor(colDescs[f], knownColumn)) + continue; } } } - // Issue 49830: if we know the column class is File based on the columnInfoMap, use it instead of trying to infer the class based on the data - if (File.class.equals(knownColumnClass)) - { - colDescs[f].clazz = knownColumnClass; - continue; - } - for (int line = inferStartLine; line < numLines; line++) { if (f >= lineFields[line].length) @@ -474,25 +468,39 @@ else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedC // use File converter for known file fields even if inferTypes = false. If inferTypes, this is already done. if (!getInferTypes()) - { - ColumnInfo knownColumn = null; - Class knownColumnClass; - if (_columnInfoMap.containsKey(name)) - knownColumn = _columnInfoMap.get(name); - else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) - knownColumn = _columnInfoMap.get(renamedColumns.get(name)); + setFileColDescriptor(colDesc, getKnownColumn(name, renamedColumns)); + } - if (knownColumn != null && String.class.equals(colDesc.clazz)) - { - knownColumnClass = knownColumn.getJavaClass(); - if (File.class.equals(knownColumnClass) && !knownColumn.getPropertyType().equals(PropertyType.ATTACHMENT)) // skip attachment conversion - colDesc.clazz = File.class; - } + _columns = colDescs; + } - } + private boolean setFileColDescriptor(ColumnDescriptor colDesc, @Nullable ColumnInfo knownColumn) + { + if (knownColumn == null) + return false; + + Class knownColumnClass = knownColumn.getJavaClass(); + + if (File.class.equals(knownColumnClass)) + { + // TODO. Issue 53498 handle attachment conversion with incoming merge from 25.7 + if (!PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) + colDesc.clazz = knownColumnClass; + + return true; } - _columns = colDescs; + return false; + } + + private ColumnInfo getKnownColumn(String name, @NotNull Map renamedColumns) + { + ColumnInfo knownColumn = null; + if (_columnInfoMap.containsKey(name)) + knownColumn = _columnInfoMap.get(name); + else if (renamedColumns.containsKey(name) && _columnInfoMap.containsKey(renamedColumns.get(name))) + knownColumn = _columnInfoMap.get(renamedColumns.get(name)); + return knownColumn; } protected String getDefaultColumnName(int col) From 461682cadab081414ba04d30d1fc5517cd56ed09 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 29 Jul 2025 18:14:26 -0700 Subject: [PATCH 15/25] refactor for attachment --- .../org/labkey/api/query/AbstractQueryUpdateService.java | 6 ++++++ api/src/org/labkey/api/reader/DataLoader.java | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 61f04e8eb8b..e498e4a6453 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -714,6 +714,12 @@ protected Map coerceTypes(Map row) { try { + // TODO. Issue 53498: handle attachment conversion with incoming merge from 25.7 +// if (PropertyType.ATTACHMENT.equals(col.getPropertyType()) && value instanceof String strVal) +// { +// if (!StringUtils.isEmpty(strVal)) +// throw new ConvertHelper.FileConversionException("Invalid attachment value: " + strVal); +// } value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass()); } catch (ConvertHelper.FileConversionException e) diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index 95daaff397b..6cbd9af8412 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -483,7 +483,11 @@ private boolean setFileColDescriptor(ColumnDescriptor colDesc, @Nullable ColumnI if (File.class.equals(knownColumnClass)) { - // TODO. Issue 53498 handle attachment conversion with incoming merge from 25.7 + // TODO. Issue 53498: handle attachment conversion with incoming merge from 25.7 +// if (PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) +// colDesc.clazz = AttachmentData.class; +// else +// colDesc.clazz = knownColumnClass; if (!PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) colDesc.clazz = knownColumnClass; From 36866fdd285df024e095ba9e6f43260d3734cc31 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 30 Jul 2025 17:58:10 -0700 Subject: [PATCH 16/25] code review changes --- .../labkey/api/data/ExpDataFileConverter.java | 111 ++++++++++-------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 293db916b5f..c88bc63fd9f 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -59,7 +59,7 @@ */ public class ExpDataFileConverter implements Converter { - private static final Converter FILE_CONVERTER = new FileConverter(); + public static final Converter FILE_CONVERTER = new FileConverter(); public static ExpData resolveExpData(JSONObject dataObject, @NotNull Container container, @NotNull User user, @NotNull Collection knownTypes) { @@ -224,75 +224,82 @@ private Object _convert(Class type, Object value) // Don't bother resolving if we don't know the container, or we don't know the user has permission to the container if (type.isAssignableFrom(File.class) && container != null && user != null && container.hasPermission(user, ReadPermission.class)) + return convert(value, fileRootPath, assayResultFileRoot, container, user); + + return null; + } + + // TODO, refactor more usages to call this method directly without using needing to set/getEnvironment + public static File convert(@NotNull Object value, @Nullable String fileRootPath, @Nullable FileLike assayResultFileRoot, Container container, User user) + { + File f = convertToFile(value, container, user, fileRootPath, assayResultFileRoot); + + // If we have a file path, make sure it's supposed to be visible in the current container + if (f != null) { - File f = convertToFile(value, container, user, fileRootPath, assayResultFileRoot); + if (f.isDirectory()) + { + if (value instanceof String) + throw new ConvertHelper.FileConversionException("Invalid file path: " + value); + else + throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); + } + + // Strip out ".." and "." + f = FileUtil.resolveFile(f); - // If we have a file path, make sure it's supposed to be visible in the current container - if (f != null) + PipeRoot root = PipelineService.get().getPipelineRootSetting(container); + if (root != null) { - if (f.isDirectory()) + File fileUnderRoot = f; + if (!f.isAbsolute()) { - if (value instanceof String) - throw new ConvertHelper.FileConversionException("Invalid file path: " + value); - else - throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); + // Interpret relative paths based on the file root + fileUnderRoot = FileUtil.appendPath(root.getRootPath(), Path.parse(f.getPath())); } - // Strip out ".." and "." - f = FileUtil.resolveFile(f); - - PipeRoot root = PipelineService.get().getPipelineRootSetting(container); - if (root != null) + if (root.isUnderRoot(fileUnderRoot) && fileUnderRoot.isFile()) { - File fileUnderRoot = f; - if (!f.isAbsolute()) - { - // Interpret relative paths based on the file root - fileUnderRoot = new File(root.getRootPath(), f.getPath()); - } - - if (root.isUnderRoot(fileUnderRoot) && fileUnderRoot.isFile()) - { - return fileUnderRoot; - } + return fileUnderRoot; } + } - FileContentService fileContent = FileContentService.get(); + FileContentService fileContent = FileContentService.get(); - // It's possible to have the file root and pipeline root pointed at different paths - if (fileContent != null) + // It's possible to have the file root and pipeline root pointed at different paths + if (fileContent != null) + { + List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); + for (FileContentService.ContentType fileRootType : fileRootTypes) { - List fileRootTypes = List.of(FileContentService.ContentType.files, FileContentService.ContentType.pipeline, FileContentService.ContentType.assayfiles); - for (FileContentService.ContentType fileRootType : fileRootTypes) + File fileRoot = fileContent.getFileRoot(container, fileRootType); + if (fileRoot != null) { - File fileRoot = fileContent.getFileRoot(container, fileRootType); - if (fileRoot != null) - { - if (f.isFile() && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) - return f; + if (f.isFile() && URIUtil.isDescendant(fileRoot.toURI(), f.toURI())) + return f; - if (!f.isAbsolute()) + if (!f.isAbsolute()) + { + try + { + File fileWithRoot = FileUtil.appendPath(fileRoot, Path.parse(f.getPath())); + if (fileWithRoot.isFile() && URIUtil.isDescendant(fileRoot.toURI(), fileWithRoot.toURI())) + return fileWithRoot; + } + catch (IllegalArgumentException ignore) { - try - { - File fileWithRoot = FileUtil.appendPath(fileRoot, Path.parse(f.getPath())); - if (fileWithRoot.isFile() && URIUtil.isDescendant(fileRoot.toURI(), fileWithRoot.toURI())) - return fileWithRoot; - } - catch (IllegalArgumentException ignore) - { - } } } } } - - if (value instanceof String) - throw new ConvertHelper.FileConversionException("Invalid file path: " + value); - else - throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); } + + if (value instanceof String) + throw new ConvertHelper.FileConversionException("Invalid file path: " + value); + else + throw new ConvertHelper.FileConversionException("Invalid file path: " + f.getPath()); } + return null; } @@ -313,7 +320,7 @@ public Object convert(Class type, Object value) } } - private File convertToFile(Object value, @NotNull Container container, @NotNull User user, @Nullable String fileRootPath, @Nullable FileLike assayResultFileRoot) + public static File convertToFile(Object value, @NotNull Container container, @NotNull User user, @Nullable String fileRootPath, @Nullable FileLike assayResultFileRoot) { if (value instanceof File f) { @@ -323,7 +330,7 @@ private File convertToFile(Object value, @NotNull Container container, @NotNull if (value instanceof JSONObject json) { // Assume the same structure as the saveBatch and getBatch APIs work with - ExpData data = resolveExpData(json, container, user, Collections.emptyList()); + ExpData data = ExpDataFileConverter.resolveExpData(json, container, user, Collections.emptyList()); if (data != null && data.getFile() != null) { return data.getFile(); From 716c06ffddf907b6763a9f2aea5753d56198e9af Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 31 Jul 2025 18:51:22 -0700 Subject: [PATCH 17/25] Fix relative path --- api/src/org/labkey/api/data/ExpDataFileConverter.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index c88bc63fd9f..5b729d9cecc 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -254,8 +254,14 @@ public static File convert(@NotNull Object value, @Nullable String fileRootPath, File fileUnderRoot = f; if (!f.isAbsolute()) { - // Interpret relative paths based on the file root - fileUnderRoot = FileUtil.appendPath(root.getRootPath(), Path.parse(f.getPath())); + try + { + // Interpret relative paths based on the file root + fileUnderRoot = FileUtil.appendPath(root.getRootPath(), Path.parse(f.getPath())); + } + catch (IllegalArgumentException ignore) + { + } } if (root.isUnderRoot(fileUnderRoot) && fileUnderRoot.isFile()) From 72977bd1ff4d9379f38093bc6ec85768b35f3004 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 1 Aug 2025 11:44:25 -0700 Subject: [PATCH 18/25] no converter --- .../assay/AbstractAssayTsvDataHandler.java | 5 +- .../org/labkey/api/data/ConvertHelper.java | 11 ++++- .../labkey/api/data/ExpDataFileConverter.java | 47 ++++++++++++++++--- .../api/dataiterator/SimpleTranslator.java | 3 +- .../api/query/AbstractQueryUpdateService.java | 12 ++--- .../api/query/DefaultQueryUpdateService.java | 12 +++-- api/src/org/labkey/api/reader/DataLoader.java | 31 ------------ .../labkey/experiment/ExpDataIterators.java | 5 +- .../experiment/api/ExpRunTableImpl.java | 2 +- .../org/labkey/study/model/StudyManager.java | 3 +- 10 files changed, 74 insertions(+), 57 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 02c00b3ee8b..5b14c36d125 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -27,7 +27,6 @@ import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.collections.Sets; -import org.labkey.api.data.AssayResultsFileConverter; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; @@ -327,8 +326,8 @@ else if (mvIndicatorColumns.contains(column.name)) else column.errorValues = ERROR_VALUE; - if (run != null && column.clazz == File.class) - column.converter = new AssayResultsFileConverter(run); +// if (run != null && column.clazz == File.class) +// column.converter = new AssayResultsFileConverter(run); // TODO: fix } return loader; diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index 4773bca07af..48684c6e3cc 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -179,7 +179,7 @@ protected void register() _register(new SimpleTimeConverter(), SimpleTime.class); _register(new ShowRowsConverter(), ShowRows.class); _register(new UserConverter(), User.class); - _register(new ExpDataFileConverter(), File.class); + _register(new NoOpConverter(), File.class); // let data iterator handle conversion _register(new FacetingBehaviorTypeConverter(), FacetingBehaviorType.class); _register(new DefaultScaleConverter(), DefaultScaleType.class); _register(new SchemaKey.Converter(), SchemaKey.class); @@ -886,6 +886,15 @@ public Object convert(Class type, Object value) } } + public static class NoOpConverter implements Converter + { + @Override + public Object convert(Class type, Object value) + { + return value; + } + } + public static class FacetingBehaviorTypeConverter implements Converter { @Override diff --git a/api/src/org/labkey/api/data/ExpDataFileConverter.java b/api/src/org/labkey/api/data/ExpDataFileConverter.java index 5b729d9cecc..5075366e201 100644 --- a/api/src/org/labkey/api/data/ExpDataFileConverter.java +++ b/api/src/org/labkey/api/data/ExpDataFileConverter.java @@ -23,6 +23,7 @@ import org.json.JSONObject; import org.labkey.api.assay.AbstractAssayProvider; import org.labkey.api.assay.AssayDataType; +import org.labkey.api.assay.AssayResultsFileWriter; import org.labkey.api.exp.api.DataType; import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExpDataClass; @@ -210,11 +211,19 @@ private static DataType getDataType(@NotNull File file, @NotNull Collection coerceTypes(Map row) { try { - // TODO. Issue 53498: handle attachment conversion with incoming merge from 25.7 -// if (PropertyType.ATTACHMENT.equals(col.getPropertyType()) && value instanceof String strVal) -// { -// if (!StringUtils.isEmpty(strVal)) -// throw new ConvertHelper.FileConversionException("Invalid attachment value: " + strVal); -// } - value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass()); + if (PropertyType.FILE_LINK.equals(col.getPropertyType()) && value instanceof String strVal) + value = ExpDataFileConverter.convert(strVal); + else + value = ConvertUtils.convert(value.toString(), col.getJavaObjectClass()); } catch (ConvertHelper.FileConversionException e) { diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index ce2de00a17f..4d4c06109a5 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -26,6 +26,7 @@ import org.labkey.api.collections.CaseInsensitiveMapWrapper; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MvUtil; import org.labkey.api.data.Parameter; @@ -841,10 +842,15 @@ protected Object convertColumnValue(ColumnInfo col, Object value, User user, Con case DATE, TIME, TIMESTAMP: return value instanceof Date ? value : ConvertUtils.convert(value.toString(), Date.class); default: - if (PropertyType.FILE_LINK == col.getPropertyType() && (value instanceof MultipartFile || value instanceof AttachmentFile)) + if (PropertyType.FILE_LINK == col.getPropertyType()) { - FileLike fl = (FileLike)_fileColumnValueMapping.saveFileColumnValue(user, c, fileLinkDirPath, col.getName(), value); - value = fl.toNioPathForRead().toString(); + if ((value instanceof MultipartFile || value instanceof AttachmentFile)) + { + FileLike fl = (FileLike)_fileColumnValueMapping.saveFileColumnValue(user, c, fileLinkDirPath, col.getName(), value); + value = fl.toNioPathForRead().toString(); + } + + return ExpDataFileConverter.convert(value); } return ConvertUtils.convert(value.toString(), col.getJdbcType().getJavaClass()); } diff --git a/api/src/org/labkey/api/reader/DataLoader.java b/api/src/org/labkey/api/reader/DataLoader.java index 6cbd9af8412..5e326c436bf 100644 --- a/api/src/org/labkey/api/reader/DataLoader.java +++ b/api/src/org/labkey/api/reader/DataLoader.java @@ -346,10 +346,6 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws { knownColumnClass = knownColumn.getJavaClass(); classesToTest.add(0, knownColumnClass); - - // Issue 49830: if we know the column class is File based on the columnInfoMap, use it instead of trying to infer the class based on the data - if (setFileColDescriptor(colDescs[f], knownColumn)) - continue; } } } @@ -465,38 +461,11 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.SkipMothershipLogging, "true", true); throw e; } - - // use File converter for known file fields even if inferTypes = false. If inferTypes, this is already done. - if (!getInferTypes()) - setFileColDescriptor(colDesc, getKnownColumn(name, renamedColumns)); } _columns = colDescs; } - private boolean setFileColDescriptor(ColumnDescriptor colDesc, @Nullable ColumnInfo knownColumn) - { - if (knownColumn == null) - return false; - - Class knownColumnClass = knownColumn.getJavaClass(); - - if (File.class.equals(knownColumnClass)) - { - // TODO. Issue 53498: handle attachment conversion with incoming merge from 25.7 -// if (PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) -// colDesc.clazz = AttachmentData.class; -// else -// colDesc.clazz = knownColumnClass; - if (!PropertyType.ATTACHMENT.equals(knownColumn.getPropertyType())) - colDesc.clazz = knownColumnClass; - - return true; - } - - return false; - } - private ColumnInfo getKnownColumn(String name, @NotNull Map renamedColumns) { ColumnInfo knownColumn = null; diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index fa8a67873ac..de906100da3 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -32,6 +32,7 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CounterDefinition; import org.labkey.api.data.DbScope; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.RemapCache; import org.labkey.api.data.SimpleFilter; @@ -2052,7 +2053,7 @@ public boolean next() throws BatchValidationException // This should be used AFTER StandardDataIteratorBuilder, say at the beginning of PersistDataIteratorBuilder (below) // The incoming dataiterator should bound to target table and have complete ColumnInfo metadata // see SimpleQueryUpdateService.convertTypes() for similar handling of FILE_LINK columns - public static class FileLinkDataIterator extends WrapperDataIterator + public static class FileLinkDataIterator extends WrapperDataIterator// { Supplier[] suppliers; String[] savedFileName; @@ -2099,6 +2100,8 @@ public static class FileLinkDataIterator extends WrapperDataIterator value = null; } } + else if (value instanceof String filePath) + return ExpDataFileConverter.convert(filePath); return value; }; } diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index c2794b0db88..f4da4be7005 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -1041,7 +1041,7 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) { try { - value = ConvertUtils.convert(String.valueOf(value), col.getJavaClass()); + value = ConvertUtils.convert(String.valueOf(value), col.getJavaClass());// } catch (ConversionException e) { diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 92f997c63d5..d80baae2140 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -3339,8 +3339,7 @@ private void parseData(User user, } // let DataIterator do conversions - if (!File.class.equals(col.clazz)) - col.clazz = String.class; + col.clazz = String.class; if (columnMap.containsKey(name)) name = columnMap.get(name); From a31ef1ffba76fe4a56853f531eee5802587baebb Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 1 Aug 2025 14:20:40 -0700 Subject: [PATCH 19/25] code review changes --- .../assay/AbstractAssayTsvDataHandler.java | 8 +-- .../api/assay/AssayUploadFileResolver.java | 1 + .../api/assay/DefaultAssayRunCreator.java | 9 +-- .../api/data/AssayResultsFileConverter.java | 61 ------------------- .../labkey/api/data/ExpDataFileConverter.java | 46 +++++--------- api/src/org/labkey/api/reader/DataLoader.java | 28 +++------ 6 files changed, 32 insertions(+), 121 deletions(-) delete mode 100644 api/src/org/labkey/api/data/AssayResultsFileConverter.java diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 5b14c36d125..2467cf8a9d3 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -326,8 +326,6 @@ else if (mvIndicatorColumns.contains(column.name)) else column.errorValues = ERROR_VALUE; -// if (run != null && column.clazz == File.class) -// column.converter = new AssayResultsFileConverter(run); // TODO: fix } return loader; @@ -797,8 +795,6 @@ else if (sampleLookup.isLookup()) DomainProperty datePD = datePropFinder; DomainProperty targetStudyPD = targetStudyPropFinder; - ExpDataFileConverter expDataFileConverter = new ExpDataFileConverter(); - return DataIteratorUtil.mapTransformer(rawData, inputCols -> { List result = new ArrayList<>(inputCols); @@ -935,8 +931,8 @@ else if (o instanceof MvFieldWrapper mvWrapper) try { File resolvedFile; - if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri()) && o instanceof String) - resolvedFile = (File) expDataFileConverter.convert(File.class, o); + if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri())) + resolvedFile = ExpDataFileConverter.convert(o); else resolvedFile = AssayUploadFileResolver.resolve(o, container, pd); if (resolvedFile != null) diff --git a/api/src/org/labkey/api/assay/AssayUploadFileResolver.java b/api/src/org/labkey/api/assay/AssayUploadFileResolver.java index 55daa9fb9e1..3ae1505dee8 100644 --- a/api/src/org/labkey/api/assay/AssayUploadFileResolver.java +++ b/api/src/org/labkey/api/assay/AssayUploadFileResolver.java @@ -47,6 +47,7 @@ public static File resolve(Object o, Container container, DomainProperty propert return null; String uri = property.getType().getTypeURI(); + // TODO, this is no longer used for FILE_LINK. Also, assay doesn't support ATTACHMENT, so this class can be removed? if (uri.equals(PropertyType.FILE_LINK.getTypeUri()) || uri.equals(PropertyType.ATTACHMENT.getTypeUri())) { File fileToResolve = null; diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index c2028a59bcc..888b7742113 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -737,8 +737,6 @@ protected void addInputDatas( // Resolve submitted values into ExpData objects protected void addDatas(Container c, @NotNull Map resolved, @NotNull Map unresolved, @Nullable Logger log) throws ValidationException { - ExpDataFileConverter expDataFileConverter = new ExpDataFileConverter(); - for (Map.Entry entry : unresolved.entrySet()) { Object o = entry.getKey(); @@ -750,7 +748,7 @@ protected void addDatas(Container c, @NotNull Map resolved, @No } else { - File file = (File) expDataFileConverter.convert(File.class, o); + File file = ExpDataFileConverter.convert(o); if (file != null) { ExpData data = ExperimentService.get().getExpDataByURL(file, c); @@ -1110,7 +1108,6 @@ protected void savePropertyObject(ExpObject object, Container container, Map entry : properties.entrySet()) { DomainProperty pd = entry.getKey(); @@ -1118,8 +1115,8 @@ protected void savePropertyObject(ExpObject object, Container container, Map Date: Fri, 1 Aug 2025 19:23:42 -0700 Subject: [PATCH 20/25] Fix assay run api, more cleaning up --- .../assay/AbstractAssayTsvDataHandler.java | 13 +-- .../api/assay/AssayRunUploadContext.java | 5 +- .../api/assay/AssayUploadFileResolver.java | 98 ------------------- .../api/assay/DefaultAssayRunCreator.java | 9 +- .../api/query/DefaultQueryUpdateService.java | 2 +- api/src/org/labkey/api/reader/DataLoader.java | 15 +-- .../labkey/experiment/ExpDataIterators.java | 5 +- .../experiment/api/ExpRunTableImpl.java | 9 +- 8 files changed, 23 insertions(+), 133 deletions(-) delete mode 100644 api/src/org/labkey/api/assay/AssayUploadFileResolver.java diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 2467cf8a9d3..3b1da1badf6 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -30,6 +30,7 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; @@ -915,9 +916,9 @@ else if (o instanceof MvFieldWrapper mvWrapper) valueMissing = false; } - // If the column is a file link or attachment, resolve the value to a File object + // If the column is a file link, resolve the value to a File object String uri = pd.getType().getTypeURI(); - if (uri.equals(PropertyType.FILE_LINK.getTypeUri()) || uri.equals(PropertyType.ATTACHMENT.getTypeUri())) + if (uri.equals(PropertyType.FILE_LINK.getTypeUri())) { if ("".equals(o)) { @@ -930,18 +931,14 @@ else if (o instanceof MvFieldWrapper mvWrapper) // File column values are stored as the absolute resolved path try { - File resolvedFile; - if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri())) - resolvedFile = ExpDataFileConverter.convert(o); - else - resolvedFile = AssayUploadFileResolver.resolve(o, container, pd); + File resolvedFile = ExpDataFileConverter.convert(o); if (resolvedFile != null) { o = resolvedFile; map.put(pd.getName(), o); } } - catch (ValidationException e) + catch (ConvertHelper.FileConversionException e) { throw new RuntimeValidationException(e); } diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContext.java b/api/src/org/labkey/api/assay/AssayRunUploadContext.java index c64fd23022d..6d44da7a716 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContext.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContext.java @@ -22,6 +22,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.collections.CollectionUtils; import org.labkey.api.data.Container; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.api.ExpProtocol; @@ -104,7 +105,7 @@ default DataIteratorBuilder getRawData() /** * Map of inputs to roles that will be attached to the assay run. - * The map key will be converted into an ExpData object using {@link org.labkey.api.data.ExpDataFileConverter} + * The map key will be converted into an ExpData object using {@link ExpDataFileConverter} * The map value is the role of the file. * Each input file will be attached as an input ExpData to the imported assay run. * NOTE: These files will not be parsed or imported by the assay's DataHandler -- use {@link #getUploadedData()} instead. @@ -336,7 +337,7 @@ public final FACTORY setRunProperties(Map rawProperties) /** * Map of inputs to roles that will be attached to the assay run. - * The map key will be converted into an ExpData object using {@link org.labkey.api.data.ExpDataFileConverter} + * The map key will be converted into an ExpData object using {@link ExpDataFileConverter} * The map value is the role of the file. * Each input file will be attached as an input ExpData to the imported assay run. * NOTE: These files will not be parsed or imported by the assay's DataHandler -- use {@link #getUploadedData()} instead. diff --git a/api/src/org/labkey/api/assay/AssayUploadFileResolver.java b/api/src/org/labkey/api/assay/AssayUploadFileResolver.java deleted file mode 100644 index 3ae1505dee8..00000000000 --- a/api/src/org/labkey/api/assay/AssayUploadFileResolver.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright (c) 2017-2019 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; - -import org.apache.commons.lang3.StringUtils; -import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.Container; -import org.labkey.api.exp.PropertyType; -import org.labkey.api.exp.property.DomainProperty; -import org.labkey.api.files.FileContentService; -import org.labkey.api.pipeline.PipeRoot; -import org.labkey.api.pipeline.PipelineService; -import org.labkey.api.query.ValidationException; -import org.labkey.api.util.URIUtil; - -import java.io.File; -import java.nio.file.Path; - -/** - * Created by klum on 6/2/2017. - */ -public class AssayUploadFileResolver -{ - /** - * Resolves files for attachment or file property columns - * - * @param o the value to resolve - * @param property the DomainProperty of the destination column - */ - @Nullable - public static File resolve(Object o, Container container, DomainProperty property) throws ValidationException - { - if (o == null) - return null; - - String uri = property.getType().getTypeURI(); - // TODO, this is no longer used for FILE_LINK. Also, assay doesn't support ATTACHMENT, so this class can be removed? - if (uri.equals(PropertyType.FILE_LINK.getTypeUri()) || uri.equals(PropertyType.ATTACHMENT.getTypeUri())) - { - File fileToResolve = null; - - if (o instanceof File) - { - fileToResolve = (File) o; - } - else if (o instanceof String) - { - // Issue 36502: Do not resolve blank string as a file name - String s = StringUtils.trimToNull((String)o); - if (s == null) - return null; - - fileToResolve = new File(s); - } - - if (fileToResolve != null) - { - // For security reasons, make sure the user hasn't tried to reference a file that's not under - // the pipeline root or @assayfiles root. Otherwise, they could get access to any file on the server - - Path assayFilesRoot = FileContentService.get().getFileRootPath(container, FileContentService.ContentType.assayfiles); - if (assayFilesRoot != null && URIUtil.isDescendant(assayFilesRoot.toUri(), fileToResolve.toURI())) - { - return null; // return null since we do not need to convert the file path - } - - PipeRoot root = PipelineService.get().findPipelineRoot(container); - if (root == null) - { - throw new ValidationException("Pipeline root not available in container " + container); - } - - if (!root.isUnderRoot(fileToResolve)) - { - File resolved = root.resolvePath(fileToResolve.toString()); - if (resolved == null) - throw new ValidationException("Cannot reference file " + fileToResolve + " from container " + container); - - return resolved; - } - } - } - return null; - } -} diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 888b7742113..fa1382130e3 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -1114,14 +1114,11 @@ protected void savePropertyObject(ExpObject object, Container container, Map renamedColumns) throws for (int f = 0; f < nCols; f++) { List classesToTest = new ArrayList<>(Arrays.asList(CONVERT_CLASSES)); - Class knownColumnClass; int classIndex = -1; //NOTE: this means we have a header row @@ -344,7 +343,7 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws if (knownColumn != null) { - knownColumnClass = knownColumn.getJavaClass(); + Class knownColumnClass = knownColumn.getJavaClass(); classesToTest.add(0, knownColumnClass); } } @@ -450,8 +449,7 @@ private void inferColumnInfo(@NotNull Map renamedColumns) throws Set columnNames = new HashSet<>(); for (ColumnDescriptor colDesc : colDescs) { - String name = colDesc.name; - if (!columnNames.add(name) && isThrowOnErrors()) + if (!columnNames.add(colDesc.name) && isThrowOnErrors()) { // TODO: This should be refactored to not throw this here, but rather, have the callers check themselves. It // is not in the interest of inferring columns that we validate duplicate columns. @@ -958,15 +956,6 @@ public DataIterator getDataIterator(final DataIteratorContext context) /** Actually create an instance of DataIterator to use, which might be subclass-specific */ protected DataIterator createDataIterator(DataIteratorContext context) throws IOException { -// ColumnDescriptor[] columnDescriptors = getActiveColumns(); -// if (context.isCrossFolderImport() || context.isCrossTypeImport()) -// { -// for (ColumnDescriptor columnDescriptor : columnDescriptors) -// { -// if (columnDescriptor.clazz.equals(File.class)) -// columnDescriptor.clazz = String.class; // defer file path validation for cross type/folder import -// } -// } return new _DataIterator(context, getActiveColumns(), isScrollable()); } diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index de906100da3..ae4b4e5bcc5 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2053,7 +2053,7 @@ public boolean next() throws BatchValidationException // This should be used AFTER StandardDataIteratorBuilder, say at the beginning of PersistDataIteratorBuilder (below) // The incoming dataiterator should bound to target table and have complete ColumnInfo metadata // see SimpleQueryUpdateService.convertTypes() for similar handling of FILE_LINK columns - public static class FileLinkDataIterator extends WrapperDataIterator// + public static class FileLinkDataIterator extends WrapperDataIterator { Supplier[] suppliers; String[] savedFileName; @@ -2100,7 +2100,8 @@ public static class FileLinkDataIterator extends WrapperDataIterator// value = null; } } - else if (value instanceof String filePath) + + if (value instanceof String filePath) return ExpDataFileConverter.convert(filePath); return value; }; diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index f4da4be7005..c2574d49933 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -32,6 +32,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DataColumn; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.ForeignKey; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MultiValuedForeignKey; @@ -1031,9 +1032,11 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) { PropertyDescriptor propertyDescriptor = col.getPropertyDescriptor(); Object oldValue = run.getProperty(propertyDescriptor); - if (propertyDescriptor.getPropertyType() == PropertyType.FILE_LINK && (value instanceof MultipartFile || value instanceof SpringAttachmentFile)) + if (propertyDescriptor.getPropertyType() == PropertyType.FILE_LINK) { - value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME); + if (value instanceof MultipartFile || value instanceof SpringAttachmentFile) + value = saveFile(user, container, col.getName(), value, AssayFileWriter.DIR_NAME); + value = ExpDataFileConverter.convert(value); } ForeignKey fk = col.getFk(); @@ -1041,7 +1044,7 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) { try { - value = ConvertUtils.convert(String.valueOf(value), col.getJavaClass());// + value = ConvertUtils.convert(String.valueOf(value), col.getJavaClass()); } catch (ConversionException e) { From f3053d52fe0b3315d9f6b65621e84c9c3a88ff95 Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 3 Aug 2025 17:47:52 -0700 Subject: [PATCH 21/25] fix more tests --- .../org/labkey/api/assay/AbstractAssayTsvDataHandler.java | 3 ++- api/src/org/labkey/api/assay/DefaultAssayRunCreator.java | 6 +++++- .../controllers/property/PropertyController.java | 3 ++- .../test/tests/study/StudyDatasetFileFieldTest.java | 8 +++++--- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 3b1da1badf6..48a33984fa0 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -22,6 +22,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; +import org.labkey.api.action.ApiUsageException; import org.labkey.api.assay.plate.AssayPlateMetadataService; import org.labkey.api.assay.sample.AssaySampleLookupContext; import org.labkey.api.collections.CaseInsensitiveHashMap; @@ -940,7 +941,7 @@ else if (o instanceof MvFieldWrapper mvWrapper) } catch (ConvertHelper.FileConversionException e) { - throw new RuntimeValidationException(e); + throw new ApiUsageException(e); } } } diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index fa1382130e3..1427a0ae910 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -1212,7 +1212,11 @@ else if (!missing) { try { - Object o = ConvertUtils.convert(value, type); + Object o; + if (type == File.class) + o = ExpDataFileConverter.convert(value); + else + o = ConvertUtils.convert(value, type); ValidatorContext validatorContext = new ValidatorContext(context.getContainer(), context.getUser()); for (ColumnValidator validator : validators) { diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index d318dd83d83..6b5b3638c84 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -46,6 +46,7 @@ import org.labkey.api.data.ContainerService; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.NameExpressionValidationResult; import org.labkey.api.data.SimpleFilter; import org.labkey.api.defaults.DefaultValueService; @@ -1187,7 +1188,7 @@ protected ObjectMapper createResponseObjectMapper() public Object execute(InferDomainForm form, BindException errors) throws Exception { Map fileMap = getFileMap(); - File file = form.getFile() != null ? (File) ConvertUtils.convert(form.getFile().toString(), File.class) : null; + File file = form.getFile() != null ? ExpDataFileConverter.convert(form.getFile()) : null; FileType guessFormat = form.isGuessFormatAsTSV() ? TabLoader.TSV_FILE_TYPE : null; DataLoader loader = null; diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index b4adc41bdc9..8c354e6a0f8 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -27,6 +27,7 @@ import org.labkey.test.util.TestDataGenerator; import org.labkey.test.util.FileBrowserHelper; import org.labkey.test.util.PasswordUtil; +import org.labkey.test.util.data.TestDataUtils; import org.openqa.selenium.NoSuchElementException; import java.io.File; @@ -242,9 +243,10 @@ public void testFileField() throws IOException, CommandException private void importFilePathError(String participantId, String sequenceNum, String filePath) { - String header = "ParticipantId\tSequenceNum\tfileField\n"; - String data = participantId + "\t" + sequenceNum + "\t" + filePath + "\n"; - setFormElement(Locator.name("text"), header + data); + String pasteData = TestDataUtils.tsvStringFromRowMaps(List.of( + Map.of("ParticipantId", participantId, "SequenceNum", sequenceNum, FILE_FIELD_1, filePath)), + List.of("ParticipantId", "SequenceNum", FILE_FIELD_1), true); + setFormElement(Locator.name("text"), pasteData); new ImportDataPage(getDriver()).submitExpectingError(); try { From e2b8a2ad4b778c2ebaa4bf0039e3880ed49aeea8 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 4 Aug 2025 14:53:35 -0700 Subject: [PATCH 22/25] fix tests for Windows with quoted path --- .../org/labkey/test/tests/study/StudyDatasetFileFieldTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index 8c354e6a0f8..7505dba3353 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -243,7 +243,7 @@ public void testFileField() throws IOException, CommandException private void importFilePathError(String participantId, String sequenceNum, String filePath) { - String pasteData = TestDataUtils.tsvStringFromRowMaps(List.of( + String pasteData = TestDataUtils.tsvStringFromRowMapsWithQuote(List.of( Map.of("ParticipantId", participantId, "SequenceNum", sequenceNum, FILE_FIELD_1, filePath)), List.of("ParticipantId", "SequenceNum", FILE_FIELD_1), true); setFormElement(Locator.name("text"), pasteData); From d5b5e250688491f35f45eb8bd2d2d9f079c82e62 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 4 Aug 2025 16:19:35 -0700 Subject: [PATCH 23/25] fix tests for Windows with quoted path --- .../org/labkey/test/tests/study/StudyDatasetFileFieldTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index 7505dba3353..3bd79fbf2e7 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -243,7 +243,7 @@ public void testFileField() throws IOException, CommandException private void importFilePathError(String participantId, String sequenceNum, String filePath) { - String pasteData = TestDataUtils.tsvStringFromRowMapsWithQuote(List.of( + String pasteData = TestDataUtils.tsvStringFromRowMapsEscapeBackslash(List.of( Map.of("ParticipantId", participantId, "SequenceNum", sequenceNum, FILE_FIELD_1, filePath)), List.of("ParticipantId", "SequenceNum", FILE_FIELD_1), true); setFormElement(Locator.name("text"), pasteData); From c2aee9e1e5ddeeaf94f36f4ebc0e6da832aa18ef Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 4 Aug 2025 18:37:43 -0700 Subject: [PATCH 24/25] more code review changes, add test for LKS file availability check --- api/src/org/labkey/api/query/DefaultQueryUpdateService.java | 2 +- study/src/org/labkey/study/model/StudyManager.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index d35c0d7d487..4d4c06109a5 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -844,7 +844,7 @@ protected Object convertColumnValue(ColumnInfo col, Object value, User user, Con default: if (PropertyType.FILE_LINK == col.getPropertyType()) { - if ((value instanceof MultipartFile || value instanceof AttachmentFile))// + if ((value instanceof MultipartFile || value instanceof AttachmentFile)) { FileLike fl = (FileLike)_fileColumnValueMapping.saveFileColumnValue(user, c, fileLinkDirPath, col.getName(), value); value = fl.toNioPathForRead().toString(); diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index d80baae2140..c3b6f37a4e6 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -198,7 +198,6 @@ import org.springframework.dao.DataIntegrityViolationException; import org.springframework.validation.BindException; -import java.io.File; import java.io.IOException; import java.math.BigDecimal; import java.sql.ResultSet; From 793f9eab1eaad908e43e5afa9019069f010d531d Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 4 Aug 2025 19:17:49 -0700 Subject: [PATCH 25/25] fix cross folder saveRows api --- query/src/org/labkey/query/controllers/QueryController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index d8249bebac9..e7881e47afc 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -4662,6 +4662,7 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole } } + QueryService.get().setEnvironment(QueryService.Environment.CONTAINER, container); List> responseRows = commandType.saveRows(qus, rowsToProcess, getUser(), container, configParameters, extraContext); if (auditEvent != null)