Skip to content

Conversation

@alberto-art3ch
Copy link
Contributor

Description

Fix duplicate key value exception to be managed as IdempotentCommandProcessUnderProcessingException with HTTP code 425

FINERACT-2096

Screenshot 2024-06-13 at 7 13 51 a m

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.

Copy link
Contributor

@adamsaghy adamsaghy left a 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!

@alberto-art3ch alberto-art3ch force-pushed the fix/idempotency_key_duplicate branch from eba07a6 to 4250074 Compare June 14, 2024 14:56
@alberto-art3ch
Copy link
Contributor Author

Please kindly check my review!

Changes applied please review

@alberto-art3ch alberto-art3ch force-pushed the fix/idempotency_key_duplicate branch from 4250074 to 09a7de6 Compare June 14, 2024 15:25
CommandSource initialCommandSource = getInitialCommandSource(wrapper, jsonCommand, maker, idempotencyKey);
return commandSourceRepository.saveAndFlush(initialCommandSource);
} catch (JpaSystemException jse) {
if (jse.getRootCause().getMessage().contains("UNIQUE_PORTFOLIO_COMMAND_SOURCE")) {
Copy link
Contributor

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! null checks added

@alberto-art3ch alberto-art3ch force-pushed the fix/idempotency_key_duplicate branch 3 times, most recently from f30692c to 5524ff3 Compare June 14, 2024 18:31
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")) {
Copy link
Contributor

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")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@adamsaghy adamsaghy left a 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!

@alberto-art3ch alberto-art3ch force-pushed the fix/idempotency_key_duplicate branch from 5524ff3 to ad2f7d8 Compare June 17, 2024 14:47
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

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

LGTM

@adamsaghy adamsaghy merged commit edc7fff into apache:develop Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants