Skip to content

Fix Windows FileLike path resolution for background R report outputs #7524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
labkey-bpatel wants to merge 3 commits into
base: develop
Choose a base branch
from
+65 −38
Open

Fix Windows FileLike path resolution for background R report outputs #7524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05cecb5
Use toNioPathForRead(), add debug logs
labkey-bpatel Mar 25, 2026
e3dede6
Code review change - fix R report output directory resolution/make th…
labkey-bpatel Mar 25, 2026
974d972
Add comment. Revert change.
labkey-bpatel Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 37 additions & 34 deletions api/src/org/labkey/api/reports/report/ScriptEngineReport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
14 changes: 14 additions & 0 deletions api/src/org/labkey/api/reports/report/r/RReport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
18 changes: 14 additions & 4 deletions api/src/org/labkey/api/reports/report/r/RReportJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -275,16 +274,22 @@ protected void processOutputs(RReport report, List<ParamReplacement> 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
Expand All @@ -294,6 +299,7 @@ protected void processOutputs(RReport report, List<ParamReplacement> 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);
}
Expand All @@ -317,6 +323,7 @@ protected void processOutputs(RReport report, List<ParamReplacement> 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
Expand All @@ -325,16 +332,19 @@ protected void processOutputs(RReport report, List<ParamReplacement> 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);
Expand Down
Loading