Merge pull request #5552 from element-hq/feature/bma/extractConsoleMessageLogger

Extract console message logger and mutualize instance of Json
This commit is contained in:
Benoit Marty 2025-10-16 18:46:18 +02:00 committed by GitHub
commit a264654438
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 126 additions and 63 deletions

View file

@ -35,6 +35,7 @@ import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.plus
import kotlinx.serialization.json.Json
import java.io.File
@BindingContainer
@ -120,4 +121,10 @@ object AppModule {
fun providesEmojibaseProvider(@ApplicationContext context: Context): EmojibaseProvider {
return DefaultEmojibaseProvider(context)
}
@Provides
@SingleIn(AppScope::class)
fun providesJson(): Json = Json {
ignoreUnknownKeys = true
}
}

View file

@ -64,6 +64,7 @@ class CallScreenPresenter(
private val appForegroundStateService: AppForegroundStateService,
@AppCoroutineScope
private val appCoroutineScope: CoroutineScope,
private val widgetMessageSerializer: WidgetMessageSerializer,
) : Presenter<CallScreenState> {
@AssistedFactory
interface Factory {
@ -258,7 +259,7 @@ class CallScreenPresenter(
}
private fun parseMessage(message: String): WidgetMessage? {
return WidgetMessageSerializer.deserialize(message).getOrNull()
return widgetMessageSerializer.deserialize(message).getOrNull()
}
private fun sendHangupMessage(widgetId: String, messageInterceptor: WidgetMessageInterceptor) {
@ -269,7 +270,7 @@ class CallScreenPresenter(
action = WidgetMessage.Action.HangUp,
data = null,
)
messageInterceptor.sendMessage(WidgetMessageSerializer.serialize(message))
messageInterceptor.sendMessage(widgetMessageSerializer.serialize(message))
}
private fun CoroutineScope.close(widgetDriver: MatrixWidgetDriver?, navigator: CallScreenNavigator) = launch(dispatchers.io) {

View file

@ -8,7 +8,6 @@
package io.element.android.features.call.impl.ui
import android.annotation.SuppressLint
import android.util.Log
import android.view.ViewGroup
import android.webkit.ConsoleMessage
import android.webkit.JavascriptInterface
@ -60,6 +59,7 @@ interface CallScreenNavigator {
internal fun CallScreenView(
state: CallScreenState,
pipState: PictureInPictureState,
onConsoleMessage: (ConsoleMessage) -> Unit,
requestPermissions: (Array<String>, RequestPermissionCallback) -> Unit,
modifier: Modifier = Modifier,
) {
@ -108,6 +108,7 @@ internal fun CallScreenView(
val callback: RequestPermissionCallback = { request.grant(it) }
requestPermissions(androidPermissions.toTypedArray(), callback)
},
onConsoleMessage = onConsoleMessage,
onCreateWebView = { webView ->
webView.addBackHandler(onBackPressed = ::handleBack)
val interceptor = WebViewWidgetMessageInterceptor(
@ -174,6 +175,7 @@ private fun CallWebView(
url: AsyncData<String>,
userAgent: String,
onPermissionsRequest: (PermissionRequest) -> Unit,
onConsoleMessage: (ConsoleMessage) -> Unit,
onCreateWebView: (WebView) -> Unit,
onDestroyWebView: (WebView) -> Unit,
modifier: Modifier = Modifier,
@ -188,7 +190,11 @@ private fun CallWebView(
factory = { context ->
WebView(context).apply {
onCreateWebView(this)
setup(userAgent, onPermissionsRequest)
setup(
userAgent = userAgent,
onPermissionsRequested = onPermissionsRequest,
onConsoleMessage = onConsoleMessage,
)
}
},
update = { webView ->
@ -208,6 +214,7 @@ private fun CallWebView(
private fun WebView.setup(
userAgent: String,
onPermissionsRequested: (PermissionRequest) -> Unit,
onConsoleMessage: (ConsoleMessage) -> Unit,
) {
layoutParams = ViewGroup.LayoutParams(
ViewGroup.LayoutParams.MATCH_PARENT,
@ -232,35 +239,7 @@ private fun WebView.setup(
}
override fun onConsoleMessage(consoleMessage: ConsoleMessage): Boolean {
val priority = when (consoleMessage.messageLevel()) {
ConsoleMessage.MessageLevel.ERROR -> Log.ERROR
ConsoleMessage.MessageLevel.WARNING -> Log.WARN
else -> Log.DEBUG
}
val message = buildString {
append(consoleMessage.sourceId())
append(":")
append(consoleMessage.lineNumber())
append(" ")
append(consoleMessage.message())
}
if (message.contains("password=")) {
// Avoid logging any messages that contain "password" to prevent leaking sensitive information
return true
}
Timber.tag("WebView").log(
priority = priority,
message = buildString {
append(consoleMessage.sourceId())
append(":")
append(consoleMessage.lineNumber())
append(" ")
append(consoleMessage.message())
},
)
onConsoleMessage(consoleMessage)
return true
}
}
@ -286,6 +265,7 @@ internal fun CallScreenViewPreview(
state = state,
pipState = aPictureInPictureState(),
requestPermissions = { _, _ -> },
onConsoleMessage = {},
)
}

View file

@ -42,6 +42,7 @@ import io.element.android.features.call.impl.pip.PipView
import io.element.android.features.call.impl.services.CallForegroundService
import io.element.android.features.call.impl.utils.CallIntentDataParser
import io.element.android.features.enterprise.api.EnterpriseService
import io.element.android.libraries.androidutils.browser.ConsoleMessageLogger
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.architecture.bindings
import io.element.android.libraries.audio.api.AudioFocus
@ -65,6 +66,7 @@ class ElementCallActivity :
@Inject lateinit var pictureInPicturePresenter: PictureInPicturePresenter
@Inject lateinit var buildMeta: BuildMeta
@Inject lateinit var audioFocus: AudioFocus
@Inject lateinit var consoleMessageLogger: ConsoleMessageLogger
private lateinit var presenter: Presenter<CallScreenState>
@ -119,6 +121,9 @@ class ElementCallActivity :
CallScreenView(
state = state,
pipState = pipState,
onConsoleMessage = {
consoleMessageLogger.log("ElementCall", it)
},
requestPermissions = { permissions, callback ->
requestPermissionCallback = callback
requestPermissionsLauncher.launch(permissions)

View file

@ -7,18 +7,20 @@
package io.element.android.features.call.impl.utils
import dev.zacsweers.metro.Inject
import io.element.android.features.call.impl.data.WidgetMessage
import io.element.android.libraries.core.extensions.runCatchingExceptions
import kotlinx.serialization.json.Json
object WidgetMessageSerializer {
private val coder = Json { ignoreUnknownKeys = true }
@Inject
class WidgetMessageSerializer(
private val json: Json,
) {
fun deserialize(message: String): Result<WidgetMessage> {
return runCatchingExceptions { coder.decodeFromString(WidgetMessage.serializer(), message) }
return runCatchingExceptions { json.decodeFromString(WidgetMessage.serializer(), message) }
}
fun serialize(message: WidgetMessage): String {
return coder.encodeToString(WidgetMessage.serializer(), message)
return json.encodeToString(WidgetMessage.serializer(), message)
}
}

View file

@ -16,6 +16,7 @@ import io.element.android.features.call.api.CallType
import io.element.android.features.call.impl.ui.CallScreenEvents
import io.element.android.features.call.impl.ui.CallScreenNavigator
import io.element.android.features.call.impl.ui.CallScreenPresenter
import io.element.android.features.call.impl.utils.WidgetMessageSerializer
import io.element.android.features.call.utils.FakeActiveCallManager
import io.element.android.features.call.utils.FakeCallWidgetProvider
import io.element.android.features.call.utils.FakeWidgetMessageInterceptor
@ -46,11 +47,13 @@ import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import org.junit.Rule
import org.junit.Test
import kotlin.time.Duration.Companion.seconds
@OptIn(ExperimentalCoroutinesApi::class) class CallScreenPresenterTest {
@OptIn(ExperimentalCoroutinesApi::class)
class CallScreenPresenterTest {
@get:Rule
val warmUpRule = WarmUpRule()
@ -409,6 +412,7 @@ import kotlin.time.Duration.Companion.seconds
languageTagProvider = FakeLanguageTagProvider("en-US"),
appForegroundStateService = appForegroundStateService,
appCoroutineScope = backgroundScope,
widgetMessageSerializer = WidgetMessageSerializer(Json { ignoreUnknownKeys = true }),
)
}
}

View file

@ -26,10 +26,10 @@ interface MessageParser {
@Inject
class DefaultMessageParser(
private val accountProviderDataSource: AccountProviderDataSource,
private val json: Json,
) : MessageParser {
override fun parse(message: String): ExternalSession {
val parser = Json { ignoreUnknownKeys = true }
val response = parser.decodeFromString(MobileRegistrationResponse.serializer(), message)
val response = json.decodeFromString(MobileRegistrationResponse.serializer(), message)
val userId = response.userId ?: error("No user ID in response")
val homeServer = response.homeServer ?: accountProviderDataSource.flow.value.url
val accessToken = response.accessToken ?: error("No access token in response")

View file

@ -13,6 +13,7 @@ import io.element.android.features.enterprise.test.FakeEnterpriseService
import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
import io.element.android.libraries.matrix.api.auth.external.ExternalSession
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import org.junit.Assert.assertThrows
import org.junit.Test
@ -68,7 +69,8 @@ class DefaultMessageParserTest {
private fun createDefaultMessageParser(): DefaultMessageParser {
return DefaultMessageParser(
AccountProviderDataSource(FakeEnterpriseService())
accountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService()),
json = Json { ignoreUnknownKeys = true },
)
}
}

View file

@ -0,0 +1,61 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.androidutils.browser
import android.util.Log
import android.webkit.ConsoleMessage
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.Inject
import timber.log.Timber
interface ConsoleMessageLogger {
fun log(
tag: String,
consoleMessage: ConsoleMessage,
)
}
@ContributesBinding(AppScope::class)
@Inject
class DefaultConsoleMessageLogger : ConsoleMessageLogger {
override fun log(
tag: String,
consoleMessage: ConsoleMessage,
) {
val priority = when (consoleMessage.messageLevel()) {
ConsoleMessage.MessageLevel.ERROR -> Log.ERROR
ConsoleMessage.MessageLevel.WARNING -> Log.WARN
else -> Log.DEBUG
}
val message = buildString {
append(consoleMessage.sourceId())
append(":")
append(consoleMessage.lineNumber())
append(" ")
append(consoleMessage.message())
}
// Avoid logging any messages that contain "password" to prevent leaking sensitive information
if (message.contains("password=")) {
return
}
Timber.tag(tag).log(
priority = priority,
message = buildString {
append(consoleMessage.sourceId())
append(":")
append(consoleMessage.lineNumber())
append(" ")
append(consoleMessage.message())
},
)
}
}

View file

@ -15,7 +15,6 @@ import dev.zacsweers.metro.SingleIn
import io.element.android.libraries.core.meta.BuildMeta
import io.element.android.libraries.network.interceptors.FormattedJsonHttpLogger
import io.element.android.libraries.network.interceptors.UserAgentInterceptor
import kotlinx.serialization.json.Json
import okhttp3.OkHttpClient
import okhttp3.logging.HttpLoggingInterceptor
import java.util.concurrent.TimeUnit
@ -35,12 +34,6 @@ object NetworkModule {
addInterceptor(userAgentInterceptor)
if (buildMeta.isDebuggable) addInterceptor(providesHttpLoggingInterceptor())
}.build()
@Provides
@SingleIn(AppScope::class)
fun providesJson(): Json = Json {
ignoreUnknownKeys = true
}
}
private fun providesHttpLoggingInterceptor(): HttpLoggingInterceptor {

View file

@ -13,9 +13,9 @@ import io.element.android.libraries.pushproviders.api.PushData
import kotlinx.serialization.json.Json
@Inject
class UnifiedPushParser {
private val json by lazy { Json { ignoreUnknownKeys = true } }
class UnifiedPushParser(
private val json: Json,
) {
fun parse(message: ByteArray, clientSecret: String): PushData? {
return tryOrNull { json.decodeFromString<PushDataUnifiedPush>(String(message)) }?.toPushData(clientSecret)
}

View file

@ -12,6 +12,7 @@ import io.element.android.libraries.matrix.test.AN_EVENT_ID
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.pushproviders.api.PushData
import io.element.android.tests.testutils.assertThrowsInDebug
import kotlinx.serialization.json.Json
import org.junit.Test
class UnifiedPushParserTest {
@ -25,7 +26,7 @@ class UnifiedPushParserTest {
@Test
fun `test edge cases UnifiedPush`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
// Empty string
assertThat(pushParser.parse("".toByteArray(), aClientSecret)).isNull()
// Empty Json
@ -36,13 +37,13 @@ class UnifiedPushParserTest {
@Test
fun `test UnifiedPush format`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
assertThat(pushParser.parse(UNIFIED_PUSH_DATA.toByteArray(), aClientSecret)).isEqualTo(validData)
}
@Test
fun `test empty roomId`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
assertThrowsInDebug {
pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray(), aClientSecret)
}
@ -50,7 +51,7 @@ class UnifiedPushParserTest {
@Test
fun `test invalid roomId`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
assertThrowsInDebug {
pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"), aClientSecret)
}
@ -58,7 +59,7 @@ class UnifiedPushParserTest {
@Test
fun `test empty eventId`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
assertThrowsInDebug {
pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""), aClientSecret)
}
@ -66,7 +67,7 @@ class UnifiedPushParserTest {
@Test
fun `test invalid eventId`() {
val pushParser = UnifiedPushParser()
val pushParser = createUnifiedPushParser()
assertThrowsInDebug {
pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"), aClientSecret)
}
@ -81,3 +82,9 @@ class UnifiedPushParserTest {
private fun String.mutate(oldValue: String, newValue: String): ByteArray {
return replace(oldValue, newValue).toByteArray()
}
fun createUnifiedPushParser(
json: Json = Json { ignoreUnknownKeys = true },
) = UnifiedPushParser(
json = json,
)

View file

@ -191,6 +191,7 @@ class VectorUnifiedPushMessagingReceiverTest {
}
private fun TestScope.createVectorUnifiedPushMessagingReceiver(
unifiedPushParser: UnifiedPushParser = createUnifiedPushParser(),
pushHandler: PushHandler = FakePushHandler(),
unifiedPushStore: UnifiedPushStore = FakeUnifiedPushStore(),
unifiedPushGatewayResolver: UnifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver(),
@ -199,7 +200,7 @@ class VectorUnifiedPushMessagingReceiverTest {
endpointRegistrationHandler: EndpointRegistrationHandler = EndpointRegistrationHandler(),
): VectorUnifiedPushMessagingReceiver {
return VectorUnifiedPushMessagingReceiver().apply {
this.pushParser = UnifiedPushParser()
this.pushParser = unifiedPushParser
this.pushHandler = pushHandler
this.guardServiceStarter = NoopGuardServiceStarter()
this.unifiedPushStore = unifiedPushStore

View file

@ -22,7 +22,7 @@ import timber.log.Timber
@Inject
class DefaultSessionWellknownRetriever(
private val matrixClient: MatrixClient,
private val parser: Json,
private val json: Json,
) : SessionWellknownRetriever {
private val domain by lazy { matrixClient.userIdServerName() }
@ -32,7 +32,7 @@ class DefaultSessionWellknownRetriever(
.getUrl(url)
.mapCatchingExceptions {
val data = String(it)
parser.decodeFromString(InternalWellKnown.serializer(), data)
json.decodeFromString(InternalWellKnown.serializer(), data)
}
.onFailure { Timber.e(it, "Failed to retrieve .well-known from $domain") }
.map { it.map() }
@ -45,7 +45,7 @@ class DefaultSessionWellknownRetriever(
.getUrl(url)
.mapCatchingExceptions {
val data = String(it)
parser.decodeFromString(InternalElementWellKnown.serializer(), data)
json.decodeFromString(InternalElementWellKnown.serializer(), data)
}
.onFailure { Timber.e(it, "Failed to retrieve Element .well-known from $domain") }
.map { it.map() }

View file

@ -244,6 +244,6 @@ class DefaultSessionWellknownRetrieverTest {
userIdServerNameLambda = { "user.domain.org" },
getUrlLambda = getUrlLambda,
),
parser = Json { ignoreUnknownKeys = true }
json = Json { ignoreUnknownKeys = true },
)
}