fix: update StorageOptions to not overwrite any previously set host (#1142)

BenWhitehead · web-flow · commit 05375c0b9b6f · 2021-11-18T17:03:56.000-05:00
When calling `StorageOptions#toBuilder()` after creating the builder the host was explicitly set to the default host. This change removes that behavior instead moving the setting of the default host to the instantiation of a new builder. Add unit tests to ensure host is set to the expected values based on different ways of creating an instance of StorageOptions. Related: * googleapis/google-cloud-java#6579 * googleapis/google-cloud-java#7004 * googleapis/google-cloud-java#7034
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java
@@ -166,7 +166,7 @@ public static StorageOptions getUnauthenticatedInstance() {
   @SuppressWarnings("unchecked")
   @Override
   public Builder toBuilder() {
-    return new Builder(this).setHost(DEFAULT_HOST);
+    return new Builder(this);
   }
 
   @Override
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java
@@ -16,6 +16,8 @@
 
 package com.google.cloud.storage;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import com.google.cloud.TransportOptions;
 import org.easymock.EasyMock;
 import org.junit.Assert;
@@ -33,4 +35,34 @@ public void testInvalidTransport() {
       Assert.assertNotNull(ex.getMessage());
     }
   }
+
+  @Test
+  public void testConfigureHostShouldBeKeptOnToBuilder() {
+    StorageOptions opts1 = StorageOptions.newBuilder().setHost("custom-host").build();
+    StorageOptions opts2 = opts1.toBuilder().build();
+
+    assertThat(opts2.getHost()).isEqualTo("custom-host");
+  }
+
+  @Test
+  public void testToBuilderShouldSpecifyDefaultIfNotOtherwiseSet() {
+    StorageOptions opts1 = StorageOptions.newBuilder().build();
+    StorageOptions opts2 = opts1.toBuilder().build();
+
+    assertThat(opts2.getHost()).isEqualTo("https://storage.googleapis.com");
+  }
+
+  @Test
+  public void testNewBuilderSpecifiesCorrectHost() {
+    StorageOptions opts1 = StorageOptions.newBuilder().build();
+
+    assertThat(opts1.getHost()).isEqualTo("https://storage.googleapis.com");
+  }
+
+  @Test
+  public void testDefaultInstanceSpecifiesCorrectHost() {
+    StorageOptions opts1 = StorageOptions.getDefaultInstance();
+
+    assertThat(opts1.getHost()).isEqualTo("https://storage.googleapis.com");
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -166,7 +166,7 @@ public static StorageOptions getUnauthenticatedInstance() {`
`166`	`166`	`@SuppressWarnings("unchecked")`
`167`	`167`	`@Override`
`168`	`168`	`public Builder toBuilder() {`
`169`		`- return new Builder(this).setHost(DEFAULT_HOST);`
	`169`	`+ return new Builder(this);`
`170`	`170`	`}`
`171`	`171`
`172`	`172`	`@Override`