Hi all, Zeyad, Aleks, We are using JsonCommand which holds the request data as a string blob and > lets the handler pass it to the desired service to be processed. > Also, the CommandSource entity stores the body request as a JSON string > blob (everywhere, there’s a "String" nightmare) > We receive the request body as a JSON string (apiRequestBodyAsJson) losing > all type info.
Yes, these problems are as relevant as it was when you raised them earlier. Why so many parameters when they're used only in 1 place at the bottom of > the execution chain? Didn't Lombok and builder patterns attempt to solve this problem or is this a different problem? but let's say everything is equally critical to make it more interesting that would mean overall 500 endpoints that need to be refactored, I dislike this problem of providing uniformity to contributions. At a very high level, let's assume 75 core APIs and 75 non-core APIs directly impacting the behaviour of core APIs (think holidays impacting repayment schedule) is sufficient. Rest 350 APIs shouldn't even be part of the core release or deployed artifact. Their respective test cases shouldn't run for every PR sent to the core and vice versa. Default report generation configs in the project management tool should exclude the modules for the other ~350 APIs. Providing a bunch of mediocre modules/APIs as part of release artifacts is a disservice not only to the users but also to those who are maintaining it. I do acknowledge the need to provide quality to non-core modules to minority user groups but it's overshadowed by previous points. Ex:- Import tool was outside Fineract and brought in within Fineract to maintain it better. With best regards, Avik Ganguly On Tue, Jul 16, 2024 at 7:23 PM Aleksandar Vidakovic < chee...@monkeysintown.com> wrote: > @James Dailey <jamespdai...@gmail.com> > Yes, the idea is to provide a template and use it then to discuss in the > community. As this is a rather large intervention in the current state of > affairs it's unlikely that this effort will cover every single API endpoint > (doesn't need to... and actually shouldn't). The point of this project is > to do most if not all of the grunt work so that we can focus in the > discussions on what we want to do (first) and giving everyone a documented > choice of options and make it easier for everyone. > > Concerning maintainability: well... if we continue the current strategy > (mixing permission enforcement with business logic, manual JSON parsing, > everything is a String blob vs type-safety... just to name a few) then you > are right, but this is the whole idea here... to change that.. > > Consider this the "120% suggestion", meaning that it is then up to the > community which parts we take and/or in which order we attack them... e. g. > I think using Spring Web MVC instead of JAX-RS makes a lot more sense > (nowadays) as it's a first class citizen in the Spring ecosystem, but if we > really choose to migrate there (immediately) that's a different thing... or > we can first attack the read requests (they are a lot simpler to refactor) > and clean them up (remove all manual parsing, no more JSON objects passing > to the business logic services, use DTOs everywhere)... another one that > can be done separately is cleaning up the permission enforcement which is > done explicitly with an injected security service in (most) of the read > request endpoints; the alternative is to configure this in a central > location (the so called web configuration) and/or with Spring Security > "access decision voters"; again, more aligned with the Spring ecosystem > best practices... the biggest block of change will be the write requests; > but even that can be organized in a way that it happens as an effort in > parallel to the current development and can be organized in smaller work > units. > > Note: we have around 50 "features"/"modules" (however we want to call > them) in Fineract... not all of those modules are equally critical > (document management... SMS notifications...), but let's say everything is > equally critical to make it more interesting... and let's assume a napkin > calculation of 10 REST endpoints per "module" that would mean overall 500 > endpoints that need to be refactored, and then let's say that the endpoints > are equally split between read and write requests... that would mean you > have 250 easy refactorings and 250 harder ones. I think those numbers are > maximums, the reality is probably lower than that. Even if I assume the > maximums: this is doable, not by a single person, but if we split this up > then it's really not that hard. > > Cheers > > On Tue, Jul 16, 2024 at 1:39 AM Zeyad Nasef <zeyad.nasef....@gmail.com> > wrote: > >> I'm discussing the current problems and the proposed solutions first, to >> hear your thoughts about the issue with write requests and make sure I am >> doing the right thing, then we can open a PR introducing what we discussed. >> >> On Tue, Jul 16, 2024, 1:58 AM James Dailey <jdai...@apache.org> wrote: >> >>> Zayed - Thanks for sharing, good transparency! >>> >>> Arnold - does this respond to the concerns you raised in the github >>> comments? >>> >>> On the substance, you’ve laid out your case and provided examples and a >>> design concept. >>> >>> If I understand correctly, you will set up some few examples but not >>> fully implement across all components. If that’s the case, then this >>> becomes a problem for maintaining as it is unlikely anyone else will take >>> it up. It might be better to simply show this as a PR and point to your >>> fork for demonstration purposes. >>> >>> Let me know if I’ve got that right. And provide a response. >>> >>> Thanks >>> >>> >>> On Mon, Jul 15, 2024 at 12:53 AM Zeyad Nasef <zeyad.nasef....@gmail.com> >>> wrote: >>> >>>> Hello everyone, >>>> >>>> I hope you're doing well. >>>> >>>> I am currently addressing the type-safety issue with write requests >>>> (aka.. commands) in our project. >>>> After thoroughly exploring the execution flow multiple times, I have >>>> documented my findings, highlighting the issues with the current execution >>>> flow and proposing potential solutions. Additionally, I have included some >>>> questions to better understand the current design. >>>> >>>> I would greatly appreciate it if you could review the document and >>>> provide any suggestions for resolving this issue. If there are any unclear >>>> or missing parts, please let me know. >>>> >>>> The document link >>>> <https://docs.google.com/document/d/14uE-m2Y9BB_un0JVfVALudO-LPb4QvSlaAew0mOGDBs/edit?usp=sharing> >>>> >>>> Jira ticket link <https://issues.apache.org/jira/browse/FINERACT-2021> >>>> >>>> Also, @chee...@monkeysintown.com <chee...@monkeysintown.com> left an >>>> important related comment here >>>> <https://github.com/apache/fineract/pull/3915#issuecomment-2213566671>. >>>> >>>> Thank you! >>>> >>>> Best regards, >>>> Zeyad Nasef >>>> >>>
