From d3d4deb8840db429c15f11cd9d279043bee7ab2f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 20 Sep 2019 15:26:48 +0200 Subject: [PATCH] Rework Action (better kotlin code) --- .../matrix/android/api/pushrules/Action.kt | 159 ++++++++++-------- .../notification/DefaultPushRuleService.kt | 5 +- .../notifications/NotificationAction.kt | 35 ++-- .../notifications/PushRuleTriggerListener.kt | 2 +- .../features/settings/push/PushRuleItem.kt | 10 +- .../troubleshoot/TestBingRulesSettings.kt | 20 +-- 6 files changed, 122 insertions(+), 109 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/Action.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/Action.kt index 797830f491..b34d2d0d34 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/Action.kt +++ b/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) { - - enum class Type(val value: String) { - NOTIFY("notify"), - DONT_NOTIFY("dont_notify"), - 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 { - fun mapFrom(pushRule: PushRule): List? { - val actions = ArrayList() - pushRule.actions.forEach { actionStrOrObj -> - if (actionStrOrObj is String) { - when (actionStrOrObj) { - Action.Type.NOTIFY.value -> Action(Action.Type.NOTIFY) - Action.Type.DONT_NOTIFY.value -> Action(Action.Type.DONT_NOTIFY) - else -> { - Timber.w("Unsupported action type ${actionStrOrObj}") - null - } - }?.let { - actions.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 - } - } +sealed class Action { + object Notify : Action() + object DoNotNotify : Action() + data class Sound(val sound: String) : Action() + data class Highlight(val highlight: Boolean) : Action() +} + + +private const val ACTION_NOTIFY = "notify" +private const val ACTION_DONT_NOTIFY = "dont_notify" +private const val ACTION_COALESCE = "coalesce" + +// 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 + * 
+ * "actions": [
+ *     "notify",
+ *     {
+ *         "set_tweak": "sound",
+ *         "value": "default"
+ *     },
+ *     {
+ *         "set_tweak": "highlight"
+ *     }
+ *   ]
+ *
+ * To
+ * [
+ *     Action.Notify,
+ *     Action.Sound("default"),
+ *     Action.Highlight(true)
+ * ]
+ *
+ *
+ */ +fun PushRule.getActions(): List { + val result = ArrayList() + + actions.forEach { actionStrOrObj -> + when (actionStrOrObj) { + ACTION_NOTIFY -> Action.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 set_tweak value ${actionStrOrObj[ACTION_OBJECT_SET_TWEAK_KEY]}") + null + } + } + } + else -> { + Timber.w("Unsupported action type $actionStrOrObj") + null + } + }?.let { + result.add(it) + } + } + + + return result } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt index 41d1431e1c..ac3ff064f4 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt +++ b/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") diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationAction.kt b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationAction.kt index 786ff5060f..6322ecf998 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationAction.kt +++ b/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 soundName: String? = null -) { - companion object { - fun extractFrom(ruleActions: List): NotificationAction { - var shouldNotify = false - var highlight = false - var sound: String? = null - ruleActions.forEach { - // TODO When - if (it.type == Action.Type.NOTIFY) shouldNotify = true - if (it.type == Action.Type.DONT_NOTIFY) shouldNotify = false - if (it.type == Action.Type.SET_TWEAK) { - if (it.tweak_action == "highlight") highlight = it.boolValue ?: false - if (it.tweak_action == "sound") sound = it.stringValue - } - } - return NotificationAction(shouldNotify, highlight, sound) + val highlight: Boolean, + val soundName: String? +) + +fun List.toNotificationAction(): NotificationAction { + var shouldNotify = false + var highlight = false + var sound: String? = null + forEach { action -> + when (action) { + is Action.Notify -> shouldNotify = true + is Action.DoNotNotify -> shouldNotify = false + is Action.Highlight -> highlight = action.highlight + is Action.Sound -> sound = action.sound } } -} \ No newline at end of file + return NotificationAction(shouldNotify, highlight, sound) +} diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/riotx/features/notifications/PushRuleTriggerListener.kt index 6711d57a22..7f1c501334 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/PushRuleTriggerListener.kt +++ b/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) { diff --git a/vector/src/main/java/im/vector/riotx/features/settings/push/PushRuleItem.kt b/vector/src/main/java/im/vector/riotx/features/settings/push/PushRuleItem.kt index 47c9a06a09..5e38a7f7ca 100644 --- a/vector/src/main/java/im/vector/riotx/features/settings/push/PushRuleItem.kt +++ b/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() { holder.view.setBackgroundColor(ContextCompat.getColor(context, R.color.vector_silver_color)) holder.ruleId.text = "[Disabled] ${pushRule.ruleId}" } - val actions = Action.mapFrom(pushRule) - if (actions.isNullOrEmpty()) { + val actions = pushRule.getActions() + 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)) diff --git a/vector/src/main/java/im/vector/riotx/features/settings/troubleshoot/TestBingRulesSettings.kt b/vector/src/main/java/im/vector/riotx/features/settings/troubleshoot/TestBingRulesSettings.kt index 8f353df282..a58d48061d 100644 --- a/vector/src/main/java/im/vector/riotx/features/settings/troubleshoot/TestBingRulesSettings.kt +++ b/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, @@ -29,15 +29,15 @@ class TestBingRulesSettings @Inject constructor(private val activeSessionHolder: private val testedRules = listOf(RuleIds.RULE_ID_CONTAIN_DISPLAY_NAME, - RuleIds.RULE_ID_CONTAIN_USER_NAME, - RuleIds.RULE_ID_ONE_TO_ONE_ROOM, - RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS) + RuleIds.RULE_ID_CONTAIN_USER_NAME, + RuleIds.RULE_ID_ONE_TO_ONE_ROOM, + RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS) val ruleSettingsName = arrayOf(R.string.settings_containing_my_display_name, - R.string.settings_containing_my_user_name, - R.string.settings_messages_in_one_to_one, - R.string.settings_messages_in_group_chat) + R.string.settings_containing_my_user_name, + R.string.settings_messages_in_one_to_one, + R.string.settings_messages_in_group_chat) override fun perform() { val session = activeSessionHolder.getSafeActiveSession() ?: return @@ -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 notifAction = NotificationAction.extractFrom(actions) + val actions = rule.getActions() + val notifAction = actions.toNotificationAction() if (!rule.enabled || !notifAction.shouldNotify) { //off oneOrMoreRuleIsOff = true