Skip to content

Commit

Permalink
fix: respect SPANNER_EMULATOR_HOST env var when autoConfigEmulator=tr…
Browse files Browse the repository at this point in the history 
…ue (#2730)

* 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 <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
@olavloite @gcf-owl-bot
olavloite and gcf-owl-bot[bot] authored Nov 10, 2023
1 parent 7e142ff commit 9c19934
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 9 deletions.
8 changes: 4 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
```
<!-- {x-version-update-end} -->

Expand Down Expand Up @@ -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
Expand Down
23 changes: 18 additions & 5 deletions ...le-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -473,7 +475,10 @@ private Builder() {}
"(?:cloudspanner:)(?<HOSTGROUP>//[\\w.-]+(?:\\.[\\w\\.-]+)*[\\w\\-\\._~:/?#\\[\\]@!\\$&'\\(\\)\\*\\+,;=.]+)?/projects/(?<PROJECTGROUP>(([a-z]|[-.:]|[0-9])+|(DEFAULT_PROJECT_ID)))(/instances/(?<INSTANCEGROUP>([a-z]|[-]|[0-9])+)(/databases/(?<DATABASEGROUP>([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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<String, String> 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;
}
Expand Down
103 changes: 103 additions & 0 deletions ...loud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,13 +35,15 @@
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;
import java.nio.charset.StandardCharsets;
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;
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit 9c19934

Please sign in to comment.