Thanks Aleks I think this is one of the key things we should try to tackle. As Aleks states, moving to type safe REST resource classes would simplify / reduce the code by a huge amount and also mean that Swagger etc just magically work. No need to code / maintain helper resource classes etc.
Thinking through this, I think there are two ways of doing this: Approach 1 - one API at a time, do the following: Step #1: Move from raw JSON string to a DTO resource class Step #2: Ensure that the community UI as well as the integration tests still work after this. Step #3: Rewrite the integration test to use the Swagger generated client. Approach 2 - one API at a time, do the following: Step #1: Rewrite the integration test to use the Swagger generated client (including fixing any issues with the Swagger resource class) Step #2: Use the Swagger resource class to create the DTO to be used for parsing and remove separate Swagger class Step #3: Ensure the community UI still works The second approach would mean we first need to fix the current Swagger resource classes. This may be helpful (i.e allow us to get the DTO definition right) or it may be throwaway work (if there’s a lot of rework between the resource class vs the DTO definition). Another benefit may be that at least step #1 could be done without breaking any existing clients. But I’m not sure which one would be most efficient. Perhaps we need to try it out and see what makes sense. What do you think? Regards Petri > On 24 Oct 2021, at 00:21, Aleksandar Vidakovic <[email protected]> > wrote: > > Hi, > > ... just as a reminder: pretty much the whole REST resource classes in > Fineract abandoned type safety and keeps the request JSON body in a simple > string variable. That makes it pretty much impossible to extract any > information about the data structures. To get around that Sanyam added > manually in his GSoC project helper classes that have no other functionality > than re-introducing type information about the JSON data structures; he did > this by painstakingly introspecting the payloads. The Swagger descriptor is > generated based on those manually maintained helper classes. I think this was > a good choice given the constraints (minimal code changes, 100% backward > compatibility), but it is very maintenance heavy... and if I think about it > then I am not so sure if these helper classes were on everyone's radar (e. g. > when introducing new REST endpoints)... I didn't check, but I would bet since > Sanyam submitted these changes not a lot of updates happened in that area. > Also: there might have been some small parts of the REST API that were not > mapped at all. If I would bet again then I'd say the Swagger file we have > covers maybe 80% (again, not scientific, could be more, could be less) of the > entire API which is more or less accurate (i. e. isn't missing any > attributes, has all types correct etc.). > > Now that we have a Fineract Java client (code generated based on the Swagger > descriptor) as an official module we could use that client in the integration > tests. That would help with 2 things: reveal any wrong Swagger mappings > immediately and remove a ton of handcrafted boilerplate REST Assured client > code (that makes up probably half of the integration test code); could help > us make the integration tests a bit more appealing. I don't think that the > integration tests - as they are now - cover 100% of the REST API (don't > remember that we have too many pull requests in that area either), but I > think it would be a good start to use the official Fineract Java client... > this would make any missing pieces more visible. > > Mid/long term I think the best solution would be to fix the JSON mapping in > all REST resource classes. Instead of feeding Google GSON with raw JSON > strings and manually mapping it to Java classes we should use the Jackson > parser (pretty much standard in Spring Boot apps and really fast) and let it > do all the de-/serialization and mapping to Java. Again, that would make a > ton of code (10%? 15%?) disappear... code that we don't have to maintain and > test. For that to happen we would need to replace those JSON string blobs > with proper types (DTOs) in the REST resource classes. Once that is in place > then the whole Swagger stuff (or OpenAPI for that matter) is just a case of > simple annotations and we can ditch all these manually maintained helper > classes. When we add new REST endpoints then we just have to remember to put > proper annotations everywhere, easy. Side effect: makes API > requests/responses faster... > > Cheers, > > Aleks > > On Sat, Oct 23, 2021 at 5:27 PM Ed Cable <[email protected] > <mailto:[email protected]>> wrote: > Chinmay, Michael, and Manthan, > > We were running into some questions regarding the swagger definitions. Are > they generated from source code and can be used to generate client libraries? > If so, does it currently cover all of the APIs within Fineract 1.x? > > We have been reviewing the fineract-client email threads > http://mail-archives.apache.org/mod_mbox/fineract-commits/202010.mbox/%[email protected]%3E > > <http://mail-archives.apache.org/mod_mbox/fineract-commits/202010.mbox/%[email protected]%3E> > and gist report > > https://gist.github.com/Grandolf49/f79537436a467dac0baa9458a38290c5 > <https://gist.github.com/Grandolf49/f79537436a467dac0baa9458a38290c5> > > but still had some doubts. > > @[email protected] <mailto:[email protected]> Can you share what additional > questions you have? > > Thanks, > > Ed > > >
