Rework Action (better kotlin code)

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/Action.kt

@@ -19,77 +19,92 @@ import im.vector.matrix.android.api.pushrules.rest.PushRule
 import timber.log.Timber
 
 
-class Action(val type: Type) {
+sealed class Action {
-
+    object Notify : Action()
-    enum class Type(val value: String) {
+    object DoNotNotify : Action()
-        NOTIFY("notify"),
+    data class Sound(val sound: String) : Action()
-        DONT_NOTIFY("dont_notify"),
+    data class Highlight(val highlight: Boolean) : Action()
-        COALESCE("coalesce"),
-        SET_TWEAK("set_tweak");
-
-        companion object {
-
-            fun safeValueOf(value: String): Type? {
-                try {
-                    return valueOf(value)
-                } catch (e: IllegalArgumentException) {
-                    return null
-                }
-            }
-        }
 }
 
-    var tweak_action: String? = null
-    var stringValue: String? = null
-    var boolValue: Boolean? = null
 
-    companion object {
+private const val ACTION_NOTIFY = "notify"
-        fun mapFrom(pushRule: PushRule): List<Action>? {
+private const val ACTION_DONT_NOTIFY = "dont_notify"
-            val actions = ArrayList<Action>()
+private const val ACTION_COALESCE = "coalesce"
-            pushRule.actions.forEach { actionStrOrObj ->
+
-                if (actionStrOrObj is String) {
+// Ref: https://matrix.org/docs/spec/client_server/latest#tweaks
+private const val ACTION_OBJECT_SET_TWEAK_KEY = "set_tweak"
+
+private const val ACTION_OBJECT_SET_TWEAK_VALUE_SOUND = "sound"
+private const val ACTION_OBJECT_SET_TWEAK_VALUE_HIGHLIGHT = "highlight"
+
+private const val ACTION_OBJECT_VALUE_KEY = "value"
+private const val ACTION_OBJECT_VALUE_VALUE_DEFAULT = "default"
+
+/**
+ * Ref: https://matrix.org/docs/spec/client_server/latest#actions
+ *
+ * Convert
+ * <pre>
+ * "actions": [
+ *     "notify",
+ *     {
+ *         "set_tweak": "sound",
+ *         "value": "default"
+ *     },
+ *     {
+ *         "set_tweak": "highlight"
+ *     }
+ *   ]
+ *
+ * To
+ * [
+ *     Action.Notify,
+ *     Action.Sound("default"),
+ *     Action.Highlight(true)
+ * ]
+ *
+ * </pre>
+ */
+fun PushRule.getActions(): List<Action> {
+    val result = ArrayList<Action>()
+
+    actions.forEach { actionStrOrObj ->
         when (actionStrOrObj) {
-                        Action.Type.NOTIFY.value      -> Action(Action.Type.NOTIFY)
+            ACTION_NOTIFY      -> Action.Notify
-                        Action.Type.DONT_NOTIFY.value -> Action(Action.Type.DONT_NOTIFY)
+            ACTION_DONT_NOTIFY -> Action.DoNotNotify
+            is Map<*, *>       -> {
+                when (actionStrOrObj[ACTION_OBJECT_SET_TWEAK_KEY]) {
+                    ACTION_OBJECT_SET_TWEAK_VALUE_SOUND     -> {
+                        (actionStrOrObj[ACTION_OBJECT_VALUE_KEY] as? String)?.let { stringValue ->
+                            Action.Sound(stringValue)
+                        }
+                        // When the value is not there, default sound (not specified by the spec)
+                                ?: Action.Sound(ACTION_OBJECT_VALUE_VALUE_DEFAULT)
+
+                    }
+                    ACTION_OBJECT_SET_TWEAK_VALUE_HIGHLIGHT -> {
+                        (actionStrOrObj[ACTION_OBJECT_VALUE_KEY] as? Boolean)?.let { boolValue ->
+                            Action.Highlight(boolValue)
+                        }
+                        // When the value is not there, default is true, says the spec
+                                ?: Action.Highlight(true)
+                    }
                     else                                    -> {
-                            Timber.w("Unsupported action type ${actionStrOrObj}")
+                        Timber.w("Unsupported set_tweak value ${actionStrOrObj[ACTION_OBJECT_SET_TWEAK_KEY]}")
+                        null
+                    }
+                }
+            }
+            else               -> {
+                Timber.w("Unsupported action type $actionStrOrObj")
                 null
             }
         }?.let {
-                        actions.add(it)
+            result.add(it)
-                    }
-                } else if (actionStrOrObj is Map<*, *>) {
-                    val tweakAction = actionStrOrObj["set_tweak"] as? String
-                    when (tweakAction) {
-                        "sound"     -> {
-                            (actionStrOrObj["value"] as? String)?.let { stringValue ->
-                                Action(Action.Type.SET_TWEAK).also {
-                                    it.tweak_action = "sound"
-                                    it.stringValue = stringValue
-                                    actions.add(it)
-                                }
-                            }
-                        }
-                        "highlight" -> {
-                            (actionStrOrObj["value"] as? Boolean)?.let { boolValue ->
-                                Action(Action.Type.SET_TWEAK).also {
-                                    it.tweak_action = "highlight"
-                                    it.boolValue = boolValue
-                                    actions.add(it)
-                                }
-                            }
-                        }
-                        else        -> {
-                            Timber.w("Unsupported action type ${actionStrOrObj}")
-                        }
-                    }
-                } else {
-                    Timber.w("Unsupported action type ${actionStrOrObj}")
-                    return null
-                }
-            }
-            return if (actions.isEmpty()) null else actions
-        }
         }
     }
 
+
+    return result
+}
+

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt

@@ -17,10 +17,10 @@ package im.vector.matrix.android.internal.session.notification
 
 import com.zhuinden.monarchy.Monarchy
 import im.vector.matrix.android.api.MatrixCallback
-import im.vector.matrix.android.api.pushrules.Action
 import im.vector.matrix.android.api.pushrules.PushRuleService
 import im.vector.matrix.android.api.pushrules.RuleKind
 import im.vector.matrix.android.api.pushrules.RuleSetKey
+import im.vector.matrix.android.api.pushrules.getActions
 import im.vector.matrix.android.api.pushrules.rest.PushRule
 import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.util.Cancelable
@@ -123,8 +123,9 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
 
     fun dispatchBing(event: Event, rule: PushRule) {
         try {
+            val actionsList = rule.getActions()
             listeners.forEach {
-                it.onMatchRule(event, Action.mapFrom(rule) ?: emptyList())
+                it.onMatchRule(event, actionsList)
             }
         } catch (e: Throwable) {
             Timber.e(e, "Error while dispatching bing")

vector/src/main/java/im/vector/riotx/features/notifications/NotificationAction.kt

@@ -19,24 +19,21 @@ import im.vector.matrix.android.api.pushrules.Action
 
 data class NotificationAction(
         val shouldNotify: Boolean,
-        val highlight: Boolean = false,
+        val highlight: Boolean,
-        val soundName: String? = null
+        val soundName: String?
-) {
+)
-    companion object {
+
-        fun extractFrom(ruleActions: List<Action>): NotificationAction {
+fun List<Action>.toNotificationAction(): NotificationAction {
     var shouldNotify = false
     var highlight = false
     var sound: String? = null
-            ruleActions.forEach {
+    forEach { action ->
-                // TODO When
+        when (action) {
-                if (it.type == Action.Type.NOTIFY) shouldNotify = true
+            is Action.Notify      -> shouldNotify = true
-                if (it.type == Action.Type.DONT_NOTIFY) shouldNotify = false
+            is Action.DoNotNotify -> shouldNotify = false
-                if (it.type == Action.Type.SET_TWEAK) {
+            is Action.Highlight   -> highlight = action.highlight
-                    if (it.tweak_action == "highlight") highlight = it.boolValue ?: false
+            is Action.Sound       -> sound = action.sound
-                    if (it.tweak_action == "sound") sound = it.stringValue
         }
     }
     return NotificationAction(shouldNotify, highlight, sound)
 }
-    }
-}

vector/src/main/java/im/vector/riotx/features/notifications/PushRuleTriggerListener.kt

@@ -40,7 +40,7 @@ class PushRuleTriggerListener @Inject constructor(
             Timber.e("Called without active session")
             return
         }
-        val notificationAction = NotificationAction.extractFrom(actions)
+        val notificationAction = actions.toNotificationAction()
         if (notificationAction.shouldNotify) {
             val notifiableEvent = resolver.resolveEvent(event, session!!)
             if (notifiableEvent == null) {

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

@@ -26,11 +26,11 @@ import androidx.core.view.isVisible
 import com.airbnb.epoxy.EpoxyAttribute
 import com.airbnb.epoxy.EpoxyModelClass
 import com.airbnb.epoxy.EpoxyModelWithHolder
-import im.vector.matrix.android.api.pushrules.Action
+import im.vector.matrix.android.api.pushrules.getActions
 import im.vector.matrix.android.api.pushrules.rest.PushRule
 import im.vector.riotx.R
 import im.vector.riotx.core.epoxy.VectorEpoxyHolder
-import im.vector.riotx.features.notifications.NotificationAction
+import im.vector.riotx.features.notifications.toNotificationAction
 
 
 @EpoxyModelClass(layout = R.layout.item_pushrule_raw)
@@ -50,12 +50,12 @@ abstract class PushRuleItem : EpoxyModelWithHolder<PushRuleItem.Holder>() {
             holder.view.setBackgroundColor(ContextCompat.getColor(context, R.color.vector_silver_color))
             holder.ruleId.text = "[Disabled] ${pushRule.ruleId}"
         }
-        val actions = Action.mapFrom(pushRule)
+        val actions = pushRule.getActions()
-        if (actions.isNullOrEmpty()) {
+        if (actions.isEmpty()) {
             holder.actionIcon.isInvisible = true
         } else {
             holder.actionIcon.isVisible = true
-            val notifAction = NotificationAction.extractFrom(actions)
+            val notifAction = actions.toNotificationAction()
 
             if (notifAction.shouldNotify && !notifAction.soundName.isNullOrBlank()) {
                 holder.actionIcon.setImageDrawable(ContextCompat.getDrawable(context, R.drawable.ic_action_notify_noisy))

vector/src/main/java/im/vector/riotx/features/settings/troubleshoot/TestBingRulesSettings.kt

@@ -15,12 +15,12 @@
  */
 package im.vector.riotx.features.settings.troubleshoot
 
-import im.vector.matrix.android.api.pushrules.Action
 import im.vector.matrix.android.api.pushrules.RuleIds
+import im.vector.matrix.android.api.pushrules.getActions
 import im.vector.riotx.R
 import im.vector.riotx.core.di.ActiveSessionHolder
 import im.vector.riotx.core.resources.StringProvider
-import im.vector.riotx.features.notifications.NotificationAction
+import im.vector.riotx.features.notifications.toNotificationAction
 import javax.inject.Inject
 
 class TestBingRulesSettings @Inject constructor(private val activeSessionHolder: ActiveSessionHolder,
@@ -50,8 +50,8 @@ class TestBingRulesSettings @Inject constructor(private val activeSessionHolder:
             var oneOrMoreRuleAreSilent = false
             for ((index, ruleId) in testedRules.withIndex()) {
                 pushRules.find { it.ruleId == ruleId }?.let { rule ->
-                    val actions = Action.mapFrom(rule) ?: return@let
+                    val actions = rule.getActions()
-                    val notifAction = NotificationAction.extractFrom(actions)
+                    val notifAction = actions.toNotificationAction()
                     if (!rule.enabled || !notifAction.shouldNotify) {
                         //off
                         oneOrMoreRuleIsOff = true