Hi Petri, ... here's the link to the Jersey 2.x configuration: https://github.com/FITER1/fineract-ng/blob/feature/01-migration-eclipse-link/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/boot/JerseyConfiguration.java
I'm saving some typing with lines 53, 81-87 (aka get all classes that have a "@Path" annotation, iterate and add to Jersey); the rest is for exception mappers etc. Let me know if that works - it should... if not then just ping and I'll have a look at this. Cheers On Wed, Oct 27, 2021 at 1:38 AM Petri Tuomola <[email protected]> wrote: > 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 >>>>> >>>>> >>>>> >>>>> >>>> >>>
