-
Notifications
You must be signed in to change notification settings - Fork 7
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.
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
Add a helper that relativizes and resolves child. #6522
base: develop
Are you sure you want to change the base?
Conversation
|
||
static FileLike resolveChildWithRelativeURI(URI rootURI, URI fileURI) | ||
{ | ||
URI relativeURI = getRelativeURI(rootURI, fileURI); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use URIUtil.relativize() here, rather than introduce a new method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, now removed
return rootURI.relativize(fileURI); | ||
} | ||
|
||
static FileLike resolveChildWithRelativeURI(URI rootURI, URI fileURI) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the use case for having a rootURI w/o already having a FileSystemLike? e.g. where did root come from? (e.g. PipeRoot can return FileSystemLike objects)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good questions! Intent was to pass rootURI created using FileSystemLike, now realized it is redundant. Please see my updates, let me know what you think! Thanks!
static FileLike resolveChildWithRelativeURI(File file) | ||
{ | ||
URI fileURI = file.toURI(); | ||
FileLike allowedRoot = new FileSystemLike.Builder(fileURI).root(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowedRoot==file???
If this were not static then allowed root would be the getRoot()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, thought that the root() was getting the root directory from a file. I will change that line to
FileLike allowedRoot = new FileSystemLike.Builder(file.getParentFile()).root();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we already "trust" file we can simply use FileSystemLike.wrapFile(). This method helper is for when we don't trust the file yet, and want to verify it is within a known directory and then "adopt" it (e.g. turn a URI/File/Path into a FileLike).
Rationale
Related Pull Requests
Changes