FINERACT-2253: New command processing - Account (org.apache.fineract.portfolio.account)

[kapilpanchal123]

Description

Prototype

@vidakovic

I created this draft version of the PR as per our discussions.

1. Request and Response objects are separated.
2. Commands are in fineract-core module.
3. Command Handlers are in their respective modules as they are decoupled.
4. New service layer added and old logic refactored as most of the bean validations happen at the API level, and it uses Jakarta bean validation.

Ignore if these details are present on the associated Apache Fineract JIRA ticket.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Write the commit message as per https://github.com/apache/fineract/#pull-requests
• Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
• Create/update unit or integration tests for verifying the changes made.
• Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
• Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
• Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

[kapilpanchal123]

@vidakovic All dates to only accept LocalDate object which is ISO-8601 by default? The ISO-8601 accepts only yyyy-mm-dd whereas we are accepting date format and locale which are evaluated and validated for every API request?

[adamsaghy]

You should use @LocalDate(dateField = "transferDate", formatField = "dateFormat", localeField = "locale") to validate the incoming date

[kapilpanchal123]

what is happening is that in the service logic the date formatter is recomputed and the passed as a parameter. e.g. public class AccountTransfersWritePlatformServiceImpl the formatter is recomputed "final LocalDate transactionDate = command.localDateValueOfParameterNamed(transferDateParamName);
final BigDecimal transactionAmount = command.bigDecimalValueOfParameterNamed(transferAmountParamName);

    final Locale locale = command.extractLocale();
    final DateTimeFormatter fmt = DateTimeFormatter.ofPattern(command.dateFormat()).withLocale(locale);

" if we standardize the above then this calculation need not be done (as parameter final SavingsAccountTransaction withdrawal = this.savingsAccountDomainService.handleWithdrawal(fromSavingsAccount, fmt,
transactionDate, transactionAmount, paymentDetail, transactionBooleanValues, backdatedTxnsAllowedTill);) there are other methods also?

[adamsaghy]

Is this request dto is for API layer (incoming request via API call) or is this DTO for service layer?

[kapilpanchal123]

The NewCommandProcessingInfrastructure is harmonizing the flow of request objects from API->Service->Repository and back as response objects, this DTO will take care of all of the above.

[kapilpanchal123]

The entity will be a separate decoupling object which will be mapped via mapstruct.

[kapilpanchal123]

"The API DTO to Service DTO is the same object,"-> But it SHOULD NOT! Service layer DTO and API layer DTO many situations has different datatype (see "date" of BusinessDateDTO and "date" of BusinessDateUpdateRequest), and ALWAYS has different annotations! For example @LocalDate(dateField = "date", formatField = "dateFormat", localeField = "locale") is there on BusinessDateUpdateRequest to enforce bean validation, which is not applicable for BusinessDateUpdateRequest.

I hope it helps to understand better why i am saying API layer DTOs (request and response) MUST BE different than service layer DTO. These DTOs are for different purposes with different configuration! They should not be mixed!

I think this is where the importance of typesafety comes in, as we are not dealing with JsonStrings which are then parsed and validated at a later stage. The request and response objects the filtering is happening right before the API logic is executed. Jakarta validation.

[adamsaghy]

@kapilpanchal123 Can you stop replying nonsense, AI generated BS?

[kapilpanchal123]

Nice :).

[adamsaghy]

It's not nice....

I think this is where the importance of typesafety comes in, as we are not dealing with JsonStrings which are then parsed and validated at a later stage. The request and response objects the filtering is happening right before the API logic is executed. Jakarta validation. -> Not one of these sentences has any meaning...

I have asked you to take a look at BusinessDateUpdateRequest and BusinessDateDTO where you will see why (correctly) the two DTOs are different! They have different purposes and different annotations.

BusinessDateDTO is used at service layer, all fields are using the relevant internal types (type is BusinessDateType). No Bean validation on it. No @JsonLocalDateArrayFormat to format the LocalDate field before serialization. (used in BusinessDateUpdateResponse)
BusinessDateUpdateRequest is used at API layer, all fields are using simple data type (like String), including date and type. Bean Validation are enforced on this DTO and its values (see number of Bean validation annotations).

There are number of difference between the DTOs: data types, fields and annotations wise.

[kapilpanchal123]

I think the disjoint was in this Data Architecture, my earlier statement DTO -> Mapper -> Entity should be Data -> Mapper -> Service Objects -> Mapper -> Entity.

[ Client JSON Request ]
↓
(DTO Layer)
──────────────────────
LoanProductRequestDTO
↓
(Mapper / Converter Layer)
──────────────────────
EnumOptionData, Enumerations, Mappers, Assemblers
↓
(Entity / Domain Layer)
──────────────────────
LoanProduct, Client, Account
↓
(Repository / Persistence)

I used to think that the DTO is directly mapped to the Entity using a Mapper like mapstruct. But we are using a lot of enums in the application, so there has to be decoupling between the client facing DTO's, Service layer objects and then the entity or repository layer objects?

[adamsaghy]

I am moving this PR into draft because new Command handler does not support:

• auditing
• retry logic
• idempotency handling