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

Migrate to RxJava2 #42

Merged
merged 6 commits into from
Jul 6, 2017
Merged

Migrate to RxJava2 #42

merged 6 commits into from
Jul 6, 2017

Conversation

BenSchwab
Copy link
Contributor

@BenSchwab BenSchwab commented Jun 28, 2017

closes #34

This is a stab at migrating to RxJava2:

Code changes: b8e5313
Test changes: ab21d32

Most of the code changes are simple mappings from: https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0.

Two things to note:

  • In 2.0 there are new reactive types (Flowable, Single, Maybe) which do not extend from a base type. Currently, this change only supports the Observable type. I'm open to supporting other types, if there is demand.
  • The other big change that affects RxGroups is the change to disposables. Observers can no longer be used multiple times if they are disposed. The locking/unlocking mechanism relies on subscribing/disposing observers. Thus, we need some sort of observer wrapper. The cleanest integration seems to be to send the emitter to the SubscriptionProxy and wrap it with a fresh DisposableObserver every time we subscribe. With the changes to disposables, it is now safe to call dispose on the observer used on the transformed observable directly, instead of going through SourceSubscription. Of course, this will still leave the SourceSubscription subscribed, until cancel is called.

The test changes may seem like a lot, but most of the changes are caused by:

  • The fact that ObservableGroup#add takes an emitter -- this means the easiest way to test was using the GroupSubscriptionTransformer
  • Observers can not be reused after being disposed

@BenSchwab BenSchwab mentioned this pull request Jun 28, 2017
Copy link
Contributor

@felipecsl felipecsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't had a chance with play with RxJava 2 much, but overall looks straightforward

public <T> Observable.Transformer<? super T, T> transform(Observer<? super T> observer,
String observableTag) {
public <T> ObservableTransformer<? super T, T> transform(Observer<? super T> observer,
String observableTag) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you should disable "align when multiline"

* is subscribed to one {@link Observable}.
*/
public <T> void resubscribe(TaggedObserver<? super T> observer) {
resubscribe(observer, Utils.getObserverTag(observer));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is using 4 spaces

@BenSchwab BenSchwab merged commit 58dd332 into 1.0 Jul 6, 2017
@BenSchwab BenSchwab deleted the bschwab--rxjava2 branch July 6, 2017 21:15
BenSchwab pushed a commit that referenced this pull request Jan 2, 2018
* Add observer tag

* annotation processor

* fix lifecycle test

* fix tests

* Add observer tag

* annotation processor

* fix lifecycle test

* fix tests

* Clean up tests and bump version to 1.0.0-SNAPSHOT

* address comments

* Create Processor Project, DRY gradle dependencies (#37)

* Move rxgroups-processor to separate project, DRY multi-module dependencies

* renamen package to processor, fix spacing

* rxgroups-android tests need to depend on processor

* Switch line limit back to 100, update gradle to 3.5

* get checkstyle to work with gradle 3.5

* Make ObservableGroup#resubscribe visible to generated code, artificat for rx-processor (#38)

* Have rxgroups-processor upload build artifact

Use variable for repo urls

* Make ObseravbleGroup#resubscribe public for generated code. Remove deprecated use of Observable#create

* Use getter/setter with JavaDoc

* fix checkstyle

* Add back javadoc quiet mode

* Use subclass to control visability

* Make mulitiple observer support more clear (#39)

* Make mulitiple observer support more clear

* fix checkystyle

* Consistently use the term observableTag for user facing code, and observerTag where necessary internally

* Add test for multiple observers, cancelAndRemoveAllWithTag

* Update all tests to reflect the use of observerTag instead of null in 1:1 case

* remove extra space

* Fix javadocs

* Remove cancelling by observableTag

* remove unused observableTag code

* fix another typo

* Add tagged observer to allow for custom tag (#40)

* Add tagged observer to allow for using a non-auto generated observer tag in exceptional circumstances

* Support auto resubcription with tagged observer

* add tests

* Add AutoTag, NonResubscribableTag, safeInitialize (#41)

* Add AutoTag, safeIntialization, and NonResubscribingTag

* revert gradle change

* Fix some comments

* Change to NonResubscribable

* fix test

* propogate cancel and remove all change to lifecyclemanager

* remove duplicated method

* fix test name

* Cache missing constructors, add super class initialization support

change some access

fix some more

* Make GroupLifecycleManager properly shadow methods in ObservableGroup

* disable check style for mock generated code

* reduce chance of concurrent modification exception

* Add supression comment filter

* Add filecontentsholder to have checkstyle actually read turn off comments

* remove duplicated remove code

* Add AnnotationProcessor code gen tests

* use config build tools version

* fix no checkstyle check

* add missing new lines

* Change version to 1.0.0-alpha1

* Prevent null tag collisions by resorting to NonResubscribingTag, if getTag returns null

* add test for custom tagging

* eliminate double map lookup

* Migrate to RxJava2 (#42)

* Migrate to RxJava2 for Observables

cleanup

* update tests and sample for rxjava2

* update travis

* Fix test checkstyle

* fix line alignment

* Fix some documentation

* Fix continuation idents, use 2 spaces in ObservableGroup (#43)

* bump version (#44)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants