Sounds great! Looking forwarded to seeing your PR, and then let's work together on this.
In the meantime I'll see if I can fix the rest of the issues with Jersey 2 - I fixed the API path issue, and now it would seem that most of the stuff works. On Wed, Oct 27, 2021, 02:29 Aleksandar Vidakovic <[email protected]> wrote: > ... true, didn't have that on the radar... even simpler than portfolio > clients that I had in mind... and I think with global configuration we > should have a bit more freedom to experiment... > > Cool, let's do that. I can get to that later today or tomorrow if > that works for you... opening a first draft PR and then we see how that > goes? > > Cheers > > On Tue, Oct 26, 2021 at 5:21 PM Petri Tuomola <[email protected]> > wrote: > >> That makes sense - it should be safer and more conservative. >> >> We can start anywhere but my suggestion would be something that is as >> simple as possible, but has integration tests built for it. That way we >> could focus on getting the structure of the code with fineract-client / >> Jackson right, rather than worrying about the intricacies of the business >> logic. And once we’ve done one, then the rest should follow the same >> pattern. >> >> How about something like the /configurations ie GlobalConfigurationApi? >> Doesn’t get much simpler than that (i.e. no business logic at all) but >> there are integration tests for this. >> >> Regards >> Petri >> >> On 26 Oct 2021, at 18:18, Aleksandar Vidakovic <[email protected]> >> wrote: >> >> ... the second approach sounds like a good strategy (as much as I'd like >> to overhaul things using the first approach)... taking things from the >> integration tests is a bit more conservative and makes sure to not drop >> anything... and in some cases we might be able to even extend the >> integration tests and cover more of the REST API (should be easier with the >> Java client lib vs. Rest Assured boilerplate code). >> >> Just to say... I think I like approach 2... where do we start? With >> something small? Portfolio client? >> >> Let me know... >> >> Cheers, >> >> Aleks >> >> On Tue, Oct 26, 2021 at 6:52 AM Petri Tuomola <[email protected]> >> wrote: >> >>> 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]> 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 >>>> and gist report >>>> >>>> https://gist.github.com/Grandolf49/f79537436a467dac0baa9458a38290c5 >>>> >>>> but still had some doubts. >>>> >>>> @[email protected] <[email protected]> Can you share what additional >>>> questions you have? >>>> >>>> Thanks, >>>> >>>> Ed >>>> >>>> >>>> >>>> >>> >>
