From 3b18870733527ae3503380504db8ea7d76ff85a4 Mon Sep 17 00:00:00 2001 From: Ken Gullaksen Date: Wed, 15 Jan 2025 10:03:57 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"Endre=20Database.Config=20til=20?= =?UTF-8?q?=C3=A5=20bruke=20JDBC=5FURL"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 73f4cc825cb201b47533407c19ce8cd872c3ab2d. --- .../notifikasjon/infrastruktur/Database.kt | 93 ++++--------------- .../executable/BackupRepositoryPerformance.kt | 4 +- .../infrastruktur/DatabaseConfigTest.kt | 71 -------------- .../notifikasjon/util/PostgresTestListener.kt | 22 +++-- 4 files changed, 32 insertions(+), 158 deletions(-) delete mode 100644 app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/DatabaseConfigTest.kt diff --git a/app/src/main/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/Database.kt b/app/src/main/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/Database.kt index da2d4b72d..41840254f 100644 --- a/app/src/main/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/Database.kt +++ b/app/src/main/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/Database.kt @@ -6,13 +6,10 @@ import kotlinx.coroutines.* import no.nav.arbeidsgiver.notifikasjon.infrastruktur.json.laxObjectMapper import no.nav.arbeidsgiver.notifikasjon.infrastruktur.json.writeValueAsStringSupportingTypeInfoInCollections import no.nav.arbeidsgiver.notifikasjon.infrastruktur.unblocking.NonBlockingDataSource -import org.apache.http.client.utils.URIBuilder -import org.apache.http.message.BasicNameValuePair import org.flywaydb.core.Flyway import org.flywaydb.core.api.configuration.FluentConfiguration import org.intellij.lang.annotations.Language import java.io.Closeable -import java.net.URI import java.sql.* import java.time.* import java.time.temporal.ChronoUnit @@ -27,25 +24,16 @@ class Database private constructor( private val dataSource: NonBlockingDataSource<*>, ) : Closeable { data class Config( - private val jdbcUrl: String, + val host: String, + val port: String, + val database: String, + val username: String, + val password: String, val migrationLocations: String, - val jdbcOpts: Map = mapOf() + val jdbcOpts: Map = mapOf() ) { - val url: JdbcUrl = JdbcUrl(jdbcUrl, jdbcOpts) - - val username: String - get() = url.username - val password: String - get() = url.password - val database: String - get() = url.database - - /** - * make a copy but change the database name - */ - fun withDatabase(database: String) = copy( - jdbcUrl = url.withDatabase(database).toString() - ) + val jdbcUrl: String + get() = "jdbc:postgresql://$host:$port/$database?${jdbcOpts.entries.joinToString("&")}" } override fun close() { @@ -55,13 +43,12 @@ class Database private constructor( companion object { private val log = logger() - fun config(name: String, envPrefix: String = "DB", jdbcOpts: Map = emptyMap()) = Config( - jdbcUrl = System.getenv("${envPrefix}_JDBC_URL") ?: "jdbc:postgresql://localhost:1337/${ - name.replace( - '_', - '-' - ) - }?password=postgres&user=postgres", + fun config(name: String, envPrefix: String = "DB", jdbcOpts: Map = mapOf()) = Config( + host = System.getenv("${envPrefix}_HOST") ?: "localhost", + port = System.getenv("${envPrefix}_PORT") ?: "1337", + username = System.getenv("${envPrefix}_USERNAME") ?: "postgres", + password = System.getenv("${envPrefix}_PASSWORD") ?: "postgres", + database = System.getenv("${envPrefix}_DATABASE") ?: name.replace('_', '-'), migrationLocations = "db/migration/$name", jdbcOpts = jdbcOpts, ) @@ -69,7 +56,9 @@ class Database private constructor( private fun Config.asHikariConfig(): HikariConfig { val config = this return HikariConfig().apply { - jdbcUrl = config.url.toString() + jdbcUrl = config.jdbcUrl + username = config.username + password = config.password driverClassName = "org.postgresql.Driver" metricRegistry = Metrics.meterRegistry minimumIdle = 1 @@ -367,51 +356,3 @@ fun measureSql( @Suppress("NULLABILITY_MISMATCH_BASED_ON_JAVA_ANNOTATIONS") return timer.recordCallable(action) } - -/** - * Utility class to aid in constructing and manipulating a jdbc url. - */ -class JdbcUrl( - url: String, - additionalOptions: Map = emptyMap() -) { - - /** - * we need to strip the jdbc: part by using schemeSpecificPart - * so that URI is able to parse correctly. - * the jdbc: prefix is added back in toString() - */ - private val uri = URIBuilder( - URI(url).also { - require(it.scheme == "jdbc") { "not a jdbc url: $url" } - }.schemeSpecificPart - ).also { - if (additionalOptions.isNotEmpty()) { - it.addParameters(additionalOptions.map { (k, v) -> BasicNameValuePair(k, v) }) - } - }.build() - - private val urlParameters = uri.query.split('&').associate { - val parts = it.split('=') - val name = parts.firstOrNull() ?: "" - val value = parts.drop(1).firstOrNull() ?: "" - Pair(name, value) - } - - val username: String - get() = urlParameters["user"]!! - val password: String - get() = urlParameters["password"]!! - val database: String - get() = uri.path.split('/').last() - - override fun toString() = "jdbc:$uri" - - /** - * make a copy but change the database name. used in tests - */ - fun withDatabase(database: String): JdbcUrl { - val newUri = URIBuilder(uri).setPath("/$database").build() - return JdbcUrl("jdbc:$newUri") - } -} \ No newline at end of file diff --git a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/executable/BackupRepositoryPerformance.kt b/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/executable/BackupRepositoryPerformance.kt index 2adfb4946..9c0690ebb 100644 --- a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/executable/BackupRepositoryPerformance.kt +++ b/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/executable/BackupRepositoryPerformance.kt @@ -1,10 +1,10 @@ package no.nav.arbeidsgiver.notifikasjon.executable import kotlinx.coroutines.runBlocking +import no.nav.arbeidsgiver.notifikasjon.kafka_backup.KafkaBackup import no.nav.arbeidsgiver.notifikasjon.infrastruktur.Database import no.nav.arbeidsgiver.notifikasjon.infrastruktur.json.laxObjectMapper import no.nav.arbeidsgiver.notifikasjon.kafka_backup.BackupRepository -import no.nav.arbeidsgiver.notifikasjon.kafka_backup.KafkaBackup import no.nav.arbeidsgiver.notifikasjon.util.EksempelHendelse import org.apache.kafka.clients.consumer.ConsumerRecord import java.time.Duration @@ -16,7 +16,7 @@ fun main() = runBlocking { Database.openDatabase( KafkaBackup.databaseConfig.copy( // https://github.com/flyway/flyway/issues/2323#issuecomment-804495818 - jdbcOpts = mapOf("preparedStatementCacheQueries" to "0") + jdbcOpts = mapOf("preparedStatementCacheQueries" to 0) ) ).use { database -> diff --git a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/DatabaseConfigTest.kt b/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/DatabaseConfigTest.kt deleted file mode 100644 index 709d1841e..000000000 --- a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/infrastruktur/DatabaseConfigTest.kt +++ /dev/null @@ -1,71 +0,0 @@ -package no.nav.arbeidsgiver.notifikasjon.infrastruktur - -import io.kotest.core.spec.style.DescribeSpec -import io.kotest.matchers.shouldBe - -class DatabaseConfigTest : DescribeSpec({ - - describe("JdbcUrl") { - it("from jdbcUrl yields correct database, username, password and url") { - val url = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val jdbcUrl = JdbcUrl(url) - - jdbcUrl.database shouldBe "mydb" - jdbcUrl.username shouldBe "myuser" - jdbcUrl.password shouldBe "mypassword" - } - - it("supports additional options via map") { - val url = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val jdbcUrl = JdbcUrl(url, mapOf("foo" to "bar")).toString() - - jdbcUrl shouldBe "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword&foo=bar" - } - - it("can create a copy with another database name") { - val url = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val jdbcUrl = JdbcUrl(url) - - jdbcUrl.withDatabase("anotherdb").toString() shouldBe "jdbc:postgresql://localhost:5432/anotherdb?user=myuser&password=mypassword" - - } - } - - describe("Database.Config") { - it("from jdbcUrl yields correct database, username, password and url") { - val jdbcUrl = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val config = Database.Config( - jdbcUrl = jdbcUrl, - migrationLocations = "", - jdbcOpts = mapOf() - ) - - config.database shouldBe "mydb" - config.username shouldBe "myuser" - config.password shouldBe "mypassword" - config.url.toString() shouldBe jdbcUrl - } - - it("supports additional options via map") { - val jdbcUrl = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val config = Database.Config( - jdbcUrl = jdbcUrl, - migrationLocations = "", - jdbcOpts = mapOf("foo" to "bar") - ) - - config.url.toString() shouldBe "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword&foo=bar" - } - - it("can create a copy with another database name") { - val jdbcUrl = "jdbc:postgresql://localhost:5432/mydb?user=myuser&password=mypassword" - val config = Database.Config( - jdbcUrl = jdbcUrl, - migrationLocations = "", - jdbcOpts = mapOf() - ) - - config.withDatabase("anotherdb").url.toString() shouldBe "jdbc:postgresql://localhost:5432/anotherdb?user=myuser&password=mypassword" - } - } -}) diff --git a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/util/PostgresTestListener.kt b/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/util/PostgresTestListener.kt index 5b29bbff0..6e2d2cd14 100644 --- a/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/util/PostgresTestListener.kt +++ b/app/src/test/kotlin/no/nav/arbeidsgiver/notifikasjon/util/PostgresTestListener.kt @@ -19,14 +19,13 @@ private suspend fun createDbFromTemplate(config: Database.Config): Database.Conf val database = mutex.withLock { "${config.database}_test-${ids.next()}" } - - DriverManager.getConnection(config.url.toString(), config.username, config.password).use { conn -> + DriverManager.getConnection(config.jdbcUrl, config.username, config.password).use { conn -> conn.createStatement().use { stmt -> @Suppress("SqlSourceToSinkFlow") stmt.executeUpdate("""create database "$database" template "$templateDb"; """) } } - return config.withDatabase(database) + return config.copy(database = database) } /** @@ -34,10 +33,10 @@ private suspend fun createDbFromTemplate(config: Database.Config): Database.Conf * template databasen brukes til å lage ferske databaser for hver test. */ @Suppress("SqlSourceToSinkFlow") -private fun templateDb(config: Database.Config): String { +private suspend fun templateDb(config: Database.Config): String { val templateDb = templateDbs.computeIfAbsent(config) { "${config.database}_template".also { db -> - DriverManager.getConnection(config.url.toString(), config.username, config.password).use { conn -> + DriverManager.getConnection(config.jdbcUrl, config.username, config.password).use { conn -> conn.createStatement().use { stmt -> val resultSet = stmt.executeQuery("SELECT datname FROM pg_database where datname like '${config.database}_test%';") @@ -59,7 +58,10 @@ private fun templateDb(config: Database.Config): String { } runBlocking { Database.openDatabase( - config = config.withDatabase(db), + config = config.copy( + port = "1337", + database = db, + ), flywayAction = { migrate() } @@ -72,11 +74,13 @@ private fun templateDb(config: Database.Config): String { fun TestConfiguration.testDatabase(config: Database.Config): Database = runBlocking { - val testConfig = createDbFromTemplate(config) + val database = createDbFromTemplate(config).database Database.openDatabase( - config = testConfig.copy( + config = config.copy( // https://github.com/flyway/flyway/issues/2323#issuecomment-804495818 - jdbcOpts = mapOf("preparedStatementCacheQueries" to "0"), + jdbcOpts = mapOf("preparedStatementCacheQueries" to 0), + port = "1337", + database = database, ), flywayAction = { /* noop. created from template. */