Chaining two nullable Optional in one pipeline

I have 2 Entity EntityOne and EntityTwo mapped to 2 tables in DB. All columns in EntityTwo are present in EntityOne too, as EntityTwo is the read table for one sub-system.

Now, below is some code to explain a scenario, I'm using spring data repo and java Optional, with java-8

    void doMagic(EntityOne entitytOne){   
        //Only if EntityOne present in Database, go delete entity Two.  
        repoOne
             .findById(entitytOne.getPrimaryKey()); //returns Optional<EntityOne>
             .ifPresent(this::deleteAssociatedEntityTwo);
    }

    void deleteAssociatedEntityTwo(EntityOne entityOne){
         // Only If able to find an EntityTwo associated with EntityOne in Database, then delete it
          fetchEntityTwo(entityOne) //returns Optional<EntityTwo>
                    .ifPresent(repoTwo::delete);
    }

Absent EntityOne::createEntityTwoPrimaryKey is a method in EntityOne class which create a new EntityTwoPrimaryKey object and populate all values.

     private Optional<EntityTwo> fetchEntityTwo(EntityOne entityOne) {
        // Only if passed entityOne is not null, query the Db and return Optional<EntityTwo>
        return Optional
                .ofNullable(entityOne)
                .map(EntityOne::createEntityTwoPrimaryKey)
                .map(repoTwo::findById) //returns Optional<EntityTwo>
                .orElse(Optional.empty());

    }

Now if you see in all above 3 methods have are optional checks to make sure action happen only if we have all data. I want to write this all code in one optional pipeline.

If I write like below, There are no compilation errors. But what will happen when repoOne.findById cant find any data in database ? I'm only checking ifPresent() after repoTwo::findById

     repoOne.findById(entityOne.getEntityOnePrimaryKey())
            .map(entityOne::createEntityTwoPrimaryKey) 
            .flatMap(repoTwo::findById) //returns Optional<EntityTwo>
            .ifPresent(repoTwo::delete);

Solution

Let's verify first if the resulting chain is correct, The first step would be breaking the methods into a single chain of Optional method calls.

void doMagic(EntityOne entityOne) {
    repoOne.findById(entityOne.getPrimaryKey())
           .ifPresent(eOne -> Optional
               .ofNullable(eOne)
               .map(entityOne::createEntityTwoPrimaryKey)
               .flatMap(repoTwo::findById)
               .ifPresent(repoTwo::delete));
}

Now we can fold entity -> Optional.ofNullable(entity) with folded ifPresent into a flattened structure:

void doMagic(EntityOne entityOne) {
    repoOne.findById(entityOne.getPrimaryKey())
           .map(entityOne::createEntityTwoPrimaryKey)
           .flatMap(repoTwo::findById)
           .ifPresent(repoTwo::delete);
}

So far so good, however, there is one more thing. But there is a dangerous thing, notice this line:

.map(entityOne::createEntityTwoPrimaryKey)

What's wrong? This method reference doesn't call the createEntityTwoPrimaryKey method of the instance captured by the lambda expression but the one passed to the method itself! It'd be equivalent to:

.map(e -> entityOne.createEntityTwoPrimaryKey(e))

Which is not correct as is error-prone to NPE because entityOne can be null. You want to use:

// both are equivalent
.map(e -> e.createEntityTwoPrimaryKey(e))
.map(EntityOne::createEntityTwoPrimaryKey)

So the final chain would look like:

void doMagic(EntityOne entityOne) {
    repoOne.findById(entityOne.getPrimaryKey())
           .map(EntityOne::createEntityTwoPrimaryKey)
           .flatMap(repoTwo::findById)
           .ifPresent(repoTwo::delete);
}

Now the transformation is really correct, so let's go back to the question:

If I write like below, There are no compilation errors. But what will happen when repoOne.findById can't find any data in the database? I'm only checking ifPresent() after repoTwo::findById.

Consider the following scenarios:

repoOne.findById returns Optional.empty() - No subsequent map and flatMap are called, hence nothing is created. Also no ifPresent is called, hence nothing is deleted.
repoOne.findById returns a non-empty Optional but map does - Again, everything from flatMap to the end is not called, hence nothing is created or deleted.
Everything proceeding flatMap returns a non-empty Optional, but flatMap does - Here the things start to be interesting because createEntityTwoPrimaryKey is whatever it does, but the removal in ifPresent is not. However, I assume if something is created it would be also most likely be found, so this scenario is really an edge case.
The final result depends on the delete method call, however, it is of the void return type. As long as no RuntimeException is thrown, it is safe.

Summary, I find the Optional chain safe. You might want to annotate the method with @Transactional to rollback on possible RuntimeException.