Skip to content

File path related issue bundle #6847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Aug 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
a39b7b6
File path related bugs
XingY Jul 15, 2025
a7dc3c7
Merge branch 'refs/heads/develop' into fb_filePathIssues
labkey-jeckels Jul 16, 2025
4f7a260
fix relative path import
XingY Jul 16, 2025
a272233
fix invalid path that starts with file scheme, or if under root but d…
XingY Jul 16, 2025
ef9b4a3
Merge branch 'develop' into fb_filePathIssues
XingY Jul 21, 2025
333ab4f
fix assay import exception handling, disable sort filter for attachme…
XingY Jul 21, 2025
d27a817
allow sort filter for attachment columns
XingY Jul 21, 2025
8d8c1dd
Bug fixes for cross folder import
XingY Jul 22, 2025
30c6657
Merge branch 'develop' into fb_filePathIssues
XingY Jul 23, 2025
618ab5a
Selenium tests
XingY Jul 24, 2025
b7f2c1f
Fix assayfile import
XingY Jul 25, 2025
4addc97
Merge branch 'develop' into fb_filePathIssues
XingY Jul 25, 2025
7f0bfbc
add test coverage for study dataset
XingY Jul 27, 2025
60a2b2a
fix various assay api, add tests for assay api
XingY Jul 28, 2025
371b587
clean
XingY Jul 28, 2025
734f58d
clean
XingY Jul 28, 2025
6bb3c3d
Merge branch 'develop' into fb_filePathIssues
XingY Jul 28, 2025
e3cbed5
Merge branch 'develop' into fb_filePathIssues
XingY Jul 29, 2025
bdb93d6
rename
XingY Jul 29, 2025
a2b48a8
refactor for attachment
XingY Jul 30, 2025
461682c
refactor for attachment
XingY Jul 30, 2025
8a723bf
Merge branch 'develop' into fb_filePathIssues
XingY Jul 30, 2025
36866fd
code review changes
XingY Jul 31, 2025
716c06f
Fix relative path
XingY Aug 1, 2025
8d38384
Merge branch 'develop' into fb_filePathIssues
XingY Aug 1, 2025
72977bd
no converter
XingY Aug 1, 2025
a31ef1f
code review changes
XingY Aug 1, 2025
233bb66
Fix assay run api, more cleaning up
XingY Aug 2, 2025
a2997a9
Merge branch 'develop' into fb_filePathIssues
XingY Aug 3, 2025
f3053d5
fix more tests
XingY Aug 4, 2025
92616e0
Merge branch 'develop' into fb_filePathIssues
XingY Aug 4, 2025
e2b8a2a
fix tests for Windows with quoted path
XingY Aug 4, 2025
d5b5e25
fix tests for Windows with quoted path
XingY Aug 4, 2025
c2aee9e
more code review changes, add test for LKS file availability check
XingY Aug 5, 2025
eec1c62
Merge branch 'develop' into fb_filepathIssues
XingY Aug 5, 2025
793f9ea
fix cross folder saveRows api
XingY Aug 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,20 @@
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;
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;
import org.labkey.api.data.ConvertHelper;
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;
Expand Down Expand Up @@ -326,8 +328,6 @@ else if (mvIndicatorColumns.contains(column.name))
else
column.errorValues = ERROR_VALUE;

if (run != null && column.clazz == File.class)
column.converter = new AssayResultsFileConverter(run);
}
return loader;

Expand Down Expand Up @@ -917,9 +917,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))
{
Expand All @@ -932,16 +932,16 @@ 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 = ExpDataFileConverter.convert(o);
if (resolvedFile != null)
{
o = resolvedFile;
map.put(pd.getName(), o);
}
}
catch (ValidationException e)
catch (ConvertHelper.FileConversionException e)
{
throw new RuntimeValidationException(e);
throw new ApiUsageException(e);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions api/src/org/labkey/api/assay/AssayRunUploadContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -336,7 +337,7 @@ public final FACTORY setRunProperties(Map<String, Object> 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.
Expand Down
97 changes: 0 additions & 97 deletions api/src/org/labkey/api/assay/AssayUploadFileResolver.java

This file was deleted.

31 changes: 23 additions & 8 deletions api/src/org/labkey/api/assay/DefaultAssayRunCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -86,10 +87,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;
Expand Down Expand Up @@ -408,6 +411,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();
Expand Down Expand Up @@ -471,7 +481,7 @@ public ExpExperiment saveExperimentRun(

return batch;
}
catch (ExperimentException | IOException 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);
Expand All @@ -481,6 +491,8 @@ public ExpExperiment saveExperimentRun(

if (e instanceof ExperimentException)
throw (ExperimentException)e;
else if (e instanceof ConvertHelper.FileConversionException)
throw new ApiUsageException(e.getMessage(), e);
else
throw new ExperimentException(e);
}
Expand Down Expand Up @@ -725,8 +737,6 @@ protected void addInputDatas(
// Resolve submitted values into ExpData objects
protected void addDatas(Container c, @NotNull Map<ExpData, String> resolved, @NotNull Map<?, String> unresolved, @Nullable Logger log) throws ValidationException
{
ExpDataFileConverter expDataFileConverter = new ExpDataFileConverter();

for (Map.Entry<?, String> entry : unresolved.entrySet())
{
Object o = entry.getKey();
Expand All @@ -738,7 +748,7 @@ protected void addDatas(Container c, @NotNull Map<ExpData, String> 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);
Expand Down Expand Up @@ -1104,10 +1114,11 @@ protected void savePropertyObject(ExpObject object, Container container, Map<Dom
String value = entry.getValue();

// resolve any file links for batch or run properties
File resolvedFile = AssayUploadFileResolver.resolve(value, container, entry.getKey());
if (resolvedFile != null)
if (pd.getType().getTypeURI().equals(PropertyType.FILE_LINK.getTypeUri()))
{
value = resolvedFile.getAbsolutePath();
File resolvedFile = ExpDataFileConverter.convert(value);
if (resolvedFile != null)
value = resolvedFile.getAbsolutePath();
}

// Treat the empty string as a null in the database, which is our normal behavior when receiving data
Expand Down Expand Up @@ -1201,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)
{
Expand Down
7 changes: 6 additions & 1 deletion api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,12 @@ protected Map<DomainProperty, String> getPropertyMapFromRequest(List<? extends D
}

if (additionalFiles.containsKey(pd))
properties.put(pd, additionalFiles.get(pd).toNioPathForRead().toString());
{
if (additionalFiles.get(pd).equals(BLANK_FILE)) // file has been removed
properties.put(pd, null);
else
properties.put(pd, additionalFiles.get(pd).toNioPathForRead().toString());
}
else
properties.put(pd, value);
}
Expand Down
30 changes: 14 additions & 16 deletions api/src/org/labkey/api/data/AbstractFileDisplayColumn.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice check here. It would be nice if getFileName() had an explicit structured return (e.g. record(String fileDisplay,Boolean available), but this is workable. I wonder about the case in FileLinkDisplayColumn where we fall back to result = f.getName() when we don't find the file in an expected "file root".

What is the expected behavior in this case? Should we treat as "unavailable"? Should we render a URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of result = f.getName() when we fail to find file in root, valid is false, and the "unavailable" suffix will be added.

I added selenium test for grid and update view for LKS unavailable file both due to invalid path (exists but out side of file root) and absent file (file deleted after upload) in this commit

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
Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/data/AbstractTableInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -2094,7 +2095,7 @@ public List<Pair<String, String>> 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;
Expand Down
Loading