Repository: GoogleCloudPlatform/pgadapter
Pull request #995 (2023-08-24 13:01:33)

test: add dialect to Connection URI and support autoConfigEmulator
Automatically add the `PostgreSQL` dialect to the Connection URL. This ensures that the underlying client library will create a `PostgreSQL` dialect database when the `autoConfigEmulator=true` property has also been set in the connection URL.

This pull request also moves the logic to build the Connection URL to a separately testable method.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#995](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (42787b4) into [postgresql-dialect](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/bccdcbc77bd318579cf60773c3d3f2351b8aa742?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (bccdcbc) will **decrease** coverage by `1.71%`.
> The diff coverage is `52.17%`.

```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect     #995      +/-   ##
========================================================
- Coverage                 90.24%   88.53%   -1.71%     
+ Complexity                 2553     2505      -48     
========================================================
  Files                       141      141              
  Lines                      8508     8515       +7     
  Branches                   1264     1264              
========================================================
- Hits                       7678     7539     -139     
- Misses                      563      675     +112     
- Partials                    267      301      +34     
```

| [Flag](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [all_tests](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | `88.53% <52.17%> (-1.71%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files Changed](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...gle/cloud/spanner/pgadapter/ConnectionHandler.java](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL0Nvbm5lY3Rpb25IYW5kbGVyLmphdmE=) | `88.72% <38.88%> (+0.82%)` | :arrow_up: |
| [...ud/spanner/pgadapter/metadata/OptionsMetadata.java](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL21ldGFkYXRhL09wdGlvbnNNZXRhZGF0YS5qYXZh) | `86.44% <100.00%> (+0.14%)` | :arrow_up: |

... and [5 files with indirect coverage changes](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/995/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform)


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java
index 36385570..80f298c0 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/ConnectionHandler.java
@@ -170,32 +170,7 @@ public class ConnectionHandler extends Thread {
   @InternalApi
   public void connectToSpanner(String database, @Nullable Credentials credentials) {
     OptionsMetadata options = getServer().getOptions();
-    String uri =
-        options.hasDefaultConnectionUrl()
-            ? options.getDefaultConnectionUrl()
-            : options.buildConnectionURL(database);
-    if (uri.startsWith("jdbc:")) {
-      uri = uri.substring("jdbc:".length());
-    }
-    uri = appendPropertiesToUrl(uri, getServer().getProperties());
-    if (System.getProperty(CHANNEL_PROVIDER_PROPERTY) != null) {
-      uri =
-          uri
-              + ";"
-              + ConnectionOptions.CHANNEL_PROVIDER_PROPERTY_NAME
-              + "="
-              + System.getProperty(CHANNEL_PROVIDER_PROPERTY);
-      // This forces the connection to use NoCredentials.
-      uri = uri + ";usePlainText=true";
-      try {
-        Class.forName(System.getProperty(CHANNEL_PROVIDER_PROPERTY));
-      } catch (ClassNotFoundException e) {
-        throw SpannerExceptionFactory.newSpannerException(
-            ErrorCode.INVALID_ARGUMENT,
-            "Unknown or invalid channel provider: "
-                + System.getProperty(CHANNEL_PROVIDER_PROPERTY));
-      }
-    }
+    String uri = buildConnectionURL(database, options, getServer().getProperties());
     ConnectionOptions.Builder connectionOptionsBuilder = ConnectionOptions.newBuilder().setUri(uri);
     if (credentials != null) {
       connectionOptionsBuilder =
@@ -255,7 +230,44 @@ public class ConnectionHandler extends Thread {
     this.extendedQueryProtocolHandler = new ExtendedQueryProtocolHandler(this);
   }
 
-  private String appendPropertiesToUrl(String url, Properties info) {
+  @VisibleForTesting
+  static String buildConnectionURL(
+      String database, OptionsMetadata options, Properties properties) {
+    String uri =
+        options.hasDefaultConnectionUrl()
+            ? options.getDefaultConnectionUrl()
+            : options.buildConnectionURL(database);
+    uri = appendPropertiesToUrl(uri, properties);
+    // We add 'dialect=postgresql' here in all cases, although it is only being used if
+    // 'autoConfigEmulator=true' has also been set in the connection URL. This dialect property is
+    // only used to determine what dialect the database should have that is being created on the
+    // emulator when 'autoConfigEmulator=true'.
+    uri = uri + ";dialect=postgresql";
+    if (System.getProperty(CHANNEL_PROVIDER_PROPERTY) != null) {
+      uri =
+          uri
+              + ";"
+              + ConnectionOptions.CHANNEL_PROVIDER_PROPERTY_NAME
+              + "="
+              + System.getProperty(CHANNEL_PROVIDER_PROPERTY);
+      // This forces the connection to use NoCredentials.
+      uri = uri + ";usePlainText=true";
+      try {
+        Class.forName(System.getProperty(CHANNEL_PROVIDER_PROPERTY));
+        // Also set the system property for the Connection API that allows us to use a custom
+        // channel provider.
+        System.setProperty("ENABLE_CHANNEL_PROVIDER", "true");
+      } catch (ClassNotFoundException e) {
+        throw SpannerExceptionFactory.newSpannerException(
+            ErrorCode.INVALID_ARGUMENT,
+            "Unknown or invalid channel provider: "
+                + System.getProperty(CHANNEL_PROVIDER_PROPERTY));
+      }
+    }
+    return uri;
+  }
+
+  private static String appendPropertiesToUrl(String url, Properties info) {
     if (info == null || info.isEmpty()) {
       return url;
     }
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java b/src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java
index b9cc3f30..e7467304 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java
@@ -78,6 +78,7 @@ public class OptionsMetadata {
     private SslMode sslMode;
     private int port;
     private String unixDomainSocketDirectory;
+    private boolean autoConfigEmulator;
     private boolean debugMode;
     private String endpoint;
     private boolean usePlainText;
@@ -270,6 +271,19 @@ public class OptionsMetadata {
       return this;
     }
 
+    /**
+     * Instructs PGAdapter to connect to the Cloud Spanner emulator and to automatically create the
+     * instance and the database in the connection request if the instance or database does not yet
+     * exist on the emulator. Setting this flag takes care of everything you need to connect to the
+     * emulator (except for starting the emulator), and the auto-creation of the instance and
+     * database removes the need for any additional scripts that must be executed before connecting
+     * to the emulator.
+     */
+    public Builder autoConfigureEmulator() {
+      this.autoConfigEmulator = true;
+      return this.setCredentials(NoCredentials.getInstance());
+    }
+
     Builder enableDebugMode() {
       this.debugMode = true;
       return this;
@@ -338,7 +352,7 @@ public class OptionsMetadata {
       if (endpoint != null) {
         addOption(args, OPTION_SPANNER_ENDPOINT, endpoint);
       }
-      if (usePlainText || numChannels != null || databaseRole != null) {
+      if (usePlainText || numChannels != null || databaseRole != null || autoConfigEmulator) {
         StringBuilder jdbcOptionBuilder = new StringBuilder();
         if (usePlainText) {
           jdbcOptionBuilder.append("usePlainText=true;");
@@ -349,6 +363,9 @@ public class OptionsMetadata {
         if (databaseRole != null) {
           jdbcOptionBuilder.append("databaseRole=").append(databaseRole).append(";");
         }
+        if (autoConfigEmulator) {
+          jdbcOptionBuilder.append("autoConfigEmulator=true;");
+        }
         addOption(args, OPTION_JDBC_PROPERTIES, jdbcOptionBuilder.toString());
       }
       if (debugMode) {
@@ -615,7 +632,10 @@ public class OptionsMetadata {
     this.credentials = null;
     this.sessionPoolOptions = null;
     this.commandMetadataParser = new CommandMetadataParser();
-    this.defaultConnectionUrl = defaultConnectionUrl;
+    this.defaultConnectionUrl =
+        defaultConnectionUrl.startsWith("jdbc:")
+            ? defaultConnectionUrl.substring("jdbc:".length())
+            : defaultConnectionUrl;
     this.proxyPort = proxyPort;
     this.socketFile = isWindows() ? "" : DEFAULT_SOCKET_DIR + File.separatorChar + SOCKET_FILE_NAME;
     this.maxBacklog = DEFAULT_MAX_BACKLOG;


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java
index b802df38..83d20e8f 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/ConnectionHandlerTest.java
@@ -14,8 +14,10 @@
 
 package com.google.cloud.spanner.pgadapter;
 
+import static com.google.cloud.spanner.pgadapter.ConnectionHandler.buildConnectionURL;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
@@ -24,11 +26,14 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import com.google.cloud.NoCredentials;
 import com.google.cloud.spanner.ErrorCode;
+import com.google.cloud.spanner.SpannerException;
 import com.google.cloud.spanner.SpannerExceptionFactory;
 import com.google.cloud.spanner.Statement;
 import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
 import com.google.cloud.spanner.connection.Connection;
+import com.google.cloud.spanner.connection.TestChannelProvider;
 import com.google.cloud.spanner.pgadapter.ConnectionHandler.ConnectionStatus;
 import com.google.cloud.spanner.pgadapter.error.PGException;
 import com.google.cloud.spanner.pgadapter.error.SQLState;
@@ -42,6 +47,7 @@ import com.google.cloud.spanner.pgadapter.utils.ClientAutoDetector.WellKnownClie
 import com.google.cloud.spanner.pgadapter.wireprotocol.ParseMessage;
 import com.google.cloud.spanner.pgadapter.wireprotocol.WireMessage;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.EOFException;
@@ -50,6 +56,8 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.InetAddress;
 import java.net.Socket;
+import java.util.Map;
+import java.util.Properties;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -659,4 +667,92 @@ public class ConnectionHandlerTest {
     assertTrue(connectionHandler.isHasDeterminedClientUsingQuery());
     assertTrue(connectionHandler.getSkippedAutoDetectParseMessages().isEmpty());
   }
+
+  @Test
+  public void testBuildConnectionUrl() {
+    OptionsMetadata options =
+        OptionsMetadata.newBuilder().setCredentials(NoCredentials.getInstance()).build();
+    // Check that the dialect is included in the connection URL. This is required to support the
+    // 'autoConfigEmulator' property.
+    assertEquals(
+        "cloudspanner:/projects/my-project/instances/my-instance/databases/my-database;userAgent=pg-adapter;dialect=postgresql",
+        buildConnectionURL(
+            "projects/my-project/instances/my-instance/databases/my-database",
+            options,
+            buildProperties(ImmutableMap.of())));
+    assertEquals(
+        "cloudspanner:/projects/my-project/instances/my-instance/databases/my-database;userAgent=pg-adapter;key1=value1;dialect=postgresql",
+        buildConnectionURL(
+            "projects/my-project/instances/my-instance/databases/my-database",
+            options,
+            buildProperties(ImmutableMap.of("key1", "value1"))));
+
+    // If the options contain a full database specification, then the database in the connection
+    // request is ignored.
+    assertEquals(
+        "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database;userAgent=pg-adapter;dialect=postgresql",
+        buildConnectionURL(
+            "projects/my-project/instances/my-instance/databases/my-database",
+            OptionsMetadata.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setProject("test-project")
+                .setInstance("test-instance")
+                .setDatabase("test-database")
+                .build(),
+            buildProperties(ImmutableMap.of())));
+    // Enable the autoConfigEmulator flag through the options builder.
+    OptionsMetadata emulatorOptions = OptionsMetadata.newBuilder().autoConfigureEmulator().build();
+    assertEquals(
+        "cloudspanner:/projects/my-project/instances/my-instance/databases/my-database;userAgent=pg-adapter;autoConfigEmulator=true;dialect=postgresql",
+        buildConnectionURL(
+            "projects/my-project/instances/my-instance/databases/my-database",
+            emulatorOptions,
+            buildProperties(emulatorOptions.getPropertyMap())));
+
+    // Set a channel provider.
+    String currentChannelProvider = System.getProperty("CHANNEL_PROVIDER");
+    String currentEnableChannelProvider = System.getProperty("ENABLE_CHANNEL_PROVIDER");
+    try {
+      System.setProperty("CHANNEL_PROVIDER", TestChannelProvider.class.getName());
+      assertEquals(
+          "cloudspanner:/projects/my-project/instances/my-instance/databases/my-database"
+              + ";userAgent=pg-adapter;dialect=postgresql"
+              + ";channelProvider=com.google.cloud.spanner.connection.TestChannelProvider"
+              + ";usePlainText=true",
+          buildConnectionURL(
+              "projects/my-project/instances/my-instance/databases/my-database",
+              options,
+              buildProperties(ImmutableMap.of())));
+      assertEquals("true", System.getProperty("ENABLE_CHANNEL_PROVIDER"));
+
+      // Set an invalid channel provider.
+      System.clearProperty("ENABLE_CHANNEL_PROVIDER");
+      System.setProperty("CHANNEL_PROVIDER", "foo");
+      assertThrows(
+          SpannerException.class,
+          () ->
+              buildConnectionURL(
+                  "projects/my-project/instances/my-instance/databases/my-database",
+                  options,
+                  buildProperties(ImmutableMap.of())));
+      assertNull(System.getProperty("ENABLE_CHANNEL_PROVIDER"));
+    } finally {
+      if (currentChannelProvider == null) {
+        System.clearProperty("CHANNEL_PROVIDER");
+      } else {
+        System.setProperty("CHANNEL_PROVIDER", currentChannelProvider);
+      }
+      if (currentEnableChannelProvider == null) {
+        System.clearProperty("ENABLE_CHANNEL_PROVIDER");
+      } else {
+        System.setProperty("ENABLE_CHANNEL_PROVIDER", currentEnableChannelProvider);
+      }
+    }
+  }
+
+  static Properties buildProperties(Map<String, String> map) {
+    Properties properties = new Properties();
+    properties.putAll(map);
+    return properties;
+  }
 }
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadataTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadataTest.java
index e4d66383..d9e1ad50 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadataTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadataTest.java
@@ -32,6 +32,7 @@ import com.google.cloud.spanner.SessionPoolOptions;
 import com.google.cloud.spanner.SpannerException;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.DdlTransactionMode;
 import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.SslMode;
+import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.TextFormat;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -496,4 +497,21 @@ public class OptionsMetadataTest {
                 .setRequireAuthentication()
                 .build());
   }
+
+  @Test
+  public void testStripJdbcPrefix() {
+    assertEquals(
+        "cloudspanner:/projects/my-project/instances/my-instance/databases/my-database",
+        new OptionsMetadata(
+                "linux",
+                "jdbc:cloudspanner:/projects/my-project/instances/my-instance/databases/my-database",
+                5432,
+                TextFormat.POSTGRESQL,
+                false,
+                false,
+                false,
+                false,
+                null)
+            .getDefaultConnectionUrl());
+  }
 }