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

Patrick Reinhart patrick at reini.net
Wed Sep 7 18:38:12 UTC 2016


The current changes can be found here:

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


On 07.09.2016 02:39, Mandy Chung wrote:
> There was already an discussion about this with Alan Bateman:
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042792.html
>>
> I sample a few existing APIs and they document UncheckedIOException in the method description rather than @throws as Alan described.
>
>> So I do not know is the best solution for this…
> Your method description already covers it.  It’s fine.  line 1386-1387 in @apiNote is redundant that I still think can be taken out.

Those lines are gone now ;-)

>>> 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 file a bug to update the spec ClassLoader::getSystemClassLoader to return non-null:
>    https://bugs.openjdk.java.net/browse/JDK-8165563
>
>>> 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
> testFailure method can be simplified to
>     long count = cl.resources(“the name”).count();
>     if (count != 1)
>         throw new Exception("expected resource is null or empty");
>
>     cl.resources("the name”)
>       .filter(url -> "file:/somefile".equals(url.toExternalForm())
>       .findFirst()
>       .orElseThrow(…)
>
> Mandy

I also made those changes as you suggested....

-Patrick





More information about the core-libs-dev mailing list