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] > <mailto:[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] > <mailto:[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] >> <mailto:[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] >> <mailto:[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] >>> <mailto:[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 >>> <mailto:[email protected]> , @Victor Manuel Romero Rodriguez >>> <mailto:[email protected]> , @Javier Borkenztain >>> <mailto:[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 >>> <mailto:[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 >>> >>> <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 >>> <https://issues.apache.org/jira/browse/FINERACT-849>, (thank you @Yemdjih >>> Kaze Nasser <mailto:[email protected]> ) and migrating to a Postgres >>> option (whilst maintaining backwards >>> compatibility?)...https://issues.apache.org/jira/browse/FINERACT-984 >>> <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 >> >
