From a90fcf3b9b6debbafdcd1ed7f9b096a592ef9c99 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 7 Sep 2016 14:14:05 -0400 Subject: [PATCH 01/14] @Override --- .../com/google/cloud/tools/appengine/cloudsdk/PathResolver.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/PathResolver.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/PathResolver.java index 9530f8ba6..c4d48501d 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/PathResolver.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/PathResolver.java @@ -31,6 +31,7 @@ public class PathResolver implements CloudSdkResolver { * * @return Path to Google Cloud SDK or null */ + @Override public Path getCloudSdkPath() { List possiblePaths = new ArrayList<>(); From 3c470fd12d0b80dfe7294bed01f15d13af68172a Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 7 Sep 2016 15:03:20 -0400 Subject: [PATCH 02/14] notice when app engine java components aren't installed --- .../AppEngineComponentsNotInstalledException.java | 11 +++++++++++ .../cloud/tools/appengine/cloudsdk/CloudSdk.java | 10 ++++++++++ .../cloud/tools/appengine/cloudsdk/CloudSdkTest.java | 9 +++++++-- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java new file mode 100644 index 000000000..ed1613412 --- /dev/null +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java @@ -0,0 +1,11 @@ +package com.google.cloud.tools.appengine.cloudsdk; + +import com.google.cloud.tools.appengine.api.AppEngineException; + +public class AppEngineComponentsNotInstalledException extends AppEngineException { + + AppEngineComponentsNotInstalledException(String message) { + super(message); + } + +} diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 5e495d93b..90681bde2 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -59,6 +59,7 @@ public class CloudSdk { private static final String DEV_APPSERVER_PY = "bin/dev_appserver.py"; private static final String JAVA_APPENGINE_SDK_PATH = "platform/google_appengine/google/appengine/tools/java/lib"; + private static final String JAVA_COMPONENTS_PATH = "platform/gcd/.appengine"; private static final String JAVA_TOOLS_JAR = "appengine-tools-api.jar"; private static final Map JAR_LOCATIONS = new HashMap<>(); private static final String WINDOWS_BUNDLED_PYTHON = "platform/bundledpython/python.exe"; @@ -181,6 +182,11 @@ private void logCommand(List command) { public Path getSdkPath() { return sdkPath; } + + // TODO does this work on windows? + Path getJavaAppEngineComponentsPath() { + return sdkPath.resolve(JAVA_COMPONENTS_PATH); + } private Path getGCloudPath() { String gcloud = GCLOUD; @@ -262,6 +268,10 @@ public void validate() throws AppEngineException { "Validation Error: Java Tools jar location '" + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not a file."); } + if (!Files.isDirectory(getJavaAppEngineComponentsPath())) { + throw new AppEngineComponentsNotInstalledException( + "Validation Error: Java App Engine components not installed"); + } } @VisibleForTesting diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java index 4319dac03..4f439b5a4 100644 --- a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -5,9 +5,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.when; -import com.google.cloud.tools.appengine.cloudsdk.CloudSdk.Builder; - import com.google.cloud.tools.appengine.api.AppEngineException; +import com.google.cloud.tools.appengine.cloudsdk.CloudSdk.Builder; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,6 +46,12 @@ public void testGetJavaAppEngineSdkPath() { assertEquals(root.resolve("platform/google_appengine/google/appengine/tools/java/lib"), builder.build().getJavaAppEngineSdkPath()); } + + @Test + public void testGetJavaAppEngineComponentsPath() { + assertEquals(root.resolve("platform/gcd/.appengine"), + builder.build().getJavaAppEngineComponentsPath()); + } @Test public void testGetJarPathJavaTools() { From 585f1cae43ae05acd3e46778b201f34c786960b8 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 07:21:57 -0400 Subject: [PATCH 03/14] use gcloud components list to look for app engine java components --- .../tools/appengine/cloudsdk/CloudSdk.java | 51 +++++++++++++++---- .../process/DefaultProcessRunner.java | 6 ++- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 5e495d93b..550e62c43 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -18,6 +18,7 @@ import com.google.cloud.tools.appengine.api.AppEngineException; import com.google.cloud.tools.appengine.cloudsdk.internal.args.GcloudArgs; +import com.google.cloud.tools.appengine.cloudsdk.internal.process.AccumulatingLineListener; import com.google.cloud.tools.appengine.cloudsdk.internal.process.DefaultProcessRunner; import com.google.cloud.tools.appengine.cloudsdk.internal.process.ProcessRunner; import com.google.cloud.tools.appengine.cloudsdk.internal.process.ProcessRunnerException; @@ -70,7 +71,8 @@ public class CloudSdk { @Nullable private final File appCommandCredentialFile; private final String appCommandOutputFormat; - private final WaitingProcessOutputLineListener runDevAppServerWaitListener; + private final WaitingProcessOutputLineListener outputLineListener; + private final AccumulatingLineListener stdOutput; private CloudSdk(Path sdkPath, String appCommandMetricsEnvironment, @@ -78,14 +80,16 @@ private CloudSdk(Path sdkPath, @Nullable File appCommandCredentialFile, String appCommandOutputFormat, ProcessRunner processRunner, - WaitingProcessOutputLineListener runDevAppServerWaitListener) { + WaitingProcessOutputLineListener outputLineListener, + AccumulatingLineListener stdOutput) { this.sdkPath = sdkPath; this.appCommandMetricsEnvironment = appCommandMetricsEnvironment; this.appCommandMetricsEnvironmentVersion = appCommandMetricsEnvironmentVersion; this.appCommandCredentialFile = appCommandCredentialFile; this.appCommandOutputFormat = appCommandOutputFormat; this.processRunner = processRunner; - this.runDevAppServerWaitListener = runDevAppServerWaitListener; + this.outputLineListener = outputLineListener; + this.stdOutput = stdOutput; // Populate jar locations. // TODO(joaomartins): Consider case where SDK doesn't contain these jars. Only App Engine @@ -100,7 +104,7 @@ private CloudSdk(Path sdkPath, /** * Uses the process runner to execute the gcloud app command with the provided arguments. * - * @param args The arguments to pass to "gcloud app" command. + * @param args the arguments to pass to "gcloud app" command */ public void runAppCommand(List args) throws ProcessRunnerException { List command = new ArrayList<>(); @@ -108,7 +112,6 @@ public void runAppCommand(List args) throws ProcessRunnerException { command.add("app"); command.addAll(args); - command.add("--quiet"); command.addAll(GcloudArgs.get("format", appCommandOutputFormat)); Map environment = Maps.newHashMap(); @@ -125,6 +128,31 @@ public void runAppCommand(List args) throws ProcessRunnerException { logCommand(command); processRunner.setEnvironment(environment); processRunner.run(command.toArray(new String[command.size()])); + processRunner.run(command.toArray(new String[command.size()])); + } + + /** + * @return true iff the specified component is installed in the local environment + */ + public boolean isComponentInstalled(String id) throws ProcessRunnerException { + List command = new ArrayList<>(); + command.add(getGCloudPath().toString()); + command.add("components"); + command.add("list"); + command.add("--format=json"); + command.add("--filter=id:" + id); + + if (stdOutput == null) { + throw new IllegalStateException("won't be able to read output"); + } + + stdOutput.clear(); + processRunner.run(command.toArray(new String[command.size()])); + + String json = stdOutput.getOutput(); + + // todo: parse the JSON + return json.contains("\"name\": \"Installed\""); } /** @@ -149,8 +177,8 @@ public void runDevAppServerCommand(List args) throws ProcessRunnerExcept processRunner.run(command.toArray(new String[command.size()])); // wait for start if configured - if (runDevAppServerWaitListener != null) { - runDevAppServerWaitListener.await(); + if (outputLineListener != null) { + outputLineListener.await(); } } @@ -266,7 +294,7 @@ public void validate() throws AppEngineException { @VisibleForTesting WaitingProcessOutputLineListener getRunDevAppServerWaitListener() { - return runDevAppServerWaitListener; + return outputLineListener; } public static class Builder { @@ -415,10 +443,14 @@ public CloudSdk build() { // Verify there aren't listeners if subprocess inherits output. // If output is inherited, then listeners won't receive anything. + AccumulatingLineListener stdOut = null; if (inheritProcessOutput && (stdOutLineListeners.size() > 0 || stdErrLineListeners.size() > 0)) { throw new AppEngineException("You cannot specify subprocess output inheritance and" + " output listeners."); + } else { + stdOut = new AccumulatingLineListener(); + stdOutLineListeners.add(stdOut); } // Construct process runner. @@ -445,7 +477,7 @@ public CloudSdk build() { return new CloudSdk(sdkPath, appCommandMetricsEnvironment, appCommandMetricsEnvironmentVersion, appCommandCredentialFile, appCommandOutputFormat, - processRunner, runDevAppServerWaitListener); + processRunner, runDevAppServerWaitListener, stdOut); } /** @@ -528,4 +560,5 @@ public int compare(CloudSdkResolver o1, CloudSdkResolver o2) { } } + } diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java index 50e0d471c..d088e4582 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java @@ -85,8 +85,9 @@ public DefaultProcessRunner(boolean async, *

If any output listeners were configured, output will go to them only. Otherwise, process * output will be redirected to the caller via inheritIO. * - * @param command The shell command to execute + * @param command the shell command to execute */ + @Override public void run(String[] command) throws ProcessRunnerException { try { // Configure process builder. @@ -135,6 +136,7 @@ public void run(String[] command) throws ProcessRunnerException { /** * Environment variables to append to the current system environment variables. */ + @Override public void setEnvironment(Map environment) { this.environment = environment; } @@ -142,6 +144,7 @@ public void setEnvironment(Map environment) { private void handleStdOut(final Process process) { final Scanner stdOut = new Scanner(process.getInputStream(), Charsets.UTF_8.name()); Thread stdOutThread = new Thread("standard-out") { + @Override public void run() { while (stdOut.hasNextLine() && !Thread.interrupted()) { String line = stdOut.nextLine(); @@ -159,6 +162,7 @@ public void run() { private void handleErrOut(final Process process) { final Scanner stdErr = new Scanner(process.getErrorStream(), Charsets.UTF_8.name()); Thread stdErrThread = new Thread("standard-err") { + @Override public void run() { while (stdErr.hasNextLine() && !Thread.interrupted()) { String line = stdErr.nextLine(); From a78d87bdaf5ad43e823b0c169cd62fb07add1dbe Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 07:23:08 -0400 Subject: [PATCH 04/14] use gcloud components list to look for app engine java components --- .../process/AccumulatingLineListener.java | 39 ++++++++++++++++++ .../cloudsdk/CloudSdkEnvironmentTest.java | 40 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java create mode 100644 src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkEnvironmentTest.java diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java new file mode 100644 index 000000000..c56fbeb88 --- /dev/null +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java @@ -0,0 +1,39 @@ +/* + * Copyright 2016 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.tools.appengine.cloudsdk.internal.process; + +import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; + +public class AccumulatingLineListener implements ProcessOutputLineListener { + + private StringBuilder output = new StringBuilder(); + + @Override + public void onOutputLine(String line) { + output.append(line + "\n"); + } + + // todo: maybe this should be a standard part of ProcessRunner API instead? + public String getOutput() { + return output.toString(); + } + + public void clear() { + output = new StringBuilder(); + } + +} diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkEnvironmentTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkEnvironmentTest.java new file mode 100644 index 000000000..df78b4394 --- /dev/null +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkEnvironmentTest.java @@ -0,0 +1,40 @@ +package com.google.cloud.tools.appengine.cloudsdk; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.nio.file.Files; + +import org.junit.Test; + +import com.google.cloud.tools.appengine.cloudsdk.internal.process.ProcessRunnerException; + +/** + * Integration tests for {@link CloudSdk} that require an installed CloudSdk instance. + */ +public class CloudSdkEnvironmentTest { + + private CloudSdk sdk = new CloudSdk.Builder().build(); + + @Test + public void testGetSdkPath() { + assertTrue(Files.exists(sdk.getSdkPath())); + } + + @Test + public void testIsComponentInstalled_true() throws ProcessRunnerException { + assertTrue(sdk.isComponentInstalled("app-engine-java")); + } + + @Test + public void testIsComponentInstalled_False() throws ProcessRunnerException { + assertFalse(sdk.isComponentInstalled("no-such-component")); + } + + @Test + public void testIsComponentInstalled_sequential() throws ProcessRunnerException { + assertTrue(sdk.isComponentInstalled("app-engine-java")); + assertFalse(sdk.isComponentInstalled("no-such-component")); + } + +} From 34580ab3f5088cf22263a8860c73a319ae6c7144 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 07:26:44 -0400 Subject: [PATCH 05/14] use gcloud components list to look for app engine java components --- .../tools/appengine/cloudsdk/CloudSdk.java | 19 +++++++++---------- .../appengine/cloudsdk/CloudSdkTest.java | 6 ------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index bbe752f50..d2f23460d 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -60,7 +60,6 @@ public class CloudSdk { private static final String DEV_APPSERVER_PY = "bin/dev_appserver.py"; private static final String JAVA_APPENGINE_SDK_PATH = "platform/google_appengine/google/appengine/tools/java/lib"; - private static final String JAVA_COMPONENTS_PATH = "platform/gcd/.appengine"; private static final String JAVA_TOOLS_JAR = "appengine-tools-api.jar"; private static final Map JAR_LOCATIONS = new HashMap<>(); private static final String WINDOWS_BUNDLED_PYTHON = "platform/bundledpython/python.exe"; @@ -210,11 +209,6 @@ private void logCommand(List command) { public Path getSdkPath() { return sdkPath; } - - // TODO does this work on windows? - Path getJavaAppEngineComponentsPath() { - return sdkPath.resolve(JAVA_COMPONENTS_PATH); - } private Path getGCloudPath() { String gcloud = GCLOUD; @@ -267,7 +261,7 @@ public Path getJarPath(String jarName) { /** * Checks whether the configured Cloud SDK Path is valid. * - * @throws AppEngineException when there is a validation error. + * @throws AppEngineException when there is a validation error */ public void validate() throws AppEngineException { if (sdkPath == null) { @@ -296,9 +290,14 @@ public void validate() throws AppEngineException { "Validation Error: Java Tools jar location '" + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not a file."); } - if (!Files.isDirectory(getJavaAppEngineComponentsPath())) { - throw new AppEngineComponentsNotInstalledException( - "Validation Error: Java App Engine components not installed"); + try { + if (!isComponentInstalled("app-engine-java")) { + throw new AppEngineComponentsNotInstalledException( + "Validation Error: App Engine Java component not installed"); + } + } catch (ProcessRunnerException ex) { + throw new AppEngineException( + "Could not determine whether App Engine Java component is installed", ex); } } diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java index 4f439b5a4..cb454d67a 100644 --- a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -46,12 +46,6 @@ public void testGetJavaAppEngineSdkPath() { assertEquals(root.resolve("platform/google_appengine/google/appengine/tools/java/lib"), builder.build().getJavaAppEngineSdkPath()); } - - @Test - public void testGetJavaAppEngineComponentsPath() { - assertEquals(root.resolve("platform/gcd/.appengine"), - builder.build().getJavaAppEngineComponentsPath()); - } @Test public void testGetJarPathJavaTools() { From 29147b0c66ac9c154d990808acfebec1ffc8f716 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 07:29:14 -0400 Subject: [PATCH 06/14] fix up tests --- .../google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java index cb454d67a..206358149 100644 --- a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -74,7 +74,7 @@ public void testNewCloudSdk_outListener() { CloudSdk sdk = builder.build(); assertNotNull(sdk.getRunDevAppServerWaitListener()); - assertEquals(2, builder.getStdOutLineListeners().size()); + assertEquals(3, builder.getStdOutLineListeners().size()); assertEquals(1, builder.getStdErrLineListeners().size()); assertEquals(1, builder.getExitListeners().size()); } @@ -85,7 +85,7 @@ public void testNewCloudSdk_errListener() { CloudSdk sdk = builder.build(); assertNotNull(sdk.getRunDevAppServerWaitListener()); - assertEquals(1, builder.getStdOutLineListeners().size()); + assertEquals(2, builder.getStdOutLineListeners().size()); assertEquals(2, builder.getStdErrLineListeners().size()); assertEquals(1, builder.getExitListeners().size()); } From b776b12333b343c0591f3b7483b74a41568889e2 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 12:07:56 -0400 Subject: [PATCH 07/14] parse JSON response --- pom.xml | 5 +++++ .../cloud/tools/appengine/cloudsdk/CloudSdk.java | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 6289064e0..d6595ca8f 100644 --- a/pom.xml +++ b/pom.xml @@ -93,6 +93,11 @@ jsr305 3.0.1 + + org.json + json + 20160810 + diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index d2f23460d..f81b254e8 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -48,6 +48,10 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONTokener; + /** * Cloud SDK CLI wrapper. */ @@ -151,8 +155,16 @@ public boolean isComponentInstalled(String id) throws ProcessRunnerException { String json = stdOutput.getOutput(); - // todo: parse the JSON - return json.contains("\"name\": \"Installed\""); + try { + JSONTokener tokener = new JSONTokener(json); + JSONObject object = new JSONObject(tokener); + JSONObject state = object.getJSONObject("state"); + String name = state.getString("name"); + return "Installed".equals(name); + } catch (JSONException | NullPointerException ex) { + throw new AppEngineException( + "Could not determine whether App Engine Java component is installed", ex); + } } /** From c70ea3394c5703fef8106e9f96af8a6f6bde23d4 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 12:15:49 -0400 Subject: [PATCH 08/14] gcloud returns JSON arrays --- .../google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index f81b254e8..73a4eb130 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -48,6 +48,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; @@ -153,11 +154,15 @@ public boolean isComponentInstalled(String id) throws ProcessRunnerException { stdOutput.clear(); processRunner.run(command.toArray(new String[command.size()])); - String json = stdOutput.getOutput(); + String json = stdOutput.getOutput().trim(); try { JSONTokener tokener = new JSONTokener(json); - JSONObject object = new JSONObject(tokener); + JSONArray array = new JSONArray(tokener); + if (array.length() == 0) { + return false; + } + JSONObject object = array.getJSONObject(0); JSONObject state = object.getJSONObject("state"); String name = state.getString("name"); return "Installed".equals(name); From 7d5e3a2ea959c92ed58c58cc6a255f2a3dba0645 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 12:23:49 -0400 Subject: [PATCH 09/14] add exception javadoc --- .../cloudsdk/AppEngineComponentsNotInstalledException.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java index ed1613412..e1ae963a0 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/AppEngineComponentsNotInstalledException.java @@ -2,6 +2,9 @@ import com.google.cloud.tools.appengine.api.AppEngineException; +/** + * User needs to run gcloud components install app-engine-java. + */ public class AppEngineComponentsNotInstalledException extends AppEngineException { AppEngineComponentsNotInstalledException(String message) { From e9083c20aa4ce3ec0dba9b217850e8f2008bc596 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 12:52:36 -0400 Subject: [PATCH 10/14] push stdOut collection into ProcessRunner --- .../tools/appengine/cloudsdk/CloudSdk.java | 21 ++------ .../process/AccumulatingLineListener.java | 5 +- .../process/DefaultProcessRunner.java | 50 +++++++++++++++++++ .../internal/process/ProcessRunner.java | 1 + .../appengine/cloudsdk/CloudSdkTest.java | 4 +- 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 73a4eb130..2c9f8c7d1 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -18,7 +18,6 @@ import com.google.cloud.tools.appengine.api.AppEngineException; import com.google.cloud.tools.appengine.cloudsdk.internal.args.GcloudArgs; -import com.google.cloud.tools.appengine.cloudsdk.internal.process.AccumulatingLineListener; import com.google.cloud.tools.appengine.cloudsdk.internal.process.DefaultProcessRunner; import com.google.cloud.tools.appengine.cloudsdk.internal.process.ProcessRunner; import com.google.cloud.tools.appengine.cloudsdk.internal.process.ProcessRunnerException; @@ -77,7 +76,6 @@ public class CloudSdk { private final File appCommandCredentialFile; private final String appCommandOutputFormat; private final WaitingProcessOutputLineListener outputLineListener; - private final AccumulatingLineListener stdOutput; private CloudSdk(Path sdkPath, String appCommandMetricsEnvironment, @@ -85,8 +83,7 @@ private CloudSdk(Path sdkPath, @Nullable File appCommandCredentialFile, String appCommandOutputFormat, ProcessRunner processRunner, - WaitingProcessOutputLineListener outputLineListener, - AccumulatingLineListener stdOutput) { + WaitingProcessOutputLineListener outputLineListener) { this.sdkPath = sdkPath; this.appCommandMetricsEnvironment = appCommandMetricsEnvironment; this.appCommandMetricsEnvironmentVersion = appCommandMetricsEnvironmentVersion; @@ -94,7 +91,6 @@ private CloudSdk(Path sdkPath, this.appCommandOutputFormat = appCommandOutputFormat; this.processRunner = processRunner; this.outputLineListener = outputLineListener; - this.stdOutput = stdOutput; // Populate jar locations. // TODO(joaomartins): Consider case where SDK doesn't contain these jars. Only App Engine @@ -147,14 +143,7 @@ public boolean isComponentInstalled(String id) throws ProcessRunnerException { command.add("--format=json"); command.add("--filter=id:" + id); - if (stdOutput == null) { - throw new IllegalStateException("won't be able to read output"); - } - - stdOutput.clear(); - processRunner.run(command.toArray(new String[command.size()])); - - String json = stdOutput.getOutput().trim(); + String json = processRunner.runSynchronously(command.toArray(new String[command.size()])); try { JSONTokener tokener = new JSONTokener(json); @@ -469,14 +458,10 @@ public CloudSdk build() { // Verify there aren't listeners if subprocess inherits output. // If output is inherited, then listeners won't receive anything. - AccumulatingLineListener stdOut = null; if (inheritProcessOutput && (stdOutLineListeners.size() > 0 || stdErrLineListeners.size() > 0)) { throw new AppEngineException("You cannot specify subprocess output inheritance and" + " output listeners."); - } else { - stdOut = new AccumulatingLineListener(); - stdOutLineListeners.add(stdOut); } // Construct process runner. @@ -503,7 +488,7 @@ public CloudSdk build() { return new CloudSdk(sdkPath, appCommandMetricsEnvironment, appCommandMetricsEnvironmentVersion, appCommandCredentialFile, appCommandOutputFormat, - processRunner, runDevAppServerWaitListener, stdOut); + processRunner, runDevAppServerWaitListener); } /** diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java index c56fbeb88..764babc0a 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/AccumulatingLineListener.java @@ -18,7 +18,7 @@ import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; -public class AccumulatingLineListener implements ProcessOutputLineListener { +class AccumulatingLineListener implements ProcessOutputLineListener { private StringBuilder output = new StringBuilder(); @@ -27,8 +27,7 @@ public void onOutputLine(String line) { output.append(line + "\n"); } - // todo: maybe this should be a standard part of ProcessRunner API instead? - public String getOutput() { + String getOutput() { return output.toString(); } diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java index d088e4582..816803a24 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/DefaultProcessRunner.java @@ -132,6 +132,56 @@ public void run(String[] command) throws ProcessRunnerException { throw new ProcessRunnerException(e); } } + + /** + * Executes a not-too-long-lived shell command synchornously and returns stdout. + * + *

If any output listeners were configured, output will go to them only. Otherwise, process + * output will be redirected to the caller via inheritIO. + * + * @param command the shell command to execute + * @return everything printed on stdout + */ + @Override + public String runSynchronously(String[] command) throws ProcessRunnerException { + try { + // Configure process builder. + final ProcessBuilder processBuilder = new ProcessBuilder(); + + // If there are no listeners, we might still want to redirect stdout and stderr to the parent + // process, or not. + if (stdErrLineListeners.isEmpty() && inheritProcessOutput) { + processBuilder.redirectError(Redirect.INHERIT); + } + if (environment != null) { + processBuilder.environment().putAll(environment); + } + + processBuilder.command(command); + + Process process = processBuilder.start(); + AccumulatingLineListener stdOut = new AccumulatingLineListener(); + stdOutLineListeners.add(stdOut); + // Only handle stdout or stderr if there are listeners. + handleStdOut(process); + if (!stdErrLineListeners.isEmpty()) { + handleErrOut(process); + } + + for (ProcessStartListener startListener : startListeners) { + startListener.onStart(process); + } + + shutdownProcessHook(process); + syncRun(process); + stdOutLineListeners.remove(stdOut); + + return stdOut.getOutput(); + + } catch (IOException | InterruptedException | IllegalThreadStateException e) { + throw new ProcessRunnerException(e); + } + } /** * Environment variables to append to the current system environment variables. diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java index f261a66f2..34c9ceb8d 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java @@ -22,6 +22,7 @@ public interface ProcessRunner { void run(String[] command) throws ProcessRunnerException; + String runSynchronously(String[] command) throws ProcessRunnerException; void setEnvironment(Map environment); diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java index 206358149..cb454d67a 100644 --- a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -74,7 +74,7 @@ public void testNewCloudSdk_outListener() { CloudSdk sdk = builder.build(); assertNotNull(sdk.getRunDevAppServerWaitListener()); - assertEquals(3, builder.getStdOutLineListeners().size()); + assertEquals(2, builder.getStdOutLineListeners().size()); assertEquals(1, builder.getStdErrLineListeners().size()); assertEquals(1, builder.getExitListeners().size()); } @@ -85,7 +85,7 @@ public void testNewCloudSdk_errListener() { CloudSdk sdk = builder.build(); assertNotNull(sdk.getRunDevAppServerWaitListener()); - assertEquals(2, builder.getStdOutLineListeners().size()); + assertEquals(1, builder.getStdOutLineListeners().size()); assertEquals(2, builder.getStdErrLineListeners().size()); assertEquals(1, builder.getExitListeners().size()); } From d1904ae93ce9fab0369a09917cd8d3efc3cc666c Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 8 Sep 2016 14:49:43 -0400 Subject: [PATCH 11/14] checkstyle fixes --- pom.xml | 10 +++++----- .../cloud/tools/appengine/cloudsdk/CloudSdk.java | 12 +++++++----- .../cloudsdk/internal/process/ProcessRunner.java | 1 + 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index d6595ca8f..cc6218b63 100644 --- a/pom.xml +++ b/pom.xml @@ -93,11 +93,11 @@ jsr305 3.0.1 - - org.json - json - 20160810 - + + org.json + json + 20160810 + diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 2c9f8c7d1..46f1634ba 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -30,6 +30,11 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONTokener; + import java.io.File; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -47,11 +52,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.json.JSONArray; -import org.json.JSONException; -import org.json.JSONObject; -import org.json.JSONTokener; - /** * Cloud SDK CLI wrapper. */ @@ -133,6 +133,8 @@ public void runAppCommand(List args) throws ProcessRunnerException { } /** + * Checks whether the specified component is installed in the local environment. + * * @return true iff the specified component is installed in the local environment */ public boolean isComponentInstalled(String id) throws ProcessRunnerException { diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java index 34c9ceb8d..440398040 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/internal/process/ProcessRunner.java @@ -22,6 +22,7 @@ public interface ProcessRunner { void run(String[] command) throws ProcessRunnerException; + String runSynchronously(String[] command) throws ProcessRunnerException; void setEnvironment(Map environment); From e21364a1930f7c49d57202702d1cb7ad74152459 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 9 Sep 2016 12:55:19 -0400 Subject: [PATCH 12/14] install cloud sdk for integration tests --- .travis.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.travis.yml b/.travis.yml index d7b848431..1cceb7d34 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,14 @@ sudo: false language: java jdk: - openjdk7 +install: + # download Cloud SDK + - wget https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-124.0.0-linux-x86_64.tar.gz + - tar -xzvf google-cloud-sdk-124.0.0-linux-x86_64.tar.gz + # update all Cloud SDK components + - gcloud components update --quiet + # add App Engine component to Cloud SDK + - gcloud components install app-engine-java --quiet cache: directories: - $HOME/.m2 From b6fc0edf545fa9c8312cc03e746baa222ddb574f Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Fri, 9 Sep 2016 14:08:25 -0400 Subject: [PATCH 13/14] add gcloud to path --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 1cceb7d34..d9d9ed2fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,5 +24,7 @@ script: env: global: + - PATH=$PWD/google-cloud-sdk/bin:$PATH + - CLOUDSDK_CORE_DISABLE_USAGE_REPORTING=true - secure: "da45ryIB/m37Gkpon/Uy6PPl1AhsUMdxgG7aeQIM1alh3Np4JwqKHobdP8tC0Qc/vb5fqfisEZEGPukh9Kxw95pRATO/QrQrsxYISMXUUc+Dlvo8WLeDq5F2LdUSn3933H9p5Mk8bKrZql+Jb+Il5S+B3Ib8uO+e3L4itrGKu5dw6i8TAxMggUjK8L6RuRYZeXMNiw3iaLjlHhNVEZ7F7RQ/gsHT2LhzybY6gfVJU+8AHhwEv+Tuapz5QCYTah6pwKqP+EQPhFlfrow9zdBfQm7m4h+uU+TB67VzZi46pAx+drC1quW+HZllutMHKM+cR13HsTET9qbFmV00cr2gZ3UXMVPX+PG1johZj0gs8s/S0+MVh6IQ0QLMWSqgJH/ULlyoU9PLE+PMs8A2qprO8NALuS3GXgvMQ8tHGuAzUVht/CxH5WTxW8nGFv8lwCIrT+m0hWi26gXWpNItC4N5GawoJQp+eWW0dO2Uko34kC3CIkLppRGxgzQWQQHkR+Hh+Yi3AzZsUzz30YRezF+G1RuQ68Va9yZ9qgei3h0Ppx2OIZ6fyiGg6Z8MHzOA8AULoe/Sz7CsfCiOyjTWmccRSl79wc0kpZwrF2qLO46LD3DgpGcfcKEB/cqcdwj/SA4QJoRcwUPSqy2eXb6ILhHA9UGlSBwluPZ2tSXDh2zxCL0=" - secure: "Z/Haq0MHdfBpJKntdYZFdK3uxUUk+jjiNzuuOt2hLa7P5XxpoHsboRykWSY41tWD5NnXfXoJvcHPASwhz2OesQcyNCCEHxsikkNRk+XUgFjyu8I3Ohm8Fya1k9gn+Xlz9uDBtoGN1e2SHhfEY+a9a5nGvv4aLVblBewt9J14su8fc4WSyDYggoZnUjB69DzIJrBL/ol0VOqlo8/vrcbWRbB/hlJ3QwKsFkehIhx5//GVdEJGqM2CZNJqG/bVLaSIoqQPqd6v3eyemnS2O5bXNIoNEq9yUWGSm1PfnagBYMzkHcqNrg5D1aYI+vFFeZe4PAsBaE6Xw/trCMtdRr5uDKXdNW8s8zwMKfOYru3zY9KTq2N5Fl/HgazGfimpmAjXqpH74+WaITmQmCZ6LuLb7hgltF+f2OY71bHJI7lGeuCraiwb6ECFIZiF9+FA6m0DXU6fCsajoThUtkRSpTMxDTAMzlrH/Kw2SzwEfuGk3evsb937pLUkL1kNxSc6GXhdyRrLTSrYgoqavjtNUN5S7v7MqrmO4R6UF1ZtJOAxWJ1a4W/kQP6S/36u8dZhCWbds78EocOH/+fsJ8vlCmTAnFEbtZkSpmFHjGrpPu0gVYdG+u96eAtKv90p2IF8bwCzFQRh1U25OlcxHKZfDgGzplR1Cy8DdJoNcH3KJKQd1Cw=" From 97da3d0ca188038263a7f8ad1b3adf10c563e2d2 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Mon, 12 Sep 2016 13:06:08 -0400 Subject: [PATCH 14/14] look for different file --- .../tools/appengine/cloudsdk/CloudSdk.java | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 46f1634ba..32ef36836 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -267,9 +267,12 @@ public Path getJarPath(String jarName) { } /** - * Checks whether the configured Cloud SDK Path is valid. + * Checks whether the Cloud SDK is installed in the supplied path + * and the App Engine Java Components have been installed. * - * @throws AppEngineException when there is a validation error + * @throws AppEngineComponentsNotInstalledException the App Engine Java + * components have not been installed + * @throws AppEngineException if a necessary component of the Cloud SDK cannot be found */ public void validate() throws AppEngineException { if (sdkPath == null) { @@ -289,24 +292,14 @@ public void validate() throws AppEngineException { + getDevAppServerPath() + "' is not a file."); } if (!Files.isDirectory(getJavaAppEngineSdkPath())) { - throw new AppEngineException( - "Validation Error: Java App Engine SDK location '" - + getJavaAppEngineSdkPath() + "' is not a directory."); + throw new AppEngineComponentsNotInstalledException( + "Validation Error: App Engine Java component not installed"); } if (!Files.isRegularFile(JAR_LOCATIONS.get(JAVA_TOOLS_JAR))) { throw new AppEngineException( "Validation Error: Java Tools jar location '" + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not a file."); } - try { - if (!isComponentInstalled("app-engine-java")) { - throw new AppEngineComponentsNotInstalledException( - "Validation Error: App Engine Java component not installed"); - } - } catch (ProcessRunnerException ex) { - throw new AppEngineException( - "Could not determine whether App Engine Java component is installed", ex); - } } @VisibleForTesting