From 36a72b63de199232d920f3f698d9e45ab4d27bb9 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 10 Dec 2023 12:49:03 +0100 Subject: [PATCH] Implement sending logs to developer (#190) * Save logs to a file * Send logs via email * Enable network logs in release builds * Remove useless chooser title * Append to logs file and ignore I/O errors * Ensure email and password are not logged * Ensure base URL is never logged * Add logs disclaimer --- app/src/main/AndroidManifest.xml | 88 +++++++------- .../mealient/data/auth/impl/AuthRepoImpl.kt | 5 +- .../data/auth/impl/CredentialsLogRedactor.kt | 38 ++++++ .../data/baseurl/impl/BaseUrlLogRedactor.kt | 49 ++++++++ .../gq/kirmanak/mealient/di/AuthModule.kt | 7 ++ .../gq/kirmanak/mealient/di/BaseURLModule.kt | 7 ++ .../mealient/extensions/ViewExtensions.kt | 1 - .../mealient/ui/activity/MainActivity.kt | 46 ++++++++ .../ui/auth/AuthenticationViewModel.kt | 2 +- .../mealient/ui/baseurl/BaseURLViewModel.kt | 8 +- app/src/main/res/drawable/ic_send.xml | 11 ++ app/src/main/res/layout/main_activity.xml | 77 ++++++------- app/src/main/res/menu/navigation_menu.xml | 63 +++++----- app/src/main/res/values/strings.xml | 6 + app/src/main/res/xml/file_provider_paths.xml | 6 + .../data/auth/impl/AuthRepoImplTest.kt | 12 +- .../ui/baseurl/BaseURLViewModelTest.kt | 10 +- datasource/build.gradle.kts | 4 +- .../gq/kirmanak/mealient/DebugModule.kt | 13 --- .../mealient/datasource/DataSourceModule.kt | 15 +++ .../gq/kirmanak/mealient/ReleaseModule.kt | 19 --- .../{LoggingModule.kt => AppenderModule.kt} | 9 +- .../kirmanak/mealient/logging/FileAppender.kt | 109 ++++++++++++++++++ .../kirmanak/mealient/logging/LogRedactor.kt | 6 + .../mealient/logging/LogcatAppender.kt | 2 +- .../gq/kirmanak/mealient/logging/Logger.kt | 9 ++ .../kirmanak/mealient/logging/LoggerImpl.kt | 14 ++- .../kirmanak/mealient/logging/LoggerModule.kt | 15 +++ ...keLoggingModule.kt => FakeLoggerModule.kt} | 6 +- 29 files changed, 500 insertions(+), 157 deletions(-) create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/auth/impl/CredentialsLogRedactor.kt create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/baseurl/impl/BaseUrlLogRedactor.kt create mode 100644 app/src/main/res/drawable/ic_send.xml create mode 100644 app/src/main/res/xml/file_provider_paths.xml delete mode 100644 datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt rename logging/src/main/kotlin/gq/kirmanak/mealient/logging/{LoggingModule.kt => AppenderModule.kt} (75%) create mode 100644 logging/src/main/kotlin/gq/kirmanak/mealient/logging/FileAppender.kt create mode 100644 logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogRedactor.kt create mode 100644 logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerModule.kt rename testing/src/main/kotlin/gq/kirmanak/mealient/test/{FakeLoggingModule.kt => FakeLoggerModule.kt} (74%) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index c2fae41..f15fe3f 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -1,46 +1,56 @@ + xmlns:tools="http://schemas.android.com/tools"> - - + + - - - - + + + + - - - - - - - - - - - - + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt index 8d2627e..5b32790 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt @@ -15,17 +15,20 @@ class AuthRepoImpl @Inject constructor( private val authDataSource: AuthDataSource, private val logger: Logger, private val signOutHandler: SignOutHandler, + private val credentialsLogRedactor: CredentialsLogRedactor, ) : AuthRepo, AuthenticationProvider { override val isAuthorizedFlow: Flow get() = authStorage.authTokenFlow.map { it != null } override suspend fun authenticate(email: String, password: String) { - logger.v { "authenticate() called with: email = $email, password = $password" } + logger.v { "authenticate() called" } + credentialsLogRedactor.set(email, password) val token = authDataSource.authenticate(email, password) authStorage.setAuthToken(token) val apiToken = authDataSource.createApiToken(API_TOKEN_NAME) authStorage.setAuthToken(apiToken) + credentialsLogRedactor.clear() } override suspend fun getAuthToken(): String? = authStorage.getAuthToken() diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/CredentialsLogRedactor.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/CredentialsLogRedactor.kt new file mode 100644 index 0000000..e73f779 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/CredentialsLogRedactor.kt @@ -0,0 +1,38 @@ +package gq.kirmanak.mealient.data.auth.impl + +import gq.kirmanak.mealient.logging.LogRedactor +import kotlinx.coroutines.flow.MutableStateFlow +import java.net.URLEncoder +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class CredentialsLogRedactor @Inject constructor() : LogRedactor { + + private data class Credentials( + val login: String, + val password: String, + val urlEncodedLogin: String = URLEncoder.encode(login, Charsets.UTF_8.name()), + val urlEncodedPassword: String = URLEncoder.encode(password, Charsets.UTF_8.name()), + ) + + private val credentialsState = MutableStateFlow(null) + + fun set(login: String, password: String) { + credentialsState.value = Credentials(login, password) + } + + fun clear() { + credentialsState.value = null + } + + override fun redact(message: String): String { + val credentials = credentialsState.value ?: return message + + return message + .replace(credentials.login, "") + .replace(credentials.urlEncodedLogin, "") + .replace(credentials.password, "") + .replace(credentials.urlEncodedPassword, "") + } +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/baseurl/impl/BaseUrlLogRedactor.kt b/app/src/main/java/gq/kirmanak/mealient/data/baseurl/impl/BaseUrlLogRedactor.kt new file mode 100644 index 0000000..bebde24 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/baseurl/impl/BaseUrlLogRedactor.kt @@ -0,0 +1,49 @@ +package gq.kirmanak.mealient.data.baseurl.impl + +import androidx.core.net.toUri +import gq.kirmanak.mealient.architecture.configuration.AppDispatchers +import gq.kirmanak.mealient.data.baseurl.ServerInfoStorage +import gq.kirmanak.mealient.logging.LogRedactor +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.launch +import javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton + +@Singleton +class BaseUrlLogRedactor @Inject constructor( + private val serverInfoStorageProvider: Provider, + private val dispatchers: AppDispatchers, +) : LogRedactor { + + private val hostState = MutableStateFlow(null) + + init { + setInitialBaseUrl() + } + + private fun setInitialBaseUrl() { + val scope = CoroutineScope(dispatchers.default + SupervisorJob()) + scope.launch { + val serverInfoStorage = serverInfoStorageProvider.get() + hostState.compareAndSet( + expect = null, + update = serverInfoStorage.getBaseURL()?.extractHost(), + ) + } + } + + fun set(baseUrl: String) { + hostState.value = baseUrl.extractHost() + } + + + override fun redact(message: String): String { + val host = hostState.value ?: return message + return message.replace(host, "") + } +} + +private fun String.extractHost() = runCatching { toUri() }.getOrNull()?.host diff --git a/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt b/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt index c4b953b..a93544c 100644 --- a/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt +++ b/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt @@ -4,13 +4,16 @@ import dagger.Binds import dagger.Module import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.auth.impl.AuthDataSourceImpl import gq.kirmanak.mealient.data.auth.impl.AuthRepoImpl import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl +import gq.kirmanak.mealient.data.auth.impl.CredentialsLogRedactor import gq.kirmanak.mealient.datasource.AuthenticationProvider +import gq.kirmanak.mealient.logging.LogRedactor import gq.kirmanak.mealient.shopping_lists.repo.ShoppingListsAuthRepo @Module @@ -31,4 +34,8 @@ interface AuthModule { @Binds fun bindShoppingListsAuthRepo(impl: AuthRepoImpl): ShoppingListsAuthRepo + + @Binds + @IntoSet + fun bindCredentialsLogRedactor(impl: CredentialsLogRedactor): LogRedactor } diff --git a/app/src/main/java/gq/kirmanak/mealient/di/BaseURLModule.kt b/app/src/main/java/gq/kirmanak/mealient/di/BaseURLModule.kt index 34ec7d9..9840377 100644 --- a/app/src/main/java/gq/kirmanak/mealient/di/BaseURLModule.kt +++ b/app/src/main/java/gq/kirmanak/mealient/di/BaseURLModule.kt @@ -4,9 +4,12 @@ import dagger.Binds import dagger.Module import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet import gq.kirmanak.mealient.data.baseurl.* +import gq.kirmanak.mealient.data.baseurl.impl.BaseUrlLogRedactor import gq.kirmanak.mealient.data.baseurl.impl.ServerInfoStorageImpl import gq.kirmanak.mealient.datasource.ServerUrlProvider +import gq.kirmanak.mealient.logging.LogRedactor @Module @InstallIn(SingletonComponent::class) @@ -23,4 +26,8 @@ interface BaseURLModule { @Binds fun bindServerUrlProvider(serverInfoRepoImpl: ServerInfoRepoImpl): ServerUrlProvider + + @Binds + @IntoSet + fun bindBaseUrlLogRedactor(impl: BaseUrlLogRedactor): LogRedactor } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/extensions/ViewExtensions.kt b/app/src/main/java/gq/kirmanak/mealient/extensions/ViewExtensions.kt index 722ab6d..c70c5eb 100644 --- a/app/src/main/java/gq/kirmanak/mealient/extensions/ViewExtensions.kt +++ b/app/src/main/java/gq/kirmanak/mealient/extensions/ViewExtensions.kt @@ -73,7 +73,6 @@ fun EditText.checkIfInputIsEmpty( ): String? { val input = if (trim) text?.trim() else text val text = input?.toString().orEmpty() - logger.d { "Input text is \"$text\"" } return text.ifEmpty { inputLayout.error = resources.getString(stringId) val textChangesLiveData = textChangesLiveData(logger) diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt index 3ca005d..00c7a0a 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/activity/MainActivity.kt @@ -1,8 +1,11 @@ package gq.kirmanak.mealient.ui.activity +import android.content.Intent +import android.net.Uri import android.os.Bundle import android.view.MenuItem import androidx.activity.viewModels +import androidx.core.content.FileProvider import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen import androidx.core.view.isVisible import androidx.core.view.iterator @@ -10,6 +13,7 @@ import androidx.drawerlayout.widget.DrawerLayout import androidx.navigation.NavController import androidx.navigation.NavDirections import androidx.navigation.fragment.NavHostFragment +import com.google.android.material.dialog.MaterialAlertDialogBuilder import dagger.hilt.android.AndroidEntryPoint import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalAddRecipeFragment import gq.kirmanak.mealient.NavGraphDirections.Companion.actionGlobalAuthenticationFragment @@ -20,10 +24,13 @@ import gq.kirmanak.mealient.R import gq.kirmanak.mealient.databinding.MainActivityBinding import gq.kirmanak.mealient.extensions.collectWhenResumed import gq.kirmanak.mealient.extensions.observeOnce +import gq.kirmanak.mealient.logging.getLogFile import gq.kirmanak.mealient.ui.ActivityUiState import gq.kirmanak.mealient.ui.BaseActivity import gq.kirmanak.mealient.ui.CheckableMenuItem +private const val EMAIL_FOR_LOGS = "mealient@gmail.com" + @AndroidEntryPoint class MainActivity : BaseActivity( binder = MainActivityBinding::bind, @@ -87,6 +94,12 @@ class MainActivity : BaseActivity( viewModel.logout() return true } + + R.id.email_logs -> { + emailLogs() + return true + } + else -> throw IllegalArgumentException("Unknown menu item id: ${menuItem.itemId}") } menuItem.isChecked = true @@ -94,6 +107,39 @@ class MainActivity : BaseActivity( return true } + private fun emailLogs() { + MaterialAlertDialogBuilder(this) + .setMessage(R.string.activity_main_email_logs_confirmation_message) + .setTitle(R.string.activity_main_email_logs_confirmation_title) + .setPositiveButton(R.string.activity_main_email_logs_confirmation_positive) { _, _ -> doEmailLogs() } + .setNegativeButton(R.string.activity_main_email_logs_confirmation_negative, null) + .show() + } + + private fun doEmailLogs() { + val logFileUri = try { + FileProvider.getUriForFile(this, "$packageName.provider", getLogFile()) + } catch (e: Exception) { + return + } + val emailIntent = buildIntent(logFileUri) + val chooserIntent = Intent.createChooser(emailIntent, null) + startActivity(chooserIntent) + } + + private fun buildIntent(logFileUri: Uri?): Intent { + val emailIntent = Intent(Intent.ACTION_SEND) + val to = arrayOf(EMAIL_FOR_LOGS) + emailIntent.setType("text/plain") + emailIntent.putExtra(Intent.EXTRA_EMAIL, to) + emailIntent.putExtra(Intent.EXTRA_STREAM, logFileUri) + emailIntent.putExtra( + Intent.EXTRA_SUBJECT, + getString(R.string.activity_main_email_logs_subject) + ) + return emailIntent + } + private fun onUiStateChange(uiState: ActivityUiState) { logger.v { "onUiStateChange() called with: uiState = $uiState" } val checkedMenuItem = when (uiState.checkedMenuItem) { diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt index 145852d..203ce39 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/auth/AuthenticationViewModel.kt @@ -22,7 +22,7 @@ class AuthenticationViewModel @Inject constructor( val uiState: LiveData> get() = _uiState fun authenticate(email: String, password: String) { - logger.v { "authenticate() called with: email = $email, password = $password" } + logger.v { "authenticate() called" } _uiState.value = OperationUiState.Progress() viewModelScope.launch { val result = runCatchingExceptCancel { authRepo.authenticate(email, password) } diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModel.kt b/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModel.kt index 6c76d96..5670905 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModel.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModel.kt @@ -7,6 +7,7 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo +import gq.kirmanak.mealient.data.baseurl.impl.BaseUrlLogRedactor import gq.kirmanak.mealient.data.recipes.RecipeRepo import gq.kirmanak.mealient.datasource.CertificateCombinedException import gq.kirmanak.mealient.datasource.NetworkError @@ -27,6 +28,7 @@ class BaseURLViewModel @Inject constructor( private val recipeRepo: RecipeRepo, private val logger: Logger, private val trustedCertificatesStore: TrustedCertificatesStore, + private val baseUrlLogRedactor: BaseUrlLogRedactor, ) : ViewModel() { private val _uiState = MutableLiveData>(OperationUiState.Initial()) @@ -36,18 +38,20 @@ class BaseURLViewModel @Inject constructor( val invalidCertificatesFlow = invalidCertificatesChannel.receiveAsFlow() fun saveBaseUrl(baseURL: String) { - logger.v { "saveBaseUrl() called with: baseURL = $baseURL" } + logger.v { "saveBaseUrl() called" } _uiState.value = OperationUiState.Progress() viewModelScope.launch { checkBaseURL(baseURL) } } private suspend fun checkBaseURL(baseURL: String) { - logger.v { "checkBaseURL() called with: baseURL = $baseURL" } + logger.v { "checkBaseURL() called" } val hasPrefix = listOf("http://", "https://").any { baseURL.startsWith(it) } val urlWithPrefix = baseURL.takeIf { hasPrefix } ?: "https://%s".format(baseURL) val url = urlWithPrefix.trimEnd { it == '/' } + baseUrlLogRedactor.set(baseUrl = url) + logger.d { "checkBaseURL: Created URL = \"$url\", with prefix = \"$urlWithPrefix\"" } if (url == serverInfoRepo.getUrl()) { logger.d { "checkBaseURL: new URL matches current" } diff --git a/app/src/main/res/drawable/ic_send.xml b/app/src/main/res/drawable/ic_send.xml new file mode 100644 index 0000000..3112f6d --- /dev/null +++ b/app/src/main/res/drawable/ic_send.xml @@ -0,0 +1,11 @@ + + + diff --git a/app/src/main/res/layout/main_activity.xml b/app/src/main/res/layout/main_activity.xml index ade2931..275da23 100644 --- a/app/src/main/res/layout/main_activity.xml +++ b/app/src/main/res/layout/main_activity.xml @@ -1,46 +1,45 @@ + xmlns:app="http://schemas.android.com/apk/res-auto" + xmlns:tools="http://schemas.android.com/tools" + android:id="@+id/drawer" + style="?drawerLayoutStyle" + android:layout_width="match_parent" + android:layout_height="match_parent" + tools:context=".ui.activity.MainActivity"> - + - + - - + + - + diff --git a/app/src/main/res/menu/navigation_menu.xml b/app/src/main/res/menu/navigation_menu.xml index 781ac62..a32cb25 100644 --- a/app/src/main/res/menu/navigation_menu.xml +++ b/app/src/main/res/menu/navigation_menu.xml @@ -1,38 +1,43 @@ - + - + - + - + - + - + + + \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d3e8e64..928fa7f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -73,4 +73,10 @@ Added %1$s to favorites Removed %1$s from favorites Shopping lists + Email logs + Mealient logs + The logs contain sensitive data such as API token, shopping lists, and recipes. API tokens can be revoked using web client. The file can be viewed and edited if you send it to yourself instead. + Sending sensitive data + Choose how to send + Cancel \ No newline at end of file diff --git a/app/src/main/res/xml/file_provider_paths.xml b/app/src/main/res/xml/file_provider_paths.xml new file mode 100644 index 0000000..7cc365b --- /dev/null +++ b/app/src/main/res/xml/file_provider_paths.xml @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt index 1a5c425..b9f2cc9 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt @@ -16,6 +16,7 @@ import io.mockk.coVerify import io.mockk.confirmVerified import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.impl.annotations.RelaxedMockK import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.runTest @@ -33,12 +34,21 @@ class AuthRepoImplTest : BaseUnitTest() { @MockK(relaxUnitFun = true) lateinit var signOutHandler: SignOutHandler + @RelaxedMockK + lateinit var credentialsLogRedactor: CredentialsLogRedactor + lateinit var subject: AuthRepo @Before override fun setUp() { super.setUp() - subject = AuthRepoImpl(storage, dataSource, logger, signOutHandler) + subject = AuthRepoImpl( + authStorage = storage, + authDataSource = dataSource, + logger = logger, + signOutHandler = signOutHandler, + credentialsLogRedactor = credentialsLogRedactor, + ) } @Test diff --git a/app/src/test/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModelTest.kt b/app/src/test/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModelTest.kt index 486bc1a..aa21195 100644 --- a/app/src/test/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModelTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/ui/baseurl/BaseURLViewModelTest.kt @@ -3,6 +3,7 @@ package gq.kirmanak.mealient.ui.baseurl import com.google.common.truth.Truth.assertThat import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo +import gq.kirmanak.mealient.data.baseurl.impl.BaseUrlLogRedactor import gq.kirmanak.mealient.data.recipes.RecipeRepo import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.datasource.TrustedCertificatesStore @@ -13,8 +14,11 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.coVerifyOrder import io.mockk.impl.annotations.MockK +import io.mockk.impl.annotations.RelaxedMockK import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.* +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test import java.io.IOException @@ -35,6 +39,9 @@ class BaseURLViewModelTest : BaseUnitTest() { @MockK(relaxUnitFun = true) lateinit var trustedCertificatesStore: TrustedCertificatesStore + @RelaxedMockK + lateinit var baseUrlLogRedactor: BaseUrlLogRedactor + lateinit var subject: BaseURLViewModel @Before @@ -46,6 +53,7 @@ class BaseURLViewModelTest : BaseUnitTest() { recipeRepo = recipeRepo, logger = logger, trustedCertificatesStore = trustedCertificatesStore, + baseUrlLogRedactor = baseUrlLogRedactor, ) } diff --git a/datasource/build.gradle.kts b/datasource/build.gradle.kts index 8e9dfcc..7ae367a 100644 --- a/datasource/build.gradle.kts +++ b/datasource/build.gradle.kts @@ -7,7 +7,7 @@ plugins { android { defaultConfig { - buildConfigField("Boolean", "LOG_NETWORK", "false") + buildConfigField("Boolean", "LOG_NETWORK", "true") consumerProguardFiles("consumer-proguard-rules.pro") } namespace = "gq.kirmanak.mealient.datasource" @@ -31,7 +31,7 @@ dependencies { implementation(platform(libs.okhttp3.bom)) implementation(libs.okhttp3.okhttp) - debugImplementation(libs.okhttp3.loggingInterceptor) + implementation(libs.okhttp3.loggingInterceptor) implementation(libs.ktor.core) implementation(libs.ktor.auth) diff --git a/datasource/src/debug/kotlin/gq/kirmanak/mealient/DebugModule.kt b/datasource/src/debug/kotlin/gq/kirmanak/mealient/DebugModule.kt index 6ca1b86..ed42644 100644 --- a/datasource/src/debug/kotlin/gq/kirmanak/mealient/DebugModule.kt +++ b/datasource/src/debug/kotlin/gq/kirmanak/mealient/DebugModule.kt @@ -10,25 +10,12 @@ import dagger.hilt.InstallIn import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.components.SingletonComponent import dagger.multibindings.IntoSet -import gq.kirmanak.mealient.datasource.BuildConfig -import gq.kirmanak.mealient.logging.Logger import okhttp3.Interceptor -import okhttp3.logging.HttpLoggingInterceptor @Module @InstallIn(SingletonComponent::class) internal object DebugModule { - @Provides - @IntoSet - fun provideLoggingInterceptor(logger: Logger): Interceptor { - val interceptor = HttpLoggingInterceptor { message -> logger.v(tag = "OkHttp") { message } } - interceptor.level = when { - BuildConfig.LOG_NETWORK -> HttpLoggingInterceptor.Level.BODY - else -> HttpLoggingInterceptor.Level.BASIC - } - return interceptor - } @Provides @IntoSet diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index cc564d5..1cc0945 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -5,13 +5,17 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet import gq.kirmanak.mealient.datasource.impl.MealieDataSourceImpl import gq.kirmanak.mealient.datasource.impl.MealieServiceKtor import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl import gq.kirmanak.mealient.datasource.impl.TrustedCertificatesStoreImpl +import gq.kirmanak.mealient.logging.Logger import kotlinx.serialization.json.Json +import okhttp3.Interceptor import okhttp3.OkHttpClient +import okhttp3.logging.HttpLoggingInterceptor import javax.inject.Singleton @Module @@ -33,6 +37,17 @@ internal interface DataSourceModule { fun provideOkHttp(okHttpBuilder: OkHttpBuilderImpl): OkHttpClient = okHttpBuilder.buildOkHttp() + @Provides + @IntoSet + fun provideLoggingInterceptor(logger: Logger): Interceptor { + val interceptor = + HttpLoggingInterceptor { message -> logger.v(tag = "OkHttp") { message } } + interceptor.level = when { + BuildConfig.LOG_NETWORK -> HttpLoggingInterceptor.Level.BODY + else -> HttpLoggingInterceptor.Level.BASIC + } + return interceptor + } } @Binds diff --git a/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt b/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt deleted file mode 100644 index a8fdf32..0000000 --- a/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt +++ /dev/null @@ -1,19 +0,0 @@ -package gq.kirmanak.mealient - -import dagger.Module -import dagger.Provides -import dagger.hilt.InstallIn -import dagger.hilt.components.SingletonComponent -import okhttp3.Interceptor -import javax.inject.Singleton - -@Module -@InstallIn(SingletonComponent::class) -object ReleaseModule { - - // Release version of the application doesn't have any interceptors but this Set - // is required by Dagger, so an empty Set is provided here - // Use @JvmSuppressWildcards because otherwise dagger can't inject it (https://stackoverflow.com/a/43149382) - @Provides - fun provideInterceptors(): Set<@JvmSuppressWildcards Interceptor> = emptySet() -} diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggingModule.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/AppenderModule.kt similarity index 75% rename from logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggingModule.kt rename to logging/src/main/kotlin/gq/kirmanak/mealient/logging/AppenderModule.kt index 7f267a3..11df41b 100644 --- a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggingModule.kt +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/AppenderModule.kt @@ -8,12 +8,13 @@ import dagger.multibindings.IntoSet @Module @InstallIn(SingletonComponent::class) -interface LoggingModule { - - @Binds - fun bindLogger(loggerImpl: LoggerImpl): Logger +internal interface AppenderModule { @Binds @IntoSet fun bindLogcatAppender(logcatAppender: LogcatAppender): Appender + + @Binds + @IntoSet + fun bindFileAppender(fileAppender: FileAppender): Appender } \ No newline at end of file diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/FileAppender.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/FileAppender.kt new file mode 100644 index 0000000..18e0e10 --- /dev/null +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/FileAppender.kt @@ -0,0 +1,109 @@ +package gq.kirmanak.mealient.logging + +import android.app.Application +import gq.kirmanak.mealient.architecture.configuration.AppDispatchers +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.launch +import java.io.BufferedWriter +import java.io.FileWriter +import java.io.IOException +import java.io.Writer +import java.time.Instant +import java.time.ZoneId +import java.time.format.DateTimeFormatter +import javax.inject.Inject +import javax.inject.Singleton + +private const val MAX_LOG_FILE_SIZE = 1024 * 1024 * 10L // 10 MB + +@Singleton +internal class FileAppender @Inject constructor( + private val application: Application, + dispatchers: AppDispatchers, +) : Appender { + + private data class LogInfo( + val logTime: Instant, + val logLevel: LogLevel, + val tag: String, + val message: String, + ) + + private val fileWriter: Writer? = createFileWriter() + + private val logChannel = Channel( + capacity = 100, + onBufferOverflow = BufferOverflow.DROP_LATEST, + ) + + private val coroutineScope = CoroutineScope(dispatchers.io + SupervisorJob()) + + private val dateTimeFormatter = + DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.systemDefault()) + + init { + startLogWriter() + } + + private fun createFileWriter(): Writer? { + val file = application.getLogFile() + if (file.length() > MAX_LOG_FILE_SIZE) { + file.delete() + } + + val writer = try { + FileWriter(file, /* append = */ true) + } catch (e: IOException) { + return null + } + + return BufferedWriter(writer) + } + + private fun startLogWriter() { + if (fileWriter == null) { + return + } + + coroutineScope.launch { + for (logInfo in logChannel) { + val time = dateTimeFormatter.format(logInfo.logTime) + val level = logInfo.logLevel.name.first() + logInfo.message.lines().forEach { + try { + fileWriter.appendLine("$time $level ${logInfo.tag}: $it") + } catch (e: IOException) { + // Ignore + } + } + } + } + } + + override fun isLoggable(logLevel: LogLevel): Boolean = true + + override fun isLoggable(logLevel: LogLevel, tag: String): Boolean = true + + override fun log(logLevel: LogLevel, tag: String, message: String) { + val logInfo = LogInfo( + logTime = Instant.now(), + logLevel = logLevel, + tag = tag, + message = message, + ) + logChannel.trySend(logInfo) + } + + protected fun finalize() { + coroutineScope.cancel("Object is being destroyed") + try { + fileWriter?.close() + } catch (e: IOException) { + // Ignore + } + } +} \ No newline at end of file diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogRedactor.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogRedactor.kt new file mode 100644 index 0000000..19aafb5 --- /dev/null +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogRedactor.kt @@ -0,0 +1,6 @@ +package gq.kirmanak.mealient.logging + +interface LogRedactor { + + fun redact(message: String): String +} \ No newline at end of file diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogcatAppender.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogcatAppender.kt index 78b4a1d..f4aff9f 100644 --- a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogcatAppender.kt +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LogcatAppender.kt @@ -4,7 +4,7 @@ import android.util.Log import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration import javax.inject.Inject -class LogcatAppender @Inject constructor( +internal class LogcatAppender @Inject constructor( private val buildConfiguration: BuildConfiguration, ) : Appender { diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/Logger.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/Logger.kt index 66b8314..53f1636 100644 --- a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/Logger.kt +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/Logger.kt @@ -1,7 +1,12 @@ package gq.kirmanak.mealient.logging +import android.content.Context +import java.io.File + typealias MessageSupplier = () -> String +private const val LOG_FILE_NAME = "log.txt" + interface Logger { fun v(throwable: Throwable? = null, tag: String? = null, messageSupplier: MessageSupplier) @@ -13,4 +18,8 @@ interface Logger { fun w(throwable: Throwable? = null, tag: String? = null, messageSupplier: MessageSupplier) fun e(throwable: Throwable? = null, tag: String? = null, messageSupplier: MessageSupplier) +} + +fun Context.getLogFile(): File { + return File(filesDir, LOG_FILE_NAME) } \ No newline at end of file diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerImpl.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerImpl.kt index 09d7976..40a194b 100644 --- a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerImpl.kt +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerImpl.kt @@ -6,6 +6,7 @@ import javax.inject.Inject class LoggerImpl @Inject constructor( private val appenders: Set<@JvmSuppressWildcards Appender>, + private val redactors: Set<@JvmSuppressWildcards LogRedactor>, ) : Logger { override fun v(throwable: Throwable?, tag: String?, messageSupplier: MessageSupplier) { @@ -45,12 +46,23 @@ class LoggerImpl @Inject constructor( if (appender.isLoggable(logLevel, logTag).not()) continue - message = message ?: (messageSupplier() + createStackTrace(t)) + message = message ?: buildLogMessage(messageSupplier, t) appender.log(logLevel, logTag, message) } } + private fun buildLogMessage( + messageSupplier: MessageSupplier, + t: Throwable? + ): String { + var message = messageSupplier() + createStackTrace(t) + for (redactor in redactors) { + message = redactor.redact(message) + } + return message + } + private fun createStackTrace(throwable: Throwable?): String = throwable?.let { Log.getStackTraceString(it) } ?.takeUnless { it.isBlank() } diff --git a/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerModule.kt b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerModule.kt new file mode 100644 index 0000000..d02312e --- /dev/null +++ b/logging/src/main/kotlin/gq/kirmanak/mealient/logging/LoggerModule.kt @@ -0,0 +1,15 @@ +package gq.kirmanak.mealient.logging + +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent + +@Module +@InstallIn(SingletonComponent::class) +interface LoggerModule { + + @Binds + fun bindLogger(loggerImpl: LoggerImpl): Logger + +} \ No newline at end of file diff --git a/testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggingModule.kt b/testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggerModule.kt similarity index 74% rename from testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggingModule.kt rename to testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggerModule.kt index 8021399..d168427 100644 --- a/testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggingModule.kt +++ b/testing/src/main/kotlin/gq/kirmanak/mealient/test/FakeLoggerModule.kt @@ -5,14 +5,14 @@ import dagger.Module import dagger.hilt.components.SingletonComponent import dagger.hilt.testing.TestInstallIn import gq.kirmanak.mealient.logging.Logger -import gq.kirmanak.mealient.logging.LoggingModule +import gq.kirmanak.mealient.logging.LoggerModule @Module @TestInstallIn( components = [SingletonComponent::class], - replaces = [LoggingModule::class] + replaces = [LoggerModule::class] ) -interface FakeLoggingModule { +interface FakeLoggerModule { @Binds fun bindFakeLogger(impl: FakeLogger): Logger