From 325bdb61bf15d085477279e0e21891c590a5c7fd Mon Sep 17 00:00:00 2001
From: sagnghos <sagnikghosh@google.com>
Date: Wed, 11 Dec 2024 11:05:30 +0000
Subject: [PATCH] feat(spanner): added default port value and unit tests

---
 .../spanner/connection/ConnectionOptions.java |  5 +++
 .../connection/ConnectionOptionsTest.java     | 37 +++++++++++++++++++
 2 files changed, 42 insertions(+)

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 18c81a6ba32..66ac70994f2 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
@@ -635,6 +635,7 @@ private Builder() {}
     @VisibleForTesting
     static final Pattern SPANNER_URI_PATTERN = Pattern.compile(SPANNER_URI_REGEX);
 
+    @VisibleForTesting
     static final Pattern EXTERNAL_HOST_PATTERN = Pattern.compile(EXTERNAL_HOST_FORMAT);
 
     private static final String HOST_GROUP = "HOSTGROUP";
@@ -1004,6 +1005,10 @@ static String determineHost(
       // The leading '//' is already included in the regex for the connection URL, so we don't need
       // to add the leading '//' to the host name here.
       host = matcher.group(Builder.HOST_GROUP);
+      if (ConnectionOptions.Builder.EXTERNAL_HOST_FORMAT.equals(matcher.pattern().pattern())
+          && !host.matches(".*:\\d+$")) {
+        host = String.format("%s:15000", host);
+      }
     }
     if (usePlainText) {
       return PLAIN_TEXT_PROTOCOL + 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 f826ec08dfc..69c4a010327 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,7 @@
 
 package com.google.cloud.spanner.connection;
 
+import static com.google.cloud.spanner.connection.ConnectionOptions.Builder.EXTERNAL_HOST_PATTERN;
 import static com.google.cloud.spanner.connection.ConnectionOptions.Builder.SPANNER_URI_PATTERN;
 import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_ENDPOINT;
 import static com.google.cloud.spanner.connection.ConnectionOptions.determineHost;
@@ -1211,4 +1212,40 @@ public void testEnableApiTracing() {
             .build()
             .isEnableApiTracing());
   }
+
+  @Test
+  public void testExternalHostPatterns() {
+    Matcher matcherWithoutInstance =
+        EXTERNAL_HOST_PATTERN.matcher("cloudspanner://localhost:15000/databases/test-db");
+    assertTrue(matcherWithoutInstance.matches());
+    assertNull(matcherWithoutInstance.group("INSTANCEGROUP"));
+    assertEquals("test-db", matcherWithoutInstance.group("DATABASEGROUP"));
+    Matcher matcherWithProperty =
+        EXTERNAL_HOST_PATTERN.matcher(
+            "cloudspanner://localhost:15000/instances/default/databases/singers-db?usePlainText=true");
+    assertTrue(matcherWithProperty.matches());
+    assertEquals("default", matcherWithProperty.group("INSTANCEGROUP"));
+    assertEquals("singers-db", matcherWithProperty.group("DATABASEGROUP"));
+    Matcher matcherWithoutPort =
+        EXTERNAL_HOST_PATTERN.matcher(
+            "cloudspanner://localhost/instances/default/databases/test-db");
+    assertTrue(matcherWithoutPort.matches());
+    assertEquals("default", matcherWithoutPort.group("INSTANCEGROUP"));
+    assertEquals("test-db", matcherWithoutPort.group("DATABASEGROUP"));
+    assertEquals(
+        "http://localhost:15000",
+        determineHost(
+            matcherWithoutPort,
+            DEFAULT_ENDPOINT,
+            /* autoConfigEmulator= */ true,
+            /* usePlainText= */ true,
+            ImmutableMap.of()));
+    Matcher matcherWithProject =
+        EXTERNAL_HOST_PATTERN.matcher(
+            "cloudspanner://localhost:15000/projects/default/instances/default/databases/singers-db");
+    assertFalse(matcherWithProject.matches());
+    Matcher matcherWithoutHost =
+        EXTERNAL_HOST_PATTERN.matcher("cloudspanner:/instances/default/databases/singers-db");
+    assertFalse(matcherWithoutHost.matches());
+  }
 }