Copilot commented on code in PR #2433:
URL: https://github.com/apache/groovy/pull/2433#discussion_r3035446410
##########
src/spec/doc/_working-with-io.adoc:
##########
@@ -88,6 +88,17 @@ makes it very easy actually:
include::../test/gdk/WorkingWithIOSpecTest.groovy[tags=file_bytes,indent=0]
----
+Groovy also provides asynchronous variants for reading and writing files via
the `groovy-nio` module.
+These return a `CompletableFuture`, making them easy to compose or use with
`async`/`await`:
Review Comment:
The documentation mentions using `CompletableFuture` with `async`/`await`,
but there doesn't appear to be corresponding Groovy language support or
referenced library in this guide. Consider rephrasing to focus on
`CompletableFuture` composition (`thenApply/thenCompose`) and blocking
(`get/join`), or explicitly reference the specific Groovy feature/library that
provides `await` semantics if intended.
```suggestion
These return a `CompletableFuture`, making them easy to compose using
methods like `thenApply` and `thenCompose`, or to wait for using `get()` or
`join()`:
```
##########
subprojects/groovy-nio/src/main/java/org/apache/groovy/nio/extensions/NioExtensions.java:
##########
@@ -2036,4 +2041,205 @@ public static <T> T withCloseable(Closeable self,
@ClosureParams(value = SimpleT
return IOGroovyMethods.withCloseable(self, action);
}
+ //
-------------------------------------------------------------------------
+ // Asynchronous file I/O extensions
+ //
-------------------------------------------------------------------------
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the platform's default charset.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally, making the I/O
+ * non-blocking. The returned {@link CompletableFuture} completes
+ * when the entire file has been read.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.textAsync.get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self) {
+ return getTextAsync(self, Charset.defaultCharset().name());
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the specified charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.getTextAsync('UTF-8').get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @param charset the charset used to decode the file content
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self, String
charset) {
+ return getBytesAsync(self).thenApply(bytes -> new String(bytes,
Charset.forName(charset)));
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a byte
array.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally. The entire file content
+ * is read into a byte array.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.bytes = [72, 105] as byte[]
+ * assert path.bytesAsync.get() == [72, 105] as byte[]
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
byte array
+ * @since 6.0.0
+ */
+ public static CompletableFuture<byte[]> getBytesAsync(Path self) {
+ CompletableFuture<byte[]> result = new CompletableFuture<>();
+ try {
+ AsynchronousFileChannel channel =
AsynchronousFileChannel.open(self, StandardOpenOption.READ);
+ long size = channel.size();
+ if (size > Integer.MAX_VALUE) {
+ channel.close();
+ result.completeExceptionally(new OutOfMemoryError("File too
large: " + size));
Review Comment:
Completing the future exceptionally with an `OutOfMemoryError` for "file too
large" is problematic because `Error` is generally reserved for JVM/runtime
failures and may be treated differently by callers/test frameworks. Prefer
completing exceptionally with a regular exception type (e.g.,
`IllegalArgumentException` or `IOException`) and include guidance about size
limits.
```suggestion
result.completeExceptionally(new IOException("File too large
to read into a single byte array: " + size + " bytes (maximum supported size is
" + Integer.MAX_VALUE + " bytes)"));
```
##########
subprojects/groovy-nio/src/main/java/org/apache/groovy/nio/extensions/NioExtensions.java:
##########
@@ -2036,4 +2041,205 @@ public static <T> T withCloseable(Closeable self,
@ClosureParams(value = SimpleT
return IOGroovyMethods.withCloseable(self, action);
}
+ //
-------------------------------------------------------------------------
+ // Asynchronous file I/O extensions
+ //
-------------------------------------------------------------------------
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the platform's default charset.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally, making the I/O
+ * non-blocking. The returned {@link CompletableFuture} completes
+ * when the entire file has been read.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.textAsync.get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self) {
+ return getTextAsync(self, Charset.defaultCharset().name());
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the specified charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.getTextAsync('UTF-8').get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @param charset the charset used to decode the file content
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self, String
charset) {
+ return getBytesAsync(self).thenApply(bytes -> new String(bytes,
Charset.forName(charset)));
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a byte
array.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally. The entire file content
+ * is read into a byte array.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.bytes = [72, 105] as byte[]
+ * assert path.bytesAsync.get() == [72, 105] as byte[]
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
byte array
+ * @since 6.0.0
+ */
+ public static CompletableFuture<byte[]> getBytesAsync(Path self) {
+ CompletableFuture<byte[]> result = new CompletableFuture<>();
+ try {
+ AsynchronousFileChannel channel =
AsynchronousFileChannel.open(self, StandardOpenOption.READ);
+ long size = channel.size();
+ if (size > Integer.MAX_VALUE) {
+ channel.close();
+ result.completeExceptionally(new OutOfMemoryError("File too
large: " + size));
+ return result;
+ }
+ ByteBuffer buf = ByteBuffer.allocate((int) size);
+ channel.read(buf, 0, null, new CompletionHandler<Integer, Void>() {
+ @Override
+ public void completed(Integer bytesRead, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ return;
+ }
+ buf.flip();
+ byte[] bytes = new byte[buf.remaining()];
+ buf.get(bytes);
+ result.complete(bytes);
+ }
+
+ @Override
+ public void failed(Throwable exc, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException ignored) {
+ }
+ result.completeExceptionally(exc);
+ }
+ });
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ }
+ return result;
+ }
+
+ /**
+ * Asynchronously write the String content to the Path using the
+ * platform's default charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.writeAsync('Hello async').get()
+ * assert path.text == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file to write to
+ * @param text the text to write
+ * @return a CompletableFuture that completes when the write is finished
+ * @since 6.0.0
+ */
+ public static CompletableFuture<Void> writeAsync(Path self, String text) {
+ return writeAsync(self, text, Charset.defaultCharset().name());
+ }
+
+ /**
+ * Asynchronously write the String content to the Path using the
+ * specified charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.writeAsync('Hello async', 'UTF-8').get()
+ * assert path.text == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file to write to
+ * @param text the text to write
+ * @param charset the charset used to encode the text
+ * @return a CompletableFuture that completes when the write is finished
+ * @since 6.0.0
+ */
+ public static CompletableFuture<Void> writeAsync(Path self, String text,
String charset) {
+ return writeBytesAsync(self, text.getBytes(Charset.forName(charset)));
+ }
+
+ /**
+ * Asynchronously write the byte array to the Path, creating the file
+ * if it doesn't exist and truncating it if it does.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.writeBytesAsync([72, 105] as byte[]).get()
+ * assert path.bytes == [72, 105] as byte[]
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file to write to
+ * @param bytes the byte array to write
+ * @return a CompletableFuture that completes when the write is finished
+ * @since 6.0.0
+ */
+ public static CompletableFuture<Void> writeBytesAsync(Path self, byte[]
bytes) {
+ CompletableFuture<Void> result = new CompletableFuture<>();
+ try {
+ AsynchronousFileChannel channel =
AsynchronousFileChannel.open(self,
+ StandardOpenOption.WRITE, StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
+ ByteBuffer buf = ByteBuffer.wrap(bytes);
+ channel.write(buf, 0, null, new CompletionHandler<Integer, Void>()
{
+ @Override
+ public void completed(Integer bytesWritten, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ return;
+ }
+ result.complete(null);
+ }
+
+ @Override
+ public void failed(Throwable exc, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException ignored) {
+ }
+ result.completeExceptionally(exc);
+ }
+ });
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ }
+ return result;
+ }
+
Review Comment:
`AsynchronousFileChannel.write` may write fewer bytes than requested. This
implementation completes after the first callback regardless of whether the
`ByteBuffer` still has remaining bytes, which can lead to partially-written
files. Consider continuing the write from the next position until
`buf.hasRemaining()` is false, then close the channel and complete the future.
```suggestion
writeBytesAsync(channel, buf, 0, result);
} catch (IOException e) {
result.completeExceptionally(e);
}
return result;
}
private static void writeBytesAsync(AsynchronousFileChannel channel,
ByteBuffer buf, long position, CompletableFuture<Void> result) {
channel.write(buf, position, null, new CompletionHandler<Integer,
Void>() {
@Override
public void completed(Integer bytesWritten, Void attachment) {
if (buf.hasRemaining()) {
writeBytesAsync(channel, buf, position + bytesWritten,
result);
return;
}
try {
channel.close();
} catch (IOException e) {
result.completeExceptionally(e);
return;
}
result.complete(null);
}
@Override
public void failed(Throwable exc, Void attachment) {
try {
channel.close();
} catch (IOException ignored) {
}
result.completeExceptionally(exc);
}
});
}
```
##########
subprojects/groovy-nio/src/main/java/org/apache/groovy/nio/extensions/NioExtensions.java:
##########
@@ -2036,4 +2041,205 @@ public static <T> T withCloseable(Closeable self,
@ClosureParams(value = SimpleT
return IOGroovyMethods.withCloseable(self, action);
}
+ //
-------------------------------------------------------------------------
+ // Asynchronous file I/O extensions
+ //
-------------------------------------------------------------------------
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the platform's default charset.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally, making the I/O
+ * non-blocking. The returned {@link CompletableFuture} completes
+ * when the entire file has been read.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.textAsync.get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self) {
+ return getTextAsync(self, Charset.defaultCharset().name());
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the specified charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.getTextAsync('UTF-8').get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @param charset the charset used to decode the file content
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self, String
charset) {
+ return getBytesAsync(self).thenApply(bytes -> new String(bytes,
Charset.forName(charset)));
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a byte
array.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally. The entire file content
+ * is read into a byte array.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.bytes = [72, 105] as byte[]
+ * assert path.bytesAsync.get() == [72, 105] as byte[]
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
byte array
+ * @since 6.0.0
+ */
+ public static CompletableFuture<byte[]> getBytesAsync(Path self) {
+ CompletableFuture<byte[]> result = new CompletableFuture<>();
+ try {
+ AsynchronousFileChannel channel =
AsynchronousFileChannel.open(self, StandardOpenOption.READ);
+ long size = channel.size();
+ if (size > Integer.MAX_VALUE) {
+ channel.close();
+ result.completeExceptionally(new OutOfMemoryError("File too
large: " + size));
+ return result;
+ }
+ ByteBuffer buf = ByteBuffer.allocate((int) size);
+ channel.read(buf, 0, null, new CompletionHandler<Integer, Void>() {
+ @Override
+ public void completed(Integer bytesRead, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ return;
+ }
+ buf.flip();
+ byte[] bytes = new byte[buf.remaining()];
+ buf.get(bytes);
+ result.complete(bytes);
Review Comment:
`AsynchronousFileChannel.read` is not guaranteed to fill the buffer in a
single operation. This implementation issues only one read and then completes
the future, which can return truncated data for larger files or under load.
Consider looping (issue another `read` from the next position until EOF/buffer
full) and only completing/closing once all bytes are read (handling `bytesRead
== -1` as EOF).
##########
subprojects/groovy-nio/src/main/java/org/apache/groovy/nio/extensions/NioExtensions.java:
##########
@@ -2036,4 +2041,205 @@ public static <T> T withCloseable(Closeable self,
@ClosureParams(value = SimpleT
return IOGroovyMethods.withCloseable(self, action);
}
+ //
-------------------------------------------------------------------------
+ // Asynchronous file I/O extensions
+ //
-------------------------------------------------------------------------
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the platform's default charset.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally, making the I/O
+ * non-blocking. The returned {@link CompletableFuture} completes
+ * when the entire file has been read.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.textAsync.get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self) {
+ return getTextAsync(self, Charset.defaultCharset().name());
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a String
+ * using the specified charset.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.text = 'Hello async'
+ * assert path.getTextAsync('UTF-8').get() == 'Hello async'
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @param charset the charset used to decode the file content
+ * @return a CompletableFuture that completes with the file content as a
String
+ * @since 6.0.0
+ */
+ public static CompletableFuture<String> getTextAsync(Path self, String
charset) {
+ return getBytesAsync(self).thenApply(bytes -> new String(bytes,
Charset.forName(charset)));
+ }
+
+ /**
+ * Asynchronously read the content of the Path and return it as a byte
array.
+ * <p>
+ * Uses {@link AsynchronousFileChannel} internally. The entire file content
+ * is read into a byte array.
+ *
+ * <pre class="groovyTestCase">
+ * import java.nio.file.Files
+ * def path = Files.createTempFile('test', '.txt')
+ * path.bytes = [72, 105] as byte[]
+ * assert path.bytesAsync.get() == [72, 105] as byte[]
+ * Files.delete(path)
+ * </pre>
+ *
+ * @param self the file whose content we want to read
+ * @return a CompletableFuture that completes with the file content as a
byte array
+ * @since 6.0.0
+ */
+ public static CompletableFuture<byte[]> getBytesAsync(Path self) {
+ CompletableFuture<byte[]> result = new CompletableFuture<>();
+ try {
+ AsynchronousFileChannel channel =
AsynchronousFileChannel.open(self, StandardOpenOption.READ);
+ long size = channel.size();
+ if (size > Integer.MAX_VALUE) {
+ channel.close();
+ result.completeExceptionally(new OutOfMemoryError("File too
large: " + size));
+ return result;
+ }
+ ByteBuffer buf = ByteBuffer.allocate((int) size);
+ channel.read(buf, 0, null, new CompletionHandler<Integer, Void>() {
+ @Override
+ public void completed(Integer bytesRead, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException e) {
+ result.completeExceptionally(e);
+ return;
+ }
+ buf.flip();
+ byte[] bytes = new byte[buf.remaining()];
+ buf.get(bytes);
+ result.complete(bytes);
+ }
+
+ @Override
+ public void failed(Throwable exc, Void attachment) {
+ try {
+ channel.close();
+ } catch (IOException ignored) {
+ }
+ result.completeExceptionally(exc);
+ }
+ });
Review Comment:
If `channel.size()` throws an `IOException`, the catch block completes the
future exceptionally but the already-opened `AsynchronousFileChannel` is never
closed, leaking a file descriptor. Consider closing the channel in a
`catch`/`finally` after open succeeds (or structuring the code so any failure
before the completion handler is installed closes the channel).
```suggestion
boolean readStarted = false;
try {
long size = channel.size();
if (size > Integer.MAX_VALUE) {
channel.close();
result.completeExceptionally(new OutOfMemoryError("File
too large: " + size));
return result;
}
ByteBuffer buf = ByteBuffer.allocate((int) size);
channel.read(buf, 0, null, new CompletionHandler<Integer,
Void>() {
@Override
public void completed(Integer bytesRead, Void
attachment) {
try {
channel.close();
} catch (IOException e) {
result.completeExceptionally(e);
return;
}
buf.flip();
byte[] bytes = new byte[buf.remaining()];
buf.get(bytes);
result.complete(bytes);
}
@Override
public void failed(Throwable exc, Void attachment) {
try {
channel.close();
} catch (IOException ignored) {
}
result.completeExceptionally(exc);
}
});
readStarted = true;
} catch (IOException | RuntimeException e) {
if (!readStarted) {
try {
channel.close();
} catch (IOException ignored) {
}
}
result.completeExceptionally(e);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
