From 4c6afb1b66c4e2e011ed22a3cc5fbe58151ffcfb Mon Sep 17 00:00:00 2001 From: davidSchuppa Date: Mon, 30 Sep 2024 15:42:01 +0200 Subject: [PATCH] feat(logger): prevent persistence of methodNotAllowed logs SUITEDEV-36662 Co-authored-by: LasOri <24588073+LasOri@users.noreply.github.com> Co-authored-by: megamegax --- .../com/emarsys/core/util/log/LoggerTest.kt | 38 +++++++++++++------ .../java/com/emarsys/core/util/log/Logger.kt | 20 ++++++++-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/core/src/androidTest/java/com/emarsys/core/util/log/LoggerTest.kt b/core/src/androidTest/java/com/emarsys/core/util/log/LoggerTest.kt index f10b2764..226785fd 100644 --- a/core/src/androidTest/java/com/emarsys/core/util/log/LoggerTest.kt +++ b/core/src/androidTest/java/com/emarsys/core/util/log/LoggerTest.kt @@ -1,5 +1,6 @@ package com.emarsys.core.util.log +import android.content.Context import com.emarsys.core.concurrency.ConcurrentHandlerHolderFactory import com.emarsys.core.database.repository.Repository import com.emarsys.core.database.repository.SqlSpecification @@ -14,7 +15,9 @@ import com.emarsys.core.provider.uuid.UUIDProvider import com.emarsys.core.shard.ShardModel import com.emarsys.core.storage.StringStorage import com.emarsys.core.util.log.entry.LogEntry +import com.emarsys.core.util.log.entry.MethodNotAllowed import com.emarsys.testUtil.AnnotationSpec +import com.emarsys.testUtil.InstrumentationRegistry import com.emarsys.testUtil.mockito.ThreadSpy import io.kotest.matchers.shouldBe import kotlinx.coroutines.runBlocking @@ -41,19 +44,20 @@ class LoggerTest : AnnotationSpec() { private lateinit var concurrentHandlerHolder: ConcurrentHandlerHolder - private lateinit var shardRepositoryMock: Repository + private lateinit var mockShardRepository: Repository private lateinit var timestampProviderMock: TimestampProvider private lateinit var uuidProviderMock: UUIDProvider private lateinit var dependencyContainer: CoreComponent private lateinit var loggerInstance: Logger private lateinit var loggerMock: Logger private lateinit var mockLogLevelStorage: StringStorage + private lateinit var context: Context @Before @Suppress("UNCHECKED_CAST") fun init() { concurrentHandlerHolder = ConcurrentHandlerHolderFactory.create() - shardRepositoryMock = mock() + mockShardRepository = mock() timestampProviderMock = mock().apply { whenever(provideTimestamp()).thenReturn(TIMESTAMP) } @@ -62,20 +66,21 @@ class LoggerTest : AnnotationSpec() { } mockLogLevelStorage = mock() + context = InstrumentationRegistry.getTargetContext() loggerInstance = Logger( concurrentHandlerHolder, - shardRepositoryMock, + mockShardRepository, timestampProviderMock, uuidProviderMock, mockLogLevelStorage, false, - mock() + context ) loggerMock = mock() dependencyContainer = FakeCoreDependencyContainer( concurrentHandlerHolder = concurrentHandlerHolder, - shardRepository = shardRepositoryMock, + shardRepository = mockShardRepository, timestampProvider = timestampProviderMock, uuidProvider = uuidProviderMock, logger = loggerMock @@ -106,7 +111,7 @@ class LoggerTest : AnnotationSpec() { val captor = ArgumentCaptor.forClass(ShardModel::class.java) runBlocking { - verify(shardRepositoryMock, timeout(100)).add(capture(captor)) + verify(mockShardRepository, timeout(100)).add(capture(captor)) } captor.value shouldBe ShardModel( @@ -124,6 +129,17 @@ class LoggerTest : AnnotationSpec() { ) } + @Test + fun testPersistLog_shouldNotAddLog_toShardRepository_whenLogEntryIsMethodNotAllowed() { + val methodNotAllowedLog = MethodNotAllowed(this::class.java, "testMethodName", mapOf()) + + loggerInstance.handleLog(LogLevel.ERROR, methodNotAllowedLog, null) + + waitForTask() + + verify(mockShardRepository, times(0)).add(any()) + } + @Test fun testPersistLog_addsOtherLog_toShardRepository() { loggerInstance.persistLog( @@ -135,7 +151,7 @@ class LoggerTest : AnnotationSpec() { val captor = ArgumentCaptor.forClass(ShardModel::class.java) runBlocking { - verify(shardRepositoryMock, timeout(100)).add(capture(captor)) + verify(mockShardRepository, timeout(100)).add(capture(captor)) } captor.value shouldBe ShardModel( UUID, @@ -153,7 +169,7 @@ class LoggerTest : AnnotationSpec() { fun testPersistLog_addsLog_toShardRepository_viaCoreSdkHandler() { val threadSpy = ThreadSpy() runBlocking { - org.mockito.Mockito.doAnswer(threadSpy).`when`(shardRepositoryMock).add(any()) + org.mockito.Mockito.doAnswer(threadSpy).`when`(mockShardRepository).add(any()) } loggerInstance.persistLog(LogLevel.ERROR, logEntryMock(), "testThreadName", null) @@ -208,7 +224,7 @@ class LoggerTest : AnnotationSpec() { ) { latch.countDown() } latch.await() runBlocking { - verify(shardRepositoryMock, timeout(100).times(0)).add(any()) + verify(mockShardRepository, timeout(100).times(0)).add(any()) } } @@ -225,7 +241,7 @@ class LoggerTest : AnnotationSpec() { ) { latch.countDown() } latch.await() runBlocking { - verify(shardRepositoryMock, times(0)).add(any()) + verify(mockShardRepository, times(0)).add(any()) } } @@ -243,7 +259,7 @@ class LoggerTest : AnnotationSpec() { ) { latch.countDown() } latch.await() runBlocking { - verify(shardRepositoryMock, times(1)).add(any()) + verify(mockShardRepository, times(1)).add(any()) } } diff --git a/core/src/main/java/com/emarsys/core/util/log/Logger.kt b/core/src/main/java/com/emarsys/core/util/log/Logger.kt index a45ee82c..2ed71136 100644 --- a/core/src/main/java/com/emarsys/core/util/log/Logger.kt +++ b/core/src/main/java/com/emarsys/core/util/log/Logger.kt @@ -15,8 +15,18 @@ import com.emarsys.core.provider.uuid.UUIDProvider import com.emarsys.core.provider.wrapper.WrapperInfoContainer import com.emarsys.core.shard.ShardModel import com.emarsys.core.storage.Storage -import com.emarsys.core.util.log.LogLevel.* -import com.emarsys.core.util.log.entry.* +import com.emarsys.core.util.log.LogLevel.DEBUG +import com.emarsys.core.util.log.LogLevel.ERROR +import com.emarsys.core.util.log.LogLevel.INFO +import com.emarsys.core.util.log.LogLevel.METRIC +import com.emarsys.core.util.log.LogLevel.TRACE +import com.emarsys.core.util.log.LogLevel.WARN +import com.emarsys.core.util.log.LogLevel.valueOf +import com.emarsys.core.util.log.entry.CrashLog +import com.emarsys.core.util.log.entry.LogEntry +import com.emarsys.core.util.log.entry.MethodNotAllowed +import com.emarsys.core.util.log.entry.asString +import com.emarsys.core.util.log.entry.toData @Mockable class Logger( @@ -80,13 +90,15 @@ class Logger( fun handleLog(logLevel: LogLevel, logEntry: LogEntry, onCompleted: (() -> Unit)? = null) { val currentThreadName = Thread.currentThread().name - core().concurrentHandlerHolder.coreHandler.post { + concurrentHandlerHolder.coreHandler.post { val isDebugMode: Boolean = 0 != context.applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE if ((verboseConsoleLoggingEnabled || logEntry is MethodNotAllowed) && isDebugMode) { logToConsole(logLevel, logEntry) } - persistLog(logLevel, logEntry, currentThreadName, onCompleted) + if (logEntry !is MethodNotAllowed) { + persistLog(logLevel, logEntry, currentThreadName, onCompleted) + } } }