edk12564 opened a new pull request, #5495: URL: https://github.com/apache/fineract/pull/5495
JIRA: FINERACT-274, FINERACT-672, FINERACT-673, FINERACT-674, FINERACT-675, FINERACT-676 Pictures coming soon. ## Before According to FINERACT-274, Fineract was creating orphaned tables whenever a datatable was rejected in the maker checker process, taking up database storage and polluting namespace. To reproduce the problem: 1) Startup a MariaDB instance of Fineract 2) Make sure global maker-checker is on. 3) Make sure maker-checker is on for datatable creation 4) Login as a maker (not a super-user) user to frontend dev server 5) Create a new datatable 6) Login as a checker user. 7) Deny the datatable creation. 8) Examine the database and find orphaned table. 9) Reconfirm by trying to create datatable with same name, which displays an error message: data table already exists. ## Changes The problem was found to affect the group, center, loan account, office, and savings accounts levels. After diagnosing the issue, I found that the problem was that these were all data table creation events. It turns out that MariaDB does not add DDL (data table changes) SQL queries to a transaction, meaning when the maker-checker system rolls everything back for checking, the DDL actually executes and makes the table. This means that when the data table is rejected, the orphaned data table exists in the database. PostgreSQL includes DDL as part of the transaction boundary, meaning it is not an issue when PostgreSQL is used. To fix the issue, I added a cleanup service after every maker-checker rejection that checks if the rejection was a data table rejection. If that was the case, it cleans up the data table in question. If not, it continues as normal. This structure allows us to keep as much original logic as possible, while working in both PostgreSQL and MariaDB use cases. To test my changes, I also added an integration test that tests the process of data table creation, maker-checker events, and another data table creation to determine if the data table was properly removed. I do this over the group, center, loan account, office, and saving account levels to make sure they don't produce the bugs mentioned in the various tickets. I tried to follow convention as much as possible. Feedback is welcome! ## After Tests all pass. Reproducing the issue no longer results in a "data table already exists" error. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [ ] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#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](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [ ] Follow our [coding conventions](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 - [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
