Skip to content

Commit 17bd222

Browse files
labkey-nickalabkey-nicka
authored
Issue 54112: Retain previous file values when reimporting a run (#7189)
1 parent c062d00 commit 17bd222

File tree

4 files changed

+206
-140
lines changed

4 files changed

+206
-140
lines changed

‎api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java‎

Lines changed: 101 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
import org.springframework.web.multipart.MultipartFile;
7171
import org.springframework.web.multipart.MultipartHttpServletRequest;
7272

73-
import jakarta.servlet.http.HttpServletRequest;
7473
import java.io.File;
7574
import java.io.IOException;
7675
import java.util.ArrayList;
@@ -85,14 +84,9 @@
8584

8685
import static java.util.Collections.emptyMap;
8786

88-
/**
89-
* User: brittp
90-
* Date: Jul 11, 2007
91-
* Time: 2:52:54 PM
92-
*/
9387
public class AssayRunUploadForm<ProviderType extends AssayProvider> extends ProtocolIdForm implements AssayRunUploadContext<ProviderType>
9488
{
95-
protected Map<DomainProperty, String> _uploadSetProperties = null;
89+
protected Map<DomainProperty, String> _batchProperties = null;
9690
protected Map<DomainProperty, String> _runProperties = null;
9791
private String _comments;
9892
private String _name;
@@ -127,54 +121,63 @@ public List<? extends DomainProperty> getRunDataProperties()
127121
public Map<DomainProperty, String> getRunProperties() throws ExperimentException
128122
{
129123
if (_runProperties == null)
130-
{
131-
Domain domain = getProvider().getRunDomain(getProtocol());
132-
List<? extends DomainProperty> properties = domain.getProperties();
133-
_runProperties = getPropertyMapFromRequest(properties);
134-
}
124+
_runProperties = getPropertyMapFromRequest(getProvider().getRunDomain(getProtocol()));
125+
135126
return Collections.unmodifiableMap(_runProperties);
136127
}
137128

138-
/** @return property descriptor to value */
139129
@Override
140130
public Map<DomainProperty, String> getBatchProperties() throws ExperimentException
141131
{
142-
if (_uploadSetProperties == null)
143-
{
144-
Domain domain = getProvider().getBatchDomain(getProtocol());
145-
_uploadSetProperties = getPropertyMapFromRequest(domain.getProperties());
146-
}
147-
return Collections.unmodifiableMap(_uploadSetProperties);
132+
if (_batchProperties == null)
133+
_batchProperties = getPropertyMapFromRequest(getProvider().getBatchDomain(getProtocol()));
134+
135+
return Collections.unmodifiableMap(_batchProperties);
148136
}
149137

150-
protected Map<DomainProperty, String> getPropertyMapFromRequest(List<? extends DomainProperty> columns) throws ExperimentException
138+
protected Map<DomainProperty, String> getPropertyMapFromRequest(Domain domain) throws ExperimentException
151139
{
140+
List<? extends DomainProperty> columns = domain.getProperties();
152141
Map<DomainProperty, String> properties = new LinkedHashMap<>();
153142
Map<DomainProperty, FileLike> additionalFiles = getAdditionalPostedFiles(columns);
143+
Map<DomainProperty, Object> defaults = getDefaultValues(domain, false);
144+
154145
for (DomainProperty pd : columns)
155146
{
156147
String propName = UploadWizardAction.getInputName(pd);
157-
String value = getRequest().getParameter(propName);
158-
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.BOOLEAN &&
159-
(value == null || value.isEmpty()))
148+
String value = StringUtils.trimToNull(getRequest().getParameter(propName));
149+
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.BOOLEAN && (value == null || value.isEmpty()))
160150
value = Boolean.FALSE.toString();
161-
value = StringUtils.trimToNull(value);
162151

163-
if (pd.getName().equals(AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME) && value != null)
164-
{
152+
if (AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME.equalsIgnoreCase(pd.getName()) && value != null)
165153
value = ParticipantVisitResolverType.Serializer.encode(value, getRequest());
166-
}
167154

168155
if (additionalFiles.containsKey(pd))
169156
{
170-
if (additionalFiles.get(pd).equals(BLANK_FILE)) // file has been removed
157+
if (BLANK_FILE.equals(additionalFiles.get(pd))) // file has been removed
171158
properties.put(pd, null);
172159
else
173160
properties.put(pd, additionalFiles.get(pd).toNioPathForRead().toString());
174161
}
175162
else
176163
properties.put(pd, value);
164+
165+
// Issue 54112: Retain previous file values when reimporting a run
166+
if (PropertyType.FILE_LINK == pd.getPropertyDescriptor().getPropertyType())
167+
{
168+
boolean postedNewFile = additionalFiles.containsKey(pd);
169+
String current = properties.get(pd);
170+
boolean hasValue = current != null && !current.isEmpty();
171+
172+
if (!postedNewFile && !hasValue)
173+
{
174+
Object prior = defaults.get(pd);
175+
if (prior != null)
176+
properties.put(pd, prior.toString());
177+
}
178+
}
177179
}
180+
178181
return properties;
179182
}
180183

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

317320
public static File getAssayDirectory(Container c, File root)
318321
{
319-
if(null != root)
322+
if (null != root)
320323
return new File(root.getAbsolutePath(), AssayFileWriter.DIR_NAME);
321324
else
322325
return new File(PipelineService.get().findPipelineRoot(c).getRootPath().getAbsolutePath(), AssayFileWriter.DIR_NAME);
323-
324326
}
325327

326328
public Map<DomainProperty, FileLike> getAdditionalPostedFiles(List<? extends DomainProperty> pds) throws ExperimentException
327329
{
328-
if (_additionalFiles == null)
329-
{
330-
Map<String, DomainProperty> fileParameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
331-
List<String> filePdNames = new ArrayList<>();
330+
if (_additionalFiles != null)
331+
return _additionalFiles;
332+
333+
Map<String, DomainProperty> fileParameters = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
334+
List<String> filePdNames = new ArrayList<>();
332335

333-
for (DomainProperty pd : pds)
336+
for (DomainProperty pd : pds)
337+
{
338+
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.FILE_LINK)
334339
{
335-
if (pd.getPropertyDescriptor().getPropertyType() == PropertyType.FILE_LINK)
336-
{
337-
fileParameters.put(UploadWizardAction.getInputName(pd), pd);
338-
filePdNames.add(pd.getName());
339-
}
340+
fileParameters.put(UploadWizardAction.getInputName(pd), pd);
341+
filePdNames.add(pd.getName());
340342
}
343+
}
341344

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

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

356-
// Hidden values in form containing previously uploaded files if previous upload resulted in error
357-
for (String fileParam : filePdNames)
358-
{
359-
if (request instanceof MultipartHttpServletRequest mhsr && null != request.getParameter(fileParam))
360-
{
361-
String previousFileName = request.getParameter(fileParam);
362-
if (null != previousFileName)
363-
{
364-
previousFile = FileUtil.appendName(getAssayDirectory(getContainer(), null), previousFileName);
365-
366-
MultipartFile multiFile = mhsr.getFileMap().get(UploadWizardAction.getInputName(fileParameters.get(fileParam)));
367-
368-
// If file is removed from form after error, override hidden file name with empty file
369-
if (null != multiFile && multiFile.getOriginalFilename().isEmpty())
370-
_additionalFiles.put(fileParameters.get(fileParam), BLANK_FILE);
371-
372-
// Only add hidden file parameter if it is a valid file in the pipeline root directory and
373-
// a new file hasn't been uploaded for that parameter
374-
if (previousFile.isFile()
375-
&& FileUtils.directoryContains(getAssayDirectory(getContainer(), null), previousFile)
376-
&& !_additionalFiles.containsKey(fileParameters.get(fileParam)))
377-
{
378-
_additionalFiles.put(fileParameters.get(fileParam), FileSystemLike.wrapFile(previousFile));
379-
}
380-
}
381-
}
382-
}
383-
}
384-
catch (IOException e)
357+
if (!(getViewContext().getRequest() instanceof MultipartHttpServletRequest request))
358+
return _additionalFiles;
359+
360+
File assayDirectory = getAssayDirectory(getContainer(), null);
361+
362+
// Hidden values in form containing previously uploaded files if the previous upload resulted in error
363+
for (String fileParam : filePdNames)
364+
{
365+
DomainProperty domainProperty = fileParameters.get(fileParam);
366+
MultipartFile multiFile = request.getFileMap().get(fileParam);
367+
368+
// If the file is removed from form after error, override hidden file name with an empty file
369+
if (null != multiFile && multiFile.getOriginalFilename().isEmpty())
385370
{
386-
throw new RuntimeException(e);
371+
_additionalFiles.put(domainProperty, BLANK_FILE);
372+
continue;
387373
}
374+
375+
String previousFileName = request.getParameter(fileParam);
376+
if (previousFileName == null)
377+
continue;
378+
379+
// Only add a hidden file parameter if it is a valid file in the pipeline root directory and
380+
// a new file hasn't been uploaded for that parameter
381+
File previousFile = FileUtil.appendName(assayDirectory, previousFileName);
382+
if (previousFile.isFile() && FileUtils.directoryContains(assayDirectory, previousFile) && !_additionalFiles.containsKey(domainProperty))
383+
_additionalFiles.put(domainProperty, FileSystemLike.wrapFile(previousFile));
388384
}
389-
else
390-
_additionalFiles = emptyMap();
391385
}
386+
catch (IOException e)
387+
{
388+
throw new RuntimeException(e);
389+
}
390+
392391
return _additionalFiles;
393392
}
394393

@@ -530,6 +529,7 @@ else if (AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME.equals(k
530529
}
531530
}
532531
}
532+
533533
return value;
534534
}
535535

@@ -580,7 +580,6 @@ public void setBatchId(Long batchId)
580580
_batchId = batchId;
581581
}
582582

583-
584583
public void clearDefaultValues(Domain domain)
585584
{
586585
DefaultValueService.get().clearDefaultValues(getContainer(), domain, getUser());
@@ -629,6 +628,11 @@ public Map<DomainProperty, Object> getDefaultValues(Domain domain, String scope)
629628
}
630629

631630
public Map<DomainProperty, Object> getDefaultValues(Domain domain) throws ExperimentException
631+
{
632+
return getDefaultValues(domain, true);
633+
}
634+
635+
private Map<DomainProperty, Object> getDefaultValues(Domain domain, boolean includeNameAndComment) throws ExperimentException
632636
{
633637
ExpRun reRun = getReRun();
634638
if (reRun != null)
@@ -661,15 +665,18 @@ else if (runDomainURI.equals(domain.getTypeURI()))
661665
// repopulated just like the rest of the domain properties, but they aren't actually part of the
662666
// domain- they're hard columns on the ExperimentRun table. Since the DomainProperty objects are
663667
// just used to calculate the input form element names, this hack works to pre-populate the values.
664-
DomainProperty nameProperty = domain.addProperty();
665-
nameProperty.setName("Name");
666-
ret.put(nameProperty, reRun.getName());
667-
nameProperty.delete();
668-
669-
DomainProperty commentsProperty = domain.addProperty();
670-
commentsProperty.setName("Comments");
671-
ret.put(commentsProperty, reRun.getComments());
672-
commentsProperty.delete();
668+
if (includeNameAndComment)
669+
{
670+
DomainProperty nameProperty = domain.addProperty();
671+
nameProperty.setName("Name");
672+
ret.put(nameProperty, reRun.getName());
673+
nameProperty.delete();
674+
675+
DomainProperty commentsProperty = domain.addProperty();
676+
commentsProperty.setName("Comments");
677+
ret.put(commentsProperty, reRun.getComments());
678+
commentsProperty.delete();
679+
}
673680
}
674681
return ret;
675682
}

‎api/src/org/labkey/api/data/AbstractFileDisplayColumn.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public void renderInputHtml(RenderContext ctx, HtmlWriter out, Object value)
286286
if (null != filename)
287287
{
288288
// Existing value, so tell the user the file name, allow the file to be removed, and a new file uploaded
289-
renderThumbnailAndRemoveLink(out, ctx, filename, input);
289+
renderThumbnailAndRemoveLink(out, ctx, formFieldName, filename, input);
290290
}
291291
else
292292
{
@@ -307,13 +307,15 @@ protected String getRemovalWarningText(String filename)
307307
return "Previous file " + filename + " will be removed.";
308308
}
309309

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

315315
DIV(
316-
id(divId),
316+
id(divId)
317+
.data("fieldName", fieldName)
318+
.cl("lk-remove-file"),
317319
(Renderable) ret -> {
318320
renderIconAndFilename(ctx, out, filename, false, false);
319321
out.write(HtmlString.NBSP);

‎api/src/org/labkey/api/data/DataColumn.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import org.labkey.api.writer.HtmlWriter;
5959

6060
import java.util.ArrayList;
61-
import java.util.Arrays;
6261
import java.util.Collection;
6362
import java.util.Collections;
6463
import java.util.Date;
@@ -153,7 +152,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
153152
_textAlign = _displayColumn.getTextAlign();
154153
}
155154

156-
157155
boolean analyticsProviderInitialized = false;
158156

159157
@Override
@@ -177,7 +175,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
177175
return super.getAnalyticsProviders();
178176
}
179177

180-
181178
@Override
182179
public @NotNull Set<ClientDependency> getClientDependencies()
183180
{
@@ -186,7 +183,6 @@ public DataColumn(ColumnInfo col, boolean withLookups)
186183
return super.getClientDependencies();
187184
}
188185

189-
190186
protected ColumnInfo getDisplayField(@NotNull ColumnInfo col, boolean withLookups)
191187
{
192188
if (!withLookups)
@@ -195,7 +191,6 @@ protected ColumnInfo getDisplayField(@NotNull ColumnInfo col, boolean withLookup
195191
return null==display ? col : display;
196192
}
197193

198-
199194
@Override
200195
public String toString()
201196
{
@@ -738,7 +733,6 @@ else if (_inputType.equalsIgnoreCase("checkbox"))
738733
return ctx.getForm() == null || col == null ? HtmlString.EMPTY_STRING : ctx.getErrors(col);
739734
}
740735

741-
742736
private void renderSelectFormInput(HtmlWriter out, String formFieldName, Object value, List<String> strValues, boolean disabledInput, NamedObjectList entryList)
743737
{
744738
SelectBuilder select = new SelectBuilder()

0 commit comments

Comments
 (0)