Merge pull request #1117 from vector-im/feature/pusher_fix

Pusher fix

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

@@ -40,24 +40,35 @@ import im.vector.matrix.android.internal.di.SerializeNulls
  *  }
  * </code>
  */
-
 @JsonClass(generateAdapter = true)
 internal data class JsonPusher(
         /**
-         * Required. This is a unique identifier for this pusher. See /set for more detail. Max length, 512 bytes.
+         * Required. This is a unique identifier for this pusher. The value you should use for this is the routing or
+         * destination address information for the notification, for example, the APNS token for APNS or the
+         * Registration ID for GCM. If your notification client has no such concept, use any unique identifier.
+         * Max length, 512 bytes.
+         *
+         * If the kind is "email", this is the email address to send notifications to.
          */
         @Json(name = "pushkey")
         val pushKey: String,
 
         /**
-         * Required. The kind of pusher. "http" is a pusher that sends HTTP pokes.
+         * Required. The kind of pusher to configure.
+         * "http" makes a pusher that sends HTTP pokes.
+         * "email" makes a pusher that emails the user with unread notifications.
+         * null deletes the pusher.
          */
         @SerializeNulls
         @Json(name = "kind")
         val kind: String?,
 
         /**
-         * Required. This is a reverse-DNS style identifier for the application. Max length, 64 chars.
+         * Required. This is a reverse-DNS style identifier for the application. It is recommended that this end
+         * with the platform, such that different platform versions get different app identifiers.
+         * Max length, 64 chars.
+         *
+         * If the kind is "email", this is "m.email".
          */
         @Json(name = "app_id")
         val appId: String,
@@ -88,15 +99,17 @@ internal data class JsonPusher(
 
         /**
          * Required. A dictionary of information for the pusher implementation itself.
+         * If kind is http, this should contain url which is the URL to use to send notifications to.
          */
         @Json(name = "data")
         val data: JsonPusherData? = null,
 
-        // Only used to update add Pusher (body of api request)
-        // Used If true, the homeserver should add another pusher with the given pushkey and App ID in addition
-        // 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.
-        // The default is false.
+        /**
+         * If true, the homeserver should add another pusher with the given pushkey and App ID in addition 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.
+         * The default is false.
+         */
         @Json(name = "append")
         val append: Boolean? = false
 )

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/pushers/JsonPusherData.kt

@@ -22,12 +22,14 @@ import com.squareup.moshi.JsonClass
 internal data class JsonPusherData(
         /**
          * Required if kind is http. The URL to use to send notifications to.
+         * MUST be an HTTPS URL with a path of /_matrix/push/v1/notify.
          */
         @Json(name = "url")
         val url: String? = null,
 
         /**
-         * The format to use when sending notifications to the Push Gateway.
+         * The format to send notifications in to Push Gateways if the kind is http.
+         * Currently the only format available is 'event_id_only'.
          */
         @Json(name = "format")
         val format: String? = null

vector/src/fdroid/java/im/vector/riotx/push/fcm/FcmHelper.kt

@@ -57,7 +57,7 @@ object FcmHelper {
      *
      * @param activity the first launch Activity
      */
-    fun ensureFcmTokenIsRetrieved(activity: Activity, pushersManager: PushersManager) {
+    fun ensureFcmTokenIsRetrieved(activity: Activity, pushersManager: PushersManager, registerPusher: Boolean) {
         // No op
     }
 

vector/src/gplay/java/im/vector/riotx/push/fcm/FcmHelper.kt

@@ -68,7 +68,7 @@ object FcmHelper {
      *
      * @param activity the first launch Activity
      */
-    fun ensureFcmTokenIsRetrieved(activity: Activity, pushersManager: PushersManager) {
+    fun ensureFcmTokenIsRetrieved(activity: Activity, pushersManager: PushersManager, registerPusher: Boolean) {
         //        if (TextUtils.isEmpty(getFcmToken(activity))) {
         // 'app should always check the device for a compatible Google Play services APK before accessing Google Play services features'
         if (checkPlayServices(activity)) {
@@ -76,7 +76,9 @@ object FcmHelper {
                 FirebaseInstanceId.getInstance().instanceId
                         .addOnSuccessListener(activity) { instanceIdResult ->
                             storeFcmToken(activity, instanceIdResult.token)
-                            pushersManager.registerPusherWithFcmKey(instanceIdResult.token)
+                            if (registerPusher) {
+                                pushersManager.registerPusherWithFcmKey(instanceIdResult.token)
+                            }
                         }
                         .addOnFailureListener(activity) { e -> Timber.e(e, "## ensureFcmTokenIsRetrieved() : failed") }
             } catch (e: Throwable) {

vector/src/main/java/im/vector/riotx/features/home/HomeActivity.kt

@@ -43,6 +43,7 @@ import im.vector.riotx.features.disclaimer.showDisclaimerDialog
 import im.vector.riotx.features.notifications.NotificationDrawerManager
 import im.vector.riotx.features.popup.PopupAlertManager
 import im.vector.riotx.features.rageshake.VectorUncaughtExceptionHandler
+import im.vector.riotx.features.settings.VectorPreferences
 import im.vector.riotx.features.workers.signout.SignOutViewModel
 import im.vector.riotx.push.fcm.FcmHelper
 import kotlinx.android.synthetic.main.activity_home.*
@@ -58,6 +59,7 @@ class HomeActivity : VectorBaseActivity(), ToolbarConfigurable {
     @Inject lateinit var vectorUncaughtExceptionHandler: VectorUncaughtExceptionHandler
     @Inject lateinit var pushManager: PushersManager
     @Inject lateinit var notificationDrawerManager: NotificationDrawerManager
+    @Inject lateinit var vectorPreferences: VectorPreferences
 
     private val drawerListener = object : DrawerLayout.SimpleDrawerListener() {
         override fun onDrawerStateChanged(newState: Int) {
@@ -73,7 +75,7 @@ class HomeActivity : VectorBaseActivity(), ToolbarConfigurable {
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
-        FcmHelper.ensureFcmTokenIsRetrieved(this, pushManager)
+        FcmHelper.ensureFcmTokenIsRetrieved(this, pushManager, vectorPreferences.areNotificationEnabledForDevice())
         sharedActionViewModel = viewModelProvider.get(HomeSharedActionViewModel::class.java)
         drawerLayout.addDrawerListener(drawerListener)
         if (isFirstCreation()) {

vector/src/main/java/im/vector/riotx/features/settings/VectorPreferences.kt

@@ -236,9 +236,9 @@ class VectorPreferences @Inject constructor(private val context: Context) {
         return defaultPrefs.getBoolean(SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY, true)
     }
 
-    fun setNotificationEnabledForDevice(enabled: Boolean?) {
+    fun setNotificationEnabledForDevice(enabled: Boolean) {
         defaultPrefs.edit {
-            putBoolean(SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY, enabled!!)
+            putBoolean(SETTINGS_ENABLE_THIS_DEVICE_PREFERENCE_KEY, enabled)
         }
     }
 

vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsNotificationPreferenceFragment.kt

@@ -161,20 +161,21 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
         val switchPref = preference as SwitchPreference
         if (switchPref.isChecked) {
             FcmHelper.getFcmToken(requireContext())?.let {
-                if (vectorPreferences.areNotificationEnabledForDevice()) {
-                    pushManager.registerPusherWithFcmKey(it)
-                }
+                pushManager.registerPusherWithFcmKey(it)
             }
         } else {
             FcmHelper.getFcmToken(requireContext())?.let {
                 pushManager.unregisterPusher(it, object : MatrixCallback<Unit> {
                     override fun onSuccess(data: Unit) {
                         session.refreshPushers()
-                        super.onSuccess(data)
                     }
 
                     override fun onFailure(failure: Throwable) {
-                        session.refreshPushers()
+                        if (!isAdded) {
+                            return
+                        }
+                        // revert the check box
+                        switchPref.isChecked = !switchPref.isChecked
                         Toast.makeText(activity, R.string.unknown_error, Toast.LENGTH_SHORT).show()
                     }
                 })

vector/src/main/java/im/vector/riotx/features/settings/push/PushGatewayAction.kt

@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2020 New Vector Ltd
+ *
+ * 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 im.vector.riotx.features.settings.push
+
+import im.vector.riotx.core.platform.VectorViewModelAction
+
+sealed class PushGatewayAction : VectorViewModelAction {
+    object Refresh : PushGatewayAction()
+}

vector/src/main/java/im/vector/riotx/features/settings/push/PushGatewaysFragment.kt

@@ -17,6 +17,7 @@
 package im.vector.riotx.features.settings.push
 
 import android.os.Bundle
+import android.view.MenuItem
 import android.view.View
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
@@ -38,6 +39,19 @@ class PushGatewaysFragment @Inject constructor(
 
     private val viewModel: PushGatewaysViewModel by fragmentViewModel(PushGatewaysViewModel::class)
 
+    override fun getMenuRes() = R.menu.menu_push_gateways
+
+    override fun onOptionsItemSelected(item: MenuItem): Boolean {
+        return when (item.itemId) {
+            R.id.refresh -> {
+                viewModel.handle(PushGatewayAction.Refresh)
+                true
+            }
+            else         ->
+                super.onOptionsItemSelected(item)
+        }
+    }
+
     override fun onResume() {
         super.onResume()
         (activity as? VectorBaseActivity)?.supportActionBar?.setTitle(R.string.settings_notifications_targets)

vector/src/main/java/im/vector/riotx/features/settings/push/PushGatewaysViewModel.kt

@@ -27,7 +27,7 @@ import com.squareup.inject.assisted.AssistedInject
 import im.vector.matrix.android.api.session.Session
 import im.vector.matrix.android.api.session.pushers.Pusher
 import im.vector.matrix.rx.RxSession
-import im.vector.riotx.core.platform.EmptyAction
+import im.vector.riotx.core.extensions.exhaustive
 import im.vector.riotx.core.platform.EmptyViewEvents
 import im.vector.riotx.core.platform.VectorViewModel
 
@@ -37,7 +37,7 @@ data class PushGatewayViewState(
 
 class PushGatewaysViewModel @AssistedInject constructor(@Assisted initialState: PushGatewayViewState,
                                                         private val session: Session)
-    : VectorViewModel<PushGatewayViewState, EmptyAction, EmptyViewEvents>(initialState) {
+    : VectorViewModel<PushGatewayViewState, PushGatewayAction, EmptyViewEvents>(initialState) {
 
     @AssistedInject.Factory
     interface Factory {
@@ -55,6 +55,8 @@ class PushGatewaysViewModel @AssistedInject constructor(@Assisted initialState:
 
     init {
         observePushers()
+        // Force a refresh
+        session.refreshPushers()
     }
 
     private fun observePushers() {
@@ -65,7 +67,13 @@ class PushGatewaysViewModel @AssistedInject constructor(@Assisted initialState:
                 }
     }
 
-    override fun handle(action: EmptyAction) {
-        // No op
+    override fun handle(action: PushGatewayAction) {
+        when (action) {
+            is PushGatewayAction.Refresh -> handleRefresh()
+        }.exhaustive
+    }
+
+    private fun handleRefresh() {
+        session.refreshPushers()
     }
 }

vector/src/main/res/menu/menu_push_gateways.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<menu xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:app="http://schemas.android.com/apk/res-auto">
+
+    <item
+        android:id="@+id/refresh"
+        android:icon="@drawable/ic_refresh_cw"
+        android:title="@string/refresh"
+        app:showAsAction="always" />
+
+</menu>

vector/src/main/res/values/strings_riotX.xml

@@ -18,7 +18,7 @@
     <string name="settings_when_rooms_are_upgraded">When rooms are upgraded</string>
     <string name="settings_troubleshoot_title">Troubleshoot</string>
     <string name="settings_notification_advanced_summary_riotx">Set notification importance by event</string>
-
+    <string name="refresh">Refresh</string>
     <!-- END Strings added by Benoit -->