Improve file path validation (#181)

Author: labkey-jeckels

genotyping/src/org/labkey/genotyping/HaplotypeAssayProvider.java

@@ -46,6 +46,7 @@
 import org.labkey.api.assay.AssayUrls;
 import org.labkey.api.study.assay.ParticipantVisitResolverType;
 import org.labkey.api.util.FileType;
+import org.labkey.api.util.HtmlString;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.util.Pair;
 import org.labkey.api.view.ActionURL;
@@ -135,7 +136,7 @@ public String getName()
     @Override
     public HttpView<?> getDataDescriptionView(AssayRunUploadForm form)
     {
-        return new HtmlView("");
+        return new HtmlView(HtmlString.EMPTY_STRING);
     }
 
     @Override
@@ -205,26 +206,18 @@ protected Map<String, Set<String>> getRequiredDomainProperties()
     {
         Map<String, Set<String>> domainMap = super.getRequiredDomainProperties();
 
-        Set<String> runProperties = domainMap.get(ExpProtocol.ASSAY_DOMAIN_RUN);
-        if (runProperties == null)
-        {
-            runProperties = new HashSet<>();
-            domainMap.put(ExpProtocol.ASSAY_DOMAIN_RUN, runProperties);
-        }
+        Set<String> runProperties = domainMap.computeIfAbsent(ExpProtocol.ASSAY_DOMAIN_RUN, k -> new HashSet<>());
         runProperties.add(ENABLED_PROPERTY_NAME);
-        for (String propName : getColumnMappingProperties(true).keySet())
-        {
-            runProperties.add(propName);
-        }
+        runProperties.addAll(getColumnMappingProperties(true).keySet());
         runProperties.add(SPECIES_COLUMN.getName());
 
         return domainMap;
     }
 
     @Override
-    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, File> uploadedFiles, AssayRunUploadForm context)
+    public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, org.labkey.vfs.FileLike> uploadedFiles, AssayRunUploadForm context)
     {
-        return Collections.singletonList(new HaplotypeDataCollector());
+        return Collections.singletonList(new HaplotypeDataCollector<>());
     }
 
     @Override

genotyping/src/org/labkey/genotyping/HaplotypeDataCollector.java

@@ -43,7 +43,7 @@ public class HaplotypeDataCollector<ContextType extends AssayRunUploadContext<Ha
     private Map<String, String> _reshowMap;
 
     @Override
-    public HttpView getView(ContextType context)
+    public HttpView<?> getView(ContextType context)
     {
         // if reshowing on error, get the data param out of the context for the JSP to use
         HttpServletRequest request = context.getRequest();

genotyping/src/org/labkey/genotyping/HaplotypeDataHandler.java

@@ -47,6 +47,7 @@
 import org.labkey.api.assay.AssayService;
 import org.labkey.api.view.ActionURL;
 import org.labkey.api.view.ViewBackgroundInfo;
+import org.labkey.vfs.FileLike;
 
 import java.io.File;
 import java.io.IOException;
@@ -70,17 +71,17 @@ public DataType getDataType()
     }
 
     @Override
-    public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context) throws ExperimentException
+    public void importFile(@NotNull ExpData data, @NotNull FileLike dataFile, @NotNull ViewBackgroundInfo info, @NotNull Logger log, @NotNull XarContext context) throws ExperimentException
     {
         if (!dataFile.exists())
         {
-            log.warn("Could not find file " + dataFile.getAbsolutePath() + " on disk for data with LSID " + data.getLSID());
+            log.warn("Could not find file " + dataFile + " on disk for data with LSID " + data.getLSID());
             return;
         }
         ExpRun expRun = data.getRun();
         if (expRun == null)
         {
-            throw new ExperimentException("Could not load haplotype file " + dataFile.getAbsolutePath() + " because it is not owned by an experiment run");
+            throw new ExperimentException("Could not load haplotype file " + dataFile + " because it is not owned by an experiment run");
         }
 
         try
@@ -100,7 +101,7 @@ public void importFile(@NotNull ExpData data, File dataFile, @NotNull ViewBackgr
             Map<String, String> animalIds = new CaseInsensitiveTreeMap<>();
             List<HaplotypeIdentifier> haplotypes = new ArrayList<>();
             List<HaplotypeAssignmentDataRow> dataRows = new ArrayList<>();
-            TabLoader tabLoader = new TabLoader(dataFile, true);
+            TabLoader tabLoader = new TabLoader(dataFile.openInputStream(), true, data.getContainer());
             List<Map<String, Object>> rowsMap = tabLoader.load();
             parseHaplotypeData(protocol, rowsMap, runPropertyValues, animalIds, haplotypes, dataRows);
 

genotyping/src/org/labkey/genotyping/IlluminaFastqParser.java

@@ -214,7 +214,7 @@ else if (reader.getLineNumber() == 1 && totalReads == 0 && !f.getName().contains
                             sampleId = _sampleNameToIdMap.get(sampleName);
                         }
                         String name = (_outputPrefix == null ? "Reads" : _outputPrefix) + "-R" + pairNumber + "-" + (sampleIdx == 0 ? "Control" : sampleId) + ".fastq.gz";
-                        File newFile = new File(targetDir, name);
+                        File newFile = FileUtil.appendName(targetDir, name);
 
                         if (!f.equals(newFile))
                         {
@@ -408,9 +408,7 @@ public void testHeaders() throws PipelineJobException, IOException
         {
             Module module = ModuleLoader.getInstance().getModule("genotyping");
             File newHeaderPath = JunitUtil.getSampleData(module, "genotyping/illumina_newHeader");
-            String newHeaderSampledataLoc = newHeaderPath.toString();
             File oldHeaderPath = JunitUtil.getSampleData(module, "genotyping");
-            String oldHeaderSampledataLoc = oldHeaderPath.toString();
             final List<String> filenamesOldHeader = Arrays.asList(
                     "IlluminaSamples-R1-4892.fastq.gz",
                     "IlluminaSamples-R1-4893.fastq.gz",
@@ -472,8 +470,7 @@ public void testHeaders() throws PipelineJobException, IOException
                     "IlluminaSamplesNewHeader-R2-4905.fastq.gz"
             );
 
-            final Pair[] pairs =
-            {
+            final var pairs = List.of(
                 new Pair<>(4892, 1),
                 new Pair<>(4893, 1),
                 new Pair<>(4894, 1),
@@ -501,11 +498,10 @@ public void testHeaders() throws PipelineJobException, IOException
                 new Pair<>(4902, 2),
                 new Pair<>(4903, 2),
                 new Pair<>(4904, 2),
-                new Pair<>(4905, 2)
-            };
+                new Pair<>(4905, 2));
 
             int i = 0;
-            int numOfPairs = pairs.length;
+            int numOfPairs = pairs.size();
             Set<Pair<Integer, Integer>> expectedOutputs = new HashSet<>();
             Map<Integer, Integer> sampleIndexToIdMap = new IntHashMap<>();
             sampleIndexToIdMap.put(0, 0);
@@ -516,15 +512,15 @@ public void testHeaders() throws PipelineJobException, IOException
 
             for (String fn : filenamesOldHeader)
             {
-                File target = new File(_testRoot, fn);
-                FileUtils.copyFile(new File(oldHeaderSampledataLoc, fn), target);
-                expectedOutputs.add((Pair<Integer, Integer>) pairs[i]);
+                File target = FileUtil.appendName(_testRoot, fn);
+                FileUtils.copyFile(FileUtil.appendName(oldHeaderPath, fn), target);
+                expectedOutputs.add(pairs.get(i));
                 oldHeaderFiles.add(target);
 
                 if (i < (numOfPairs / 2))
                 {
-                    sampleIndexToIdMap.put(i + 1, (Integer)pairs[i].getKey());
-                    sampleIdToIndexMap.put((Integer)pairs[i].getKey(), i + 1);
+                    sampleIndexToIdMap.put(i + 1, pairs.get(i).getKey());
+                    sampleIdToIndexMap.put(pairs.get(i).getKey(), i + 1);
                 }
                 i++;
             }
@@ -537,29 +533,15 @@ public void testHeaders() throws PipelineJobException, IOException
 
             for (String fn : filenamesNewHeader)
             {
-                File target = new File(_testRoot, fn);
-                FileUtils.copyFile(new File(newHeaderSampledataLoc, fn), target);
+                File target = FileUtil.appendName(_testRoot, fn);
+                FileUtils.copyFile(FileUtil.appendName(newHeaderPath, fn), target);
                 newHeaderFiles.add(target);
             }
 
             parser = new IlluminaFastqParser(null, sampleIndexToIdMap, sampleIdToIndexMap, Collections.emptyMap(), LogManager.getLogger(HeaderTestCase.class), newHeaderFiles);
             outputs = parser.parseFastqFiles(null);
             Assert.assertEquals("Outputs from parseFastqFiles with new headers were not as expected.", expectedOutputs, outputs.keySet());
         }
-
-        // @After // TODO: Disabling to debug gradle failure on TeamCity
-        public void cleanup() throws IOException
-        {
-            if (container != null)
-            {
-                ContainerManager.delete(container, TestContext.get().getUser());
-            }
-
-            if (_testRoot != null)
-            {
-                FileUtils.deleteDirectory(_testRoot);
-            }
-        }
     }
 }