From 83b6889f8fbcd27b5f6ca026a3b9906922e8dc64 Mon Sep 17 00:00:00 2001 From: Julien Viet Date: Sat, 21 May 2016 16:41:41 +0200 Subject: [PATCH] Creating or failing a future will null should always return a failed future - fixes #1426 --- src/main/java/io/vertx/core/Future.java | 8 +++---- .../vertx/core/http/impl/HttpServerImpl.java | 3 ++- .../io/vertx/core/impl/FutureFactoryImpl.java | 10 ++++---- .../java/io/vertx/core/impl/FutureImpl.java | 24 +++++++++---------- .../io/vertx/core/net/impl/NetServerImpl.java | 3 ++- .../java/io/vertx/core/spi/FutureFactory.java | 8 +++---- .../java/io/vertx/test/core/FutureTest.java | 21 +++++++++++++++- 7 files changed, 49 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/vertx/core/Future.java b/src/main/java/io/vertx/core/Future.java index 113805fcc25..e00a2e24358 100644 --- a/src/main/java/io/vertx/core/Future.java +++ b/src/main/java/io/vertx/core/Future.java @@ -50,7 +50,7 @@ static Future future() { * @return the future */ static Future succeededFuture() { - return factory.completedFuture(); + return factory.succeededFuture(); } /** @@ -61,7 +61,7 @@ static Future succeededFuture() { * @return the future */ static Future succeededFuture(T result) { - return factory.completedFuture(result); + return factory.succeededFuture(result); } /** @@ -73,7 +73,7 @@ static Future succeededFuture(T result) { */ @GenIgnore static Future failedFuture(Throwable t) { - return factory.completedFuture(t); + return factory.failedFuture(t); } /** @@ -84,7 +84,7 @@ static Future failedFuture(Throwable t) { * @return the future */ static Future failedFuture(String failureMessage) { - return factory.completedFuture(failureMessage, true); + return factory.failureFuture(failureMessage); } /** diff --git a/src/main/java/io/vertx/core/http/impl/HttpServerImpl.java b/src/main/java/io/vertx/core/http/impl/HttpServerImpl.java index 1bb75b39a0e..54a248b82f0 100644 --- a/src/main/java/io/vertx/core/http/impl/HttpServerImpl.java +++ b/src/main/java/io/vertx/core/http/impl/HttpServerImpl.java @@ -504,7 +504,8 @@ public int actualPort() { private void executeCloseDone(final ContextImpl closeContext, final Handler> done, final Exception e) { if (done != null) { - closeContext.runOnContext((v) -> done.handle(Future.failedFuture(e))); + Future fut = e != null ? Future.failedFuture(e) : Future.succeededFuture(); + closeContext.runOnContext((v) -> done.handle(fut)); } } diff --git a/src/main/java/io/vertx/core/impl/FutureFactoryImpl.java b/src/main/java/io/vertx/core/impl/FutureFactoryImpl.java index 0f3e8caeeb6..5ce88712580 100644 --- a/src/main/java/io/vertx/core/impl/FutureFactoryImpl.java +++ b/src/main/java/io/vertx/core/impl/FutureFactoryImpl.java @@ -31,22 +31,22 @@ public Future future() { // TODO - for completed futures with null values we could maybe reuse a static instance to save allocation @Override - public Future completedFuture() { + public Future succeededFuture() { return new FutureImpl<>((T)null); } @Override - public Future completedFuture(T result) { + public Future succeededFuture(T result) { return new FutureImpl<>(result); } @Override - public Future completedFuture(Throwable t) { + public Future failedFuture(Throwable t) { return new FutureImpl<>(t); } @Override - public Future completedFuture(String failureMessage, boolean failed) { - return new FutureImpl<>(failureMessage, true); + public Future failureFuture(String failureMessage) { + return new FutureImpl<>(failureMessage); } } diff --git a/src/main/java/io/vertx/core/impl/FutureImpl.java b/src/main/java/io/vertx/core/impl/FutureImpl.java index a919626e16c..6ab34edf148 100644 --- a/src/main/java/io/vertx/core/impl/FutureImpl.java +++ b/src/main/java/io/vertx/core/impl/FutureImpl.java @@ -28,30 +28,30 @@ class FutureImpl implements Future, Handler> { private Throwable throwable; /** - * Create a FutureResult that hasn't completed yet + * Create a future that hasn't completed yet */ FutureImpl() { } /** - * Create a VoidResult that has already completed - * @param t The Throwable or null if succeeded + * Create a future that has already failed + * @param t the throwable */ FutureImpl(Throwable t) { - if (t == null) { - complete(null); - } else { - fail(t); - } + fail(t != null ? t : new NoStackTraceThrowable(null)); } - FutureImpl(String failureMessage, boolean failed) { + /** + * Create a future that has already failed + * @param failureMessage the failure message + */ + FutureImpl(String failureMessage) { this(new NoStackTraceThrowable(failureMessage)); } /** - * Create a FutureResult that has already succeeded - * @param result The result + * Create a future that has already succeeded + * @param result the result */ FutureImpl(T result) { complete(result); @@ -143,7 +143,7 @@ public void handle(AsyncResult ar) { */ public void fail(Throwable throwable) { checkComplete(); - this.throwable = throwable; + this.throwable = throwable != null ? throwable : new NoStackTraceThrowable(null); failed = true; checkCallHandler(); } diff --git a/src/main/java/io/vertx/core/net/impl/NetServerImpl.java b/src/main/java/io/vertx/core/net/impl/NetServerImpl.java index 22ebbfeebf7..8ae756de108 100644 --- a/src/main/java/io/vertx/core/net/impl/NetServerImpl.java +++ b/src/main/java/io/vertx/core/net/impl/NetServerImpl.java @@ -372,7 +372,8 @@ private void actualClose(ContextImpl closeContext, Handler> do private void executeCloseDone(ContextImpl closeContext, Handler> done, Exception e) { if (done != null) { - closeContext.runOnContext(v -> done.handle(Future.failedFuture(e))); + Future fut = e == null ? Future.succeededFuture() : Future.failedFuture(e); + closeContext.runOnContext(v -> done.handle(fut)); } } diff --git a/src/main/java/io/vertx/core/spi/FutureFactory.java b/src/main/java/io/vertx/core/spi/FutureFactory.java index 0c59bf6b218..7953c9f4b2c 100644 --- a/src/main/java/io/vertx/core/spi/FutureFactory.java +++ b/src/main/java/io/vertx/core/spi/FutureFactory.java @@ -25,11 +25,11 @@ public interface FutureFactory { Future future(); - Future completedFuture(); + Future succeededFuture(); - Future completedFuture(T result); + Future succeededFuture(T result); - Future completedFuture(Throwable t); + Future failedFuture(Throwable t); - Future completedFuture(String failureMessage, boolean failed); + Future failureFuture(String failureMessage); } diff --git a/src/test/java/io/vertx/test/core/FutureTest.java b/src/test/java/io/vertx/test/core/FutureTest.java index db6b46db9ac..ac1f43324cd 100644 --- a/src/test/java/io/vertx/test/core/FutureTest.java +++ b/src/test/java/io/vertx/test/core/FutureTest.java @@ -19,6 +19,7 @@ import io.vertx.core.CompositeFuture; import io.vertx.core.Future; import io.vertx.core.Handler; +import io.vertx.core.impl.NoStackTraceThrowable; import org.junit.Test; import java.util.ArrayList; @@ -185,6 +186,24 @@ public void testFailFutureToHandler() { assertEquals(cause, fut.cause()); } + + @Test + public void testCreateFailedWithNullFailure() { + Future future = Future.failedFuture((Throwable)null); + Checker checker = new Checker<>(future); + NoStackTraceThrowable failure = (NoStackTraceThrowable) checker.assertFailed(); + assertNull(failure.getMessage()); + } + + @Test + public void testFailurFutureWithNullFailure() { + Future future = Future.future(); + future.fail((Throwable)null); + Checker checker = new Checker<>(future); + NoStackTraceThrowable failure = (NoStackTraceThrowable) checker.assertFailed(); + assertNull(failure.getMessage()); + } + @Test public void testAllSucceeded() { testAllSucceeded(CompositeFuture::all); @@ -602,7 +621,7 @@ Throwable assertFailed() { } } -/* + /* private void assertSucceeded(Future future, T expected) { assertTrue(future.isComplete()); assertTrue(future.succeeded());