Fix crash during deck filtering (#11880)

Fixes a `ConcurrentModificationException`: the wrong list was modified

* Create copy of list when filtering

* Optimize containsFilterString

* Add error catching

* Remove whitespace

* Add comment and better naming

* Rename usage of childrenCopy variable I forgot to rename

* Change to more efficient solution

* Remove unecessary comment

* Add unit tests for DeckFilter

* Remove unused imports

* Add copywrite

* Remove whitespace

* Format fixes

* Add last name to dictionary

* Flipped actual and expected in assert statement

* Removed try/catch block in publishResults()

* Remove unused import

* Added null check

* Revert "Optimize containsFilterString"

This reverts commit 5b617b0.

* Reverting to using !! operator on results

* Rename test method

.idea/dictionaries/usernames.xml

@@ -20,6 +20,7 @@
       <w>Freidle</w>
       <w>Goel</w>
       <w>Gruber</w>
+      <w>Guelman</w>
       <w>Houssam</w>
       <w>Jadhav</w>
       <w>Jolta</w>

AnkiDroid/src/main/java/com/ichi2/anki/widgets/DeckAdapter.kt

@@ -25,6 +25,7 @@ import android.view.View
 import android.view.View.OnLongClickListener
 import android.view.ViewGroup
 import android.widget.*
+import androidx.annotation.VisibleForTesting
 import androidx.core.content.ContextCompat
 import androidx.recyclerview.widget.RecyclerView
 import com.ichi2.anki.R
@@ -343,7 +344,8 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)
         return DeckFilter()
     }
 
-    private inner class DeckFilter : TypedFilter<TreeNode<AbstractDeckTreeNode>>(mDeckList) {
+    @VisibleForTesting
+    inner class DeckFilter(deckList: List<TreeNode<AbstractDeckTreeNode>> = mDeckList) : TypedFilter<TreeNode<AbstractDeckTreeNode>>(deckList) {
         override fun filterResults(constraint: CharSequence, items: List<TreeNode<AbstractDeckTreeNode>>): List<TreeNode<AbstractDeckTreeNode>> {
             val filterPattern = constraint.toString().lowercase(Locale.getDefault()).trim { it <= ' ' }
             return items.mapNotNull { t: TreeNode<AbstractDeckTreeNode> -> filterDeckInternal(filterPattern, t) }
@@ -375,7 +377,7 @@ class DeckAdapter(private val layoutInflater: LayoutInflater, context: Context)
 
             // we have a root, and a list of trees with the counts already calculated.
             return TreeNode(root.value).apply {
-                children.addAll(ret)
+                this.children.addAll(ret)
             }
         }
 

AnkiDroid/src/main/java/com/ichi2/utils/TypedFilter.kt

@@ -49,8 +49,7 @@ abstract class TypedFilter<T>(private val getCurrentItems: (() -> List<T>)) : Fi
 
     @Suppress("UNCHECKED_CAST")
     override fun publishResults(constraint: CharSequence?, results: FilterResults?) {
-        // this is only ever called from performFiltering so we can guarantee the value is non-null
-        // and can be cast to List<T>
+        // this is only ever called from performFiltering so we can guarantee the value can be cast to List<T>
         val list = results!!.values as List<T>
         publishResults(constraint, list)
     }

AnkiDroid/src/test/java/com/ichi2/anki/DeckAdapterFilterTest.kt

@@ -0,0 +1,92 @@
+/****************************************************************************************
+ * Copyright (c) 2022 Shai Guelman <[email protected]>                        *
+ *                                                                                      *
+ * This program is free software; you can redistribute it and/or modify it under        *
+ * the terms of the GNU General Public License as published by the Free Software        *
+ * Foundation; either version 3 of the License, or (at your option) any later           *
+ * version.                                                                             *
+ *                                                                                      *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY      *
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A      *
+ * PARTICULAR PURPOSE. See the GNU General Public License for more details.             *
+ *                                                                                      *
+ * You should have received a copy of the GNU General Public License along with         *
+ * this program.  If not, see <http://www.gnu.org/licenses/>.                           *
+ ****************************************************************************************/
+package com.ichi2.anki
+
+import com.ichi2.anki.widgets.DeckAdapter
+import com.ichi2.libanki.sched.AbstractDeckTreeNode
+import com.ichi2.libanki.sched.DeckTreeNode
+import com.ichi2.libanki.sched.TreeNode
+import org.junit.Assert
+import org.junit.Before
+import org.junit.Test
+import org.mockito.Mock
+import org.mockito.MockitoAnnotations
+
+class DeckAdapterFilterTest {
+
+    @Mock
+    private lateinit var adapter: DeckAdapter
+
+    private lateinit var filter: DeckAdapter.DeckFilter
+
+    @Before
+    fun setUp() {
+        MockitoAnnotations.openMocks(this)
+        filter = adapter.DeckFilter(deckList)
+    }
+
+    @Test
+    fun verifyFilterResultsReturnsCorrectList() {
+        val pattern = "Math"
+
+        val actual = filter.filterResults(pattern, deckList)
+        val expected = deckList.getByDids(0, 4, 5, 6, 8)
+
+        Assert.assertEquals(expected, actual)
+    }
+
+    @Test
+    fun verifyFilterResultsReturnsEmptyForNoMatches() {
+        val deckList = deckList
+        val pattern = "geometry"
+
+        val actual = filter.filterResults(pattern, deckList)
+
+        Assert.assertTrue(actual.isEmpty())
+    }
+
+    private val deckList: MutableList<TreeNode<AbstractDeckTreeNode>>
+        get() {
+            val deckList: MutableList<TreeNode<AbstractDeckTreeNode>> = mutableListOf(
+                TreeNode(DeckTreeNode("Chanson", 0)),
+                TreeNode(DeckTreeNode("Chanson::A Vers", 1)),
+                TreeNode(DeckTreeNode("Chanson::A Vers::1", 2)),
+                TreeNode(DeckTreeNode("Chanson::A Vers::Other", 3)),
+                TreeNode(DeckTreeNode("Chanson::Math HW", 4)),
+                TreeNode(DeckTreeNode("Chanson::Math HW::Theory", 5)),
+                TreeNode(DeckTreeNode("Chanson::Important", 6)),
+                TreeNode(DeckTreeNode("Chanson::Important::Stuff", 7)),
+                TreeNode(DeckTreeNode("Chanson::Important::Math", 8)),
+                TreeNode(DeckTreeNode("Chanson::Important::Stuff::Other Stuff", 9)),
+            )
+
+            deckList.getByDid(0).children.addAll(deckList.getByDids(1, 4, 6))
+            deckList.getByDid(1).children.addAll(deckList.getByDids(2, 3))
+            deckList.getByDid(4).children.addAll(deckList.getByDids(5))
+            deckList.getByDid(6).children.addAll(deckList.getByDids(7, 8))
+            deckList.getByDid(7).children.addAll(deckList.getByDids(9))
+
+            return deckList
+        }
+
+    private fun List<TreeNode<AbstractDeckTreeNode>>.getByDid(did: Long): TreeNode<AbstractDeckTreeNode> {
+        return this.first { it.value.did == did }
+    }
+
+    private fun List<TreeNode<AbstractDeckTreeNode>>.getByDids(vararg dids: Long): List<TreeNode<AbstractDeckTreeNode>> {
+        return this.filter { it.value.did in dids }
+    }
+}