From 9c19934a6170232f6ac2478ef9bfcdb2914d2562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 10 Nov 2023 11:51:33 +0100 Subject: [PATCH] fix: respect SPANNER_EMULATOR_HOST env var when autoConfigEmulator=true (#2730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: respect SPANNER_EMULATOR_HOST env var when autoConfigEmulator=true The Connection API would always use the default emulator host when autoConfigEmulator=true was set in the connection string. This makes it harder to override the host name for the emulator when you also want the emulator to automatically create the instance and database that you connect to, as the only way to set it is to add it to the connection string. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot --- README.md | 8 +- .../spanner/connection/ConnectionOptions.java | 23 +++- .../connection/ConnectionOptionsTest.java | 103 ++++++++++++++++++ 3 files changed, 125 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 9f5b837c996..73d80d9d362 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.26.0') +implementation platform('com.google.cloud:libraries-bom:26.27.0') implementation 'com.google.cloud:google-cloud-spanner' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-spanner:6.52.1' +implementation 'com.google.cloud:google-cloud-spanner:6.53.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.52.1" +libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.53.0" ``` @@ -432,7 +432,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-spanner.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.52.1 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.53.0 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index 47397100acd..e8c2855af50 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -48,6 +48,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.regex.Matcher; @@ -191,6 +192,7 @@ public String[] getValidValues() { private static final String PLAIN_TEXT_PROTOCOL = "http:"; private static final String HOST_PROTOCOL = "https:"; private static final String DEFAULT_HOST = "https://spanner.googleapis.com"; + private static final String SPANNER_EMULATOR_HOST_ENV_VAR = "SPANNER_EMULATOR_HOST"; private static final String DEFAULT_EMULATOR_HOST = "http://localhost:9010"; /** Use plain text is only for local testing purposes. */ private static final String USE_PLAIN_TEXT_PROPERTY_NAME = "usePlainText"; @@ -473,7 +475,10 @@ private Builder() {} "(?:cloudspanner:)(?//[\\w.-]+(?:\\.[\\w\\.-]+)*[\\w\\-\\._~:/?#\\[\\]@!\\$&'\\(\\)\\*\\+,;=.]+)?/projects/(?(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?([a-z]|[-]|[0-9])+)(/databases/(?([a-z]|[-]|[_]|[0-9])+))?)?(?:[?|;].*)?"; private static final String SPANNER_URI_REGEX = "(?is)^" + SPANNER_URI_FORMAT + "$"; - private static final Pattern SPANNER_URI_PATTERN = Pattern.compile(SPANNER_URI_REGEX); + + @VisibleForTesting + static final Pattern SPANNER_URI_PATTERN = Pattern.compile(SPANNER_URI_REGEX); + private static final String HOST_GROUP = "HOSTGROUP"; private static final String PROJECT_GROUP = "PROJECTGROUP"; private static final String INSTANCE_GROUP = "INSTANCEGROUP"; @@ -706,7 +711,7 @@ private ConnectionOptions(Builder builder) { this.autoConfigEmulator = parseAutoConfigEmulator(this.uri); this.dialect = parseDialect(this.uri); this.usePlainText = this.autoConfigEmulator || parseUsePlainText(this.uri); - this.host = determineHost(matcher, autoConfigEmulator, usePlainText); + this.host = determineHost(matcher, autoConfigEmulator, usePlainText, System.getenv()); this.rpcPriority = parseRPCPriority(this.uri); this.delayTransactionStartUntilFirstWrite = parseDelayTransactionStartUntilFirstWrite(this.uri); this.trackSessionLeaks = parseTrackSessionLeaks(this.uri); @@ -791,11 +796,19 @@ private ConnectionOptions(Builder builder) { } } - private static String determineHost( - Matcher matcher, boolean autoConfigEmulator, boolean usePlainText) { + @VisibleForTesting + static String determineHost( + Matcher matcher, + boolean autoConfigEmulator, + boolean usePlainText, + Map environment) { if (matcher.group(Builder.HOST_GROUP) == null) { if (autoConfigEmulator) { - return DEFAULT_EMULATOR_HOST; + if (Strings.isNullOrEmpty(environment.get(SPANNER_EMULATOR_HOST_ENV_VAR))) { + return DEFAULT_EMULATOR_HOST; + } else { + return PLAIN_TEXT_PROTOCOL + "//" + environment.get(SPANNER_EMULATOR_HOST_ENV_VAR); + } } else { return DEFAULT_HOST; } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java index 7690ea3934c..ca0b779981f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java @@ -16,6 +16,8 @@ package com.google.cloud.spanner.connection; +import static com.google.cloud.spanner.connection.ConnectionOptions.Builder.SPANNER_URI_PATTERN; +import static com.google.cloud.spanner.connection.ConnectionOptions.determineHost; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -33,6 +35,7 @@ import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerOptions; +import com.google.common.collect.ImmutableMap; import com.google.common.io.BaseEncoding; import com.google.common.io.Files; import java.io.File; @@ -40,6 +43,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Objects; +import java.util.regex.Matcher; import org.junit.Test; import org.junit.function.ThrowingRunnable; import org.junit.runner.RunWith; @@ -153,6 +157,105 @@ public void testBuildWithAutoConfigEmulator() { assertTrue(options.isUsePlainText()); } + @Test + public void testDetermineHost() { + final String uriWithoutHost = + "cloudspanner:/projects/test-project-123/instances/test-instance-123/databases/test-database-123"; + Matcher matcherWithoutHost = SPANNER_URI_PATTERN.matcher(uriWithoutHost); + assertTrue(matcherWithoutHost.find()); + final String uriWithHost = + "cloudspanner://custom.host.domain:1234/projects/test-project-123/instances/test-instance-123/databases/test-database-123"; + Matcher matcherWithHost = SPANNER_URI_PATTERN.matcher(uriWithHost); + assertTrue(matcherWithHost.find()); + + assertEquals( + DEFAULT_HOST, + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ false, + ImmutableMap.of())); + assertEquals( + DEFAULT_HOST, + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ false, + ImmutableMap.of("FOO", "bar"))); + assertEquals( + "http://localhost:9010", + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ false, + ImmutableMap.of())); + assertEquals( + "http://localhost:9011", + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ false, + ImmutableMap.of("SPANNER_EMULATOR_HOST", "localhost:9011"))); + assertEquals( + "http://localhost:9010", + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ true, + ImmutableMap.of())); + assertEquals( + "http://localhost:9011", + determineHost( + matcherWithoutHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ true, + ImmutableMap.of("SPANNER_EMULATOR_HOST", "localhost:9011"))); + + // A host in the connection string has precedence over all other options. + assertEquals( + "https://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ false, + ImmutableMap.of())); + assertEquals( + "http://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ true, + ImmutableMap.of())); + assertEquals( + "http://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ true, + ImmutableMap.of())); + assertEquals( + "https://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ false, + ImmutableMap.of())); + assertEquals( + "http://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ false, + /* usePlainText= */ true, + ImmutableMap.of("SPANNER_EMULATOR_HOST", "localhost:9011"))); + assertEquals( + "https://custom.host.domain:1234", + determineHost( + matcherWithHost, + /* autoConfigEmulator= */ true, + /* usePlainText= */ false, + ImmutableMap.of("SPANNER_EMULATOR_HOST", "localhost:9011"))); + } + @Test public void testBuildWithRouteToLeader() { final String BASE_URI =