Review request for #7087549

[Brandon Passanisi]

Hi Alan.  Thank you for your comments.  I have created an updated webrev 
based on your suggestions for the alias to review.  Here's the url of 
the updated webrev followed by a summary of the changes:

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

Summary of changes:

1. The test now uses a PassThroughProvider with a custom option, as you 
have suggested in this review thread.
2. I renamed the test file from TestOpenOptions.java to 
CustomOptions.java to reflect the changes to the test.
3. I changed some of the text in the comments give them better descriptions.
4. Some cosmetic changes, including formatting, the use of braces, and 
moving of some comments.

Thanks.

On 12/12/2011 2:14 AM, Alan Bateman wrote:
> On 09/12/2011 20:59, Brandon Passanisi wrote:
>> Hi Alan.  I believe I understand the intention now.  Are you 
>> suggesting the following:
>>
>>  1. Change the last line of newInputStream (line 384 in the webrev
>>     version of FileSystemProvider.java) so that the options parameter
>>     is passed into the Files.newByteChannel() call, i.e.: return
>>     Channels.newInputStream(Files.newByteChannel(path, options));
>>  2. Since the null check is done by newByteChannel, remove the
>>     Objects.requireNonNull call.
>>
>> If this is correct, I can create an updated webrev for review.
> Exactly! The bug is only an issue for providers that support 
> additional open options but don't override newInputStream. With the 
> above changes then the opens are passed through to newByteChannel so 
> the provider can handle them. With that clear then I think the test 
> should be focused on exercising newInputStream with custom options, 
> something like the following:
>
>     static enum CustomOptions implements OpenOption {
>         IGNORE,
>     }
>
>     static class MyProvider extends 
> PassThroughFileSystem.PassThroughProvider {
>         public MyProvider() {
>         }
>         public InputStream newInputStream(Path path, OpenOption... 
> options)
>             throws IOException
>         {
>             if (options.length != 1)
>                 throw new RuntimeException("One option expected");
>             if (options[0] != CustomOptions.IGNORE)
>                 throw new RuntimeException(options[0] + " not expected");
>             return super.newInputStream(path);
>         }
>     }
>
>     public static void main(String[] args) throws Exception {
>         FileSystemProvider provider = new MyProvider();
>         Map<String,?> env = Collections.emptyMap();
>         URI uri = URI.create("pass:///");
>         FileSystem fs = provider.newFileSystem(uri, env);
>
>         :
>         InputStream in = Files.newInputStream(path, CustomOptions.IGNORE);
>     }
>
> PassThroughFileSystem is in test/java/nio/file/Files and is used by 
> tests that need to use a custom file system.
>
> -Alan
>
>

-- 
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/20111216/12cbb7a7/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/20111216/12cbb7a7/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/20111216/12cbb7a7/green-for-email-sig_0.gif