RFR: JDK-8161230 ClassLoader: add resource methods returning java.util.stream.Stream

[Patrick Reinhart]

Hi Mandy,

The current changes can be found here:

http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.02


On 02.09.2016 23:11, Mandy Chung wrote:
>> On Sep 2, 2016, at 2:50 AM, Patrick Reinhart <patrick at reini.net> wrote:
>>
>> Updated the existing http://cr.openjdk.java.net/~reinhapa/reviews/8161230/webrev.01
> ClassLoader::resources returning Stream<URL> is a good addition. 
>  
> 1386      * {@code IOException} occur getting the next resource element, it must be 
> 1387      * wrapped into an {@code UncheckedIOException}.
>
> This has been mentioned in the paragraph above. This can be dropped from @apiNote. And add:
>
> @throws UncheckedIOException if I/O errors occur
There was already an discussion about this with Alan Bateman:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042792.html

So I do not know is the best solution for this...

> 1392      * @return  An stream of resource {@link java.net.URL {@code URL}}
>
> {@code…} is not needed as @link generates the text in <code>..</code> format.  You can simply write:
>    {@link java.net.URL URL}
>
> typo s/An/A

I have that fixed those now in the actual revision and hopefully using
the correct "a" all the times.

> I’m unsure if “resource URL” clearly describes it.  What about
>    @return A stream of {@code URL} representing the location of the resources with the given name.
>
> 1399      * @since  1.9
>
> should be @since 9.

Also this has been fixed - old habits ;-)
> I have reservation in adding ClassLoader::systemResources static method.
>
> ClassLoader::getSystemResources is a convenient method that is equivalent to calling ClassLoader::getSystemClassLoader().getResources(), since the system class loader is never null.  This method includes the resources in the application’s classpath.
>
> With the new ClassLoader::getPlatformClassLoader() method, a better way to get the “system” resources from JDK (not the ones from the classpath), it can call:
>    ClassLoader::getPlatformClassLoader().resources();
>
> To get a Stream of URL of the resources visible to system class loader, it can call the new resources method:
>    ClassLoader::getSystemClassLoader().resources();
>
> So it doesn’t seem that the proposed systemResources static method is necessary.

Hmm... We may have overlooked this, when we started by just looking into
the API changes in general. I did those with Paul Sandoz in more deep in
the
following thread:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042752.html

and this proposal is based on the "go" from Paul here:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/043012.html

> I suggest to combine the two tests in a single test, ResourcesStreamTest and add @bug JBS-id
>
> ResourcesSuccessCase
>
>   46         List<URL> resources = stream.collect(Collectors.toList());
>   52         URL url = resources.get(0);
>
> You can check the size first.  This can be simplified to use filter/findFirst to file the expected URL (yes need to call cl.resources again that is not an issue).

I have done this also in the new revision

> Mandy
> P.S. process wide - RDP1 starts.  This enhancement would need a sponsor and also request for approval [1].  I’m off for the long weekend and will follow up next Tuesday on this.
>
> [1] http://openjdk.java.net/projects/jdk9/fc-extension-process

- Patrick