Cleanup ObservableList implementation

app/src/main/java/com/topjohnwu/magisk/databinding/DiffObservableList.kt

@@ -5,23 +5,20 @@ import androidx.databinding.ListChangeRegistry
 import androidx.databinding.ObservableList
 import androidx.recyclerview.widget.DiffUtil
 import androidx.recyclerview.widget.ListUpdateCallback
-import java.util.*
+import java.util.AbstractList
 
-/**
- * @param callback    The callback that controls the behavior of the DiffObservableList.
- * @param detectMoves True if DiffUtil should try to detect moved items, false otherwise.
- */
 open class DiffObservableList<T>(
-    private val callback: Callback<T>,
-    private val detectMoves: Boolean = true
+    private val callback: Callback<T>
 ) : AbstractList<T>(), ObservableList<T> {
 
-    protected var list: MutableList<T> = ArrayList()
+    protected var list: List<T> = emptyList()
     private val listeners = ListChangeRegistry()
     protected val listCallback = ObservableListUpdateCallback()
 
     override val size: Int get() = list.size
 
+    override fun get(index: Int) = list[index]
+
     /**
      * Calculates the list of update operations that can convert this list into the given one.
      *
@@ -30,8 +27,7 @@ open class DiffObservableList<T>(
      * list into the given one.
      */
     fun calculateDiff(newItems: List<T>): DiffUtil.DiffResult {
-        val frozenList = ArrayList(list)
-        return doCalculateDiff(frozenList, newItems)
+        return doCalculateDiff(list, newItems)
     }
 
     protected fun doCalculateDiff(oldItems: List<T>, newItems: List<T>): DiffUtil.DiffResult {
@@ -51,7 +47,7 @@ open class DiffObservableList<T>(
                 val newItem = newItems[newItemPosition]
                 return callback.areContentsTheSame(oldItem, newItem)
             }
-        }, detectMoves)
+        }, true)
     }
 
     /**
@@ -63,26 +59,10 @@ open class DiffObservableList<T>(
      */
     @MainThread
     fun update(newItems: List<T>, diffResult: DiffUtil.DiffResult) {
-        list = newItems.toMutableList()
+        list = ArrayList(newItems)
         diffResult.dispatchUpdatesTo(listCallback)
     }
 
-    /**
-     * Sets this list to the given items. This is a convenience method for calling [ ][.calculateDiff] followed by [.update].
-     *
-     *
-     * **Warning!** If the lists are large this operation may be too slow for the main thread. In
-     * that case, you should call [.calculateDiff] on a background thread and then
-     * [.update] on the main thread.
-     *
-     * @param newItems The items to set this list to.
-     */
-    @MainThread
-    fun update(newItems: List<T>) {
-        val diffResult = doCalculateDiff(list, newItems)
-        update(newItems, diffResult)
-    }
-
     override fun addOnListChangedCallback(listener: ObservableList.OnListChangedCallback<out ObservableList<T>>) {
         listeners.add(listener)
     }
@@ -91,53 +71,6 @@ open class DiffObservableList<T>(
         listeners.remove(listener)
     }
 
-    override fun get(index: Int) = list[index]
-
-    override fun add(index: Int, element: T) {
-        list.add(index, element)
-        notifyAdd(index, 1)
-    }
-
-    override fun addAll(elements: Collection<T>) = addAll(size, elements)
-
-    override fun addAll(index: Int, elements: Collection<T>): Boolean {
-        val added = list.addAll(index, elements)
-        if (added) {
-            notifyAdd(index, elements.size)
-        }
-        return added
-    }
-
-    override fun clear() {
-        val oldSize = size
-        list.clear()
-        if (oldSize != 0) {
-            notifyRemove(0, oldSize)
-        }
-    }
-
-    override fun remove(element: T): Boolean {
-        val index = indexOf(element)
-        return if (index >= 0) {
-            removeAt(index)
-            true
-        } else {
-            false
-        }
-    }
-
-    override fun removeAt(index: Int): T {
-        val element = list.removeAt(index)
-        notifyRemove(index, 1)
-        return element
-    }
-
-    override fun set(index: Int, element: T): T {
-        val old = list.set(index, element)
-        listeners.notifyChanged(this, index, 1)
-        return old
-    }
-
     private fun notifyAdd(start: Int, count: Int) {
         listeners.notifyInserted(this, start, count)
     }
@@ -146,38 +79,8 @@ open class DiffObservableList<T>(
         listeners.notifyRemoved(this, start, count)
     }
 
-    /**
-     * A Callback class used by DiffUtil while calculating the diff between two lists.
-     */
     interface Callback<T> {
-        /**
-         * Called by the DiffUtil to decide whether two object represent the same Item.
-         *
-         *
-         * For example, if your items have unique ids, this method should check their id equality.
-         *
-         * @param oldItem The old item.
-         * @param newItem The new item.
-         * @return True if the two items represent the same object or false if they are different.
-         */
         fun areItemsTheSame(oldItem: T, newItem: T): Boolean
-
-        /**
-         * Called by the DiffUtil when it wants to check whether two items have the same data.
-         * DiffUtil uses this information to detect if the contents of an item has changed.
-         *
-         *
-         * DiffUtil uses this method to check equality instead of [Object.equals] so
-         * that you can change its behavior depending on your UI.
-         *
-         *
-         * This method is called only if [.areItemsTheSame] returns `true` for
-         * these items.
-         *
-         * @param oldItem The old item.
-         * @param newItem The new item which replaces the old item.
-         * @return True if the contents of the items are the same or false if they are different.
-         */
         fun areContentsTheSame(oldItem: T, newItem: T): Boolean
     }
 

app/src/main/java/com/topjohnwu/magisk/databinding/FilterableDiffObservableList.kt

@@ -5,14 +5,13 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
-import java.util.Collections
 
-class FilterableDiffObservableList<T>(
+open class FilterableDiffObservableList<T>(
     callback: Callback<T>,
     private val scope: CoroutineScope
 ) : DiffObservableList<T>(callback) {
 
-    private var sublist: MutableList<T> = list
+    private var sublist: List<T> = emptyList()
     private var job: Job? = null
 
     // ---
@@ -20,10 +19,11 @@ class FilterableDiffObservableList<T>(
     fun filter(filter: (T) -> Boolean) {
         job?.cancel()
         job = scope.launch(Dispatchers.Default) {
+            val oldList = sublist
             val newList = list.filter(filter)
-            val diff = synchronized(this) { doCalculateDiff(sublist, newList) }
+            val diff = doCalculateDiff(oldList, newList)
             withContext(Dispatchers.Main) {
-                sublist = Collections.synchronizedList(newList)
+                sublist = newList
                 diff.dispatchUpdatesTo(listCallback)
             }
         }
@@ -35,34 +35,6 @@ class FilterableDiffObservableList<T>(
         return sublist[index]
     }
 
-    override fun add(element: T): Boolean {
-        return sublist.add(element)
-    }
-
-    override fun add(index: Int, element: T) {
-        sublist.add(index, element)
-    }
-
-    override fun addAll(elements: Collection<T>): Boolean {
-        return sublist.addAll(elements)
-    }
-
-    override fun addAll(index: Int, elements: Collection<T>): Boolean {
-        return sublist.addAll(index, elements)
-    }
-
-    override fun remove(element: T): Boolean {
-        return sublist.remove(element)
-    }
-
-    override fun removeAt(index: Int): T {
-        return sublist.removeAt(index)
-    }
-
-    override fun set(index: Int, element: T): T {
-        return sublist.set(index, element)
-    }
-
     override val size: Int
         get() = sublist.size
 }

app/src/main/java/com/topjohnwu/magisk/databinding/Helpers.kt

@@ -1,9 +0,0 @@
-package com.topjohnwu.magisk.databinding
-
-import kotlinx.coroutines.CoroutineScope
-
-fun <T : AnyDiffRvItem> diffListOf() =
-    DiffObservableList(DiffRvItem.callback<T>())
-
-fun <T : AnyDiffRvItem> filterableListOf(scope: CoroutineScope) =
-    FilterableDiffObservableList(DiffRvItem.callback<T>(), scope)

app/src/main/java/com/topjohnwu/magisk/databinding/RecyclerViewItems.kt

@@ -3,6 +3,7 @@ package com.topjohnwu.magisk.databinding
 import androidx.databinding.PropertyChangeRegistry
 import androidx.databinding.ViewDataBinding
 import androidx.recyclerview.widget.RecyclerView
+import kotlinx.coroutines.CoroutineScope
 
 abstract class RvItem {
     abstract val layoutRes: Int
@@ -33,27 +34,6 @@ abstract class DiffRvItem<T> : RvItem() {
             is ComparableRv<*> -> comparableEqual(other)
             else -> this == other
         }
-
-    companion object {
-        private val callback = object : DiffObservableList.Callback<DiffRvItem<Any>> {
-            override fun areItemsTheSame(
-                oldItem: DiffRvItem<Any>,
-                newItem: DiffRvItem<Any>
-            ): Boolean {
-                return oldItem::class == newItem::class && oldItem.itemSameAs(newItem)
-            }
-
-            override fun areContentsTheSame(
-                oldItem: DiffRvItem<Any>,
-                newItem: DiffRvItem<Any>
-            ): Boolean {
-                return oldItem.contentSameAs(newItem)
-            }
-        }
-
-        @Suppress("UNCHECKED_CAST")
-        fun <T : AnyDiffRvItem> callback() = callback as DiffObservableList.Callback<T>
-    }
 }
 
 typealias AnyDiffRvItem = DiffRvItem<*>
@@ -65,3 +45,27 @@ abstract class ObservableDiffRvItem<T> : DiffRvItem<T>(), ObservableHost {
 abstract class ObservableRvItem : RvItem(), ObservableHost {
     override var callbacks: PropertyChangeRegistry? = null
 }
+
+private object DiffRvItemCallback : DiffObservableList.Callback<DiffRvItem<Any>> {
+    override fun areItemsTheSame(
+        oldItem: DiffRvItem<Any>,
+        newItem: DiffRvItem<Any>
+    ): Boolean {
+        return oldItem::class == newItem::class && oldItem.itemSameAs(newItem)
+    }
+
+    override fun areContentsTheSame(
+        oldItem: DiffRvItem<Any>,
+        newItem: DiffRvItem<Any>
+    ): Boolean {
+        return oldItem.contentSameAs(newItem)
+    }
+}
+
+@Suppress("UNCHECKED_CAST")
+class DiffRvItemList<T: AnyDiffRvItem> : DiffObservableList<T>(DiffRvItemCallback as Callback<T>)
+
+@Suppress("UNCHECKED_CAST")
+class DiffRvItemFilterList<T: AnyDiffRvItem>(
+    scope: CoroutineScope
+) : FilterableDiffObservableList<T>(DiffRvItemCallback as Callback<T>, scope)

app/src/main/java/com/topjohnwu/magisk/ui/deny/DenyListViewModel.kt

@@ -7,8 +7,8 @@ import androidx.lifecycle.viewModelScope
 import com.topjohnwu.magisk.BR
 import com.topjohnwu.magisk.arch.AsyncLoadViewModel
 import com.topjohnwu.magisk.core.di.AppContext
+import com.topjohnwu.magisk.databinding.DiffRvItemFilterList
 import com.topjohnwu.magisk.databinding.bindExtra
-import com.topjohnwu.magisk.databinding.filterableListOf
 import com.topjohnwu.magisk.databinding.set
 import com.topjohnwu.magisk.ktx.concurrentMap
 import com.topjohnwu.superuser.Shell
@@ -38,7 +38,7 @@ class DenyListViewModel : AsyncLoadViewModel() {
             query()
         }
 
-    val items = filterableListOf<DenyListRvItem>(viewModelScope)
+    val items = DiffRvItemFilterList<DenyListRvItem>(viewModelScope)
     val extraBindings = bindExtra {
         it.put(BR.viewModel, this)
     }

app/src/main/java/com/topjohnwu/magisk/ui/flash/FlashViewModel.kt

@@ -15,7 +15,7 @@ import com.topjohnwu.magisk.core.tasks.FlashZip
 import com.topjohnwu.magisk.core.tasks.MagiskInstaller
 import com.topjohnwu.magisk.core.utils.MediaStoreUtils
 import com.topjohnwu.magisk.core.utils.MediaStoreUtils.outputStream
-import com.topjohnwu.magisk.databinding.diffListOf
+import com.topjohnwu.magisk.databinding.DiffRvItemList
 import com.topjohnwu.magisk.databinding.set
 import com.topjohnwu.magisk.events.SnackbarEvent
 import com.topjohnwu.magisk.ktx.reboot
@@ -40,7 +40,7 @@ class FlashViewModel : BaseViewModel() {
     var showReboot = Info.isRooted
         set(value) = set(value, field, { field = it }, BR.showReboot)
 
-    val items = diffListOf<ConsoleItem>()
+    val items = DiffRvItemList<ConsoleItem>()
     lateinit var args: FlashFragmentArgs
 
     private val logItems = mutableListOf<String>().synchronized()

app/src/main/java/com/topjohnwu/magisk/ui/log/LogViewModel.kt

@@ -11,8 +11,8 @@ import com.topjohnwu.magisk.core.Info
 import com.topjohnwu.magisk.core.repository.LogRepository
 import com.topjohnwu.magisk.core.utils.MediaStoreUtils
 import com.topjohnwu.magisk.core.utils.MediaStoreUtils.outputStream
+import com.topjohnwu.magisk.databinding.DiffRvItemList
 import com.topjohnwu.magisk.databinding.bindExtra
-import com.topjohnwu.magisk.databinding.diffListOf
 import com.topjohnwu.magisk.databinding.set
 import com.topjohnwu.magisk.events.SnackbarEvent
 import com.topjohnwu.magisk.ktx.timeFormatStandard
@@ -34,7 +34,7 @@ class LogViewModel(
 
     // --- su log
 
-    val items = diffListOf<LogRvItem>()
+    val items = DiffRvItemList<LogRvItem>()
     val extraBindings = bindExtra {
         it.put(BR.viewModel, this)
     }

app/src/main/java/com/topjohnwu/magisk/ui/module/ModuleViewModel.kt

@@ -10,10 +10,10 @@ import com.topjohnwu.magisk.core.Info
 import com.topjohnwu.magisk.core.base.ContentResultCallback
 import com.topjohnwu.magisk.core.model.module.LocalModule
 import com.topjohnwu.magisk.core.model.module.OnlineModule
+import com.topjohnwu.magisk.databinding.DiffRvItemList
 import com.topjohnwu.magisk.databinding.MergeObservableList
 import com.topjohnwu.magisk.databinding.RvItem
 import com.topjohnwu.magisk.databinding.bindExtra
-import com.topjohnwu.magisk.databinding.diffListOf
 import com.topjohnwu.magisk.databinding.set
 import com.topjohnwu.magisk.events.GetContentEvent
 import com.topjohnwu.magisk.events.SnackbarEvent
@@ -26,7 +26,7 @@ class ModuleViewModel : AsyncLoadViewModel() {
 
     val bottomBarBarrierIds = intArrayOf(R.id.module_update, R.id.module_remove)
 
-    private val itemsInstalled = diffListOf<LocalModuleRvItem>()
+    private val itemsInstalled = DiffRvItemList<LocalModuleRvItem>()
 
     val items = MergeObservableList<RvItem>()
     val extraBindings = bindExtra {

app/src/main/java/com/topjohnwu/magisk/ui/superuser/SuperuserViewModel.kt

@@ -34,7 +34,7 @@ class SuperuserViewModel(
     private val itemNoData = TextItem(R.string.superuser_policy_none)
 
     private val itemsHelpers = ObservableArrayList<TextItem>()
-    private val itemsPolicies = diffListOf<PolicyRvItem>()
+    private val itemsPolicies = DiffRvItemList<PolicyRvItem>()
 
     val items = MergeObservableList<AnyDiffRvItem>()
         .insertList(itemsHelpers)