diff --git a/experiment/src/org/labkey/experiment/DataURLRelativizer.java b/experiment/src/org/labkey/experiment/DataURLRelativizer.java index 0c75bbb58dc..341668d4a63 100644 --- a/experiment/src/org/labkey/experiment/DataURLRelativizer.java +++ b/experiment/src/org/labkey/experiment/DataURLRelativizer.java @@ -22,6 +22,7 @@ import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; import org.labkey.api.view.ActionURL; +import org.labkey.experiment.api.ExperimentServiceImpl; import org.labkey.experiment.controllers.exp.ExperimentController; import java.io.IOException; @@ -75,21 +76,16 @@ public URLRewriter createURLRewriter() @Override public String rewriteURL(Path path, ExpData data, String roleName, ExpRun expRun, User user, String rootFilePath) throws ExperimentException { - try - { - if (path == null) - return null; + if (path == null) + return null; - if (expRun == null || expRun.getFilePathRoot() == null) - { - return FileUtil.pathToString(path); - } - return FileUtil.relativizeUnix(expRun.getFilePathRootPath(), path, false); - } - catch (IOException e) + if (expRun == null || expRun.getFilePathRoot() == null) { - throw new ExperimentException(e); + return FileUtil.pathToString(path); } + + // NOTE: this will only write a relative path if the file is a direct descendant of the root. + return ExperimentServiceImpl.get().generatePathStringRelativeToRootIfUnderRoot(path, expRun.getFilePathRootPath()); } }; } diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 910158d21dc..d57867806c2 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -153,6 +153,7 @@ import org.labkey.experiment.lineage.ExpLineageServiceImpl; import org.labkey.experiment.lineage.LineagePerfTest; import org.labkey.experiment.pipeline.ExperimentPipelineProvider; +import org.labkey.experiment.pipeline.XarTestPipelineJob; import org.labkey.experiment.samples.DataClassFolderImporter; import org.labkey.experiment.samples.DataClassFolderWriter; import org.labkey.experiment.samples.SampleStatusFolderImporter; @@ -1079,7 +1080,8 @@ public Collection getSummary(Container c) SampleTypeServiceImpl.TestCase.class, StorageNameGenerator.TestCase.class, StorageProvisionerImpl.TestCase.class, - UniqueValueCounterTestCase.class + UniqueValueCounterTestCase.class, + XarTestPipelineJob.TestCase.class ); } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 21f9e1a7194..24cca6884d6 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -768,33 +768,10 @@ public ExpDataImpl createData(URI uri, XarSource source) throws XarFormatExcepti String[] parts = pathStr.split("/"); String name = FileUtil.decodeSpaces(parts[parts.length - 1]); Path path = FileUtil.getPath(source.getXarContext().getContainer(), uri); - if (path != null) { - try - { - path = FileUtil.stringToPath(source.getXarContext().getContainer(), - source.getCanonicalDataFileURL(FileUtil.pathToString(path))); - - // Only convert to a relative path if this is a descendant of the root: - path = path.normalize(); - if (URIUtil.isDescendant(source.getRootPath().toUri(), path.toUri())) - { - pathStr = FileUtil.relativizeUnix(source.getRootPath(), path, false); - } - else - { - pathStr = FileUtil.pathToString(path); - } - } - catch (IOException e) - { - pathStr = FileUtil.pathToString(path); - } - } - else - { - pathStr = FileUtil.uriToString(uri); + path = FileUtil.stringToPath(source.getXarContext().getContainer(), source.getCanonicalDataFileURL(FileUtil.pathToString(path))); + pathStr = generatePathStringRelativeToRootIfUnderRoot(path, source.getRootPath()); } Lsid.LsidBuilder lsid = new Lsid.LsidBuilder(LsidUtils.resolveLsidFromTemplate(AutoFileLSIDReplacer.AUTO_FILE_LSID_SUBSTITUTION, source.getXarContext(), "Data", new AutoFileLSIDReplacer(pathStr, source.getXarContext().getContainer(), source))); @@ -816,6 +793,30 @@ public ExpDataImpl createData(URI uri, XarSource source) throws XarFormatExcepti return data; } + public String generatePathStringRelativeToRootIfUnderRoot(@NotNull Path path, @Nullable Path pipeRootPath) + { + String pathStr; + try + { + // Only convert to a relative path if this is a descendant of the root: + path = path.normalize(); + if (pipeRootPath != null && URIUtil.isDescendant(pipeRootPath.toUri(), path.toUri())) + { + pathStr = FileUtil.relativizeUnix(pipeRootPath, path, false); + } + else + { + pathStr = FileUtil.pathToString(path); + } + } + catch (IOException e) + { + pathStr = FileUtil.pathToString(path); + } + + return pathStr; + } + @Override public ExpDataImpl createData(Container container, @NotNull DataType type) { diff --git a/experiment/src/org/labkey/experiment/pipeline/XarTestPipelineJob.java b/experiment/src/org/labkey/experiment/pipeline/XarTestPipelineJob.java new file mode 100644 index 00000000000..3505a179f0f --- /dev/null +++ b/experiment/src/org/labkey/experiment/pipeline/XarTestPipelineJob.java @@ -0,0 +1,549 @@ +package org.labkey.experiment.pipeline; + +import org.apache.commons.io.FileUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; +import org.labkey.api.data.WorkbookContainerType; +import org.labkey.api.exp.api.ExpData; +import org.labkey.api.exp.api.ExpRun; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.pipeline.XarGeneratorId; +import org.labkey.api.module.Module; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.pipeline.AbstractTaskFactory; +import org.labkey.api.pipeline.AbstractTaskFactorySettings; +import org.labkey.api.pipeline.ParamParser; +import org.labkey.api.pipeline.PipeRoot; +import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.pipeline.PipelineJobService; +import org.labkey.api.pipeline.PipelineService; +import org.labkey.api.pipeline.PipelineStatusFile; +import org.labkey.api.pipeline.RecordedAction; +import org.labkey.api.pipeline.RecordedActionSet; +import org.labkey.api.pipeline.TaskFactory; +import org.labkey.api.pipeline.TaskId; +import org.labkey.api.pipeline.TaskPipeline; +import org.labkey.api.pipeline.TaskPipelineSettings; +import org.labkey.api.pipeline.file.AbstractFileAnalysisJob; +import org.labkey.api.pipeline.file.FileAnalysisJobSupport; +import org.labkey.api.query.FieldKey; +import org.labkey.api.reader.Readers; +import org.labkey.api.security.User; +import org.labkey.api.util.FileType; +import org.labkey.api.util.FileUtil; +import org.labkey.api.util.TestContext; +import org.labkey.api.util.URLHelper; +import org.labkey.api.view.ViewBackgroundInfo; +import org.labkey.experiment.ExperimentModule; +import org.labkey.vfs.FileLike; +import org.labkey.vfs.FileSystemLike; + +import java.io.BufferedReader; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * The goal is this class is to provide a "minimal" test case to exercise the Pipeline/XarGenerator code. + * As filesystem-related permissions are tightened, this can be extended to include more test cases, such as: + * + * - PipelineJob in Container1 referencing a file in /Shared + * - PipelineJob in a workbook that references a file in the parent container + * - PipelineJob in a workbook that references a file in a sibling workbook + * - Check that inappropriate cross-folder access is disallowed + * - If LK implements a scheme to determine allowable locations outside the LK root, test that here. + */ +public class XarTestPipelineJob extends PipelineJob implements FileAnalysisJobSupport +{ + public static final String PROVIDER_NAME = "XarTestPipelineJob Provider"; + + private TaskId _taskPipelineId; + + private String _jobName; + private FileLike _webserverJobDir; + private List _inputFiles; + private List _outputFiles; + + // Default constructor for serialization + protected XarTestPipelineJob() + { + } + + private XarTestPipelineJob(Container c, User user, PipeRoot pipeRoot, String jobName, List inputFiles, List outputFiles) + { + super(PROVIDER_NAME, new ViewBackgroundInfo(c, user, null), pipeRoot); + + _taskPipelineId = getTaskIdForEngine(); + _webserverJobDir = getOrCreateBaseDir(c, jobName); + setLogFile(getOrCreateLogFile(c, jobName)); + + _jobName = jobName; + _inputFiles = inputFiles; + _outputFiles = outputFiles; + } + + private static Path getOrCreateLogFile(Container c, String jobName) + { + return getOrCreateBaseDir(c, jobName).resolveChild(FileUtil.makeLegalName(jobName) + ".log").toNioPathForWrite(); + } + + private static FileLike getOrCreateBaseDir(Container c, String jobName) + { + PipeRoot pipeRoot = PipelineService.get().findPipelineRoot(c); + if (pipeRoot == null) + { + throw new IllegalStateException("Pipeline root not found for: " + c.getPath()); + } + + FileLike baseDir = pipeRoot.resolvePathToFileLike(FileUtil.makeLegalName(jobName)); + if (!baseDir.exists()) + { + try + { + baseDir.mkdirs(); + } + catch (IOException e) + { + throw new RuntimeException("Unable to create baseDir: " + c.getPath(), e); + } + } + + return baseDir; + } + + public static XarTestPipelineJob createJob(Container c, User user, String jobName, List inputFiles, List outputFiles) + { + PipeRoot pipelineRoot = PipelineService.get().getPipelineRootSetting(c); + + return new XarTestPipelineJob(c, user, pipelineRoot, jobName, inputFiles, outputFiles); + } + + @Nullable + @Override + public TaskId getActiveTaskId() + { + //ensure this TaskFactory is registered: + try + { + TaskId taskFactoryId = getTaskFactoryId(); + try + { + if (PipelineJobService.get().getTaskFactory(taskFactoryId) == null) + { + registerTaskPipeline(); + } + } + catch (NullPointerException e) + { + //this indicates the TaskFactory has not been registered yet + getLogger().error("A NullPointerException was thrown in XarTestPipelineJob", e); + registerTaskPipeline(); + } + } + catch (CloneNotSupportedException e) + { + getLogger().error(e.getMessage(), e); + } + + return super.getActiveTaskId(); + } + + private static TaskId getTaskIdForEngine() + { + return new TaskId(XarTestPipelineJob.class, PipelineJobService.LocationType.WebServer.name()); + } + + private static TaskId getTaskFactoryId() + { + return new TaskId(XarTestTaskFactory.class, PipelineJobService.LocationType.WebServer.name()); + } + + public static void registerTaskPipeline() throws CloneNotSupportedException + { + //first register TaskFactory + TaskId taskFactoryId = getTaskFactoryId(); + PipelineJobService.get().addTaskFactory(new XarTestTaskFactory(taskFactoryId)); + + //then TaskPipeline + TaskId taskPipelineId = getTaskIdForEngine(); + TaskFactory xarFact = PipelineJobService.get().getTaskFactory(new TaskId(XarGeneratorId.class)); + if (xarFact == null) + { + throw new IllegalStateException("Unable to find TaskFactory for XarGeneratorId.class"); + } + + TaskPipelineSettings settings = new TaskPipelineSettings(taskPipelineId); + settings.setTaskProgressionSpec(new Object[]{taskFactoryId, xarFact.getId()}); + settings.setDeclaringModule(ModuleLoader.getInstance().getModule(ExperimentModule.class)); + PipelineJobService.get().addTaskPipeline(settings); + } + + @Override + public TaskPipeline getTaskPipeline() + { + return PipelineJobService.get().getTaskPipeline(_taskPipelineId); + } + + @Override + public URLHelper getStatusHref() + { + return null; + } + + @Override + public String getDescription() + { + return _jobName; + } + + private static final String INPUT_ROLE = "Input File"; + private static final String OUTPUT_ROLE = "Output File"; + + private static class XarTestTaskFactory extends AbstractTaskFactory + { + public XarTestTaskFactory(TaskId id) + { + super(id); + + setLocation(PipelineJobService.LocationType.WebServer.name()); + } + + @Override + public Task createTask(PipelineJob job) + { + return new Task<>(this, job) + { + @NotNull + @Override + public RecordedActionSet run() + { + // The purpose of this is to specify files outside the LK folder root: + RecordedAction action = new RecordedAction(XarTestTaskFactory.class.getName()); + + if (getJob() instanceof XarTestPipelineJob xj) + { + xj._inputFiles.forEach(x -> action.addInput(x.toURI(), INPUT_ROLE)); + + xj._outputFiles.forEach(x -> action.addOutput(x.toURI(), OUTPUT_ROLE, false)); + } + + return new RecordedActionSet(action); + } + }; + } + + @Override + public List getInputTypes() + { + return null; + } + + @Override + public List getProtocolActionNames() + { + return Arrays.asList(XarTestTaskFactory.class.getName()); + } + + @Override + public String getStatusName() + { + return "RUNNING"; + } + + @Override + public boolean isJobComplete(PipelineJob job) + { + return false; + } + } + + @Override + public String getProtocolName() + { + return _jobName; + } + + @Override + public String getJoinedBaseName() + { + throw new IllegalArgumentException("not implemented"); + } + + @Override + public List getSplitBaseNames() + { + throw new IllegalArgumentException("not implemented"); + } + + @Override + public String getBaseName() + { + return getAnalysisDirectory().getName(); + } + + @Override + public String getBaseNameForFileType(FileType fileType) + { + return getBaseName(); + } + + @Override + public FileLike getDataDirectory() + { + return _webserverJobDir; + } + + @Override + public FileLike getAnalysisDirectory() + { + return _webserverJobDir; + } + + @Override + public FileLike findInputFile(String name) + { + return getAnalysisDirectory().resolveChild(name); + } + + @Override + public FileLike findOutputFile(String name) + { + return getAnalysisDirectory().resolveChild(name); + } + + @Override + public FileLike findOutputFile(@NotNull String outputDir, @NotNull String fileName) + { + return AbstractFileAnalysisJob.getOutputFile(outputDir, fileName, getPipeRoot(), getLogger(), getAnalysisDirectory()); + } + + @Override + public ParamParser createParamParser() + { + return PipelineJobService.get().createParamParser(); + } + + @Override + public @Nullable FileLike getParametersFile() + { + return null; + } + + @Override + public List getInputFiles() + { + return _inputFiles; + } + + @Override + public FileType.gzSupportLevel getGZPreference() + { + return FileType.gzSupportLevel.PREFER_GZ; + } + + public static class TestCase extends Assert + { + private static final String PROJECT_NAME = "XarPipelineTestProject"; + + @BeforeClass + public static void initialSetUp() + { + //pre-clean + cleanup(); + + Container project = ContainerManager.getForPath(PROJECT_NAME); + if (project == null) + { + project = ContainerManager.createContainer(ContainerManager.getRoot(), PROJECT_NAME, TestContext.get().getUser()); + Set modules = new HashSet<>(project.getActiveModules()); + modules.add(ModuleLoader.getInstance().getModule(ExperimentModule.class)); + project.setActiveModules(modules); + } + } + + @AfterClass + public static void cleanup() + { + Container project = ContainerManager.getForPath(PROJECT_NAME); + if (project != null) + { + File pipelineRoot = PipelineService.get().getPipelineRootSetting(project).getRootPath(); + try + { + if (pipelineRoot.exists()) + { + FileUtils.deleteDirectory(pipelineRoot); + } + } + catch (IOException e) + { + throw new RuntimeException(e); + } + + ContainerManager.deleteAll(project, TestContext.get().getUser()); + } + } + + @Test + public void xarTest() throws Exception + { + // For testing, intentionally use the real file system room to test relative paths + File root = new File("/"); + doXarTest( + "XarTestJob1", + Arrays.asList(FileSystemLike.wrapFile(root, new File("/arbitrary/path/outside/lkRoot/myFile.txt"))), + Arrays.asList(FileSystemLike.wrapFile(root, new File("/another/arbitrary/path/outside/lkRoot/myFile.txt"))) + ); + + Container project = ContainerManager.getForPath(PROJECT_NAME); + PipeRoot projectRoot = PipelineService.get().getPipelineRootSetting(project); + Assert.assertNotNull(PROJECT_NAME + " pipeline root is null", projectRoot); + + // We expect /Shared to be an allowable output location: + PipeRoot sharedRoot = PipelineService.get().getPipelineRootSetting(ContainerManager.getSharedContainer()); + Assert.assertNotNull("Shared pipeline root is null", sharedRoot); + + // A pipeline job submitted to a project/folder should be able to access files in /Shared + doXarTest( + "XarTestJob_UsingShared", + Arrays.asList( + projectRoot.resolvePathToFileLike("myFileInJobFolder.txt"), + sharedRoot.resolvePathToFileLike("myFileInSharedFolder.txt") + ), + Arrays.asList( + projectRoot.resolvePathToFileLike("myOutputFileInJobFolder.txt"), + sharedRoot.resolvePathToFileLike("myOutputFileInSharedFolder.txt") + ) + ); + + // Now create workbooks in this folder: + Container wb1 = ContainerManager.createContainer(project, null, "WB1", null, WorkbookContainerType.NAME, TestContext.get().getUser()); + Container wb2 = ContainerManager.createContainer(project, null, "WB2", null, WorkbookContainerType.NAME, TestContext.get().getUser()); + + PipeRoot wb1Root = PipelineService.get().getPipelineRootSetting(wb1); + Assert.assertNotNull("wb1Root is null", wb1Root); + PipeRoot wb2Root = PipelineService.get().getPipelineRootSetting(wb2); + Assert.assertNotNull("wb2Root is null", wb2Root); + + // A pipeline job submitted to a workbook should be able to reference files in /Shared, the parent folder, or sibling workbooks: + doXarTest( + "XarTestJob_AcrossWorkbooks", + Arrays.asList( + projectRoot.resolvePathToFileLike("myFileInJobFolder.txt"), + sharedRoot.resolvePathToFileLike("myFileInSharedFolder.txt"), + wb1Root.resolvePathToFileLike("myFileInWB1Folder.txt"), + wb2Root.resolvePathToFileLike("myFileInWB2Folder.txt") + ), + Arrays.asList( + projectRoot.resolvePathToFileLike("myOutputFileInJobFolder.txt"), + sharedRoot.resolvePathToFileLike("myOutputFileInSharedFolder.txt"), + wb1Root.resolvePathToFileLike("myOutputFileInWB1Folder.txt"), + wb2Root.resolvePathToFileLike("myOutputFileInWB2Folder.txt") + ), + wb2 + ); + } + + @Test + public void xarTestRelativePaths() throws Exception + { + // For testing, intentionally use the real file system room to test relative paths + File root = new File("/"); + // NOTE: these will get converted to absolute paths by the pipeline code when they are saved, so this passes right now: + //Assert.assertThrows("Maybe LabKey shouldn't allow this", Exception.class, () -> { + doXarTest( + "XarTestJob_QuestionablePaths", + Arrays.asList(FileSystemLike.wrapFile(root, new File("../../../root/myFile.txt"))), + Arrays.asList(FileSystemLike.wrapFile(root, new File("../../../users/root/anotherFile.txt"))) + ); + //}); + } + + private void doXarTest(String jobName, List inputFiles, List outputFiles) throws Exception + { + doXarTest(jobName, inputFiles, outputFiles, ContainerManager.getForPath(PROJECT_NAME)); + } + + private void doXarTest(String jobName, List inputFiles, List outputFiles, Container c) throws Exception + { + PipelineJob job1 = XarTestPipelineJob.createJob(c, TestContext.get().getUser(), jobName, inputFiles, outputFiles); + PipelineService.get().queueJob(job1); + long start = System.currentTimeMillis(); + long timeout = 10 * 1000; //10 secs + while (!isJobDone(job1)) + { + Thread.sleep(1000); + + long duration = System.currentTimeMillis() - start; + if (duration > timeout) + { + //NOTE: it's possible a job could time out on a busy cluster. rather than fail, continue in case there's a second engine to test + _log.warn("timed out waiting for job: " + job1.getDescription()); + break; + } + } + + PipelineStatusFile sf = PipelineService.get().getStatusFile(job1.getJobGUID()); + Assert.assertNotNull("Missing status file", sf); + + List runs = ExperimentService.get().getExpRunsForJobId(sf.getRowId()); + Assert.assertEquals("Wrong run number", 1, runs.size()); + + ExpRun run = runs.get(0); + List inputs = run.getInputDatas(INPUT_ROLE, null); + Assert.assertEquals("Wrong input number", inputFiles.size(), inputs.size()); + + List outputs = run.getOutputDatas(null); + Assert.assertEquals("Wrong output number", outputFiles.size(), outputs.size()); + } + + private static boolean isJobDone (PipelineJob job) throws Exception + { + TableInfo ti = PipelineService.get().getJobsTable(job.getUser(), job.getContainer()); + TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("job"), job.getJobGUID()), null); + Map map = ts.getMap(); + + if (PipelineJob.TaskStatus.complete.matches((String) map.get("status"))) + return true; + + //look for errors + boolean error = PipelineJob.TaskStatus.error.matches((String) map.get("status")); + if (error) + { + StringBuilder sb = new StringBuilder(); + try (BufferedReader reader = Readers.getReader(job.getLogFile())) + { + sb.append("*******************\n"); + sb.append("Error running XarTestPipelineJob.TestCase. Pipeline log:\n"); + String line; + while ((line = reader.readLine()) != null) + { + sb.append(line).append('\n'); + } + + sb.append("*******************\n"); + } + + _log.error(sb.toString()); + + throw new Exception("There was an error running job: " + job.getDescription()); + } + + return false; + } + } +} diff --git a/visualization/resources/web/vis/genericChart/genericChartHelper.js b/visualization/resources/web/vis/genericChart/genericChartHelper.js index eeaf19d7dfd..25825e93efd 100644 --- a/visualization/resources/web/vis/genericChart/genericChartHelper.js +++ b/visualization/resources/web/vis/genericChart/genericChartHelper.js @@ -1940,6 +1940,7 @@ LABKEY.vis.GenericChartHelper = new function(){ var queryChartData = function(renderTo, queryConfig, chartConfig, callback) { queryConfig.containerPath = LABKEY.container.path; + queryConfig.method = 'POST'; // GitHub Issue 731 if (queryConfig.filterArray && queryConfig.filterArray.length > 0) { var filters = [];