Skip to content

WiP: Improve matching of DataSourceUrl #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,72 @@
package com.github.yusufugurozbek.testcontainers.port.updater

data class DataSourceUrl(val beforePort: String, val port: String, val afterPort: String?) {
import com.github.yusufugurozbek.testcontainers.port.updater.settings.MatchMode
import com.github.yusufugurozbek.testcontainers.port.updater.settings.MatchMode.*
import com.intellij.database.dataSource.LocalDataSource
import com.intellij.database.util.common.isNotNullOrEmpty

data class DataSourceUrl(val beforePort: String, val port: String, val afterPort: String?, private val urlExtractor: DataSourceUrlExtractor) {

constructor(beforePort: String, port: String, afterPort: String?) : this(beforePort, port, afterPort, DataSourceUrlExtractor())

init {
setUrlExtractor(urlExtractor)
}

companion object {
private lateinit var extractor: DataSourceUrlExtractor

private fun setUrlExtractor(extractor: DataSourceUrlExtractor) {
if (!::extractor.isInitialized) {
this.extractor = extractor
}
}
Comment on lines +8 to +23
Copy link
Collaborator Author

@jimonthebarn jimonthebarn Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yusufugurozbek
This seems a little convoluted here. The issue I was trying to solve is that for whatever reason the test classes could not find the constructor anymore if i define it it with a default value for the extractor like such:

data class DataSourceUrl(val beforePort: String, val port: String, val afterPort: String?, private val urlExtractor: DataSourceUrlExtractor = DataSourceUrlExtractor())


fun from(dataSource: LocalDataSource): DataSourceUrl? = from(dataSource.url)

fun from(url: String?): DataSourceUrl? {
return url?.takeIf { it.isNotNullOrEmpty }
?.let { extractor.extract(it)?.let(::toDataSourceUrl) }
}

private fun toDataSourceUrl(matchResult: MatchResult?): DataSourceUrl? {
val groups = matchResult?.groups
val beforePort = groups?.get("beforePort")?.value
val port = groups?.get("port")?.value
val afterPort = groups?.get("afterPort")?.value

return if (beforePort != null && port != null) {
DataSourceUrl(beforePort, port, afterPort)
} else {
null
}
}
}

fun matches(other: DataSourceUrl, matchMode: MatchMode): Boolean =
when (matchMode) {
EXACT -> this.equalsIgnoringPort(other)
EVERYTHING -> this.beforePort == other.beforePort
WITH_TESTCONTAINERS_PARAMETER ->
this.toString().contains("testcontainers=true") and
(this.beforePort == other.beforePort)
}

private fun equalsIgnoringPort(other: DataSourceUrl): Boolean =
(this.beforePort == other.beforePort) and (this.afterPort == other.afterPort)

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is DataSourceUrl) return false

return beforePort == other.beforePort &&
port == other.port &&
afterPort == other.afterPort
}

override fun hashCode(): Int {
return 31 * beforePort.hashCode() + port.hashCode() + (afterPort?.hashCode() ?: 0)
}

override fun toString(): String = "$beforePort:$port$afterPort"
}

fun DataSourceUrl.equalsIgnoringPort(other: DataSourceUrl): Boolean =
(this.beforePort == other.beforePort) and (this.afterPort == other.afterPort)
Original file line number Diff line number Diff line change
@@ -1,42 +1,11 @@
package com.github.yusufugurozbek.testcontainers.port.updater

import com.intellij.database.dataSource.LocalDataSource

class DataSourceUrlExtractor {
Copy link
Collaborator Author

@jimonthebarn jimonthebarn Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yusufugurozbek
Now that this actually only extracts the DataSourcesUrls - "segments" I'm not too happy with the naming yet. Whats your take on this? Maybe DataSourceComponentExtractor or DataSourceComponentMatcher which is a better hint at what is actually being returned. Yup naming is hard. :)


companion object {
private val regex: Regex = "\\s*(?<beforePort>.*):(?<port>\\d{1,5})(?<afterPort>.*;\\S*|\\S*)".toRegex()
}

internal fun extract(from: String): DataSourceUrl? {
val matchResult = regex.find(from)
return toDataSourceUrl(matchResult)
}

internal fun toDataSourceUrl(from: LocalDataSource): DataSourceUrl? {
if (from.url.isNullOrEmpty()) {
return null
}
val matchResult = regex.find(from.url!!)
return toDataSourceUrl(matchResult)
}

private fun toDataSourceUrl(matchResult: MatchResult?): DataSourceUrl? {
if (matchResult == null) {
return null
}
fun extract(from: String?): MatchResult? = from?.let { regex.find(from) }

val beforePort = matchResult.groups["beforePort"]
val port = matchResult.groups["port"]

if (beforePort == null || port == null) {
return null
}

val beforePortValue = beforePort.value
val portValue = port.value
val afterPortValue = matchResult.groups["afterPort"]?.value

return DataSourceUrl(beforePortValue, portValue, afterPortValue)
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package com.github.yusufugurozbek.testcontainers.port.updater.impl

import com.github.yusufugurozbek.testcontainers.port.updater.DataSourceUrl
import com.github.yusufugurozbek.testcontainers.port.updater.DataSourceUrlExtractor
import com.github.yusufugurozbek.testcontainers.port.updater.api.DataSourceUpdater
import com.github.yusufugurozbek.testcontainers.port.updater.common.TpuNotifier
import com.github.yusufugurozbek.testcontainers.port.updater.equalsIgnoringPort
import com.github.yusufugurozbek.testcontainers.port.updater.settings.LoggingFormat
import com.github.yusufugurozbek.testcontainers.port.updater.settings.MatchMode
import com.github.yusufugurozbek.testcontainers.port.updater.settings.TpuSettingsState
import com.intellij.database.dataSource.LocalDataSource
import com.intellij.openapi.components.service
Expand All @@ -18,15 +15,16 @@ import kotlinx.serialization.json.jsonPrimitive

class DataSourceUpdaterImpl(private var project: Project) : DataSourceUpdater {

private var urlExtractor: DataSourceUrlExtractor = DataSourceUrlExtractor()

private var settingsState: TpuSettingsState = project.service()

override fun update(localDataSources: List<LocalDataSource>, logEntryText: String) {
val logEntry = getLogMessage(logEntryText)

val splitLogEntry = logEntry.split(settingsState.logEntryPrefix)
if (splitLogEntry.size == 2) {
urlExtractor.extract(splitLogEntry[1])?.let { logEntryDataSourceUrl ->
val message = splitLogEntry[1]
DataSourceUrl.from(message)?.let { logEntryDataSourceUrl ->
localDataSources
.filter { it.url != logEntryDataSourceUrl.toString() }
.forEach { update(it, logEntryDataSourceUrl) }
Expand All @@ -37,7 +35,7 @@ class DataSourceUpdaterImpl(private var project: Project) : DataSourceUpdater {
private fun getLogMessage(logEntryText: String): String {
if (settingsState.loggingFormat == LoggingFormat.JSON && logEntryText.contains(settingsState.logEntryPrefix)) {
try {
return Json.parseToJsonElement(logEntryText).jsonObject["message"]!!.jsonPrimitive.content
return Json.parseToJsonElement(logEntryText).jsonObject["message"]?.jsonPrimitive?.content ?: ""
} catch (e: Exception) {
thisLogger().warn("JSON log message cannot be extracted", e)
}
Expand All @@ -46,22 +44,16 @@ class DataSourceUpdaterImpl(private var project: Project) : DataSourceUpdater {
}

private fun update(localDataSource: LocalDataSource, logEntryDataSourceUrl: DataSourceUrl) {
val localDataSourceUrl = urlExtractor.toDataSourceUrl(localDataSource) ?: return
val dataSourceUrl = DataSourceUrl.from(localDataSource) ?: return

if (localDataSourceUrl.port == logEntryDataSourceUrl.port) {
if (dataSourceUrl.port == logEntryDataSourceUrl.port) {
return
}

val isUpdatable = when (settingsState.matchMode) {
MatchMode.EXACT -> localDataSourceUrl.equalsIgnoringPort(logEntryDataSourceUrl)
MatchMode.EVERYTHING -> localDataSourceUrl.beforePort == logEntryDataSourceUrl.beforePort
MatchMode.WITH_TESTCONTAINERS_PARAMETER ->
localDataSourceUrl.toString().contains("testcontainers=true") and
(localDataSourceUrl.beforePort == logEntryDataSourceUrl.beforePort)
}
val isUpdatable = dataSourceUrl.matches(logEntryDataSourceUrl, settingsState.matchMode)

if (isUpdatable) {
val newUrl = localDataSourceUrl.toString().replace(localDataSourceUrl.port, logEntryDataSourceUrl.port)
val newUrl = dataSourceUrl.copy(port = logEntryDataSourceUrl.port).toString()
localDataSource.url = newUrl
if (settingsState.isNotificationsEnabled) {
TpuNotifier.notify(project, "Updated data source URL: $newUrl")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.github.yusufugurozbek.testcontainers.port.updater

import com.github.yusufugurozbek.testcontainers.port.updater.settings.MatchMode
import com.intellij.testFramework.UsefulTestCase
import junit.framework.TestCase
import junit.framework.TestCase.*

internal class DataSourceUrlExtractorTest : UsefulTestCase() {

private val sut = DataSourceUrlExtractor()

fun `test parts can be successfully extracted from well formed input`() {
val result = sut.extract("jdbc:postgresql://localhost:11111/test")

assertEquals("jdbc:postgresql://localhost:11111/test", result?.groups?.get(0)?.value)
assertEquals("jdbc:postgresql://localhost", result?.groups?.get(1)?.value)
assertEquals("11111", result?.groups?.get(2)?.value)
assertEquals("/test", result?.groups?.get(3)?.value)
}

fun `test extraction of null value returns null`() {
assertNull(sut.extract(null))
}

fun `test extraction of empty value returns null`() {
assertNull(sut.extract(""))
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.github.yusufugurozbek.testcontainers.port.updater

import com.github.yusufugurozbek.testcontainers.port.updater.settings.MatchMode
import com.intellij.testFramework.UsefulTestCase

internal class DataSourceUrlTest : UsefulTestCase() {
Expand All @@ -21,6 +22,82 @@ internal class DataSourceUrlTest : UsefulTestCase() {
fun `test DataSourceUrl toString successfully returns full URL`() {
val dataSourceUrl = DataSourceUrl("beforePort", "port", "?afterPort")

assertEquals(dataSourceUrl.toString(), "beforePort:port?afterPort")
assertEquals("beforePort:port?afterPort", dataSourceUrl.toString())
}

fun `test datasource from string successfully returns result`() {
val result = DataSourceUrl.from("jdbc:postgresql://localhost:11111/test")!!

assertEquals("jdbc:postgresql://localhost", result.beforePort)
assertEquals("11111", result.port)
assertEquals("/test", result.afterPort)
}

fun `test datasource from string returns null on null input`() {
val result = DataSourceUrl.from(null)

assertNull(result)
}

fun `test datasource from string returns null on empty input`() {
val result = DataSourceUrl.from("")

assertNull(result)
}

fun `test matchMode EXACT successfully returns true for matching urls`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePort", "port", "afterPort")

assertTrue(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EXACT))
}

fun `test matchMode EXACT returns false for not matching beforePort`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePortNoMatch", "port", "afterPort")

assertFalse(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EXACT))
}

fun `test matchMode EXACT returns true if everything but port matches`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePort", "portNoMatch", "afterPort")

assertTrue(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EXACT))
}

fun `test matchMode EXACT returns false for not matching afterPort`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePort", "port", "afterPortNoMatch")

assertFalse(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EXACT))
}

fun `test matchMode EVERYTHING successfully returns true if beforePort matches`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "portA", "afterPortA")
val dataSourceUrlB = DataSourceUrl("beforePort", "portB", "afterPortB")

assertTrue(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EVERYTHING))
}

fun `test matchMode EVERYTHING returns false for not matching beforePort`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePortNoMatch", "port", "afterPort")

assertFalse(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.EVERYTHING))
}

fun `test matchMode WITH_TESTCONTAINERS_PARAMETER successfully returns true for matching beforePort and afterPort containing testcontainers=true`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "dontCareA", "testcontainers=true")
val dataSourceUrlB = DataSourceUrl("beforePort", "dontCareB", "dontCareC")
Copy link
Collaborator Author

@jimonthebarn jimonthebarn Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yusufugurozbek
The match mode WITH_TESTCONTAINERS_PARAMETER seems a little weird to me. Especially the condition we're currently checking here which essentially requires for the current DataSourceUrl to have "testcontainers=true" within afterPort but it doesn't care whether the new DataSourceUrl also has it set. This follows the current implementation which I did not want to change as I'm oblivious to the intended semantic for the match mode.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check this issue before we discuss it further?


assertTrue(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.WITH_TESTCONTAINERS_PARAMETER))
}

fun `test matchMode WITH_TESTCONTAINERS_PARAMETER returns false for missing afterPort containing testcontainers=true`() {
val dataSourceUrlA = DataSourceUrl("beforePort", "port", "afterPort")
val dataSourceUrlB = DataSourceUrl("beforePort", "port", "afterPort")

assertFalse(dataSourceUrlA.matches(dataSourceUrlB, MatchMode.WITH_TESTCONTAINERS_PARAMETER))
}
}
Loading