Fixed
Status Update
Comments
jb...@google.com <jb...@google.com>
il...@google.com <il...@google.com>
mg...@google.com <mg...@google.com>
ap...@google.com <ap...@google.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
Description
Version used:
2.8.0-alpha01
In the latest alpha version, there's now the ability to retrieve
Closeable
objects previously added to aViewModel
, which is great.I did notice though that the (internal)
setTagIfAbsent
method has been removed andViewModel.viewModelScope
(which previously usedsetTagIfAbsent
) now usesgetCloseable
andaddCloseable
separately.I'm thinking this might be problematic as the operation appears to no longer be atomic? E.g. the
viewModelScope
may be saved and then a new one saved again, which will overwrite the first one in theMap
and hence whatever job(s) it had launched would no longer be cancelled with theViewModel
.I assume that the issue of concurrency is important here given the
synchronized
blocks, and that other scopes seem to have pretty stringent measures (e.g.Lifecycle.coroutineScope
). MaybeCollections.synchronizedMap
andCollections.synchronizedSet
could also be helpful?Aside from concurrency issues, the new
addCloseable
method doesn't actually take into consideration whether aCloseable
has already been added with the same key, so there is plenty of potential for accidental overwrites, which would likely cause leaks when the the original overwrittenCloseable
disappears from theMap
.Regardless of the above, it would be nice to have something along the lines of
putIfAbsent
and/orcomputeIfAbsent
in the publicViewModel
API as part of this new feature, alongside the standardaddCloseable
andgetCloseable
.