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

Removed unnecessary upstream.cancel() call for casually finished upstream sequences. #6992

Conversation

sobersanta
Copy link
Contributor

@sobersanta sobersanta commented May 20, 2020

… finished normally via onComplete/onError from upstream;

minor code cleanup - unnecessary Disposable implementation to avoid method name clash
public void noUpstreamCancelOnCasualChainClose() {
AtomicBoolean parentUpstreamCancelled = new AtomicBoolean(false);
Flowable.range(1, 10)
.doOnCancel(new Action() {
Copy link
Member

Choose a reason for hiding this comment

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

You can use lambdas in tests now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool!
Should I change it?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that's fine, I've refactored whole file to lambdas.

@akarnokd akarnokd added this to the 3.1 milestone May 20, 2020
@codecov
Copy link

codecov bot commented May 20, 2020

Codecov Report

Merging #6992 into 3.x will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #6992      +/-   ##
============================================
+ Coverage     99.49%   99.52%   +0.03%     
- Complexity     6669     6670       +1     
============================================
  Files           742      742              
  Lines         47267    47268       +1     
  Branches       6374     6375       +1     
============================================
+ Hits          47026    47044      +18     
+ Misses          114      105       -9     
+ Partials        127      119       -8     
Impacted Files Coverage Δ Complexity Δ
...l/operators/flowable/FlowablePublishMulticast.java 100.00% <100.00%> (ø) 2.00 <0.00> (ø)
...tivex/rxjava3/disposables/CompositeDisposable.java 98.14% <0.00%> (-1.86%) 39.00% <0.00%> (-1.00%)
...l/operators/observable/ObservableFlatMapMaybe.java 90.14% <0.00%> (-1.41%) 2.00% <0.00%> (ø%)
...rnal/operators/flowable/FlowableFlatMapSingle.java 94.18% <0.00%> (-1.17%) 2.00% <0.00%> (ø%)
...nternal/operators/observable/ObservableCreate.java 99.14% <0.00%> (-0.86%) 2.00% <0.00%> (ø%)
...nternal/operators/observable/ObservableReplay.java 99.46% <0.00%> (-0.54%) 22.00% <0.00%> (ø%)
.../operators/observable/ObservableCombineLatest.java 100.00% <0.00%> (+0.61%) 6.00% <0.00%> (ø%)
.../operators/observable/ObservableFlatMapSingle.java 98.41% <0.00%> (+0.79%) 2.00% <0.00%> (ø%)
...ternal/operators/observable/ObservableGroupBy.java 100.00% <0.00%> (+1.09%) 2.00% <0.00%> (ø%)
...nternal/operators/observable/ObservableWindow.java 100.00% <0.00%> (+1.81%) 3.00% <0.00%> (ø%)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca222c2...39c8321. Read the comment docs.

@akarnokd akarnokd merged commit e2b1d2f into ReactiveX:3.x May 20, 2020
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.

3.x: unnecessary call to upstream .cancel() in FlowablePublishMulticast operator
2 participants