From ecbec0df91c147b03517325bc19a0f1917f3b289 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 24 Apr 2025 15:02:07 +0100 Subject: [PATCH 1/2] MS-120 Skipping non-existent privacy notice download 404 logging --- .../config/store/ConfigRepositoryImpl.kt | 7 ++- .../config/store/ConfigRepositoryImplTest.kt | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt b/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt index 8f6a72f276..9018edbabd 100644 --- a/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt +++ b/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt @@ -24,6 +24,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.FlowCollector import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map +import retrofit2.HttpException import javax.inject.Inject internal class ConfigRepositoryImpl @Inject constructor( @@ -36,6 +37,7 @@ internal class ConfigRepositoryImpl @Inject constructor( companion object { @VisibleForTesting const val PRIVACY_NOTICE_FILE = "privacy_notice" + private const val NOT_FOUND_STATUS_CODE = 404 } override suspend fun getProject(): Project = localDataSource.getProject() @@ -125,7 +127,10 @@ internal class ConfigRepositoryImpl @Inject constructor( localDataSource.storePrivacyNotice(projectId, language, privacyNotice) flowCollector.emit(Succeed(language, privacyNotice)) } catch (t: Throwable) { - Simber.i("Failed to download privacy notice", t) + if ((t.cause as? HttpException)?.code() != NOT_FOUND_STATUS_CODE) { + // Non-existence of resource isn't considered a download failure + Simber.i("Failed to download privacy notice", t) + } flowCollector.emit( if (t is BackendMaintenanceException) { FailedBecauseBackendMaintenance(language, t, t.estimatedOutage) diff --git a/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt b/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt index 47288dae0c..8e562538bf 100644 --- a/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt +++ b/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt @@ -17,9 +17,15 @@ import com.simprints.infra.config.store.testtools.deviceState import com.simprints.infra.config.store.testtools.project import com.simprints.infra.config.store.testtools.projectConfiguration import com.simprints.infra.config.store.tokenization.TokenizationProcessor +import com.simprints.infra.logging.Simber import com.simprints.infra.network.SimNetwork import com.simprints.infra.network.exceptions.BackendMaintenanceException import com.simprints.testtools.common.syntax.assertThrows +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import okhttp3.ResponseBody.Companion.toResponseBody +import retrofit2.HttpException +import retrofit2.Response import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -27,6 +33,7 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest +import org.junit.After import org.junit.Before import org.junit.Test @@ -47,6 +54,7 @@ class ConfigRepositoryImplTest { @Before fun setup() { + mockkStatic(Simber::class) configServiceImpl = ConfigRepositoryImpl( localDataSource, remoteDataSource, @@ -56,6 +64,11 @@ class ConfigRepositoryImplTest { ) } + @After + fun cleanup() { + unmockkStatic(Simber::class) + } + @Test fun `should get the project locally if available`() = runTest { coEvery { localDataSource.getProject() } returns project @@ -260,4 +273,42 @@ class ConfigRepositoryImplTest { assertThat(result).isEqualTo(config) coVerify { localDataSource.getProjectConfiguration() } } + + @Test + fun `should log when failing to download privacy notice with non-404 response status code`() = runTest { + val code = 500 + val exception = Exception("Server error").apply { + initCause(HttpException(Response.error(code, "".toResponseBody(null)))) + } + every { localDataSource.hasPrivacyNoticeFor(PROJECT_ID, LANGUAGE) } returns false + coEvery { + remoteDataSource.getPrivacyNotice( + PROJECT_ID, + "${PRIVACY_NOTICE_FILE}_$LANGUAGE", + ) + } throws exception + + configServiceImpl.getPrivacyNotice(PROJECT_ID, LANGUAGE).toList() + + verify(exactly = 1) { Simber.i(eq("Failed to download privacy notice"), eq(exception)) } + } + + @Test + fun `should not log when failing to download privacy notice with 404 response status code`() = runTest { + val code = 404 + val exception = Exception("Resource not found").apply { + initCause(HttpException(Response.error(code, "".toResponseBody(null)))) + } + every { localDataSource.hasPrivacyNoticeFor(PROJECT_ID, LANGUAGE) } returns false + coEvery { + remoteDataSource.getPrivacyNotice( + PROJECT_ID, + "${PRIVACY_NOTICE_FILE}_$LANGUAGE", + ) + } throws exception + + configServiceImpl.getPrivacyNotice(PROJECT_ID, LANGUAGE).toList() + + verify(exactly = 0) { Simber.i(any(), any()) } + } } From 1155ab328e59b1752b298838fd295b182677ea66 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 6 May 2025 15:04:58 +0100 Subject: [PATCH 2/2] MS-120 HTTP 404 library constant used --- .../com/simprints/infra/config/store/ConfigRepositoryImpl.kt | 4 ++-- .../simprints/infra/config/store/ConfigRepositoryImplTest.kt | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt b/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt index 9018edbabd..1ffc910197 100644 --- a/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt +++ b/infra/config-store/src/main/java/com/simprints/infra/config/store/ConfigRepositoryImpl.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.FlowCollector import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import retrofit2.HttpException +import java.net.HttpURLConnection import javax.inject.Inject internal class ConfigRepositoryImpl @Inject constructor( @@ -37,7 +38,6 @@ internal class ConfigRepositoryImpl @Inject constructor( companion object { @VisibleForTesting const val PRIVACY_NOTICE_FILE = "privacy_notice" - private const val NOT_FOUND_STATUS_CODE = 404 } override suspend fun getProject(): Project = localDataSource.getProject() @@ -127,7 +127,7 @@ internal class ConfigRepositoryImpl @Inject constructor( localDataSource.storePrivacyNotice(projectId, language, privacyNotice) flowCollector.emit(Succeed(language, privacyNotice)) } catch (t: Throwable) { - if ((t.cause as? HttpException)?.code() != NOT_FOUND_STATUS_CODE) { + if ((t.cause as? HttpException)?.code() != HttpURLConnection.HTTP_NOT_FOUND) { // Non-existence of resource isn't considered a download failure Simber.i("Failed to download privacy notice", t) } diff --git a/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt b/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt index 8e562538bf..2386621f05 100644 --- a/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt +++ b/infra/config-store/src/test/java/com/simprints/infra/config/store/ConfigRepositoryImplTest.kt @@ -36,6 +36,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Before import org.junit.Test +import java.net.HttpURLConnection class ConfigRepositoryImplTest { companion object { @@ -276,7 +277,7 @@ class ConfigRepositoryImplTest { @Test fun `should log when failing to download privacy notice with non-404 response status code`() = runTest { - val code = 500 + val code = HttpURLConnection.HTTP_INTERNAL_ERROR // 500 val exception = Exception("Server error").apply { initCause(HttpException(Response.error(code, "".toResponseBody(null)))) } @@ -295,7 +296,7 @@ class ConfigRepositoryImplTest { @Test fun `should not log when failing to download privacy notice with 404 response status code`() = runTest { - val code = 404 + val code = HttpURLConnection.HTTP_NOT_FOUND val exception = Exception("Resource not found").apply { initCause(HttpException(Response.error(code, "".toResponseBody(null)))) }