From e56181df8b010dc22f9edf426dcfeee54d772127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Wed, 13 Jul 2016 14:46:05 -0400 Subject: [PATCH 1/6] Make getSdkPath public for testing. --- .../google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 b31cdc217..4f49a5e98 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 @@ -25,6 +25,7 @@ import com.google.cloud.tools.appengine.cloudsdk.process.ProcessExitListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessStartListener; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.Maps; @@ -175,7 +176,7 @@ private void logCommand(List command) { logger.info("submitting command: " + WHITESPACE_JOINER.join(command)); } - private Path getSdkPath() { + @VisibleForTesting public Path getSdkPath() { return sdkPath; } @@ -195,7 +196,7 @@ public Path getJavaAppEngineSdkPath() { return getSdkPath().resolve(JAVA_APPENGINE_SDK_PATH); } - private Path getJavaToolsJar() { + public Path getJavaToolsJar() { return getJavaAppEngineSdkPath().resolve(JAVA_TOOLS_JAR); } @@ -253,9 +254,9 @@ public static class Builder { * The home directory of Google Cloud SDK. If not set, will attempt to look for the SDK in known * install locations. */ - public Builder sdkPath(File sdkPathFile) { + public Builder sdkPath(Path sdkPathFile) { if (sdkPathFile != null) { - this.sdkPath = sdkPathFile.toPath(); + this.sdkPath = sdkPathFile; } return this; } From 0fff81ee0dbc71bef152601cf5417bd2c6c25924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Thu, 14 Jul 2016 09:58:26 -0400 Subject: [PATCH 2/6] Removes VisibleForTesting and moves comment. --- .../google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 4f49a5e98..c326f9936 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 @@ -25,7 +25,6 @@ import com.google.cloud.tools.appengine.cloudsdk.process.ProcessExitListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessStartListener; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.Maps; @@ -176,7 +175,7 @@ private void logCommand(List command) { logger.info("submitting command: " + WHITESPACE_JOINER.join(command)); } - @VisibleForTesting public Path getSdkPath() { + public Path getSdkPath() { return sdkPath; } @@ -251,8 +250,7 @@ public static class Builder { private int runDevAppServerWaitSeconds; /** - * The home directory of Google Cloud SDK. If not set, will attempt to look for the SDK in known - * install locations. + * The home directory of Google Cloud SDK. */ public Builder sdkPath(Path sdkPathFile) { if (sdkPathFile != null) { @@ -353,6 +351,9 @@ public Builder runDevAppServerWait(int runDevAppServerWaitSeconds) { /** * Create a new instance of {@link CloudSdk}. + * + * If {@code sdKPath} is not set, this method will attempt to look for the SDK in known install + * locations. */ public CloudSdk build() { From 6aba4449698cdbff66b92902ecea207e460f894b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Thu, 14 Jul 2016 11:17:20 -0400 Subject: [PATCH 3/6] Fixes several minor issues. --- .../cloud/tools/appengine/cloudsdk/CloudSdk.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 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 c326f9936..7943049d6 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 @@ -251,10 +251,12 @@ public static class Builder { /** * The home directory of Google Cloud SDK. + * + * @param sdkPath the root path for the Cloud SDK */ - public Builder sdkPath(Path sdkPathFile) { - if (sdkPathFile != null) { - this.sdkPath = sdkPathFile; + public Builder sdkPath(Path sdkPath) { + if (sdkPath != null) { + this.sdkPath = sdkPath; } return this; } @@ -352,7 +354,7 @@ public Builder runDevAppServerWait(int runDevAppServerWaitSeconds) { /** * Create a new instance of {@link CloudSdk}. * - * If {@code sdKPath} is not set, this method will attempt to look for the SDK in known install + *

If {@code sdKPath} is not set, this method will look for the SDK in known install * locations. */ public CloudSdk build() { From 3b4754d41623a2d3df6390ffc5e863dc5d794d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Fri, 15 Jul 2016 11:52:54 -0400 Subject: [PATCH 4/6] Fixes an issue with resolving paths in CloudSdk and adds unit tests. --- .../tools/appengine/cloudsdk/CloudSdk.java | 29 ++++++++++---- .../appengine/cloudsdk/CloudSdkTest.java | 40 +++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java 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 7943049d6..84676f465 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 @@ -32,6 +32,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.logging.Logger; @@ -52,6 +53,7 @@ public class CloudSdk { "platform/google_appengine/google/appengine/tools/java/lib"; private static final String JAVA_TOOLS_JAR = "appengine-tools-api.jar"; private static final String WINDOWS_BUNDLED_PYTHON = "platform/bundledpython/python.exe"; + private static final Map JAR_LOCATIONS = new HashMap<>(); private final Path sdkPath; private final ProcessRunner processRunner; @@ -93,6 +95,12 @@ private CloudSdk(Path sdkPath, String appCommandMetricsEnvironment, this.processRunner = new DefaultProcessRunner(async, stdOutLineListeners, stdErrLineListeners, exitListeners, startListeners); + // Populate jar locations. + JAR_LOCATIONS.put("servlet-api.jar", + getJavaAppEngineSdkPath().resolve("shared/servlet-api.jar")); + JAR_LOCATIONS.put("jsp-api.jar", getJavaAppEngineSdkPath().resolve("shared/jsp-api.jar")); + JAR_LOCATIONS.put(JAVA_TOOLS_JAR, + sdkPath.resolve(JAVA_APPENGINE_SDK_PATH).resolve(JAVA_TOOLS_JAR)); } /** @@ -162,7 +170,7 @@ public void runAppCfgCommand(List args) throws ProcessRunnerException { command.add( Paths.get(System.getProperty("java.home")).resolve("bin/java").toAbsolutePath().toString()); command.add("-cp"); - command.add(getJavaToolsJar().toAbsolutePath().toString()); + command.add(JAR_LOCATIONS.get(JAVA_TOOLS_JAR).toString()); command.add("com.google.appengine.tools.admin.AppCfg"); command.addAll(args); @@ -195,14 +203,20 @@ public Path getJavaAppEngineSdkPath() { return getSdkPath().resolve(JAVA_APPENGINE_SDK_PATH); } - public Path getJavaToolsJar() { - return getJavaAppEngineSdkPath().resolve(JAVA_TOOLS_JAR); - } - private Path getWindowsPythonPath() { return getSdkPath().resolve(WINDOWS_BUNDLED_PYTHON); } + /** + * Gets the file system location for an SDK jar. + * + * @param jarName the jar file name. For example, "servlet-api.jar" + * @return the path in the file system + */ + public Path getJarPath(String jarName) { + return JAR_LOCATIONS.get(jarName); + } + /** * Checks whether the configured Cloud SDK Path is valid. * @@ -229,9 +243,10 @@ public void validate() throws AppEngineException { "Validation Error: Java App Engine SDK path '" + getJavaAppEngineSdkPath() + "' is not valid"); } - if (!getJavaToolsJar().toFile().isFile()) { + if (!JAR_LOCATIONS.get(JAVA_TOOLS_JAR).toFile().isFile()) { throw new AppEngineException( - "Validation Error: Java Tools jar path '" + getJavaToolsJar() + "' is not valid"); + "Validation Error: Java Tools jar path '" + + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not valid"); } } 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 new file mode 100644 index 000000000..bb7f6739d --- /dev/null +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -0,0 +1,40 @@ +package com.google.cloud.tools.appengine.cloudsdk; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; + +import java.nio.file.Path; +import java.nio.file.Paths; + +/** + * Unit tests for {@link CloudSdk} + */ +@RunWith(MockitoJUnitRunner.class) +public class CloudSdkTest { + @Test + public void testGetSdkPath() { + Path location = Paths.get("/"); + CloudSdk sdk = new CloudSdk.Builder().sdkPath(location).build(); + assertEquals(location, sdk.getSdkPath()); + } + + @Test + public void testGetJavaAppEngineSdkPath() { + Path location = Paths.get("/"); + CloudSdk sdk = new CloudSdk.Builder().sdkPath(location).build(); + assertEquals(location.resolve("platform/google_appengine/google/appengine/tools/java/lib"), + sdk.getJavaAppEngineSdkPath()); + } + + @Test + public void testGetJarPathJavaTools() { + Path location = Paths.get("/"); + CloudSdk sdk = new CloudSdk.Builder().sdkPath(location).build(); + assertEquals(Paths.get("/platform/google_appengine/google/appengine" + + "/tools/java/lib/appengine-tools-api.jar"), + sdk.getJarPath("appengine-tools-api.jar")); + } +} From 2e3db75adee1ba79dcc0a56885e4a14f3f9e9e49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Fri, 15 Jul 2016 14:40:45 -0400 Subject: [PATCH 5/6] Changed error messages. Replaced toFile() with Files.isDirectory()/isRegularFile(). --- .../tools/appengine/cloudsdk/CloudSdk.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 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 84676f465..098bf131e 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 @@ -29,6 +29,7 @@ import com.google.common.collect.Maps; import java.io.File; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -96,6 +97,8 @@ private CloudSdk(Path sdkPath, String appCommandMetricsEnvironment, exitListeners, startListeners); // Populate jar locations. + // TODO(joaomartins): Consider case where SDK doesn't contain these jars. Only App Engine + // SDK does. JAR_LOCATIONS.put("servlet-api.jar", getJavaAppEngineSdkPath().resolve("shared/servlet-api.jar")); JAR_LOCATIONS.put("jsp-api.jar", getJavaAppEngineSdkPath().resolve("shared/jsp-api.jar")); @@ -226,27 +229,28 @@ public void validate() throws AppEngineException { if (sdkPath == null) { throw new AppEngineException("Validation Error: SDK path is null"); } - if (!sdkPath.toFile().isDirectory()) { + if (!Files.isDirectory(sdkPath)) { throw new AppEngineException( - "Validation Error: SDK directory '" + sdkPath + "' is not valid"); + "Validation Error: SDK location '" + sdkPath + "' is a directory."); } - if (!getGCloudPath().toFile().isFile()) { + if (!Files.isRegularFile(getGCloudPath())) { throw new AppEngineException( - "Validation Error: gcloud path '" + getGCloudPath() + "' is not valid"); + "Validation Error: gcloud location '" + getGCloudPath() + "' is not a file."); } - if (!getDevAppServerPath().toFile().isFile()) { + if (!Files.isRegularFile(getDevAppServerPath())) { throw new AppEngineException( - "Validation Error: dev_appserver.py path '" + getDevAppServerPath() + "' is not valid"); + "Validation Error: dev_appserver.py location '" + + getDevAppServerPath() + "' is not a file."); } - if (!getJavaAppEngineSdkPath().toFile().isDirectory()) { + if (!Files.isDirectory(getJavaAppEngineSdkPath())) { throw new AppEngineException( - "Validation Error: Java App Engine SDK path '" + getJavaAppEngineSdkPath() - + "' is not valid"); + "Validation Error: Java App Engine SDK location '" + + getJavaAppEngineSdkPath() + "' is not a directory."); } - if (!JAR_LOCATIONS.get(JAVA_TOOLS_JAR).toFile().isFile()) { + if (!Files.isRegularFile(JAR_LOCATIONS.get(JAVA_TOOLS_JAR))) { throw new AppEngineException( - "Validation Error: Java Tools jar path '" - + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not valid"); + "Validation Error: Java Tools jar location '" + + JAR_LOCATIONS.get(JAVA_TOOLS_JAR) + "' is not a file."); } } From f2f298ece23e52a398f36fb142e9edd46d633863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Andr=C3=A9=20Martins?= Date: Fri, 15 Jul 2016 14:53:14 -0400 Subject: [PATCH 6/6] Fixes message typos. --- .../com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 4 ++-- 1 file changed, 2 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 098bf131e..6603cb166 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 @@ -231,7 +231,7 @@ public void validate() throws AppEngineException { } if (!Files.isDirectory(sdkPath)) { throw new AppEngineException( - "Validation Error: SDK location '" + sdkPath + "' is a directory."); + "Validation Error: SDK location '" + sdkPath + "' is not a directory."); } if (!Files.isRegularFile(getGCloudPath())) { throw new AppEngineException( @@ -373,7 +373,7 @@ public Builder runDevAppServerWait(int runDevAppServerWaitSeconds) { /** * Create a new instance of {@link CloudSdk}. * - *

If {@code sdKPath} is not set, this method will look for the SDK in known install + *

If {@code sdkPath} is not set, this method will look for the SDK in known install * locations. */ public CloudSdk build() {