Search code examples
androidandroid-support-libraryandroid-recyclerviewlistadapterandroidx

DiffUtil breaks contract for areContentTheSame [fix coming in next release]


recently I've found strange crashes in my app. I've found out that they are caused by ListAdapter -> DiffUtil underneath. Contract says that areContentsTheSame callback will be called only if areItemsTheSame returns true for corresponding items. Problem is areContentsTheSame is called for items that areItemsTheSame was never called.

I'm testing it on String items so it shouldn't be related to my own recycler implementation. I'm really confused if it's my fault (there is almost no logic now) or bug in DiffUtil tool

I've created simple Instrumented Test that is failing on mentioned case - could someone more experienced take a look at that :

package com.example.diffutilbug

import android.util.Log
import android.view.View
import android.view.ViewGroup
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter
import androidx.recyclerview.widget.RecyclerView
import junit.framework.Assert.assertTrue
import kotlinx.coroutines.*
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.BlockJUnit4ClassRunner

@RunWith(BlockJUnit4ClassRunner::class)
internal class ExampleUnitTest {

    @Test
    fun testDiffUtil4() {

        val handler = CoroutineExceptionHandler { _, exception ->
            throw exception
        }
        // adapter compare items :
        // areItemsTheSame -> compare length of String
        // areContentsTheSame -> compare content with ==
        val adapter = StringAdapterJunit(handler)

        runBlocking {
            adapter.submitList(
                mutableListOf<String>(
                    "1",//1,
                    "22",//2,
                    "333",//3,
                    "4444",//4,
                    "55555",//5,
                    "666666",//6,
                    "7777777",//7,
                    "88888888",//8,
                    "999999999",//9,
                    "55555",//5,
                    "1010101010",//10,
                    "1010109999",//10,
                    "55555",//5,
                    "1313131313",//10,
                    "1414141414",//10,
                    "55555",//5,
                    "1313131313",//10,
                    "1414141414",//10,
                    "55555"//5

                )
            )

            delay(40)

            adapter.submitList(
                mutableListOf<String>(
                    "55555",//5,
                    "1010101010",//10,
                    "1010109999",//10,
                    "55555",//11,
                    "1313131313",//10,
                    "1414141414",//10,
                    "11111111111"//11
                )
            )

            delay(500)
        }
    }

}

// Stub Adapter for Strings that uses DiffUtil underneath.
// logs all callbacks to logcat

class StringAdapterJunit(val handler: CoroutineExceptionHandler) : ListAdapter<String, RecyclerView.ViewHolder>(object : DiffUtil.ItemCallback<String>() {
    override fun areItemsTheSame(oldItem: String, newItem: String): Boolean {
        Log.e("DiffUtilTest", "areItemsTheSame comparing $oldItem with $newItem = ${oldItem.length == newItem.length}")
        return oldItem.length == newItem.length
    }

    override fun areContentsTheSame(oldItem: String, newItem: String): Boolean {
        //should be called only if areContentsTheSame == true
        Log.e(
            "DiffUtilTest",
            "areContentsTheSame error = ${oldItem.length != newItem.length} comparing $oldItem with $newItem"
        )

        runBlocking {
            GlobalScope.launch(handler + Dispatchers.Main) {
                assertTrue("areContentsTheSame can be called only if areItemsTheSame return true" , areItemsTheSame(oldItem, newItem))
            }.join()
        }
        return oldItem == newItem
    }

    override fun getChangePayload(oldItem: String, newItem: String): Any? {
    //should be called only if areItemsTheSame = true and areContentsTheSame = false

        Log.e(
            "DiffUtilTest",
            "getChangePayload error = ${oldItem.length == newItem.length && oldItem == newItem} $oldItem with $newItem"
        )
        return null
    }
}) {
    // stub implementation on adapter - never used
    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int) = object : RecyclerView.ViewHolder(View(null)) {}


    override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {}

    override fun getItemViewType(position: Int): Int = getItem(position).length
}

and gradle dependencies needed for it:

dependencies {
    implementation"org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
    testImplementation 'junit:junit:4.12'
    androidTestImplementation 'androidx.test:runner:1.1.1'
    androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1'
    androidTestImplementation 'androidx.test.ext:junit:1.1.0'

    //coroutines
    implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.0.1'
    implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.0.1'
    implementation 'androidx.recyclerview:recyclerview:1.0.0'
}

please note that you need to add

android.useAndroidX=true
android.enableJetifier=true

in your gradle.properties

coroutines and handler for exceptions added because DiffUtil computes diff on background thread and JUnit handles assertion only on main thread

=====================================================

Fix in next alpha : will be released in alpha 3 - PR to look after https://android-review.googlesource.com/c/platform/frameworks/support/+/1253271 thanks, can't wait to remove all workarounds!


Solution

  • I've get respond from google and they confirm that there is bug in DiffUtil when lists contain duplicated items (nulls, same objects etc.)

    My current workaround is to check "contract" by myself before execution so:

    override fun areItemsTheSame(oldItem: String, newItem: String): Boolean {
        return compare items
    }
    
    override fun areContentsTheSame(oldItem: String, newItem: String): Boolean {
        //should be called only if areItemsTheSame == true
        return areItemsTheSame(oldItem, newItem) && compare items contents
    
    }
    
    override fun getChangePayload(oldItem: String, newItem: String): Any? {
        //should be called only if areItemsTheSame = true and areContentsTheSame = false
        if(areItemsTheSame(oldItem, newItem) && !areContentsTheSame(oldItem, newItem)) {
            return compute changePayload
        } else {
             return null
        }
    }
    

    will update answer when issue gest resolved