FINERACT-2501: Add operationIds and refactor feign methods to fix unstable deduplication numbers generated by swagger#5597
Conversation
e77c0ec to
9e263a0
Compare
|
Looks like changes to WorkingCapitalStepDef weren't in this commit. That's my bad. Repushing now! |
9e263a0 to
1ba2514
Compare
…currently used in tests - added operationIds to all endpoints for Feign methods currently used in these - changed these first because changing other methods seems to break builds since these tests will shift due to items being taken out of the OpenApi spec - added changes to WorkingCapitalStepDef
1ba2514 to
4b81df6
Compare
|
@edk12564 Would you mind to introduce operationId for ALL of our APIs, please? |
Of course! Would it be okay to have 2 PRs though between me and @Ambika-Sony . I have been communicating with her to split the ticket since she wanted to help out. We took a 50/50 split on modules earlier. The other issue is that CI/CD tests on PRs that change all operationIds will fail I think if I don't push this item first, since the tests will still be using the old feign method names to test any new PRs. Should we maybe have 3 PRs? this, the rest of my 50%, and Ambika's work? Happy to do whatever you think is right here! I also saw the self service API got removed, so I'll remove any changes there. I also found a lot of items I missed. I was looking in the wrong build file. Extremely sorry about that. Making edits now. |
|
sure. you can split ;) |
JIRA: FINERACT-2501
Description
According to FINERACT-2501, deduplication numbers in Feign client methods are unstable. More on this below.
The Problem
Fineract uses Swagger/OpenAPI to auto-generate Feign client methods from our endpoints. Since many endpoints share the same method name (e.g., retrieveAll()), Swagger appends a deduplication number to create unique operationIds. For example, retrieveAll35() for SavingsProductsApiResource.retrieveAll().
These deduplication numbers are unstable. When a new endpoint with a duplicate name is added, the numbers can shift. In my testing, after adding a new dummy retrieveAll() endpoint, retrieveAll35() no longer pointed to SavingsProductsApiResource. It actually shifted to retrieveAll36().
Since we hardcode these deduplicated method names in our Cucumber tests, any shift will silently call the wrong API, breaking tests.
To start, the currently used Feign methods must be refactored, and the endpoints must have operationIds added to them. This is because changes in other modules will cause these in use methods to change due to instability. This results in build failures.
Changes
The methodology for naming OperationIds is methodName + entityName + descriptionClause. For example, update() for SavingsProducts that updates using an externalId becomes updateSavingsProductsByExternalId.
Results
All affected tests in integration and Cucumber tests pass.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Your assigned reviewer(s) will follow our guidelines for code reviews.