... @Petri Tuomola <[email protected]> I think I can help with getting Jersey 2.x to start in the Spring context... have a solution that is only a handful of lines... let me know where we should coordinate... any open PR?
On Tue, Oct 26, 2021 at 6:37 AM Petri Tuomola <[email protected]> wrote: > Of course! If someone has already done the work to port Fineract to > support Postgres - even including supporting migration from MySQL - it > would be great for this to be contributed back to the codebase. Happy to > wait for this to be merged before attempting any of the other changes. > > Aleksandar - I started looking into the Jersey upgrade. Getting things to > compile was pretty straightforward, but for some reason I haven’t yet > managed to get Jersey servlet to start serving any of the requests, I just > get 404. The configuration class does get called but for some reason the > servlet is not getting registered correctly. If you have your lines of code > that made this to work, it would be great to reuse them and save the effort > of having to debug where this goes wrong. > > But I don’t think this Jersey work will interfere in any way with the > Postgres changes. > > Regards > Petri > > On 26 Oct 2021, at 06:12, James Dailey <[email protected]> wrote: > > Alexander and Petri - > > Thank you! > > Hey, on the topic of Postgres, I know that Alberto, Victor, and Istvan > have been discussing how to follow up on tickets that have been there for a > year. I'd like to propose that we agree on an approach here and get to > work. Victor and his colleague Alberto has indicated to me that he's > willing to bring in Liquibase as a way to manage the migration from MySQL > to Postgres. And, I believe that more than one person has argued to allow > the option of either mySQL or Postgres, via a configuration or build > /switch. Liquibase is licensed as Apache2.0 and Postgres connector is BSD > license, so already Apache compliant, so that's good. > > While there may be some interactions with the components that you want to > revise, I understand from Alberto that migrating to Postgres would help > solve some of the performance issues and potentially resolve problematic > SQL statements, charset encoding, timezone settings, etc... thus bringing > some overall improvements. > > So, checking in with you two - making sure that if Alberto is able to > bring those on, you can review and maybe hold off on a bunch of component > changes just yet? (i.e. on the theory that too much change makes debugging > problematic). Update to Jersey is ok? > > Thanks, > James > > On Sun, Oct 17, 2021, 3:17 AM Aleksandar Vidakovic < > [email protected]> wrote: > >> Hi Petri, >> >> ... agree with the low hanging fruit... those parts (upgrade Jersey, >> embrace auto-configuration) I can easily re-do/extract from my experiments. >> As for the more intrusive refactorings: there are so many code changes and >> I didn't merge upstream for 2 years or so... I think those parts are not >> really useful, and maybe I would apply a different strategy, namely pick >> one package at a time and create smaller chunks of changes. >> >> So, if you want we can split up the low hanging fruits to get the party >> going... which gives everyone some time to build their opinion about the >> more invasive changes and add more ideas/concerns. >> >> Concerning splitting the code into modules: you are right with the >> cross-dependencies... they are a bit of a pain and in a perfect world we >> would attack this issue immediately (aka create self contained modules with >> ideally no cross dependencies to other Fineract modules)... but that thing >> is a major undertaking and would take more time; I tried this with >> portfolio client and ran immediately into a chain of refactorings that >> touched more than I intended. I think we could still split up the code into >> separate JARs, accept the cross dependencies as they are and just take care >> of any eventual circular dependencies (those obviously won't work with >> modules). Like this, the change is more or less mechanical: create a Gradle >> sub-module per package that we identify as a "module", copy-paste code into >> a new location, and make sure that that module references all needed cross >> dependencies/modules, make sure sources compile, create pull request >> (rinse, repeat). I think this would be - if not perfect - still a nice to >> have if it's only for an improved developer experience (we won't need to do >> hard-forks of the upstream project anymore for customizations)... once the >> code is available in smaller pieces I think it will be easier to reason >> about specific modules, it's dependencies and any improvements that we want >> to do. And as you said: later we can take module by module and make them >> more independent which is also easier to vote on, review, create PRs >> etc.... there might be modules that we can refactor aggressively, because >> there are no (or not too many) customizations out there (and therefor won't >> break any critical code); there might be other modules (loan?) that might >> have a lot of customizations out there and will take a bit longer to >> materialize (with some heads up for the community to prepare themselves and >> some proper release planning aka major version increment). >> >> As for your concerns about development and refactoring in a multi-module >> project: if we would follow above strategy then refactorings across modules >> are still possible as all modules will reside in the same Git repo/main >> project (IntelliJ does this quite well); so with this minimal approach we >> kind of get the best of both worlds... we'll have multiple modules while >> technically Fineract as a whole is still a monolithic application with all >> the code visible (you don't have to load multiple Git repos like in >> Fineract CN)... it would be a first step without being too disruptive. I >> had projects that looked like that and they worked quite ok. >> >> If you have any preferences... eliminate Spring XML... or Jersey >> upgrade... your choice... I have a couple more smaller improvements >> accumulated (like faster Tomcat connector configuration, disable tenant >> feature for more performance in single tenant installations...) that would >> be easier to introduce when some of the cleanup happens before. >> >> Cheers, >> >> Aleks >> >> P.S.: ... there's another refactoring/improvement I'd like to throw in >> here which is kind of small, but potentially touches a lot of files... we >> are using the tenant utility class in pretty much all cache annotations to >> ensure that the tenants' cache data is separated; we could achieve the same >> thing just by defining a cache key generator bean in a Spring Java config >> and could get rid of all those SpEL references... would make the >> annotations cleaner and easier to maintain. Just not sure how to classify >> this one... kind of low hanging fruit, but with some major git changes... >> >> On Sun, Oct 17, 2021 at 2:30 AM Petri Tuomola <[email protected]> >> wrote: >> >>> (Cc’ing the dev list to keep everyone in the loop) >>> >>> Aleksandar - thanks - that’s brilliant! This is exactly what I had in >>> mind with my last paragraph re simplifying / removing boilerplate code. >>> It’s great to hear you’ve already thought through the different aspects of >>> this, and even tried it out so you know what the challenges are. Excellent >>> news! >>> >>> Do you happen to have any of the code lying around from your previous >>> refactoring attempt? If yes, would be great to see if we can extract some >>> PRs from that to address the low hanging fruit (e.g. Jersey upgrade and >>> Spring Boot config)? If not, I can see if I can find some time to look into >>> these in the next couple of weeks. >>> >>> Personally I think it would be great to start with some of the simpler >>> things - Jersey, Spring Boot, perhaps the Postgres support - and then move >>> onto clearing the manual JSON mapping. After these we would already be in a >>> much better place. The domain / DTO work sounds more intrusive but perhaps >>> that can then be tackled one package at a time, as you suggest. >>> >>> The only thing I was not quite sure about was the benefits of splitting >>> this into multiple JARs. The code looks so tightly coupled that I’m >>> wondering if it’s possible to separate modules that could actually evolve >>> independently? If not, then having to handle multiple JARs is just going to >>> add pain to the refactoring… >>> >>> Regards >>> Petri >>> >>> On 16 Oct 2021, at 18:00, Aleksandar Vidakovic < >>> [email protected]> wrote: >>> >>> Hi Petri, >>> >>> ... agree with what you wrote... some additional notes from some >>> previous attempts of mine to reduce the boilerplate: a while back (2 >>> years?) I went down the rabbithole and refactored Fineract 1.x >>> "aggressively" just to see how hard/easy it would be to do this and if it >>> would be possible to keep backward compatibility: >>> >>> - I used Lombok all over the place >>> - introduce Mapstruct for more convenient (read: less manual code, >>> more code generation) mapping between api and domain layer >>> - upgraded Jersey from 1.x to 2.3 >>> - embrace Spring Boot starter packages (aka auto-configuration) as >>> much as possible and remove all old-school Spring XML config (and any >>> home >>> config - e. g. JDBC configuration - that can be replaced with an >>> auto-configuration equivalent) >>> - experimented with replacing the default Tomcat container with >>> something more performant like Undertow >>> - started to replace all the manual JSON mapping with GSON with >>> Jackson and let it do it's magic; for that to happen all REST resource >>> classes need to be refactored for type safety (at the moment all JSON is >>> handled in string blobs) >>> >>> First I thought that cleaning up the api (DTOs) and domain classes would >>> be the easiest thing to achieve, in the end these should be only "dumb" >>> classes with simple attributes and a bunch of getters and setters... turns >>> out that these are the hardest to clean up, because over the years a lot of >>> business logic code leaked into these classes (see Loan.java, my favorite >>> example). Another seemingly simple thing makes cleanup efforts harder on >>> that front: pretty much all these classes were designed with immutability >>> in mind; all/most data is set via constructors; I see the utility of >>> immutable data structures, but having 25 constructor parameters makes this >>> awfully hard to debug and reason about; my preference would be some fluent >>> builder api to set these values (makes it very readable and clear to >>> understand), but if we would do this then a lot of existing code would >>> break... changes in that area pretty much change the whole code base. So, >>> not sure if this is going to happen soon. As a consequence this creates >>> then a roadblock for using Mapstruct, which works very well with "standard" >>> bean classes (read: Lombok @Data), but the existing DTOs and domain classes >>> are just too inaccessible for the Mapstruct code generator... which is a >>> pity... just on that front we could save a ton of lines of code. But: not >>> all hope is lost here... will come back to this a bit later here. >>> >>> Upgrading Jersey was quite straight forward with some - relatively - >>> harmless changes... the Spring configuration changed (found an easy way to >>> add the resource classes with just a handful lines of code); some JAX-RS >>> annotations needed to be fixed (the one I remember was somewhere around >>> document management... path was not mapped properly). This one could be a >>> nice PR and would immediately help us get rid of the old Jersey stuff. >>> >>> Getting rid of the XML configuration is something I would like to see go >>> rather sooner than later, because it creates all kind of weird issues if >>> you want to bring in some other Spring Boot components (I had a case where >>> I added Apache Camel to the mix and just couldn't get it work with Spring >>> Java config and had to revert back to the XML configuration). Refactoring >>> this part would introduce some kind of backward incompatibility, but only >>> concerning configuration... which I find acceptable given the improvements. >>> Overall, doable and wouldn't be that much of a disruptive PR. >>> >>> Once the Spring configuration issues are solved then replacing the >>> servlet engine becomes really easy... that change can be offered as an >>> optional Gradle build task. Default distribution would still be Tomcat >>> (because of the licenses etc.), but people could choose to build themselves >>> a Fineract Jetty or Undertow distribution themselves. Again, low hanging >>> fruit once we change to Spring Java config. >>> >>> Cleaning up the REST API layer would be a bit of a bigger undertaking in >>> terms of code changes, but overall less disruptive than touching the DTO >>> and domain classes. The problem in that area is that - pretty much - all >>> JSON HTTP request bodies are first stored in a string, that string is then >>> used with those manually created mapper classes to deserialize them into >>> Java classes (DTOs); serialization works pretty much the same way. I think >>> the reason for all this manual code was the somewhat irregular structure of >>> the JSON objects. Since all this mapping code was created a lot of >>> improvements went into Jackson... including a lot of convenience concerning >>> even the most challenging mapping requirements (e. g. try to map Java >>> generic types). I think it's a given that the manual mapping cannot beat >>> Jackson in terms of maintenance and performance... so this change would be >>> actually something that I would really like to see. A nice side effect >>> would be that we could get rid of all those Swagger related helper classes; >>> those were introduced to help with the lack of type safety in the REST >>> resource classes (aka JSON string blobs). If we could pull this one off we >>> could get rid of all manual JSON mapping and the Swagger helper classes... >>> I bet this would reduce the code size by at least 10%. >>> >>> I think before we attack these items (and anything else that we come up >>> with) it would be worth splitting up the code first "as is" into separate >>> modules (JARs) and publishing them on Maven central. Once that happens we >>> can start applying above refactorings on a not so critical module (e. g. >>> portfolio client), take the lessons learned and apply them one by one to >>> all other modules. >>> >>> My 2 cents... pretty sure there are more ideas to be added here. >>> >>> Cheers, >>> >>> Aleks >>> >>> >>> On Sat, Oct 16, 2021 at 7:15 AM Petri Tuomola <[email protected]> >>> wrote: >>> >>>> James - thanks for sharing this >>>> >>>> I think there would be a lot of interesting “mini-projects” that could >>>> be undertaken to refactor / modernise Fineract 1.x. The benefit of such an >>>> approach would be backward compatibility, allowing us to bring improvements >>>> to all Fineract 1.x users without a migration. >>>> >>>> As you state, performance is definitely one of the key areas that we >>>> should look to address. I’m personally not certain that PostgresQL / >>>> EclipseLink would give significantly better performance than MySQL / >>>> OpenJPA, but it would be interesting to performance test this. However, I’m >>>> pretty sure we can get significant performance improvements simply by >>>> tuning the existing SQL queries / schema / business logic. >>>> >>>> But to undertake this, we’d first need a robust performance test suite >>>> in place to establish the baseline and then to measure the improvements. >>>> Writing the tests themselves may not be that much of a challenge - but we’d >>>> need the infrastructure to run these in a consistent environment on a >>>> regular basis. AWS / GCP would be one option, though it requires at least >>>> some funding… but perhaps there are some free options out there? >>>> >>>> The migration to PostgresQL would be interesting due to the licensing >>>> issues with MySQL JDBC connector though. I started working on this earlier >>>> but didn’t get a chance to finish. Will see if I can pick this up when I >>>> have some spare time again. >>>> >>>> Another possible improvement would be to see if we could rework the >>>> code to reduce the amount of “boilerplate” code for serialisation / >>>> validation / data access. In a typical pull request today, a large >>>> percentage of the code is there just to add the new parameter(s) to the >>>> APIs / database / function signatures. Perhaps with some of the modern >>>> libraries we could reduce the amount of code that is required to simply >>>> pass data around, making the Fineract code more focused on business >>>> functionality (and hence easier to manage/refactor/maintain). >>>> >>>> Anyway - just some thoughts. I’m unfortunately at the moment completely >>>> swamped with work, but will see if I can pick up some of these ideas when I >>>> have some free time again. @Aleksandar Vidakovic - it would be great to see >>>> your list of improvement / modernisation ideas? >>>> >>>> Thanks >>>> >>>> Regards >>>> Petri >>>> >>>> >>>> On 7 Oct 2021, at 02:52, James Dailey <[email protected]> wrote: >>>> >>>> Devs - >>>> >>>> During ApacheCON, a few of the Fineract participants gathered in the >>>> usual "Birds of a Feather" - where people of like interests flock together >>>> - and one topic we took up was some general roadmapping for Fineract1.x >>>> >>>> A key discussion was around componentization of Fineract1.x, and here >>>> is my attempt to bring this rich discussion back to the list. @Edward >>>> Cable <[email protected]> , @Victor Manuel Romero Rodriguez >>>> <[email protected]> , @Javier Borkenztain <[email protected]> >>>> >>>> >>>> Aleksandar had proposed, during his ApacheCON @home2021 talk that day, >>>> an effort to REVISE and RATIONALIZE the underlying components to be more >>>> "rational", "streamlined", "better at high volumes", which amounted to some >>>> fairly substantial restructuring of the existing components. >>>> This idea may help with the code debt that has crept into the code over >>>> the last 10 years. It would make the project more easily maintained. >>>> It may also allow fineract1.x to evolve into possibly a better behaving >>>> "microservice" (fineract1.x acting as a microservice itself) or (again >>>> maybe) a composition of microservices. >>>> In particular, the class structure around products and portfolio was >>>> highlighted as being highly complex to deal with. Currently, code changes >>>> in one area could have unanticipated impacts in other areas and test >>>> coverage is not sufficient to catch all of these issues. >>>> So, in summary, we admired the problem a bit, and agreed to start on >>>> some of the more manageable smaller changes, whilst still looking to a more >>>> intensive re-write. @Aleksandar Vidakovic <[email protected]> : >>>> are you able to put some high level Tickets in Jira for this? This will >>>> take some serious effort and needs to be scoped out. >>>> >>>> Secondly, this rewrite/revision of Fineract1.x was then compared to the >>>> FineractCN codebase strategy which began life as a rewrite of the Fineract >>>> structure from the ground up. Some devs are currently proposing PRs >>>> related to the core microservices (e.g. Identity) based on their >>>> experiences using the existing code base in production. The evolution of >>>> FineractCN into a full release will need some attention from the Community >>>> to make happen. The proposal for how to get to a release is still at ==> >>>> https://cwiki.apache.org/confluence/display/FINERACT/Apache+Fineract+CN+Community+Roadmap >>>> . >>>> Let's start a separate thread on that. >>>> >>>> Thirdly, going back to Fineract1.x, one of the key issues is around the >>>> scalability and performance of the platform. A number of participants >>>> mentioned that fineract1.x is starting to be used in increasingly >>>> large-scale scenarios. As discussed on list previously, there are potential >>>> strategies, including replacing OpenJPA with EclipseLink ==> >>>> https://issues.apache.org/jira/browse/FINERACT-849, (thank you @Yemdjih >>>> Kaze Nasser <[email protected]> ) and migrating to a Postgres >>>> option (whilst maintaining backwards compatibility?)... >>>> https://issues.apache.org/jira/browse/FINERACT-984. There was some >>>> interest in moving that forward. >>>> >>>> There were a lot of good discussions at Birds of a Feather and some fun >>>> moments. If we missed you this time, there's always next. >>>> >>>> Thank you. >>>> James >>>> >>>> >>>> >>> >
