Deprecate LightyModule#startBlocking

Deparecate practically unused LightyModule#startBlocking method which helps us to get rid of CountDownLatch in AbstractLightyModule. This method seems to be mixing concepts of using Future and CountDownLatch synchronisation tools. IMO there is no need to block start method. Additionally, there were some unusual usages in tests. They were trying to invoke a blocking call and wrap its result into Future. JIRA: LIGHTY-299 Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech> (cherry picked from commit 187f7f6)
PANTHEONtech · Oct 30, 2024 · b5da5b8 · b5da5b8
1 parent bee9058
commit b5da5b8
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 63 deletions.
diff --git a/...e/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java b/...e/lighty-controller/src/main/java/io/lighty/core/controller/api/AbstractLightyModule.java
@@ -140,7 +140,9 @@ public void startBlocking() throws InterruptedException {
      * Start and block until shutdown is requested.
      * @param initFinishCallback callback that will be called after start is completed.
      * @throws InterruptedException thrown in case module initialization fails.
+     * @deprecated Use @{@code start.get()} instead in case you want blocking start.
      */
+    @Deprecated(forRemoval = true)
     public void startBlocking(final Consumer<Boolean> initFinishCallback) throws InterruptedException {
         Futures.addCallback(start(), new FutureCallback<Boolean>() {
             @Override

diff --git a/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java b/lighty-core/lighty-controller/src/main/java/io/lighty/core/controller/api/LightyModule.java
@@ -30,7 +30,9 @@ public interface LightyModule {
      * Start and block until shutdown is requested.
      *
      * @throws InterruptedException thrown in case module initialization fails.
+     * @deprecated Use @{@code start.get()} instead in case you want blocking start.
      */
+    @Deprecated(forRemoval = true)
     void startBlocking() throws InterruptedException;
 
     /**

diff --git a/...-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java b/...-core/lighty-controller/src/test/java/io/lighty/core/controller/api/LightyModuleTest.java
@@ -7,13 +7,9 @@
  */
 package io.lighty.core.controller.api;
 
-import com.google.common.util.concurrent.SettableFuture;
 import io.lighty.core.controller.impl.LightyControllerBuilder;
 import io.lighty.core.controller.impl.util.ControllerConfigUtils;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -26,7 +22,6 @@
 public class LightyModuleTest {
     private static final long MAX_INIT_TIMEOUT = 15000L;
     private static final long MAX_SHUTDOWN_TIMEOUT = 15000L;
-    private static final long SLEEP_AFTER_SHUTDOWN_TIMEOUT = 800L;
 
     private ExecutorService executorService;
     private LightyModule moduleUnderTest;
@@ -84,62 +79,4 @@ public void testShutdown_before_start() throws Exception {
         this.moduleUnderTest.shutdown(MAX_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS);
         Mockito.verify(executorService, Mockito.times(0)).execute(Mockito.any());
     }
-
-    @Test
-    public void testStartBlocking_and_shutdown() throws Exception {
-        this.moduleUnderTest = getModuleUnderTest(getExecutorService());
-        startStopBlocking(this.moduleUnderTest instanceof AbstractLightyModule);
-    }
-
-    @Test
-    public void testStartStopBlocking() throws Exception {
-        this.moduleUnderTest = getModuleUnderTest(getExecutorService());
-        startStopBlocking(false);
-    }
-
-    private void startStopBlocking(final boolean isAbstract) throws Exception {
-        Future<Boolean> startBlockingFuture;
-        if (isAbstract) {
-            startBlockingFuture = startBlockingOnLightyModuleAbstractClass();
-        } else {
-            startBlockingFuture = startBlockingOnLightyModuleInterface();
-        }
-        //test if thread which invokes startBlocking method is still running (it should be)
-        Assert.assertFalse(startBlockingFuture.isDone());
-
-        this.moduleUnderTest.shutdown(MAX_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS);
-        try {
-            //test if thread which invokes startBlocking method is done after shutdown was called
-            //(after small timeout due to synchronization);
-            startBlockingFuture.get(SLEEP_AFTER_SHUTDOWN_TIMEOUT, TimeUnit.MILLISECONDS);
-        } catch (TimeoutException e) {
-            Assert.fail("Waiting for finish of startBlocking method thread timed out. you may consider to adjust"
-                    + "timeout by overriding SLEEP_AFTER_SHUTDOWN_TIMEOUT", e);
-        }
-
-        Mockito.verify(executorService, Mockito.times(2)).execute(Mockito.any());
-    }
-
-    private Future<Boolean> startBlockingOnLightyModuleAbstractClass() throws ExecutionException, InterruptedException {
-        SettableFuture<Boolean> initDoneFuture = SettableFuture.create();
-        Future<Boolean> startFuture = Executors.newSingleThreadExecutor().submit(() -> {
-            ((AbstractLightyModule)this.moduleUnderTest).startBlocking(initDoneFuture::set);
-            return true;
-        });
-        try {
-            initDoneFuture.get(MAX_INIT_TIMEOUT, TimeUnit.MILLISECONDS);
-        } catch (TimeoutException e) {
-            Assert.fail("Init timed out.", e);
-        }
-        return startFuture;
-    }
-
-    private Future<Boolean> startBlockingOnLightyModuleInterface() throws InterruptedException {
-        Future<Boolean> startFuture = Executors.newSingleThreadExecutor().submit(() -> {
-            this.moduleUnderTest.startBlocking();
-            return true;
-        });
-        Thread.sleep(MAX_INIT_TIMEOUT);
-        return startFuture;
-    }
 }