From 2eb6f3d8909702d310d5846cd494d44a93e62925 Mon Sep 17 00:00:00 2001 From: Alexey Illarionov Date: Thu, 16 May 2024 15:05:10 +0300 Subject: [PATCH] Refactor ViolationListener (#279) --- .../initializer/TestStrictModeInitializer.kt | 28 ++++++------- .../DebugStrictModeInitializer.kt | 4 +- .../util/strictmode/IgnoredViolation.kt | 13 +++++++ .../StrictModeViolationException.kt | 8 ++++ .../pixnews/util/strictmode/ViolationExt.kt | 29 ++++++++++---- .../util/strictmode/ViolationListener.kt | 39 +++++++------------ .../util/strictmode/ViolationPolicy.kt | 12 ++++++ 7 files changed, 85 insertions(+), 48 deletions(-) create mode 100644 app/src/main/kotlin/ru/pixnews/util/strictmode/IgnoredViolation.kt create mode 100644 app/src/main/kotlin/ru/pixnews/util/strictmode/StrictModeViolationException.kt create mode 100644 app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationPolicy.kt diff --git a/app/src/androidTest/kotlin/ru/pixnews/inject/initializer/TestStrictModeInitializer.kt b/app/src/androidTest/kotlin/ru/pixnews/inject/initializer/TestStrictModeInitializer.kt index 36044103..71f71921 100644 --- 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 index b9510403..6cc892cc 100644 --- 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 new file mode 100644 index 00000000..da54e759 --- /dev/null +++ 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 new file mode 100644 index 00000000..30897fd6 --- /dev/null +++ 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 index 4c2be8d0..3b885483 100644 --- a/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationExt.kt +++ b/app/src/main/kotlin/ru/pixnews/util/strictmode/ViolationExt.kt @@ -10,47 +10,58 @@ 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 } @@ -58,7 +69,9 @@ internal val isInstanceCountViolation: Violation.() -> Boolean = { * 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 index 7da9473f..e72e941f 100644 --- 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 Boolean> = emptyList(), + allowList: List = 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 Boolean> = emptyList(), + allowList: List = 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 Boolean> = emptyList(), + private val logIgnoredViolation: Boolean = false, + private val allowList: List = 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 new file mode 100644 index 00000000..819fd896 --- /dev/null +++ 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, +}