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 > > <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] > <mailto:[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] > <mailto:[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] > <mailto:[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] >> <mailto:[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] >> <mailto:[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] >>> <mailto:[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 >>> >>> >>> >> >
