extracting the add pusher logic for the worker and delegating to the task from the worker

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushers/PushersService.kt

@@ -52,7 +52,25 @@ interface PushersService {
      *                          (LiveData<WorkInfo> status = workManager.getWorkInfoByIdLiveData(<UUID>))
      * @throws [InvalidParameterException] if a parameter is not correct
      */
-    fun addHttpPusher(pushkey: String,
+    suspend fun addHttpPusher(pushkey: String,
+                              appId: String,
+                              profileTag: String,
+                              lang: String,
+                              appDisplayName: String,
+                              deviceDisplayName: String,
+                              url: String,
+                              append: Boolean,
+                              withEventIdOnly: Boolean)
+
+    /**
+     * Enqueues a new HTTP pusher via the WorkManager API.
+     * Ref: https://matrix.org/docs/spec/client_server/latest#post-matrix-client-r0-pushers-set
+     *
+     * @return A work request uuid. Can be used to listen to the status
+     *         (LiveData<WorkInfo> status = workManager.getWorkInfoByIdLiveData(<UUID>))
+     * @throws [InvalidParameterException] if a parameter is not correct
+     */
+    fun enqueueAddHttpPusher(pushkey: String,
                              appId: String,
                              profileTag: String,
                              lang: String,
@@ -75,16 +93,14 @@ interface PushersService {
      *                          to any others with different user IDs. Otherwise, the homeserver must remove any other pushers
      *                          with the same App ID and pushkey for different users. Typically We always want to append for
      *                          email pushers since we don't want to stop other accounts notifying to the same email address.
-     * @return                  A work request uuid. Can be used to listen to the status
-     *                          (LiveData<WorkInfo> status = workManager.getWorkInfoByIdLiveData(<UUID>))
      * @throws [InvalidParameterException] if a parameter is not correct
      */
-    fun addEmailPusher(email: String,
+    suspend fun addEmailPusher(email: String,
                                lang: String,
                                emailBranding: String,
                                appDisplayName: String,
                                deviceDisplayName: String,
-                       append: Boolean = true): UUID
+                               append: Boolean = true)
 
     /**
      * Directly ask the push gateway to send a push to this device

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushers/AddPusherTask.kt

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2021 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.matrix.android.sdk.internal.session.pushers
+
+import com.zhuinden.monarchy.Monarchy
+import org.matrix.android.sdk.api.session.pushers.PusherState
+import org.matrix.android.sdk.internal.database.mapper.toEntity
+import org.matrix.android.sdk.internal.database.model.PusherEntity
+import org.matrix.android.sdk.internal.database.query.where
+import org.matrix.android.sdk.internal.di.SessionDatabase
+import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
+import org.matrix.android.sdk.internal.network.executeRequest
+import org.matrix.android.sdk.internal.task.Task
+import org.matrix.android.sdk.internal.util.awaitTransaction
+import javax.inject.Inject
+
+internal interface AddPusherTask : Task<AddPusherTask.Params, Unit> {
+    data class Params(val pusher: JsonPusher)
+}
+
+internal class DefaultAddPusherTask @Inject constructor(
+        private val pushersAPI: PushersAPI,
+        @SessionDatabase private val monarchy: Monarchy,
+        private val globalErrorReceiver: GlobalErrorReceiver
+) : AddPusherTask {
+    override suspend fun execute(params: AddPusherTask.Params) {
+        val pusher = params.pusher
+        try {
+            setPusher(pusher)
+        } catch (error: Throwable) {
+            monarchy.awaitTransaction { realm ->
+                PusherEntity.where(realm, pusher.pushKey).findFirst()?.let {
+                    // update it
+                    it.state = PusherState.FAILED_TO_REGISTER
+                }
+            }
+            throw error
+        }
+    }
+
+    private suspend fun setPusher(pusher: JsonPusher) {
+        executeRequest(globalErrorReceiver) {
+            pushersAPI.setPusher(pusher)
+        }
+        monarchy.awaitTransaction { realm ->
+            val echo = PusherEntity.where(realm, pusher.pushKey).findFirst()
+            if (echo != null) {
+                // update it
+                echo.appDisplayName = pusher.appDisplayName
+                echo.appId = pusher.appId
+                echo.kind = pusher.kind
+                echo.lang = pusher.lang
+                echo.profileTag = pusher.profileTag
+                echo.data?.format = pusher.data?.format
+                echo.data?.url = pusher.data?.url
+                echo.state = PusherState.REGISTERED
+            } else {
+                pusher.toEntity().also {
+                    it.state = PusherState.REGISTERED
+                    realm.insertOrUpdate(it)
+                }
+            }
+        }
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushers/AddPusherWorker.kt

@@ -18,17 +18,8 @@ package org.matrix.android.sdk.internal.session.pushers
 import android.content.Context
 import androidx.work.WorkerParameters
 import com.squareup.moshi.JsonClass
-import com.zhuinden.monarchy.Monarchy
 import org.matrix.android.sdk.api.failure.Failure
-import org.matrix.android.sdk.api.session.pushers.PusherState
-import org.matrix.android.sdk.internal.database.mapper.toEntity
-import org.matrix.android.sdk.internal.database.model.PusherEntity
-import org.matrix.android.sdk.internal.database.query.where
-import org.matrix.android.sdk.internal.di.SessionDatabase
-import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
-import org.matrix.android.sdk.internal.network.executeRequest
 import org.matrix.android.sdk.internal.session.SessionComponent
-import org.matrix.android.sdk.internal.util.awaitTransaction
 import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker
 import org.matrix.android.sdk.internal.worker.SessionWorkerParams
 import javax.inject.Inject
@@ -43,9 +34,7 @@ internal class AddPusherWorker(context: Context, params: WorkerParameters) :
             override val lastFailureMessage: String? = null
     ) : SessionWorkerParams
 
-    @Inject lateinit var pushersAPI: PushersAPI
-    @Inject @SessionDatabase lateinit var monarchy: Monarchy
-    @Inject lateinit var globalErrorReceiver: GlobalErrorReceiver
+    @Inject lateinit var addPusherTask: AddPusherTask
 
     override fun injectWith(injector: SessionComponent) {
         injector.inject(this)
@@ -58,20 +47,12 @@ internal class AddPusherWorker(context: Context, params: WorkerParameters) :
             return Result.failure()
         }
         return try {
-            setPusher(pusher)
+            addPusherTask.execute(AddPusherTask.Params(pusher))
             Result.success()
         } catch (exception: Throwable) {
             when (exception) {
                 is Failure.NetworkConnection -> Result.retry()
-                else                         -> {
-                    monarchy.awaitTransaction { realm ->
-                        PusherEntity.where(realm, pusher.pushKey).findFirst()?.let {
-                            // update it
-                            it.state = PusherState.FAILED_TO_REGISTER
-                        }
-                    }
-                    Result.failure()
-                }
+                else                         -> Result.failure()
             }
         }
     }
@@ -79,29 +60,4 @@ internal class AddPusherWorker(context: Context, params: WorkerParameters) :
     override fun buildErrorParams(params: Params, message: String): Params {
         return params.copy(lastFailureMessage = params.lastFailureMessage ?: message)
     }
-
-    private suspend fun setPusher(pusher: JsonPusher) {
-        executeRequest(globalErrorReceiver) {
-            pushersAPI.setPusher(pusher)
-        }
-        monarchy.awaitTransaction { realm ->
-            val echo = PusherEntity.where(realm, pusher.pushKey).findFirst()
-            if (echo != null) {
-                // update it
-                echo.appDisplayName = pusher.appDisplayName
-                echo.appId = pusher.appId
-                echo.kind = pusher.kind
-                echo.lang = pusher.lang
-                echo.profileTag = pusher.profileTag
-                echo.data?.format = pusher.data?.format
-                echo.data?.url = pusher.data?.url
-                echo.state = PusherState.REGISTERED
-            } else {
-                pusher.toEntity().also {
-                    it.state = PusherState.REGISTERED
-                    realm.insertOrUpdate(it)
-                }
-            }
-        }
-    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushers/DefaultPushersService.kt

@@ -30,7 +30,6 @@ import org.matrix.android.sdk.internal.session.pushers.gateway.PushGatewayNotify
 import org.matrix.android.sdk.internal.task.TaskExecutor
 import org.matrix.android.sdk.internal.task.configureWith
 import org.matrix.android.sdk.internal.worker.WorkerParamsFactory
-import java.security.InvalidParameterException
 import java.util.UUID
 import java.util.concurrent.TimeUnit
 import javax.inject.Inject
@@ -41,6 +40,7 @@ internal class DefaultPushersService @Inject constructor(
         @SessionId private val sessionId: String,
         private val getPusherTask: GetPushersTask,
         private val pushGatewayNotifyTask: PushGatewayNotifyTask,
+        private val addPusherTask: AddPusherTask,
         private val removePusherTask: RemovePusherTask,
         private val taskExecutor: TaskExecutor
 ) : PushersService {
@@ -58,7 +58,7 @@ internal class DefaultPushersService @Inject constructor(
                 .executeBy(taskExecutor)
     }
 
-    override fun addHttpPusher(pushkey: String,
+    override fun enqueueAddHttpPusher(pushkey: String,
                                       appId: String,
                                       profileTag: String,
                                       lang: String,
@@ -67,10 +67,11 @@ internal class DefaultPushersService @Inject constructor(
                                       url: String,
                                       append: Boolean,
                                       withEventIdOnly: Boolean
-    ) = addPusher(
+    ): UUID {
+        return enqueueAddPusher(
                 JsonPusher(
                         pushKey = pushkey,
-                    kind = Pusher.KIND_HTTP,
+                        kind = "http",
                         appId = appId,
                         profileTag = profileTag,
                         lang = lang,
@@ -80,15 +81,42 @@ internal class DefaultPushersService @Inject constructor(
                         append = append
                 )
         )
+    }
 
-    override fun addEmailPusher(email: String,
+    override suspend fun addHttpPusher(pushkey: String,
+                                       appId: String,
+                                       profileTag: String,
+                                       lang: String,
+                                       appDisplayName: String,
+                                       deviceDisplayName: String,
+                                       url: String,
+                                       append: Boolean,
+                                       withEventIdOnly: Boolean) {
+        addPusherTask.execute(
+                AddPusherTask.Params(
+                        JsonPusher(
+                                pushKey = pushkey,
+                                kind = "http",
+                                appId = appId,
+                                profileTag = profileTag,
+                                lang = lang,
+                                appDisplayName = appDisplayName,
+                                deviceDisplayName = deviceDisplayName,
+                                data = JsonPusherData(url, EVENT_ID_ONLY.takeIf { withEventIdOnly }),
+                                append = append
+                        )
+                )
+        )
+    }
+
+    override suspend fun addEmailPusher(email: String,
                                         lang: String,
                                         emailBranding: String,
                                         appDisplayName: String,
                                         deviceDisplayName: String,
-                                append: Boolean
-    ) = addPusher(
-            JsonPusher(
+                                        append: Boolean) {
+        addPusherTask.execute(
+                AddPusherTask.Params(JsonPusher(
                         pushKey = email,
                         kind = Pusher.KIND_EMAIL,
                         appId = Pusher.APP_ID_EMAIL,
@@ -98,11 +126,11 @@ internal class DefaultPushersService @Inject constructor(
                         deviceDisplayName = deviceDisplayName,
                         data = JsonPusherData(brand = emailBranding),
                         append = append
+                ))
         )
-    )
+    }
 
-    private fun addPusher(pusher: JsonPusher): UUID {
-        pusher.validateParameters()
+    private fun enqueueAddPusher(pusher: JsonPusher): UUID {
         val params = AddPusherWorker.Params(sessionId, pusher)
         val request = workManagerProvider.matrixOneTimeWorkRequestBuilder<AddPusherWorker>()
                 .setConstraints(WorkManagerProvider.workConstraints)
@@ -113,13 +141,6 @@ internal class DefaultPushersService @Inject constructor(
         return request.id
     }
 
-    private fun JsonPusher.validateParameters() {
-        // Do some parameter checks. It's ok to throw Exception, to inform developer of the problem
-        if (pushKey.length > 512) throw InvalidParameterException("pushkey should not exceed 512 chars")
-        if (appId.length > 64) throw InvalidParameterException("appId should not exceed 64 chars")
-        data?.url?.let { url -> if ("/_matrix/push/v1/notify" !in url) throw InvalidParameterException("url should contain '/_matrix/push/v1/notify'") }
-    }
-
     override suspend fun removePusher(pusher: Pusher) {
         removePusher(pusher.pushKey, pusher.appId)
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushers/JsonPusher.kt

@@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.session.pushers
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 import org.matrix.android.sdk.internal.di.SerializeNulls
+import java.security.InvalidParameterException
 
 /**
  * Example:
@@ -112,4 +113,11 @@ internal data class JsonPusher(
          */
         @Json(name = "append")
         val append: Boolean? = false
-)
+) {
+    init {
+        // Do some parameter checks. It's ok to throw Exception, to inform developer of the problem
+        if (pushKey.length > 512) throw InvalidParameterException("pushkey should not exceed 512 chars")
+        if (appId.length > 64) throw InvalidParameterException("appId should not exceed 64 chars")
+        data?.url?.let { url -> if ("/_matrix/push/v1/notify" !in url) throw InvalidParameterException("url should contain '/_matrix/push/v1/notify'") }
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushers/PushersModule.kt

@@ -65,6 +65,9 @@ internal abstract class PushersModule {
     @Binds
     abstract fun bindSavePushRulesTask(task: DefaultSavePushRulesTask): SavePushRulesTask
 
+    @Binds
+    abstract fun bindAddPusherTask(task: DefaultAddPusherTask): AddPusherTask
+
     @Binds
     abstract fun bindRemovePusherTask(task: DefaultRemovePusherTask): RemovePusherTask
 

vector/src/main/java/im/vector/app/core/pushers/PushersManager.kt

@@ -48,7 +48,7 @@ class PushersManager @Inject constructor(
         val currentSession = activeSessionHolder.getActiveSession()
         val profileTag = DEFAULT_PUSHER_FILE_TAG + "_" + abs(currentSession.myUserId.hashCode())
 
-        return currentSession.addHttpPusher(
+        return currentSession.enqueueAddHttpPusher(
                 pushKey,
                 stringProvider.getString(R.string.pusher_app_id),
                 profileTag,
@@ -61,7 +61,7 @@ class PushersManager @Inject constructor(
         )
     }
 
-    fun registerEmailForPush(email: String) {
+    suspend fun registerEmailForPush(email: String) {
         val currentSession = activeSessionHolder.getActiveSession()
         val appName = appNameProvider.getAppName()
         currentSession.addEmailPusher(