stablekernel/aqueduct

Attempting to update a db row which does not exist will not throw but rather returns null / empty

Closed this issue · 2 comments

When using the update() method of the Query object, if you specify a WHERE filter for which no existing data satisfies, the query is run on no data (as it should) but then returns an empty list to the user. This seems vague.

The updateOne() method returns null which is even more vague. Should these not throw an exception seeing as an update query which fails to actually update anything is in fact, a failure?

Pending favorable discussion on this, I am happy to implement exception handling on this and make a PR.

For update returning empty list is the only correct behavior. It always returns all updated records as you did not updated any an empty list is "all" you have updated.

As for updateOne, you can argue that it could be an exception, the problem is that you will have to change the fetchOne also. I think returning null is better as a conditional check is much easier to do then catching an exception:

final q = Query<User>(context)..where((u) => u.id).equalTo(1);
final user = await q.fetchOne();
if(q == null){
  throw "No user found";
}
// do something with the user
final q = Query<User>(context)..where((u) => u.id).equalTo(1);
try {
  final user = await q.fetchOne();
  // do something with the user
} catch on (e) {
  // do something else maybe throw another exception 
}

On top of that you could create extention on Query:

extension OneOrThrow<T extends ManagedObject> on Query<T> {
  Future<T> fetchOneOrThrow() async {
    final result = await fetchOne();
   return result ?? throw Response.notFound();
  }

  Future<T> updateOneOrThrow() async {
    final result = await updateOne();
    return result ?? throw Response.notFound();
  }
}

Fair enough. Perhaps documentation is the better route then. I was misinterpreting the empty return as something having silently failed. Did I miss something in the existing documentation which defines this behavior?