Skip to content
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

FIX: Improved error message in the case of SQL constraint violation on foreign keys #4167

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Agnul97
Copy link
Contributor

@Agnul97 Agnul97 commented Jan 10, 2025

Brief description of the PR.
I wanted to return a more specific error message that notifies clients that they are trying to break a fk constraints, to let them know precisely what they are doing wrong. Currently, in such cases, a too general 500 internal persistence error is returned

Description of the solution adopted
With these changes, a 409 error is returned instead, with a message that notifies precisely a fk violation if it's detected. Otherwise, a more general error message is returned if other SQL constraint violations are present

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 38.88889% with 11 lines in your changes missing coverage. Please review.

Project coverage is 16.71%. Comparing base (4e2973c) to head (4ee9ece).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...QLIntegrityConstraintViolationExceptionMapper.java 0.00% 9 Missing ⚠️
...KapuaSQLIntegrityConstraintViolationException.java 50.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #4167   +/-   ##
==========================================
  Coverage      16.71%   16.71%           
  Complexity        22       22           
==========================================
  Files           2039     2041    +2     
  Lines          53018    53036   +18     
  Branches        4453     4455    +2     
==========================================
+ Hits            8860     8867    +7     
- Misses         43755    43766   +11     
  Partials         403      403           
Files with missing lines Coverage Δ
...clipse/kapua/commons/util/KapuaExceptionUtils.java 97.50% <100.00%> (+0.27%) ⬆️
...c/main/java/org/eclipse/kapua/KapuaErrorCodes.java 100.00% <100.00%> (ø)
...KapuaSQLIntegrityConstraintViolationException.java 50.00% <50.00%> (ø)
...QLIntegrityConstraintViolationExceptionMapper.java 0.00% <0.00%> (ø)

@Agnul97 Agnul97 marked this pull request as draft January 13, 2025 14:26
@Agnul97 Agnul97 marked this pull request as ready for review January 13, 2025 15:01
@Agnul97 Agnul97 force-pushed the fix-erorr_message_fk_constraint branch from 7f5d201 to 4ee9ece Compare January 13, 2025 15:20
@Coduz Coduz added the Bug This is a bug or an unexpected behaviour. Fix it! label Jan 13, 2025
@Agnul97 Agnul97 marked this pull request as draft January 14, 2025 15:35
@Agnul97 Agnul97 marked this pull request as ready for review January 14, 2025 15:37
Copy link
Contributor

@Coduz Coduz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are ok, but the fact that it would be better if that was a generic exception that does not strictly relates to SQL and Foreign Keys.

KapuaIntegrityConstraintViolationException with message Entity constraint violation error. This entity relates to other entities and cannot be deleted.

@Agnul97 Agnul97 force-pushed the fix-erorr_message_fk_constraint branch from 89ed3e9 to 933a0f5 Compare January 28, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug or an unexpected behaviour. Fix it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants