Repository: GoogleCloudPlatform/pgadapter
Pull request #1398 (2024-02-12 18:29:43)

test: verify that the JDBC driver is recognized with weird date style
The PostgreSQL JDBC driver by accident used a weird date style only in version 42.7.0. These tests verify that the driver is always recognized.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/1398?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`9482268`)](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/9482268f06a5bb3bd548038cd3697c6cff6cbacc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) 90.55% compared to head [(`b708c23`)](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/1398?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) 90.53%.


<details><summary>Additional details and impacted files</summary>


```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect    #1398      +/-   ##
========================================================
- Coverage                 90.55%   90.53%   -0.03%     
  Complexity                 2634     2634              
========================================================
  Files                       141      141              
  Lines                      9008     9008              
  Branches                   1313     1313              
========================================================
- Hits                       8157     8155       -2     
- Misses                      573      575       +2     
  Partials                    278      278              
```

| [Flag](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/1398/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/1398/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | `90.53% <ø> (-0.03%)` | :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.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/1398?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform).


Patch:
diff --git a/pom.xml b/pom.xml
index 7b251caa..e0e73ebe 100644
--- a/pom.xml
+++ b/pom.xml
@@ -83,7 +83,7 @@
       <dependency>
         <groupId>io.opentelemetry</groupId>
         <artifactId>opentelemetry-bom</artifactId>
-        <version>1.33.0</version>
+        <version>1.35.0</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java
index 5ec40302..44d94cd7 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/ProxyServer.java
@@ -183,7 +183,7 @@ public class ProxyServer extends AbstractApiService {
         listenerThread.start();
       }
       try {
-        if (startupLatch.await(30L, TimeUnit.SECONDS)) {
+        if (startupLatch.await(options.getStartupTimeout().toMillis(), TimeUnit.MILLISECONDS)) {
           notifyStarted();
         } else {
           throw SpannerExceptionFactory.newSpannerException(
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 e96faa5a..e5730088 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
@@ -34,6 +34,7 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Level;
@@ -49,6 +50,7 @@ import org.json.simple.JSONObject;
 
 /** Metadata extractor for CLI. */
 public class OptionsMetadata {
+  static Duration DEFAULT_STARTUP_TIMEOUT = Duration.ofSeconds(30L);
 
   /**
    * Builder class for creating an instance of {@link OptionsMetadata}.
@@ -87,6 +89,7 @@ public class OptionsMetadata {
     private boolean debugMode;
     private String endpoint;
     private boolean usePlainText;
+    private Duration startupTimeout = DEFAULT_STARTUP_TIMEOUT;
 
     Builder() {}
 
@@ -323,6 +326,11 @@ public class OptionsMetadata {
       return this;
     }
 
+    Builder setStartupTimeout(Duration timeout) {
+      this.startupTimeout = timeout;
+      return this;
+    }
+
     public OptionsMetadata build() {
       if (Strings.isNullOrEmpty(project) && !Strings.isNullOrEmpty(instance)) {
         throw SpannerExceptionFactory.newSpannerException(
@@ -542,6 +550,7 @@ public class OptionsMetadata {
   private final Map<String, String> propertyMap;
   private final String serverVersion;
   private final boolean debugMode;
+  private final Duration startupTimeout;
 
   /**
    * Creates a new instance of {@link OptionsMetadata} from the given arguments.
@@ -550,7 +559,7 @@ public class OptionsMetadata {
    * of calling this method directly.
    */
   public OptionsMetadata(String[] args) {
-    this(System.getenv(), System.getProperty("os.name", ""), args);
+    this(System.getenv(), System.getProperty("os.name", ""), DEFAULT_STARTUP_TIMEOUT, args);
   }
 
   private OptionsMetadata(Builder builder) {
@@ -559,11 +568,13 @@ public class OptionsMetadata {
         System.getProperty("os.name", ""),
         builder.toCommandLineArguments(),
         builder.credentials,
-        builder.sessionPoolOptions);
+        builder.sessionPoolOptions,
+        builder.startupTimeout);
   }
 
-  OptionsMetadata(Map<String, String> environment, String osName, String[] args) {
-    this(environment, osName, args, null, null);
+  OptionsMetadata(
+      Map<String, String> environment, String osName, Duration startupTimeout, String[] args) {
+    this(environment, osName, args, null, null, startupTimeout);
   }
 
   OptionsMetadata(
@@ -571,7 +582,8 @@ public class OptionsMetadata {
       String osName,
       String[] args,
       @Nullable Credentials credentials,
-      @Nullable SessionPoolOptions sessionPoolOptions) {
+      @Nullable SessionPoolOptions sessionPoolOptions,
+      Duration startupTimeout) {
     this.environment = Preconditions.checkNotNull(environment);
     this.osName = osName;
     this.commandLine = buildOptions(args);
@@ -630,6 +642,7 @@ public class OptionsMetadata {
     this.disableLocalhostCheck = commandLine.hasOption(OPTION_DISABLE_LOCALHOST_CHECK);
     this.serverVersion = commandLine.getOptionValue(OPTION_SERVER_VERSION, DEFAULT_SERVER_VERSION);
     this.debugMode = commandLine.hasOption(OPTION_INTERNAL_DEBUG_MODE);
+    this.startupTimeout = startupTimeout;
   }
 
   /**
@@ -701,6 +714,7 @@ public class OptionsMetadata {
     this.disableLocalhostCheck = false;
     this.serverVersion = DEFAULT_SERVER_VERSION;
     this.debugMode = false;
+    this.startupTimeout = DEFAULT_STARTUP_TIMEOUT;
   }
 
   private Map<String, String> parseProperties(String propertyOptions) {
@@ -1252,6 +1266,10 @@ public class OptionsMetadata {
     return this.debugMode;
   }
 
+  public Duration getStartupTimeout() {
+    return this.startupTimeout;
+  }
+
   public JSONObject getCommandMetadataJSON() {
     return this.commandMetadataJSON;
   }


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
index 55028934..89d23906 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -874,6 +874,20 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
     }
   }
 
+  @Test
+  public void testShowWellKnownClient() throws SQLException {
+    try (Connection connection =
+        DriverManager.getConnection(
+            String.format("jdbc:postgresql://localhost:%d/", pgServer.getLocalPort()))) {
+      try (ResultSet resultSet =
+          connection.createStatement().executeQuery("show spanner.well_known_client")) {
+        assertTrue(resultSet.next());
+        assertEquals("JDBC", resultSet.getString(1));
+        assertFalse(resultSet.next());
+      }
+    }
+  }
+
   @Test
   public void testSetWellKnownClient() throws SQLException {
     for (String client : new String[] {"pgx", "npgsql", "sqlalchemy2"}) {
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ProxyServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ProxyServerTest.java
new file mode 100644
index 00000000..e55e8500
--- /dev/null
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/ProxyServerTest.java
@@ -0,0 +1,54 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.cloud.spanner.pgadapter;
+
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
+import com.google.cloud.spanner.pgadapter.metadata.TestOptionsMetadataBuilder;
+import java.time.Duration;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class ProxyServerTest {
+
+  @Test
+  public void testFailedStartProxyServer() {
+    ProxyServer server = new ProxyServer(OptionsMetadata.newBuilder().setPort(0).build());
+    server.startServer();
+
+    // Try to start another server on the same port.
+    TestOptionsMetadataBuilder builder = new TestOptionsMetadataBuilder();
+    ProxyServer server2 =
+        new ProxyServer(
+            builder
+                .setStartupTimeout(Duration.ofMillis(10L))
+                .setPort(server.getLocalPort())
+                .build());
+    IllegalStateException exception =
+        assertThrows(IllegalStateException.class, server2::startServer);
+    assertTrue(
+        exception.getMessage(),
+        exception
+            .getMessage()
+            .contains(
+                "Expected the service InnerService [FAILED] to be RUNNING, but the service has FAILED"));
+
+    server.stopServer();
+  }
+}
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 23fc5d99..5bd82644 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
@@ -14,6 +14,7 @@
 
 package com.google.cloud.spanner.pgadapter.metadata;
 
+import static com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.DEFAULT_STARTUP_TIMEOUT;
 import static com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.parseSslMode;
 import static com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.toServerVersionNum;
 import static org.junit.Assert.assertEquals;
@@ -53,6 +54,7 @@ public class OptionsMetadataTest {
           new OptionsMetadata(
               Collections.emptyMap(),
               os,
+              DEFAULT_STARTUP_TIMEOUT,
               new String[] {"-p", "p", "-i", "i", "-c", "credentials.json"});
       if (options.isWindows()) {
         assertEquals("", options.getSocketFile(5432));
@@ -71,6 +73,7 @@ public class OptionsMetadataTest {
           new OptionsMetadata(
               Collections.emptyMap(),
               os,
+              DEFAULT_STARTUP_TIMEOUT,
               new String[] {"-p p", "-i i", "-c \"\"", "-dir /pgadapter"});
       assertEquals(
           "/pgadapter" + File.separatorChar + ".s.PGSQL.5432", options.getSocketFile(5432));
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/metadata/TestOptionsMetadataBuilder.java b/src/test/java/com/google/cloud/spanner/pgadapter/metadata/TestOptionsMetadataBuilder.java
index 843c890f..2b7218b7 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/metadata/TestOptionsMetadataBuilder.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/metadata/TestOptionsMetadataBuilder.java
@@ -14,6 +14,8 @@
 
 package com.google.cloud.spanner.pgadapter.metadata;
 
+import java.time.Duration;
+
 public class TestOptionsMetadataBuilder extends OptionsMetadata.Builder {
 
   @Override
@@ -28,8 +30,15 @@ public class TestOptionsMetadataBuilder extends OptionsMetadata.Builder {
     return this;
   }
 
+  @Override
   public TestOptionsMetadataBuilder setUsePlainText() {
     super.setUsePlainText();
     return this;
   }
+
+  @Override
+  public TestOptionsMetadataBuilder setStartupTimeout(Duration timeout) {
+    super.setStartupTimeout(timeout);
+    return this;
+  }
 }
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/utils/ClientAutoDetectorTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/utils/ClientAutoDetectorTest.java
index 7a47aa9a..ca6613e3 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/utils/ClientAutoDetectorTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/utils/ClientAutoDetectorTest.java
@@ -93,6 +93,28 @@ public class ClientAutoDetectorTest {
                 "client_encoding", "UTF8",
                 "DateStyle", "ISO",
                 "TimeZone", "some-time-zone")));
+    // Version 42.7.0 of the JDBC driver by accident used 'MDY' as the DateStyle.
+    assertEquals(
+        WellKnownClient.JDBC,
+        ClientAutoDetector.detectClient(
+            ImmutableList.of("user", "database", "client_encoding", "DateStyle", "TimeZone"),
+            ImmutableMap.of(
+                "user", "some-user",
+                "database", "some-database",
+                "client_encoding", "UTF8",
+                "DateStyle", "ISO, MDY",
+                "TimeZone", "some-time-zone")));
+    // Other values for DateStyle have never been used.
+    assertNotEquals(
+        WellKnownClient.JDBC,
+        ClientAutoDetector.detectClient(
+            ImmutableList.of("user", "database", "client_encoding", "DateStyle", "TimeZone"),
+            ImmutableMap.of(
+                "user", "some-user",
+                "database", "some-database",
+                "client_encoding", "UTF8",
+                "DateStyle", "ISO, DMY",
+                "TimeZone", "some-time-zone")));
     // Additional parameters are allowed.
     assertEquals(
         WellKnownClient.JDBC,