Refactor ViolationListener (#279)

illarionov · May 16, 2024 · 2eb6f3d · 2eb6f3d
1 parent 5803262
commit 2eb6f3d
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 48 deletions.
diff --git a/app/src/androidTest/kotlin/ru/pixnews/inject/initializer/TestStrictModeInitializer.kt b/app/src/androidTest/kotlin/ru/pixnews/inject/initializer/TestStrictModeInitializer.kt
@@ -17,14 +17,14 @@ import ru.pixnews.anvil.codegen.initializer.inject.ContributesInitializer
 import ru.pixnews.foundation.initializers.Initializer
 import ru.pixnews.inject.DebugStrictModeInitializerModule
 import ru.pixnews.util.strictmode.ViolationPolicy.FAIL
-import ru.pixnews.util.strictmode.isFontRequestViolation
-import ru.pixnews.util.strictmode.isGmsDiskReadViolation
-import ru.pixnews.util.strictmode.isInstanceCountViolation
-import ru.pixnews.util.strictmode.isInstrumentationDexMakerViolation
-import ru.pixnews.util.strictmode.isProfileSizeOfAppViolation
-import ru.pixnews.util.strictmode.isTypefaceFullFlipFontViolation
-import ru.pixnews.util.strictmode.isUntaggedSocketViolation
+import ru.pixnews.util.strictmode.fontRequestViolation
+import ru.pixnews.util.strictmode.gmsDiskReadViolation
+import ru.pixnews.util.strictmode.instanceCountViolation
+import ru.pixnews.util.strictmode.instrumentationDexMakerViolation
+import ru.pixnews.util.strictmode.profileSizeOfAppViolation
 import ru.pixnews.util.strictmode.setupViolationListener
+import ru.pixnews.util.strictmode.typefaceFullFlipFontViolation
+import ru.pixnews.util.strictmode.untaggedSocketViolation
 import javax.inject.Inject
 
 @ContributesInitializer(replaces = [DebugStrictModeInitializerModule::class])
@@ -73,13 +73,13 @@ class TestStrictModeInitializer @Inject constructor(logger: Logger) : Initialize
 
     private companion object {
         val ALLOWLIST = listOf(
-            isFontRequestViolation,
-            isGmsDiskReadViolation,
-            isInstanceCountViolation,
-            isInstrumentationDexMakerViolation,
-            isProfileSizeOfAppViolation,
-            isTypefaceFullFlipFontViolation,
-            isUntaggedSocketViolation,
+            fontRequestViolation,
+            gmsDiskReadViolation,
+            instanceCountViolation,
+            instrumentationDexMakerViolation,
+            profileSizeOfAppViolation,
+            typefaceFullFlipFontViolation,
+            untaggedSocketViolation,
         )
     }
 }
diff --git a/app/src/main/kotlin/ru/pixnews/initializers/DebugStrictModeInitializer.kt b/app/src/main/kotlin/ru/pixnews/initializers/DebugStrictModeInitializer.kt
@@ -13,8 +13,8 @@ import androidx.fragment.app.strictmode.FragmentStrictMode.Policy
 import co.touchlab.kermit.Logger
 import ru.pixnews.foundation.initializers.Initializer
 import ru.pixnews.util.strictmode.ViolationPolicy.LOG
-import ru.pixnews.util.strictmode.isUntaggedSocketViolation
 import ru.pixnews.util.strictmode.setupViolationListener
+import ru.pixnews.util.strictmode.untaggedSocketViolation
 import javax.inject.Inject
 
 @Suppress("MagicNumber")
@@ -40,7 +40,7 @@ class DebugStrictModeInitializer @Inject constructor(logger: Logger) : Initializ
                     logger = logger,
                     policy = LOG,
                     logAllowListViolation = false,
-                    allowList = listOf(isUntaggedSocketViolation),
+                    allowList = listOf(untaggedSocketViolation),
                 )
                 .build(),
         )

diff --git a/app/src/main/kotlin/ru/pixnews/util/strictmode/IgnoredViolation.kt b/app/src/main/kotlin/ru/pixnews/util/strictmode/IgnoredViolation.kt
@@ -0,0 +1,13 @@
+/*
+ * Copyright (c) 2024, the Pixnews project authors and contributors. Please see the AUTHORS file for details.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+ */
+
+package ru.pixnews.util.strictmode
+
+import android.os.strictmode.Violation
+
+class IgnoredViolation(
+    val name: String,
+    val predicate: Violation.() -> Boolean,
+)
diff --git a/app/src/main/kotlin/ru/pixnews/util/strictmode/StrictModeViolationException.kt b/app/src/main/kotlin/ru/pixnews/util/strictmode/StrictModeViolationException.kt
@@ -0,0 +1,8 @@
+/*
+ * Copyright (c) 2024, the Pixnews project authors and contributors. Please see the AUTHORS file for details.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+ */
+
+package ru.pixnews.util.strictmode
+
+class StrictModeViolationException(throwable: Throwable) : RuntimeException(throwable)
diff --git a/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationExt.kt b/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationExt.kt
@@ -10,55 +10,68 @@ import android.os.strictmode.DiskReadViolation
 import android.os.strictmode.DiskWriteViolation
 import android.os.strictmode.InstanceCountViolation
 import android.os.strictmode.UntaggedSocketViolation
-import android.os.strictmode.Violation
 
 /**
  * Disk read/write violations on [androidx.test.runner.MonitoringInstrumentation.specifyDexMakerCacheProperty]
  */
 @SuppressLint("NewApi")
-internal val isInstrumentationDexMakerViolation: Violation.() -> Boolean = {
+val instrumentationDexMakerViolation: IgnoredViolation = IgnoredViolation(
+    "androidx.test.runner.MonitoringInstrumentation#specifyDexMakerCacheProperty Disk Violation",
+) {
     (this is DiskReadViolation || this is DiskWriteViolation) && this.stackTrace.any {
         it.fileName == "MonitoringInstrumentation.java" && it.methodName == "specifyDexMakerCacheProperty"
     }
 }
 
 @SuppressLint("NewApi")
-internal val isTypefaceFullFlipFontViolation: Violation.() -> Boolean = {
+internal val typefaceFullFlipFontViolation: IgnoredViolation = IgnoredViolation(
+    "Typeface.java#{get/set}FullFlipFont Disk Read Violation",
+) {
     this is DiskReadViolation && this.stackTrace.any {
         it.fileName == "Typeface.java" && (it.methodName == "getFullFlipFont" || it.methodName == "setFlipFonts")
     }
 }
 
 @SuppressLint("NewApi")
-internal val isFontRequestViolation: Violation.() -> Boolean = {
+internal val fontRequestViolation: IgnoredViolation = IgnoredViolation(
+    "FontRequestWorker Disk Read Violation",
+) {
     this is DiskReadViolation && this.stackTrace.any {
         it.className == "androidx.core.provider.FontRequestWorker" && it.fileName == "FontRequestWorker.java"
     }
 }
 
 @SuppressLint("NewApi")
-internal val isProfileSizeOfAppViolation: Violation.() -> Boolean = {
+internal val profileSizeOfAppViolation: IgnoredViolation = IgnoredViolation(
+    "ActivityThread.java#getProfileSizeOfApp Disk Read Violation",
+) {
     this is DiskReadViolation && this.stackTrace.any {
         it.fileName == "ActivityThread.java" && it.methodName == "getProfileSizeOfApp"
     }
 }
 
 @SuppressLint("NewApi")
-internal val isUntaggedSocketViolation: Violation.() -> Boolean = {
+internal val untaggedSocketViolation: IgnoredViolation = IgnoredViolation(
+    "Untagged Socket Violation",
+) {
     this is UntaggedSocketViolation
 }
 
 // Class instance limit occasionally triggered in instrumented tests for unknown reasons
 @SuppressLint("NewApi")
-internal val isInstanceCountViolation: Violation.() -> Boolean = {
+internal val instanceCountViolation: IgnoredViolation = IgnoredViolation(
+    "Instance Count Violation",
+) {
     this is InstanceCountViolation
 }
 
 /**
  * Difficult to diagnose violation in GMS
  */
 @SuppressLint("NewApi")
-internal val isGmsDiskReadViolation: Violation.() -> Boolean = {
+internal val gmsDiskReadViolation: IgnoredViolation = IgnoredViolation(
+    ":com.google.android.gms Disk Read Violation",
+) {
     (this is DiskReadViolation) && this.stackTrace.any {
         it.fileName?.contains(":com.google.android.gms") ?: false
     }

diff --git a/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationListener.kt b/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationListener.kt
@@ -23,7 +23,7 @@ internal fun StrictMode.VmPolicy.Builder.setupViolationListener(
     logger: Logger,
     policy: ViolationPolicy = LOG,
     logAllowListViolation: Boolean = false,
-    allowList: List<Violation.() -> Boolean> = emptyList(),
+    allowList: List<IgnoredViolation> = emptyList(),
 ): StrictMode.VmPolicy.Builder = if (VERSION.SDK_INT >= VERSION_CODES.P) {
     val listener = ViolationListener(logger, policy, logAllowListViolation, allowList)
     this.penaltyListener(createThreadExecutor(), listener)
@@ -39,7 +39,7 @@ internal fun StrictMode.ThreadPolicy.Builder.setupViolationListener(
     logger: Logger,
     policy: ViolationPolicy = LOG,
     logAllowListViolation: Boolean = false,
-    allowList: List<Violation.() -> Boolean> = emptyList(),
+    allowList: List<IgnoredViolation> = emptyList(),
 ): StrictMode.ThreadPolicy.Builder = if (VERSION.SDK_INT >= VERSION_CODES.P) {
     val listener = ViolationListener(logger, policy, logAllowListViolation, allowList)
     this.penaltyListener(createThreadExecutor(), listener)
@@ -55,8 +55,8 @@ internal fun StrictMode.ThreadPolicy.Builder.setupViolationListener(
 internal class ViolationListener(
     private val logger: Logger,
     private val policy: ViolationPolicy = LOG,
-    private val logAllowListViolation: Boolean = false,
-    private val allowList: List<Violation.() -> Boolean> = emptyList(),
+    private val logIgnoredViolation: Boolean = false,
+    private val allowList: List<IgnoredViolation> = emptyList(),
 ) : OnVmViolationListener, OnThreadViolationListener {
     override fun onVmViolation(violation: Violation) {
         onViolation("VM", violation)
@@ -70,35 +70,26 @@ internal class ViolationListener(
         type: String,
         violation: Violation,
     ) {
-        if (violation.shouldSkip()) {
-            if (logAllowListViolation) {
+        val ignoredViolation = allowList.find { it.predicate(violation) }
+
+        if (ignoredViolation != null) {
+            if (logIgnoredViolation) {
                 logger.e {
-                    "Skipping $type violation `$violation: message: ${violation.message}`, " +
-                            "stracktrace: ${violation.stackTraceToString()}"
+                    "Skipping ${ignoredViolation.name} ($type/$violation, message: ${violation.message})`"
                 }
             }
-            return
-        }
-        when (policy) {
-            LOG -> logger.e(violation) { "$type violation" }
-            FAIL -> throw StrictModeViolationException(violation)
-            IGNORE -> Unit
+        } else {
+            when (policy) {
+                LOG -> logger.e(violation) { "$type violation" }
+                FAIL -> throw StrictModeViolationException(violation)
+                IGNORE -> Unit
+            }
         }
     }
 
-    private fun Violation.shouldSkip(): Boolean = allowList.any { it() }
-
     companion object {
         fun createThreadExecutor(): Executor = Executor {
             it.run()
         }
     }
 }
-
-class StrictModeViolationException(throwable: Throwable) : RuntimeException(throwable)
-
-enum class ViolationPolicy {
-    IGNORE,
-    LOG,
-    FAIL,
-}
diff --git a/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationPolicy.kt b/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationPolicy.kt
@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) 2024, the Pixnews project authors and contributors. Please see the AUTHORS file for details.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+ */
+
+package ru.pixnews.util.strictmode
+
+enum class ViolationPolicy {
+    IGNORE,
+    LOG,
+    FAIL,
+}