Allow clients to pass in environment for devappserver runs (#381)

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/api/devserver/DefaultRunConfiguration.java

@@ -16,8 +16,12 @@
 
 package com.google.cloud.tools.appengine.api.devserver;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
 import java.io.File;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Plain Java bean implementation of {@link RunConfiguration}.
@@ -48,6 +52,7 @@ public class DefaultRunConfiguration implements RunConfiguration {
   private String defaultGcsBucketName;
   private Boolean clearDatastore;
   private File datastorePath;
+  private Map<String, String> environment;
 
   @Override
   public List<File> getServices() {
@@ -172,7 +177,7 @@ public List<String> getJvmFlags() {
   }
 
   public void setJvmFlags(List<String> jvmFlags) {
-    this.jvmFlags = jvmFlags;
+    this.jvmFlags = jvmFlags != null ? ImmutableList.copyOf(jvmFlags) : null;
   }
 
   @Override
@@ -264,4 +269,13 @@ public File getDatastorePath() {
   public void setDatastorePath(File datastorePath) {
     this.datastorePath = datastorePath;
   }
+
+  @Override
+  public Map<String, String> getEnvironment() {
+    return environment;
+  }
+
+  public void setEnvironment(Map<String, String> environment) {
+    this.environment = environment != null ? ImmutableMap.copyOf(environment) : null;
+  }
 }

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/api/devserver/RunConfiguration.java

@@ -18,6 +18,7 @@
 
 import java.io.File;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Configuration of running local Development Server.
@@ -78,4 +79,6 @@ public interface RunConfiguration {
   Boolean getClearDatastore();
 
   File getDatastorePath();
+
+  Map<String, String> getEnvironment();
 }

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkAppEngineDevServer1.java

@@ -123,6 +123,10 @@ public void run(RunConfiguration config) throws AppEngineException {
           + Joiner.on(",").withKeyValueSeparator("=").join(appEngineEnvironment));
     }
 
+    if (config.getEnvironment() != null) {
+      appEngineEnvironment.putAll(config.getEnvironment());
+    }
+
     try {
       sdk.runDevAppServer1Command(jvmArguments, arguments, appEngineEnvironment);
     } catch (ProcessRunnerException e) {

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkAppEngineDevServer2.java

@@ -90,8 +90,9 @@ public void run(RunConfiguration config) throws AppEngineException {
     arguments.addAll(DevAppServerArgs.get("skip_sdk_update_check", config.getSkipSdkUpdateCheck()));
     arguments
         .addAll(DevAppServerArgs.get("default_gcs_bucket_name", config.getDefaultGcsBucketName()));
-    arguments .addAll(DevAppServerArgs.get("clear_datastore", config.getClearDatastore()));
-    arguments .addAll(DevAppServerArgs.get("datastore_path", config.getDatastorePath()));
+    arguments.addAll(DevAppServerArgs.get("clear_datastore", config.getClearDatastore()));
+    arguments.addAll(DevAppServerArgs.get("datastore_path", config.getDatastorePath()));
+    arguments.addAll(DevAppServerArgs.get("env_var", config.getEnvironment()));
 
     try {
       sdk.runDevAppServerCommand(arguments);

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/args/Args.java

@@ -121,11 +121,11 @@ static List<String> path(String name, Path path) {
   }
 
   /**
-   * Produces a key/value pair list from a {@link Map}.
+   * Produces a single element list with a key/value pair comma separated string from a {@link Map}.
    *
-   * @return [key1=value1,key2=value2,...], [] if keyValueMapping=empty/null
+   * @return {@code ["key1=value1,key2=value2,..."]} or {@code []} if keyValueMapping=empty/null
    */
-  public static List<String> keyValues(Map<?, ?> keyValueMapping) {
+  static List<String> keyValueString(Map<?, ?> keyValueMapping) {
     List<String> result = Lists.newArrayList();
     if (keyValueMapping != null && keyValueMapping.size() > 0) {
       for (Map.Entry<?, ?> entry : keyValueMapping.entrySet()) {
@@ -137,4 +137,23 @@ public static List<String> keyValues(Map<?, ?> keyValueMapping) {
 
     return Collections.emptyList();
   }
+
+  /**
+   * Produces a flagged key/value pair list given a flag name and a {@link Map}.
+   *
+   * @return {@code [--flagName, key1=value1, --flagName, key2=value2, ...]} or {@code []}
+   *        if keyValueMapping=empty/null
+   */
+  static List<String> flaggedKeyValues(final String flagName, Map<?, ?> keyValueMapping) {
+    List<String> result = Lists.newArrayList();
+    if (keyValueMapping != null && keyValueMapping.size() > 0) {
+      for (Map.Entry<?, ?> entry : keyValueMapping.entrySet()) {
+        result.addAll(string(flagName, entry.getKey() + "=" + entry.getValue()));
+      }
+
+      return result;
+    }
+
+    return Collections.emptyList();
+  }
 }

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/args/DevAppServerArgs.java

@@ -21,35 +21,36 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Command Line argument helper for dev_appserver based command.
  */
 public class DevAppServerArgs {
 
   /**
-   * @return {@code [--name=value]} or {@code []} if value=null.
+   * Returns {@code [--name=value]} or {@code []} if value=null.
    */
   public static List<String> get(String name, String value) {
     return Args.stringWithEq(name, value);
   }
 
   /**
-   * @return {@code [--name=value1, --name=value2, ...]} or {@code []} if value=null.
+   * Returns {@code [--name=value1, --name=value2, ...]} or {@code []} if value=null.
    */
   public static List<String> get(String name, List<String> values) {
     return Args.stringsWithEq(name, values);
   }
 
   /**
-   * @return {@code [--name=value]} or {@code []} if value=null.
+   * Returns {@code [--name=value]} or {@code []} if value=null.
    */
   public static List<String> get(String name, Integer value) {
     return Args.integerWithEq(name, value);
   }
 
   /**
-   * @return {@code [--name=true]} if value=true, {@code [--name=false]} if value=false,
+   * Returns {@code [--name=true]} if value=true, {@code [--name=false]} if value=false,
    *     {@code []} if value=null.
    */
   public static List<String> get(String name, Boolean value) {
@@ -60,7 +61,7 @@ public static List<String> get(String name, Boolean value) {
   }
 
   /**
-   * @return {@code [--name=filePath]} or {@code []} if file=null.
+   * Returns {@code [--name=filePath]} or {@code []} if file=null.
    */
   public static List<String> get(String name, File file) {
     if (file != null) {
@@ -71,4 +72,12 @@ public static List<String> get(String name, File file) {
     }
     return Collections.emptyList();
   }
+
+  /**
+   * Returns {@code [--name, key1=val1, --name, key2=val2, ...]} or {@code []} if
+   *      keyValues=empty/null.
+   */
+  public static List<String> get(String name, Map<String, String> keyValues) {
+    return Args.flaggedKeyValues(name, keyValues);
+  }
 }

appengine-plugins-core/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/args/GcloudArgs.java

@@ -77,7 +77,7 @@ public static List<String> get(String name, Path path) {
    * @return {@code [key1=value1,key2=value2,...]}, {@code []} if keyValueMapping=empty/null
    */
   public static List<String> get(Map<?, ?> keyValueMapping) {
-    return Args.keyValues(keyValueMapping);
+    return Args.keyValueString(keyValueMapping);
   }
 
   /**

appengine-plugins-core/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkAppEngineDevServer1Test.java

@@ -120,6 +120,7 @@ public void testPrepareCommand_allFlags() throws Exception {
     configuration.setPort(8090);
     configuration.setJvmFlags(ImmutableList.of("-Dflag1", "-Dflag2"));
     configuration.setDefaultGcsBucketName("buckets");
+    configuration.setEnvironment(null);
 
     // these params are not used by devappserver1 and will log warnings
     configuration.setAdminHost("adminHost");
@@ -257,7 +258,7 @@ public void testPrepareCommand_noFlagsMultiModule() throws AppEngineException, P
   }
 
   @Test
-  public void testPrepareCommand_environmentVariables() throws AppEngineException, ProcessRunnerException {
+  public void testPrepareCommand_appEngineWebXmlEnvironmentVariables() throws AppEngineException, ProcessRunnerException {
     DefaultRunConfiguration configuration = new DefaultRunConfiguration();
     configuration.setServices(ImmutableList.of(java8Service1EnvVars));
 
@@ -275,7 +276,7 @@ public void testPrepareCommand_environmentVariables() throws AppEngineException,
   }
 
   @Test
-  public void testPrepareCommand_multipleServicesDuplicateEnvironmentVariables() throws AppEngineException, ProcessRunnerException {
+  public void testPrepareCommand_multipleServicesDuplicateAppEngineWebXmlEnvironmentVariables() throws AppEngineException, ProcessRunnerException {
     DefaultRunConfiguration configuration = new DefaultRunConfiguration();
     configuration.setServices(ImmutableList.of(java8Service1EnvVars, java8Service2EnvVars));
 
@@ -294,6 +295,48 @@ public void testPrepareCommand_multipleServicesDuplicateEnvironmentVariables() t
     verify(sdk, times(1)).runDevAppServer1Command(expectedJvmArgs, expectedFlags, expectedEnvironment);
   }
 
+  @Test
+  public void testPrepareCommand_clientSuppliedEnvironmentVariables() throws AppEngineException, ProcessRunnerException {
+    DefaultRunConfiguration configuration = new DefaultRunConfiguration();
+    configuration.setServices(ImmutableList.of(java7Service));
+
+    Map<String, String> clientEnvironmentVariables = ImmutableMap.of("mykey1", "myval1", "mykey2", "myval2");
+    configuration.setEnvironment(clientEnvironmentVariables);
+
+    List<String> expectedFlags = ImmutableList.of("--allow_remote_shutdown",
+        "--disable_update_check", pathToJava7Service.toString());
+    List<String> expectedJvmArgs = ImmutableList
+        .of("-javaagent:" + fakeJavaSdkHome.resolve("agent/appengine-agent.jar").toAbsolutePath()
+            .toString());
+
+    devServer.run(configuration);
+
+    verify(sdk, times(1)).runDevAppServer1Command(expectedJvmArgs, expectedFlags, clientEnvironmentVariables);
+  }
+
+  @Test
+  public void testPrepareCommand_clientSuppliedAndAppEngineWebXmlEnvironmentVariables() throws AppEngineException, ProcessRunnerException {
+    DefaultRunConfiguration configuration = new DefaultRunConfiguration();
+    configuration.setServices(ImmutableList.of(java8Service1EnvVars));
+
+    Map<String, String> clientEnvironmentVariables = ImmutableMap.of("mykey1", "myval1", "mykey2", "myval2");
+    configuration.setEnvironment(clientEnvironmentVariables);
+
+    List<String> expectedFlags = ImmutableList.of("--allow_remote_shutdown",
+        "--disable_update_check", "--no_java_agent", pathToJava8Service1WithEnvVars.toString());
+
+    List<String> expectedJvmArgs = ImmutableList.of("-Duse_jetty9_runtime=true",
+            "-D--enable_all_permissions=true");
+
+    Map<String, String> appEngineEnvironment = ImmutableMap.of("key1", "val1", "key2", "val2");
+    Map<String, String> expectedEnvironment = Maps.newHashMap(appEngineEnvironment);
+    expectedEnvironment.putAll(clientEnvironmentVariables);
+
+    devServer.run(configuration);
+
+    verify(sdk, times(1)).runDevAppServer1Command(expectedJvmArgs, expectedFlags, expectedEnvironment);
+  }
+
   @Test
   public void testCheckAndWarnIgnored_withSetValue() {
     devServer.checkAndWarnIgnored(new Object(), "testName");

appengine-plugins-core/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkAppEngineDevServer2Test.java

@@ -22,6 +22,7 @@
 import com.google.cloud.tools.test.utils.SpyVerifier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -35,6 +36,7 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
+import java.util.Map;
 
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.times;
@@ -95,6 +97,7 @@ public void testPrepareCommand_allFlags() throws Exception {
     configuration.setDefaultGcsBucketName("buckets");
     configuration.setClearDatastore(true);
     configuration.setDatastorePath(fakeDatastorePath.toFile());
+    configuration.setEnvironment(null);
 
     SpyVerifier.newVerifier(configuration).verifyDeclaredSetters();
 
@@ -114,7 +117,7 @@ public void testPrepareCommand_allFlags() throws Exception {
     verify(sdk, times(1)).runDevAppServerCommand(eq(expected));
 
     SpyVerifier.newVerifier(configuration).verifyDeclaredGetters(
-        ImmutableMap.<String, Integer>of("getServices", 3));
+        ImmutableMap.of("getServices", 3));
 
   }
 
@@ -151,4 +154,20 @@ public void testPrepareCommand_noFlags() throws AppEngineException, ProcessRunne
     verify(sdk, times(1)).runDevAppServerCommand(eq(expected));
   }
 
+  @Test
+  public void testPrepareCommand_clientEnvVars() throws AppEngineException, ProcessRunnerException {
+    DefaultRunConfiguration configuration = new DefaultRunConfiguration();
+    configuration.setServices(ImmutableList.of(new File("exploded-war/")));
+
+    Map<String, String> clientEnvVars = ImmutableMap.of("key1", "val1", "key2", "val2");
+    configuration.setEnvironment(clientEnvVars);
+
+    List<String> expectedArgs = ImmutableList
+        .of("exploded-war", "--env_var", "key1=val1", "--env_var", "key2=val2");
+
+    devServer.run(configuration);
+
+    verify(sdk, times(1)).runDevAppServerCommand(eq(expectedArgs));
+  }
+
 }

appengine-plugins-core/src/test/java/com/google/cloud/tools/appengine/cloudsdk/internal/args/DevAppServerArgsTest.java

@@ -16,11 +16,14 @@
 
 package com.google.cloud.tools.appengine.cloudsdk.internal.args;
 
+import com.google.common.collect.ImmutableMap;
+
 import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 
@@ -63,4 +66,12 @@ public void testGet_integer() {
 
     assertEquals(Arrays.asList("--port=8080"), DevAppServerArgs.get("port", 8080));
   }
+
+  @Test
+  public void testGet_flaggedKeyValues() {
+    Map<String, String> keyValues = ImmutableMap.of("key1", "val1", "key2", "val2");
+
+    assertEquals(Arrays.asList("--name", "key1=val1", "--name", "key2=val2"),
+        DevAppServerArgs.get("name", keyValues));
+  }
 }