RFR: 8303260: (fc) FileChannel::transferFrom should support position > size() [v2]

[Alan Bateman]

On Wed, 15 Mar 2023 21:49:15 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Enhance `FileChannel::transferFrom` to extend the channel's file if `position > size()`. This is consistent with `FileChannel::write(ByteBuffer, long)`.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8303260: Convert test to JUnit 5

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 98:

> 96:     @MethodSource("fastParamProvider")
> 97:     void fromFast(long initialPosition, int bufSize, long offset)
> 98:         throws IOException {

Aligning the "throws IOException {" with the method body makes it hard to read, can you move this to the previous line?

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 104:

> 102:             byte[] bytes = new byte[bufSize];
> 103:             RND.nextBytes(bytes);
> 104:             src.write(ByteBuffer.wrap(bytes), 0);

Return value from write probably needs to be checked as a short write is possible in some cases.

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 115:

> 113:     @ParameterizedTest
> 114:     @MethodSource("slowParamProvider")
> 115:     void fromSlow(long initialPosition, int bufSize, long offset)

A suggestion here is to rename this to fromReadingByteChannel to make a bit clearer.

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 135:

> 133:      *
> 134:      * @throws RuntimeException if an unexpected number of bytes is transferred
> 135:      *                          or the transferred values are not as expected

This is a JUnit tests so I assume it should use the assertXXX methods rather than throwing RuntimeException.

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 144:

> 142:                                                    READ, WRITE)) {
> 143:             target.position(initialPos);
> 144:             target.write(ByteBuffer.wrap(new byte[] {(byte)42}));

I assume you should assertEquals that write return 1 here.

test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java line 149:

> 147:             long transferred = target.transferFrom(src, position, count);
> 148:             if (transferred != count)
> 149:                 throw new RuntimeException(transferred + " != " + count);

transferFrom is allowed to return a result < count. There are cases where this can happen in the JDK implementation so I think you just want to check that transferred > 0 and that compare that portion of the file, right?

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

PR: https://git.openjdk.org/jdk/pull/12797