diff --git a/src/main/java/io/neonbee/NeonBee.java b/src/main/java/io/neonbee/NeonBee.java index 14a22dbf..22dd1306 100644 --- a/src/main/java/io/neonbee/NeonBee.java +++ b/src/main/java/io/neonbee/NeonBee.java @@ -195,7 +195,7 @@ public static Future create() { * @return the future to a new NeonBee instance initialized with default options and a new Vert.x instance */ public static Future create(NeonBeeOptions options) { - return create(() -> newVertx(options), options); + return create((OwnVertxSupplier) () -> newVertx(options), options); } @VisibleForTesting @@ -222,9 +222,16 @@ static Future create(Supplier> vertxFutureSupplier, NeonB // at this point at any failure that occurs, it is in our responsibility to properly close down the created // Vert.x instance again. we have to be vigilant the fact that a runtime exception could happen anytime! Function> closeVertx = throwable -> { + if (!(vertxFutureSupplier instanceof OwnVertxSupplier)) { + // the Vert.x instance is *not* owned by us, thus don't close it either + logger.error("Failure during bootstrap phase.", throwable); // NOPMD slf4j + return failedFuture(throwable); + } + // the instance has been created, but after initialization some post-initialization // tasks went wrong, stop Vert.x again. This will also call the close hook and clean up. logger.error("Failure during bootstrap phase. Shutting down Vert.x instance.", throwable); + // we wait for Vert.x to close, before we propagate the reason why booting failed return vertx.close().transform(closeResult -> failedFuture(throwable)); }; @@ -583,4 +590,11 @@ public void registerLocalConsumer(String verticleAddress) { public void unregisterLocalConsumer(String verticleAddress) { localConsumers.remove(verticleAddress); } + + /** + * Hidden marker supplier interface, that indicates to the boot-stage that an own Vert.x instance was created and we + * must be held responsible responsible to close it again. + */ + @VisibleForTesting + interface OwnVertxSupplier extends Supplier> {} } diff --git a/src/test/java/io/neonbee/NeonBeeExtension.java b/src/test/java/io/neonbee/NeonBeeExtension.java index 98647bb3..137b152d 100644 --- a/src/test/java/io/neonbee/NeonBeeExtension.java +++ b/src/test/java/io/neonbee/NeonBeeExtension.java @@ -45,11 +45,11 @@ public class NeonBeeExtension implements ParameterResolver, BeforeTestExecutionCallback, AfterTestExecutionCallback, BeforeEachCallback, AfterEachCallback, BeforeAllCallback, AfterAllCallback { - private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeExtension.class); + public static final int DEFAULT_TIMEOUT_DURATION = 60; - private static final int DEFAULT_TIMEOUT_DURATION = 60; + public static final TimeUnit DEFAULT_TIMEOUT_UNIT = TimeUnit.SECONDS; - private static final TimeUnit DEFAULT_TIMEOUT_UNIT = TimeUnit.SECONDS; + private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeExtension.class); private static class ContextList extends ArrayList { @@ -225,34 +225,39 @@ public T get() { } } - private NeonBee createNeonBee(NeonBeeOptions neonBeeOptions) { + private NeonBee createNeonBee(NeonBeeOptions options) { CountDownLatch latch = new CountDownLatch(1); - AtomicReference neonBeeHolder = new AtomicReference<>(); - LOGGER.info("Before NeonBee init."); - NeonBee.create(neonBeeOptions).onComplete(ar -> { - LOGGER.info("NeonBee AsyncResult Handler. {}", ar.succeeded()); + AtomicReference neonBeeBox = new AtomicReference<>(); + AtomicReference errorBox = new AtomicReference<>(); + + NeonBee.create(options).onComplete(ar -> { if (ar.succeeded()) { - neonBeeHolder.set(ar.result()); + neonBeeBox.set(ar.result()); } else { - LOGGER.error("Error while initializing NeonBee.", ar.cause()); // NOPMD slf4j + errorBox.set(ar.cause()); } + latch.countDown(); }); try { - LOGGER.info("Before CountDownLatch wait."); - if (latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) { - LOGGER.info("After CountDownLatch wait."); - NeonBee neonBee = neonBeeHolder.get(); - LOGGER.info("NeonBee Result {}.", neonBee); - if (neonBee != null) { - return neonBee; - } + if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) { + throw new VertxException(new TimeoutException("Failed to initialize NeonBee in time")); } } catch (InterruptedException e) { - LOGGER.error("NeonBee initialization failed.", e); + throw new VertxException("Got interrupted when initializing NeonBee", e); } - throw new VertxException("NeonBee initialization failed."); + + Throwable throwable = errorBox.get(); + if (throwable != null) { + if (throwable instanceof RuntimeException) { + throw (RuntimeException) throwable; + } else { + throw new VertxException("Could not create NeonBee", throwable); + } + } + + return neonBeeBox.get(); } @SuppressWarnings("FutureReturnValueIgnored") @@ -289,9 +294,11 @@ private ThrowingConsumer closeNeonBee() { } latch.countDown(); }); + if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) { throw new TimeoutException("Closing the Vertx context timed out"); } + Throwable throwable = errorBox.get(); if (throwable != null) { if (throwable instanceof Exception) { diff --git a/src/test/java/io/neonbee/NeonBeeTest.java b/src/test/java/io/neonbee/NeonBeeTest.java index 47003a28..23bfa2f5 100644 --- a/src/test/java/io/neonbee/NeonBeeTest.java +++ b/src/test/java/io/neonbee/NeonBeeTest.java @@ -1,17 +1,27 @@ package io.neonbee; import static com.google.common.truth.Truth.assertThat; +import static io.neonbee.NeonBeeMockHelper.defaultVertxMock; +import static io.neonbee.NeonBeeMockHelper.registerNeonBeeMock; import static io.neonbee.NeonBeeProfile.ALL; import static io.neonbee.NeonBeeProfile.CORE; import static io.neonbee.NeonBeeProfile.INCUBATOR; import static io.neonbee.NeonBeeProfile.STABLE; import static io.neonbee.internal.helper.StringHelper.EMPTY; import static io.neonbee.test.helper.OptionsHelper.defaultOptions; +import static io.vertx.core.Future.failedFuture; +import static io.vertx.core.Future.succeededFuture; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.lang.reflect.Method; +import java.nio.file.Files; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; +import java.util.function.Supplier; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; @@ -21,6 +31,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import io.neonbee.NeonBee.OwnVertxSupplier; import io.neonbee.config.NeonBeeConfig; import io.neonbee.internal.tracking.MessageDirection; import io.neonbee.internal.tracking.TrackingDataLoggingStrategy; @@ -28,11 +39,13 @@ import io.neonbee.test.base.NeonBeeTestBase; import io.neonbee.test.helper.WorkingDirectoryBuilder; import io.vertx.core.AbstractVerticle; +import io.vertx.core.Future; import io.vertx.core.Handler; import io.vertx.core.Vertx; import io.vertx.core.eventbus.DeliveryContext; import io.vertx.core.eventbus.EventBus; import io.vertx.core.json.JsonObject; +import io.vertx.junit5.Checkpoint; import io.vertx.junit5.Timeout; import io.vertx.junit5.VertxTestContext; @@ -126,8 +139,8 @@ void testRegisterAndUnregisterLocalConsumer() { @Test @DisplayName("Vert.x should add eventbus interceptors.") void testDecorateEventbus() throws Exception { - Vertx vertx = NeonBeeMockHelper.defaultVertxMock(); - NeonBee neonBee = NeonBeeMockHelper.registerNeonBeeMock(vertx, + Vertx vertx = defaultVertxMock(); + NeonBee neonBee = registerNeonBeeMock(vertx, new NeonBeeConfig(new JsonObject().put("trackingDataHandlingStrategy", "wrongvalue"))); EventBus eventBus = mock(EventBus.class); when(vertx.eventBus()).thenReturn(eventBus); @@ -158,6 +171,44 @@ void testFilterByProfile() { assertThat(NeonBee.filterByAutoDeployAndProfiles(SystemVerticle.class, List.of(ALL))).isFalse(); } + @Test + @Timeout(value = 10, timeUnit = TimeUnit.SECONDS) + @DisplayName("NeonBee should close only self-owned Vert.x instances if boot fails") + void testCloseVertxOnError(VertxTestContext testContext) { + Checkpoint checkpoint = testContext.checkpoint(3); + + BiConsumer check = (ownVertx, closeFails) -> { + Vertx failingVertxMock = mock(Vertx.class); + when(failingVertxMock.fileSystem()).thenThrow(new RuntimeException("Failing Vert.x!")); + when(failingVertxMock.close()).thenReturn(closeFails ? failedFuture("ANY FAILURE!!") : succeededFuture()); + + Supplier> vertxSupplier; + if (ownVertx) { + vertxSupplier = (OwnVertxSupplier) () -> succeededFuture(failingVertxMock); + } else { + vertxSupplier = () -> succeededFuture(failingVertxMock); + } + + NeonBee.create(vertxSupplier, defaultOptions()).onComplete(testContext.failing(throwable -> { + testContext.verify(() -> { + // assert hat it is always + assertThat(throwable.getMessage()).isEqualTo("Failing Vert.x!"); + verify(failingVertxMock, times(ownVertx ? 1 : 0)).close(); + checkpoint.flag(); + }); + })); + }; + + // fail the boot, but close Vert.x fine and ensure a Vert.x that is NOT owned by the outside is closed + check.accept(true, false); + + // fail the boot and assure that Vert.x is not closed for an instance that is provided from the outside + check.accept(false, false); + + // fail the boot and also the Vert.x close + check.accept(true, true); + } + @NeonBeeDeployable(profile = CORE) private static class CoreVerticle extends AbstractVerticle { // empty class (comment needed as spotless formatter works different on windows) diff --git a/src/test/java/io/neonbee/test/base/NeonBeeTestBase.java b/src/test/java/io/neonbee/test/base/NeonBeeTestBase.java index 183bce57..5fa18bf8 100644 --- a/src/test/java/io/neonbee/test/base/NeonBeeTestBase.java +++ b/src/test/java/io/neonbee/test/base/NeonBeeTestBase.java @@ -1,10 +1,14 @@ package io.neonbee.test.base; +import static io.neonbee.NeonBeeExtension.DEFAULT_TIMEOUT_DURATION; +import static io.neonbee.NeonBeeExtension.DEFAULT_TIMEOUT_UNIT; +import static io.neonbee.NeonBeeProfile.NO_WEB; +import static io.neonbee.NeonBeeProfile.WEB; import static io.neonbee.internal.helper.ConfigHelper.readConfig; import static io.neonbee.test.helper.OptionsHelper.defaultOptions; import static io.neonbee.test.helper.WorkingDirectoryBuilder.readDeploymentOptions; +import static io.neonbee.test.helper.WorkingDirectoryBuilder.writeDeploymentOptions; import static io.vertx.core.Future.failedFuture; -import static io.vertx.core.Future.succeededFuture; import java.io.IOException; import java.io.InputStream; @@ -17,6 +21,8 @@ import java.util.Optional; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.olingo.commons.api.edm.FullQualifiedName; import org.junit.jupiter.api.AfterEach; @@ -26,6 +32,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.ImmutableList; import com.google.common.io.Resources; import io.neonbee.NeonBee; @@ -46,6 +53,7 @@ import io.neonbee.test.helper.DummyVerticleHelper.DummyDataVerticleFactory; import io.neonbee.test.helper.DummyVerticleHelper.DummyEntityVerticleFactory; import io.neonbee.test.helper.FileSystemHelper; +import io.neonbee.test.helper.SystemHelper; import io.neonbee.test.helper.WorkingDirectoryBuilder; import io.neonbee.test.listeners.StaleVertxChecker; import io.vertx.core.DeploymentOptions; @@ -71,6 +79,12 @@ public class NeonBeeTestBase { private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeTestBase.class); + // create a dummy JsonObject indicating that the provideUserPrincipal method was not overridden. depending on the + // test case the provideUserPrincipal method maybe should return null in some cases, thus we need another way to + // check if we should deploy the dummy verticle. this private object can never be returned by anyone overriding the + // method, thus if any other value than this is returned, we can be sure a sub-class overrode the method + private static final JsonObject NO_USER_PRINCIPAL = new JsonObject(); + private Path workingDirPath; private NeonBee neonBee; @@ -79,6 +93,7 @@ public class NeonBeeTestBase { @BeforeEach @Timeout(value = 5, timeUnit = TimeUnit.SECONDS) + @SuppressWarnings("ReferenceEquality") public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo) throws Exception { // associate the Vert.x instance to the current test (unfortunately the only "identifier" that is shared between // TestInfo and TestIdentifier is the display name) @@ -93,6 +108,22 @@ public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo) adaptOptions(testInfo, options); options.setWorkingDirectory(workingDirPath); + // probe for a custom user principal + AtomicBoolean customUserPrincipal = new AtomicBoolean(false); + if (provideUserPrincipal(testInfo) != NO_USER_PRINCIPAL) { // do NOT use equals here! compare references! + if (!WEB.isActive(options.getActiveProfiles())) { + testContext.failNow(new IllegalStateException( + "A custom user principal can only be set, if the WEB profile is active!")); + return; + } + + // add the NO_WEB profile to the active profiles list, this way we won't have to undeploy the ServerVerticle + // again later on and can deploy our dummy ServerVerticle right away + options.setActiveProfiles(new ImmutableList.Builder().add(NO_WEB) + .addAll(options.getActiveProfiles()).build()); + customUserPrincipal.set(true); + } + URL defaultLogbackConfig = Resources.getResource(NeonBeeTestBase.class, "NeonBeeTestBase-Logback.xml"); try (InputStream is = Resources.asByteSource(defaultLogbackConfig).openStream()) { Files.copy(is, options.getConfigDirectory().resolve("logback.xml")); @@ -113,26 +144,43 @@ public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo) } else { neonBee = asyncNeonBee.result(); - DeploymentOptions serverVerticleOptions = readDeploymentOptions(ServerVerticle.class, workingDirPath); + if (customUserPrincipal.get()) { + // use a dummy ServerVerticle that returns the principal by calling the provideUserPrincipal method + DeploymentOptions serverVerticleOptions = + readDeploymentOptions(ServerVerticle.class, workingDirPath); - Optional.ofNullable(provideUserPrincipal(testInfo)).map(userPrincipal -> { - // Replace current ServerVerticle with a dummy ServerVerticle that also has a dummy AuthHandler to - // provide the user principal specified in the provideUserPrincipal method - ServerVerticle dummyServerVerticle = createDummyServerVerticle(testInfo); + try { + // just to be sure, try to fetch a new port to use + options.setServerPort(SystemHelper.getFreePort()); + } catch (IOException e) { // NOPMD + // let's continue with the old port which should be still free (hopefully) + } + ServerVerticle dummyServerVerticle = createDummyServerVerticle(testInfo); + serverVerticleOptions.getConfig().put("port", options.getServerPort()); serverVerticleOptions.getConfig().put("authenticationChain", new JsonArray()); - - isDummyServerVerticleDeployed = true; - return undeployVerticles(ServerVerticle.class) - .compose(nothing -> deployVerticle(dummyServerVerticle, serverVerticleOptions)); - }).orElse(succeededFuture()).onComplete(testContext.succeeding(v -> { - latch.countDown(); + writeDeploymentOptions(ServerVerticle.class, serverVerticleOptions, workingDirPath); + + undeployVerticles(ServerVerticle.class) // just to be sure, NO_WEB should not deploy a server + .compose(nothing -> deployVerticle(dummyServerVerticle, serverVerticleOptions)) + .onComplete(result -> { + if (result.succeeded()) { + isDummyServerVerticleDeployed = true; + } + + testContext.succeedingThenComplete().handle(result.mapEmpty()); + latch.countDown(); + }); + } else { testContext.completeNow(); - })); + latch.countDown(); + } } }); - latch.await(); + if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) { + throw new TimeoutException("Preparing NeonBee timed out"); + } } @AfterEach @@ -183,7 +231,7 @@ protected void adaptOptions(TestInfo testInfo, NeonBeeOptions.Mutable options) { */ @SuppressWarnings("PMD.UnusedFormalParameter") protected JsonObject provideUserPrincipal(TestInfo testInfo) { - return null; + return NO_USER_PRINCIPAL; } /**