From b8b465d7418b6128bfda4e65d529392889aa8edb Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 9 Mar 2016 14:29:17 +0100 Subject: [PATCH] SONAR-7435 add ProcessCommandWrapper to WebServer pico container wraps usage of ProcessCommand from a any bean in the WebServer --- .../server/app/ProcessCommandWrapper.java | 32 +++++ .../server/app/ProcessCommandWrapperImpl.java | 90 ++++++++++++++ .../platformlevel/PlatformLevel1.java | 2 + .../server/platform/ws/RestartAction.java | 33 +---- .../app/ProcessCommandWrapperImplTest.java | 114 ++++++++++++++++++ .../server/platform/ws/RestartActionTest.java | 46 ++----- .../server/platform/ws/SystemWsTest.java | 3 +- 7 files changed, 253 insertions(+), 67 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapper.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/app/ProcessCommandWrapperImplTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapper.java b/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapper.java new file mode 100644 index 00000000000..3310debda32 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapper.java @@ -0,0 +1,32 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.app; + +public interface ProcessCommandWrapper { + /** + * Requests to the main process that SQ be restarted. + */ + void requestSQRestart(); + + /** + * Notifies any listening process that the WebServer is operational. + */ + void notifyOperational(); +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java b/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java new file mode 100644 index 00000000000..e24efc871f4 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java @@ -0,0 +1,90 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.app; + +import java.io.File; +import org.sonar.api.config.Settings; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; +import org.sonar.process.DefaultProcessCommands; +import org.sonar.process.ProcessCommands; + +import static com.google.common.base.Preconditions.checkArgument; +import static org.sonar.process.ProcessEntryPoint.PROPERTY_PROCESS_INDEX; +import static org.sonar.process.ProcessEntryPoint.PROPERTY_SHARED_PATH; + +public class ProcessCommandWrapperImpl implements ProcessCommandWrapper { + private static final Logger LOG = Loggers.get(ProcessCommandWrapperImpl.class); + + private final Settings settings; + + public ProcessCommandWrapperImpl(Settings settings) { + this.settings = settings; + } + + @Override + public void requestSQRestart() { + call(VoidMethod.ASK_FOR_RESTART); + } + + @Override + public void notifyOperational() { + call(VoidMethod.SET_OPERATIONAL); + } + + private void call(VoidMethod command) { + File shareDir = nonNullValueAsFile(PROPERTY_SHARED_PATH); + int processNumber = nonNullAsInt(PROPERTY_PROCESS_INDEX); + try (ProcessCommands commands = new DefaultProcessCommands(shareDir, processNumber, false)) { + command.callOn(commands); + } catch (Exception e) { + LOG.warn("Failed to close ProcessCommands", e); + } + } + + private enum VoidMethod { + SET_OPERATIONAL() { + @Override + void callOn(ProcessCommands processCommands) { + processCommands.setOperational(); + } + }, + ASK_FOR_RESTART() { + @Override + void callOn(ProcessCommands processCommands) { + processCommands.askForRestart(); + } + }; + + abstract void callOn(ProcessCommands processCommands); + } + + private int nonNullAsInt(String key) { + String s = settings.getString(key); + checkArgument(s != null, "Property %s is not set", key); + return Integer.parseInt(s); + } + + public File nonNullValueAsFile(String key) { + String s = settings.getString(key); + checkArgument(s != null, "Property %s is not set", key); + return new File(s); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java index 361e35c3558..e2984c15958 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java @@ -34,6 +34,7 @@ import org.sonar.db.purge.PurgeProfiler; import org.sonar.db.semaphore.SemaphoresImpl; import org.sonar.db.version.DatabaseVersion; import org.sonar.db.version.MigrationStepModule; +import org.sonar.server.app.ProcessCommandWrapperImpl; import org.sonar.server.computation.property.CePropertyDefinitions; import org.sonar.server.db.EmbeddedDatabaseFactory; import org.sonar.server.issue.index.IssueIndex; @@ -68,6 +69,7 @@ public class PlatformLevel1 extends PlatformLevel { addExtraRootComponents(); add( new SonarQubeVersionProvider(), + ProcessCommandWrapperImpl.class, ServerSettings.class, ServerImpl.class, UuidFactoryImpl.INSTANCE, diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/RestartAction.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/RestartAction.java index 97d84b0609e..69fba19b975 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/RestartAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/RestartAction.java @@ -19,7 +19,6 @@ */ package org.sonar.server.platform.ws; -import java.io.File; import org.sonar.api.config.Settings; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; @@ -27,30 +26,27 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.web.UserRole; -import org.sonar.process.DefaultProcessCommands; -import org.sonar.process.ProcessCommands; +import org.sonar.server.app.ProcessCommandWrapper; import org.sonar.server.platform.Platform; import org.sonar.server.user.UserSession; -import static com.google.common.base.Preconditions.checkArgument; - /** * Implementation of the {@code restart} action for the System WebService. */ public class RestartAction implements SystemWsAction { private static final Logger LOGGER = Loggers.get(RestartAction.class); - private static final String PROPERTY_SHARED_PATH = "process.sharedDir"; - private static final String PROPERTY_PROCESS_INDEX = "process.index"; private final UserSession userSession; private final Settings settings; private final Platform platform; + private final ProcessCommandWrapper processCommandWrapper; - public RestartAction(UserSession userSession, Settings settings, Platform platform) { + public RestartAction(UserSession userSession, Settings settings, Platform platform, ProcessCommandWrapper processCommandWrapper) { this.userSession = userSession; this.settings = settings; this.platform = platform; + this.processCommandWrapper = processCommandWrapper; } @Override @@ -74,28 +70,9 @@ public class RestartAction implements SystemWsAction { } else { userSession.checkPermission(UserRole.ADMIN); LOGGER.info("SonarQube restart requested by {}", userSession.getLogin()); - - File shareDir = nonNullValueAsFile(PROPERTY_SHARED_PATH); - int processNumber = nonNullAsInt(PROPERTY_PROCESS_INDEX); - try (ProcessCommands commands = new DefaultProcessCommands(shareDir, processNumber, false)) { - commands.askForRestart(); - } catch (Exception e) { - LOGGER.warn("Failed to close ProcessCommands", e); - } + processCommandWrapper.requestSQRestart(); } response.noContent(); } - private int nonNullAsInt(String key) { - String s = settings.getString(key); - checkArgument(s != null, "Property %s is not set", key); - return Integer.parseInt(s); - } - - public File nonNullValueAsFile(String key) { - String s = settings.getString(key); - checkArgument(s != null, "Property %s is not set", key); - return new File(s); - } - } diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/ProcessCommandWrapperImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/ProcessCommandWrapperImplTest.java new file mode 100644 index 00000000000..db49ac62632 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/app/ProcessCommandWrapperImplTest.java @@ -0,0 +1,114 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.app; + +import java.io.File; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; +import org.sonar.api.config.Settings; +import org.sonar.process.DefaultProcessCommands; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.process.ProcessEntryPoint.PROPERTY_PROCESS_INDEX; +import static org.sonar.process.ProcessEntryPoint.PROPERTY_SHARED_PATH; + +public class ProcessCommandWrapperImplTest { + private static final int PROCESS_NUMBER = 2; + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private Settings settings = new Settings(); + + @Test + public void requestSQRestart_throws_IAE_if_process_sharedDir_property_not_set() throws Exception { + ProcessCommandWrapperImpl processCommandWrapper = new ProcessCommandWrapperImpl(settings); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Property process.sharedDir is not set"); + + processCommandWrapper.requestSQRestart(); + } + + @Test + public void requestSQRestart_throws_IAE_if_process_index_property_not_set() throws Exception { + settings.setProperty(PROPERTY_SHARED_PATH, temp.newFolder().getAbsolutePath()); + ProcessCommandWrapperImpl processCommandWrapper = new ProcessCommandWrapperImpl(settings); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Property process.index is not set"); + + processCommandWrapper.requestSQRestart(); + } + + @Test + public void requestSQRestart_updates_shareMemory_file() throws Exception { + File tmpDir = temp.newFolder().getAbsoluteFile(); + settings.setProperty(PROPERTY_SHARED_PATH, tmpDir.getAbsolutePath()); + settings.setProperty(PROPERTY_PROCESS_INDEX, PROCESS_NUMBER); + + ProcessCommandWrapperImpl underTest = new ProcessCommandWrapperImpl(settings); + underTest.requestSQRestart(); + + try (DefaultProcessCommands processCommands = new DefaultProcessCommands(tmpDir, PROCESS_NUMBER, false)) { + assertThat(processCommands.askedForRestart()).isTrue(); + } + } + + @Test + public void notifyOperational_throws_IAE_if_process_sharedDir_property_not_set() throws Exception { + ProcessCommandWrapperImpl processCommandWrapper = new ProcessCommandWrapperImpl(settings); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Property process.sharedDir is not set"); + + processCommandWrapper.notifyOperational(); + } + + @Test + public void notifyOperational_throws_IAE_if_process_index_property_not_set() throws Exception { + settings.setProperty(PROPERTY_SHARED_PATH, temp.newFolder().getAbsolutePath()); + ProcessCommandWrapperImpl processCommandWrapper = new ProcessCommandWrapperImpl(settings); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Property process.index is not set"); + + processCommandWrapper.notifyOperational(); + } + + @Test + public void notifyOperational_updates_shareMemory_file() throws Exception { + File tmpDir = temp.newFolder().getAbsoluteFile(); + settings.setProperty(PROPERTY_SHARED_PATH, tmpDir.getAbsolutePath()); + settings.setProperty(PROPERTY_PROCESS_INDEX, PROCESS_NUMBER); + + ProcessCommandWrapperImpl underTest = new ProcessCommandWrapperImpl(settings); + underTest.notifyOperational(); + + try (DefaultProcessCommands processCommands = new DefaultProcessCommands(tmpDir, PROCESS_NUMBER, false)) { + assertThat(processCommands.isOperational()).isTrue(); + } + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/RestartActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/RestartActionTest.java index 21b51b62ca3..082da463f46 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/RestartActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/RestartActionTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.platform.ws; -import java.io.File; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -28,7 +27,7 @@ import org.sonar.api.config.Settings; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.api.web.UserRole; -import org.sonar.process.DefaultProcessCommands; +import org.sonar.server.app.ProcessCommandWrapper; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.platform.Platform; import org.sonar.server.tester.UserSessionRule; @@ -49,10 +48,11 @@ public class RestartActionTest { @Rule public LogTester logTester = new LogTester(); - Settings settings = new Settings(); - Platform platform = mock(Platform.class); - RestartAction sut = new RestartAction(userSessionRule, settings, platform); - WsActionTester actionTester = new WsActionTester(sut); + private Settings settings = new Settings(); + private Platform platform = mock(Platform.class); + private ProcessCommandWrapper processCommandWrapper = mock(ProcessCommandWrapper.class); + private RestartAction sut = new RestartAction(userSessionRule, settings, platform, processCommandWrapper); + private WsActionTester actionTester = new WsActionTester(sut); @Test public void restart_if_dev_mode() throws Exception { @@ -73,40 +73,12 @@ public class RestartActionTest { } @Test - public void fail_process_sharedDir_property_not_set_in_production_mode() throws Exception { + public void calls_ProcessCommandWrapper_requestForSQRestart_in_production_mode() throws Exception { userSessionRule.login().setGlobalPermissions(UserRole.ADMIN); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Property process.sharedDir is not set"); - - actionTester.newRequest().execute(); - } - - @Test - public void fail_process_index_property_not_set_in_production_mode() throws Exception { - userSessionRule.login().setGlobalPermissions(UserRole.ADMIN); - settings.setProperty("process.sharedDir", temp.newFolder().getAbsolutePath()); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Property process.index is not set"); - - actionTester.newRequest().execute(); - } - - @Test - public void askForRestart_in_shared_memory_in_production_mode() throws Exception { - int processNumber = 2; - File tempFolder = temp.newFolder().getAbsoluteFile(); - - userSessionRule.login().setGlobalPermissions(UserRole.ADMIN); - settings.setProperty("process.sharedDir", tempFolder.getAbsolutePath()); - settings.setProperty("process.index", processNumber); - - DefaultProcessCommands processCommands = new DefaultProcessCommands(tempFolder, processNumber); - actionTester.newRequest().execute(); - assertThat(processCommands.askedForRestart()).isTrue(); + verify(processCommandWrapper).requestSQRestart(); } @Test @@ -114,8 +86,6 @@ public class RestartActionTest { String login = "BigBother"; userSessionRule.login(login).setGlobalPermissions(UserRole.ADMIN); - settings.setProperty("process.sharedDir", temp.newFolder().getAbsoluteFile().getAbsolutePath()); - settings.setProperty("process.index", 2); actionTester.newRequest().execute(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SystemWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SystemWsTest.java index ecbb768aab5..773e6d5b0a5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SystemWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/ws/SystemWsTest.java @@ -22,6 +22,7 @@ package org.sonar.server.platform.ws; import org.junit.Test; import org.sonar.api.config.Settings; import org.sonar.api.server.ws.WebService; +import org.sonar.server.app.ProcessCommandWrapper; import org.sonar.server.platform.Platform; import org.sonar.server.tester.AnonymousMockUserSession; import org.sonar.server.user.UserSession; @@ -33,7 +34,7 @@ public class SystemWsTest { @Test public void define() { - RestartAction action1 = new RestartAction(mock(UserSession.class), mock(Settings.class), mock(Platform.class)); + RestartAction action1 = new RestartAction(mock(UserSession.class), mock(Settings.class), mock(Platform.class), mock(ProcessCommandWrapper.class)); InfoAction action2 = new InfoAction(new AnonymousMockUserSession()); SystemWs ws = new SystemWs(action1, action2); WebService.Context context = new WebService.Context(); -- 2.39.5