Thanks Alexander - that’s spot on! It seems that PUT / POST / DELETE don’t work on our HTTP (i.e. non-SSL) endpoint - they magically become GETs. When I switched to HTTPS then the requests worked fine.
Seems like another thing to fix - either stop supporting HTTP altogether, or make sure that the redirects preserve the action. I’ve raised a JIRA for this (FINERACT-1423). Regards Petri > On 31 Oct 2021, at 22:52, Alexander Finnigan <[email protected]> wrote: > > It might be the side effect of redirecting. > > Its a wild guess but if you are sending POST to http://localhost:8080 > <http://localhost:8080/fineract-provider>/fineract-provider > <http://localhost:8080/fineract-provider>.. you will get an http 302 to > redirect your request to https://localhost:8443 > <https://localhost:8443/fineract-provider>/fineract-provider > <https://localhost:8443/fineract-provider> and it became GET for some > reason... > > Try to hit the https://localhost:8443 <https://localhost:8443/> directly. > > Regards > Alex > >> On 29 Oct 2021, at 16:45, Aleksandar Vidakovic <[email protected] >> <mailto:[email protected]>> wrote: >> >> ... 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] >> <mailto:[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] >>> <mailto:[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 >>>>> >>>>> >>>>> >>>> >>> >> >
