Context
I'm working on a major change for openfeign/feign that will allow Feign Client to stream large request bodies instead of caching them in-memory. The idea is that Request.Body will store a ThrowingConsumer<OutputStream, IOException> instead of a byte[] for body representation.
Since Feign Client is an abstraction layer around multiple low-level HTTP clients — including Vert.x — I need to bridge java.io.OutputStream to io.vertx.core.streams.ReadStream. io.cloudonix.vertx.javaio.OutputToReadStream is a perfect fit for this, enabling code like:
OutputToReadStream stream = new OutputToReadStream(vertx);
Future<HttpResponse<Buffer>> sendStreamFuture = httpClientRequest.sendStream(stream);
Future<Void> writeFuture = vertx.executeBlocking(() -> {
try (stream) {
body.writeTo(stream);
}
return null;
});
Future<HttpResponse<Buffer>> response = Future.all(sendStreamFuture, writeFuture)
.map(composite -> sendStreamFuture.result());
Problem
Feign Client supports both Vert.x 4 and Vert.x 5. When attempting to use OutputToReadStream in a Vert.x 4 environment, the following error is thrown at runtime:
java.lang.NoClassDefFoundError: io/vertx/core/Completable
The root cause is in this line:
|
pipeTo(sink).andThen(promise); |
io.vertx.core.Future#andThen(io.vertx.core.Completable<? super T>) method was introduced in Vert.x 5 and does not exist in Vert.x 4, causing a NoClassDefFoundError for io.vertx.core.Completable at class loading time.
Proposed Fix
Replacing the andThen call with an equivalent onComplete handler would restore Vert.x 4 compatibility while remaining fully compatible with Vert.x 5:
pipeTo(sink).onComplete(ar -> {
if (ar.succeeded()) {
promise.complete(ar.result());
} else {
promise.fail(ar.cause());
}
});
io.vertx.core.Future#andThen(io.vertx.core.Handler<io.vertx.core.AsyncResult<T>>) has been available since Vert.x 4 and is still present in Vert.x 5, so this change is fully backward compatible.
Impact
This single-line change would:
- Make
io.cloudonix:vertx-java.io usable in Vert.x 4 environments
- Unblock the integration of
io.cloudonix:vertx-java.io into io.github.openfeign:feign-vertx, enabling streaming request body support for the Feign Vert.x client
I understand the library now primarily targets Vert.x 5, but given how minimal and non-breaking this fix is, I believe it's worth considering. I'll happily submit a Pull Request for the same.
Context
I'm working on a major change for openfeign/feign that will allow Feign Client to stream large request bodies instead of caching them in-memory. The idea is that
Request.Bodywill store aThrowingConsumer<OutputStream, IOException>instead of abyte[]for body representation.Since Feign Client is an abstraction layer around multiple low-level HTTP clients — including Vert.x — I need to bridge
java.io.OutputStreamtoio.vertx.core.streams.ReadStream.io.cloudonix.vertx.javaio.OutputToReadStreamis a perfect fit for this, enabling code like:Problem
Feign Client supports both Vert.x 4 and Vert.x 5. When attempting to use
OutputToReadStreamin a Vert.x 4 environment, the following error is thrown at runtime:The root cause is in this line:
vertx-java.io/src/main/java/io/cloudonix/vertx/javaio/OutputToReadStream.java
Line 74 in 8fdad93
io.vertx.core.Future#andThen(io.vertx.core.Completable<? super T>)method was introduced in Vert.x 5 and does not exist in Vert.x 4, causing aNoClassDefFoundErrorforio.vertx.core.Completableat class loading time.Proposed Fix
Replacing the
andThencall with an equivalentonCompletehandler would restore Vert.x 4 compatibility while remaining fully compatible with Vert.x 5:io.vertx.core.Future#andThen(io.vertx.core.Handler<io.vertx.core.AsyncResult<T>>)has been available since Vert.x 4 and is still present in Vert.x 5, so this change is fully backward compatible.Impact
This single-line change would:
io.cloudonix:vertx-java.iousable in Vert.x 4 environmentsio.cloudonix:vertx-java.iointoio.github.openfeign:feign-vertx, enabling streaming request body support for the Feign Vert.x clientI understand the library now primarily targets Vert.x 5, but given how minimal and non-breaking this fix is, I believe it's worth considering. I'll happily submit a Pull Request for the same.