On Mon, 7 Jul 2025 07:33:58 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
> Adds a new `ofFileChannel(FileChannel channel, long offset, long length)`
> method to `java.net.HttpRequest.BodyPublishers` to provide an `HttpClient`
> publisher to upload a certain region of a file. The new publisher does not
> modify the state of the passed `FileChannel`, streams the file channel bytes
> as it publishes (i.e., avoids reading the entire file into the memory), and
> can be leveraged to implement sliced uploads. As noted in the Javadoc:
>
>> 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.
>
> ### Implementation notes
>
> - `FileChannel` is preferred over `{Readable,Seekable}ByteChannel`, since the
> latter does not provide a positional read without modifying the state of the
> `FileChannel`, which is necessary to use a single `FileChannel` instance to
> implement sliced uploads.
> - `ofFileChannel(FileChannel,long,long)` is preferred over
> `ofPath(Path,long,long)` to avoid overloading the maximum file descriptor
> limit of the platform.
src/java.net.http/share/classes/java/net/http/HttpRequest.java line 739:
> 737: * @param channel a file channel
> 738: * @param offset the offset of the first byte
> 739: * @param length the number of bytes to use
I'd suggest to say something like:
> the number of bytes to send
or
> the number of bytes to read from the file channel
src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java
line 488:
> 486: ByteBuffer buffer = Utils.BUFSIZE > remaining
> 487: ? Utils.getBufferWithAtMost((int) remaining)
> 488: : Utils.getBuffer();
I suggest to change `Utils.getBufferWithAtMost` to take a long instead of an
int, and replace the ternary operator here with simply:
ByteBuffer buffer = Utils.getBufferWithAtMost(remaining);
src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java line
386:
> 384: * @throws IllegalArgumentException if {@code capacity < 0}
> 385: */
> 386: public static ByteBuffer getBufferWithAtMost(int maxCapacity) {
Suggestion:
public static ByteBuffer getBufferWithAtMost(long maxCapacity) {
We could accept `long` here.
src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java line
392:
> 390: "capacity < 0: (%s < 0)".formatted(maxCapacity));
> 391: }
> 392: int effectiveCapacity = Math.clamp(maxCapacity, 0, BUFSIZE);
Suggestion:
int effectiveCapacity = (int) Math.min(maxCapacity, (long) BUFSIZE);
Use Math.min(long,long) - since BUFZIZE is an int we can safely cast back to
int. No need to clamp at 0 since we verified maxCapacity >= 0
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2189357089
PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2189371496
PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2189386033
PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2189383920
