1

Below is a web service which gives the latest version among all books with a given Id. //Resource controller

@Path("latestVersionBook/{id}")
@GET
@Produces(MediaType.APPLICATION_JSON)
public Response getLatestVersionBook(long id){
     bookList=service.getLatestVersionBook(id)
     return Response.ok(bookList).build();

//backend Service Below is the service method that uses jpa to get the books

public getLatestVersionBook(long id){
    Query query = entityManager.createQuery("from Books where id = :id");
    query.setParameter("id", id);
    return query.getResultList().get(0);
}

If the id is an invalid (for which no books exist in the table) this code will throw NullPointerException. Since the transition from view layer would be UI presents list of IDs-->User sends requests for one of this ids. So it is not a usual case in which the returned list will be null.Should I check for null or let the code throw NullPointerException and let the exception mapper generate the response as BAD_REQUEST.

If i check for null/return empty list then I have to check for the same in resource controller too to generate a Response with Bad_Request.

if(!bookList.isEmpty())
    return Response.status(Status.BAD_REQUEST).build();
else  
    return Response.ok(bookList).build();

Also It doesn't appear to be a good thing to put such validation(which need DB access) at a level of input validation available in Jersey /Spring similar frameworks as I would be doing the same select in the service and during custom validation to check if Id is present.

What is a good practice to do such validations?

PS: I am not sure if the above code will give book object with the latest(last) id ,if id is an auto-increment field.And the query can be modified to fetch book only for last Id.However for above question assume this works

Paul Samsotha
  • 205,037
  • 37
  • 486
  • 720
rakesh99
  • 1,234
  • 19
  • 34

2 Answers2

1

In this case we generally send a 404. The URL is the identifier for the resource. If part of the URL is used as an identifier for determining the resource, then the appropriate reply for a resource not being found by that identifier, is a 404 Not Found.

Generally, personally what I do is just throw an exception, and let Jersey handle it. For instance

if (book == null) {
    throw new WebApplicationException(Response.Status.NOT_FOUND));
}

Jersey will handle this exception for us and send a 404 status. We could easily handle the Response ourselves, but it's just a habit I've gotten into, to throw the exception. AFAIK, there's no real convention/best practice, as to which one is better, but sometimes you are tied to an interface contract, and returning a Response is not possible. In this case, throwing the exception would make sense.

Here are some resources you might find interesting:

Community
  • 1
  • 1
Paul Samsotha
  • 205,037
  • 37
  • 486
  • 720
  • Does allowing the npe to be thrown and let exception mapper generate a 404 has any disadvantage over this? besides the turnaround time of a malformed request would a little more..which i feel isn't really a downside – rakesh99 Jun 16 '15 at 21:34
  • If the performance is big concern, you should test it, but like I said, in your case, you can easily return `Response.status(Response.Status.NOT_FOUND)`. The point is the status that should handle this type of problem request. – Paul Samsotha Jun 16 '15 at 21:37
  • I already have a exception mapper to handle other possible exceptions so having null checks in controller would be unnecessary in this case.Right? – rakesh99 Jun 16 '15 at 21:41
  • 1
    I mean an Exception Mapper to handle NullPointerException, and return a 404 doesn't make much sense, since a NullPointerException could be cause by anything. It could be a system error, in which case you should send a 500. If you are going with the exception way, we still need to make sure the right status is sent for the right reason. Using WebApplicationException is useful. If you are using Jersey 2.x, there is a whole [hierarchy of exceptions](http://stackoverflow.com/questions/26796363/catch-all-exceptions-and-also-return-custom-errors-in-jersey/26798284#26798284) with built in statuses – Paul Samsotha Jun 16 '15 at 21:46
0

That would be an approach:

if(bookList != null && bookList.size() > 0){
   //HTTP 200 OK
   return Response.status(Response.Status.OK).entity(bookList).build();
}
else{
   //HTTP 204 NO CONTENT
   return Response.status(Response.Status.NO_CONTENT).entity("Book not found").build();
}
molabss
  • 1
  • 1
  • 4
  • can you also please explain the benefit of this approach over let npe be thrown and exception mapper generating the response?If exceptions are costly,serializing a dummy/null object to json would be same.I forgot to @Produces to the method and what is the benefit of reducing the turn around time of a malformed request? – rakesh99 Jun 16 '15 at 21:18
  • In my humble opnion I think unnecessary throw exception simply because a record not found in the database . So I use the above approach , less complexity. The use of a 404 status seems appropriate for this case , as cited by peeskillet . But I prefer to leave it strictly to when a static resource is not found. As the own HTTP 1.1 specification. The book ID is dynamic , so for this case 'd rather send NO CONTENT status code. Hugs [ ] – molabss Jun 17 '15 at 00:20