(FR) RX improvements [63317956]

Fixed

Bug

Status Update

No update yet.

Description

ji...@gmail.com

created issue #1

Jul 5, 2017 12:04PM

Component used: Room with RxRoom
Version used: 1.0.0alpha3
Devices/Android versions reproduced on: all

After using Room and RxRoom I was lacking of some functionalities like the ability to return an Single/Completable from insert queries.

For example something like this would be super helpful :
@Insert(onConflict = OnConflictStrategy.REPLACE)
Completable insertAll(CatalogEntry... entries);

@Insert(onConflict = OnConflictStrategy.REPLACE)
Single<Long> insertAll(CatalogEntry... entries);

@Insert(onConflict = OnConflictStrategy.REPLACE)
Single<List<Long>> insertAll(CatalogEntry... entries);

Also when doing a query for retrieving a list, why should I use Flowable, if I just want to get a list my query should be like:

@Query("SELECT * FROM catalog_entries")
Single<List<CatalogEntry>> getAll();

Thanks for your work ! I love it already :)

Comments

de...@gmail.com <de...@gmail.com> #2Jul 6, 2017 02:33AM

I haven't looks on repro yet, but there is no such thing as AlwaysOnLifecycle in latest versions, LiveData has strong references on all observers (including that were added as observeForever). If nothings keeps a livedata, then it is gced.

yb...@google.com <yb...@google.com> #3Jul 17, 2017 04:06PM

Assigned to yb...@google.com.

ah I see: in observe with Lifecycle we add LiveData as observer to Lifecycle and it keeps it alive. However keeping observerforever + livedata, when nothing has reference on them, is a memory leak by definition: user can't clean this memory anyhow, because neither observer / livedata could be referenced.

ji...@gmail.com <ji...@gmail.com> #4Jul 17, 2017 07:19PM

It is true, though from usage perspective, it is not obvious at all.

e.g. you are in a class, like a singleton repository where you want to observe the database for some query.
and you write:

MyRepository(dao : MyDao) {
dao.someLiveData().observeForever(....do something...);
}

And it suddenly stops working :/ which is very non-obvious from the developer's perspective.

If we kept the lifecycle in memory as long as it has observers, wouldn't it solve the problem since they can break the chain by calling stopObserving(observer) ?
When they use observeForever, they are already responsible to remove the observer so I think it is fine?

ra...@gmail.com <ra...@gmail.com> #5Aug 30, 2017 09:41PM

well, the issue is: no reference on observer, you can't break the chain because you don't have a reference. If you have a reference, LiveData (unfortunately) wouldn't be gc-ed, because even AlwaysActiveObserver inrenally has strong reference on livedata.

ra...@gmail.com <ra...@gmail.com> #6Jan 11, 2018 02:47PM

It turns out I had an app with the exact above use case: a singleton repository observing a database. The code was working properly with arch components 1.0.0 but unpredictably with 1.1.0. I think this is a good example of a leaky abstraction.
Since I want to observe the database forever with no intention to remove the observer at any point, I had no reason to keep a reference to the LiveData and it ended up being GCed along with the observers in it.
You could also argue that the problem is not with LiveData but with Room using weak references to observers but the developer is not supposed to know the Room implementation details.

la...@x2mobile.net <la...@x2mobile.net> #7Mar 26, 2018 01:18PM

I think the real problem is that LiveData has no callback to know that no one wants it anymore; like onReset in Loader.

So there's 2 options for database observers:
1. It either has to un-register observer in onInactive and on next onActive re-query db. Problematic is if nothing has changed you have to filter values or dispatch unnecessarily, and you'll pay the price of query which could be expensive.
2. Register weak observer, or have wear ref to LiveData

Room uses option 2. I created my own LiveData and I used the same (

https://bitbucket.org/snippets/Boza-s6/xe8749/sample-livedata-with-features-like-loaders#EmailContentLiveData.java-104), because it was obvious solution, but now I see it will not work in every possible case.

I think there should be statically referenced Lifecycle object to which LiveData subscribes when using observeForever() (and observer is anonymous class, maybe?).

je...@gmail.com <je...@gmail.com> #8Mar 27, 2018 12:51PM

I ran into the same problem yesterday. It tooked me hours to understand what is happening and why my observeForever-observer suddenly stopped being invoked.
I think it is a really bad design that someone should know the implementation details of generated Room DB Code.

I want to use observerForever the same way like i use the simple observe and dont want to ran into problems because of internal implementation details.

fr...@gmail.com <fr...@gmail.com> #9Apr 24, 2018 05:48AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bab31cacd4ae98057b694f6ae4c781f5f25ee0c9
Author: Yigit Boyar <yboyar@google.com>
Date: Wed Oct 31 23:41:00 2018

Fix live data observe forever in room

This CL fixes a problem where if DAO returned LiveData is
observed with observeForever but there are no references
to the LiveData, it would eventually be garbage collected.

To avoid this, InvalidationTracker now manages these LiveData
objects and strongly references them as long as there is an
active observer.
To be 100%, we should actually do it when LiveData has observers
but right now that API does not exist in the LiveData side
to get notified when the observer count changes 0 and 1.

I've also cleaned up some test code to avoid duplicating the
test observers in each test.

I've also added an integration test that uses room w/o any of
the optional dependencies to ensure that we don't break the
contract on compileOnly dependencies.

Bug: 74477406
Test: LiveDataQueryTest, InvalidationTrackerTest, NoAppCompatTestApp
Change-Id: If88114306d7e20ddaed0312a1203a0d14b710145

M arch/core-testing/src/main/java/androidx/arch/core/executor/TaskExecutorWithFakeMainThread.java
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/LiveDataQueryResultBinder.kt
M room/compiler/src/test/data/daoWriter/output/ComplexDao.java
A room/integration-tests/noappcompattestapp/.gitignore
A room/integration-tests/noappcompattestapp/README.md
A room/integration-tests/noappcompattestapp/build.gradle
A room/integration-tests/noappcompattestapp/src/androidTest/java/androidx/room/integration/noappcompat/BareDatabaseTest.java
A room/integration-tests/noappcompattestapp/src/main/AndroidManifest.xml
A room/integration-tests/noappcompattestapp/src/main/java/androidx/room/integration/noappcompat/BareDatabase.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/paging/DataSourceFactoryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/InvalidationTrackerTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/LiveDataQueryTest.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/QueryTransactionTest.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestLifecycleOwner.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestObserver.java
M room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/TestUtil.java
M room/runtime/build.gradle
A room/runtime/src/main/java/androidx/room/InvalidationLiveDataContainer.java
M room/runtime/src/main/java/androidx/room/InvalidationTracker.java
A room/runtime/src/main/java/androidx/room/RoomTrackingLiveData.java
A room/runtime/src/test/java/androidx/room/InvalidationLiveDataContainerTest.kt
M room/runtime/src/test/java/androidx/room/InvalidationTrackerTest.java
M settings.gradle

https://android-review.googlesource.com/810053

https://goto.google.com/android-sha1/bab31cacd4ae98057b694f6ae4c781f5f25ee0c9

sh...@gmail.com <sh...@gmail.com> #10Apr 24, 2018 05:03PM

FYI this requires client to update to Room 2.1.0-alpha03 but it is fully compatible with 1.x that WM uses.

do...@gmail.com <do...@gmail.com> #11May 22, 2018 12:10PM

actually it won't work because the change is in generated code. I'll try to backport itinto WM.

wh...@gmail.com <wh...@gmail.com> #12Jun 5, 2018 11:24AM

re-opening for WM part.

ej...@gmail.com <ej...@gmail.com> #13Jun 10, 2018 10:41PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 3fb46bd8602faaf9c34aa969f8ae589c251c2f72
Author: Yigit Boyar <yboyar@google.com>
Date: Fri Nov 30 13:19:34 2018

Track LiveData returned by the WM

This CL works around a bug in Room 1.x where LiveData returned
by Room might be garbage collected.

Now work manager tracks any live data it returns as long as the
LiveData is active.

Bug: 74477406
Test: ObserForeverTest, WorkManagerLiveDataTrackerTest
Change-Id: I1e45dade1e8a3c29d6c35f7cb0c6e072c68ee658

A work/workmanager/src/androidTest/java/androidx/work/impl/ObserveForeverTest.java
M work/workmanager/src/main/java/androidx/work/impl/WorkManagerImpl.java
A work/workmanager/src/main/java/androidx/work/impl/WorkManagerLiveDataTracker.java
A work/workmanager/src/test/java/androidx/work/impl/WorkManagerLiveDataTrackerTest.java

https://android-review.googlesource.com/838833

https://goto.google.com/android-sha1/3fb46bd8602faaf9c34aa969f8ae589c251c2f72

vs...@gmail.com <vs...@gmail.com> #14Jun 11, 2018 11:23AM

+1 for completable for @Delete @Update and @Insert

lo...@gmail.com <lo...@gmail.com> #15Jun 14, 2018 10:34AM

+1 for Completable support as well

56...@gmail.com <56...@gmail.com> #16Jun 14, 2018 10:43AM

Please Give a star instead of adding new comments.

ju...@gmail.com <ju...@gmail.com> #17Jun 28, 2018 12:20PM

This would be a nice improvement!

My initial response is to wrap the insert in a Completable when exposing the call to a consumer because of the time spent writing to the file system on the main thread. Guessing would probably desire the same API for delete.

Out of the box without LiveData/RxJava support for those types of calls it would violate strict rules and run into the guards of Room ensures that database is never accessed on the main thread.

fo...@gmail.com <fo...@gmail.com> #18Aug 14, 2018 08:38AM

It's really necessary thing and it's not implemented still(

sh...@gmail.com <sh...@gmail.com> #19Aug 14, 2018 12:22PM

+1 Come on we need support for completable. its been more than an year :(

yb...@google.com <yb...@google.com> #20Aug 14, 2018 05:34PM

Reassigned to fl...@google.com.

we are targeting this for 2.1.

bo...@li.ru <bo...@li.ru> #21Aug 17, 2018 06:02AM

+1 for completable for @Delete @Update and @Insert

ai...@gmail.com <ai...@gmail.com> #22Aug 20, 2018 09:54PM

+1 Come on guys!! ^_^

ap...@google.com <ap...@google.com> #23Aug 22, 2018 10:22PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c0c36b71b1e73eeaa49e5f29c2e3ee3923c811da
Author: Florina Muntenescu <florinam@google.com>
Date: Wed Aug 15 10:49:18 2018

Refactoring insert/update/delete to allow extensibility around the return type

Bug: 63317956
Test: tests updated
Change-Id: I5f8168fbd5cf08e64cb80c1835f4eebef6b4e018

M room/compiler/src/main/kotlin/androidx/room/processor/DeletionMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/processor/InsertionMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
M room/compiler/src/main/kotlin/androidx/room/processor/UpdateMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/DeleteOrUpdateMethodBinder.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/InsertMethodBinder.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/InstantDeleteOrUpdateMethodBinder.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/InstantInsertMethodBinder.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/DeleteOrUpdateMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InsertMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InstantDeleteOrUpdateMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InstantInsertMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/result/DeleteOrUpdateMethodAdapter.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/result/InsertMethodAdapter.kt
M room/compiler/src/main/kotlin/androidx/room/vo/DeletionMethod.kt
M room/compiler/src/main/kotlin/androidx/room/vo/InsertionMethod.kt
M room/compiler/src/main/kotlin/androidx/room/vo/ShortcutMethod.kt
M room/compiler/src/main/kotlin/androidx/room/vo/UpdateMethod.kt
M room/compiler/src/main/kotlin/androidx/room/writer/DaoWriter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/DeletionMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/processor/InsertionMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/processor/UpdateMethodProcessorTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt

https://android-review.googlesource.com/732628

https://goto.google.com/android-sha1/c0c36b71b1e73eeaa49e5f29c2e3ee3923c811da

ap...@google.com <ap...@google.com> #24Sep 6, 2018 10:26AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 524c841aa2bec6fa72768b1aeb40f6c6b6734907
Author: Florina Muntenescu <florinam@google.com>
Date: Mon Sep 03 16:37:48 2018

Adding RxJava support to insert

Bug: 63317956
Change-Id: Ic122774f591319dcd70d7f362af94adb0342184b
Test: InsertionMethodProcessorTest and TypeAdapterStoreTest updated

M room/compiler/src/main/kotlin/androidx/room/ext/javapoet_ext.kt
M room/compiler/src/main/kotlin/androidx/room/processor/InsertionMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/processor/ProcessorErrors.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/RxCallableInsertMethodBinder.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InsertMethodBinderProvider.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InstantInsertMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/RxCallableInsertMethodBinderProvider.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/result/InsertMethodAdapter.kt
A room/compiler/src/test/data/common/input/rxjava2/Completable.java
A room/compiler/src/test/data/common/input/rxjava2/Maybe.java
A room/compiler/src/test/data/common/input/rxjava2/Single.java
M room/compiler/src/test/kotlin/androidx/room/processor/InsertionMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/test_util.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt

https://android-review.googlesource.com/745108

https://goto.google.com/android-sha1/524c841aa2bec6fa72768b1aeb40f6c6b6734907

ap...@google.com <ap...@google.com> #25Sep 7, 2018 04:17PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit ecce72935bbe7987a9c0c1639f4aa245153c717a
Author: Florina Muntenescu <florinam@google.com>
Date: Fri Sep 07 15:36:18 2018

Add RxJava support for Delete and Update methods in Room.

Support for update and delete returning Integer also added.
Now, update and delete support:
* void
* int
* Integer
* Completable
* Single<Integer>
* Maybe<Integer>

Refactored the DeleteOrUpdateMethodAdapter to hold the return type, like InsertMethodAdapter is.

Bug: 63317956
Test: DaoWriterTest, TypeAdapterStoreTest, kotlin integration tests
Change-Id: Ic8d3cff21b9e8179fed237a18421c13b3e59059b

M room/compiler/src/main/kotlin/androidx/room/processor/DeletionMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/processor/UpdateMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/DeleteOrUpdateMethodBinder.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/InstantDeleteOrUpdateMethodBinder.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binder/RxCallableDeleteOrUpdateMethodBinder.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/DeleteOrUpdateMethodBinderProvider.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/InstantDeleteOrUpdateMethodBinderProvider.kt
A room/compiler/src/main/kotlin/androidx/room/solver/shortcut/binderprovider/RxCallableDeleteOrUpdateMethodBinderProvider.kt
M room/compiler/src/main/kotlin/androidx/room/solver/shortcut/result/DeleteOrUpdateMethodAdapter.kt
M room/compiler/src/main/kotlin/androidx/room/vo/DeletionMethod.kt
M room/compiler/src/main/kotlin/androidx/room/vo/ShortcutMethod.kt
M room/compiler/src/main/kotlin/androidx/room/vo/UpdateMethod.kt
M room/compiler/src/main/kotlin/androidx/room/writer/DaoWriter.kt
M room/compiler/src/test/data/common/input/rxjava2/Completable.java
M room/compiler/src/test/data/common/input/rxjava2/Maybe.java
M room/compiler/src/test/data/common/input/rxjava2/Single.java
M room/compiler/src/test/data/daoWriter/input/DeletionDao.java
M room/compiler/src/test/data/daoWriter/input/UpdateDao.java
M room/compiler/src/test/data/daoWriter/output/DeletionDao.java
M room/compiler/src/test/data/daoWriter/output/UpdateDao.java
M room/compiler/src/test/kotlin/androidx/room/processor/ShortcutMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/solver/TypeAdapterStoreTest.kt
M room/compiler/src/test/kotlin/androidx/room/writer/DaoWriterTest.kt
M room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/dao/BooksDao.kt

https://android-review.googlesource.com/750573

https://goto.google.com/android-sha1/ecce72935bbe7987a9c0c1639f4aa245153c717a

fl...@google.com <fl...@google.com> #26Sep 8, 2018 03:31PM

Marked as fixed.

Support for Single, Maybe and Completable added for insert, update and delete. To be released in Room 2.1

vf...@thoughtworks.com <vf...@thoughtworks.com> #27Apr 9, 2019 10:37AM

When is Room 2.1 going to be released?

sl...@gmail.com <sl...@gmail.com> #28Jul 5, 2019 11:08AM

artifact was moved to

https://mvnrepository.com/artifact/androidx.room