marschall / memoryfilesystem

An in memory implementation of a JSR-203 file system
282 stars 36 forks source link

Path.touri().getPath() returns null #141

Closed blacelle closed 1 year ago

blacelle commented 1 year ago

Hi. I'm a new MemoryFileSystem user, for Cleanthat. Thanks


I get unexpectly null on Path (from memoryFileSystem), when calling .getUri.getPath().

Here is a reproduction case:

public class TestMemoryFileSystem {
    @Test
    public void testGetUriGetPath() throws IOException {
        FileSystem fs = MemoryFileSystemBuilder.newEmpty().build("myFs");
        Path p = fs.getPath("root", "directory", "file.txt");
        Assertions.assertThat(p.toUri().toASCIIString()).isEqualTo("memory:myFs:///root/directory/file.txt");
        Assertions.assertThat(p.toUri().getPath()).isEqualTo("/root/directory/file.txt");
    }
}

The uri looks like: memory:toto:///root/directory/file.txt.

Is this a limitation with a specific underlying reason, or could I hope a structural change of the URI to enable Java URI parsing a path from it?

Looking at java.net.URI.Parser.parse(boolean), the uri parsing fails around:

            if (at(p, n, '/')) {                     <-- KO, we we get a '_' instead of '/'

                p = parseHierarchical(p, n);
            } else {
                // opaque; need to create the schemeSpecificPart
                int q = scan(p, n, "#");
                if (q <= p)
                    failExpecting("scheme-specific part", p);
                checkChars(p, q, L_URIC, H_URIC, "opaque part");
                schemeSpecificPart = input.substring(p, q);
                p = q;
            }

I feel URI expects a raw uri like memory:/_g_1:///root/directory/file.txt ?


Spec wise:

https://www.rfc-editor.org/rfc/rfc3986#section-3.2

The authority component is preceded by a double slash ("//") and is terminated by the next slash ("/"), question mark ("?"), or number sign ("#") character, or by the end of the URI.

https://www.rfc-editor.org/rfc/rfc3986#section-3.3

If a URI contains an authority component, then the path component must either be empty or begin with a slash ("/") character.


Alternative URI proposal:

@Test
public void testUriProposal() throws IOException, URISyntaxException {
    URI urlProposal = new URI("memory", "myFs", "/root/directory/file.txt", null);
    Assertions.assertThat(urlProposal.toASCIIString()).isEqualTo("memory://myFs/root/directory/file.txt");
    Assertions.assertThat(urlProposal.getPath()).isEqualTo("/root/directory/file.txt");
}
marschall commented 1 year ago

Interesting, I'll have a look and report back.

marschall commented 1 year ago

Having looked a bit more into it I'm undecided so far. We have the constraint thatURI#getScheme() has to return FileSystemProvider#getScheme(). If you check the the URI constructors there are basically two variants. One with a scheme and a path, and an opaque one with a scheme and scheme specific part.

We could put the name of the file system as the first element of the path but then URI#getPath() would return the file path with a prefix.

May I ask why you expect URI#getPath() to be non-null? Path#toUri() makes no such guarantees. For example the path of JAR URIs is also null URI.create("jar:file:///C:/proj/parser/jar/parser.jar!/test.xml").getPath()

blacelle commented 1 year ago

May I ask why you expect URI#getPath() to be non-null?

This is expected by some library I rely on:

https://github.com/openrewrite/rewrite/blob/main/rewrite-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11ParserInputFileObject.java#L116

@Override
public boolean isNameCompatible(String simpleName, Kind kind) {
    String baseName = simpleName + kind.extension;
    return kind.equals(getKind())
            && (baseName.equals(toUri().getPath()).              <--- Here, null is OK
            || toUri().getPath().endsWith("/" + baseName));    <--- Here, null is NPE
}

For some reason, they get the path after going through a URI. It looks awkward to me, but here I am.

For what it worth, the same scenario is OK with Jimfs.newFileSystem(). With JimFS, the URI looks lile : jimfs://dfc87a7e-bf29-4569-ad85-3033052cc909/someModule/src/main/java/some_package/someFilePath.java (which is similar to my suggestion)

marschall commented 1 year ago

Sorry, I somehow missed your reply. Do you happen to have a reproducer? I'm looking into opening a PR against OpenRewrite.

marschall commented 1 year ago

https://github.com/openrewrite/rewrite/pull/3010 let's see where it goes

marschall commented 1 year ago

My PR against OpenRewrite got merged so I'm closing this as won´t fix. I hope that's ok for you.

blacelle commented 1 year ago

Nice @marschall