Review request for #7087549

[Brandon Passanisi]

Hello nio-dev.  I was wondering if somebody could please review the 
proposed fix for bug #7087549 : (fs) FileSystemProvider.newInputStream 
throws IllegalArgumentException when custom open options.  The 
information pertaining to the webrev and a description of the changes 
are below:

    Bug URL: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7087549
    Webrev: http://cr.openjdk.java.net/~alanb/7087549/webrev/
    <http://cr.openjdk.java.net/%7Ealanb/7087549/webrev/>


Description of changes:

src/share/classes/java/nio/file/spi/FileSystemProvider.java:

    The changes were based on information provided in the bug report. 
    The if-condition was modified because of the following quote within
    the mail message specified in the bug report: "This seems to be an
    oversight, it should allow any open option except WRITE and APPEND
    through." Here's the link to the message:
    http://mail.openjdk.java.net/pipermail/nio-discuss/2011-September/000615.html

    The inclusion of Objects.requireNonNull method is because of the
    following quote within the bug report: "That issue also causes
    incorrect null-argument behavior: if options array contains null
    element the method throws UOE instad of NPE."  If the inclusion of
    the Objects.requireNonNull method is determined to be ok, it would
    likely be a good idea to make the same change to
    FileSystemProvider.newOutputStream.  If so, a new bug could be
    created for this or the change could be included within a revised
    webrev.  If you agree and have a preference, please let me know.

test/java/nio/file/spi/TestOpenOptions.java:

    I wrote a test program that tests all of the available OpenOption
    values against the new behavior for
    FileSystemProvider.newInputStream. It also tests the behavior using
    a null value passed into newInputStream, both as an individual
    option and as a member of an OpenOption[] array.

    In the test, I chose to silently ignore IOExceptions.  I did this
    because I wasn't sure the best value to pass in as a path to
    FileSystems.getDefault().getPath() because I wasn't sure if there
    exists a path that we can guarantee to be on all platforms that can
    be accessible without failure.  So, I chose to use the empty string
    FileSystems.getDefault().getPath("") and figured that might be ok
    for the test because the testing of the OpenOption behavior is done
    before newInputStream delegates to Channel.newInputStream.  And,
    that might be the thing that is most useful during testing.  But, if
    you think there should be a different value passed into getPath()
    for use with this test please let me know.

Thanks.
-- 
Oracle <http://www.oracle.com>
Brandon Passanisi | Principle Member of Technical Staff

Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
developing practices and products that help protect the environment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20111209/04b7f6ae/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20111209/04b7f6ae/oracle_sig_logo.gif 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: green-for-email-sig_0.gif
Type: image/gif
Size: 356 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20111209/04b7f6ae/green-for-email-sig_0.gif