Fix TestDatabasePool (#1131)

TestDatabasePool was:

 1. Catching the wrong exception (`PersistenceException` instead of
    `SQLException`)
 2. Not releasing database names for re-use to the correct pool (instead
    they would be released to a brand new pool)

This change fixes both of these issues and adds an integration test that
works on a real MySQL database, and a regression test for database name
releases.

Author: keeferrourke

misk-hibernate-testing/src/main/kotlin/misk/jdbc/MySqlTestDatabasePoolBackend.kt

@@ -16,7 +16,7 @@ import javax.sql.DataSource
 internal class MySqlTestDatabasePoolBackend @Inject constructor(
   val config: DataSourceConfig
 ) : TestDatabasePool.Backend {
-  private val connection: Connection by lazy {
+  internal val connection: Connection by lazy {
     try {
       DriverDataSource(
           config.buildJdbcUrl(Environment.TESTING),

misk-hibernate-testing/src/main/kotlin/misk/jdbc/TestDatabasePool.kt

@@ -1,6 +1,7 @@
 package misk.jdbc
 
 import misk.logging.getLogger
+import java.sql.SQLException
 import java.time.Clock
 import java.time.Duration
 import java.time.LocalDate
@@ -66,7 +67,13 @@ class TestDatabasePool(
 
   /** Return a config-specific pool, creating it if necessary. */
   internal fun getPool(config: DataSourceConfig): ConfigSpecificPool {
-    val key = config.database!!
+    var key = config.database!!
+    for (pool in poolsByKey.values) {
+      if (key.startsWith(pool.key)) {
+        key = pool.key
+        break
+      }
+    }
     return poolsByKey.computeIfAbsent(key) { ConfigSpecificPool(key, config.type) }
   }
 
@@ -80,7 +87,7 @@ class TestDatabasePool(
     private val formatter = DateTimeFormatter.BASIC_ISO_DATE
 
     /** Database names that are available. */
-    private val pool = LinkedBlockingDeque<String>()
+    internal val pool = LinkedBlockingDeque<String>()
 
     /** Decodes a database name string. */
     fun decode(databaseName: String): DatabaseName? {
@@ -127,7 +134,7 @@ class TestDatabasePool(
         try {
           backend.createDatabase(databaseName.toString())
           break
-        } catch (_: PersistenceException) {
+        } catch (_: SQLException) {
           logger.info { "Lost a race trying to allocate a test database $databaseName" }
           databaseName = databaseName.copy(version = databaseName.version + 1)
         }
@@ -140,7 +147,11 @@ class TestDatabasePool(
       val evictBefore = LocalDate.now(clock).minus(Period.ofDays(retention.toDays().toInt()))
       val evictBeforeYearMonthDay = evictBefore.format(formatter).toLong()
 
-      val oldDatabases = getDatabases().filter { it.yearMonthDay < evictBeforeYearMonthDay }
+      val oldDatabases = if (retention.isZero) {
+        getDatabases()
+      } else {
+        getDatabases().filter { it.yearMonthDay < evictBeforeYearMonthDay }
+      }
 
       for (database in oldDatabases) {
         backend.dropDatabase("$database")

misk-hibernate-testing/src/test/kotlin/misk/jdbc/TestDatabasePoolTest.kt

@@ -10,7 +10,10 @@ import misk.testing.MiskTestModule
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+import java.sql.SQLException
 import java.time.Clock
+import java.time.Duration
 import javax.inject.Inject
 import javax.persistence.PersistenceException
 
@@ -28,7 +31,12 @@ class TestDatabasePoolTest {
   @Inject private lateinit var clock: Clock
   @Inject private lateinit var backend: FakeDatabaseBackend
 
-  private val config = DataSourceConfig(type = DataSourceType.MYSQL, database = "test")
+  private val config = DataSourceConfig(
+      type = DataSourceType.MYSQL,
+      database = "test",
+      username = "root",
+      password = ""
+  )
 
   private lateinit var testDatabasePool: TestDatabasePool
 
@@ -103,6 +111,38 @@ class TestDatabasePoolTest {
     )
   }
 
+  @Test fun releasesDatabaseNamesForReuse() {
+    assertThat(testDatabasePool.takeDatabase(config).database).isEqualTo("test__20180101__1")
+    assertThat(testDatabasePool.takeDatabase(config).database).isEqualTo("test__20180101__2")
+    testDatabasePool.releaseDatabase(config.copy(database = "test__20180101__1"))
+    assertThat(testDatabasePool.takeDatabase(config).database).isEqualTo("test__20180101__1")
+  }
+
+  @Test fun integrationTest() {
+    val realDatabasePool = TestDatabasePool(MySqlTestDatabasePoolBackend(config), clock)
+    realDatabasePool.getPool(config)
+    realDatabasePool.pruneOldDatabases(retention = Duration.ZERO)
+
+    // Create a database. This will be the first one that the pool tries to create.
+    realDatabasePool.backend.createDatabase("test__20180101__1")
+
+    // Demonstrate that attempting to create an existing database throws.
+    assertThrows<SQLException> {
+      realDatabasePool.backend.createDatabase("test__20180101__1")
+    }
+
+    // Assert that the database pool does not yet know about any database names.
+    assertThat(realDatabasePool.getPool(config).pool).isEmpty()
+
+    // At this point, the pool has tried to allocate "test__20180101__1", failed, and bumped the
+    // sequence number.
+    assertThat(realDatabasePool.takeDatabase(config).database).isEqualTo("test__20180101__2")
+
+    // If we return a name to the database pool, it will be the next database to be taken.
+    realDatabasePool.releaseDatabase(config.copy(database = "test__20180101__1"))
+    assertThat(realDatabasePool.takeDatabase(config).database).isEqualTo("test__20180101__1")
+  }
+
   private fun backendHasDatabases() {
     val todaysDatabases = listOf(
         "test__20180101__1",