Is my implementation of a simple CRUD service with Spring WebFlux correct?

I am implementing a simple Movie based CRUD API using Spring WebFlux and Reactive Spring Data MongoDB. I want to make sure that my implementation is correct and that I am properly using Flux and Mono to implement the CRUD operations. I also want to make sure that I am properly handling any errors or null values. I am very new to this programming paradigm and Spring WebFlux, so I am not sure about the correctness of the implementation of the Controller and Service layer, I want to make sure I am adhering to Spring WebFlux and Project Reactor best practices.

```
@Repository
public interface MovieRepository extends ReactiveMongoRepository {

Flux findByRating(String rating);
}

public interface MovieService {
Flux list();

Flux findByRating(String rating);

Mono update(String id, MovieRequest movieRequest);

Mono create(Mono movieRequest);

Mono read(String id);

Mono delete(String id);
}

@Service
public class MovieServiceImpl implements MovieService {

@Autowired
private MovieRepository movieRepository;

@Override
public Flux list(){
return movieRepository.findAll();
}

@Override
public Flux findByRating(final String rating){
return movieRepository.findByRating(rating);
}

@Override
public Mono update(String id, MovieRequest movieRequest) {

return movieRepository.findOne(id).map(existingMovie -> {

if(movieRequest.getDescription() != null){
existingMovie.setDescription(movieRequest.getDescription());
}
if(movieRequest.getRating() != null){
existingMovie.setRating(movieRequest.getRating());
}
if(movieRequest.getTitle() != null) {
existingMovie.setTitle(movieRequest.getTitle());
}

return existingMovie;

}).then(movieRepository::save);
}

@Override
public Mono create(Mono movieRequest) {

return movieRequest.map(newMovie -> {

This is not necessarily an in-depth review, but I see one issue and one improvement:

The CRUD method for delete in your service should combine operators

Currently you have

@Override
public Mono delete(String id) {
    Mono movie = movieRepository.findOne(id);
    movieRepository.delete(id);
    return movie;
}

This is problematic because you return the result of findOne rather than the combination of 1) waiting for the DB to find your movie then 2) getting the ack from the DB that it deleted it. The result of delete is never subscribed to, which means that it is never actually executed! Even if you added .subscribe() there, that could mean you execute both calls in parallel and your controller's response time would be tied to the fetch rather than the delete.

So you need to return a Mono that also triggers and represents completion of the delete.

The problem is that delete in the repository returns a Mono. There is a new Mono.untilOther operator in the upcoming 3.0.6 release of Reactor that will be usable like this:

@Override
public Mono delete(String id) {
    return movieRepository.findOne(id)
        .untilOther(movieRepository.delete(id));
}

This will trigger the findOne and hold on emitting its value until the delete has completed.

But for now release is 3.0.5 so in the meantime here is a workaround:

@Override
public Mono delete(String id) {
    return movieRepository.findOne(id)
        .flatMap(oldValue -> 
            movieRepository.delete(id)
                           .then(Mono.just(oldValue))
        )
        .singleOrEmpty();
}

The idea here is to first go to the DB to fetch the current value, and one this is done trigger the delete inside a flatMap. Once the delete completes (then(...)), we're still inside the flatmap and the value from findOne has been captured within the flatmap context, so we can re-emit it using Mono.just(...).

Now because of flatMap we are now dealing with a Flux, but we know it will not contain more than one value ever, so we can revert to a Mono using singleOrEmpty. Note it could be empty if the key doesn't exist in DB.

which leads me to:

Some CrudRepository methods can return an empty Mono/Flux

This you probably want to represent as a 404, especially for the read endpoint.
You can use .defaultIfEmpty(ResponseEntity.notFound()) for that.

StackExchange Code Review Q#159139, answer score: 6