Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Design] RxJava support #1651

Closed
wants to merge 2 commits into from
Closed

[Design] RxJava support #1651

wants to merge 2 commits into from

Conversation

cmelchior
Copy link
Contributor

This PR contains a document describing how we could go about adding RxJava support to Realm.

See #865 for the issue and https://github.com/realm/realm-java/tree/cm-lab-rxjava for some earlier prototype code.

Update 26/10-2015
After some discussion, we are narrowing down to Option B:

  • First class .observable() support for all Realm classes.
  • RxJava as an optional dependency.
  • Support RxJava 1., RxJava 2. and possible RxMobile using factory classes for constructing the observables for the correct version of Rx*. Should be pluggable. Depending on the differences between the factory classes we might want to release this as a separate dependency. If we are adding custom schedulers making it a separate plugin also makes a lot more sense.
  • Investigate if we can use a custom schedulers to avoid Realms thread restrictions.

Update 07/12-2015

Basic API (v1)

  • Add the following API's that work on top of RealmChangeListener
    • Realm.asObservable()
    • RealmObject.asObservable()
    • RealmResults.asObservable()
    • DynamicRealm.asObservable()
    • DynamicRealmObject.asObservable()
    • RealmList.asObservable() (not yet supported)

Using asObservable() instead of observable(). It will be consistent with the wording used by e.g Subjects and will fit well when we add asCursor() ( #1090 )

  • Add the following to support different user configured ways of creating Observables.
    • New interface: RxObservableFactory
    • New class: RealmObservabableFactory for automatically converting RealmChangeListener to Observables (default behavior).
    • Add RealmConfiguration.Builder.rxFactory(RxObservableFactory)

Advanced API (v2):

  • Add the following to RealmQuery, so people can execute queries on other threads. It will require exposing query handover in some shape.
    • RealmQuery.asObservable()
    • New class: DetachedCopyObservableFactory variant that automatically uses Realm.copyFromRealm to copy RealmObject/RealmResults to memory. Ideally it should be possible to control which thread does the copying which require exposing object/RealmResult handover.

Implementation notes:

  • Realm objects are still thread confined and will be automatically updated. This will not change before Thread handoff for RealmObject and RealmResults / Frozen state #1208 is implemented. It means that some operators in RxJava have to be used with care. Realm.copyFromRealm can provide a temporary work-around, but is not without cost itself.

Examples:

RealmConfiguration config = new RealmConfiguration.Builder()
  .rxFactory(new MySpecialRxJavaObservableFactory())
  .build();

// Async queries done in their own worker thread, all other operations can be done on the main thread
// beware of operators that work on special schedulers.
realm.where(Person.class).findAllAsync().asObservable()
  .filter(results.isLoaded()) // Only proceed once results are available
  .subscribe(showResultsOnScreen(results));

// Execute query on background/copy data to memory to work around thread confinement/live-updates at the cost of zero copy/consistency.
realm.where(AllPerson.class).asObservable()
  .flatmap(query.findAll().asObservable())
  .map(realm.copyFromRealm(results)) 
  .subscribeOn(Schedulers.computation())
  .observeOn(AndroidSchedulers.mainThread())
  .subscribe(showResultsOnScreen(results)); 

@realm/java

@cmelchior cmelchior mentioned this pull request Oct 23, 2015
@fuwaneko
Copy link

I'm certainly against C option to include RxJava. While it might be easiest way to do it, libraries like Retrofit have moved towards hybrid A/B approach exactly because it's the worst way to do it too. I see it similar to Retrofit, there's an "adapter" addon which turns findAll() and similar methods to return Observable and it has to be registered on configuration step. And given the fact that Realm instances can be configured when calling getInstance(), programmer can temporarily enable/disable it too by simply providing/not providing adapter.

Also, it would be awesome for Observable to emit onNext with updated results every time Realm changes. Or even better, when specific RealmResults change.

Should these Observables also be deferred? I.e. wait until at least one subscriber?

@cmelchior
Copy link
Contributor Author

The Adapter approach is really cool. Unfortunately it works for Retrofit because the user defines the interfaces, unlike Realm where we provide them.

Regarding onNext, then that is exactly what we intend to do. This is already happening today because RealmObjects auto-refresh themselves, so it is just a matter of putting RxJava semantics on top of our own ChangeListeners.

As I mention in the docs, then we can only defer work if you observe on a RealmQuery (which brings a few more headaches regarding our different query options). Observables created from e.g. RealmResults would already have done the work as it is similar to observing on a List.

@fuwaneko
Copy link

@cmelchior Ah, I see, so unless RealmQuery is used, it's practically just async query wrapped in Observable?

As for adapter approach, I was just telling my thoughts. I kinda figured it's not directly applicable. In any case C approach would introduce more coupling and I'm on "opt-in" side instead of "opt-out" even though I always use RxJava on Android and it's not a big deal for me personally.

@cmelchior
Copy link
Contributor Author

@fuwaneko Yes, that is correct.

I totally agree with your point about case C. It is pretty much a choice between "less coupling and slightly worse API" or "stronger coupling and better API".

@fuwaneko
Copy link

@cmelchior haha, in that case I vote for less coupling and B version. In any case, thanks for working on adding this.

@lpessoa
Copy link

lpessoa commented Oct 23, 2015

Would go for option B as well. Better keep it open to allow adding support for future libs that may come up.

@FranciscoE-Hudl
Copy link

Why not go with simple Futures, so RxJava is not baked into the codebase yet still usable. Option A is fine for me as long as you don't branch to RxRealm.

@cmelchior
Copy link
Contributor Author

We had some talks internally about this, and by the looks of it we will probably go with some variant of proposal B. It has been summarized in the top post.

@kboyarshinov
Copy link

I like the option B as well. Will be pretty straightforward to use. But I do not think that RxJava support should be added today even if it can be. Threading and notification issues are preferred to be resolved before imo. This won't confuse users and lead them to inappropriate usage scenarios. Would be good if RxJava support will be in 1.0 release. And if it do not it will be still better than unfinished RxJava compatible version.

Anyway 👍 for making this.

@cmelchior cmelchior mentioned this pull request Nov 1, 2015
12 tasks
@cmelchior
Copy link
Contributor Author

Support has been released as part of 0.87. Closing

@cmelchior cmelchior closed this Dec 18, 2015
@cmelchior cmelchior removed the P2 label Dec 18, 2015
@antonshkurenko
Copy link

What about RxJava 2 support?

@kneth
Copy link
Contributor

kneth commented Sep 27, 2016

@tonyshkurenko Please create a new issue for RxJava 2 support. Thanks in advance.

@cmelchior cmelchior mentioned this pull request Oct 29, 2016
@cmelchior cmelchior deleted the cm/rxjava branch September 2, 2017 11:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants