Fixed
Status Update
Comments
fi...@gmail.com <fi...@gmail.com> #2
Project: platform/frameworks/support
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
https://android-review.googlesource.com/1123258
https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
cy...@zen.ly <cy...@zen.ly> #3
fi...@gmail.com <fi...@gmail.com> #4
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically ( b/140759491 ).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
https://android-review.googlesource.com/1288456
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
fi...@gmail.com <fi...@gmail.com> #5
any news?
yb...@google.com <yb...@google.com>
ap...@google.com <ap...@google.com> #8
Project: platform/frameworks/support
Branch: androidx-master-dev
commit a013a90ce233809bb659f5d978d6250826f35c0c
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Mar 11 15:11:38 2020
Benchmarks for diff util
This CL adds benchmarks for DiffUtil which is good to have
before we make changes to handle duplicate items properly
Bug: 123376278
Test: DiffBenchmark
Change-Id: I4b23c75caa47aaeddf8262cc66154f69589db823
A recyclerview/recyclerview-benchmark/src/androidTest/java/androidx/recyclerview/benchmark/DiffBenchmark.kt
https://android-review.googlesource.com/1255508
Branch: androidx-master-dev
commit a013a90ce233809bb659f5d978d6250826f35c0c
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Mar 11 15:11:38 2020
Benchmarks for diff util
This CL adds benchmarks for DiffUtil which is good to have
before we make changes to handle duplicate items properly
Bug: 123376278
Test: DiffBenchmark
Change-Id: I4b23c75caa47aaeddf8262cc66154f69589db823
A recyclerview/recyclerview-benchmark/src/androidTest/java/androidx/recyclerview/benchmark/DiffBenchmark.kt
fi...@gmail.com <fi...@gmail.com> #9
thanks. Which release will be first with this fix included?
yb...@google.com <yb...@google.com> #10
sorry it missed alpha 2 but should be out in alpha 3 of recyclerview.
here is the CL to follow:
once it is merged you can use a build from CI but keep in mind that it has latest changes in RV.
ap...@google.com <ap...@google.com> #11
Project: platform/frameworks/support
Branch: androidx-master-dev
commit adebffff1a528c659f635280902993d5a30ecfc5
Author: Yigit Boyar <yboyar@google.com>
Date: Mon Mar 09 16:24:52 2020
Fix duplicate items handling in diff util
This CL fixes a bug in DiffUtil where it would get confused when
and item in the original list is duplicated, by shortcircuting the
path between the duplicate instances.
This CL fixes that bug by "unfortunately" re-implementing the
snake finding and path finding logic. I just couldn't figure out
what was wrong in the previous implementation and it was just too
hard to read with a billion offsets etc everywhere.
This implementation is easier to read, more true to the algorithm
in the paper at the cost of some more function calls.
On a pixel 3 (where allocations matter less), it is consistently
faster ~10-20% except when large item chunks move where it is
5% slower.
see measurements:https://paste.googleplex.com/6430892597706752
Bug: 123376278
Test: DiffUtilTest with duplicate cases
Change-Id: I94f64db7162c0107f9af512cdd55c0d39b44cabf
M leanback/leanback/src/androidTest/java/androidx/leanback/widget/ObjectAdapterTest.java
M paging/runtime/src/androidTest/java/androidx/paging/NullPaddedListDiffHelperTest.kt
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/DiffUtil.java
M recyclerview/recyclerview/src/test/java/androidx/recyclerview/widget/DiffUtilTest.kt
https://android-review.googlesource.com/1253271
Branch: androidx-master-dev
commit adebffff1a528c659f635280902993d5a30ecfc5
Author: Yigit Boyar <yboyar@google.com>
Date: Mon Mar 09 16:24:52 2020
Fix duplicate items handling in diff util
This CL fixes a bug in DiffUtil where it would get confused when
and item in the original list is duplicated, by shortcircuting the
path between the duplicate instances.
This CL fixes that bug by "unfortunately" re-implementing the
snake finding and path finding logic. I just couldn't figure out
what was wrong in the previous implementation and it was just too
hard to read with a billion offsets etc everywhere.
This implementation is easier to read, more true to the algorithm
in the paper at the cost of some more function calls.
On a pixel 3 (where allocations matter less), it is consistently
faster ~10-20% except when large item chunks move where it is
5% slower.
see measurements:
Bug: 123376278
Test: DiffUtilTest with duplicate cases
Change-Id: I94f64db7162c0107f9af512cdd55c0d39b44cabf
M leanback/leanback/src/androidTest/java/androidx/leanback/widget/ObjectAdapterTest.java
M paging/runtime/src/androidTest/java/androidx/paging/NullPaddedListDiffHelperTest.kt
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/DiffUtil.java
M recyclerview/recyclerview/src/test/java/androidx/recyclerview/widget/DiffUtilTest.kt
yb...@google.com <yb...@google.com>
fi...@gmail.com <fi...@gmail.com> #12
thanks!
sa...@gmail.com <sa...@gmail.com> #13
Was this released? If yes in which version? Thank you
yb...@google.com <yb...@google.com> #14
unfortunately it is still in 1.2 alpha due to some dependencies but you can use the alpha of RecyclerView, it is fairly stable.
Description
I'm testing it on String items so it shouldn't be related to my own recycler implementation.
I've created simple Instrumented Test that is failing on mentioned case:
#####################################
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