Implement the current spec for event match conditions

changelog.d/6457.bugfix

@@ -0,0 +1 @@
+Fix that replies to @roomba would be highlighted as a room ping. Contributed by Nico.

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,40 +29,36 @@ 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?
         val rawJson = MoshiProvider.providesMoshi().adapter(Event::class.java).toJsonValue(event) as? Map<*, *>
                 ?: 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()) {
-                    // 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()
-                }
-                val regex = Regex(modPattern, RegexOption.DOT_MATCHES_ALL)
+            if (key == "content.body") {
+                val regex = Regex("(\\W|^)" + pattern.simpleGlobToRegExp() + "(\\W|$)", 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

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

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
         }
     }

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",

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

@@ -73,7 +73,7 @@ abstract class PushRuleItem : VectorEpoxyModel<PushRuleItem.Holder>(R.layout.ite
             pushRule.conditions?.forEachIndexed { i, condition ->
                 if (i > 0) description.append("\n")
                 description.append(
-                        condition.asExecutableCondition(pushRule)?.technicalDescription()
+                        condition.asExecutableCondition()?.technicalDescription()
                                 ?: "UNSUPPORTED"
                 )
             }