reactor / reactor-core

Non-Blocking Reactive Foundation for the JVM
http://projectreactor.io
Apache License 2.0
4.98k stars 1.21k forks source link

Fix marble for Flux.next() #1399

Closed daniildxb closed 5 years ago

daniildxb commented 6 years ago

Described behaviour

Take one element from the stream and ignore rest. current_marble_next

Actual behaviour

Take one element from the stream and cancel subscription.

next

Code reference

Flux.next() returns MonoNext, which cancels subscription onNext. However cancel is absent on a marble.

        @Override
        public void onNext(T t) {
            if (done) {
                Operators.onNextDropped(t, actual.currentContext());
                return;
            }

            s.cancel();
            actual.onNext(t);
            onComplete();
        }

Reactor Core version

3.2.1

simonbasle commented 6 years ago

As said in the PR, the essence of this diagram is showing how a Flux (multiple elements) is turned into a Mono (single element) by only considering the first element from the source.

In my opinion, is should not be a goal of marble diagrams to be 100% exact and to show the whole detail of implementation, although - granted - we do show cancel and request signal on a bunch of marbles.

However, maybe a good middle ground would be to show a cancel() arrow going from the operator to the source, while still keeping the source as it is in the current diagram (ie. with multiple marbles).

pinging @smaldini for opinion.

Note that as we are migrating the whole set of marbles from OmniGraffle + PNG to SVG only, we cannot accept PRs on marbles until said migration is done. (#1390)

daniildxb commented 6 years ago

@simonbasle Ok, thanks for the feedback.

thekalinga commented 6 years ago

@simonbasle One way to show that its a Flux is to have the marbles after cancel to be shown in grey color which implies that they will no longer be pushed downstream, while ate the sametime showing the source is a Flux capable of delivering multiple items had next not been applied