Fix invalid connection URLs returned by PostgreSQLContainer (fixes te…

…stcontainers#1976) (testcontainers#2049) * Reproduce testcontainers#1976 in tests * Override constructUrlForConnection for PostgreSQLContainer * Lighten PostgreSQLConnectionURLTest * Lighten PostgreSQLConnectionURLTest * Cater for base URLs without query strings
quincy · May 28, 2020 · 84b895e · 84b895e
1 parent 27299d8
commit 84b895e
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 1 deletion.
diff --git a/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java b/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java
@@ -7,6 +7,7 @@
 import java.util.HashSet;
 import java.util.Set;
 
+import static java.lang.String.format;
 import static java.time.temporal.ChronoUnit.SECONDS;
 
 /**
@@ -24,6 +25,8 @@ public class PostgreSQLContainer<SELF extends PostgreSQLContainer<SELF>> extends
 
     private static final String FSYNC_OFF_OPTION = "fsync=off";
 
+    private static final String QUERY_PARAM_SEPARATOR = "&";
+
     public PostgreSQLContainer() {
         this(IMAGE + ":" + DEFAULT_TAG);
     }
@@ -59,7 +62,24 @@ public String getDriverClassName() {
     @Override
     public String getJdbcUrl() {
         // Disable Postgres driver use of java.util.logging to reduce noise at startup time
-        return "jdbc:postgresql://" + getContainerIpAddress() + ":" + getMappedPort(POSTGRESQL_PORT) + "/" + databaseName + "?loggerLevel=OFF";
+        return format("jdbc:postgresql://%s:%d/%s?loggerLevel=OFF", getContainerIpAddress(), getMappedPort(POSTGRESQL_PORT), databaseName);
+    }
+
+    @Override
+    protected String constructUrlForConnection(String queryString) {
+        String baseUrl = getJdbcUrl();
+
+        if ("".equals(queryString)) {
+            return baseUrl;
+        }
+
+        if (!queryString.startsWith("?")) {
+            throw new IllegalArgumentException("The '?' character must be included");
+        }
+
+        return baseUrl.contains("?")
+            ? baseUrl + QUERY_PARAM_SEPARATOR + queryString.substring(1)
+            : baseUrl + queryString;
     }
 
     @Override

diff --git a/...s/postgresql/src/test/java/org/testcontainers/containers/PostgreSQLConnectionURLTest.java b/...s/postgresql/src/test/java/org/testcontainers/containers/PostgreSQLConnectionURLTest.java
@@ -0,0 +1,68 @@
+package org.testcontainers.containers;
+
+import org.junit.Test;
+
+import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertFalse;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertThrows;
+import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
+
+public class PostgreSQLConnectionURLTest {
+
+    @Test
+    public void shouldCorrectlyAppendQueryString() {
+        PostgreSQLContainer postgres = new FixedJdbcUrlPostgreSQLContainer();
+        String connectionUrl = postgres.constructUrlForConnection("?stringtype=unspecified&stringtype=unspecified");
+        String queryString = connectionUrl.substring(connectionUrl.indexOf('?'));
+
+        assertTrue("Query String contains expected params", queryString.contains("&stringtype=unspecified&stringtype=unspecified"));
+        assertEquals("Query String starts with '?'", 0, queryString.indexOf('?'));
+        assertFalse("Query String does not contain extra '?'", queryString.substring(1).contains("?"));
+    }
+
+    @Test
+    public void shouldCorrectlyAppendQueryStringWhenNoBaseParams() {
+        PostgreSQLContainer postgres = new NoParamsUrlPostgreSQLContainer();
+        String connectionUrl = postgres.constructUrlForConnection("?stringtype=unspecified&stringtype=unspecified");
+        String queryString = connectionUrl.substring(connectionUrl.indexOf('?'));
+
+        assertTrue("Query String contains expected params", queryString.contains("?stringtype=unspecified&stringtype=unspecified"));
+        assertEquals("Query String starts with '?'", 0, queryString.indexOf('?'));
+        assertFalse("Query String does not contain extra '?'", queryString.substring(1).contains("?"));
+    }
+
+    @Test
+    public void shouldReturnOriginalURLWhenEmptyQueryString() {
+        PostgreSQLContainer postgres = new FixedJdbcUrlPostgreSQLContainer();
+        String connectionUrl = postgres.constructUrlForConnection("");
+
+        assertTrue("Query String remains unchanged", postgres.getJdbcUrl().equals(connectionUrl));
+    }
+
+    @Test
+    public void shouldRejectInvalidQueryString() {
+        assertThrows("Fails when invalid query string provided", IllegalArgumentException.class,
+            () -> new NoParamsUrlPostgreSQLContainer().constructUrlForConnection("stringtype=unspecified"));
+    }
+
+    static class FixedJdbcUrlPostgreSQLContainer extends PostgreSQLContainer {
+
+        @Override
+        public String getContainerIpAddress() {
+            return "localhost";
+        }
+
+        @Override
+        public Integer getMappedPort(int originalPort) {
+            return 34532;
+        }
+    }
+
+    static class NoParamsUrlPostgreSQLContainer extends PostgreSQLContainer {
+
+        @Override
+        public String getJdbcUrl() {
+            return "jdbc:postgresql://host:port/database";
+        }
+    }
+}