Made use of the proper ConflictingEntityException and ObjectNotFoundE… #191
Conversation
…xception on those use cases, and introduced try-with-resources for the used DslContext.
|
Kudos, SonarCloud Quality Gate passed! |
| { | ||
| return DslContexts.obtain(mizoolConnectionProvider).executeDelete(record); | ||
| int deletedRows = dslContext.executeDelete(record); | ||
| if (deletedRows != 1) |
There was a problem hiding this comment.
This is changing the behaviour in situations where we want to delete rows but none are present.
I am still not comfortable with that approach to handle these situations as erroneous since during a regular cleanup there might be a delete operation on already empty tables. I recall having that discussion already once but do not remember the outcome.
What are your thoughts? @JanDoerrenhaus @bannmann
There was a problem hiding this comment.
So far, we have been following the pattern, that if you DELETE an item that does not exist, a 404 is returned, not a 200. This class is meant to be used for the standard CRUD operations of REST stacks, not generically for each and every SQL delete that may ever happen. So if I see a "regular cleanup" using this class, I would question the sanity of the author.
There was a problem hiding this comment.
What Jan said.
Sure, there are cases where we don't care if rows were already deleted (or never existed in the first place). For those cases, we simply create a separate store with a behavior-revealing name (maybe FooCleanupStore or FooEnsureNoneExistStore for a Foo table).
|
FYI @gregorxiv |
…xception on those use cases, and introduced try-with-resources for the used DslContext.