RFR: 8329829: HttpClient: Add a BodyPublishers.ofFileChannel method [v8]

[Volkan Yazici]

On Tue, 19 Aug 2025 06:40:18 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Volkan Yazici has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve style for declaring multiple consecutive fields of the same type
>
> The implementation and test changes look good to me.
> 
> I had some suggestions for the javadoc text. Here's what I had in mind, if Daniel and Michael suggest that the current form in this PR is OK, then it's OK to not change it:
> 
> 
> /**
>  * {@return a request body publisher whose body is the {@code length}
>  * content bytes read from the provided file {@code channel} starting
>  * from the specified {@code offset}}
>  * <p>
>  * This method does not modify the {@code channel}'s position.
>  * <p>
>  * This method does not close the {@code channel}. The caller is
>  * expected to close the {@code channel} when no longer needed.
>  *
>  * @apiNote
>  * This method can be used to either publish just a portion of a
>  * file's content as the request body or to publish different portions
>  * of the file's content concurrently.
>  * The typical approach to concurrently publish different portions of a
>  * file's content is to create an instance of {@link FileChannel}
>  * and then create multiple {@code HttpRequest}s each of which use
>  * a {@code ofFileChannel BodyPublisher} with a different non-overlapping
>  * offset and length.
>  *
>  * @param channel a file channel
>  * @param offset the offset of the first byte
>  * @param length the number of bytes to read from the file channel
>  *
>  * @throws IndexOutOfBoundsException if the specified byte range is
>  * found to be {@linkplain Objects#checkFromIndexSize(long, long, long)
>  * out of bounds} compared with the size of the file referred by the
>  * channel
>  *
>  * @throws IOException if the size of the {@code channel} cannot be
>  *                      determined or the {@code channel} is closed
>  *
>  * @throws NullPointerException if {@code channel} is null
>  *
>  * @since 26
>  */
> 
> 
> 
> (Apart from the text changes, do note that the `@throws IOException` specification in that text is intentionally changed to note that it will be thrown if the given channel is closed, in addition to if the size() cannot be determined).

@jaikiran, regarding your spec. suggestion:


          * content bytes read from the provided file {@code channel} starting
          * from the specified {@code offset}}
          * <p>
-         * The {@linkplain FileChannel file channel} will be read using
-         * {@link FileChannel#read(ByteBuffer, long) FileChannel.read(ByteBuffer buffer, long position)},
-         * which does not modify the channel's position. Thus, the same file
-         * channel may be shared between several publishers passed to
-         * concurrent requests.
+         * This method does not modify the {@code channel}'s position.
          * <p>
-         * The file channel will not be closed upon completion. The caller is
-         * expected to manage the life cycle of the channel, and close it
-         * appropriately when not needed anymore.
+         * This method does not close the {@code channel}. The caller is
+         * expected to close the {@code channel} when no longer needed.
+         *
+         * @apiNote
+         * This method can be used to either publish just a portion of a
+         * file's content as the request body or to publish different portions
+         * of the file's content concurrently.
+         * The typical approach to concurrently publish different portions of a
+         * file's content is to create an instance of {@link FileChannel}
+         * and then create multiple {@code HttpRequest}s each of which use
+         * a {@code ofFileChannel BodyPublisher} with a different non-overlapping
+         * offset and length.
          *
          * @param channel a file channel
          * @param offset the offset of the first byte
@@ -744,8 +749,8 @@
          * out of bounds} compared with the size of the file referred by the
          * channel
          *
-         * @throws IOException if the size of the file referred by the provided
-         * channel cannot be read while verifying the specified byte range
+         * @throws IOException if the size of the {@code channel} cannot be
+         *                      determined or the {@code channel} is closed
          *
          * @throws NullPointerException if {@code channel} is null
          *


1. I see you removed the explicit mention of the `FileChannel#read(ByteBuffer, long)` usage. I had placed it intentionally there, as requested by @dfuch, for `FileChannel` can have custom implementations, and we better communicate what `FC` method we use under the hood and with which assumptions.
2. Regarding the _"the size of the {@code channel}"_ expression, maybe a little bit grammar policing, but a channel doesn't have a size, instead, the file referred by the channel has a size – `FC::size` Javadoc is worded in this way too.

I will wait for the input of @dfuch and/or @Michael-Mc-Mahon, and then submit the CSR.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/26155#issuecomment-3199748167