# Most efficient way to check if user is allowed to update an object

So I have an user which can create his own Locations.

I implemented a webservice in order to add and update locations. If the user is not the owner of the location i want to throw an appropriate exception so that the user knows whats wrong.

The current method looks like this:

private void updateLocation(String loginToken, MapLocation ml) throws Exception{

PreparedStatement stmt = conn.prepareStatement("SELECT username FROM Location WHERE idLoc=?");
stmt.setInt(1, ml.getLocationId());
ResultSet rs = stmt.executeQuery();
while (rs.next()) {
}
throw new Exception("no location found!!"); //appropriate exception

throw new Exception("you are not the creator of this location!"); //appropriate exception

PreparedStatement stmtVerification = conn.prepareStatement("UPDATE Location SET positionX=?,positionY=?"
+ " WHERE idLoc=?");
stmtVerification.setDouble(1, ml.getPosX());
stmtVerification.setDouble(2, ml.getPosY());
stmtVerification.setInt(3, ml.getLocationId());
stmtVerification.executeUpdate();

disconnectFromMySQL();
}


The second method I implemented is shorter but the errors are not clear:

private void updateLocationV2(String loginToken, MapLocation ml) throws Exception{
throw new Exception("username is not the same as username of maplocation object"); //not sure if neccesary

PreparedStatement stmtVerification = conn.prepareStatement("UPDATE Location SET positionX=?,positionY=?"
+ " WHERE idLoc=? and username=?");
stmtVerification.setDouble(1, ml.getPosX());
stmtVerification.setDouble(2, ml.getPosY());
stmtVerification.setInt(3, ml.getLocationId());

int executeUpdate = stmtVerification.executeUpdate();
if(executeUpdate==0)
throw new Exception("update failed. either wrong username or idLocation is wrong"); //not clear what is the error BUT shorter
disconnectFromMySQL();
}


So yeah, now I have two methods which update my location but I am not sure which of them is the best. The first method has the advantage that the errors are clear but its slower since I have two queries (select and update). the second method is faster but the errors are not clear. WHich one of them is better, or is there even a better solution?

EDIT: This is my webservice method which calls the update function:

@PUT
@Path("/update")
@Consumes({MediaType.APPLICATION_JSON})
public Response update(String token, MapLocation ml) {
Response r = Response.status(Response.Status.OK).entity("location updated").build();
try {
isUpdated = db.updateLocation(token,ml);
} catch (Exception ex) {
}
return r;
}

• Without seeing the rest of the class it is very hard to tell which is the better of the two functions. It also depends on what the requirements for the implementation are. The question is also unclear because there could be a third way that is best. – pacmaninbw Jun 15 at 12:40
• @pacmaninbw there are no requirements. I am making my own project. So right know I dont know whats the best way to make an update. Yes there could be better solutions but i dont know which is the best – Savan Luffy Jun 15 at 13:29
• @SavanLuffy even if it's your own project you have requirements. If you don't know them you should think them out :) – IEatBagels Jun 18 at 17:36

Soo ... I'm not quite sure how to formulate this without it sounding brash, but... Both of these methods are less than ideal...

Let's follow the behavioural flow to see what issues this code has:

• The webservice portion you provide only understands PUT requests.
Conventionally, PUT requests are used to create objects, not to update them. Instead one would expect a POST endpoint for that.
• You first create an OK response and then overwrite it in case something fails. That's somewhat... annoying in comparison to the simpler direct return.

try {
db.updateLocation(token, ml);
return Response.status(Status.OK).entity("location updated").build();
} catch (Exception e) {
}

• All this code assumes that the user gets to see the Exception message.

• Considering that the webservice code does not compile (because the declaration for isUpdated is missing) I hope that you don't actually pretend that every Exception ever is the user's fault by throwing a HTTP 400 to them...

The root cause of your conundrum is that you're not using the power of widely used java sql abstractions. Instead of manually writing SQL statements and performing updates, you should really make use of JPA and Entities.

If you had these, all of this would boil down to the following simplified code:

private void updateLocation(String username, MapLocation update) {
MapLocation stored = em.find(MapLocation.class, update.getLocationId());
if (stored == null) {
throw new LocationDoesNotExistException();
}
throw new IllegalLocationUpdateException();
}
em.persist(update);
}


Note that the codesample above makes some simplifying assumptions about transaction management and the class hierarchy of the Exceptions it throws.

Note that I'm suggesting to rewrite your webservice method to something like:

public Response update(String loginToken, MapLocation request) {
try {