-
Notifications
You must be signed in to change notification settings - Fork 2.2k
FINERACT-2096: Fix duplicate key value exception #3933
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
FINERACT-2096: Fix duplicate key value exception #3933
Conversation
fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java
Show resolved
Hide resolved
adamsaghy
left a comment
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.
Please kindly check my review!
eba07a6 to
4250074
Compare
Changes applied please review |
4250074 to
09a7de6
Compare
| CommandSource initialCommandSource = getInitialCommandSource(wrapper, jsonCommand, maker, idempotencyKey); | ||
| return commandSourceRepository.saveAndFlush(initialCommandSource); | ||
| } catch (JpaSystemException jse) { | ||
| if (jse.getRootCause().getMessage().contains("UNIQUE_PORTFOLIO_COMMAND_SOURCE")) { |
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.
Please use null checks! Root cause and message both can be null!
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.
Done! null checks added
f30692c to
5524ff3
Compare
| return commandSourceRepository.saveAndFlush(initialCommandSource); | ||
| } catch (JpaSystemException jse) { | ||
| final String message = (jse.getRootCause() != null) ? jse.getRootCause().getMessage() : null; | ||
| if (message != null && message.contains("UNIQUE_PORTFOLIO_COMMAND_SOURCE")) { |
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.
I reckon we need to slightly modify to work with MariaDB and Postgres as well:
message.toLowerCase().contains("unique_portfolio_command_source")
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.
Done!
adamsaghy
left a comment
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.
Please kindly check my review!
5524ff3 to
ad2f7d8
Compare
adamsaghy
left a comment
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
Description
Fix duplicate key value exception to be managed as
IdempotentCommandProcessUnderProcessingExceptionwith HTTP code 425FINERACT-2096
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per https://github.com/apache/fineract/#pull-requests
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.