Skip to content

Issue 54112: Retain previous file values when reimporting a run #7189

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.

Sign up for GitHub

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
labkey-nicka merged 7 commits into from
Nov 11, 2025
Merged

Issue 54112: Retain previous file values when reimporting a run #7189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fcb2967
Initial updates
labkey-nicka Nov 4, 2025
0a3a5d2
Issue 54112: Retain previous file values when reimporting a run
labkey-nicka Nov 6, 2025
3531539
Issue 54112: regression tests
labkey-nicka Nov 7, 2025
0680ec5
Revise TODO
labkey-nicka Nov 8, 2025
5155025
Banish "upload set" terminology
labkey-jeckels Nov 11, 2025
efeff5d
Update comment
labkey-nicka Nov 11, 2025
25d508d
Unused import
labkey-nicka Nov 11, 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
195 changes: 101 additions & 94 deletions api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@
import org.springframework.web.multipart.MultipartFile;
import org.springframework.web.multipart.MultipartHttpServletRequest;

import jakarta.servlet.http.HttpServletRequest;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -85,14 +84,9 @@

import static java.util.Collections.emptyMap;

/**
* User: brittp
* Date: Jul 11, 2007
* Time: 2:52:54 PM
*/
public class AssayRunUploadForm<ProviderType extends AssayProvider> extends ProtocolIdForm implements AssayRunUploadContext<ProviderType>
{
protected Map<DomainProperty, String> _uploadSetProperties = null;
protected Map<DomainProperty, String> _batchProperties = null;
protected Map<DomainProperty, String> _runProperties = null;
private String _comments;
private String _name;
Expand Down Expand Up @@ -127,54 +121,63 @@ public List<? extends DomainProperty> getRunDataProperties()
public Map<DomainProperty, String> getRunProperties() throws ExperimentException
{
if (_runProperties == null)
{
Domain domain = getProvider().getRunDomain(getProtocol());
List<? extends DomainProperty> properties = domain.getProperties();
_runProperties = getPropertyMapFromRequest(properties);
}
_runProperties = getPropertyMapFromRequest(getProvider().getRunDomain(getProtocol()));

return Collections.unmodifiableMap(_runProperties);
}

/** @return property descriptor to value */
@Override
public Map<DomainProperty, String> getBatchProperties() throws ExperimentException
{
if (_uploadSetProperties == null)
{
Domain domain = getProvider().getBatchDomain(getProtocol());
_uploadSetProperties = getPropertyMapFromRequest(domain.getProperties());
}
return Collections.unmodifiableMap(_uploadSetProperties);
if (_batchProperties == null)
_batchProperties = getPropertyMapFromRequest(getProvider().getBatchDomain(getProtocol()));

return Collections.unmodifiableMap(_batchProperties);
}

protected Map<DomainProperty, String> getPropertyMapFromRequest(List<? extends DomainProperty> columns) throws ExperimentException
protected Map<DomainProperty, String> getPropertyMapFromRequest(Domain domain) throws ExperimentException
{
List<? extends DomainProperty> columns = domain.getProperties();
Map<DomainProperty, String> properties = new LinkedHashMap<>();
Map<DomainProperty, FileLike> additionalFiles = getAdditionalPostedFiles(columns);
Map<DomainProperty, Object> defaults = getDefaultValues(domain, false);

for (DomainProperty pd : columns)
{
String propName = UploadWizardAction.getInputName(pd);
String value = getRequest().getParameter(propName);
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.BOOLEAN &&
(value == null || value.isEmpty()))
String value = StringUtils.trimToNull(getRequest().getParameter(propName));
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.BOOLEAN && (value == null || value.isEmpty()))
value = Boolean.FALSE.toString();
value = StringUtils.trimToNull(value);

if (pd.getName().equals(AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME) && value != null)
{
if (AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME.equalsIgnoreCase(pd.getName()) && value != null)
value = ParticipantVisitResolverType.Serializer.encode(value, getRequest());
}

if (additionalFiles.containsKey(pd))
{
if (additionalFiles.get(pd).equals(BLANK_FILE)) // file has been removed
if (BLANK_FILE.equals(additionalFiles.get(pd))) // file has been removed
properties.put(pd, null);
else
properties.put(pd, additionalFiles.get(pd).toNioPathForRead().toString());
}
else
properties.put(pd, value);

// Issue 54112: Retain previous file values when reimporting a run
if (PropertyType.FILE_LINK == pd.getPropertyDescriptor().getPropertyType())
{
boolean postedNewFile = additionalFiles.containsKey(pd);
String current = properties.get(pd);
boolean hasValue = current != null && !current.isEmpty();

if (!postedNewFile && !hasValue)
{
Object prior = defaults.get(pd);
if (prior != null)
properties.put(pd, prior.toString());
}
}
}

return properties;
}

Expand Down Expand Up @@ -316,79 +319,75 @@ public Map<DomainProperty, FileLike> getAdditionalFiles()

public static File getAssayDirectory(Container c, File root)
{
if(null != root)
if (null != root)
return new File(root.getAbsolutePath(), AssayFileWriter.DIR_NAME);
else
return new File(PipelineService.get().findPipelineRoot(c).getRootPath().getAbsolutePath(), AssayFileWriter.DIR_NAME);

}

public Map<DomainProperty, FileLike> getAdditionalPostedFiles(List<? extends DomainProperty> pds) throws ExperimentException
{
if (_additionalFiles == null)
{
Map<String, DomainProperty> fileParameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
List<String> filePdNames = new ArrayList<>();
if (_additionalFiles != null)
return _additionalFiles;

Map<String, DomainProperty> fileParameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
List<String> filePdNames = new ArrayList<>();

for (DomainProperty pd : pds)
for (DomainProperty pd : pds)
{
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.FILE_LINK)
{
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.FILE_LINK)
{
fileParameters.put(UploadWizardAction.getInputName(pd), pd);
filePdNames.add(pd.getName());
}
fileParameters.put(UploadWizardAction.getInputName(pd), pd);
filePdNames.add(pd.getName());
}
}

if (!fileParameters.isEmpty())
{
AssayFileWriter<AssayRunUploadForm<ProviderType>> writer = new AssayFileWriter<>();
try
{
// Initialize member variable so we know that we've already tried to save the posted files in case of error
_additionalFiles = new HashMap<>();
Map<String, FileLike> postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false);
for (Map.Entry<String, FileLike> entry : postedFiles.entrySet())
_additionalFiles.put(fileParameters.get(entry.getKey()), entry.getValue());
if (fileParameters.isEmpty())
return _additionalFiles = emptyMap();

File previousFile;
HttpServletRequest request = getViewContext().getRequest();
AssayFileWriter<AssayRunUploadForm<ProviderType>> writer = new AssayFileWriter<>();
try
{
// Initialize member variable so we know that we've already tried to save the posted files in case of error
_additionalFiles = new HashMap<>();
Map<String, FileLike> postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false);
for (Map.Entry<String, FileLike> entry : postedFiles.entrySet())
_additionalFiles.put(fileParameters.get(entry.getKey()), entry.getValue());

// Hidden values in form containing previously uploaded files if previous upload resulted in error
for (String fileParam : filePdNames)
{
if (request instanceof MultipartHttpServletRequest mhsr && null != request.getParameter(fileParam))
{
String previousFileName = request.getParameter(fileParam);
if (null != previousFileName)
{
previousFile = FileUtil.appendName(getAssayDirectory(getContainer(), null), previousFileName);

MultipartFile multiFile = mhsr.getFileMap().get(UploadWizardAction.getInputName(fileParameters.get(fileParam)));

// If file is removed from form after error, override hidden file name with empty file
if (null != multiFile && multiFile.getOriginalFilename().isEmpty())
_additionalFiles.put(fileParameters.get(fileParam), BLANK_FILE);

// Only add hidden file parameter if it is a valid file in the pipeline root directory and
// a new file hasn't been uploaded for that parameter
if (previousFile.isFile()
&& FileUtils.directoryContains(getAssayDirectory(getContainer(), null), previousFile)
&& !_additionalFiles.containsKey(fileParameters.get(fileParam)))
{
_additionalFiles.put(fileParameters.get(fileParam), FileSystemLike.wrapFile(previousFile));
}
}
}
}
}
catch (IOException e)
if (!(getViewContext().getRequest() instanceof MultipartHttpServletRequest request))
return _additionalFiles;

File assayDirectory = getAssayDirectory(getContainer(), null);

// Hidden values in form containing previously uploaded files if the previous upload resulted in error
for (String fileParam : filePdNames)
{
DomainProperty domainProperty = fileParameters.get(fileParam);
MultipartFile multiFile = request.getFileMap().get(fileParam);

// If the file is removed from form after error, override hidden file name with an empty file
if (null != multiFile && multiFile.getOriginalFilename().isEmpty())
{
throw new RuntimeException(e);
_additionalFiles.put(domainProperty, BLANK_FILE);
continue;
}

String previousFileName = request.getParameter(fileParam);
if (previousFileName == null)
continue;

// Only add a hidden file parameter if it is a valid file in the pipeline root directory and
// a new file hasn't been uploaded for that parameter
File previousFile = FileUtil.appendName(assayDirectory, previousFileName);
if (previousFile.isFile() && FileUtils.directoryContains(assayDirectory, previousFile) && !_additionalFiles.containsKey(domainProperty))
_additionalFiles.put(domainProperty, FileSystemLike.wrapFile(previousFile));
}
else
_additionalFiles = emptyMap();
}
catch (IOException e)
{
throw new RuntimeException(e);
}

return _additionalFiles;
}

Expand Down Expand Up @@ -530,6 +529,7 @@ else if (AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME.equals(k
}
}
}

return value;
}

Expand Down Expand Up @@ -580,7 +580,6 @@ public void setBatchId(Long batchId)
_batchId = batchId;
}


public void clearDefaultValues(Domain domain)
{
DefaultValueService.get().clearDefaultValues(getContainer(), domain, getUser());
Expand Down Expand Up @@ -629,6 +628,11 @@ public Map<DomainProperty, Object> getDefaultValues(Domain domain, String scope)
}

public Map<DomainProperty, Object> getDefaultValues(Domain domain) throws ExperimentException
{
return getDefaultValues(domain, true);
}

private Map<DomainProperty, Object> getDefaultValues(Domain domain, boolean includeNameAndComment) throws ExperimentException
{
ExpRun reRun = getReRun();
if (reRun != null)
Expand Down Expand Up @@ -661,15 +665,18 @@ else if (runDomainURI.equals(domain.getTypeURI()))
// repopulated just like the rest of the domain properties, but they aren't actually part of the
// domain- they're hard columns on the ExperimentRun table. Since the DomainProperty objects are
// just used to calculate the input form element names, this hack works to pre-populate the values.
DomainProperty nameProperty = domain.addProperty();
nameProperty.setName("Name");
ret.put(nameProperty, reRun.getName());
nameProperty.delete();

DomainProperty commentsProperty = domain.addProperty();
commentsProperty.setName("Comments");
ret.put(commentsProperty, reRun.getComments());
commentsProperty.delete();
if (includeNameAndComment)
{
DomainProperty nameProperty = domain.addProperty();
nameProperty.setName("Name");
ret.put(nameProperty, reRun.getName());
nameProperty.delete();

DomainProperty commentsProperty = domain.addProperty();
commentsProperty.setName("Comments");
ret.put(commentsProperty, reRun.getComments());
commentsProperty.delete();
}
}
return ret;
}
Expand Down
8 changes: 5 additions & 3 deletions api/src/org/labkey/api/data/AbstractFileDisplayColumn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public void renderInputHtml(RenderContext ctx, HtmlWriter out, Object value)
if (null != filename)
{
// Existing value, so tell the user the file name, allow the file to be removed, and a new file uploaded
renderThumbnailAndRemoveLink(out, ctx, filename, input);
renderThumbnailAndRemoveLink(out, ctx, formFieldName, filename, input);
}
else
{
Expand All @@ -307,13 +307,15 @@ protected String getRemovalWarningText(String filename)
return "Previous file " + filename + " will be removed.";
}

private void renderThumbnailAndRemoveLink(HtmlWriter out, RenderContext ctx, String filename, InputBuilder<?> filePicker)
private void renderThumbnailAndRemoveLink(HtmlWriter out, RenderContext ctx, String fieldName, String filename, InputBuilder<?> filePicker)
{
String divId = GUID.makeGUID();
String linkId = "remove" + divId;

DIV(
id(divId),
id(divId)
.data("fieldName", fieldName)
.cl("lk-remove-file"),
(Renderable) ret -> {
renderIconAndFilename(ctx, out, filename, false, false);
out.write(HtmlString.NBSP);
Expand Down
6 changes: 0 additions & 6 deletions api/src/org/labkey/api/data/DataColumn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.labkey.api.writer.HtmlWriter;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -153,7 +152,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
_textAlign = _displayColumn.getTextAlign();
}


boolean analyticsProviderInitialized = false;

@Override
Expand All @@ -177,7 +175,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
return super.getAnalyticsProviders();
}


@Override
public @NotNull Set<ClientDependency> getClientDependencies()
{
Expand All @@ -186,7 +183,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
return super.getClientDependencies();
}


protected ColumnInfo getDisplayField(@NotNull ColumnInfo col, boolean withLookups)
{
if (!withLookups)
Expand All @@ -195,7 +191,6 @@ protected ColumnInfo getDisplayField(@NotNull ColumnInfo col, boolean withLookup
return null==display ? col : display;
}


@Override
public String toString()
{
Expand Down Expand Up @@ -738,7 +733,6 @@ else if (_inputType.equalsIgnoreCase("checkbox"))
return ctx.getForm() == null || col == null ? HtmlString.EMPTY_STRING : ctx.getErrors(col);
}


private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object value, List<String> strValues, boolean disabledInput, NamedObjectList entryList)
{
SelectBuilder select = new SelectBuilder()
Expand Down
Loading