From be1d504b9d859e7f3cbd7fa0411f7fa05c0428eb Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 8 Oct 2015 09:03:59 +0200 Subject: [PATCH 1/5] Overload RemoteGcsHelper.create to take projectId and key path --- .../gcloud/storage/RemoteGcsHelper.java | 64 ++++++++++++------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java index f4c9b22a47b5..9940ad57af27 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java @@ -92,46 +92,32 @@ public static String generateBucketName() { } /** - * Creates a {@code RemoteGcsHelper} object. + * Creates a {@code RemoteGcsHelper} object for the given project id and JSON key path. * + * @param projectId id of the project to be used for running the tests + * @param keyPath path to the JSON key to be used for running the tests * @param options creation options * @return A {@code RemoteGcsHelper} object for the provided options. - * @throws com.google.gcloud.storage.RemoteGcsHelper.GcsHelperException if environment variables - * {@code GCLOUD_TESTS_PROJECT_ID} and {@code GCLOUD_TESTS_KEY} are not set or if the file - * pointed by {@code GCLOUD_TESTS_KEY} does not exist + * @throws com.google.gcloud.storage.RemoteGcsHelper.GcsHelperException if the file pointed by + * {@code keyPath} does not exist */ - public static RemoteGcsHelper create(Option... options) throws GcsHelperException { + public static RemoteGcsHelper create(String projectId, String keyPath, Option... options) + throws GcsHelperException { boolean keyFromClassPath = false; Map, Option> optionsMap = Option.asImmutableMap(options); if (optionsMap.containsKey(KeyFromClasspath.class)) { keyFromClassPath = ((KeyFromClasspath) optionsMap.get(KeyFromClasspath.class)).keyFromClasspath(); } - String projectId = System.getenv(PROJECT_ID_ENV_VAR); - String stringKeyPath = System.getenv(PRIVATE_KEY_ENV_VAR); - if (projectId == null) { - String message = "Environment variable " + PROJECT_ID_ENV_VAR + " not set"; - if (log.isLoggable(Level.WARNING)) { - log.log(Level.WARNING, message); - } - throw new GcsHelperException(message); - } - if (stringKeyPath == null) { - String message = "Environment variable " + PRIVATE_KEY_ENV_VAR + " not set"; - if (log.isLoggable(Level.WARNING)) { - log.log(Level.WARNING, message); - } - throw new GcsHelperException(message); - } try { InputStream keyFileStream; if (keyFromClassPath) { - keyFileStream = RemoteGcsHelper.class.getResourceAsStream(stringKeyPath); + keyFileStream = RemoteGcsHelper.class.getResourceAsStream(keyPath); if (keyFileStream == null) { - throw new FileNotFoundException(stringKeyPath + " not found in classpath"); + throw new FileNotFoundException(keyPath + " not found in classpath"); } } else { - keyFileStream = new FileInputStream(stringKeyPath); + keyFileStream = new FileInputStream(keyPath); } StorageOptions storageOptions = StorageOptions.builder() .authCredentials(AuthCredentials.createForJson(keyFileStream)) @@ -151,6 +137,36 @@ public static RemoteGcsHelper create(Option... options) throws GcsHelperExceptio } } + /** + * Creates a {@code RemoteGcsHelper} object. Project id and path to JSON key are read from two + * environment variables: {@code GCLOUD_TESTS_PROJECT_ID} and {@code GCLOUD_TESTS_KEY}. + * + * @param options creation options + * @return A {@code RemoteGcsHelper} object for the provided options. + * @throws com.google.gcloud.storage.RemoteGcsHelper.GcsHelperException if environment variables + * {@code GCLOUD_TESTS_PROJECT_ID} and {@code GCLOUD_TESTS_KEY} are not set or if the file + * pointed by {@code GCLOUD_TESTS_KEY} does not exist + */ + public static RemoteGcsHelper create(Option... options) throws GcsHelperException { + String projectId = System.getenv(PROJECT_ID_ENV_VAR); + String keyPath = System.getenv(PRIVATE_KEY_ENV_VAR); + if (projectId == null) { + String message = "Environment variable " + PROJECT_ID_ENV_VAR + " not set"; + if (log.isLoggable(Level.WARNING)) { + log.log(Level.WARNING, message); + } + throw new GcsHelperException(message); + } + if (keyPath == null) { + String message = "Environment variable " + PRIVATE_KEY_ENV_VAR + " not set"; + if (log.isLoggable(Level.WARNING)) { + log.log(Level.WARNING, message); + } + throw new GcsHelperException(message); + } + return create(projectId, keyPath, options); + } + private static class DeleteBucketTask implements Callable { private Storage storage; From 385dd4efdd1a0d5a3be81bf4a5d89d87ca167449 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 8 Oct 2015 09:35:32 +0200 Subject: [PATCH 2/5] Add default RetryParams to storage options built from RemoteGcsHelper --- .../test/java/com/google/gcloud/storage/RemoteGcsHelper.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java index 9940ad57af27..b6e8daeec445 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelper.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableMap; import com.google.gcloud.AuthCredentials; +import com.google.gcloud.RetryParams; import com.google.gcloud.storage.RemoteGcsHelper.Option.KeyFromClasspath; import java.io.FileInputStream; @@ -122,6 +123,7 @@ public static RemoteGcsHelper create(String projectId, String keyPath, Option... StorageOptions storageOptions = StorageOptions.builder() .authCredentials(AuthCredentials.createForJson(keyFileStream)) .projectId(projectId) + .retryParams(RetryParams.getDefaultInstance()) .build(); return new RemoteGcsHelper(storageOptions); } catch (FileNotFoundException ex) { From c23b9a230a34af229d2bde99036a2bcd64505b04 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 8 Oct 2015 09:56:26 +0200 Subject: [PATCH 3/5] Check storage not null before deleting bucket in ITStorageTest --- .../src/test/java/com/google/gcloud/storage/ITStorageTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java index d2056e8cbbb1..f4cf11d9a73f 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java @@ -66,7 +66,7 @@ public static void beforeClass() { @AfterClass public static void afterClass() throws ExecutionException, TimeoutException, InterruptedException { - if (!RemoteGcsHelper.forceDelete(storage, bucket, 5, TimeUnit.SECONDS)) { + if (storage != null && !RemoteGcsHelper.forceDelete(storage, bucket, 5, TimeUnit.SECONDS)) { if (log.isLoggable(Level.WARNING)) { log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", bucket); } From e1a50bc0063c25a4bfa729c8b305f06e061f0497 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 8 Oct 2015 09:28:38 -0700 Subject: [PATCH 4/5] Add scheme to user-provided host if necessary --- .../gcloud/spi/DefaultDatastoreRpc.java | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java index 2f245260b325..8fb9a1308565 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java @@ -40,6 +40,10 @@ import org.json.JSONObject; import org.json.JSONTokener; +import java.net.InetAddress; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.UnknownHostException; import java.util.HashMap; import java.util.Map; @@ -62,14 +66,48 @@ public class DefaultDatastoreRpc implements DatastoreRpc { } public DefaultDatastoreRpc(DatastoreOptions options) { + String normalizedHost = normalizeHost(options.host()); client = DatastoreFactory.get().create( new Builder() .dataset(options.projectId()) - .host(options.host()) + .host(normalizedHost) .initializer(options.httpRequestInitializer()) .build()); } + private static String normalizeHost(String host) { + host = host.toLowerCase(); + if (includesScheme(host)) { + if (host.startsWith("https://") && isLocalHost(host)) { + throw new IllegalArgumentException( + "\"https\" is not supported for localhost. Use \"http\" instead."); + } else { + return host; + } + } + return "http://" + host; + } + + private static boolean isLocalHost(String host) { + if (host != null) { + try { + String normalizedHost = host; + if (!includesScheme(normalizedHost)) { + normalizedHost = "http://" + normalizedHost; + } + InetAddress hostAddr = InetAddress.getByName(new URL(normalizedHost).getHost()); + return hostAddr.isAnyLocalAddress() || hostAddr.isLoopbackAddress(); + } catch (UnknownHostException | MalformedURLException e) { + // ignore + } + } + return false; + } + + private static boolean includesScheme(String url) { + return url.startsWith("http://") || url.startsWith("https://"); + } + private static DatastoreRpcException translate(DatastoreException exception) { String message = exception.getMessage(); String reasonStr = ""; From b8923f4765277e8420d31ca2b88d312f8d58061d Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 8 Oct 2015 10:26:52 -0700 Subject: [PATCH 5/5] simplify logic in host normalization --- .../com/google/gcloud/spi/DefaultDatastoreRpc.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java index 8fb9a1308565..e20bd9a3f9d6 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java @@ -32,6 +32,7 @@ import com.google.api.services.datastore.client.DatastoreException; import com.google.api.services.datastore.client.DatastoreFactory; import com.google.api.services.datastore.client.DatastoreOptions.Builder; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.gcloud.datastore.DatastoreOptions; import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; @@ -78,12 +79,9 @@ public DefaultDatastoreRpc(DatastoreOptions options) { private static String normalizeHost(String host) { host = host.toLowerCase(); if (includesScheme(host)) { - if (host.startsWith("https://") && isLocalHost(host)) { - throw new IllegalArgumentException( - "\"https\" is not supported for localhost. Use \"http\" instead."); - } else { - return host; - } + Preconditions.checkArgument(!(host.startsWith("https://") && isLocalHost(host)), + "\"https\" is not supported for localhost. Use \"http\" instead."); + return host; } return "http://" + host; }