Add RxJava support #1710

cmelchior · 2015-11-01T12:20:52Z

This PR fixes #865

This PR is a feature branch for collecting all RxJava sub-branches before merging. Right now the roadmap is this:

1) This branch adds simple RxJava support by adding RealmConfiguration methods / Factory interface and a simple implementation supporting RealmObject/RealmResults/Realm.
2) Sample project showing the current caveats and work-arounds to be mindful off (Adding RxJava example #1940)
3) Add support for RealmQuery observable. We need to expose handover of queries to other threads. Being tracked in Add support for RealmQuery.asObservable() #1955 .
4) Custom DetachedCopyObservableFactory class for making all RealmData in-memory data. We need to expose handover for RealmObject/RealmResults. -> Probably not possible to do transparently before Thread handoff for RealmObject and RealmResults / Frozen state #1208 at which it most likely becomes moot anyway.
5) Add Support for RealmList (not relevant before fine-grained notification and changelisteners have been added). Being tracked in Add support for RealmList.asObservable() #1956 .

1 + 2 is required before merging to main branch, 3 would be nice to have, 4 + 5 will probably take a little longer and will tie into handover mechanisms already considered elsewhere.

API: (for step 1)

Realm.asObservable();
RealmObject.asObservable();
RealmResults.asObservable();
DynamicRealm.asObservable();
DynamicRealmObject.asObservable();
RealmConfiguration.Builder.rxFactory(RxObservableFactory factory);

Note that all Rx classes are in a new package io.realm.rx. Should we move these into io.realm as that currently holds all our public API? It doesn't conflict with the proposal of adding io.realm.android (as RxJava is not Android only), but something to consider when adding future classes/functionality.

Finalize API proposal in [Design] RxJava support #1651. Especially if RxObservableFactory should move to its own dependency. Conclusion: Keep factory classes in repo for now. Other questions:
1. We cannot have rx classes in a io.realm.rx without making BaseRealm public, is having them in io.realm OK? Otherwise should we make BaseRealm public but perhaps exclude it from JavaDoc? Answer: Added methods explicitly naming Realm/DynamicRealm to work around this.
2. Should we expose observable methods on RealmQuery? YES: See other PR
3. RealmObject is now RealmObject<E extends RealmObject<E>> which is slightly painful if you want type safe observables for RealmObjects. You need to do public class Person extends RealmObject<Person>. It is not mandatory though. Answer: YES, IntelliJ will give you warning if no generic parameter is given, but otherwise it works fine in all other cases, i.e. only RxJava users will notice it.
Fix changelistener bug ChangeListener not triggered on newly created RealmObject #1884
Fix changeListeners not triggering for synchronous queries on RealmResults/RealmObjects ( Change listener not being triggered on realm updates #1676).
Use copyFromRealm for InMemoryObservableFactory ( Adds Realm.copyFromRealm() #1849 )
Investigate the possibility to use a custom RealmScheduler to facilitate moving across threads. Just like our current async API does now. Conclusion: Not possible as the scheduler does not have access to the parameters being parsed around. With auto-closable Realms it will be possible, or wait for freeze() semantics.
More examples of Observables with 'gotchas' have to be added.
Throw proper error message if calling observable() and RxJava is not on the classpath

emanuelez · 2015-11-02T13:43:42Z

realm/realm-library/src/main/java/io/realm/RealmObject.java

@@ -81,7 +82,7 @@
 */

 @RealmClass
-public abstract class RealmObject {
+public abstract class RealmObject<E extends RealmObject> {


Ouch :( That's not pretty...

No, it is horrible. Fortunately it is only required for RealmObjects as both RealmResults and RealmList has the type information.

I tried different versions of generics black magic, but AFAIK it isn't possible for an abstract class to return a subclass type without it being explicitly defined somewhere (either as a generic like here, or as input parameter to a method)

It kind of makes me reconsider RealmObservable.from() and allow users to throw anything at it, a bit like http://reactivex.io/RxJava/javadoc/rx/Observable.html#from(java.util.concurrent.Future)

Yes, the pattern is not unknown from RxJava itself: Observable.from(), Observable.just(), but it really really break up the flow when you are coding. I tried it when working on the examples.

Upside is that you don't actually need the generic, but if you don't add it you just get Object as the return type in the Observable chain.

I have been contemplating if we should add an explicit ListenerInterface, but that just seems like unnecessary boilerplate

austynmahoney · 2015-12-08T18:38:49Z

realm/realm-library/src/main/java/io/realm/rx/RealmObservableFactory.java

+                final RealmChangeListener listener = new RealmChangeListener() {
+                    @Override
+                    public void onChange() {
+                        subscriber.onNext(realm);


I believe this should be checking subscriber.isUnsubscribed() before calling onNext to follow the Rx contract. Same with all the other from methods.

More info:
http://ryanharter.com/blog/2015/07/07/wrapping-existing-libraries-with-rxjava/
https://www.reddit.com/r/androiddev/comments/3vebo3/resubmission_respect_the_observable_contract_when/

Hey @austynmahoney
Thanks for the pointers. However I added the subscriber.add(...) below to prevent that exact scenario. That should unregister the listener when unsubscribing which means it shouldn't be needed to check for subscriber.isUnsubscribed in the callback. I won't rule out I made a mistake somewhere though, so i'll go over it again :)

Source: http://stackoverflow.com/questions/26695125/how-to-get-notified-of-a-observers-unsubscribe-action-in-a-custom-observable-in

I'd still vote for following the contract and adding the check anyways. If some code changes down the road, or the unsubscribe happens on another scheduler, it could end up breaking the contract.

cmelchior · 2015-12-14T14:16:04Z

Ready for review

@realm/java
@donnfelker

kneth · 2015-12-16T11:55:39Z

realm/realm-library/src/androidTest/java/io/realm/RxJavaTests.java

@@ -0,0 +1,288 @@
+package io.realm;


Standard header?

kneth · 2015-12-16T12:38:12Z

I see no tests for RealmQuery and ReamList. We should check if the correct exception is thrown - just to ensure that the JavaDoc is consistent with the behaviour.

cmelchior · 2015-12-16T12:42:30Z

We don't have to write any tests for them yet, as the API is not exposed, so there is nothing that can crash. I added from(RealmQuery) and from(RealmList) to make the interface future-proof and avoid having to introduce breaking changes to it later.

i.e. without the methods RealmQuery.asObservable() and RealmList.asObservable() those methods are currently no-ops.

nhachicha · 2015-12-16T15:41:22Z

realm/realm-library/src/main/java/io/realm/rx/RxObservableFactory.java

+     * Creates an Observable for a {@link Realm}. It should emit the initial state of the Realm when subscribed to and
+     * on each subsequent update of the Realm.
+     *
+     * Realm observables are hot observables as Realms automatically are kept up to date.


Realm observables are hot observables as Realms automatically are kept up to date.

as Realms are automatically kept up to date.

same for the below repetitions

cmelchior · 2015-12-16T19:43:26Z

Issues fixed @realm/java

kneth · 2015-12-17T08:58:50Z

👍 (you have responded to my comments)

emanuelez · 2015-12-17T09:02:28Z

realm/realm-library/src/androidTest/java/io/realm/RxJavaTests.java

+        realm.where(AllTypes.class).findFirstAsync().<AllTypes>asObservable().subscribe(new Action1<AllTypes>() {
+            @Override
+            public void call(AllTypes rxObject) {
+                subscriberCalled.addAndGet(1);


This could be subscriberCalled.incrementAndGet()

emanuelez · 2015-12-17T09:11:20Z

👍 apart from very minor comments

Add RxJava support

akarnokd · 2015-12-17T12:16:24Z

realm/realm-library/src/main/java/io/realm/rx/RealmObservableFactory.java

+
+                // Immediately call onNext with the current value, as due to Realms auto-update, it will be the latest
+                // value.
+                subscriber.onNext(results);


You can also check subscriber.isUnsubscribed() here as well or drop the if on line 128 depending on how eagerly you want to handle the small removeChangeListener/unsubscription window.

cmelchior added P2 labels Nov 1, 2015

emanuelez reviewed Nov 2, 2015
View reviewed changes

cmelchior mentioned this pull request Dec 2, 2015

RxJava example #1870

Closed

austynmahoney reviewed Dec 8, 2015
View reviewed changes

cmelchior mentioned this pull request Dec 10, 2015

Add sample app demonstrating offline mode #1926

Closed

3 tasks

cmelchior added Review and removed P2 labels Dec 14, 2015

cmelchior mentioned this pull request Dec 14, 2015

Adding RxJava example #1940

Merged

cmelchior self-assigned this Dec 15, 2015

kneth reviewed Dec 16, 2015
View reviewed changes

kneth mentioned this pull request Dec 16, 2015

A RealmObject with no @PrimaryKey cannot be updated #1845

Closed

nhachicha reviewed Dec 16, 2015
View reviewed changes

emanuelez reviewed Dec 17, 2015
View reviewed changes

This was referenced Dec 17, 2015

Add support for RealmQuery.asObservable() #1955

Open

Add support for RealmList.asObservable() #1956

Closed

Add RxJava support

d6db860

cmelchior force-pushed the feature-rxjava branch from c54bca1 to d6db860 Compare December 17, 2015 11:02

cmelchior added a commit that referenced this pull request Dec 17, 2015

Merge pull request #1710 from realm/feature-rxjava

becf589

Add RxJava support

cmelchior merged commit becf589 into master Dec 17, 2015

cmelchior removed the Review label Dec 17, 2015

artem-zinnatullin mentioned this pull request Dec 17, 2015

API for Realm database pushtorefresh/storio#169

Closed

akarnokd reviewed Dec 17, 2015
View reviewed changes

emanuelez deleted the feature-rxjava branch November 25, 2016 08:04

github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RxJava support #1710

Add RxJava support #1710

cmelchior commented Nov 1, 2015

emanuelez Nov 2, 2015

cmelchior Nov 2, 2015

emanuelez Nov 2, 2015

cmelchior Nov 2, 2015

austynmahoney Dec 8, 2015

cmelchior Dec 8, 2015

austynmahoney Dec 8, 2015

cmelchior commented Dec 14, 2015

kneth Dec 16, 2015

cmelchior Dec 16, 2015

kneth commented Dec 16, 2015

cmelchior commented Dec 16, 2015

nhachicha Dec 16, 2015

cmelchior commented Dec 16, 2015

kneth commented Dec 17, 2015

emanuelez Dec 17, 2015

emanuelez commented Dec 17, 2015

akarnokd Dec 17, 2015

Add RxJava support #1710

Add RxJava support #1710

Conversation

cmelchior commented Nov 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmelchior commented Dec 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kneth commented Dec 16, 2015

cmelchior commented Dec 16, 2015

Choose a reason for hiding this comment

cmelchior commented Dec 16, 2015

kneth commented Dec 17, 2015

Choose a reason for hiding this comment

emanuelez commented Dec 17, 2015

Choose a reason for hiding this comment