FINERACT-274: Not able to create the same datatable which was rejected by the maker checker before

[edk12564]

JIRA: FINERACT-274, FINERACT-672, FINERACT-673, FINERACT-674, FINERACT-675, FINERACT-676

Problem

According to FINERACT-274, when a new datatable was rejected in the maker checker process, it creates an orphaned table by mistake, forcing developers to delete the table manually in their database. I've gone ahead and fixed this issue by adding a datatable cleanup service for maker checker rejections. More on this below.

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 during data table creation events, the transaction in maker-checker gets rolled backed. However, it turns out that MariaDB does not add DDL (data table changes) queries to a transaction, meaning when the maker-checker system rolls everything back for checking, the DDL actually executes and makes the table anyways. 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, but I may have missed some things so feedback is very welcome!

Results

Tests all pass:

For MariaDB:

For PostgreSQL:

Reproducing the issue no longer results in a "data table already exists" error:

Demo video is too big for Github, but here is the list of tables after submitting the maker command. You can see the test table.

And here is the list of tables after rejecting with the checker user. You can see there is no test table.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Write the commit message as per our guidelines
• 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 our 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". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

[IOhacker]

LGTM

[adamsaghy]

I dont think any of the changes are needed.

The attached integration tests are green without messing with transaction boundaries, neither having the "cleanup".

The maker-checker is fully rolled back, so no left over data. You can create datatable if it was rejected priorly.

Kindly asking you to double check!