Skip to content
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

JimfsPath.resolve can accept Path instance other than JimfsPath #112

Closed
altair2010 opened this issue Jul 10, 2020 · 2 comments
Closed

JimfsPath.resolve can accept Path instance other than JimfsPath #112

altair2010 opened this issue Jul 10, 2020 · 2 comments
Labels
invalid P3 type=enhancement Make an existing feature better

Comments

@altair2010
Copy link

altair2010 commented Jul 10, 2020

JimfsPath.resolve(Path) doesn't accept Path instance if it's not a JimfsPath one, even if the given Path is a relative one.

When the given Path is not absoute why not to convert to string and parse if to create a JimfsPath ?
something like this

public JimfsPath resolve(Path other) {
    if(!other.isAbsolute()) {
        // Convert other in a JimfsPath using String representation
        other = pathService.parsePath(other.toString());
    }
    
    // Start of existing code
    JimfsPath otherPath = checkPath(other);
    if (otherPath == null) {
      throw new ProviderMismatchException(other.toString());
    }
    ...
}

My use case is

public void productionCode(Path rootPath) {
    ...
    Path relPath = Paths.get("xxx", "yyy", "zzz");
    ...
    Path fullPath = rootPath.resolve(relPath);
    ...
}

@Test
public void test() {
    // Given
    FileSystem fileSystem = Jimfs.newFileSystem();
    Path rootPath = fileSystem.getPath("input").toAbsolutePath();

    // When
    productionCode(rootPath);
}

I see discussion on #12, i understand the limitation of JimfsPath.resolve(Path) for absolute path, but resolving relative path can be done (IMO)

  • using toString() and parsePath
  • using Path.iterator() to retrive all part of the path
@netdpb netdpb added P3 type=enhancement Make an existing feature better labels Jul 13, 2020
@JakeWharton
Copy link

JakeWharton commented Aug 19, 2020

This behavior is consistent with the Java-provided filesystems (default, zip, etc.) so I am against this change.

$ jshell
|  Welcome to JShell -- Version 14.0.2
|  For an introduction type: /help intro

jshell> FileSystems.newFileSystem(URI.create("jar:file:/Users/jw/test.zip"), Map.of())
$1 ==> /Users/jw/test.zip

jshell> $1.getPath("./relative.txt")
$2 ==> ./relative.txt

jshell> Paths.get("/Users/jw").resolve($2)
|  Exception java.nio.file.ProviderMismatchException
|        at UnixPath.toUnixPath (UnixPath.java:198)
|        at UnixPath.resolve (UnixPath.java:410)
|        at UnixPath.resolve (UnixPath.java:43)
|        at (#3:1)

jshell> Paths.get("/Users/jw").resolve($2.toString())
$3 ==> /Users/jw/./relative.txt

@cgdecker
Copy link
Member

cgdecker commented Jan 6, 2021

In addition to what Jake said about the behavior being consistent with the Java-provided filesystems:

  • I would argue that your code is incorrect usage of the APIs (the Path relPath = Paths.get("xxx", "yyy", "zzz"); line specifically); since your productionCode method accepts a Path, the correct way to create a Path that you intend to use with rootPath would be rootPath.getFileSystem().getPath("xxx", "yyy", "zzz"). If you do that, there's no file system mismatch issue.
  • It doesn't make sense in general to try to resolve a Path from one file system against a Path from another given that file systems may have differing semantics. For example, what would you expect to happen if you attempt to resolve UNIX-like Path "foo\bar\baz" (a single filename) against a Windows file system? Different people or different situations might expect different things to happen, and it doesn't really make sense for each file system implementation to have to attempt to deal with any possible implementation of Path it might be given. This is more just my opinion on why the existing file systems don't attempt to deal with this and why we don't want to either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid P3 type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

4 participants