From ba7955b70accd49e6d0196ee754156e9728b649a Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Tue, 1 Feb 2022 12:27:40 -0800 Subject: [PATCH] Minor type and style fixes, remove unneeded constructor --- README.md | 20 +++++++++---------- .../embedded/DatabaseConnectionPreparer.java | 2 +- .../postgres/embedded/EmbeddedPostgres.java | 18 ++++------------- .../postgres/embedded/LiquibasePreparer.java | 2 +- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 8d7a36b3..024832fb 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ test with a "real" Postgres without requiring end users to install and set up a The release of 1.0 brings major changes to the innards of this library. -Previous pre 1.x versions used an embedded tarball. This was extemely fast (a major plus), but we switched to a docker based version +Previous pre 1.x versions used an embedded tarball. This was extremely fast (a major plus), but we switched to a docker based version for these reasons: Advantages @@ -20,7 +20,7 @@ Advantages * You need a tarball for every linux distribution as PG 10+ no longer ship a "universal binary" for linux. This means a lot of support and maintenance work. * Easy to switch docker image tag to upgrade versions - no need for a whole new pg-embedded version. * More maintainable and secure (you can pull docker images you trust, instead of trusting our tarballs running in your security context) -* Trivial to do a build oneself based on the official Postgres image adding extensions, setup scripts etc - see https://github.com/docker-library/docs/blob/master/postgres/README.md for details. +* Trivial to do a build oneself based on the official Postgres image adding extensions, setup scripts etc. - see https://github.com/docker-library/docs/blob/master/postgres/README.md for details. Admittedly, a few disadvantages --- @@ -53,7 +53,7 @@ public SingleInstancePostgresRule pg = EmbeddedPostgresRules.singleInstance(); This simply has JUnit manage an instance of EmbeddedPostgres (start, stop). You can then use this to get a DataSource with: `pg.getEmbeddedPostgres().getPostgresDatabase();` -Additionally you may use the [`EmbeddedPostgres`](src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java) class directly by manually starting and stopping the instance; see [`EmbeddedPostgresTest`](src/test/java/com/opentable/db/postgres/embedded/EmbeddedPostgresTest.java) for an example. +Additionally, you may use the [`EmbeddedPostgres`](src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java) class directly by manually starting and stopping the instance; see [`EmbeddedPostgresTest`](src/test/java/com/opentable/db/postgres/embedded/EmbeddedPostgresTest.java) for an example. Default username/password is: postgres/postgres and the default database is 'postgres' @@ -97,7 +97,7 @@ independent databases gives you. ## Postgres version -The default is to use the docker hub registry and pull a tag, hardcoded in `EmbeddedPostgres`. Currently this is "13-latest", +The default is to use the docker hub registry and pull a tag, hardcoded in `EmbeddedPostgres`. Currently, this is "13-latest", as this fits the needs of OpenTable, however you can change this easily. This is super useful, both to use a newer version of Postgres, or to build your own DockerFile with additional extensions. @@ -132,7 +132,7 @@ the network. ## Using JUnit5 -JUnit5 does not have `@Rule`. So below is an example for how to create tests using JUnit5 and embedded postgress, it creates a Spring context and uses JDBI: +JUnit5 does not have `@Rule`. So below is an example for how to create tests using JUnit5 and embedded postgres, it creates a Spring context and uses JDBI: ```java @ExtendWith(SpringExtension.class) @@ -158,7 +158,7 @@ class DaoTestUsingJunit5 { } /** - * This class is here as inner class for brevity + * This class is here as inner class for brevity, * but it's better to have only one for all tests. */ @Configuration @@ -232,14 +232,14 @@ But certainly there's no real reason you can't use TestContainers directly - the * Why not _use a maven plugin approach like fabric8-docker-maven? -Honestly I suspect this is a better approach in that it doesn't try to maintain it's own version of the Docker API, and -runs outside the tests, reducing issues like forking and threading conflicts. However it would have been too major an overhaul +Honestly I suspect this is a better approach in that it doesn't try to maintain its own version of the Docker API, and +runs outside the tests, reducing issues like forking and threading conflicts. However, it would have been too major an overhaul for our users. * "I really prefer the old embedded postgres approach. It's faster." * We recommend those who prefer the embedded tarball use https://github.com/zonkyio/embedded-postgres which was forked a couple - years ago from the embedded branch and is kept reasonably up to date. - * Another alternative is flapdoodle's embedded postgres. + of years ago from the embedded branch and is kept reasonably up to date. + * Another alternative is Flapdoodle's embedded postgres, but that is deprecated in favor of testcontainers too. Both libraries suffer from many of the cons that bedeviled upkeep of this library for years, but they are certainly viable options for many. diff --git a/src/main/java/com/opentable/db/postgres/embedded/DatabaseConnectionPreparer.java b/src/main/java/com/opentable/db/postgres/embedded/DatabaseConnectionPreparer.java index 2792d7e0..ac41744b 100644 --- a/src/main/java/com/opentable/db/postgres/embedded/DatabaseConnectionPreparer.java +++ b/src/main/java/com/opentable/db/postgres/embedded/DatabaseConnectionPreparer.java @@ -19,7 +19,7 @@ import javax.sql.DataSource; /** - * Provides a default implementation of the DatabasePreparer, and adds an additional + * Provides a default implementation of the DatabasePreparer, and adds a * method */ public interface DatabaseConnectionPreparer extends DatabasePreparer { diff --git a/src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java b/src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java index e7da11ad..8480d3be 100644 --- a/src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java +++ b/src/main/java/com/opentable/db/postgres/embedded/EmbeddedPostgres.java @@ -47,6 +47,7 @@ * Core class of the library, providing a builder (with reasonable defaults) to wrap * testcontainers and launch postgres container. */ +@SuppressWarnings("OptionalUsedAsFieldOrParameterType") public class EmbeddedPostgres implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(EmbeddedPostgres.class); @@ -57,7 +58,7 @@ public class EmbeddedPostgres implements Closeable { // 1) If this is defined, then it's assumed this contains the full image and tag... static final String ENV_DOCKER_IMAGE="PG_FULL_IMAGE"; // 2)Otherwise if this is defined, we'll use this as the prefix, and combine with the DOCKER_DEFAULT_TAG below - // This is already used in TestContainers as a env var, so it's useful to reuse for consistency. + // This is already used in TestContainers as an env var, so it's useful to reuse for consistency. static final String ENV_DOCKER_PREFIX = "TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX"; // 3) Otherwise we'll just pull from docker hub with the DOCKER_DEFAULT_TAG static final DockerImageName DOCKER_DEFAULT_IMAGE_NAME = DockerImageName.parse(POSTGRES); @@ -68,18 +69,6 @@ public class EmbeddedPostgres implements Closeable { private final UUID instanceId = UUID.randomUUID(); - - EmbeddedPostgres(Map postgresConfig, - Map localeConfig, - Map bindMounts, - Optional network, - Optional networkAlias, - DockerImageName image, - String databaseName - ) throws IOException { - this(postgresConfig, localeConfig, bindMounts, network, networkAlias, image, DEFAULT_PG_STARTUP_WAIT, databaseName); - } - EmbeddedPostgres(Map postgresConfig, Map localeConfig, Map bindMounts, @@ -214,6 +203,7 @@ public String getPassword() { return postgreDBContainer.getPassword(); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") public static class Builder { private final Map config = new HashMap<>(); private final Map localeConfig = new HashMap<>(); @@ -302,7 +292,7 @@ public Builder setBindMount(BindMount bindMount) { } /** - * Set up a shared network and the alias. This is useful if you have multiple containers + * Set up a shared network and the alias. This is useful if you have multiple containers, * and they need to communicate with each other. * @param network The Network. Usually Network.Shared. * @param networkAlias an alias by which other containers in the network can refer to this container diff --git a/src/main/java/com/opentable/db/postgres/embedded/LiquibasePreparer.java b/src/main/java/com/opentable/db/postgres/embedded/LiquibasePreparer.java index 2a33abbe..95505ec9 100644 --- a/src/main/java/com/opentable/db/postgres/embedded/LiquibasePreparer.java +++ b/src/main/java/com/opentable/db/postgres/embedded/LiquibasePreparer.java @@ -51,7 +51,7 @@ private LiquibasePreparer(String location, Contexts contexts) { @Override public void prepare(DataSource ds) throws SQLException { - try (Connection connection = ds.getConnection();){ + try (Connection connection = ds.getConnection()){ Database database = getInstance().findCorrectDatabaseImplementation(new JdbcConnection(connection)); Liquibase liquibase = new Liquibase(location, new ClassLoaderResourceAccessor(), database); //NOPMD