diff --git a/changelog.d/6457.bugfix b/changelog.d/6457.bugfix new file mode 100644 index 0000000000..89ba075378 --- /dev/null +++ b/changelog.d/6457.bugfix @@ -0,0 +1 @@ +Fix that replies to @roomba would be highlighted as a room ping. Contributed by Nico. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index 8875807b8a..15d5cd3153 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -17,8 +17,6 @@ package org.matrix.android.sdk.api.session.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,18 +29,14 @@ 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, - /** - * true to match only words. In this case pattern will not be considered as a glob - */ - val wordsOnly: Boolean + val pattern: String ) : Condition { override fun isSatisfied(event: Event, conditionResolver: ConditionResolver): Boolean { return conditionResolver.resolveEventMatchCondition(event, this) } - override fun technicalDescription() = "'$key' matches '$pattern', words only '$wordsOnly'" + override fun technicalDescription() = "'$key' matches '$pattern'" fun isSatisfied(event: Event): Boolean { // TODO encrypted events? @@ -50,21 +44,28 @@ class EventMatchCondition( ?: return false val value = extractField(rawJson, key) ?: return false - // Patterns with no special glob characters should be treated as having asterisks prepended - // and appended when testing the condition. + // The match is performed case-insensitively, and must match the entire value of + // the event field given by `key` (though see below regarding `content.body`). The + // exact meaning of "case insensitive" is defined by the implementation of the + // homeserver. + // + // As a special case, if `key` is `content.body`, then `pattern` must instead + // match any substring of the value of the property which starts and ends at a + // word boundary. return try { - if (wordsOnly) { - value.caseInsensitiveFind(pattern) - } else { - val modPattern = if (pattern.hasSpecialGlobChar()) { + if (key == "content.body") { + val modPattern = if (pattern.startsWith("*") && pattern.endsWith("*")) { // Regex.containsMatchIn() is way faster without leading and trailing // stars, that don't make any difference for the evaluation result pattern.removePrefix("*").removeSuffix("*").simpleGlobToRegExp() } else { - pattern.simpleGlobToRegExp() + "(\\W|^)" + pattern.simpleGlobToRegExp() + "(\\W|$)" } - val regex = Regex(modPattern, RegexOption.DOT_MATCHES_ALL) + val regex = Regex(modPattern, setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) regex.containsMatchIn(value) + } else { + val regex = Regex(pattern.simpleGlobToRegExp(), setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) + regex.matches(value) } } catch (e: Throwable) { // e.g PatternSyntaxException diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt index ec0936e4c8..1b53801d0b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt @@ -22,7 +22,6 @@ import org.matrix.android.sdk.api.session.pushrules.ContainsDisplayNameCondition import org.matrix.android.sdk.api.session.pushrules.EventMatchCondition import org.matrix.android.sdk.api.session.pushrules.Kind import org.matrix.android.sdk.api.session.pushrules.RoomMemberCountCondition -import org.matrix.android.sdk.api.session.pushrules.RuleIds import org.matrix.android.sdk.api.session.pushrules.SenderNotificationPermissionCondition import timber.log.Timber @@ -59,11 +58,11 @@ data class PushCondition( val iz: String? = null ) { - fun asExecutableCondition(rule: PushRule): Condition? { + fun asExecutableCondition(): Condition? { return when (Kind.fromString(kind)) { Kind.EventMatch -> { if (key != null && pattern != null) { - EventMatchCondition(key, pattern, rule.ruleId == RuleIds.RULE_ID_CONTAIN_USER_NAME) + EventMatchCondition(key, pattern) } else { Timber.e("Malformed Event match condition") null diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt index b9d06a934d..15e803bb91 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt @@ -28,7 +28,7 @@ internal class PushRuleFinder @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(rule)?.isSatisfied(event, conditionResolver) ?: false + it.asExecutableCondition()?.isSatisfied(event, conditionResolver) ?: false } ?: false } } diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt index c4a3404e80..3ddf940241 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt @@ -49,7 +49,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_type_condition() { - val condition = EventMatchCondition("type", "m.room.message", false) + val condition = EventMatchCondition("type", "m.room.message") val simpleTextEvent = createSimpleTextEvent("Yo wtf?") @@ -67,12 +67,12 @@ class PushRulesConditionTest : MatrixTest { ) assert(condition.isSatisfied(simpleTextEvent)) - assert(!condition.isSatisfied(simpleRoomMemberEvent)) + assertFalse(condition.isSatisfied(simpleRoomMemberEvent)) } @Test fun test_eventmatch_path_condition() { - val condition = EventMatchCondition("content.msgtype", "m.text", false) + val condition = EventMatchCondition("content.msgtype", "m.text") val simpleTextEvent = createSimpleTextEvent("Yo wtf?") @@ -89,28 +89,29 @@ class PushRulesConditionTest : MatrixTest { ).toContent(), originServerTs = 0 ).apply { - assert(EventMatchCondition("content.membership", "invite", false).isSatisfied(this)) + assert(EventMatchCondition("content.membership", "invite").isSatisfied(this)) } } @Test fun test_eventmatch_cake_condition() { - val condition = EventMatchCondition("content.body", "cake", false) + val condition = EventMatchCondition("content.body", "cake") assert(condition.isSatisfied(createSimpleTextEvent("How was the cake?"))) - assert(condition.isSatisfied(createSimpleTextEvent("Howwasthecake?"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("Howwasthecake?"))) } @Test fun test_eventmatch_cakelie_condition() { - val condition = EventMatchCondition("content.body", "cake*lie", false) + val condition = EventMatchCondition("content.body", "cake*lie") assert(condition.isSatisfied(createSimpleTextEvent("How was the cakeisalie?"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("How was the notcakeisalie?"))) } @Test fun test_eventmatch_words_only_condition() { - val condition = EventMatchCondition("content.body", "ben", true) + val condition = EventMatchCondition("content.body", "ben") assertFalse(condition.isSatisfied(createSimpleTextEvent("benoit"))) assertFalse(condition.isSatisfied(createSimpleTextEvent("Hello benoit"))) @@ -124,9 +125,24 @@ class PushRulesConditionTest : MatrixTest { assert(condition.isSatisfied(createSimpleTextEvent("BEN"))) } + @Test + fun test_eventmatch_at_room_condition() { + val condition = EventMatchCondition("content.body", "@room") + + assertFalse(condition.isSatisfied(createSimpleTextEvent("@roomba"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("room benoit"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("abc@roomba"))) + + assert(condition.isSatisfied(createSimpleTextEvent("@room"))) + assert(condition.isSatisfied(createSimpleTextEvent("@room, ben"))) + assert(condition.isSatisfied(createSimpleTextEvent("@ROOM"))) + assert(condition.isSatisfied(createSimpleTextEvent("Use:@room"))) + assert(condition.isSatisfied(createSimpleTextEvent("Don't ping @room!"))) + } + @Test fun test_notice_condition() { - val conditionEqual = EventMatchCondition("content.msgtype", "m.notice", false) + val conditionEqual = EventMatchCondition("content.msgtype", "m.notice") 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 5a1dd055bd..57b2595590 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 @@ -73,7 +73,7 @@ abstract class PushRuleItem : VectorEpoxyModel(R.layout.ite pushRule.conditions?.forEachIndexed { i, condition -> if (i > 0) description.append("\n") description.append( - condition.asExecutableCondition(pushRule)?.technicalDescription() + condition.asExecutableCondition()?.technicalDescription() ?: "UNSUPPORTED" ) }