JSR 203 file system implementation

I’ve committed the beginnings of the JSR 203 file system implementation I was talking about to the nio-fs branch. All of it is under the VASSAL.tools.nio package, where VASSAL.tools.nio corresponds to what will be in java.nio in JDK7.

The VASSAL.tools.nio.channels and VASSAL.tools.nio.file packages contain mostly interfaces. The implementation is mostly in VASSAL.tools.nio.file.fs. What’s there are (incomplete) implementations for UNIX-style file systems and Windows-style file systems. Once we have those working, the next thing to do is create a read-only ZIP file system and a read-write ZIP file system (the former to be based on ZipFile, the latter on FileArchive, which is already in the trunk).

The first thing to do is write some tests for the methods in Path. I’ll probably start in on some of that over the weekend.

Calsir, I’m guessing that you haven’t written tests with JUnit before?

Well, for once the guess is wrong :slight_smile:, as the tutorial for eclipse and java (eclipsetutorial.sourceforge.net/ … inner.html) that I used was focused on TDD. In the previous project (a small applet to use in vassal for calculating odds in an rpg) I managed to use tests successfully. Your current plan seems quite ambitious to me, as I would not know where to begin to write the methods for accessing the file system, even with the help provided by the interface and abstract class declarations.

If you want, I can take a look at creating tests for WindowsPath.

Thus spake “Calsir”:

Great!

We don’t have to write much of anything for accessing the file system
ourselves. We can simply wrap java.io.File for things like file attributes,
and use existing channel and stream clases for those things. (I’ve coded
those things already, mostly.) A bunch of other methods we won’t use or have
no way to implement without native code, so I’ve made them throw
UnsupportedOperationExceptions.

The methods which aren’t finished—and the ones for which we could use
tests—are the ones which have to do with path manipulation. Those mainly
amount to a bunch of string processing, of the sort which is easy to get
wrong.

That would be great. On Sunday, I’ll get together a list of exactly what
needs doing.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

like File.canExecute()? Or are we to use JSE6 for this implementation? It gives me error when I try to give it compliance to 1.5, as the method is not supported by it. I am trying to figure out a work-around, but I can think of no solution.

I see.

Good :slight_smile:.

Thus spake “Calsir”:

That’s a mistake. File.canExecute() shouldn’t be used, since it’s a 1.6
method. (One drawback of Sun’s compiler is that the -source and -target
flags don’t prevent you from calling newer methods—all they do is ensure
that properly-versioned class files are written, hence I didn’t see any
warning for this.)

Anyway, we don’t need it, so it’s ok to throw an
UnsupportedOperationException. I’ve fixed that.

There are a few places in our codebase where we do call 1.6 methods when
running on 1.6, which we do via reflection. You can find an exmaple of
this in the big static block in VASSAL.tools.ApplicationIcons. I don’t
expect that we’ll need any reflection for this project, though.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Calsir”:

There’s barely any nontrivial code outside of the classes which implement
Path. Here’s a list of methods which I’ve spotted which should have tests:

Path.compareTo
Path.endsWith
Path.equals
Path.getName
Path.getNameCount
Path.getParent
Path.getRoot
Path.isAbsolute
Path.isSameFile
Path.iterator
Path.normalize
Path.resolve
Path.startsWith
Path.subpath
Path.toAbsolutePath
Path.toRealPath
Path.toString
Path.toURi

Files.createDirectories

For the Path methods, this means a test for each Path implementation (for
starters, that’s WindowsPath and UnixPath), but there should be a lot of
overlap among all of the implementations, so I expect that we’ll be able
to reuse most of the tests. (Also with the code—I think a great deal of
the code will ultimately end up in AbstractPath or RealPath, rather than
in the various leaf classes.)

How would you like to divide this up? There are no tests at present, and
some of the implementation for these methods is also missing.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

I have one question: what is the rationale between the splitting of the three abstract classes Path, AbstractPath and RealPath? Couldn’t at least two of them (AbstractPath and RealPath) be merged, or even the whole set? I am asking because I have no idea on where to put the code, since that every code that goes to RealPath could go directly to Path, changing the calls in the various FileSystem subclasses.

About the tests/implementations, I could very well begin by implementing WindowsPath, since I have no linux box. Besides, we could use some TDD and first write the tests, then the implementations. About the current list, I could take half of them (say, from Path.compareTo to Path.isAbsolute). Just one question, should we create two classes of tests to avoid conflicts, or is svn capable of handling them on its own?

Thus spake “Calsir”:

Path is intended to be wholly abstract, i.e., there’s supposed to be no
code in it. I’d rather it be an interface, but JSR 203 defined it to be
an abstract base class, so we have to follow.

I’m intending for AbstractPath to contain code which is the same regardless
of what FileSystem the Path comes from.

I’m intending for RealPath to contain code which is common to Paths which
point to files on disk.

The reason for AbstractPath and RealPath being different is that ZipPath,
which is for ZIP archives and doesn’t exist yet, will be a subslass of
AbstractPath but not of RealPath.

Because we already have a specification—namely the Javadoc for JSR 203—
there’s no harm in writing the code at the same time as the tests. I’ve
been filling in some of the missing method code this evening, so there’s
much less of that left to implement now.

The tests should go under the test hierarchy, not the src hierarchy; I
won’t be working in test/VASSAL/tools/nio at all until I’m done with
writing code under sr/VASSAL/tools/nio, so if you want to work on the
tests we won’t be stepping on each other’s toes.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

I am unable to post through the mailing list: using the forum now.

Joel Uckelman wrote:

Path is intended to be wholly abstract, i.e., there’s supposed to be no
code in it. I’d rather it be an interface, but JSR 203 defined it to be
an abstract base class, so we have to follow.

I’m intending for AbstractPath to contain code which is the same regardless
of what FileSystem the Path comes from.

I’m intending for RealPath to contain code which is common to Paths which
point to files on disk.

The reason for AbstractPath and RealPath being different is that ZipPath,
which is for ZIP archives and doesn’t exist yet, will be a subslass of
AbstractPath but not of RealPath.

Ok, now the rationale is clear. I will write the tests for WindowsPath, AbstractPath and RealPath in their own test classes (named ClassNameTest) in the proper location under test/ . There are redefinitions of abstract methods (with the abstract modifier) in RealPath. Shall I put the test in RealPathTest and then flag them with @ignore or shall I just forget about them?

I can also write the tests for UnixPath but I will be unable to run them.

Because we already have a specification—namely the Javadoc for JSR 203—
there’s no harm in writing the code at the same time as the tests. I’ve
been filling in some of the missing method code this evening, so there’s
much less of that left to implement now.

I will follow that.

Thus spake “Calsir”:

If this persists and it’s a problem on our end, let me know.

I’m not sure how you can write tests for RealPath, because it’s abstract—
you can’t construct an object of type RealPath without it being a subclass.

Just write tests for WindowsPath and UnixPath. (The only difference I see
between WindowsPath and UnixPath is that roots are not the same, so I figure
that the test will hardly differ at all. If you use File.separator for the
separator, you don’t even have to care whether / or \ is being used.)


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Calsir”:

These are disappearing as we speak. Either they’re going to become concrete
becasue I’m implementing them in RealPath, or they’re going to disappear
I’m implementing them in RealPath’s subclasses.

J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

If this persists and it’s a problem on our end, let me know.

Testing right now :slight_smile:… And it failed (using the forum again). I’ll look into this tomorrow.

I’m not sure how you can write tests for RealPath, because it’s abstract—
you can’t construct an object of type RealPath without it being a subclass.

Yeah, I was thinking about it right after I sent the message. After some googling, I found that it is possible to declare RealPathTest and AbstractPathTest as abstract, then implement the methods in the subclasses. Or use Mock objects (as in mockito, here: code.google.com/p/mockito/).

Just write tests for WindowsPath and UnixPath. (The only difference I see
between WindowsPath and UnixPath is that roots are not the same, so I figure
that the test will hardly differ at all. If you use File.separator for the
separator, you don’t even have to care whether / or \ is being used.)

Ok, I will write tests for WindowsPath with that in mind.

I am also creating the stubs for the test methods thanks to eclipse.

When I try to run WindowsPathTest i get

java.util.regex.PatternSyntaxException: Unexpected internal error near index 11 ^[A-Za-z]:\ ^ at java.util.regex.Pattern.error(Pattern.java:1713) at java.util.regex.Pattern.compile(Pattern.java:1466) at java.util.regex.Pattern.<init>(Pattern.java:1133) at java.util.regex.Pattern.compile(Pattern.java:823) at java.util.regex.Pattern.matches(Pattern.java:928) at java.lang.String.matches(String.java:2090) at VASSAL.tools.nio.file.winfs.WindowsPath.findRootSep(WindowsPath.java:21) at VASSAL.tools.nio.file.fs.RealPath.splitPath(RealPath.java:74) [snipped the rest]

I get the same error both with 1.6 and 1.5. If I comment the culprit line

    if (s.matches("^[A-Za-z]:\\")) return 2;

the tests run smoothly. Looking at the regex, it looks like a normal match: “look at the beginning of the string (^) for a single letter [A-Za-z]followed by a colon( : ) and a backslash(\)”. Thoughts?

Besides, should not findRootSep be static?

Thus spake “Calsir”:

Absolutely. But Java doesn’t permit abstract static methods.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

I should have seen that coming…
I committed most of the tests. There is some error either in splitPath() or in getRoot() (both in RealPath).

I do not understand whether it is intended that (parts[0] == 0) means that the path is relative or not. I say so because, while debugging, I have seen that many absolute paths (c:\foo\bar) have parts[0] = 0.

For instance, here is the path and the parts of WindowsPathTest.pathTestingDirectory on my machine:

D:\Arch\Docs\Src\Java\Vassal 3.2-Local\test\VASSAL\tools\nio\file\winfs\testingDirectory

[0, 3, 8, 13, 17, 22, 39, 44, 51, 57, 61, 66, 72, 89]

Mind that the “” were resolved by the debugger to “”. There is something odd with the previous logic, as the first non-root element should beginning with character at index 3 (“Arch”).

If I understand correctly WindowsPath.findRootSep(), its aim is to return the index of the separator, 2 for “c:”, 1 for “” and 0 for “/”. -1 If no separator exists, so that 0 identifies a relative path. It seems that it conflicts with the previous definition.

I also added a “-1” to the for cycle in normalize(), since I would constantly get indexOutOfBoundException. Its code does not work anyhow. Now the for cycle behaves correctly, beginning at length-1 and ending at i==1.

If I am allowed a remark, “l” (lowercase L) makes for a very poor variable name.

Thus spake “Calsir”:

mockito looks quite interesting. Mocking classes can be a pain.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Calsir”:

I always forget that Java regexes require an absurd number of backslashes
when you want to match one backslash.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Calsir”:

That’s a bug, then. The first element of parts should be the index of the
first character of the first name in the path. That is: path[0] == 0 iff
the Path is relative. (Additionally, path.length == 0 iff the Path is
just a root component. In all other cases, the Path is absolute and has
at least one name element.)

I’ll have a look at that right now.

(Sure, remark away.) Why do you say that?


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

I fixed findRootSep and solved this issue: string.matches matches the whole string to the regex, rather than only the beginning.

I used File.listRoots() to get the available roots in WindowsPath, UNC names are still resolved through regexes.

I am still trying to figure out how to do normalize() properly. The current implementation fails to consider multiple … occurrences, as in c:\dir1\dir2…\dir3 (which should target c:\dir3).