Chaining two nullable Optional in one pipeline

Viewed 550

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);
1 Answers

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.

Related