... right... that reminds me of another thing: to make the SSL configuration optional... at the moment this is hardwired in the Spring XML configuration, but I would argue in most - production - scenarios the whole SSL stuff is handled by some proxy setup... another reason to fully embrace starter packages and auto configuration...
On Sun, Oct 31, 2021 at 5:48 PM 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.. you will get an http 302 to > redirect your request to *https://localhost:8443 > <https://localhost:8443>*/fineract-provider > and it became GET for some reason... > > Try to hit the https://localhost:8443 directly. > > Regards > Alex > > Sent from my iPhone > > On 29 Oct 2021, at 16:46, Aleksandar Vidakovic <[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]> > 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 >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>
