From 05375c0b9b6f9fde2e6cefb1af6a695aa3b01732 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 18 Nov 2021 17:03:56 -0500 Subject: [PATCH] fix: update StorageOptions to not overwrite any previously set host (#1142) 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: * https://github.com/googleapis/google-cloud-java/pull/6579 * https://github.com/googleapis/google-cloud-java/issues/7004 * https://github.com/googleapis/google-cloud-java/pull/7034 --- .../google/cloud/storage/StorageOptions.java | 2 +- .../cloud/storage/StorageOptionsTest.java | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) 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 index a15a90381..f3de842fb 100644 --- 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 index 18ff0343c..5c9c708c1 100644 --- 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"); + } }