What is a good idiom for nested flatMaps in Java Reactor?

Question

I inherited responsibility for a REST service written in Java using Spring and associated libraries including Reactor. For expensive operations like REST calls out or database operations the code is extensively wrapping the results in Reactor Mono.

There are all sorts of things that need to be addressed in the code but one that keeps showing up is nested flatMaps over Monos for sequences of expensive operations that end up up indented several levels deep into an unreadable mess. I find it extra irksome because I came from Scala where this way of using flatMap is not as bad because of the for comprehension syntactic sugar keeping everything at roughly the same level of scope instead of going deeper.

I have so far had no success finding a way to approach this to make it more readable apart from a massive refactor (and even then I am not sure where to start such a refactor).

Anonymized example based on the code, (all syntax errors are from anonymization):

public Mono<OutputData> userActivation(InputData input) {
    Mono<DataType1> d1 = service.expensiveOp1(input);

    Mono<OutputData> result =
        d1
          .flatMap(
            d1 -> {
              return service
                  .expensiveOp2(d1.foo())
                  .flatMap(
                      d2 -> {
                        if (Status.ACTIVE.equals(d2.getStatus())) {
                          throw new ConflictException("Already active");
                        }

                        return service
                            .expensiveOp3(d1.bar(), d2.baz())
                            .flatMap(
                                d3 -> {
                                  d2.setStatus(Status.ACTIVE);

                                  return service
                                      .expensiveOp5(d1, d2, d3)
                                      .flatMap(
                                          d4 -> {
                                            return service.expensiveOp6(d1, d4.foobar())
                                          });
                                });
                      });
            })

    return result;
}

Answer 1

Yuck. A few things I don't like about that snippet, but I'll start with the big one - the nesting.

The only reason for the nesting is that in (for example) expensiveOp5() you need a reference to d1, d2 and d3, not just d4 - so you can't just map through "normally", because you lose those earlier references. Sometimes it's possible to refactor these dependencies away in a particular context, so I'd examine that route first.

However, if it's not possible or desirable, I've tended to find deeply nested flatMap() calls like this are best replaced with intermediate objects through composition.

If you have a bunch of classes as follows for instance:

@Data
class IntermediateResult1 {
    private DataType1 d1;
    private DataType2 d2;
}

@Data
class IntermediateResult2 {
    public IntermediateResult2(IntermediateResult1 i1, DataType3 d3) {
        this.d1 = i1.getD1();
        this.d2 = i1.getD2();
        this.d3 = d3;
    }
    private DataType1 d1;
    private DataType2 d2;
    private DataType3 d3;
}

...and so on, then you can just do something like:

    return d1.flatMap(d1 -> service.expensiveOp2(d1.foo()).map(d2 -> new IntermediateResult1(d1, d2)))
             .flatMap(i1 -> service.expensiveOp3(i1).map(s3 -> new IntermediateResult2(i1, d3)))
             //etc.

Of course, you can also then break out the calls into their own methods to make it clearer (which I'd probably advise in this case):

return d1.flatMap(this::doOp1)
         .flatMap(this::doOp2)
         .flatMap(this::doOp3)
         .flatMap(this::doOp4)
         .flatMap(this::doOp5);

Obviously, the names I've used above should be considered nothing but placeholders - you should think carefully about these names, as good naming here will make reasoning about and explaining the reactive stream much more natural.

Aside from the nesting, two other points worth noting in that code:

• Use return Mono.error(new ConflictException("Already active")); rather than throwing explicitly, as it makes it much clearer that you're dealing with an explicit Mono.error in the stream.
• Never use mutable methods like setStatus() half way through a reactive chain - that's asking for problems later on. Instead, use something like the with pattern to generate a new instance of d2 with an updated field. You can then call expensiveOp5(d1, d2.withStatus(Status.ACTIVE), d3) whilst forfeiting that setter call.