From 07be6b3fd44888493862aff51148243e4d2451ec Mon Sep 17 00:00:00 2001 From: Julien Viet Date: Wed, 15 Aug 2018 00:46:00 +0200 Subject: [PATCH] NullPointerException when reporting a deployment failure without a completion handler - fixes #2590 --- .../io/vertx/core/impl/DeploymentManager.java | 16 +++-- .../io/vertx/test/core/DeploymentTest.java | 67 ++++++++++++++----- .../vertx/test/core/VerticleFactoryTest.java | 15 +++-- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/vertx/core/impl/DeploymentManager.java b/src/main/java/io/vertx/core/impl/DeploymentManager.java index 14d7efb09..a7c2894fd 100644 --- a/src/main/java/io/vertx/core/impl/DeploymentManager.java +++ b/src/main/java/io/vertx/core/impl/DeploymentManager.java @@ -102,17 +102,23 @@ public class DeploymentManager { try { verticle = verticleSupplier.get(); } catch (Exception e) { - completionHandler.handle(Future.failedFuture(e)); + if (completionHandler != null) { + completionHandler.handle(Future.failedFuture(e)); + } return; } if (verticle == null) { - completionHandler.handle(Future.failedFuture("Supplied verticle is null")); + if (completionHandler != null) { + completionHandler.handle(Future.failedFuture("Supplied verticle is null")); + } return; } verticles.add(verticle); } if (verticles.size() != nbInstances) { - completionHandler.handle(Future.failedFuture("Same verticle supplied more than once")); + if (completionHandler != null) { + completionHandler.handle(Future.failedFuture("Same verticle supplied more than once")); + } return; } Verticle[] verticlesArray = verticles.toArray(new Verticle[verticles.size()]); @@ -176,7 +182,9 @@ public class DeploymentManager { try { deployVerticle(resolvedName, options, completionHandler); } catch (Exception e) { - completionHandler.handle(Future.failedFuture(e)); + if (completionHandler != null) { + completionHandler.handle(Future.failedFuture(e)); + } } return; } else { diff --git a/src/test/java/io/vertx/test/core/DeploymentTest.java b/src/test/java/io/vertx/test/core/DeploymentTest.java index 861d55119..866f705b6 100644 --- a/src/test/java/io/vertx/test/core/DeploymentTest.java +++ b/src/test/java/io/vertx/test/core/DeploymentTest.java @@ -31,13 +31,7 @@ import java.io.File; import java.net.URL; import java.net.URLClassLoader; import java.nio.file.Files; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -45,6 +39,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Supplier; /** * @@ -1390,34 +1385,64 @@ public class DeploymentTest extends VertxTestBase { @Test public void testDeploySupplier() throws Exception { JsonObject config = generateJSONObject(); - Set myVerticles = new HashSet<>(); - vertx.deployVerticle(() -> { + Set myVerticles = Collections.synchronizedSet(new HashSet<>()); + Supplier supplier = () -> { MyVerticle myVerticle = new MyVerticle(); myVerticles.add(myVerticle); return myVerticle; - }, new DeploymentOptions().setInstances(4).setConfig(config), onSuccess(deploymentId -> { + }; + DeploymentOptions options = new DeploymentOptions().setInstances(4).setConfig(config); + Consumer check = deploymentId -> { myVerticles.forEach(myVerticle -> { assertEquals(deploymentId, myVerticle.deploymentID); assertEquals(config, myVerticle.config); assertTrue(myVerticle.startCalled); }); + }; + + // Without completion handler + vertx.deployVerticle(supplier, options); + assertWaitUntil(() -> vertx.deploymentIDs().size() == 1); + String id = vertx.deploymentIDs().iterator().next(); + check.accept(id); + myVerticles.clear(); + vertx.undeploy(id); + assertWaitUntil(() -> vertx.deploymentIDs().size() == 0); + + // With completion handler + vertx.deployVerticle(supplier, options, onSuccess(deploymentId -> { + check.accept(deploymentId); testComplete(); })); await(); } @Test - public void testDeploySupplierNull() throws Exception { - vertx.deployVerticle(() -> null, new DeploymentOptions(), onFailure(t -> { + public void testDeploySupplierNull() { + Supplier supplier = () -> null; + DeploymentOptions options = new DeploymentOptions(); + // Without completion handler + vertx.deployVerticle(supplier, options); + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); + // With completion handler + vertx.deployVerticle(supplier, options, onFailure(t -> { + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); testComplete(); })); await(); } @Test - public void testDeploySupplierDuplicate() throws Exception { + public void testDeploySupplierDuplicate() { MyVerticle myVerticle = new MyVerticle(); - vertx.deployVerticle(() -> myVerticle, new DeploymentOptions().setInstances(2), onFailure(t -> { + Supplier supplier = () -> myVerticle; + DeploymentOptions options = new DeploymentOptions().setInstances(2); + // Without completion handler + vertx.deployVerticle(supplier, options); + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); + // With completion handler + vertx.deployVerticle(supplier, options, onFailure(t -> { + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); assertFalse(myVerticle.startCalled); testComplete(); })); @@ -1425,17 +1450,23 @@ public class DeploymentTest extends VertxTestBase { } @Test - public void testDeploySupplierThrowsException() throws Exception { - vertx.deployVerticle(() -> { + public void testDeploySupplierThrowsException() { + Supplier supplier = () -> { throw new RuntimeException("boum"); - }, new DeploymentOptions().setInstances(2), onFailure(t -> { + }; + // Without completion handler + vertx.deployVerticle(supplier, new DeploymentOptions()); + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); + // With completion handler + vertx.deployVerticle(supplier, new DeploymentOptions().setInstances(2), onFailure(t -> { + assertEquals(Collections.emptySet(), vertx.deploymentIDs()); testComplete(); })); await(); } @Test - public void testDeployClass() throws Exception { + public void testDeployClass() { JsonObject config = generateJSONObject(); vertx.deployVerticle(ReferenceSavingMyVerticle.class, new DeploymentOptions().setInstances(4).setConfig(config), onSuccess(deploymentId -> { ReferenceSavingMyVerticle.myVerticles.forEach(myVerticle -> { diff --git a/src/test/java/io/vertx/test/core/VerticleFactoryTest.java b/src/test/java/io/vertx/test/core/VerticleFactoryTest.java index cd9bd1e5e..8e4863902 100644 --- a/src/test/java/io/vertx/test/core/VerticleFactoryTest.java +++ b/src/test/java/io/vertx/test/core/VerticleFactoryTest.java @@ -268,13 +268,14 @@ public class VerticleFactoryTest extends VertxTestBase { } }; ; vertx.registerVerticleFactory(fact); - vertx.runOnContext(v -> { - vertx.deployVerticle("resolve:someid", onFailure(err -> { - // Expected since we deploy a non multi-threaded worker verticle - assertEquals(IllegalArgumentException.class, err.getClass()); - testComplete(); - })); - }); + // Without completion handler + vertx.deployVerticle("resolve:someid"); + // With completion handler + vertx.deployVerticle("resolve:someid", onFailure(err -> { + // Expected since we deploy a non multi-threaded worker verticle + assertEquals(IllegalArgumentException.class, err.getClass()); + testComplete(); + })); await(); }