implemented restarter to move control over the restart process to the core

scm-core/src/main/java/sonia/scm/lifecycle/RestartEvent.java

@@ -36,63 +36,37 @@ package sonia.scm.lifecycle;
 import sonia.scm.event.Event;
 
 /**
- * This event can be used to force a restart of the webapp context. The restart
- * event is useful during plugin development, because we don't have to restart
- * the whole server, to see our changes. The restart event could also be used
- * to install or upgrade plugins.
- *
- * But the restart event should be used carefully, because the whole context
- * will be restarted and that process could take some time.
- *
+ * This event indicates a forced restart of scm-manager.
  * @author Sebastian Sdorra
  * @since 2.0.0
  */
 @Event
-public class RestartEvent
-{
+public class RestartEvent {
 
-  /**
-   * Constructs ...
-   *
-   *
-   * @param cause
-   * @param reason
-   */
-  public RestartEvent(Class<?> cause, String reason)
-  {
+  private final Class<?> cause;
+  private final String reason;
+
+  RestartEvent(Class<?> cause, String reason) {
     this.cause = cause;
     this.reason = reason;
   }
 
-  //~--- get methods ----------------------------------------------------------
-
   /**
    * The class which has fired the restart event.
    *
-   *
    * @return class which has fired the restart event
    */
-  public Class<?> getCause()
-  {
+  public Class<?> getCause() {
     return cause;
   }
 
   /**
    * Returns the reason for the restart.
    *
-   *
    * @return reason for restart
    */
-  public String getReason()
-  {
+  public String getReason() {
     return reason;
   }
 
-  //~--- fields ---------------------------------------------------------------
-
-  /** cause of restart */
-  private final Class<?> cause;
-
-  /** reason for restart */
-  private final String reason;
 }

scm-core/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java

@@ -4,6 +4,10 @@ package sonia.scm.lifecycle;
  * Exception is thrown if a restart is not supported or a restart strategy is misconfigured.
  */
 public class RestartNotSupportedException extends RuntimeException {
+  RestartNotSupportedException(String message) {
+    super(message);
+  }
+
   RestartNotSupportedException(String message, Throwable cause) {
     super(message, cause);
   }

scm-core/src/main/java/sonia/scm/lifecycle/Restarter.java

@@ -0,0 +1,25 @@
+package sonia.scm.lifecycle;
+
+/**
+ * {@link Restarter} is able to restart scm-manager.
+ *
+ * @since 2.0.0
+ */
+public interface Restarter {
+
+  /**
+   * Return {@code true} if restarting scm-manager is supported.
+   *
+   * @return {@code true} if restart is supported
+   */
+  boolean isSupported();
+
+  /**
+   * Issues a restart. The method will fire a {@link RestartEvent} to notify the system about the upcoming restart.
+   * If restarting is not supported by the underlying platform a {@link RestartNotSupportedException} is thrown.
+   *
+   * @param cause cause of the restart. This should be the class which calls this method.
+   * @param reason reason for the required restart.
+   */
+  void restart(Class<?> cause, String reason);
+}

scm-test/src/main/java/sonia/scm/lifecycle/RestartEventFactory.java

@@ -0,0 +1,15 @@
+package sonia.scm.lifecycle;
+
+/**
+ * Creates restart events for testing.
+ * This is required, because the constructor of {@link RestartEvent} is package private.
+ */
+public final class RestartEventFactory {
+
+  private RestartEventFactory(){}
+
+  public static RestartEvent create(Class<?> cause, String reason) {
+    return new RestartEvent(cause, reason);
+  }
+
+}

scm-webapp/src/main/java/sonia/scm/lifecycle/DefaultRestarter.java

@@ -0,0 +1,41 @@
+package sonia.scm.lifecycle;
+
+import com.google.common.annotations.VisibleForTesting;
+import sonia.scm.event.ScmEventBus;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+@Singleton
+public class DefaultRestarter implements Restarter {
+
+  private ScmEventBus eventBus;
+  private RestartStrategy strategy;
+
+  @Inject
+  public DefaultRestarter() {
+    this(
+      ScmEventBus.getInstance(),
+      RestartStrategy.get(Thread.currentThread().getContextClassLoader()).orElse(null)
+    );
+  }
+
+  @VisibleForTesting
+  DefaultRestarter(ScmEventBus eventBus, RestartStrategy strategy) {
+    this.eventBus = eventBus;
+    this.strategy = strategy;
+  }
+
+  @Override
+  public boolean isSupported() {
+    return strategy != null;
+  }
+
+  @Override
+  public void restart(Class<?> cause, String reason) {
+    if (!isSupported()) {
+      throw new RestartNotSupportedException("restarting is not supported");
+    }
+    eventBus.post(new RestartEvent(cause, reason));
+  }
+}

scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java

@@ -9,6 +9,8 @@ import sonia.scm.SCMContext;
 import sonia.scm.SCMContextProvider;
 import sonia.scm.io.DefaultFileSystem;
 import sonia.scm.io.FileSystem;
+import sonia.scm.lifecycle.DefaultRestarter;
+import sonia.scm.lifecycle.Restarter;
 import sonia.scm.plugin.PluginLoader;
 import sonia.scm.repository.RepositoryLocationResolver;
 import sonia.scm.repository.xml.MetadataStore;
@@ -61,6 +63,8 @@ public class BootstrapModule extends AbstractModule {
 
     bind(FileSystem.class, DefaultFileSystem.class);
 
+    bind(Restarter.class, DefaultRestarter.class);
+
     // note CipherUtil uses an other generator
     bind(CipherHandler.class).toInstance(CipherUtil.getInstance().getCipherHandler());
 

scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginManager.java

@@ -41,6 +41,7 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.NotFoundException;
 import sonia.scm.event.ScmEventBus;
 import sonia.scm.lifecycle.RestartEvent;
+import sonia.scm.lifecycle.Restarter;
 import sonia.scm.version.Version;
 
 import javax.inject.Inject;
@@ -68,20 +69,21 @@ public class DefaultPluginManager implements PluginManager {
 
   private static final Logger LOG = LoggerFactory.getLogger(DefaultPluginManager.class);
 
-  private final ScmEventBus eventBus;
   private final PluginLoader loader;
   private final PluginCenter center;
   private final PluginInstaller installer;
+  private final Restarter restarter;
+
   private final Collection<PendingPluginInstallation> pendingInstallQueue = new ArrayList<>();
   private final Collection<PendingPluginUninstallation> pendingUninstallQueue = new ArrayList<>();
   private final PluginDependencyTracker dependencyTracker = new PluginDependencyTracker();
 
   @Inject
-  public DefaultPluginManager(ScmEventBus eventBus, PluginLoader loader, PluginCenter center, PluginInstaller installer) {
-    this.eventBus = eventBus;
+  public DefaultPluginManager(PluginLoader loader, PluginCenter center, PluginInstaller installer, Restarter restarter) {
     this.loader = loader;
     this.center = center;
     this.installer = installer;
+    this.restarter = restarter;
 
     this.computeInstallationDependencies();
   }
@@ -242,16 +244,8 @@ public class DefaultPluginManager implements PluginManager {
     }
   }
 
-  @VisibleForTesting
-  void triggerRestart(String cause) {
-    new Thread(() -> {
-      try {
-        Thread.sleep(200);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-      }
-      eventBus.post(new RestartEvent(PluginManager.class, cause));
-    }).start();
+  private void triggerRestart(String cause) {
+    restarter.restart(PluginManager.class, cause);
   }
 
   private void cancelPending(List<PendingPluginInstallation> pendingInstallations) {

scm-webapp/src/main/java/sonia/scm/update/MigrationWizardServlet.java

@@ -12,6 +12,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.event.ScmEventBus;
 import sonia.scm.lifecycle.RestartEvent;
+import sonia.scm.lifecycle.Restarter;
 import sonia.scm.update.repository.DefaultMigrationStrategyDAO;
 import sonia.scm.update.repository.MigrationStrategy;
 import sonia.scm.update.repository.V1Repository;
@@ -41,11 +42,13 @@ class MigrationWizardServlet extends HttpServlet {
 
   private final XmlRepositoryV1UpdateStep repositoryV1UpdateStep;
   private final DefaultMigrationStrategyDAO migrationStrategyDao;
+  private final Restarter restarter;
 
   @Inject
-  MigrationWizardServlet(XmlRepositoryV1UpdateStep repositoryV1UpdateStep, DefaultMigrationStrategyDAO migrationStrategyDao) {
+  MigrationWizardServlet(XmlRepositoryV1UpdateStep repositoryV1UpdateStep, DefaultMigrationStrategyDAO migrationStrategyDao, Restarter restarter) {
     this.repositoryV1UpdateStep = repositoryV1UpdateStep;
     this.migrationStrategyDao = migrationStrategyDao;
+    this.restarter = restarter;
   }
 
   @Override
@@ -121,7 +124,12 @@ class MigrationWizardServlet extends HttpServlet {
 
     ThreadContext.bind(new Subject.Builder(new DefaultSecurityManager()).authenticated(false).buildSubject());
 
-    ScmEventBus.getInstance().post(new RestartEvent(MigrationWizardServlet.class, "wrote migration data"));
+    if (restarter.isSupported()) {
+      restarter.restart(MigrationWizardServlet.class, "wrote migration data");
+    } else {
+      LOG.error("Restarting is not supported on this platform.");
+      LOG.error("Please do a manual restart");
+    }
   }
 
   private List<RepositoryLineEntry> getRepositoryLineEntries() {

scm-webapp/src/test/java/sonia/scm/lifecycle/DefaultRestarterTest.java

@@ -0,0 +1,69 @@
+package sonia.scm.lifecycle;
+
+import com.github.legman.Subscribe;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.event.ScmEventBus;
+
+import javax.swing.*;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.verify;
+
+@ExtendWith(MockitoExtension.class)
+class DefaultRestarterTest {
+
+  @Mock
+  private ScmEventBus eventBus;
+
+  @Captor
+  private ArgumentCaptor<RestartEvent> eventCaptor;
+
+  @Test
+  void shouldLoadStrategyOnCreation() {
+    System.setProperty(RestartStrategyFactory.PROPERTY_STRATEGY, ExitRestartStrategy.NAME);
+    try {
+      DefaultRestarter restarter = new DefaultRestarter();
+      assertThat(restarter.isSupported()).isTrue();
+    } finally {
+      System.clearProperty(RestartStrategyFactory.PROPERTY_STRATEGY);
+    }
+  }
+
+  @Test
+  void shouldReturnFalseIfRestartStrategyIsNotAvailable() {
+    DefaultRestarter restarter = new DefaultRestarter(eventBus, null);
+    assertThat(restarter.isSupported()).isFalse();
+  }
+
+  @Test
+  void shouldReturnTrueIfRestartStrategyIsAvailable() {
+    DefaultRestarter restarter = new DefaultRestarter();
+    assertThat(restarter.isSupported()).isTrue();
+  }
+
+  @Test
+  void shouldThrowRestartNotSupportedException() {
+    DefaultRestarter restarter = new DefaultRestarter(eventBus,null);
+    assertThrows(
+      RestartNotSupportedException.class, () -> restarter.restart(DefaultRestarterTest.class, "test")
+    );
+  }
+
+  @Test
+  void shouldFireRestartEvent() {
+    DefaultRestarter restarter = new DefaultRestarter(eventBus, new ExitRestartStrategy());
+    restarter.restart(DefaultRestarterTest.class, "testing");
+
+    verify(eventBus).post(eventCaptor.capture());
+
+    RestartEvent event = eventCaptor.getValue();
+    assertThat(event.getCause()).isEqualTo(DefaultRestarterTest.class);
+    assertThat(event.getReason()).isEqualTo("testing");
+  }
+}

scm-webapp/src/test/java/sonia/scm/plugin/DefaultPluginManagerTest.java

@@ -12,10 +12,12 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junitpioneer.jupiter.TempDirectory;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.NotFoundException;
 import sonia.scm.ScmConstraintViolationException;
+import sonia.scm.lifecycle.Restarter;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -54,13 +56,15 @@ class DefaultPluginManagerTest {
   @Mock
   private PluginInstaller installer;
 
+  @Mock
+  private Restarter restarter;
+
+  @InjectMocks
   private DefaultPluginManager manager;
 
   @Mock
   private Subject subject;
 
-  private boolean restartTriggered = false;
-
   @BeforeEach
   void mockInstaller() {
     lenient().when(installer.install(any())).then(ic -> {
@@ -69,16 +73,6 @@ class DefaultPluginManagerTest {
     });
   }
 
-  @BeforeEach
-  void createPluginManagerToTestWithCapturedRestart() {
-    manager = new DefaultPluginManager(null, loader, center, installer) { // event bus is only used in restart and this is replaced here
-      @Override
-      void triggerRestart(String cause) {
-        restartTriggered = true;
-      }
-    };
-  }
-
   @Nested
   class WithAdminPermissions {
 
@@ -185,7 +179,7 @@ class DefaultPluginManagerTest {
       manager.install("scm-git-plugin", false);
 
       verify(installer).install(git);
-      assertThat(restartTriggered).isFalse();
+      verify(restarter, never()).restart(any(), any());
     }
 
     @Test
@@ -263,7 +257,7 @@ class DefaultPluginManagerTest {
       manager.install("scm-git-plugin", true);
 
       verify(installer).install(git);
-      assertThat(restartTriggered).isTrue();
+      verify(restarter).restart(any(), any());
     }
 
     @Test
@@ -272,7 +266,7 @@ class DefaultPluginManagerTest {
       when(loader.getInstalledPlugins()).thenReturn(ImmutableList.of(gitInstalled));
 
       manager.install("scm-git-plugin", true);
-      assertThat(restartTriggered).isFalse();
+      verify(restarter, never()).restart(any(), any());
     }
 
     @Test
@@ -294,14 +288,14 @@ class DefaultPluginManagerTest {
       manager.install("scm-review-plugin", false);
       manager.executePendingAndRestart();
 
-      assertThat(restartTriggered).isTrue();
+      verify(restarter).restart(any(), any());
     }
 
     @Test
     void shouldNotSendRestartEventWithoutPendingPlugins() {
       manager.executePendingAndRestart();
 
-      assertThat(restartTriggered).isFalse();
+      verify(restarter, never()).restart(any(), any());
     }
 
     @Test
@@ -452,7 +446,7 @@ class DefaultPluginManagerTest {
 
       manager.executePendingAndRestart();
 
-      assertThat(restartTriggered).isTrue();
+      verify(restarter).restart(any(), any());
     }
 
     @Test

scm-webapp/src/test/java/sonia/scm/update/MigrationWizardServletTest.java

@@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
+import sonia.scm.lifecycle.Restarter;
 import sonia.scm.update.repository.DefaultMigrationStrategyDAO;
 import sonia.scm.update.repository.MigrationStrategy;
 import sonia.scm.update.repository.V1Repository;
@@ -27,6 +28,8 @@ class MigrationWizardServletTest {
   XmlRepositoryV1UpdateStep updateStep;
   @Mock
   DefaultMigrationStrategyDAO migrationStrategyDao;
+  @Mock
+  Restarter restarter;
 
   @Mock
   HttpServletRequest request;
@@ -40,7 +43,7 @@ class MigrationWizardServletTest {
 
   @BeforeEach
   void initServlet() {
-    servlet = new MigrationWizardServlet(updateStep, migrationStrategyDao) {
+    servlet = new MigrationWizardServlet(updateStep, migrationStrategyDao, restarter) {
       @Override
       void respondWithTemplate(HttpServletResponse resp, Map<String, Object> model, String templateName) {
         renderedTemplateName = templateName;

scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java

@@ -20,6 +20,7 @@ import sonia.scm.lifecycle.RestartEvent;
 import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.event.ScmEventBus;
+import sonia.scm.lifecycle.RestartEventFactory;
 import sonia.scm.plugin.PluginLoader;
 
 import javax.servlet.http.HttpServletRequest;
@@ -114,7 +115,7 @@ public class I18nServletTest {
   public void shouldCleanCacheOnRestartEvent() {
     ScmEventBus.getInstance().register(servlet);
 
-    ScmEventBus.getInstance().post(new RestartEvent(I18nServlet.class, "Restart to reload the plugin resources"));
+    ScmEventBus.getInstance().post(RestartEventFactory.create(I18nServlet.class, "Restart to reload the plugin resources"));
 
     verify(cache).clear();
   }