update as per feedback

googleapis · Nov 17, 2018 · b16a326 · b16a326
1 parent c405eb7
commit b16a326
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 45 deletions.
diff --git a/...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/...le-cloud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
@@ -892,7 +892,7 @@ class SignUrlOption implements Serializable {
     private final Object value;
 
     enum Option {
-      HTTP_METHOD, CONTENT_TYPE, MD5, EXT_HEADERS, SERVICE_ACCOUNT_CRED,SERVICE_ENDPOINT
+      HTTP_METHOD, CONTENT_TYPE, MD5, EXT_HEADERS, SERVICE_ACCOUNT_CRED, HOST_NAME
     }
 
     private SignUrlOption(Option option, Object value) {
@@ -955,10 +955,10 @@ public static SignUrlOption signWith(ServiceAccountSigner signer) {
     }
 
     /**
-    * Provides a host name to sign the URL. If not provided than host name will be default 
+    * Provides a host name to sign the URL. If not provided than default host name will be used. 
     */ 
     public static SignUrlOption withHostName(String hostName){
-      return new SignUrlOption(Option.SERVICE_ENDPOINT, hostName);	
+      return new SignUrlOption(Option.HOST_NAME, hostName);	
     }
   }
 
@@ -2114,7 +2114,7 @@ public static Builder newBuilder() {
    *     granularity supported is 1 second, finer granularities will be truncated
    * @param unit time unit of the {@code duration} parameter
    * @param options optional URL signing options
-   *        {@code SignUrlOption.withExtHostName()} option is used for external host name of signed url
+   *     {@code SignUrlOption.withHostName()} option to sign url with custom hostname.
    * @throws IllegalStateException if {@link SignUrlOption#signWith(ServiceAccountSigner)} was not
    *     used and no implementation of {@link ServiceAccountSigner} was provided to
    *     {@link StorageOptions}

diff --git a/...loud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/...loud-clients/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
@@ -84,7 +84,10 @@ final class StorageImpl extends BaseService<StorageOptions> implements Storage {
   private static final String EMPTY_BYTE_ARRAY_MD5 = "1B2M2Y8AsgTpgAmY7PhCfg==";
   private static final String EMPTY_BYTE_ARRAY_CRC32C = "AAAAAA==";
   private static final String PATH_DELIMITER = "/";
-  private static final String DEFAULT_STORAGE_HOST = "https://storage.googleapis.com";
+  /**
+   * SignedUrls uses GCS XML API endpoint.
+   */
+  private static final String STORAGE_XML_HOST_NAME = "https://storage.googleapis.com";
 
   private static final Function<Tuple<Storage, Boolean>, Boolean> DELETE_FUNCTION =
       new Function<Tuple<Storage, Boolean>, Boolean>() {
@@ -537,11 +540,11 @@ public URL signUrl(BlobInfo blobInfo, long duration, TimeUnit unit, SignUrlOptio
       byte[] signatureBytes =
           credentials.sign(signatureInfo.constructUnsignedPayload().getBytes(UTF_8));
       StringBuilder stBuilder = new StringBuilder();
-      if (optionMap.get(SignUrlOption.Option.SERVICE_ENDPOINT) == null){
-    	stBuilder.append(DEFAULT_STORAGE_HOST).append(path);
+      if (optionMap.get(SignUrlOption.Option.HOST_NAME) == null) {
+    	stBuilder.append(STORAGE_XML_HOST_NAME).append(path);
       }
       else {
-    	stBuilder.append(optionMap.get(SignUrlOption.Option.SERVICE_ENDPOINT)).append(path);
+    	stBuilder.append(optionMap.get(SignUrlOption.Option.HOST_NAME)).append(path);
       }
 
       String signature =

diff --git a/...-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java b/...-clients/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageImplTest.java
@@ -26,6 +26,37 @@
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
+import com.google.api.client.googleapis.json.GoogleJsonError;
+import com.google.api.core.ApiClock;
+import com.google.api.gax.paging.Page;
+import com.google.api.services.storage.model.Policy.Bindings;
+import com.google.api.services.storage.model.StorageObject;
+import com.google.api.services.storage.model.TestIamPermissionsResponse;
+import com.google.auth.oauth2.ServiceAccountCredentials;
+import com.google.cloud.Identity;
+import com.google.cloud.Policy;
+import com.google.cloud.ReadChannel;
+import com.google.cloud.ServiceOptions;
+import com.google.cloud.Tuple;
+import com.google.cloud.WriteChannel;
+import com.google.cloud.storage.Acl.Project;
+import com.google.cloud.storage.Acl.Project.ProjectRole;
+import com.google.cloud.storage.Acl.Role;
+import com.google.cloud.storage.Acl.User;
+import com.google.cloud.storage.Storage.BlobSourceOption;
+import com.google.cloud.storage.Storage.BlobTargetOption;
+import com.google.cloud.storage.Storage.BlobWriteOption;
+import com.google.cloud.storage.Storage.BucketSourceOption;
+import com.google.cloud.storage.Storage.CopyRequest;
+import com.google.cloud.storage.spi.StorageRpcFactory;
+import com.google.cloud.storage.spi.v1.RpcBatch;
+import com.google.cloud.storage.spi.v1.StorageRpc;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.common.io.BaseEncoding;
+import com.google.common.net.UrlEscapers;
+
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
@@ -60,37 +91,6 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import com.google.api.client.googleapis.json.GoogleJsonError;
-import com.google.api.core.ApiClock;
-import com.google.api.gax.paging.Page;
-import com.google.api.services.storage.model.Policy.Bindings;
-import com.google.api.services.storage.model.StorageObject;
-import com.google.api.services.storage.model.TestIamPermissionsResponse;
-import com.google.auth.oauth2.ServiceAccountCredentials;
-import com.google.cloud.Identity;
-import com.google.cloud.Policy;
-import com.google.cloud.ReadChannel;
-import com.google.cloud.ServiceOptions;
-import com.google.cloud.Tuple;
-import com.google.cloud.WriteChannel;
-import com.google.cloud.storage.Acl.Project;
-import com.google.cloud.storage.Acl.Project.ProjectRole;
-import com.google.cloud.storage.Acl.Role;
-import com.google.cloud.storage.Acl.User;
-import com.google.cloud.storage.Storage.BlobSourceOption;
-import com.google.cloud.storage.Storage.BlobTargetOption;
-import com.google.cloud.storage.Storage.BlobWriteOption;
-import com.google.cloud.storage.Storage.BucketSourceOption;
-import com.google.cloud.storage.Storage.CopyRequest;
-import com.google.cloud.storage.spi.StorageRpcFactory;
-import com.google.cloud.storage.spi.v1.RpcBatch;
-import com.google.cloud.storage.spi.v1.StorageRpc;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-import com.google.common.io.BaseEncoding;
-import com.google.common.net.UrlEscapers;
-
 public class StorageImplTest {
 
   private static final String BUCKET_NAME1 = "b1";
@@ -1638,7 +1638,7 @@ public void testSignUrl()
   }
 
   @Test
-  public void testSignUrlWithCustomUrl()
+  public void testSignUrlWithHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     EasyMock.replay(storageRpcMock);
@@ -1721,7 +1721,7 @@ public void testSignUrlLeadingSlash()
   }
 
   @Test
-  public void testSignUrlLeadingSlashWithCustomUrl()
+  public void testSignUrlLeadingSlashWithHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     String blobName = "/b1";
@@ -1815,7 +1815,7 @@ public void testSignUrlWithOptions()
   }
 
   @Test
-  public void testSignUrlWithOptionsAndCustomUrl()
+  public void testSignUrlWithOptionsAndHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     EasyMock.replay(storageRpcMock);
@@ -1920,7 +1920,7 @@ public void testSignUrlForBlobWithSpecialChars()
   }
 
   @Test
-  public void testSignUrlForBlobWithSpecialCharsAndCustomUrl()
+  public void testSignUrlForBlobWithSpecialCharsAndHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     // List of chars under test were taken from
@@ -2029,7 +2029,7 @@ public void testSignUrlWithExtHeaders()
   }
 
   @Test
-  public void testSignUrlWithExtHeadersAndCustomUrl()
+  public void testSignUrlWithExtHeadersAndHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     EasyMock.replay(storageRpcMock);
@@ -2130,7 +2130,7 @@ public void testSignUrlForBlobWithSlashes()
   }
 
   @Test
-  public void testSignUrlForBlobWithSlashesAndCustomUrl()
+  public void testSignUrlForBlobWithSlashesAndHostName()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           UnsupportedEncodingException {
     EasyMock.replay(storageRpcMock);