From: Sébastien Lesaint Date: Mon, 11 Sep 2017 12:19:23 +0000 (+0200) Subject: SONAR-9773 add AbstractCommand#suppressEnvVariable X-Git-Tag: 6.6-RC1~286 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=29206c1d0c22b77881da32ae38aeac504e5b2d4c;p=sonarqube.git SONAR-9773 add AbstractCommand#suppressEnvVariable which allows to remove from a command an env variable defined for current process and remove support for null value in AbstractCommand#setEnvVariable --- diff --git a/server/sonar-main/src/main/java/org/sonar/application/process/ProcessLauncherImpl.java b/server/sonar-main/src/main/java/org/sonar/application/process/ProcessLauncherImpl.java index 17547f420bf..b0b968f05eb 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/process/ProcessLauncherImpl.java +++ b/server/sonar-main/src/main/java/org/sonar/application/process/ProcessLauncherImpl.java @@ -158,7 +158,9 @@ public class ProcessLauncherImpl implements ProcessLauncher { ProcessBuilder processBuilder = processBuilderSupplier.get(); processBuilder.command(commands); processBuilder.directory(javaCommand.getWorkDir()); - processBuilder.environment().putAll(javaCommand.getEnvVariables()); + Map environment = processBuilder.environment(); + environment.putAll(javaCommand.getEnvVariables()); + javaCommand.getSuppressedEnvVariables().forEach(environment::remove); processBuilder.redirectErrorStream(true); return processBuilder; } diff --git a/server/sonar-process/src/main/java/org/sonar/process/System2.java b/server/sonar-process/src/main/java/org/sonar/process/System2.java new file mode 100644 index 00000000000..1c9ca5c51c6 --- /dev/null +++ b/server/sonar-process/src/main/java/org/sonar/process/System2.java @@ -0,0 +1,49 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info 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.process; + +import java.util.Map; + +/** + * An interface allowing to wrap around static call to {@link System} class. + */ +public interface System2 { + System2 INSTANCE = new System2() { + @Override + public Map getenv() { + return System.getenv(); + } + + @Override + public String getenv(String name) { + return System.getenv(name); + } + }; + + /** + * Proxy to {@link System#getenv()} + */ + Map getenv(); + + /** + * Proxy to {@link System#getenv(String)}. + */ + String getenv(String name); +} diff --git a/server/sonar-process/src/main/java/org/sonar/process/command/AbstractCommand.java b/server/sonar-process/src/main/java/org/sonar/process/command/AbstractCommand.java index 3a880ea6a50..d968b8adc74 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/command/AbstractCommand.java +++ b/server/sonar-process/src/main/java/org/sonar/process/command/AbstractCommand.java @@ -21,11 +21,14 @@ package org.sonar.process.command; import java.io.File; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Properties; +import java.util.Set; import javax.annotation.Nullable; import org.sonar.process.ProcessId; +import org.sonar.process.System2; import static java.util.Objects.requireNonNull; @@ -34,12 +37,14 @@ public abstract class AbstractCommand { private final ProcessId id; // program arguments private final Map arguments = new LinkedHashMap<>(); - private final Map envVariables = new HashMap<>(System.getenv()); + private final Map envVariables; + private final Set suppressedEnvVariables = new HashSet<>(); private final File workDir; - protected AbstractCommand(ProcessId id, File workDir) { + protected AbstractCommand(ProcessId id, File workDir, System2 system2) { this.id = requireNonNull(id, "ProcessId can't be null"); this.workDir = requireNonNull(workDir, "workDir can't be null"); + this.envVariables = new HashMap<>(system2.getenv()); } public ProcessId getProcessId() { @@ -79,12 +84,21 @@ public abstract class AbstractCommand { return envVariables; } - public T setEnvVariable(String key, @Nullable String value) { - if (value == null) { - envVariables.remove(key); - } else { - envVariables.put(key, value); - } + public Set getSuppressedEnvVariables() { + return suppressedEnvVariables; + } + + public T suppressEnvVariable(String key) { + requireNonNull(key, "key can't be null"); + suppressedEnvVariables.add(key); + envVariables.remove(key); + return castThis(); + } + + public T setEnvVariable(String key, String value) { + envVariables.put( + requireNonNull(key, "key can't be null"), + requireNonNull(value, "value can't be null")); return castThis(); } } diff --git a/server/sonar-process/src/main/java/org/sonar/process/command/EsCommand.java b/server/sonar-process/src/main/java/org/sonar/process/command/EsCommand.java index 9bfff4627bb..a6d59ac7195 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/command/EsCommand.java +++ b/server/sonar-process/src/main/java/org/sonar/process/command/EsCommand.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Properties; import org.sonar.process.ProcessId; +import org.sonar.process.System2; import org.sonar.process.es.EsFileSystem; import org.sonar.process.es.EsYmlSettings; import org.sonar.process.jmvoptions.EsJvmOptions; @@ -39,7 +40,7 @@ public class EsCommand extends AbstractCommand { private EsYmlSettings esYmlSettings; public EsCommand(ProcessId id, File workDir) { - super(id, workDir); + super(id, workDir, System2.INSTANCE); } public EsFileSystem getFileSystem() { diff --git a/server/sonar-process/src/main/java/org/sonar/process/command/JavaCommand.java b/server/sonar-process/src/main/java/org/sonar/process/command/JavaCommand.java index 7659539b41b..fd461b2b5f6 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/command/JavaCommand.java +++ b/server/sonar-process/src/main/java/org/sonar/process/command/JavaCommand.java @@ -23,6 +23,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; import org.sonar.process.ProcessId; +import org.sonar.process.System2; import org.sonar.process.jmvoptions.JvmOptions; public class JavaCommand extends AbstractCommand> { @@ -33,7 +34,7 @@ public class JavaCommand extends AbstractCommand classpath = new ArrayList<>(); public JavaCommand(ProcessId id, File workDir) { - super(id, workDir); + super(id, workDir, System2.INSTANCE); } public JvmOptions getJvmOptions() { @@ -72,6 +73,7 @@ public class JavaCommand extends AbstractCommand(null, temp.newFolder()) { + new AbstractCommand(null, temp.newFolder(), System2.INSTANCE) { }; } @@ -52,7 +61,7 @@ public class AbstractCommandTest { expectedException.expect(NullPointerException.class); expectedException.expectMessage("workDir can't be null"); - new AbstractCommand(ProcessId.WEB_SERVER, null) { + new AbstractCommand(ProcessId.WEB_SERVER, null, System2.INSTANCE) { }; } @@ -60,7 +69,7 @@ public class AbstractCommandTest { @Test public void test_command_with_complete_information() throws Exception { File workDir = temp.newFolder(); - AbstractCommand command = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir) { + AbstractCommand command = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir, System2.INSTANCE) { }; @@ -79,4 +88,61 @@ public class AbstractCommandTest { assertThat(command.getEnvVariables().size()).isEqualTo(System.getenv().size() + 1); } + @Test + public void setEnvVariable_fails_with_NPE_if_key_is_null() throws IOException { + File workDir = temp.newFolder(); + AbstractCommand underTest = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir, System2.INSTANCE) { + + }; + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("key can't be null"); + + underTest.setEnvVariable(null, randomAlphanumeric(30)); + } + + @Test + public void setEnvVariable_fails_with_NPE_if_value_is_null() throws IOException { + File workDir = temp.newFolder(); + AbstractCommand underTest = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir, System2.INSTANCE) { + + }; + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("value can't be null"); + + underTest.setEnvVariable(randomAlphanumeric(30), null); + } + + @Test + public void constructor_puts_System_getEnv_into_map_of_env_variables() throws IOException { + File workDir = temp.newFolder(); + System2 system2 = Mockito.mock(System2.class); + Map env = IntStream.range(0, 1 + new Random().nextInt(99)).mapToObj(String::valueOf).collect(Collectors.toMap(i -> "key" + i, j -> "value" + j)); + when(system2.getenv()).thenReturn(env); + AbstractCommand underTest = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir, system2) { + + }; + + assertThat(underTest.getEnvVariables()).isEqualTo(env); + } + + @Test + public void suppressEnvVariable_remove_existing_env_variable_and_add_variable_to_set_of_suppressed_variables() throws IOException { + File workDir = temp.newFolder(); + System2 system2 = Mockito.mock(System2.class); + Map env = new HashMap<>(); + String key1 = randomAlphanumeric(3); + env.put(key1, randomAlphanumeric(9)); + when(system2.getenv()).thenReturn(env); + AbstractCommand underTest = new AbstractCommand(ProcessId.ELASTICSEARCH, workDir, system2) { + + }; + + underTest.suppressEnvVariable(key1); + + assertThat(underTest.getEnvVariables()).doesNotContainKey(key1); + assertThat(underTest.getSuppressedEnvVariables()).containsOnly(key1); + } + }