Aman-Mittal commented on code in PR #5722:
URL: https://github.com/apache/fineract/pull/5722#discussion_r3017120348
##########
fineract-provider/src/main/java/org/apache/fineract/template/domain/Template.java:
##########
@@ -90,14 +90,20 @@ public static Template fromJson(final JsonCommand command) {
break;
}
- final JsonArray array = command.arrayOfParameterNamed("mappers");
-
final List<TemplateMapper> mappersList = new ArrayList<>();
- for (final JsonElement element : array) {
- mappersList.add(new
TemplateMapper(element.getAsJsonObject().get("mappersorder").getAsInt(),
- element.getAsJsonObject().get("mapperskey").getAsString(),
-
element.getAsJsonObject().get("mappersvalue").getAsString()));
+ try {
Review Comment:
I noticed ObjectMapper is being instantiated in multiple places—should we
consider reusing a shared instance (e.g., via Spring) for consistency and
efficiency, or is this intentional for now?
##########
fineract-provider/build.gradle:
##########
@@ -184,15 +184,15 @@ configurations.driver.each {File file ->
tasks.register('createDB') {
description = "Creates the MariaDB Database. Needs database name to be
passed (like: -PdbName=someDBname)"
doLast {
- def sql = Sql.newInstance('jdbc:mariadb://localhost:3306/', mysqlUser,
mysqlPassword, 'org.mariadb.jdbc.Driver')
+ def sql =
Sql.newInstance('jdbc:mariadb://localhost:3306/?allowPublicKeyRetrieval=true&useSSL=false',
mysqlUser, mysqlPassword, 'org.mariadb.jdbc.Driver')
Review Comment:
JDBC URL change in build.gradle—should that be handled in a separate PR,
since it’s not directly related to the JSON migration?
##########
fineract-provider/src/main/java/org/apache/fineract/template/service/JpaTemplateDomainService.java:
##########
@@ -96,21 +94,25 @@ public CommandProcessingResult updateTemplate(final Long
templateId, final JsonC
}
template.setType(type);
- final JsonArray array = command.arrayOfParameterNamed("mappers");
- final List<TemplateMapper> mappersList = new ArrayList<>();
- for (final JsonElement element : array) {
- mappersList.add(new
TemplateMapper(element.getAsJsonObject().get("mappersorder").getAsInt(),
- element.getAsJsonObject().get("mapperskey").getAsString(),
-
element.getAsJsonObject().get("mappersvalue").getAsString()));
+ try {
+ final ObjectMapper objectMapper = new ObjectMapper();
+ final JsonNode rootNode = objectMapper.readTree(command.json());
+ final JsonNode mappersNode = rootNode.get("mappers");
+ final List<TemplateMapper> mappersList = new ArrayList<>();
+ if (mappersNode != null && mappersNode.isArray()) {
+ for (final JsonNode element : mappersNode) {
+ mappersList.add(new
TemplateMapper(element.get("mappersorder").asInt(),
element.get("mapperskey").asText(),
Review Comment:
For parsing the mappers array, would it make sense to leverage Jackson’s
data binding (e.g., mapping directly to a List<TemplateMapper>) instead of
manual tree traversal, or is this intentional for now?
--
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]
