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

Align hasCustomOnError behavior of CallbackCompletableObserver with L… #7326

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Conversation

hqzxzwb
Copy link
Contributor

@hqzxzwb hqzxzwb commented Aug 26, 2021

…ambdaObserver, ConsumerSingleObserver and so on

Background:
CallbackCompletableObserver.hasCustomOnError has a different implementation than other LambdaConsumerIntrospection implementations. This causes hasCustomOnError to return true even if one passes Functions.ON_ERROR_MISSING to Completable.subscribe(onComplete, onError). In contrast, if one passes Functions.ON_ERROR_MISSING to Single.subscribe(onComplete, onError), the ConsumerSingleObserver returns false from hasCustomOnError, and the case is the same with Single for Observable, Maybe and Flowable. This makes Completable a bit unnecessarily special.

Changes made:
Use the same strategy from other observable classes.

Might be breaking:
This change is making ABI changes on CallbackCompletableObserver. I am not sure if we need to maintain ABI stability even for classes in internal package. If it is needed, I am happy to add one more commit to keepCallbackCompletableObserver's ABI stable.

…ambdaObserver, ConsumerSingleObserver and so on
@akarnokd akarnokd added this to the 3.1-support milestone Aug 26, 2021
@codecov
Copy link

codecov bot commented Aug 26, 2021

Codecov Report

Merging #7326 (30a0d89) into 3.x (804b8ef) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #7326      +/-   ##
============================================
+ Coverage     99.53%   99.55%   +0.02%     
+ Complexity     6784     6781       -3     
============================================
  Files           751      751              
  Lines         47490    47482       -8     
  Branches       6378     6378              
============================================
+ Hits          47269    47273       +4     
+ Misses          102       93       -9     
+ Partials        119      116       -3     
Impacted Files Coverage Δ
...in/java/io/reactivex/rxjava3/core/Completable.java 100.00% <100.00%> (ø)
...nternal/observers/CallbackCompletableObserver.java 100.00% <100.00%> (ø)
.../operators/flowable/FlowableBlockingSubscribe.java 93.02% <0.00%> (-4.66%) ⬇️
.../operators/observable/ObservableFlatMapSingle.java 94.44% <0.00%> (-3.97%) ⬇️
...nternal/operators/parallel/ParallelReduceFull.java 93.06% <0.00%> (-1.99%) ⬇️
...operators/flowable/FlowableFlatMapCompletable.java 98.71% <0.00%> (-1.29%) ⬇️
...3/internal/operators/flowable/FlowablePublish.java 99.00% <0.00%> (-1.00%) ⬇️
...l/operators/observable/ObservableFlatMapMaybe.java 91.54% <0.00%> (-0.71%) ⬇️
.../reactivex/rxjava3/observers/BaseTestConsumer.java 99.46% <0.00%> (-0.54%) ⬇️
...ernal/operators/flowable/FlowableFlatMapMaybe.java 96.89% <0.00%> (-0.52%) ⬇️
... and 47 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 804b8ef...30a0d89. Read the comment docs.

@akarnokd akarnokd merged commit 939b5ce into ReactiveX:3.x Aug 26, 2021
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