Fix push notifications for UnifiedPush distributors without provided matrix gateway

libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushGatewayResolver.kt

@@ -12,12 +12,14 @@ 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
 
 interface UnifiedPushGatewayResolver {
-    suspend fun getGateway(endpoint: String): String
+    suspend fun getGateway(endpoint: String, previousGateway: String?): String
 }
 
 @ContributesBinding(AppScope::class)
@@ -27,7 +29,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, previousGateway: String?): String {
         val url = tryOrNull(
             onError = { logger.d(it, "Cannot parse endpoint as an URL") }
         ) {
@@ -47,14 +49,24 @@ class DefaultUnifiedPushGatewayResolver @Inject constructor(
                     val discoveryResponse = api.discover()
                     if (discoveryResponse.unifiedpush.gateway == "matrix") {
                         logger.d("The endpoint seems to be a valid UnifiedPush gateway")
+                        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")
+                        UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL
+                    }
+                } catch (exception: HttpException) {
+                    if (exception.code() == HttpURLConnection.HTTP_NOT_FOUND) {
+                        logger.i("Checking for UnifiedPush endpoint yielded 404, using fallback")
+                        UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL
+                    } else {
+                        logger.e(exception, "Error checking for UnifiedPush endpoint")
+                        previousGateway ?: customUrl
                     }
                 } catch (throwable: Throwable) {
                     logger.e(throwable, "Error checking for UnifiedPush endpoint")
+                    previousGateway ?: customUrl
                 }
-                // Always return the custom url.
-                customUrl
             }
         }
     }

libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt

@@ -63,7 +63,7 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() {
     override fun onNewEndpoint(context: Context, endpoint: String, instance: String) {
         Timber.tag(loggerTag.value).i("onNewEndpoint: $endpoint")
         coroutineScope.launch {
-            val gateway = unifiedPushGatewayResolver.getGateway(endpoint)
+            val gateway = unifiedPushGatewayResolver.getGateway(endpoint, unifiedPushStore.getPushGateway(instance))
             unifiedPushStore.storePushGateway(instance, gateway)
             val result = newGatewayHandler.handle(endpoint, gateway, instance)
                 .onFailure {

libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushGatewayResolverTest.kt

@@ -41,7 +41,7 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("https://custom.url")
+        val result = sut.getGateway("https://custom.url", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url")
         assertThat(result).isEqualTo("https://custom.url/_matrix/push/v1/notify")
     }
@@ -54,7 +54,7 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("https://custom.url:123")
+        val result = sut.getGateway("https://custom.url:123", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url:123")
         assertThat(result).isEqualTo("https://custom.url:123/_matrix/push/v1/notify")
     }
@@ -67,7 +67,7 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("https://custom.url:123/some/path")
+        val result = sut.getGateway("https://custom.url:123/some/path", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url:123")
         assertThat(result).isEqualTo("https://custom.url:123/_matrix/push/v1/notify")
     }
@@ -80,7 +80,7 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("http://custom.url:123/some/path")
+        val result = sut.getGateway("http://custom.url:123/some/path", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url:123")
         assertThat(result).isEqualTo("http://custom.url:123/_matrix/push/v1/notify")
     }
@@ -93,7 +93,7 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("http://custom.url")
+        val result = sut.getGateway("http://custom.url", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("http://custom.url")
         assertThat(result).isEqualTo("http://custom.url/_matrix/push/v1/notify")
     }
@@ -106,22 +106,22 @@ class DefaultUnifiedPushGatewayResolverTest {
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("invalid")
+        val result = sut.getGateway("invalid", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isNull()
         assertThat(result).isEqualTo(UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL)
     }
 
     @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, the default url is returned`() = runTest {
         val unifiedPushApiFactory = FakeUnifiedPushApiFactory(
             discoveryResponse = invalidDiscoveryResponse
         )
         val sut = createDefaultUnifiedPushGatewayResolver(
             unifiedPushApiFactory = unifiedPushApiFactory
         )
-        val result = sut.getGateway("https://custom.url")
+        val result = sut.getGateway("https://custom.url", null)
         assertThat(unifiedPushApiFactory.baseUrlParameter).isEqualTo("https://custom.url")
-        assertThat(result).isEqualTo("https://custom.url/_matrix/push/v1/notify")
+        assertThat(result).isEqualTo(UnifiedPushConfig.DEFAULT_PUSH_GATEWAY_HTTP_URL)
     }
 
     private fun TestScope.createDefaultUnifiedPushGatewayResolver(

libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnifiedPushGatewayResolver.kt

@@ -12,7 +12,7 @@ import io.element.android.tests.testutils.lambda.lambdaError
 class FakeUnifiedPushGatewayResolver(
     private val getGatewayResult: (String) -> String = { lambdaError() },
 ) : UnifiedPushGatewayResolver {
-    override suspend fun getGateway(endpoint: String): String {
+    override suspend fun getGateway(endpoint: String, previousGateway: String?): String {
         return getGatewayResult(endpoint)
     }
 }