From e8efa5a0d0c674a142805ed5d277971a7b246e90 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Tue, 18 Feb 2025 11:33:38 -0800 Subject: [PATCH 1/2] NextFlow improvements: unique job names, full file list, skip template file (#508) --- .../labkey/nextflow/NextFlowController.java | 31 +++++++++++++------ .../pipeline/NextFlowPipelineJob.java | 12 ++++--- .../nextflow/pipeline/NextFlowRunTask.java | 26 +++++++++++----- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/nextflow/src/org/labkey/nextflow/NextFlowController.java b/nextflow/src/org/labkey/nextflow/NextFlowController.java index 69e1297d..ea758886 100644 --- a/nextflow/src/org/labkey/nextflow/NextFlowController.java +++ b/nextflow/src/org/labkey/nextflow/NextFlowController.java @@ -3,7 +3,6 @@ import lombok.Getter; import lombok.Setter; import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.Logger; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; import org.labkey.api.action.FormViewAction; @@ -14,6 +13,7 @@ import org.labkey.api.data.PropertyStore; import org.labkey.api.pipeline.PipeRoot; import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.pipeline.PipelineProvider; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.pipeline.PipelineStatusUrls; import org.labkey.api.pipeline.browse.PipelinePathForm; @@ -31,13 +31,13 @@ import org.labkey.api.util.Path; import org.labkey.api.util.URLHelper; import org.labkey.api.util.element.Select; -import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.HtmlView; import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.nextflow.pipeline.NextFlowPipelineJob; +import org.labkey.nextflow.pipeline.NextFlowProtocol; import org.springframework.validation.BindException; import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; @@ -64,8 +64,6 @@ public class NextFlowController extends SpringActionController private static final DefaultActionResolver _actionResolver = new DefaultActionResolver(NextFlowController.class); public static final String NAME = "nextflow"; - private static final Logger LOG = LogHelper.getLogger(NextFlowController.class, NAME); - public NextFlowController() { setActionResolver(_actionResolver); @@ -262,24 +260,39 @@ public void validateCommand(AnalyzeForm o, Errors errors) @Override public ModelAndView getView(AnalyzeForm o, boolean b, BindException errors) { + List selectedFiles = o.getValidatedFiles(getContainer(), false); + if (selectedFiles.isEmpty()) + { + return new HtmlView(HtmlString.of("Couldn't find input file(s)")); + } + // NextFlow operates on the full directory so show the list to the user, regardless of what they selected + // from the file listing + File inputDir = selectedFiles.get(0).getParentFile(); + + File[] inputFiles = inputDir.listFiles(new PipelineProvider.FileTypesEntryFilter(NextFlowProtocol.INPUT_TYPES)); + if (inputFiles == null || inputFiles.length == 0) + { + return new HtmlView(HtmlString.of("Couldn't find input file(s)")); + } + NextFlowConfiguration config = NextFlowManager.get().getConfiguration(); if (config.getNextFlowConfigFilePath() != null) { File configDir = new File(config.getNextFlowConfigFilePath()); if (configDir.isDirectory()) { - File[] files = configDir.listFiles(); - if (files != null && files.length > 0) + File[] configFiles = configDir.listFiles(); + if (configFiles != null && configFiles.length > 0) { - List configFiles = Arrays.asList(files); return new HtmlView("NextFlow Runner", DIV( FORM(at(method, "POST"), INPUT(at(hidden, true, name, "launch", value, true)), Arrays.stream(o.getFile()).map(f -> INPUT(at(hidden, true, name, "file", value, f))).toList(), "Files: ", - UL(Arrays.stream(o.getFile()).map(DOM::LI)), + UL(Arrays.stream(inputFiles).map(File::getName).map(DOM::LI)), "Config: ", - new Select.SelectBuilder().name("configFile").addOptions(configFiles.stream().filter(f -> f.isFile() && f.getName().toLowerCase().endsWith(".config")).map(File::getName).sorted(String.CASE_INSENSITIVE_ORDER).toList()).build(), + new Select.SelectBuilder().name("configFile").addOptions(Arrays.stream(configFiles).filter(f -> f.isFile() && f.getName().toLowerCase().endsWith(".config")).map(File::getName).sorted(String.CASE_INSENSITIVE_ORDER).toList()).build(), + DOM.BR(), new Button.ButtonBuilder("Start NextFlow").submit(true).build()))); } } diff --git a/nextflow/src/org/labkey/nextflow/pipeline/NextFlowPipelineJob.java b/nextflow/src/org/labkey/nextflow/pipeline/NextFlowPipelineJob.java index 6f31dafa..8a3cd0c0 100644 --- a/nextflow/src/org/labkey/nextflow/pipeline/NextFlowPipelineJob.java +++ b/nextflow/src/org/labkey/nextflow/pipeline/NextFlowPipelineJob.java @@ -59,24 +59,26 @@ public NextFlowPipelineJob(ViewBackgroundInfo info, @NotNull PipeRoot root, Path super(new NextFlowProtocol(), NextFlowPipelineProvider.NAME, info, root, config.getFileName().toString(), config, inputFiles, false, false); this.config = config; setLogFile(log); - LOG.info("NextFlow job queued: {}", getJsonJobInfo()); + LOG.info("NextFlow job queued: {}", getJsonJobInfo(null)); } - protected JSONObject getJsonJobInfo() + protected JSONObject getJsonJobInfo(Long invocationCount) { JSONObject result = new JSONObject(); result.put("user", getUser().getEmail()); result.put("container", getContainer().getPath()); result.put("filePath", getLogFilePath().getParent().toString()); - result.put("runName", getNextFlowRunName()); + result.put("runName", getNextFlowRunName(invocationCount)); result.put("configFile", getConfig().getFileName().toString()); return result; } - protected String getNextFlowRunName() + protected String getNextFlowRunName(Long invocationCount) { PipelineStatusFile file = PipelineService.get().getStatusFile(getJobGUID()); - return file == null ? "Unknown" : ("LabKeyJob" + file.getRowId()); + String result = file == null ? "Unknown" : ("LabKeyJob" + file.getRowId()); + result += invocationCount == null ? "" : ("_" + invocationCount); + return result; } @Override diff --git a/nextflow/src/org/labkey/nextflow/pipeline/NextFlowRunTask.java b/nextflow/src/org/labkey/nextflow/pipeline/NextFlowRunTask.java index 35670120..2749796f 100644 --- a/nextflow/src/org/labkey/nextflow/pipeline/NextFlowRunTask.java +++ b/nextflow/src/org/labkey/nextflow/pipeline/NextFlowRunTask.java @@ -2,6 +2,9 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DbSequence; +import org.labkey.api.data.DbSequenceManager; import org.labkey.api.exp.XarFormatException; import org.labkey.api.pipeline.AbstractTaskFactory; import org.labkey.api.pipeline.AbstractTaskFactorySettings; @@ -37,6 +40,8 @@ public class NextFlowRunTask extends WorkDirectoryTask public static final String ACTION_NAME = "NextFlow"; + private static final DbSequence INVOCATION_SEQUENCE = DbSequenceManager.get(ContainerManager.getRoot(), NextFlowRunTask.class.getName()); + public NextFlowRunTask(Factory factory, PipelineJob job) { super(factory, job); @@ -46,7 +51,12 @@ public NextFlowRunTask(Factory factory, PipelineJob job) public @NotNull RecordedActionSet run() throws PipelineJobException { Logger log = getJob().getLogger(); - NextFlowPipelineJob.LOG.info("Starting to execute NextFlow: {}", getJob().getJsonJobInfo()); + + // NextFlow requires a unique job name for every execution. Increment a counter to append as a suffix to + // ensure uniqueness + long invocationCount = INVOCATION_SEQUENCE.next(); + INVOCATION_SEQUENCE.sync(); + NextFlowPipelineJob.LOG.info("Starting to execute NextFlow: {}", getJob().getJsonJobInfo(invocationCount)); SecurityManager.TransformSession session = null; boolean success = false; @@ -73,10 +83,10 @@ public NextFlowRunTask(Factory factory, PipelineJob job) File dir = getJob().getLogFile().getParentFile(); getJob().runSubProcess(secretsPB, dir); - ProcessBuilder executionPB = new ProcessBuilder(getArgs()); + ProcessBuilder executionPB = new ProcessBuilder(getArgs(invocationCount)); getJob().runSubProcess(executionPB, dir); log.info("Job Finished"); - NextFlowPipelineJob.LOG.info("Finished executing NextFlow: {}", getJob().getJsonJobInfo()); + NextFlowPipelineJob.LOG.info("Finished executing NextFlow: {}", getJob().getJsonJobInfo(invocationCount)); RecordedAction action = new RecordedAction(ACTION_NAME); for (Path inputFile : getJob().getInputFilePaths()) @@ -100,14 +110,16 @@ public NextFlowRunTask(Factory factory, PipelineJob job) } if (!success) { - NextFlowPipelineJob.LOG.info("Failed executing NextFlow: {}", getJob().getJsonJobInfo()); + NextFlowPipelineJob.LOG.info("Failed executing NextFlow: {}", getJob().getJsonJobInfo(invocationCount)); } } } private void addOutputs(RecordedAction action, Path path, Logger log) throws IOException { - if (Files.isRegularFile(path)) + // Skip results.sky.zip files - it's the template document. We want the file output doc that includes + // the replicate analysis + if (Files.isRegularFile(path) && !path.endsWith("results.sky.zip")) { action.addOutput(path.toFile(), "Output", false); if (path.toString().toLowerCase().endsWith(".sky.zip")) @@ -164,7 +176,7 @@ private boolean hasAwsSection(Path configFile) throws PipelineJobException } - private @NotNull List getArgs() throws PipelineJobException + private @NotNull List getArgs(long invocationCount) throws PipelineJobException { NextFlowConfiguration config = NextFlowManager.get().getConfiguration(); Path configFile = getJob().getConfig(); @@ -189,7 +201,7 @@ private boolean hasAwsSection(Path configFile) throws PipelineJobException args.add("-c"); args.add(configFile.toAbsolutePath().toString()); args.add("-name"); - args.add(getJob().getNextFlowRunName()); + args.add(getJob().getNextFlowRunName(invocationCount)); return args; } From 3e81970cdc85ca51faf9bb34491057cc8be55cb9 Mon Sep 17 00:00:00 2001 From: vagisha Date: Tue, 18 Feb 2025 12:58:30 -0800 Subject: [PATCH 2/2] Test for copying a folder containing moved Skyline documents to Panorama Public (#502) * Added test for moving Skyline documents across LabKey folders. * Attempt to fix test failing on TeamCity. * Do not fail if container cannot be deleted because it does not exist. * Attempt to fix test failing on TeamCity. Move documents from project to another project instead of between subfolders of the same project. * Reverted back to using test folder with tricky characters --- .../PanoramaPublicBaseTest.java | 10 +- .../PanoramaPublicMoveSkyDocTest.java | 117 ++++++++++++++++++ 2 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicMoveSkyDocTest.java diff --git a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicBaseTest.java b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicBaseTest.java index 883ade85..189bcf47 100644 --- a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicBaseTest.java +++ b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicBaseTest.java @@ -306,7 +306,7 @@ private void submitForm() clickAndWait(Locator.linkWithText("Back to Experiment Details")); // Navigate to the experiment details page. } - void copyExperimentAndVerify(String projectName, String folderName, String experimentTitle, String destinationFolder, String shortAccessUrl) + void copyExperimentAndVerify(String projectName, @Nullable String folderName, String experimentTitle, String destinationFolder, String shortAccessUrl) { copyExperimentAndVerify(projectName, folderName, null, experimentTitle, null, false, true, destinationFolder, shortAccessUrl); } @@ -325,7 +325,7 @@ void makeCopy(String shortAccessUrl, String experimentTitle, String destinationF makeCopy(shortAccessUrl, experimentTitle, recopy, deleteOldCopy, destinationFolder, true); } - void copyExperimentAndVerify(String projectName, String folderName, @Nullable List subfolders, String experimentTitle, + void copyExperimentAndVerify(String projectName, @Nullable String folderName, @Nullable List subfolders, String experimentTitle, @Nullable Integer version, boolean recopy, boolean deleteOldCopy, String destinationFolder, String shortAccessUrl) { @@ -333,7 +333,7 @@ void copyExperimentAndVerify(String projectName, String folderName, @Nullable Li destinationFolder, shortAccessUrl, true); } - void copyExperimentAndVerify(String projectName, String folderName, @Nullable List subfolders, String experimentTitle, + void copyExperimentAndVerify(String projectName, @Nullable String folderName, @Nullable List subfolders, String experimentTitle, @Nullable Integer version, boolean recopy, boolean deleteOldCopy, String destinationFolder, String shortAccessUrl, boolean symlinks) { @@ -406,7 +406,7 @@ protected final void uncheck(String label) } } - private void verifyCopy(String shortAccessUrl, String experimentTitle, @Nullable Integer version, String projectName, String folderName, List subfolders, boolean recopy) + private void verifyCopy(String shortAccessUrl, String experimentTitle, @Nullable Integer version, String projectName, @Nullable String folderName, List subfolders, boolean recopy) { // Verify the copy goToProjectHome(PANORAMA_PUBLIC); @@ -454,7 +454,7 @@ private void verifyCopy(String shortAccessUrl, String experimentTitle, @Nullable text += "\nEmail: " + REVIEWER_PREFIX; } String messageText = new BodyWebPart(getDriver(), "View Message").getComponentElement().getText(); - var srcFolderTxt = "Source folder: " + "/" + projectName + "/" + folderName; + var srcFolderTxt = "Source folder: " + "/" + projectName + (folderName == null ? "" : "/" + folderName); assertTextPresent(new TextSearcher(messageText), text, srcFolderTxt); // Unescaped special Markdown characters in the message may cause the password to render incorrectly. diff --git a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicMoveSkyDocTest.java b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicMoveSkyDocTest.java new file mode 100644 index 00000000..a469f8de --- /dev/null +++ b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PanoramaPublicMoveSkyDocTest.java @@ -0,0 +1,117 @@ +package org.labkey.test.tests.panoramapublic; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.labkey.test.BaseWebDriverTest; +import org.labkey.test.Locator; +import org.labkey.test.TestFileUtils; +import org.labkey.test.categories.External; +import org.labkey.test.categories.MacCossLabModules; +import org.labkey.test.components.CustomizeView; +import org.labkey.test.util.DataRegionTable; + +import static org.junit.Assert.fail; + + +@Category({External.class, MacCossLabModules.class}) +@BaseWebDriverTest.ClassTimeout(minutes = 5) +public class PanoramaPublicMoveSkyDocTest extends PanoramaPublicBaseTest +{ + private static final String SKY_FILE_1 = "MRMer.zip"; + private static final String SKY_FILE_2 = "MRMer_renamed_protein.zip"; + private static final String SKY_FILE_3 = "SmMolLibA.sky.zip"; + + @Test + public void testExperimentCopy() + { + String projectName = getProjectName(); + String sourceFolder = "SourceFolder"; + String sourceSubfolder = "SourceSubFolder"; + String targetFolder = "TargetFolder"; + + log("Creating source folder " + sourceFolder); + setupSourceFolder(projectName, sourceFolder, SUBMITTER); + log("Creating subfolder, " + sourceSubfolder + " in folder " + sourceFolder); + setupSubfolder(projectName, sourceFolder, sourceSubfolder, FolderType.Experiment, SUBMITTER); + + goToProjectHome(); + log("Creating target folder " + targetFolder); + setupSourceFolder(projectName, targetFolder, SUBMITTER); + + impersonate(SUBMITTER); + updateSubmitterAccountInfo("One"); + + goToProjectFolder(projectName, sourceFolder); + log("Importing " + SKY_FILE_1 + " in folder " + sourceFolder); + importData(SKY_FILE_1, 1); + goToDashboard(); + log("Moving " + SKY_FILE_1 + " FROM " + sourceFolder + " TO " + targetFolder); + moveDocument(SKY_FILE_1, targetFolder, 1); + + var skyDocSourceFolder = sourceFolder + "/" + sourceSubfolder; + goToProjectFolder(projectName, skyDocSourceFolder); + log("Importing " + SKY_FILE_2 + " in folder " + skyDocSourceFolder); + importData(SKY_FILE_2, 1); + goToDashboard(); + log("Moving " + SKY_FILE_2 + " FROM " + skyDocSourceFolder + "TO " + targetFolder); + moveDocument(SKY_FILE_2, targetFolder, 2); + + goToProjectFolder(projectName, targetFolder); + log("Importing " + SKY_FILE_3 + " in folder " + skyDocSourceFolder); + importData(SKY_FILE_3, 3); + + log("Creating and submitting an experiment"); + String experimentTitle = "Experiment to test moving Skyline documents from other folders"; + var expWebPart = createExperimentCompleteMetadata(experimentTitle); + expWebPart.clickSubmit(); + String shortAccessLink = submitWithoutPXId(); + + // Copy the experiment to the Panorama Public project + var panoramaCopyFolder = "Copy of " + targetFolder; + log("Copying experiment to folder " + panoramaCopyFolder +" in the Panorama Public project"); + copyExperimentAndVerify(projectName, targetFolder, experimentTitle, panoramaCopyFolder, shortAccessLink); + goToProjectFolder(PANORAMA_PUBLIC, panoramaCopyFolder); + verifyRunFilePathRoot(SKY_FILE_1, PANORAMA_PUBLIC, panoramaCopyFolder); + verifyRunFilePathRoot(SKY_FILE_2, PANORAMA_PUBLIC, panoramaCopyFolder); + verifyRunFilePathRoot(SKY_FILE_3, PANORAMA_PUBLIC, panoramaCopyFolder); + } + + private void moveDocument(String skylineDocName, String targetFolder, int jobCount) + { + DataRegionTable table = new DataRegionTable.DataRegionFinder(getDriver()).withName("TargetedMSRuns").waitFor(); + table.checkCheckbox(0); + table.clickHeaderButton("Move"); + waitAndClickAndWait(Locator.linkWithText(targetFolder)); + waitForPipelineJobsToComplete(jobCount, false); + + goToDashboard(); + + // Verify that the document moved + table = new DataRegionTable.DataRegionFinder(getDriver()).withName("TargetedMSRuns").waitFor(); + int rowIndex = table.getRowIndex("File", skylineDocName); + if (rowIndex < 0) + fail("Unable to find row for moved Skyline document: " + skylineDocName); + + verifyRunFilePathRoot(skylineDocName, getProjectName(), targetFolder); + } + + private void verifyRunFilePathRoot(String skylineDocName, String projectName, String targetFolder) + { + // Verify that exp.run filePathRoot is set to the target folder + portalHelper.navigateToQuery("exp", "Runs"); + DataRegionTable queryGrid = new DataRegionTable("query", this); + CustomizeView view = queryGrid.openCustomizeGrid(); + view.showHiddenItems(); + view.addColumn("FilePathRoot"); + view.applyCustomView(); + int rowIndex = queryGrid.getRowIndex("Name", skylineDocName); + if (rowIndex < 0) + fail("Unable to find row for Skyline document in exp.Runs query grid: " + skylineDocName); + + var expectedFileRoot = TestFileUtils.getDefaultFileRoot(projectName + "/" + targetFolder).getPath(); + var filePathRoot = queryGrid.getDataAsText(rowIndex, "FilePathRoot"); + + Assert.assertEquals("Unexpected FilePathRoot ",expectedFileRoot, filePathRoot); + } +}