From eeda91743a9781e018c171b6103baf4b61f088b4 Mon Sep 17 00:00:00 2001
From: Jocelyne <jocelyne.abihaidar@jetbrains.com>
Date: Thu, 24 Aug 2023 12:51:30 +0200
Subject: [PATCH] fix: Remove false warning log

-This warning was first eliminated here https://github.com/JetBrains/Exposed/pull/541/files. Then it was accidentally introduced again here https://github.com/JetBrains/Exposed/commit/77f11a99adf18e7b0db863b58c68d2d9ab8cd1ff because of the change made to the getOrNull function.
-A new dependency was added (io.github.hakky54:logcaptor) to assert that no logging took place
---
 .../org/jetbrains/exposed/sql/ResultRow.kt    | 44 ++++++++++---------
 exposed-tests/build.gradle.kts                |  1 +
 .../exposed/sql/tests/shared/dml/JoinTests.kt | 10 ++++-
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ResultRow.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ResultRow.kt
index 22ee8d40ca..23afb52e43 100644
--- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ResultRow.kt
+++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ResultRow.kt
@@ -19,16 +19,34 @@ class ResultRow(
      *
      * @see [getOrNull] to get null in the cases an exception would be thrown
      */
-    operator fun <T> get(expression: Expression<T>): T {
+    operator fun <T> get(expression: Expression<T>): T = getInternal(expression, checkNullability = true)
+
+    operator fun <T> set(expression: Expression<out T>, value: T) {
+        setInternal(expression, value)
+        lookUpCache.remove(expression)
+    }
+
+    private fun <T> setInternal(expression: Expression<out T>, value: T) {
+        val index = getExpressionIndex(expression)
+        data[index] = value
+    }
+
+    fun <T> hasValue(expression: Expression<T>): Boolean = fieldIndex[expression]?.let { data[it] != NotInitializedValue } ?: false
+
+    fun <T> getOrNull(expression: Expression<T>): T? = if (hasValue(expression)) getInternal(expression, checkNullability = false) else null
+
+    private fun <T> getInternal(expression: Expression<T>, checkNullability: Boolean): T {
         if (expression in lookUpCache) return lookUpCache[expression] as T
 
         val d = getRaw(expression)
 
-        if (d == null && expression is Column<*> && expression.dbDefaultValue != null && !expression.columnType.nullable) {
-            exposedLogger.warn(
-                "Column ${TransactionManager.current().fullIdentity(expression)} is marked as not null, " +
-                    "has default db value, but returns null. Possible have to re-read it from DB."
-            )
+        if (checkNullability) {
+            if (d == null && expression is Column<*> && expression.dbDefaultValue != null && !expression.columnType.nullable) {
+                exposedLogger.warn(
+                    "Column ${TransactionManager.current().fullIdentity(expression)} is marked as not null, " +
+                        "has default db value, but returns null. Possible have to re-read it from DB."
+                )
+            }
         }
 
         val result = database?.dialect?.let {
@@ -40,20 +58,6 @@ class ResultRow(
         return result
     }
 
-    operator fun <T> set(expression: Expression<out T>, value: T) {
-        setInternal(expression, value)
-        lookUpCache.remove(expression)
-    }
-
-    private fun <T> setInternal(expression: Expression<out T>, value: T) {
-        val index = getExpressionIndex(expression)
-        data[index] = value
-    }
-
-    fun <T> hasValue(expression: Expression<T>): Boolean = fieldIndex[expression]?.let { data[it] != NotInitializedValue } ?: false
-
-    fun <T> getOrNull(expression: Expression<T>): T? = if (hasValue(expression)) get(expression) else null
-
     @Suppress("UNCHECKED_CAST")
     private fun <T> rawToColumnValue(raw: T?, expression: Expression<T>): T {
         return when {
diff --git a/exposed-tests/build.gradle.kts b/exposed-tests/build.gradle.kts
index 3841af3a7c..e823be481f 100644
--- a/exposed-tests/build.gradle.kts
+++ b/exposed-tests/build.gradle.kts
@@ -33,6 +33,7 @@ dependencies {
     testCompileOnly("com.impossibl.pgjdbc-ng", "pgjdbc-ng", Versions.postgreNG)
     compileOnly("com.h2database", "h2", Versions.h2)
     testCompileOnly("org.xerial", "sqlite-jdbc", Versions.sqlLite3)
+    testImplementation("io.github.hakky54:logcaptor:2.9.0")
 }
 
 tasks.withType<Test>().configureEach {
diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/JoinTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/JoinTests.kt
index c459a7c497..552bff9428 100644
--- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/JoinTests.kt
+++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/JoinTests.kt
@@ -1,5 +1,6 @@
 package org.jetbrains.exposed.sql.tests.shared.dml
 
+import nl.altindag.log.LogCaptor
 import org.jetbrains.exposed.dao.id.IntIdTable
 import org.jetbrains.exposed.sql.*
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
@@ -190,7 +191,10 @@ class JoinTests : DatabaseTestsBase() {
         }
     }
 
-    @Test fun testNoWarningsOnLeftJoinRegression() {
+    @Test
+    fun testNoWarningsOnLeftJoinRegression() {
+        val logCaptor = LogCaptor.forName(exposedLogger.name)
+
         val MainTable = object : Table("maintable") {
             val id = integer("idCol")
         }
@@ -208,7 +212,9 @@ class JoinTests : DatabaseTestsBase() {
                 .single()
                 .getOrNull(JoinTable.data)
 
-            // Assert no logging took place. No idea how to.
+            // Assert no logging took place
+            assertTrue(logCaptor.warnLogs.isEmpty())
+            assertTrue(logCaptor.errorLogs.isEmpty())
         }
     }
 }