From d99053cb0b890c0e3bd054379a923bcf21ce0dd8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 9 Mar 2016 15:08:46 +0100 Subject: [PATCH] SONAR-7435 replace DefaultProcessCommands constructors by static methods --- .../org/sonar/process/monitor/Monitor.java | 2 +- .../sonar/process/DefaultProcessCommands.java | 23 +++++++-- .../org/sonar/process/ProcessEntryPoint.java | 4 +- .../process/DefaultProcessCommandsTest.java | 49 +++++++++++++------ .../server/app/ProcessCommandWrapperImpl.java | 2 +- .../app/ProcessCommandWrapperImplTest.java | 4 +- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java index 9a9feeab241..6dc22a292f0 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/process/monitor/Monitor.java @@ -356,7 +356,7 @@ public class Monitor { private boolean askedForStop() { File tempDir = fileSystem.getTempDir(); - try (DefaultProcessCommands processCommands = new DefaultProcessCommands(tempDir, CURRENT_PROCESS_NUMBER, false)) { + try (DefaultProcessCommands processCommands = DefaultProcessCommands.secondary(tempDir, CURRENT_PROCESS_NUMBER)) { if (processCommands.askedForStop()) { return true; } diff --git a/server/sonar-process/src/main/java/org/sonar/process/DefaultProcessCommands.java b/server/sonar-process/src/main/java/org/sonar/process/DefaultProcessCommands.java index 6d726ee9172..a617ea4580b 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/DefaultProcessCommands.java +++ b/server/sonar-process/src/main/java/org/sonar/process/DefaultProcessCommands.java @@ -30,15 +30,28 @@ public class DefaultProcessCommands implements ProcessCommands { private final AllProcessesCommands allProcessesCommands; private final ProcessCommands delegate; - public DefaultProcessCommands(File directory, int processNumber) { - this(directory, processNumber, true); - } - - public DefaultProcessCommands(File directory, int processNumber, boolean clean) { + private DefaultProcessCommands(File directory, int processNumber, boolean clean) { this.allProcessesCommands = new AllProcessesCommands(directory); this.delegate = clean ? allProcessesCommands.createAfterClean(processNumber) : allProcessesCommands.create(processNumber); } + /** + * Main DefaultProcessCommands will clear the shared memory space of the specified process number when created and will + * then write and/or read to it. + * Therefor there should be only one main DefaultProcessCommands. + */ + public static DefaultProcessCommands main(File directory, int processNumber) { + return new DefaultProcessCommands(directory, processNumber, true); + } + + /** + * Secondary DefaultProcessCommands will read and write to the shared memory space but will not clear it. Therefor, there + * can be any number of them. + */ + public static DefaultProcessCommands secondary(File directory, int processNumber) { + return new DefaultProcessCommands(directory, processNumber, false); + } + @Override public boolean isUp() { return delegate.isUp(); diff --git a/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java b/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java index 8968210526d..84cbb22f093 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java +++ b/server/sonar-process/src/main/java/org/sonar/process/ProcessEntryPoint.java @@ -138,8 +138,8 @@ public class ProcessEntryPoint implements Stoppable { public static ProcessEntryPoint createForArguments(String[] args) { Props props = ConfigurationUtils.loadPropsFromCommandLineArgs(args); - ProcessCommands commands = new DefaultProcessCommands( - props.nonNullValueAsFile(PROPERTY_SHARED_PATH), Integer.parseInt(props.nonNullValue(PROPERTY_PROCESS_INDEX))); + ProcessCommands commands = DefaultProcessCommands.main( + props.nonNullValueAsFile(PROPERTY_SHARED_PATH), Integer.parseInt(props.nonNullValue(PROPERTY_PROCESS_INDEX))); return new ProcessEntryPoint(props, new SystemExit(), commands); } } diff --git a/server/sonar-process/src/test/java/org/sonar/process/DefaultProcessCommandsTest.java b/server/sonar-process/src/test/java/org/sonar/process/DefaultProcessCommandsTest.java index 89d07923427..4bd68e86c7c 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/DefaultProcessCommandsTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/DefaultProcessCommandsTest.java @@ -45,7 +45,7 @@ public class DefaultProcessCommandsTest { FileUtils.deleteQuietly(dir); try { - new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands.main(dir, PROCESS_NUMBER); fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("Not a valid directory: " + dir.getAbsolutePath()); @@ -56,7 +56,7 @@ public class DefaultProcessCommandsTest { public void child_process_update_the_mapped_memory() throws Exception { File dir = temp.newFolder(); - DefaultProcessCommands commands = new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands commands = DefaultProcessCommands.main(dir, PROCESS_NUMBER); assertThat(commands.isUp()).isFalse(); commands.setUp(); @@ -67,7 +67,7 @@ public class DefaultProcessCommandsTest { public void ask_for_stop() throws Exception { File dir = temp.newFolder(); - DefaultProcessCommands commands = new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands commands = DefaultProcessCommands.main(dir, PROCESS_NUMBER); assertThat(commands.askedForStop()).isFalse(); commands.askForStop(); @@ -78,7 +78,7 @@ public class DefaultProcessCommandsTest { public void ask_for_restart() throws Exception { File dir = temp.newFolder(); - DefaultProcessCommands commands = new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands commands = DefaultProcessCommands.main(dir, PROCESS_NUMBER); assertThat(commands.askedForRestart()).isFalse(); commands.askForRestart(); @@ -89,7 +89,7 @@ public class DefaultProcessCommandsTest { public void acknowledgeAskForRestart_has_no_effect_when_no_restart_asked() throws Exception { File dir = temp.newFolder(); - DefaultProcessCommands commands = new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands commands = DefaultProcessCommands.main(dir, PROCESS_NUMBER); assertThat(commands.askedForRestart()).isFalse(); commands.acknowledgeAskForRestart(); @@ -100,7 +100,7 @@ public class DefaultProcessCommandsTest { public void acknowledgeAskForRestart_resets_askForRestart_has_no_effect_when_no_restart_asked() throws Exception { File dir = temp.newFolder(); - DefaultProcessCommands commands = new DefaultProcessCommands(dir, PROCESS_NUMBER); + DefaultProcessCommands commands = DefaultProcessCommands.main(dir, PROCESS_NUMBER); commands.askForRestart(); assertThat(commands.askedForRestart()).isTrue(); @@ -110,24 +110,43 @@ public class DefaultProcessCommandsTest { } @Test - public void constructor_fails_if_processNumber_is_less_than_0() throws Exception { - File dir = temp.newFolder(); + public void main_fails_if_processNumber_is_less_than_0() throws Exception { int processNumber = -2; - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Process number " + processNumber + " is not valid"); + expectProcessNumberNoValidIAE(processNumber); - new DefaultProcessCommands(dir, processNumber); + DefaultProcessCommands.main(temp.newFolder(), processNumber); } @Test - public void getProcessCommands_fails_if_processNumber_is_higher_than_MAX_PROCESSES() throws Exception { - File dir = temp.newFolder(); + public void main_fails_if_processNumber_is_higher_than_MAX_PROCESSES() throws Exception { + int processNumber = MAX_PROCESSES + 1; + + expectProcessNumberNoValidIAE(processNumber); + + DefaultProcessCommands.main(temp.newFolder(), processNumber); + } + + @Test + public void secondary_fails_if_processNumber_is_less_than_0() throws Exception { + int processNumber = -2; + + expectProcessNumberNoValidIAE(processNumber); + + DefaultProcessCommands.secondary(temp.newFolder(), processNumber); + } + + @Test + public void secondary_fails_if_processNumber_is_higher_than_MAX_PROCESSES() throws Exception { int processNumber = MAX_PROCESSES + 1; + expectProcessNumberNoValidIAE(processNumber); + + DefaultProcessCommands.secondary(temp.newFolder(), processNumber); + } + + private void expectProcessNumberNoValidIAE(int processNumber) { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Process number " + processNumber + " is not valid"); - - new DefaultProcessCommands(dir, processNumber); } } 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 index e24efc871f4..3278c80ad5e 100644 --- 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 @@ -52,7 +52,7 @@ public class ProcessCommandWrapperImpl implements ProcessCommandWrapper { 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)) { + try (ProcessCommands commands = DefaultProcessCommands.secondary(shareDir, processNumber)) { command.callOn(commands); } catch (Exception e) { LOG.warn("Failed to close ProcessCommands", e); 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 index db49ac62632..40d6774dd31 100644 --- 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 @@ -71,7 +71,7 @@ public class ProcessCommandWrapperImplTest { ProcessCommandWrapperImpl underTest = new ProcessCommandWrapperImpl(settings); underTest.requestSQRestart(); - try (DefaultProcessCommands processCommands = new DefaultProcessCommands(tmpDir, PROCESS_NUMBER, false)) { + try (DefaultProcessCommands processCommands = DefaultProcessCommands.secondary(tmpDir, PROCESS_NUMBER)) { assertThat(processCommands.askedForRestart()).isTrue(); } } @@ -106,7 +106,7 @@ public class ProcessCommandWrapperImplTest { ProcessCommandWrapperImpl underTest = new ProcessCommandWrapperImpl(settings); underTest.notifyOperational(); - try (DefaultProcessCommands processCommands = new DefaultProcessCommands(tmpDir, PROCESS_NUMBER, false)) { + try (DefaultProcessCommands processCommands = DefaultProcessCommands.secondary(tmpDir, PROCESS_NUMBER)) { assertThat(processCommands.isOperational()).isTrue(); } } -- 2.39.5