diff --git a/api/src/org/labkey/api/reports/report/ScriptEngineReport.java b/api/src/org/labkey/api/reports/report/ScriptEngineReport.java index edb0cdc1e2d..3bdeea35386 100644 --- a/api/src/org/labkey/api/reports/report/ScriptEngineReport.java +++ b/api/src/org/labkey/api/reports/report/ScriptEngineReport.java @@ -60,8 +60,7 @@ import org.labkey.api.reports.report.r.view.TsvOutput; import org.labkey.api.thumbnail.Thumbnail; import org.labkey.api.util.FileUtil; -import org.labkey.api.util.Path; -import org.labkey.api.util.UnexpectedException; +import org.labkey.api.util.UnexpectedException; import org.labkey.api.view.HttpView; import org.labkey.api.view.VBox; import org.labkey.api.view.ViewContext; @@ -71,8 +70,7 @@ import javax.script.ScriptEngine; import javax.script.ScriptException; -import java.io.File; -import java.io.IOException; +import java.io.IOException; import java.io.PrintWriter; import java.sql.ResultSetMetaData; import java.sql.SQLException; @@ -212,36 +210,41 @@ public FileLike createInputDataFile(@NotNull ViewContext context) throws SQLExce * Note: This method used to stash results in members (_tempFolder and _tempFolderPipeline), but that no longer works * now that we cache reports between threads (e.g., Thread.currentThread().getId() is part of the path). */ - public FileLike getReportDirFileLike(@NotNull String executingContainerId) - { - boolean isPipeline = BooleanUtils.toBoolean(getDescriptor().getProperty(ScriptReportDescriptor.Prop.runInBackground)); - return getReportDirFileLike(executingContainerId, isPipeline); - } - - protected FileLike getReportDirFileLike(@NotNull String executingContainerId, boolean isPipeline) - { - FileLike tempRoot = getTempRootFileLike(getDescriptor()); - String reportId = FileUtil.makeLegalName(String.valueOf(getDescriptor().getReportId())).replaceAll(" ", "_"); - FileLike tempFolder; - - try - { - if (isPipeline) - { - String identifier = RReportJob.getJobIdentifier(); - if (identifier != null) - tempFolder = FileUtil.appendPath(tempRoot, - Path.parse(executingContainerId + File.separator + "Report_" + reportId + File.separator + identifier)); - else - tempFolder = FileUtil.appendPath(tempRoot, - Path.parse(executingContainerId + File.separator + "Report_" + reportId)); - } - else - tempFolder = FileUtil.appendPath(tempRoot, - Path.parse(executingContainerId + File.separator + "Report_" + reportId + File.separator + Thread.currentThread().getId())); - - FileUtil.mkdirs(tempFolder); - return tempFolder; + public FileLike getReportDirFileLike(@NotNull String executingContainerId) + { + boolean isPipeline = BooleanUtils.toBoolean(getDescriptor().getProperty(ScriptReportDescriptor.Prop.runInBackground)); + return getReportDirFileLike(executingContainerId, isPipeline); + } + + protected FileLike getBackgroundOutputDirFileLike(@NotNull String executingContainerId) + { + FileLike tempRoot = getTempRootFileLike(getDescriptor()); + String reportId = FileUtil.makeLegalName(String.valueOf(getDescriptor().getReportId())).replaceAll(" ", "_"); + + // Build FileLike paths from VFS path segments instead of File.separator so Windows + // backslashes do not get folded into a single org.labkey.api.util.Path segment. + return tempRoot.resolveChild(executingContainerId).resolveChild("Report_" + reportId); + } + + protected FileLike getReportDirFileLike(@NotNull String executingContainerId, boolean isPipeline) + { + FileLike tempFolder; + + try + { + tempFolder = getBackgroundOutputDirFileLike(executingContainerId); + + if (isPipeline) + { + String identifier = RReportJob.getJobIdentifier(); + if (identifier != null) + tempFolder = tempFolder.resolveChild(identifier); + } + else + tempFolder = tempFolder.resolveChild(String.valueOf(Thread.currentThread().getId())); + + FileUtil.mkdirs(tempFolder); + return tempFolder; } catch (IOException e) { diff --git a/api/src/org/labkey/api/reports/report/r/RReport.java b/api/src/org/labkey/api/reports/report/r/RReport.java index 83371fcb9b7..8629be1af03 100644 --- a/api/src/org/labkey/api/reports/report/r/RReport.java +++ b/api/src/org/labkey/api/reports/report/r/RReport.java @@ -696,6 +696,20 @@ public FileLike getReportDirFileLike(@NotNull String executingContainerId) return super.getReportDirFileLike(executingContainerId); } + @Override + protected FileLike getBackgroundOutputDirFileLike(@NotNull String executingContainerId) + { + FileLike reportDir = null; + + if (getKnitrFormat() != RReportDescriptor.KnitrFormat.None) + reportDir = getCacheDir(executingContainerId); + + if (reportDir != null) + return reportDir; + + return super.getBackgroundOutputDirFileLike(executingContainerId); + } + @Override public void clearCache() { diff --git a/api/src/org/labkey/api/reports/report/r/RReportJob.java b/api/src/org/labkey/api/reports/report/r/RReportJob.java index 2413636089c..e63397754b0 100644 --- a/api/src/org/labkey/api/reports/report/r/RReportJob.java +++ b/api/src/org/labkey/api/reports/report/r/RReportJob.java @@ -42,7 +42,6 @@ import java.io.File; import java.io.Serializable; -import java.nio.file.CopyOption; import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.HashMap; @@ -275,16 +274,22 @@ protected void processOutputs(RReport report, List outputSubst { // write the output substitution map to disk so we can render the view later FileLike reportDir = report.getReportDirFileLike(getJob().getContainerId()); + FileLike parentDir = report.getBackgroundOutputDirFileLike(getJob().getContainerId()); FileLike substitutionMap; - if (reportDir.getName().equals(getJobIdentifier())) + // ScriptEngineReport#getReportDirFileLike() appends the ThreadLocal job identifier only + // when background execution needs a per-job child directory under parentDir. + boolean hasJobSpecificReportDir = !reportDir.equals(parentDir); + if (hasJobSpecificReportDir) { - FileLike parentDir = reportDir.getParent(); // clean up the destination folder for (FileLike file : parentDir.getChildren()) { if (!file.isDirectory() && !"log".equalsIgnoreCase(FileUtil.getExtension(file))) + { + getJob().debug("deleting parent file=" + file); file.delete(); + } } // rewrite the parameter replacement files to point to the destination folder @@ -294,6 +299,7 @@ protected void processOutputs(RReport report, List outputSubst for (FileLike file : replacement.getFiles()) { FileLike newFile = parentDir.resolveChild(file.getName()); + getJob().debug("move replacement file =" + replacement.getName() + " from=" + file + " to=" + newFile); file.move(newFile); newFiles.add(newFile); } @@ -317,6 +323,7 @@ protected void processOutputs(RReport report, List outputSubst { newFile = FileUtil.createTempFile(LOG_FILE_PREFIX, ".log", parentDir); getJob().setLogFile(newFile); + getJob().debug("copy log file from=" + file + " to=" + newFile); FileUtil.copyFile(file, newFile, StandardCopyOption.REPLACE_EXISTING); } // report.log != getLogFile(), just regular file @@ -325,16 +332,19 @@ protected void processOutputs(RReport report, List outputSubst String logFileContent = PageFlowUtil.getStreamContentsAsString(file.openInputStream()); if (!StringUtils.isEmpty(logFileContent)) getJob().info("REPORT.LOG CONTENTS:\n" + logFileContent); + getJob().debug("move log file from=" + file + " to=" + newFile); file.move(newFile); } } else { + getJob().debug("move file from=" + file + " to=" + newFile); file.move(newFile); } } + getJob().debug("delete reportDir=" + reportDir); FileUtil.deleteDir(reportDir); - substitutionMap = reportDir.getParent().resolveChild(RReport.SUBSTITUTION_MAP); + substitutionMap = parentDir.resolveChild(RReport.SUBSTITUTION_MAP); } else substitutionMap = reportDir.resolveChild(RReport.SUBSTITUTION_MAP);