Check if resource exists before inserting to the database or wait until the DAO throw the Exception?

Question

I have two paths:

• /students
• /students/{id}/addresses

...with the following behavior:

POST to /students - 201 Created (if successfully created the Student) POST to /students/{id}/addresses - 201 Created (if successfully created the Address under the requested user) Now, imagine a situation where the client POST to /students/12/addresses, but there's no Student with id = 12.

How should I handle this scenario?

Should I insert without any check and wait for DAO to throw the Exceptions or, before inserting, I should check for Student's existance?

1st option:

StudentController.java

@Autowired
private AddressService addressService;

@PostMapping("/students/{id}/addresses")
public ResponseEntity<?> handleRequestOfCreateAddressToStudent(@PathVariable("id") Long studentId, @RequestBody Address address) {
    address.setStudent(new Student(studentId));
    Address createdAddress = addressService.saveAddress(address);

    URI location = ServletUriComponentsBuilder.fromCurrentRequest().path("/{id}").build(createdAddress.getId());
    return ResponseEntity.created(location).build();
}

AddressService.java

@Autowired
private AddressRepository addressRepository;

public Address saveAddress(Address address) {
    // omitted for brevity...
    return addressRepository.save(address);
}

This way, since I'm using Spring framework, the line return addressRepository.save(address); would throw a DataIntegrityViolationException, because the constraint is violated (there's no Student with such ID to bind with that Address). And, since I need to send the response to the client, I would have to translate the exception that is throwed.

2nd option:

StudentController.java

@Autowired
private StudentService studentService;

@Autowired
private AddressService addressService;

@PostMapping("/students/{id}/addresses")
public ResponseEntity<?> handleRequestOfCreateAddressToStudent(@PathVariable("id") Long studentId, @RequestBody Address address) throws StudentNotFoundException {
    Student student = studentService.findById(studentId);

    if(student == null)
        throw new StudentNotFoundException("message");

    address.setStudent(new Student(studentId));
    Address createdAddress = addressService.saveAddress(address);

    URI location = ServletUriComponentsBuilder.fromCurrentRequest().path("/{id}").build(createdAddress.getId());
    return ResponseEntity.created(location).build();
}

This way, I don't need to translate the Exception, as I already know that the Student does not exist so that I can throw a custom exception.

My thoughts in both approaches:

1. Without checking, I do reduce the code and the number of times I trigger my database;
2. Checking, I do know exactly why I'm unable to add the Address (the Student does not exist) and can throw a custom exception without having to translate it;

Which one is the best approach to this scenario?

Should I wait for the DAO to throw the Exception, or should I prevent it from throwing by checking if the Student exists?

Answer 1

You should never use exceptions as a flow-control device. Check for the existence of the Student first, then add the address. As you noted, this give you more control over the messaging you send, plus you keep all the logic in the same flow (as opposed to returning messages from both the main block and the exception handler).

Answer 2

It may certainly be a valid concern to reduce latency and demand on the backend for performance reasons. But that pales in comparison to correctness:

TOCTOU, fully written as Time-Of-Check-Time-Of-Use, is a very well known error coming from thinking in a strictly linear model (like cooperative multi-threading in Windows 3.11 was) while you actually have independant, concurrent, non-synchronized actions.

Well, unless you also support deleting of students, at least if you found the student, it will still be there later. Can you guarantee that? Also, that it won't ever change? Just go for the more performant and always correct approach.

Some say exceptions are a performance-hog, and depending on language and implementation they might not be wrong. Measure it, and if that's the case, error-codes are also an option. There is a reason many libraries feature various TryXYZ() functions in addition to ParseXYZ(), as failure might be common.

Answer 3

That business exception should be thrown by your service layer. Your persistence layer should throw a data exception. Then, in your controller, you'll only translate that business exception to an HTTP response.

This is an exceptional case (who's requesting to your REST API about a non existing student?) so the handling should be as such.

Answer 4

If we want to be strict towards HTTP I will return an HTTP 404 Not found.

Since the URL doesn't point to an existing location you can't find it, thus 404.

For your service layer if you want a detailled exception handling, then you should have a StudentNotFoundException (or eventually a more generic EntityNotFoundException) which you would translate to 404. For the implementation, whether you try to search for student before inserting of try to insert straight into the table doesn't matter a lot. Unless you're going to have hundred of thousands tries of inserts. If you need ever need a classic "import initialization data" you will likely need a proper code just to handle that specific case, no need to have your main service worrying about that one case.

Note that here using Exception is not flow control, it is really an exception that lead to answer to a standard error.