... great that you got it working... This GET issue is weird... my bet would be that one of the existing filters in Fineract does something funky... or maybe that we have a mix of Spring XML and Java config? ... I used the latest Jersey in a couple of projects (with Spring Boot) and don't remember that I came across a similar behavior.
I'll have a look at this as soon as I can... On Fri, Oct 29, 2021 at 3:45 PM Petri Tuomola <[email protected]> wrote: > Thanks Aleks. I actually got it working with a slightly different > approach, also resolved all the class path issues etc. > > But now stuck with a very weird issue: Jersey receives all requests as > GET, even if you send in POST / PUT / DELETE etc. > > I’ve tried debugging and the request is picked up by Tomcat with the right > method, but once it has passed through the FilterChain the method is always > GET. > > So something funny going on with Jersey / filters etc.. need to debug this > more when I have some time. > > Regards > Petri > > On 27 Oct 2021, at 09:33, Aleksandar Vidakovic <[email protected]> > wrote: > > 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>> >
