Adopt FileLike for more pipeline APIs #7134
Conversation
|
|
||
| // Load parameter files | ||
| _parametersOverrides = getInputParameters().getInputParameters(); | ||
|
|
There was a problem hiding this comment.
One thought about _dirData. I imagine this often points into a larger FileSystemLike corresponding to a PipeRoot. If this analysis is supposed to read/write files in this sub-directory, is there any value in creating a new FileSystemLike rooted here?
| private AnalysisScript(File script) | ||
| { | ||
| try | ||
| if (!script.exists()) |
There was a problem hiding this comment.
It looks like there is a direct path from SaveProtocolAction to here, however it's not clear that any FS specific permissions have been validated. Should there be a version of FileSystemLike.wrap (or this new?) method take a user or proof of authorization for user events? (we could also have an .unsafe() to mimic usages elsewhere e.g. SQLFragment() and HtmlString()).
There was a problem hiding this comment.
See the check here:
It would be nice to switch to mandating that all scripts be placed in the special WebDav @scripts directory or in a module, at which point AnalysisScript could use that as the root.
I don't think that we can put a permission check at this level as the user who is executing the script by uploading assay data may not have the permissions to configure the script in the first place.
There was a problem hiding this comment.
| return !root.isCloudRoot() ? | ||
| new LocalDirectory(workingDir.toFile(), moduleName, baseLogFileName) : | ||
| new LocalDirectory(root.getContainer(), moduleName, root, baseLogFileName); | ||
| new LocalDirectory(workingDir.toFile(), baseLogFileName) : |
There was a problem hiding this comment.
This class is a big TODO. Later...
labkey-matthewb
left a comment
labkey-matthewb
left a comment
There was a problem hiding this comment.
Some questions and minor suggestions.
| { | ||
| try | ||
| { | ||
| return FileSystemLike.wrapFile(getDataDirectory(), file); |
There was a problem hiding this comment.
It looks like Ian's getVerifiedFileLike() was meant for this purpose. There should probably be a new non-static method for "adopting" a File into a FileSystemLike. E.g. FileLike ret = getDataDirectoryFileLike().wrapDescendant(File f);
There was a problem hiding this comment.
I like this idea but am not sure we want to have FileLike methods that accept a File. Could be OK as an interim approach, but I'm inclined to do another pass in a followup that tries to convert a lot more of these methods (plus WorkDirectory, LocalDirectory, etc) to take FileLike instead of File or Path
There was a problem hiding this comment.
It's definitely a method to aid work in progress (then again so is wrapFile()).
| { | ||
| ret = new FileSystemLocal(uri, canReadFiles, canWriteFiles, canDeleteRoot); | ||
| } | ||
|
|
There was a problem hiding this comment.
static FileLike getVerifiedFileLike() looks out of place here (old code), and now unused?
There was a problem hiding this comment.
It's got a few uses in OConnorLabModules where it's providing important protection against dangerous paths. Not huge to refactor but not trivial either, so I propose leaving it for this PR's purposes
| List<FileLike> getChildren(); | ||
|
|
||
| @NotNull | ||
| default List<FileLike> getChildren(Predicate<FileLike> filter) |
There was a problem hiding this comment.
Would getChildren(Predicate<Pair<String(name),Boolean(isdirectory)>> filter) be useful? That would let the FileLike impl push down the predicate (before new FileLike())
There was a problem hiding this comment.
I like the idea but when I tried to implement it, it was going to be clunky to avoid checking if it's a directory when we didn't need or want to.
| result.put("jmsType", PipelineService.get().getJmsType().toString()); | ||
|
|
||
| result.put("pipelineRootCount", PipelineService.get().getAllPipelineRoots().size()); | ||
| result.put("supplementalDirectories", new TableSelector(PipelineSchema.getInstance().getTableInfoPipelineRoots(), new SimpleFilter("SupplementalPath", null, CompareType.NONBLANK), null).getRowCount()); |
# Conflicts: # pipeline/src/org/labkey/pipeline/PipelineController.java
Rationale
FileLike helps us do better validation of file paths. We can use it instead of File or Path in places like where the user selects files to take a pipeline action.
Changes
FileLikeTasks 📍