I have a newly created Springboot service class to handle CRUD operations using the JPA derived classed.

There are many examples of this on the internet, some include existsById checks on delete, update (and get) - some don't.

This is what mine looks like:

public class BirdServiceImpl implements BirdService {

    private final BirdRepository birdRepository;

    @Override
    public Bird create(Bird bird) {
        log.info("Saving new bird: {}", bird.getName() );
        return birdRepository.save(bird);
    }

    @Override
    public Collection<Bird> list() {
        log.info("Finding all birds");
        return birdRepository.findAll();
    }

    @Override
    public Bird get(Long id) {
        return birdRepository.findById(id).get();
    }

    @Override
    public Bird update(Bird bird) {
        return null;
    }

    @Override
    public void delete(Long id) {
        log.info("About to delete bird : {}", id);
        Bird bird = new Bird();
        if (birdRepository.existsById(id)) {
             birdRepository.deleteById(id);
        }
    }
}

QUESTION: Should those checks existsById be in there or should there be some Exception Handling and if so where?

How can one be sure that the delete was successful as it doesn't return anything?

CodePudding user response：

You can use existsById and if it returns true you can delete the specific record otherwise you can throw an exception that it didn't exist.

           if (birdRepository.existsById(id)) {
                     birdRepository.deleteById(id);
                }else {
                     throw new EmptyResultDataAccessException(
                     String.format("No %s entity with id %s exists!", entityInformation.getJavaType(), id), 1)
    );
  }

CodePudding user response：

As per my knowledge two exception is thrown by the deleteById in following cases

1. In case the given id is null IllegalArgumentException is thrown.
2. In case given id is not present in the persistence store EmptyResultDataAccessException is thrown.

There is no need to check using existsById as it will initiate unnecessary query to the persistence store (Database).

My suggestion would be following:

1. If nothing special to be done, except just returning particular HTTP Status in the api response, then do not handle this exception in the Service layer. Two cases are possible here

   1.1 You want to handle every non existing id cases similarly for every services. In that case you want to handle this in the Controller Advice. Return predefined http status and proper error message for the exceptions thrown by the Service layers.

   1.2 You might want to handle deletion failed due to non existence differently, in that case declare a dedicate exception class extended from RuntimeException. Catch the EmptyResultDataAccessException inside the service and prepare a exception object of your newly created exception then throw it. Then handle that exception inside the Controller Advice specially.

Following is how to handle exception in Controller Advice

@RestControllerAdvice()
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {
    @ExceptionHandler(EmptyResultDataAccessException.class)
    @ResponseStatus(value = HttpStatus.NOT_FOUND)
    public ResponseEntity error(EmptyResultDataAccessException ex) {
        return ...; // Build your response body here.
    }
}

2. You might want to do some special things like how my deletion call failed. In that case catch the exception inside the Service and do what you want to do (i.e you may log entry in the activity log that deletion failed for that id). After doing your business task throw appropriate exception (based on point 1.1 or 1.2 whichever suits your case well) which is meant to be handled in the Controller Advice.