RFR 8211919: ZipDirectoryStream should provide a stream of paths that are relative to the directory

Langer, Christoph christoph.langer at sap.com
Mon Jan 14 14:13:50 UTC 2019


Hi Lance,

as I'm currently active in that area, it's an easy one for me to review this ��

Overall the change looks good, thanks for doing it. Here are some few nits I discovered:

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java:
in the comments:
line 426: Maybe you could take the chance to improve the text of the comment ?
// (1) assume all path from zip file itself is "normalized" -> (1) assume each path from zip file is "normalized"
line 432: I think the word "also" can be removed

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java:
line 95: Could you please also fix the indentation of @Override (4 instead of 3 spaces)

test/jdk/jdk/nio/zipfs/Basic.java:
line 45: I think you should remove 8211385 from the @bug tag, since you removed the DirectoryStream tests from this file

test/jdk/jdk/nio/zipfs/ZipFsDirectoryStreamTests.java
I think you should add a @bug tag for both, 8211385 and 8211919
line 53: UNZIPFS_MAP: not needed
line 70: ; not needed in try statement
line 72: jarFile should be createad outside of try block
line 97: spelling: "filter"
line 106: space before + missing
line 144: ";" is not needed and bracket of try could be closed on this line
line 170: Spelling: use "a" instead of "an" ?
line 187: "an" instead of "a" ?
line 205: same ("an" instead of "a")
line 215: the assert for IllegalStateException could be dropped here, as it is a superclass of ClosedDirectoryStreamException which is asserted in line 219? Unless you want to use this as a test for ClosedDirectoryStreamException class hierarchy...
line 238: I think this commented debug code should be removed.
line 306: Looks like the matcher instance is not needed
line 309: you could use an import java.util.zip.ZipException?
line 314: The check should rather be: if (ds.iterator().hasNext()) throw...

Best regards
Christoph

> -----Original Message-----
> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> Of Lance Andersen
> Sent: Samstag, 12. Januar 2019 21:13
> To: core-libs-dev <core-libs-dev at openjdk.java.net>; nio-dev <nio-
> dev at openjdk.java.net>
> Subject: RFR 8211919: ZipDirectoryStream should provide a stream of paths
> that are relative to the directory
> 
> Hi all,
> 
> The following patch addresses the issue where the stream of paths where
> not relative to the specified directory as needed.
> 
> As part of the fix, I added additional DirectoryStream tests similar to the tests
> in nio/file/DirectoryStream/Basic.java.
> 
> Webrev can be found:
> http://cr.openjdk.java.net/~lancea/8211919/webrev.00/
> 
> Best
> Lance
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
> 
> 



More information about the core-libs-dev mailing list