-
Notifications
You must be signed in to change notification settings - Fork 243
Categorizing Conflict error codes #1090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some nits/ suggestions
src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, just a question, plus a request to use system.text.json if possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the change!
Why make this change?
To recognize exceptional conditions like insertion of rows with duplicate key as Conflicting operations (return exception with
HttpStatusCode 409
), rather than throwingInternalServerError
. This is just one example, and we can have other conflicting operations in the database as well.What is this change?
Added a set
ConflictExceptionCodes
toDbExceptionParser.cs
which will store the error numbers to be considered for returning exception withHttpStatusCode 409
.How was this tested?
Added regression test to InsertApiTestBase.cs class (overridden in each of the child class), which tries to insert a row with duplicate primary key into the database table and thus fails accordingly throwing exception with 409 status code.
Additional Change:
Till this point, the test result verification expected the actual and expected exception messages to be exactly the same. However, the test added in this PR
InsertOneTestViolatingUniqueKeyConstraint
returned a dynamic exception message changing on every test run for MsSql. For such dynamic cases, we can now have an optionisExpectedErrorMsgSubstr
which if set to true, will only look for a substring (which remains constant among multiple test runs) in the actual error message, rather than expecting them to be identical.Sample Request(s)
Database chosen: MsSql
Request: {
"categoryName": "SciFi",
"pieceid":"1",
"categoryid": "1"
}
Response: {
"error": {
"code": "DatabaseOperationFailed",
"message": "Violation of PRIMARY KEY constraint 'PK__stocks__9CC9595F28BE875B'. Cannot insert duplicate key in object 'dbo.stocks'. The duplicate key value is (1, 1).",
"status": 409
}
}