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

2.x: Flowable as a Publisher to be fully RS compliant #5112

Merged
merged 1 commit into from
Feb 18, 2017

Conversation

akarnokd
Copy link
Member

This PR performs the changes suggested in #5110.

  • Introduce FlowableSubscriber with extra textual specification on its relaxed nature
  • Flowable.subscribe(Subscriber) checks for FlowableSubscriber and if not found, it wraps the incoming RS Subscriber into a StrictSubscriber that follows the RS spec to the letter at any cost.
  • Introduce Flowable.subscribe(FlowableSubscribe) that most internal operators will use
  • Change AbstractFlowableWithUpstream to accept Flowable as a source, update operators
  • Some operators were useful with raw Publisher input, these were duplicated on their outer containing type but use the same internal FlowableSubscriber
  • Removed "cheat" from the TCK tests, adjusted timeout on delay
  • Replaced most implements Subscriber with implements FlowableSubscriber
  • Replaced most new Subscriber with new FlowableSubscriber in tests, the rest is required for testing the strictness itself.
  • strict() is now an identity operator with suggested scheduled removal.

Performance impact estimation

  • Most primary use of a Flowable should go through subscribe(FlowableSubscribe) and thus no overhead change.
  • Where the API mandated Publisher as input, providing a Flowable will have an instanceof check at subscription time and routed to `subscribe(FlowableSubscriber) if the consumer is part of RxJava 2 itself.

@akarnokd akarnokd added this to the 2.1 milestone Feb 18, 2017
Copy link
Contributor

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

Looks good!

@akarnokd akarnokd merged commit a00ea07 into ReactiveX:2.x Feb 18, 2017
@akarnokd akarnokd deleted the ReactiveStreamsCompliance branch February 18, 2017 09:04
@akarnokd
Copy link
Member Author

Okay, let's go from here. In the unlikely case the spec becomes more forgiving, the undo is just to remove cast/wrapping from Flowable.subscribe and everything works the same as FlowableSubscriber is an interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants