mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-03 06:55:41 +00:00
Show the account a UnifiedPush registration belongs to in the distributor
A distributor such as ntfy or DAVx5 lists every registration it holds, but Element registered each one with no message for the distributor, so an account added from a second session was indistinguishable from the first. RegisterUnifiedPushUseCase.execute now takes the session id and passes it as messageForDistributor, which the connector has accepted since 3.x. The value is the raw Matrix ID, which is not personal data by the project's own logging rules, and no new string resource is needed. UnifiedPushProvider already has the MatrixClient in hand at the only call site, so the PushProvider interface is unchanged and nothing outside the module moves. Relates to https://github.com/element-hq/element-x-android/issues/6426
This commit is contained in:
+7
-3
@@ -12,6 +12,7 @@ import android.content.Context
|
|||||||
import dev.zacsweers.metro.AppScope
|
import dev.zacsweers.metro.AppScope
|
||||||
import dev.zacsweers.metro.ContributesBinding
|
import dev.zacsweers.metro.ContributesBinding
|
||||||
import io.element.android.libraries.di.annotations.ApplicationContext
|
import io.element.android.libraries.di.annotations.ApplicationContext
|
||||||
|
import io.element.android.libraries.matrix.api.core.SessionId
|
||||||
import io.element.android.libraries.pushproviders.api.Distributor
|
import io.element.android.libraries.pushproviders.api.Distributor
|
||||||
import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler
|
import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler
|
||||||
import kotlinx.coroutines.flow.filter
|
import kotlinx.coroutines.flow.filter
|
||||||
@@ -21,7 +22,7 @@ import org.unifiedpush.android.connector.UnifiedPush
|
|||||||
import kotlin.time.Duration.Companion.seconds
|
import kotlin.time.Duration.Companion.seconds
|
||||||
|
|
||||||
interface RegisterUnifiedPushUseCase {
|
interface RegisterUnifiedPushUseCase {
|
||||||
suspend fun execute(distributor: Distributor, clientSecret: String): Result<Unit>
|
suspend fun execute(distributor: Distributor, clientSecret: String, sessionId: SessionId): Result<Unit>
|
||||||
}
|
}
|
||||||
|
|
||||||
@ContributesBinding(AppScope::class)
|
@ContributesBinding(AppScope::class)
|
||||||
@@ -29,11 +30,14 @@ class DefaultRegisterUnifiedPushUseCase(
|
|||||||
@ApplicationContext private val context: Context,
|
@ApplicationContext private val context: Context,
|
||||||
private val endpointRegistrationHandler: EndpointRegistrationHandler,
|
private val endpointRegistrationHandler: EndpointRegistrationHandler,
|
||||||
) : RegisterUnifiedPushUseCase {
|
) : RegisterUnifiedPushUseCase {
|
||||||
override suspend fun execute(distributor: Distributor, clientSecret: String): Result<Unit> {
|
override suspend fun execute(distributor: Distributor, clientSecret: String, sessionId: SessionId): Result<Unit> {
|
||||||
UnifiedPush.saveDistributor(context, distributor.value)
|
UnifiedPush.saveDistributor(context, distributor.value)
|
||||||
// This will trigger the callback
|
// This will trigger the callback
|
||||||
// VectorUnifiedPushMessagingReceiver.onNewEndpoint
|
// VectorUnifiedPushMessagingReceiver.onNewEndpoint
|
||||||
UnifiedPush.register(context = context, instance = clientSecret)
|
// The session id is passed as the message for the distributor, so that its UI can tell one
|
||||||
|
// registration from another when several accounts are registered.
|
||||||
|
// See https://github.com/element-hq/element-x-android/issues/6426
|
||||||
|
UnifiedPush.register(context = context, instance = clientSecret, messageForDistributor = sessionId.value)
|
||||||
// Wait for VectorUnifiedPushMessagingReceiver.onNewEndpoint to proceed
|
// Wait for VectorUnifiedPushMessagingReceiver.onNewEndpoint to proceed
|
||||||
@Suppress("RunCatchingNotAllowed")
|
@Suppress("RunCatchingNotAllowed")
|
||||||
return runCatching {
|
return runCatching {
|
||||||
|
|||||||
+1
-1
@@ -36,7 +36,7 @@ class UnifiedPushProvider(
|
|||||||
|
|
||||||
override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result<Unit> {
|
override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result<Unit> {
|
||||||
val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId)
|
val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId)
|
||||||
return registerUnifiedPushUseCase.execute(distributor, clientSecret)
|
return registerUnifiedPushUseCase.execute(distributor, clientSecret, matrixClient.sessionId)
|
||||||
.onSuccess {
|
.onSuccess {
|
||||||
unifiedPushStore.setDistributorValue(matrixClient.sessionId, distributor.value)
|
unifiedPushStore.setDistributorValue(matrixClient.sessionId, distributor.value)
|
||||||
}
|
}
|
||||||
|
|||||||
+4
-3
@@ -12,6 +12,7 @@ import androidx.test.platform.app.InstrumentationRegistry
|
|||||||
import com.google.common.truth.Truth.assertThat
|
import com.google.common.truth.Truth.assertThat
|
||||||
import io.element.android.libraries.matrix.test.AN_EXCEPTION
|
import io.element.android.libraries.matrix.test.AN_EXCEPTION
|
||||||
import io.element.android.libraries.matrix.test.A_SECRET
|
import io.element.android.libraries.matrix.test.A_SECRET
|
||||||
|
import io.element.android.libraries.matrix.test.A_SESSION_ID
|
||||||
import io.element.android.libraries.pushproviders.api.Distributor
|
import io.element.android.libraries.pushproviders.api.Distributor
|
||||||
import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler
|
import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler
|
||||||
import io.element.android.libraries.pushproviders.unifiedpush.registration.RegistrationResult
|
import io.element.android.libraries.pushproviders.unifiedpush.registration.RegistrationResult
|
||||||
@@ -41,7 +42,7 @@ class DefaultRegisterUnifiedPushUseCaseTest : RobolectricTest() {
|
|||||||
delay(100)
|
delay(100)
|
||||||
endpointRegistrationHandler.registrationDone(RegistrationResult(A_SECRET, Result.success(Unit)))
|
endpointRegistrationHandler.registrationDone(RegistrationResult(A_SECRET, Result.success(Unit)))
|
||||||
}
|
}
|
||||||
val result = useCase.execute(aDistributor, A_SECRET)
|
val result = useCase.execute(aDistributor, A_SECRET, A_SESSION_ID)
|
||||||
assertThat(result.isSuccess).isTrue()
|
assertThat(result.isSuccess).isTrue()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -56,7 +57,7 @@ class DefaultRegisterUnifiedPushUseCaseTest : RobolectricTest() {
|
|||||||
delay(100)
|
delay(100)
|
||||||
endpointRegistrationHandler.registrationDone(RegistrationResult(A_SECRET, Result.failure(AN_EXCEPTION)))
|
endpointRegistrationHandler.registrationDone(RegistrationResult(A_SECRET, Result.failure(AN_EXCEPTION)))
|
||||||
}
|
}
|
||||||
val result = useCase.execute(aDistributor, A_SECRET)
|
val result = useCase.execute(aDistributor, A_SECRET, A_SESSION_ID)
|
||||||
assertThat(result.isSuccess).isFalse()
|
assertThat(result.isSuccess).isFalse()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -67,7 +68,7 @@ class DefaultRegisterUnifiedPushUseCaseTest : RobolectricTest() {
|
|||||||
endpointRegistrationHandler = endpointRegistrationHandler
|
endpointRegistrationHandler = endpointRegistrationHandler
|
||||||
)
|
)
|
||||||
val aDistributor = Distributor("aValue", "aName")
|
val aDistributor = Distributor("aValue", "aName")
|
||||||
val result = useCase.execute(aDistributor, A_SECRET)
|
val result = useCase.execute(aDistributor, A_SECRET, A_SESSION_ID)
|
||||||
assertThat(result.isSuccess).isFalse()
|
assertThat(result.isSuccess).isFalse()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+4
-3
@@ -8,13 +8,14 @@
|
|||||||
|
|
||||||
package io.element.android.libraries.pushproviders.unifiedpush
|
package io.element.android.libraries.pushproviders.unifiedpush
|
||||||
|
|
||||||
|
import io.element.android.libraries.matrix.api.core.SessionId
|
||||||
import io.element.android.libraries.pushproviders.api.Distributor
|
import io.element.android.libraries.pushproviders.api.Distributor
|
||||||
import io.element.android.tests.testutils.lambda.lambdaError
|
import io.element.android.tests.testutils.lambda.lambdaError
|
||||||
|
|
||||||
class FakeRegisterUnifiedPushUseCase(
|
class FakeRegisterUnifiedPushUseCase(
|
||||||
private val result: (Distributor, String) -> Result<Unit> = { _, _ -> lambdaError() }
|
private val result: (Distributor, String, SessionId) -> Result<Unit> = { _, _, _ -> lambdaError() }
|
||||||
) : RegisterUnifiedPushUseCase {
|
) : RegisterUnifiedPushUseCase {
|
||||||
override suspend fun execute(distributor: Distributor, clientSecret: String): Result<Unit> {
|
override suspend fun execute(distributor: Distributor, clientSecret: String, sessionId: SessionId): Result<Unit> {
|
||||||
return result(distributor, clientSecret)
|
return result(distributor, clientSecret, sessionId)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+4
-4
@@ -62,7 +62,7 @@ class UnifiedPushProviderTest {
|
|||||||
@Test
|
@Test
|
||||||
fun `register ok`() = runTest {
|
fun `register ok`() = runTest {
|
||||||
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
|
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
|
||||||
val executeLambda = lambdaRecorder<Distributor, String, Result<Unit>> { _, _ -> Result.success(Unit) }
|
val executeLambda = lambdaRecorder<Distributor, String, SessionId, Result<Unit>> { _, _, _ -> Result.success(Unit) }
|
||||||
val setDistributorValueResultLambda = lambdaRecorder<UserId, String, Unit> { _, _ -> }
|
val setDistributorValueResultLambda = lambdaRecorder<UserId, String, Unit> { _, _ -> }
|
||||||
val unifiedPushProvider = createUnifiedPushProvider(
|
val unifiedPushProvider = createUnifiedPushProvider(
|
||||||
pushClientSecret = FakePushClientSecret(
|
pushClientSecret = FakePushClientSecret(
|
||||||
@@ -82,7 +82,7 @@ class UnifiedPushProviderTest {
|
|||||||
.with(value(A_SESSION_ID))
|
.with(value(A_SESSION_ID))
|
||||||
executeLambda.assertions()
|
executeLambda.assertions()
|
||||||
.isCalledOnce()
|
.isCalledOnce()
|
||||||
.with(value(Distributor("value", "Name")), value(A_SECRET))
|
.with(value(Distributor("value", "Name")), value(A_SECRET), value(A_SESSION_ID))
|
||||||
setDistributorValueResultLambda.assertions()
|
setDistributorValueResultLambda.assertions()
|
||||||
.isCalledOnce()
|
.isCalledOnce()
|
||||||
.with(value(A_SESSION_ID), value("value"))
|
.with(value(A_SESSION_ID), value("value"))
|
||||||
@@ -91,7 +91,7 @@ class UnifiedPushProviderTest {
|
|||||||
@Test
|
@Test
|
||||||
fun `register ko`() = runTest {
|
fun `register ko`() = runTest {
|
||||||
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
|
val getSecretForUserResultLambda = lambdaRecorder<SessionId, String> { A_SECRET }
|
||||||
val executeLambda = lambdaRecorder<Distributor, String, Result<Unit>> { _, _ -> Result.failure(AN_EXCEPTION) }
|
val executeLambda = lambdaRecorder<Distributor, String, SessionId, Result<Unit>> { _, _, _ -> Result.failure(AN_EXCEPTION) }
|
||||||
val setDistributorValueResultLambda = lambdaRecorder<UserId, String, Unit>(ensureNeverCalled = true) { _, _ -> }
|
val setDistributorValueResultLambda = lambdaRecorder<UserId, String, Unit>(ensureNeverCalled = true) { _, _ -> }
|
||||||
val unifiedPushProvider = createUnifiedPushProvider(
|
val unifiedPushProvider = createUnifiedPushProvider(
|
||||||
pushClientSecret = FakePushClientSecret(
|
pushClientSecret = FakePushClientSecret(
|
||||||
@@ -111,7 +111,7 @@ class UnifiedPushProviderTest {
|
|||||||
.with(value(A_SESSION_ID))
|
.with(value(A_SESSION_ID))
|
||||||
executeLambda.assertions()
|
executeLambda.assertions()
|
||||||
.isCalledOnce()
|
.isCalledOnce()
|
||||||
.with(value(Distributor("value", "Name")), value(A_SECRET))
|
.with(value(Distributor("value", "Name")), value(A_SECRET), value(A_SESSION_ID))
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user