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

5 files changed, 145 insertions, 13 deletions

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<String, String> getenv() {
+      return System.getenv();
+    }
+
+    @Override
+    public String getenv(String name) {
+      return System.getenv(name);
+    }
+  };
+
+  /**
+   * Proxy to {@link System#getenv()}
+   */
+  Map<String, String> 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<T extends AbstractCommand> {
   private final ProcessId id;
   // program arguments
   private final Map<String, String> arguments = new LinkedHashMap<>();
-  private final Map<String, String> envVariables = new HashMap<>(System.getenv());
+  private final Map<String, String> envVariables;
+  private final Set<String> 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<T extends AbstractCommand> {
     return envVariables;
   }
 
-  public T setEnvVariable(String key, @Nullable String value) {
-    if (value == null) {
-      envVariables.remove(key);
-    } else {
-      envVariables.put(key, value);
-    }
+  public Set<String> 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<EsCommand> {
   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<T extends JvmOptions> extends AbstractCommand<JavaCommand<T>> {
@@ -33,7 +34,7 @@ public class JavaCommand<T extends JvmOptions> extends AbstractCommand<JavaComma
   private final List<String> classpath = new ArrayList<>();
 
   public JavaCommand(ProcessId id, File workDir) {
-    super(id, workDir);
+    super(id, workDir, System2.INSTANCE);
   }
 
   public JvmOptions<T> getJvmOptions() {
@@ -72,6 +73,7 @@ public class JavaCommand<T extends JvmOptions> extends AbstractCommand<JavaComma
       ", classpath=" + classpath +
       ", arguments=" + getArguments() +
       ", envVariables=" + getEnvVariables() +
+      ", suppressedEnvVariables=" + getSuppressedEnvVariables() +
       '}';
   }
 }
diff --git a/server/sonar-process/src/test/java/org/sonar/process/command/AbstractCommandTest.java b/server/sonar-process/src/test/java/org/sonar/process/command/AbstractCommandTest.java
index 62f59f11a48..dec2dde6008 100644
--- a/server/sonar-process/src/test/java/org/sonar/process/command/AbstractCommandTest.java
+++ b/server/sonar-process/src/test/java/org/sonar/process/command/AbstractCommandTest.java
@@ -21,14 +21,23 @@ package org.sonar.process.command;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Properties;
+import java.util.Random;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
 import org.sonar.process.ProcessId;
+import org.sonar.process.System2;
 
+import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
 
 public class AbstractCommandTest {
 
@@ -42,7 +51,7 @@ public class AbstractCommandTest {
     expectedException.expect(NullPointerException.class);
     expectedException.expectMessage("ProcessId can't be null");
 
-    new AbstractCommand<AbstractCommand>(null, temp.newFolder()) {
+    new AbstractCommand<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<AbstractCommand>(ProcessId.WEB_SERVER, null) {
+    new AbstractCommand<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<String, String> 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<String, String> 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);
+  }
+
 }