Skip to content

Convert ValidateReadsetFilesAction and ImportReferenceSequencesAction to use FileLike #350

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

Merged
labkey-bpatel merged 6 commits into from
Apr 10, 2025
Merged

Convert ValidateReadsetFilesAction and ImportReferenceSequencesAction to use FileLike #350

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd196e3
Update ValidateReadsetFilesAction and ImportReferenceSequencesAction …
labkey-bpatel Apr 8, 2025
21e947e
Update 'DownloadTempImageAction' to use FileLike.
labkey-bpatel Apr 8, 2025
0bbf7ea
Use getPath() in the error message.
labkey-bpatel Apr 8, 2025
f06440b
Use resolveFile()
labkey-bpatel Apr 9, 2025
e219ea4
Remove DownloadTempImageAction and ConvertTextToFileAction and its as…
labkey-bpatel Apr 9, 2025
ce11f9a
Adjust error, use resolveFile()
labkey-bpatel Apr 9, 2025
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
44 changes: 35 additions & 9 deletions SequenceAnalysis/src/org/labkey/sequenceanalysis/SequenceAnalysisController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
import org.labkey.sequenceanalysis.util.FastqUtils;
import org.labkey.sequenceanalysis.util.SequenceUtil;
import org.labkey.vfs.FileLike;
import org.labkey.vfs.FileSystemLike;
import org.springframework.beans.PropertyValues;
import org.springframework.validation.BindException;
import org.springframework.validation.Errors;
Expand Down Expand Up @@ -434,8 +435,21 @@ public static class DownloadTempImageAction extends ExportAction<TempImageAction
@Override
public void export(TempImageAction form, HttpServletResponse response, BindException errors) throws Exception
Copy link
Author

@labkey-bpatel labkey-bpatel Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbimber - Can you please provide a usage scenario for this action. Thanks!

Copy link
Collaborator

@bbimber bbimber Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you asking for a way to test it?

Copy link
Collaborator

@bbimber bbimber Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I believe both DownloadTempImageAction and ConvertTextToFileAction are really old code that I dont see used anywhere. Rather than refactor them, those classes and the associated form classes could be removed. Let me know if you want to do this or have me do it.

Copy link
Author

@labkey-bpatel labkey-bpatel Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well was wondering how that action gets called and how the values for directory and filename gets set.
But sounds good, if they are not being used, then I can remove them as part of this PR.

{
File parentDir = form.getDirectory() == null ? FileUtil.getTempDirectory() : new File(FileUtil.getTempDirectory(), form.getDirectory());
File targetFile = new File(parentDir, form.getFileName());
File targetFile;
FileLike tempDirRoot = new FileSystemLike.Builder(FileUtil.getTempDirectory()).root();
if (!(tempDirRoot.getFileSystem().isDescendant(tempDirRoot, new File(form.getDirectory()).toURI())))
{
throw new FileNotFoundException("Directory '" + form.getDirectory() + "' is not the descendant of '" + tempDirRoot.getPath() + "'");
}
FileLike parentDirFileLike = form.getDirectory() == null ? tempDirRoot : tempDirRoot.resolveFile(new Path(form.getDirectory()));
File parentDir = FileSystemLike.toFile(parentDirFileLike);

if (!(parentDirFileLike.getFileSystem().isDescendant(parentDirFileLike, new File(form.getFileName()).toURI())))
{
throw new FileNotFoundException("File '" + form.getFileName() + "' is not the descendant of '" + parentDirFileLike.getPath() + "'");
}

targetFile = FileSystemLike.toFile(parentDirFileLike.resolveChild(form.getFileName()));
targetFile = FileUtil.getAbsoluteCaseSensitiveFile(targetFile);

if (!NetworkDrive.exists(targetFile))
Expand Down Expand Up @@ -1119,14 +1133,21 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors)
if (form.getFileNames() != null)
{
//TODO: consider proper container??
File root = PipelineService.get().findPipelineRoot(getContainer()).getRootPath();
File base = root;
PipeRoot root = PipelineService.get().findPipelineRoot(getContainer());

if (null == root)
{
throw new PipelineJobException("Unable to find pipeline root for container: " + getContainer().getPath());
}

FileLike base = root.getRootFileLike();

if (form.getPath() != null)
base = new File(base, form.getPath());
base = base.resolveChild(form.getPath());

for (String fileName : form.getFileNames())
{
File f = new File(base, fileName);
File f = FileSystemLike.toFile(base.resolveChild(fileName));
ExpData data = ExperimentService.get().getExpDataByURL(f, getContainer());
if (data != null)
{
Expand All @@ -1137,7 +1158,7 @@ public ApiResponse execute(ValidateReadsetImportForm form, BindException errors)
Map<String, Object> map = new HashMap<>();
map.put("fileName", fileName);
map.put("filePath", f.getPath());
map.put("relPath", FileUtil.relativePath(FileUtil.getAbsoluteCaseSensitiveFile(root).getPath(), FileUtil.getAbsoluteCaseSensitiveFile(f).getPath()));
map.put("relPath", FileUtil.relativePath(FileUtil.getAbsoluteCaseSensitiveFile(FileSystemLike.toFile(root.getRootFileLike())).getPath(), FileUtil.getAbsoluteCaseSensitiveFile(f).getPath()));
map.put("container", getContainer().getId());
map.put("containerPath", getContainer().getPath());
String basename = SequenceTaskHelper.getUnzippedBaseName(fileName);
Expand Down Expand Up @@ -2320,7 +2341,12 @@ public Object execute(ImportFastaSequencesForm form, BindException errors) throw
//resolve files
List<File> files = new ArrayList<>();
PipeRoot root = PipelineService.get().getPipelineRootSetting(getContainer());
File baseDir = StringUtils.trimToNull(form.getPath()) == null ? root.getRootPath() : new File(root.getRootPath(), form.getPath());
FileLike baseDir = null != root ? root.getRootFileLike() : null;
if (baseDir == null)
{
errors.reject(ERROR_MSG, "Pipeline root not configured");
return null;
}
if (!baseDir.exists())
{
errors.reject(ERROR_MSG, "Unable to find directory: " + baseDir.getPath());
Expand All @@ -2335,7 +2361,7 @@ public Object execute(ImportFastaSequencesForm form, BindException errors) throw

for (String fn : form.getFileNames())
{
File f = new File(baseDir, fn);
File f = FileSystemLike.toFile(baseDir.resolveChild(fn));
Copy link

@labkey-matthewb labkey-matthewb Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to convert back to File here. FileLike.exists() should work

Copy link

@labkey-matthewb labkey-matthewb Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind I see that this gets passed to the job. That's a future story.

if (f.exists())
{
files.add(f);
Expand Down