Redesign MessageChain.cleanupRubbishMessageElements() (#1607)

* Redesign `MessageChain.cleanupRubbishMessageElements()`

* Fix logic

* `CleanupRubbishMessageElementsTest`

* Fix testing unit

* more testing

mirai-core/src/commonMain/kotlin/message/ReceiveMessageHandler.kt

@@ -218,76 +218,82 @@ internal object ReceiveMessageTransformer {
             }
             index++
         }
+
+        // delete empty plain text
+        removeAll { it is PlainText && it.content.isEmpty() }
     }
 
     fun MessageChain.cleanupRubbishMessageElements(): MessageChain {
-        var previousLast: SingleMessage? = null
+        val builder = MessageChainBuilder(initialSize = count()).also {
-        var last: SingleMessage? = null
+            it.addAll(this)
-        return buildMessageChain(initialSize = this.count()) {
+        }
-            this@cleanupRubbishMessageElements.forEach { element ->
-                @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
-                if (last is LongMessageInternal && element is PlainText) {
-                    if (element == UNSUPPORTED_MERGED_MESSAGE_PLAIN) {
-                        previousLast = last
-                        last = element
-                        return@forEach
-                    }
-                }
-                if (last is PokeMessage && element is PlainText) {
-                    if (element == UNSUPPORTED_POKE_MESSAGE_PLAIN) {
-                        previousLast = last
-                        last = element
-                        return@forEach
-                    }
-                }
-                if (last is VipFace && element is PlainText) {
-                    val l = last as VipFace
-                    if (element.content.length == 4 + (l.count / 10) + l.kind.name.length) {
-                        previousLast = last
-                        last = element
-                        return@forEach
-                    }
-                }
-                // 解决tim发送的语音无法正常识别
-                if (element is PlainText) {
-                    if (element == UNSUPPORTED_VOICE_MESSAGE_PLAIN) {
-                        previousLast = last
-                        last = element
-                        return@forEach
-                    }
-                }
 
-                if (element is PlainText && last is At && previousLast is QuoteReply
+        kotlin.run moveQuoteReply@{ // Move QuoteReply after MessageSource
-                    && element.content.startsWith(' ')
+            val exceptedQuoteReplyIndex = builder.indexOfFirst { it is MessageSource } + 1
-                ) {
+            val quoteReplyIndex = builder.indexOfFirst { it is QuoteReply }
-                    // Android QQ 发送, 是 Quote+At+PlainText(" xxx") // 首空格
+            if (quoteReplyIndex < 1) return@moveQuoteReply
-                    removeLastOrNull() // At
+            if (quoteReplyIndex != exceptedQuoteReplyIndex) {
-                    val new = PlainText(element.content.substring(1))
+                val qr = builder[quoteReplyIndex]
-                    add(new)
+                builder.removeAt(quoteReplyIndex)
-                    previousLast = null
+                builder.add(exceptedQuoteReplyIndex, qr)
-                    last = new
+            }
-                    return@forEach
+        }
-                }
 
-                if (element is QuoteReply) {
+        kotlin.run quote@{
-                    // 客户端为兼容早期不支持 QuoteReply 的客户端而添加的 At
+            val quoteReplyIndex = builder.indexOfFirst { it is QuoteReply }
-                    removeLastOrNull()?.let { rm ->
+            if (quoteReplyIndex >= 0) {
-                        if ((rm as? PlainText)?.content != " ") add(rm)
+                // QuoteReply + At + PlainText(space 1)
-                        else removeLastOrNull()?.let { rm2 ->
+                if (quoteReplyIndex < builder.size - 1) {
-                            if (rm2 !is At) add(rm2)
+                    if (builder[quoteReplyIndex + 1] is At) {
+                        builder.removeAt(quoteReplyIndex + 1)
+                    }
+                    if (quoteReplyIndex < builder.size - 1) {
+                        val elm = builder[quoteReplyIndex + 1]
+                        if (elm is PlainText && elm.content.startsWith(' ')) {
+                            if (elm.content.length == 1) {
+                                builder.removeAt(quoteReplyIndex + 1)
+                            } else {
+                                builder[quoteReplyIndex + 1] = PlainText(elm.content.substring(1))
+                            }
                         }
                     }
+                    return@quote
                 }
-
-                append(element)
-
-                previousLast = last
-                last = element
             }
-
-            // 处理分片信息
-            compressContinuousPlainText()
         }
+
+        // TIM audios
+        if (builder.any { it is Audio }) {
+            builder.remove(UNSUPPORTED_VOICE_MESSAGE_PLAIN)
+        }
+
+        kotlin.run { // VipFace
+            val vipFaceIndex = builder.indexOfFirst { it is VipFace }
+            if (vipFaceIndex >= 0 && vipFaceIndex < builder.size - 1) {
+                val l = builder[vipFaceIndex] as VipFace
+                val text = builder[vipFaceIndex + 1]
+                if (text is PlainText) {
+                    if (text.content.length == 4 + (l.count / 10) + l.kind.name.length) {
+                        builder.removeAt(vipFaceIndex + 1)
+                    }
+                }
+            }
+        }
+
+        fun removeSuffixText(index: Int, text: PlainText) {
+            if (index >= 0 && index < builder.size - 1) {
+                if (builder[index + 1] == text) {
+                    builder.removeAt(index + 1)
+                }
+            }
+        }
+
+        removeSuffixText(builder.indexOfFirst { it is LongMessageInternal }, UNSUPPORTED_MERGED_MESSAGE_PLAIN)
+        removeSuffixText(builder.indexOfFirst { it is PokeMessage }, UNSUPPORTED_POKE_MESSAGE_PLAIN)
+
+        builder.compressContinuousPlainText()
+
+        return builder.asMessageChain()
     }
 
     private fun decodeText(text: ImMsgBody.Text, list: MessageChainBuilder) {

mirai-core/src/commonTest/kotlin/message/CleanupRubbishMessageElementsTest.kt

@@ -0,0 +1,156 @@
+/*
+ * Copyright 2019-2021 Mamoe Technologies and contributors.
+ *
+ * 此源代码的使用受 GNU AFFERO GENERAL PUBLIC LICENSE version 3 许可证的约束, 可以在以下链接找到该许可证.
+ * Use of this source code is governed by the GNU AGPLv3 license that can be found through the following link.
+ *
+ * https://github.com/mamoe/mirai/blob/dev/LICENSE
+ */
+
+package net.mamoe.mirai.internal.message
+
+import net.mamoe.mirai.internal.message.ReceiveMessageTransformer.cleanupRubbishMessageElements
+import net.mamoe.mirai.message.data.*
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.TestInstance
+import kotlin.test.assertEquals
+
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+internal class CleanupRubbishMessageElementsTest {
+    //region
+    private val replySource = OfflineMessageSourceImplData(
+        kind = MessageSourceKind.GROUP,
+        ids = intArrayOf(1),
+        botId = 1,
+        time = 1,
+        fromId = 87,
+        targetId = 7454,
+        originalMessage = messageChainOf(),
+        internalIds = intArrayOf(8711)
+    )
+
+    private val source = OfflineMessageSourceImplData(
+        kind = MessageSourceKind.GROUP,
+        ids = intArrayOf(1),
+        botId = 1,
+        time = 1,
+        fromId = 2,
+        targetId = 3,
+        originalMessage = messageChainOf(),
+        internalIds = intArrayOf(9)
+    )
+    //endregion
+
+    private fun assertCleanup(excepted: MessageChain, source: MessageChain) {
+        assertEquals(
+            excepted,
+            source.cleanupRubbishMessageElements()
+        )
+        assertEquals(
+            noMessageSource(excepted),
+            noMessageSource(source).cleanupRubbishMessageElements()
+        )
+    }
+
+    @Test
+    fun testQuoteAtSpace() {
+        // Windows PC QQ
+        assertCleanup(
+            messageChainOf(source, QuoteReply(replySource), PlainText("Hello!")),
+            messageChainOf(source, At(123), PlainText(" "), QuoteReply(replySource), PlainText("Hello!")),
+        )
+
+        // QQ Android
+        assertCleanup(
+            messageChainOf(source, QuoteReply(replySource), PlainText("Hello!")),
+            messageChainOf(source, QuoteReply(replySource), At(1234567890), PlainText(" Hello!")),
+        )
+    }
+
+    @Test
+    fun testTIMAudio() {
+        val audio = OnlineAudioImpl("0", byteArrayOf(), 0, AudioCodec.SILK, "", 0, null)
+        assertCleanup(
+            messageChainOf(source, audio),
+            messageChainOf(source, audio, UNSUPPORTED_VOICE_MESSAGE_PLAIN),
+        )
+    }
+
+    @Test
+    fun testPokeMessageCleanup() {
+        val poke = PokeMessage("", 1, 1)
+        assertCleanup(
+            messageChainOf(source, poke),
+            messageChainOf(source, poke, UNSUPPORTED_POKE_MESSAGE_PLAIN),
+        )
+    }
+
+    @Test
+    fun testVipFaceCleanup() {
+        val vf = VipFace(VipFace.Kind(1, "Test!"), 50)
+        assertCleanup(
+            messageChainOf(source, vf),
+            messageChainOf(source, vf, PlainText("----CCCCCTest!")),
+        )
+    }
+
+    @Test
+    fun testLongMessageInternalCleanup() {
+        val li = LongMessageInternal("", "")
+        assertCleanup(
+            messageChainOf(source, li),
+            messageChainOf(source, li, UNSUPPORTED_MERGED_MESSAGE_PLAIN),
+        )
+    }
+
+    @Test
+    fun testCompressContinuousPlainText() {
+        assertCleanup(
+            messageChainOf(PlainText("1234567890")),
+            "12 3   45 6  789 0".split(" ").map(::PlainText).toMessageChain(),
+        )
+        assertCleanup(
+            msg(source, At(123456), "Hello! How are you?"),
+            msg(source, At(123456), "Hello", "!", " ", "How", " ", "are ", "you?"),
+        )
+    }
+
+    @Test
+    fun testEmptyPlainTextRemoved() {
+        assertCleanup(
+            messageChainOf(),
+            "                     ".split(" ").map(::PlainText).toMessageChain(),
+        )
+        assertCleanup(
+            msg(AtAll),
+            msg("", AtAll, "", "", ""),
+        )
+    }
+
+    @Test
+    fun testBlankPlainTextLiving() {
+        assertCleanup(
+            msg("    "),
+            msg("", " ", "  ", " "),
+        )
+    }
+
+    //region
+
+    private fun msg(vararg msgs: Any?): MessageChain {
+        return msgs.map { elm ->
+            when (elm) {
+                is Message -> elm
+                is String -> PlainText(elm)
+                else -> PlainText(elm.toString())
+            }
+        }.toMessageChain()
+    }
+
+    private fun noMessageSource(c: MessageChain): MessageChain {
+        @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
+        return createMessageChainImplOptimized(c.filterNot { it is MessageSource })
+    }
+
+    //endregion
+}