Unstable Feign method names from OpenApi deduplication numbers (relevant for Cucumber testing)

[Edward Kang <ed...@gmail.com>]

Hi everyone!

After the most recent Fineract call, I started working alongside many
others on the Cucumber testing initiative. While I was working, I began
investigating the implementation of a test core for our savings products,
so we can extend our tests to that area. However, I might have come across
something interesting here that can lead to issues. My write up on this is
below:

While writing some savings product generation logic I found we are required
to use retrieveAll35(), which is the Feign client representation of the
retrieveAll() method from the SavingsProductsApiResource. However, since we
use Swagger to autogenerate the Feign method name, the 35 is added as a
part of Swagger’s deduplication process, since no operationId is allowed to
be the same in Swagger’s eyes.

After some reading on this topic, it turns out that this deduplication
number can be unstable, and might lead to failing tests in the future. This
is because the deduplication number can change when a new endpoint is
added, if tags are accidentally modified, or due to other unidentified
issues. I think since we are hardcoding our method names in our tests using
these deduplication numbers, this can lead to issues of calling the wrong
API with our feign client.

More on this here:

https://github.com/springdoc/springdoc-openapi/issues/2481

https://github.com/springdoc/springdoc-openapi/issues/675

https://github.com/OpenAPITools/openapi-generator/issues/19865

https://github.com/springdoc/springdoc-openapi/issues/96

To further confirm this issue, I created a new retrieveAll() endpoint to
Fineract, and after examining the OpenApi spec in fineract.yaml in the
build, I found that the reference to retrieveAll35 had changed and was no
longer pointing to the SavingsProductsApiResource. It seems new endpoints
of the same name can get added in the middle of the list for deduplication
ordering. This will break future tests that implement these types of
functions.

Also pointing out that it seems the reason this didn’t affect the loan
module tests so far is because most of the loan module methods are named
uniquely with names similar to retrieveAllLoanProducts. And in other cases,
we ended up dodging the issue. For example, I investigated the recent
loan-originator's retrieveAll endpoint added, in the current fineract.yaml,
the retrieveAll in that endpoint got assigned retrieveAll28(). It turns out
there might have been a shift, but retrieveAll28+ might not be used
anywhere, probably meaning we dodged the issue temporarily. The fact that
it got inserted at retrieveAll28, while retrieveAll40 exists, is also
telling.

Attached below I have fineract.yaml files depicting OpenApi operationIds
before and after adding my new dummy endpoint. You can see that the dummy
endpoint must get added somewhere in the middle or at the front, and causes
a shift in operationId for the loan-originator module.

I am proposing a new ticket (
https://issues.apache.org/jira/browse/FINERACT-2501) that will add
operationId arguments to our @Operation tags in each ApiResource. This
should give us exact controllable names for our Feign methods, while not
breaking our current implementations across the codebase. Below I have an
example. I think this should be especially impactful for people that are
working on Cucumber testing, since they will be using Feign client for a
lot of new modules that don't have unique endpoint names.

What does everyone think about this? If confirmed an issue and with
permission, I can begin working on a fix right away!

-- 
Edward E. Kang
edward.kang12@gmail.com
972-768-6940