From 86a44b90359b5072ea39ad523f86ee676683047e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Dec 2024 13:18:37 +0100 Subject: [PATCH 1/5] Ensure that when no Matrix gateway exists, the default one is used. --- .../unifiedpush/UnifiedPushGatewayResolver.kt | 30 ++++++++++++++----- .../VectorUnifiedPushMessagingReceiver.kt | 15 ++++++++++ .../DefaultUnifiedPushGatewayResolverTest.kt | 18 +++++------ .../FakeUnifiedPushGatewayResolver.kt | 4 +-- .../VectorUnifiedPushMessagingReceiverTest.kt | 4 +-- 5 files changed, 51 insertions(+), 20 deletions(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt index 26b900eb87..51f9703fe0 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt @@ -12,12 +12,21 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.core.data.tryOrNull import io.element.android.libraries.di.AppScope import kotlinx.coroutines.withContext +import retrofit2.HttpException import timber.log.Timber +import java.net.HttpURLConnection import java.net.URL import javax.inject.Inject +sealed interface UnifiedPushGatewayResolverResult { + data class Success(val gateway: String) : UnifiedPushGatewayResolverResult + data class Error(val gateway: String) : UnifiedPushGatewayResolverResult + data object NoMatrixGateway : UnifiedPushGatewayResolverResult + data object ErrorInvalidUrl : UnifiedPushGatewayResolverResult +} + interface UnifiedPushGatewayResolver { - suspend fun getGateway(endpoint: String): String + suspend fun getGateway(endpoint: String): UnifiedPushGatewayResolverResult } @ContributesBinding(AppScope::class) @@ -27,7 +36,7 @@ class DefaultUnifiedPushGatewayResolver @Inject constructor( ) : UnifiedPushGatewayResolver { private val logger = Timber.tag("DefaultUnifiedPushGatewayResolver") - override suspend fun getGateway(endpoint: String): String { + override suspend fun getGateway(endpoint: String): UnifiedPushGatewayResolverResult { val url = tryOrNull( onError = { logger.d(it, "Cannot parse endpoint as an URL") } ) { @@ -35,7 +44,7 @@ class DefaultUnifiedPushGatewayResolver @Inject constructor( } return if (url == null) { logger.d("Using default gateway") - UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL + UnifiedPushGatewayResolverResult.ErrorInvalidUrl } else { val port = if (url.port != -1) ":${url.port}" else "" val customBase = "${url.protocol}://${url.host}$port" @@ -47,14 +56,21 @@ class DefaultUnifiedPushGatewayResolver @Inject constructor( val discoveryResponse = api.discover() if (discoveryResponse.unifiedpush.gateway == "matrix") { logger.d("The endpoint seems to be a valid UnifiedPush gateway") + UnifiedPushGatewayResolverResult.Success(customUrl) } else { - logger.e("The endpoint does not seem to be a valid UnifiedPush gateway") + // The endpoint returned a 200 OK but didn't promote an actual matrix gateway, which means it doesn't have any + logger.w("The endpoint does not seem to be a valid UnifiedPush gateway, using fallback") + UnifiedPushGatewayResolverResult.NoMatrixGateway } } catch (throwable: Throwable) { - logger.e(throwable, "Error checking for UnifiedPush endpoint") + if ((throwable as? HttpException)?.code() == HttpURLConnection.HTTP_NOT_FOUND) { + logger.i("Checking for UnifiedPush endpoint yielded 404, using fallback") + UnifiedPushGatewayResolverResult.NoMatrixGateway + } else { + logger.e(throwable, "Error checking for UnifiedPush endpoint") + UnifiedPushGatewayResolverResult.Error(customUrl) + } } - // Always return the custom url. - customUrl } } } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt index 51e6729fda..eb40a7f1e0 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt @@ -64,6 +64,21 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { Timber.tag(loggerTag.value).i("onNewEndpoint: $endpoint") coroutineScope.launch { val gateway = unifiedPushGatewayResolver.getGateway(endpoint) + .let { gatewayResult -> + when (gatewayResult) { + is UnifiedPushGatewayResolverResult.Error -> { + // Use previous gateway if any, or the provided one + unifiedPushStore.getPushGateway(instance) ?: gatewayResult.gateway + } + UnifiedPushGatewayResolverResult.ErrorInvalidUrl, + UnifiedPushGatewayResolverResult.NoMatrixGateway -> { + UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL + } + is UnifiedPushGatewayResolverResult.Success -> { + gatewayResult.gateway + } + } + } unifiedPushStore.storePushGateway(instance, gateway) val result = newGatewayHandler.handle(endpoint, gateway, instance) .onFailure { diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt index b854f999f5..afe0bd2489 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt @@ -43,7 +43,7 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("https://custom.url") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url") - assertThat(result).isEqualTo("https://custom.url/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Success("https://custom.url/_matrix/push/v1/notify")) } @Test @@ -56,7 +56,7 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("https://custom.url:123") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url:123") - assertThat(result).isEqualTo("https://custom.url:123/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Success("https://custom.url:123/_matrix/push/v1/notify")) } @Test @@ -69,7 +69,7 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("https://custom.url:123/some/path") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url:123") - assertThat(result).isEqualTo("https://custom.url:123/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Success("https://custom.url:123/_matrix/push/v1/notify")) } @Test @@ -82,7 +82,7 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("http://custom.url:123/some/path") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url:123") - assertThat(result).isEqualTo("http://custom.url:123/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Success("http://custom.url:123/_matrix/push/v1/notify")) } @Test @@ -95,11 +95,11 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("http://custom.url") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url") - assertThat(result).isEqualTo("http://custom.url/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Error("http://custom.url/_matrix/push/v1/notify")) } @Test - fun `when a custom url is invalid, the default url is returned`() = runTest { + fun `when a custom url is invalid, ErrorInvalidUrl is returned`() = runTest { val unifiedPushApiFactory = FakeUnifiedPushApiFactory( discoveryResponse = matrixDiscoveryResponse ) @@ -108,11 +108,11 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("invalid") assertThat(unifiedPushApiFactory.baseUrlParameter).isNull() - assertThat(result).isEqualTo(UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL) + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.ErrorInvalidUrl) } @Test - fun `when a custom url provides a invalid matrix gateway, the custom url is still returned`() = runTest { + fun `when a custom url provides a invalid matrix gateway, NoMatrixGateway is returned`() = runTest { val unifiedPushApiFactory = FakeUnifiedPushApiFactory( discoveryResponse = invalidDiscoveryResponse ) @@ -121,7 +121,7 @@ class DefaultUnifiedPushGatewayResolverTest { ) val result = sut.getGateway("https://custom.url") assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url") - assertThat(result).isEqualTo("https://custom.url/_matrix/push/v1/notify") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.NoMatrixGateway) } private fun TestScope.createDefaultUnifiedPushGatewayResolver( diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayResolver.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayResolver.kt index 661c636d43..5d0cef8991 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayResolver.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayResolver.kt @@ -10,9 +10,9 @@ package io.element.android.libraries.pushproviders.unifiedpush import io.element.android.tests.testutils.lambda.lambdaError class FakeUnifiedPushGatewayResolver( - private val getGatewayResult: (String) -> String = { lambdaError() }, + private val getGatewayResult: (String) -> UnifiedPushGatewayResolverResult = { lambdaError() }, ) : UnifiedPushGatewayResolver { - override suspend fun getGateway(endpoint: String): String { + override suspend fun getGateway(endpoint: String): UnifiedPushGatewayResolverResult { return getGatewayResult(endpoint) } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt index be5f66e0f0..2293f97c3e 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt @@ -104,7 +104,7 @@ class VectorUnifiedPushMessagingReceiverTest { val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver( unifiedPushStore = unifiedPushStore, unifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver( - getGatewayResult = { "aGateway" } + getGatewayResult = { UnifiedPushGatewayResolverResult.Success("aGateway") } ), endpointRegistrationHandler = endpointRegistrationHandler, unifiedPushNewGatewayHandler = unifiedPushNewGatewayHandler, @@ -144,7 +144,7 @@ class VectorUnifiedPushMessagingReceiverTest { val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver( unifiedPushStore = unifiedPushStore, unifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver( - getGatewayResult = { "aGateway" } + getGatewayResult = { UnifiedPushGatewayResolverResult.Success("aGateway") } ), endpointRegistrationHandler = endpointRegistrationHandler, unifiedPushNewGatewayHandler = unifiedPushNewGatewayHandler, From e2cb0173a6068a749bf8a2670f248cb62112b54e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 30 Dec 2024 13:20:06 +0100 Subject: [PATCH 2/5] Fix issue with logger. --- .../unifiedpush/UnifiedPushGatewayResolver.kt | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt index 51f9703fe0..0af1d62e7d 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt @@ -34,40 +34,38 @@ class DefaultUnifiedPushGatewayResolver @Inject constructor( private val unifiedPushApiFactory: UnifiedPushApiFactory, private val coroutineDispatchers: CoroutineDispatchers, ) : UnifiedPushGatewayResolver { - private val logger = Timber.tag("DefaultUnifiedPushGatewayResolver") - override suspend fun getGateway(endpoint: String): UnifiedPushGatewayResolverResult { val url = tryOrNull( - onError = { logger.d(it, "Cannot parse endpoint as an URL") } + onError = { Timber.tag("DefaultUnifiedPushGatewayResolver").d(it, "Cannot parse endpoint as an URL") } ) { URL(endpoint) } return if (url == null) { - logger.d("Using default gateway") + Timber.tag("DefaultUnifiedPushGatewayResolver").d("ErrorInvalidUrl") UnifiedPushGatewayResolverResult.ErrorInvalidUrl } else { val port = if (url.port != -1) ":${url.port}" else "" val customBase = "${url.protocol}://${url.host}$port" val customUrl = "$customBase/_matrix/push/v1/notify" - logger.i("Testing $customUrl") + Timber.tag("DefaultUnifiedPushGatewayResolver").i("Testing $customUrl") return withContext(coroutineDispatchers.io) { val api = unifiedPushApiFactory.create(customBase) try { val discoveryResponse = api.discover() if (discoveryResponse.unifiedpush.gateway == "matrix") { - logger.d("The endpoint seems to be a valid UnifiedPush gateway") + Timber.tag("DefaultUnifiedPushGatewayResolver").d("The endpoint seems to be a valid UnifiedPush gateway") UnifiedPushGatewayResolverResult.Success(customUrl) } else { // The endpoint returned a 200 OK but didn't promote an actual matrix gateway, which means it doesn't have any - logger.w("The endpoint does not seem to be a valid UnifiedPush gateway, using fallback") + Timber.tag("DefaultUnifiedPushGatewayResolver").w("The endpoint does not seem to be a valid UnifiedPush gateway, using fallback") UnifiedPushGatewayResolverResult.NoMatrixGateway } } catch (throwable: Throwable) { if ((throwable as? HttpException)?.code() == HttpURLConnection.HTTP_NOT_FOUND) { - logger.i("Checking for UnifiedPush endpoint yielded 404, using fallback") + Timber.tag("DefaultUnifiedPushGatewayResolver").i("Checking for UnifiedPush endpoint yielded 404, using fallback") UnifiedPushGatewayResolverResult.NoMatrixGateway } else { - logger.e(throwable, "Error checking for UnifiedPush endpoint") + Timber.tag("DefaultUnifiedPushGatewayResolver").e(throwable, "Error checking for UnifiedPush endpoint") UnifiedPushGatewayResolverResult.Error(customUrl) } } From 4c8af0a66cba34e7133bab34cfb91a17b5030827 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 31 Dec 2024 09:32:09 +0100 Subject: [PATCH 3/5] Add missing tests on DefaultUnifiedPushGatewayResolver --- .../DefaultUnifiedPushGatewayResolverTest.kt | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt index afe0bd2489..190bdad4da 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt @@ -14,7 +14,11 @@ import io.element.android.libraries.pushproviders.unifiedpush.network.DiscoveryU import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest +import okhttp3.ResponseBody.Companion.toResponseBody import org.junit.Test +import retrofit2.HttpException +import retrofit2.Response +import java.net.HttpURLConnection internal val matrixDiscoveryResponse = { DiscoveryResponse( @@ -98,6 +102,36 @@ class DefaultUnifiedPushGatewayResolverTest { assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Error("http://custom.url/_matrix/push/v1/notify")) } + @Test + fun `when a custom url is not found (404), NoMatrixGateway is returned`() = runTest { + val unifiedPushApiFactory = FakeUnifiedPushApiFactory( + discoveryResponse = { + throw HttpException(Response.error(HttpURLConnection.HTTP_NOT_FOUND, "".toResponseBody())) + } + ) + val sut = createDefaultUnifiedPushGatewayResolver( + unifiedPushApiFactory = unifiedPushApiFactory + ) + val result = sut.getGateway("http://custom.url") + assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.NoMatrixGateway) + } + + @Test + fun `when a custom url is forbidden (403), Error is returned`() = runTest { + val unifiedPushApiFactory = FakeUnifiedPushApiFactory( + discoveryResponse = { + throw HttpException(Response.error(HttpURLConnection.HTTP_FORBIDDEN, "".toResponseBody())) + } + ) + val sut = createDefaultUnifiedPushGatewayResolver( + unifiedPushApiFactory = unifiedPushApiFactory + ) + val result = sut.getGateway("http://custom.url") + assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url") + assertThat(result).isEqualTo(UnifiedPushGatewayResolverResult.Error("http://custom.url/_matrix/push/v1/notify")) + } + @Test fun `when a custom url is invalid, ErrorInvalidUrl is returned`() = runTest { val unifiedPushApiFactory = FakeUnifiedPushApiFactory( From 9ada3791d7cdc48ecadb91c58d75d7e5ce5eca2b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 31 Dec 2024 09:38:44 +0100 Subject: [PATCH 4/5] Add test on VectorUnifiedPushMessagingReceiver.onReceive --- .../VectorUnifiedPushMessagingReceiverTest.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt index 2293f97c3e..ab726a9c82 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.pushproviders.unifiedpush +import android.content.Intent import androidx.test.platform.app.InstrumentationRegistry import app.cash.turbine.test import com.google.common.truth.Truth.assertThat @@ -27,12 +28,23 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertThrows import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class VectorUnifiedPushMessagingReceiverTest { + @Test + fun `onReceive does the binding`() = runTest { + val context = InstrumentationRegistry.getInstrumentation().context + val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver() + // The binding is not found in the test env. + assertThrows(IllegalStateException::class.java) { + vectorUnifiedPushMessagingReceiver.onReceive(context, Intent()) + } + } + @Test fun `onUnregistered does nothing`() = runTest { val context = InstrumentationRegistry.getInstrumentation().context From b85d4bac1d2a564528ff5bf8bd1ff29cabfb48ba Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 31 Dec 2024 09:52:43 +0100 Subject: [PATCH 5/5] UnifiedPush: extract logic to resolve the gateway url and unit test it. --- .../UnifiedPushGatewayUrlResolver.kt | 43 ++++++++++ .../VectorUnifiedPushMessagingReceiver.kt | 15 +--- ...efaultUnifiedPushGatewayUrlResolverTest.kt | 83 +++++++++++++++++++ .../FakeUnifiedPushGatewayUrlResolver.kt | 18 ++++ .../VectorUnifiedPushMessagingReceiverTest.kt | 12 ++- 5 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayUrlResolver.kt create mode 100644 libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayUrlResolverTest.kt create mode 100644 libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayUrlResolver.kt diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayUrlResolver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayUrlResolver.kt new file mode 100644 index 0000000000..d338d55096 --- /dev/null +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayUrlResolver.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.unifiedpush + +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import javax.inject.Inject + +interface UnifiedPushGatewayUrlResolver { + fun resolve( + gatewayResult: UnifiedPushGatewayResolverResult, + instance: String, + ): String +} + +@ContributesBinding(AppScope::class) +class DefaultUnifiedPushGatewayUrlResolver @Inject constructor( + private val unifiedPushStore: UnifiedPushStore, +) : UnifiedPushGatewayUrlResolver { + override fun resolve( + gatewayResult: UnifiedPushGatewayResolverResult, + instance: String, + ): String { + return when (gatewayResult) { + is UnifiedPushGatewayResolverResult.Error -> { + // Use previous gateway if any, or the provided one + unifiedPushStore.getPushGateway(instance) ?: gatewayResult.gateway + } + UnifiedPushGatewayResolverResult.ErrorInvalidUrl, + UnifiedPushGatewayResolverResult.NoMatrixGateway -> { + UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL + } + is UnifiedPushGatewayResolverResult.Success -> { + gatewayResult.gateway + } + } + } +} diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt index eb40a7f1e0..096fb69c57 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt @@ -28,6 +28,7 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { @Inject lateinit var guardServiceStarter: GuardServiceStarter @Inject lateinit var unifiedPushStore: UnifiedPushStore @Inject lateinit var unifiedPushGatewayResolver: UnifiedPushGatewayResolver + @Inject lateinit var unifiedPushGatewayUrlResolver: UnifiedPushGatewayUrlResolver @Inject lateinit var newGatewayHandler: UnifiedPushNewGatewayHandler @Inject lateinit var endpointRegistrationHandler: EndpointRegistrationHandler @Inject lateinit var coroutineScope: CoroutineScope @@ -65,19 +66,7 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { coroutineScope.launch { val gateway = unifiedPushGatewayResolver.getGateway(endpoint) .let { gatewayResult -> - when (gatewayResult) { - is UnifiedPushGatewayResolverResult.Error -> { - // Use previous gateway if any, or the provided one - unifiedPushStore.getPushGateway(instance) ?: gatewayResult.gateway - } - UnifiedPushGatewayResolverResult.ErrorInvalidUrl, - UnifiedPushGatewayResolverResult.NoMatrixGateway -> { - UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL - } - is UnifiedPushGatewayResolverResult.Success -> { - gatewayResult.gateway - } - } + unifiedPushGatewayUrlResolver.resolve(gatewayResult, instance) } unifiedPushStore.storePushGateway(instance, gateway) val result = newGatewayHandler.handle(endpoint, gateway, instance) diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayUrlResolverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayUrlResolverTest.kt new file mode 100644 index 0000000000..46495b39e2 --- /dev/null +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayUrlResolverTest.kt @@ -0,0 +1,83 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.unifiedpush + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class DefaultUnifiedPushGatewayUrlResolverTest { + @Test + fun `resolve ErrorInvalidUrl returns the default gateway`() { + val sut = createDefaultUnifiedPushGatewayUrlResolver() + val result = sut.resolve( + gatewayResult = UnifiedPushGatewayResolverResult.ErrorInvalidUrl, + instance = "", + ) + assertThat(result).isEqualTo(UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL) + } + + @Test + fun `resolve NoMatrixGateway returns the default gateway`() { + val sut = createDefaultUnifiedPushGatewayUrlResolver() + val result = sut.resolve( + gatewayResult = UnifiedPushGatewayResolverResult.NoMatrixGateway, + instance = "", + ) + assertThat(result).isEqualTo(UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL) + } + + @Test + fun `resolve Success returns the url`() { + val sut = createDefaultUnifiedPushGatewayUrlResolver() + val result = sut.resolve( + gatewayResult = UnifiedPushGatewayResolverResult.Success("aUrl"), + instance = "", + ) + assertThat(result).isEqualTo("aUrl") + } + + @Test + fun `resolve Error returns the current url when available`() { + val sut = createDefaultUnifiedPushGatewayUrlResolver( + unifiedPushStore = FakeUnifiedPushStore( + getPushGatewayResult = { instance -> + assertThat(instance).isEqualTo("instance") + "aCurrentUrl" + }, + ) + ) + val result = sut.resolve( + gatewayResult = UnifiedPushGatewayResolverResult.Error("aUrl"), + instance = "instance", + ) + assertThat(result).isEqualTo("aCurrentUrl") + } + + @Test + fun `resolve Error returns the url if no current url is available`() { + val sut = createDefaultUnifiedPushGatewayUrlResolver( + unifiedPushStore = FakeUnifiedPushStore( + getPushGatewayResult = { instance -> + assertThat(instance).isEqualTo("instance") + null + }, + ) + ) + val result = sut.resolve( + gatewayResult = UnifiedPushGatewayResolverResult.Error("aUrl"), + instance = "instance", + ) + assertThat(result).isEqualTo("aUrl") + } + + private fun createDefaultUnifiedPushGatewayUrlResolver( + unifiedPushStore: UnifiedPushStore = FakeUnifiedPushStore(), + ) = DefaultUnifiedPushGatewayUrlResolver( + unifiedPushStore = unifiedPushStore, + ) +} diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayUrlResolver.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayUrlResolver.kt new file mode 100644 index 0000000000..b4ac6da4aa --- /dev/null +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayUrlResolver.kt @@ -0,0 +1,18 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.pushproviders.unifiedpush + +import io.element.android.tests.testutils.lambda.lambdaError + +class FakeUnifiedPushGatewayUrlResolver( + private val resolveResult: (UnifiedPushGatewayResolverResult, String) -> String = { _, _ -> lambdaError() }, +) : UnifiedPushGatewayUrlResolver { + override fun resolve(gatewayResult: UnifiedPushGatewayResolverResult, instance: String): String { + return resolveResult(gatewayResult, instance) + } +} diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt index ab726a9c82..2a402004ed 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt @@ -118,6 +118,9 @@ class VectorUnifiedPushMessagingReceiverTest { unifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver( getGatewayResult = { UnifiedPushGatewayResolverResult.Success("aGateway") } ), + unifiedPushGatewayUrlResolver = FakeUnifiedPushGatewayUrlResolver( + resolveResult = { _, _ -> "aGatewayUrl" } + ), endpointRegistrationHandler = endpointRegistrationHandler, unifiedPushNewGatewayHandler = unifiedPushNewGatewayHandler, ) @@ -133,7 +136,7 @@ class VectorUnifiedPushMessagingReceiverTest { } storePushGatewayResult.assertions() .isCalledOnce() - .with(value(A_SECRET), value("aGateway")) + .with(value(A_SECRET), value("aGatewayUrl")) storeUpEndpointResult.assertions() .isCalledOnce() .with(value(A_SECRET), value("anEndpoint")) @@ -158,6 +161,9 @@ class VectorUnifiedPushMessagingReceiverTest { unifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver( getGatewayResult = { UnifiedPushGatewayResolverResult.Success("aGateway") } ), + unifiedPushGatewayUrlResolver = FakeUnifiedPushGatewayUrlResolver( + resolveResult = { _, _ -> "aGatewayUrl" } + ), endpointRegistrationHandler = endpointRegistrationHandler, unifiedPushNewGatewayHandler = unifiedPushNewGatewayHandler, ) @@ -173,7 +179,7 @@ class VectorUnifiedPushMessagingReceiverTest { } storePushGatewayResult.assertions() .isCalledOnce() - .with(value(A_SECRET), value("aGateway")) + .with(value(A_SECRET), value("aGatewayUrl")) storeUpEndpointResult.assertions() .isNeverCalled() } @@ -182,6 +188,7 @@ class VectorUnifiedPushMessagingReceiverTest { pushHandler: PushHandler = FakePushHandler(), unifiedPushStore: UnifiedPushStore = FakeUnifiedPushStore(), unifiedPushGatewayResolver: UnifiedPushGatewayResolver = FakeUnifiedPushGatewayResolver(), + unifiedPushGatewayUrlResolver: UnifiedPushGatewayUrlResolver = FakeUnifiedPushGatewayUrlResolver(), unifiedPushNewGatewayHandler: UnifiedPushNewGatewayHandler = FakeUnifiedPushNewGatewayHandler(), endpointRegistrationHandler: EndpointRegistrationHandler = EndpointRegistrationHandler(), ): VectorUnifiedPushMessagingReceiver { @@ -191,6 +198,7 @@ class VectorUnifiedPushMessagingReceiverTest { this.guardServiceStarter = NoopGuardServiceStarter() this.unifiedPushStore = unifiedPushStore this.unifiedPushGatewayResolver = unifiedPushGatewayResolver + this.unifiedPushGatewayUrlResolver = unifiedPushGatewayUrlResolver this.newGatewayHandler = unifiedPushNewGatewayHandler this.endpointRegistrationHandler = endpointRegistrationHandler this.coroutineScope = this@createVectorUnifiedPushMessagingReceiver