diff --git a/scm-core/src/main/java/sonia/scm/lifecycle/RestartEvent.java b/scm-core/src/main/java/sonia/scm/lifecycle/RestartEvent.java index 1978cb9d7c..1b692d8081 100644 --- a/scm-core/src/main/java/sonia/scm/lifecycle/RestartEvent.java +++ b/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; } diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java b/scm-core/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java similarity index 80% rename from scm-webapp/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java rename to scm-core/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java index 53ab75ff90..062f639096 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/RestartNotSupportedException.java +++ b/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); } diff --git a/scm-core/src/main/java/sonia/scm/lifecycle/Restarter.java b/scm-core/src/main/java/sonia/scm/lifecycle/Restarter.java new file mode 100644 index 0000000000..547d26e391 --- /dev/null +++ b/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); +} diff --git a/scm-test/src/main/java/sonia/scm/lifecycle/RestartEventFactory.java b/scm-test/src/main/java/sonia/scm/lifecycle/RestartEventFactory.java new file mode 100644 index 0000000000..59d27fe4c2 --- /dev/null +++ b/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); + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/DefaultRestarter.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/DefaultRestarter.java new file mode 100644 index 0000000000..0a787d239a --- /dev/null +++ b/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)); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java b/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java index 06f6462af0..5ad623f8a4 100644 --- a/scm-webapp/src/main/java/sonia/scm/lifecycle/modules/BootstrapModule.java +++ b/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()); diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginManager.java b/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginManager.java index f57d932e1e..731b7b12d5 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginManager.java +++ b/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 pendingInstallQueue = new ArrayList<>(); private final Collection 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 pendingInstallations) { diff --git a/scm-webapp/src/main/java/sonia/scm/update/MigrationWizardServlet.java b/scm-webapp/src/main/java/sonia/scm/update/MigrationWizardServlet.java index b0ee63c0e8..b518bef698 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/MigrationWizardServlet.java +++ b/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 getRepositoryLineEntries() { diff --git a/scm-webapp/src/test/java/sonia/scm/lifecycle/DefaultRestarterTest.java b/scm-webapp/src/test/java/sonia/scm/lifecycle/DefaultRestarterTest.java new file mode 100644 index 0000000000..822b7fad64 --- /dev/null +++ b/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 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"); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/DefaultPluginManagerTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/DefaultPluginManagerTest.java index f6aa1d7301..ed575734f2 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/DefaultPluginManagerTest.java +++ b/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 diff --git a/scm-webapp/src/test/java/sonia/scm/update/MigrationWizardServletTest.java b/scm-webapp/src/test/java/sonia/scm/update/MigrationWizardServletTest.java index 683c446af7..286e8855f3 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/MigrationWizardServletTest.java +++ b/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 model, String templateName) { renderedTemplateName = templateName; diff --git a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java index f177d44e63..e060d7a2e8 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java +++ b/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(); }