Clean up Logging.log

Split log level and alert level logging into their own methods to make
the code easier to follow.

Author: jkasten2

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt

@@ -89,45 +89,59 @@ object Logging {
         throwable: Throwable?,
     ) {
         val fullMessage = "[${Thread.currentThread().name}] $message"
-        if (level.compareTo(logLevel) < 1) {
-            when (level) {
-                LogLevel.VERBOSE -> android.util.Log.v(TAG, fullMessage, throwable)
-                LogLevel.DEBUG -> android.util.Log.d(TAG, fullMessage, throwable)
-                LogLevel.INFO -> android.util.Log.i(TAG, fullMessage, throwable)
-                LogLevel.WARN -> android.util.Log.w(TAG, fullMessage, throwable)
-                LogLevel.ERROR, LogLevel.FATAL -> android.util.Log.e(TAG, message, throwable)
-                else -> {}
-            }
+
+        logToLogcat(level, fullMessage, throwable)
+        showVisualLogging(level, fullMessage, throwable)
+        callLogListeners(level, fullMessage, throwable)
+    }
+
+    private fun logToLogcat(
+        level: LogLevel,
+        message: String,
+        throwable: Throwable?,
+    ) {
+        if (level.compareTo(logLevel) >= 1) return
+        when (level) {
+            LogLevel.VERBOSE -> android.util.Log.v(TAG, message, throwable)
+            LogLevel.DEBUG -> android.util.Log.d(TAG, message, throwable)
+            LogLevel.INFO -> android.util.Log.i(TAG, message, throwable)
+            LogLevel.WARN -> android.util.Log.w(TAG, message, throwable)
+            LogLevel.ERROR, LogLevel.FATAL -> android.util.Log.e(TAG, message, throwable)
+            else -> {}
         }
+    }
 
-        if (level.compareTo(visualLogLevel) < 1 && applicationService?.current != null) {
-            try {
-                var fullMessage: String? = "$message\n".trimIndent()
-                if (throwable != null) {
-                    fullMessage += throwable.message
-                    val sw = StringWriter()
-                    val pw = PrintWriter(sw)
-                    throwable.printStackTrace(pw)
-                    fullMessage += sw.toString()
-                }
-                val finalFullMessage = fullMessage
-
-                suspendifyOnMain {
-                    val currentActivity = applicationService?.current
-                    if (currentActivity != null) {
-                        AlertDialog
-                            .Builder(currentActivity)
-                            .setTitle(level.toString())
-                            .setMessage(finalFullMessage)
-                            .show()
-                    }
+    private fun showVisualLogging(
+        level: LogLevel,
+        message: String,
+        throwable: Throwable?,
+    ) {
+        if (level.compareTo(visualLogLevel) >= 1) return
+
+        try {
+            var fullMessage: String? = "$message\n".trimIndent()
+            if (throwable != null) {
+                fullMessage += throwable.message
+                val sw = StringWriter()
+                val pw = PrintWriter(sw)
+                throwable.printStackTrace(pw)
+                fullMessage += sw.toString()
+            }
+            val finalFullMessage = fullMessage
+
+            suspendifyOnMain {
+                val currentActivity = applicationService?.current
+                if (currentActivity != null) {
+                    AlertDialog
+                        .Builder(currentActivity)
+                        .setTitle(level.toString())
+                        .setMessage(finalFullMessage)
+                        .show()
                 }
-            } catch (t: Throwable) {
-                android.util.Log.e(TAG, "Error showing logging message.", t)
             }
+        } catch (t: Throwable) {
+            android.util.Log.e(TAG, "Error showing logging message.", t)
         }
-
-        callLogListeners(level, message, throwable)
     }
 
     private fun callLogListeners(

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt

@@ -6,6 +6,7 @@ import com.onesignal.debug.OneSignalLogEvent
 import com.onesignal.debug.internal.logging.Logging
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.string.shouldEndWith
 
 class TestLogLister : ILogListener {
     val calls = ArrayList<String>()
@@ -15,6 +16,11 @@ class TestLogLister : ILogListener {
     }
 }
 
+infix fun <T : Collection<String>> T.shouldHaveEachItemEndWith(expected: Array<String>): T {
+    this.forEachIndexed { index, it -> it shouldEndWith expected[index] }
+    return this
+}
+
 class LoggingTests : FunSpec({
     beforeAny {
         Logging.logLevel = LogLevel.NONE
@@ -29,7 +35,7 @@ class LoggingTests : FunSpec({
         Logging.debug("test")
 
         // Then
-        listener.calls shouldBe arrayOf("test")
+        listener.calls shouldHaveEachItemEndWith arrayOf("test")
     }
 
     test("addListener twice") {
@@ -42,7 +48,7 @@ class LoggingTests : FunSpec({
         Logging.debug("test")
 
         // Then
-        listener.calls shouldBe arrayOf("test")
+        listener.calls shouldHaveEachItemEndWith arrayOf("test")
     }
 
     test("removeListener") {
@@ -83,7 +89,7 @@ class LoggingTests : FunSpec({
         Logging.debug("test3")
 
         // Then
-        nestedListener.calls shouldBe arrayOf("test2", "test3")
+        nestedListener.calls shouldHaveEachItemEndWith arrayOf("test2", "test3")
     }
 
     test("removeListener nested") {
@@ -102,6 +108,6 @@ class LoggingTests : FunSpec({
         Logging.debug("test2")
 
         // Then
-        calls shouldBe arrayOf("test")
+        calls shouldHaveEachItemEndWith arrayOf("test")
     }
 })