From 2e618999d9ff481c89920b69e23169c56efaf046 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 25 Aug 2020 17:26:16 +0200 Subject: [PATCH] Words containing my name should not trigger notifications (Fixes #1781) It adds a specific behavior for rule with id RuleIds.RULE_ID_CONTAIN_USER_NAME --- CHANGES.md | 1 + .../sdk/api/pushrules/EventMatchCondition.kt | 23 ++++++++---- .../sdk/api/pushrules/rest/PushCondition.kt | 5 ++- .../notification/ProcessEventForPushTask.kt | 2 +- .../api/pushrules/PushrulesConditionTest.kt | 37 +++++++++++++------ .../features/settings/push/PushRuleItem.kt | 6 +-- 6 files changed, 49 insertions(+), 25 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d3a16c9b29..2ab1214b36 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,7 @@ Improvements 🙌: Bugfix 🐛: - Display name not shown under Settings/General (#1926) + - Words containing my name should not trigger notifications (#1781) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/EventMatchCondition.kt index a6e3836a7a..c10d2fc1df 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/EventMatchCondition.kt @@ -18,6 +18,7 @@ package org.matrix.android.sdk.api.pushrules import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.internal.di.MoshiProvider +import org.matrix.android.sdk.internal.util.caseInsensitiveFind import org.matrix.android.sdk.internal.util.hasSpecialGlobChar import org.matrix.android.sdk.internal.util.simpleGlobToRegExp import timber.log.Timber @@ -31,14 +32,18 @@ class EventMatchCondition( * The glob-style pattern to match against. Patterns with no special glob characters should * be treated as having asterisks prepended and appended when testing the condition. */ - val pattern: String + val pattern: String, + /** + * true to match only words. In this case pattern will not be considered as a glob + */ + val wordsOnly: Boolean ) : Condition { override fun isSatisfied(event: Event, conditionResolver: ConditionResolver): Boolean { return conditionResolver.resolveEventMatchCondition(event, this) } - override fun technicalDescription() = "'$key' Matches '$pattern'" + override fun technicalDescription() = "'$key' Matches '$pattern', words only '$wordsOnly'" fun isSatisfied(event: Event): Boolean { // TODO encrypted events? @@ -48,14 +53,18 @@ class EventMatchCondition( // Patterns with no special glob characters should be treated as having asterisks prepended // and appended when testing the condition. - try { - val modPattern = if (pattern.hasSpecialGlobChar()) pattern.simpleGlobToRegExp() else "*$pattern*".simpleGlobToRegExp() - val regex = Regex(modPattern, RegexOption.DOT_MATCHES_ALL) - return regex.containsMatchIn(value) + return try { + if (wordsOnly) { + value.caseInsensitiveFind(pattern) + } else { + val modPattern = if (pattern.hasSpecialGlobChar()) pattern.simpleGlobToRegExp() else "*$pattern*".simpleGlobToRegExp() + val regex = Regex(modPattern, RegexOption.DOT_MATCHES_ALL) + regex.containsMatchIn(value) + } } catch (e: Throwable) { // e.g PatternSyntaxException Timber.e(e, "Failed to evaluate push condition") - return false + false } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushCondition.kt index e16c1df314..29466ca33b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushCondition.kt @@ -23,6 +23,7 @@ import org.matrix.android.sdk.api.pushrules.ContainsDisplayNameCondition import org.matrix.android.sdk.api.pushrules.EventMatchCondition import org.matrix.android.sdk.api.pushrules.Kind import org.matrix.android.sdk.api.pushrules.RoomMemberCountCondition +import org.matrix.android.sdk.api.pushrules.RuleIds import org.matrix.android.sdk.api.pushrules.SenderNotificationPermissionCondition import timber.log.Timber @@ -59,11 +60,11 @@ data class PushCondition( val iz: String? = null ) { - fun asExecutableCondition(): Condition? { + fun asExecutableCondition(rule: PushRule): Condition? { return when (Kind.fromString(kind)) { Kind.EventMatch -> { if (key != null && pattern != null) { - EventMatchCondition(key, pattern) + EventMatchCondition(key, pattern, rule.ruleId == RuleIds.RULE_ID_CONTAIN_USER_NAME) } else { Timber.e("Malformed Event match condition") null diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt index 49a92acc54..a0667cc4a3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt @@ -100,7 +100,7 @@ internal class DefaultProcessEventForPushTask @Inject constructor( return rules.firstOrNull { rule -> // All conditions must hold true for an event in order to apply the action for the event. rule.enabled && rule.conditions?.all { - it.asExecutableCondition()?.isSatisfied(event, conditionResolver) ?: false + it.asExecutableCondition(rule)?.isSatisfied(event, conditionResolver) ?: false } ?: false } } diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/pushrules/PushrulesConditionTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/pushrules/PushrulesConditionTest.kt index be5aeaaf0f..e428c7ab22 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/pushrules/PushrulesConditionTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/pushrules/PushrulesConditionTest.kt @@ -16,6 +16,12 @@ package org.matrix.android.sdk.api.pushrules +import io.mockk.every +import io.mockk.mockk +import org.amshove.kluent.shouldBe +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test import org.matrix.android.sdk.MatrixTest import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.toContent @@ -24,12 +30,6 @@ import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomMemberContent import org.matrix.android.sdk.api.session.room.model.message.MessageTextContent import org.matrix.android.sdk.internal.session.room.RoomGetter -import io.mockk.every -import io.mockk.mockk -import org.amshove.kluent.shouldBe -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test class PushrulesConditionTest: MatrixTest { @@ -39,7 +39,7 @@ class PushrulesConditionTest: MatrixTest { @Test fun test_eventmatch_type_condition() { - val condition = EventMatchCondition("type", "m.room.message") + val condition = EventMatchCondition("type", "m.room.message", false) val simpleTextEvent = Event( type = "m.room.message", @@ -65,7 +65,7 @@ class PushrulesConditionTest: MatrixTest { @Test fun test_eventmatch_path_condition() { - val condition = EventMatchCondition("content.msgtype", "m.text") + val condition = EventMatchCondition("content.msgtype", "m.text", false) val simpleTextEvent = Event( type = "m.room.message", @@ -86,13 +86,13 @@ class PushrulesConditionTest: MatrixTest { ).toContent(), originServerTs = 0 ).apply { - assert(EventMatchCondition("content.membership", "invite").isSatisfied(this)) + assert(EventMatchCondition("content.membership", "invite", false).isSatisfied(this)) } } @Test fun test_eventmatch_cake_condition() { - val condition = EventMatchCondition("content.body", "cake") + val condition = EventMatchCondition("content.body", "cake", false) Event( type = "m.room.message", @@ -115,7 +115,7 @@ class PushrulesConditionTest: MatrixTest { @Test fun test_eventmatch_cakelie_condition() { - val condition = EventMatchCondition("content.body", "cake*lie") + val condition = EventMatchCondition("content.body", "cake*lie", false) val simpleTextEvent = Event( type = "m.room.message", @@ -126,9 +126,22 @@ class PushrulesConditionTest: MatrixTest { assert(condition.isSatisfied(simpleTextEvent)) } + @Test + fun test_eventmatch_words_only_condition() { + val condition = EventMatchCondition("content.body", "ben", true) + + val simpleTextEvent = Event( + type = "m.room.message", + eventId = "mx0", + content = MessageTextContent("m.text", "benoit").toContent(), + originServerTs = 0) + + assertFalse(condition.isSatisfied(simpleTextEvent)) + } + @Test fun test_notice_condition() { - val conditionEqual = EventMatchCondition("content.msgtype", "m.notice") + val conditionEqual = EventMatchCondition("content.msgtype", "m.notice", false) Event( type = "m.room.message", diff --git a/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt b/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt index 9799b32c38..0144b162e9 100644 --- a/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt +++ b/vector/src/main/java/im/vector/app/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 org.matrix.android.sdk.api.pushrules.getActions -import org.matrix.android.sdk.api.pushrules.rest.PushRule import im.vector.app.R import im.vector.app.core.epoxy.VectorEpoxyHolder import im.vector.app.features.notifications.toNotificationAction +import org.matrix.android.sdk.api.pushrules.getActions +import org.matrix.android.sdk.api.pushrules.rest.PushRule @EpoxyModelClass(layout = R.layout.item_pushrule_raw) abstract class PushRuleItem : EpoxyModelWithHolder() { @@ -68,7 +68,7 @@ abstract class PushRuleItem : EpoxyModelWithHolder() { val description = StringBuffer() pushRule.conditions?.forEachIndexed { i, condition -> if (i > 0) description.append("\n") - description.append(condition.asExecutableCondition()?.technicalDescription() + description.append(condition.asExecutableCondition(pushRule)?.technicalDescription() ?: "UNSUPPORTED") } if (description.isBlank()) {