Copilot commented on code in PR #6031: URL: https://github.com/apache/fineract/pull/6031#discussion_r3467443109
########## fineract-doc/src/docs/en/modularization.adoc: ########## @@ -0,0 +1,142 @@ += Event-driven architecture for modular design + +== Summary + +Fineract's feature modules import each other's internal classes directly: loan reads Review Comment: This document is added to the docs tree but (in the current PR state) it isn’t included from any rendered entry point (e.g. `src/docs/en/index.adoc` or a chapter index), so it won’t appear in the generated HTML/PDF output and will be effectively orphaned. Please add an `include::modularization.adoc[leveloffset=+1]` in the appropriate index (or a chapter index) so it is part of the documentation build. ########## fineract-doc/src/docs/en/modularization.adoc: ########## @@ -0,0 +1,142 @@ += Event-driven architecture for modular design + +== Summary + +Fineract's feature modules import each other's internal classes directly: loan reads +savings entities, savings calls accounting services, accounting depends on organisation +repositories. Two shared modules already exist to prevent this -- `fineract-core` (read-only +contracts) and `fineract-command` (write-side commands and events) -- but nothing enforces +their use, so the boundary has eroded. We propose to make those two modules the only +permitted cross-feature dependency: reads cross a boundary as a `fineract-core` interface +returning a DTO, writes cross as a `fineract-command` command or domain event. Every direct +feature-to-feature import is migrated to one of those two mechanisms, in risk order, and an +ArchUnit rule is activated in CI to keep the boundary from eroding again. A Spring Modulith +boundary scanner already exists and reports the current violation set per module, giving a +before/after count for each phase. The REST API and integration tests stay unchanged +throughout. + +== Status + +Under Development + +== Who is involved + +* Mansi Maurya (proposal owner, lead developer) +* Aleksandar Vidakovic (reviewer) + +== Background / Motivation + +The package layout reflects a layered architecture (REST, DTOs, services, repositories), but +the feature modules are not isolated from one another. Over the years, services and +repositories from one feature have been imported directly into others, with no rule to stop +it. `fineract-core` and `fineract-command` exist, but using them is optional; a developer can +import another feature's repository and the build still passes. + +The concrete cost: + +* *Refactoring is unsafe.* Changing an internal class in one feature can silently break a + consumer in an unrelated module, surfacing only at test or runtime. +* *Ownership is diffuse.* When several features import the same repository, adding a field, + changing a query, or deprecating a method turns into a multi-team negotiation. +* *Isolated testing is impractical.* Unit-testing one service pulls in the transitive + dependency graph of everything it imports. +* *Event-driven execution is blocked.* Direct synchronous calls between features leave no + seam at which to introduce events, asynchronous handling, or an outbox. + +The boundary scanner quantifies the problem today: accounting, savings, loan, group, teller, +batch and the deposit modules each reference types from other features. The numbers per +module are tracked so each phase shows measurable reduction. + +== Goals + +* `fineract-core` becomes the single home for shared, read-only contracts: read-service + interfaces, the DTOs/projections they return (`ClientSummaryDTO`, `OfficeDTO`, `ChargeDTO`, + `LoanAccountSummaryDTO`, ...), and shared value objects and enums. No business logic, no + feature-owned entities. +* `fineract-command` becomes the single home for cross-feature write contracts: command and + domain-event definitions, the command/event bus, transactional outbox/inbox, and saga + support for long-running flows. +* Every direct feature-to-feature import is removed and re-expressed as a `fineract-core` + read or a `fineract-command` write. +* An ArchUnit rule is wired into CI once the violation count reaches zero, failing any build + that reintroduces a cross-feature import. +* The REST API stays 100% backward-compatible: no changes to endpoints, request shapes, or + response shapes. +* Integration tests stay green at every phase boundary; no phase requires a big-bang cutover. + +== Exclusions + +* The storage layer -- JPA entities, schema, query performance -- is out of scope; that is a + separate effort. +* No changes to existing REST API contracts or response shapes. +* No external message brokers; outbox/inbox stays within the database transaction for now. +* Performance tuning and asynchronous execution are not in scope, though the event-driven + boundaries introduced here make them straightforward to add later. + +== Change proposed + +=== Reads: `fineract-core` contracts + +Instead of importing another feature's repository to perform a read, a feature depends on an +interface declared in `fineract-core`, and the owning feature provides the implementation. +The interface returns a DTO -- a plain, stable projection -- so the data crossing a boundary +is never a live entity. This inverts the dependency: both the caller and the owner depend on +`fineract-core`, neither on the other. + +---- + accounting ──depends on──► fineract-core (ChargeReadService) ◄──implements── charge +---- Review Comment: The literal block uses box-drawing/arrow Unicode characters (e.g. `──►`), which are prone to rendering as tofu/missing glyphs in the PDF backend depending on the Asciidoctor PDF font setup. Using plain ASCII arrows (or a diagram include) is more robust across HTML/PDF outputs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
