On Fri, 18 Sep 2020 11:32:26 GMT, Chris Hegarty <che...@openjdk.org> wrote:
>> Daniel Fuchs has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull
>> request contains four additional commits since
>> the last revision:
>> - Merge remote-tracking branch 'origin/master' into concat-bs-8252374
>> - Merge remote-tracking branch 'origin/master' into concat-bs-8252374
>> - Merge remote-tracking branch 'origin/master' into concat-bs-8252374
>> - 8252374: Add a new factory method to concatenate a sequence of
>> BodyPublisher instances into a single publisher.
>
> src/java.net.http/share/classes/java/net/http/HttpRequest.java line 661:
>
>> 659:
>> 660: /**
>> 661: * Creates a {@code BodyPublisher} that publishes a request
>
> Trivially, should we replace "Creates" with "Returns" to allow for the
> possibility of, say, a singleton empty publisher
> (rather than having to "create" a new empty each time)
Good point!
> src/java.net.http/share/classes/java/net/http/HttpRequest.java line 668:
>
>> 666: * is returned. Otherwise each publisher is lazily subscribed
>> to in turn,
>> 667: * until all the body bytes are published, an error occurs, or
>> the
>> 668: * returned publisher's subscription is cancelled.
>
> We should probably say what the publishers `contentLength` will return.
OK.
> src/java.net.http/share/classes/java/net/http/HttpRequest.java line 680:
>
>> 678: * cancelled}, or an error occurs while publishing the bytes,
>> not all
>> 679: * publishers in the sequence may be subscribed to.
>> 680: * A publisher may be subscribed several times in sequence if
>> the
>
> I think that this sentence can be moved out of the implNote and into
> normative text
OK.
> src/java.net.http/share/classes/java/net/http/HttpRequest.java line 688:
>
>> 686: * by each publisher in the sequence.
>> 687: */
>> 688: public static BodyPublisher concat(BodyPublisher... publishers)
>> {
>
> It's a choice whether to allow an empty publishers array, or an array of just
> 1, or to disallow (throw
> IllegalArgumentException), but that will depend on expected usage.
Yes - returning `noBody()` seems more natural - since varargs allows you to
call `BodyPublishers.concat()` with no
parameters. I toyed with the idea of defining the method as `BodyPublisher
concat(BodyPublisher first, BodyPublisher...
rest)` but that's very cumbersome for the case where you already have a
`List<BodyPublishers>` or a `BodyPublisher[]`
array.
-------------
PR: https://git.openjdk.java.net/jdk/pull/57
