From 2b122a17ea477e3bac67ce9f37b4e7f1f0bccf39 Mon Sep 17 00:00:00 2001 From: Jose Alberto Hernandez Date: Sat, 11 Jun 2022 19:37:01 -0500 Subject: [PATCH] Fix for reading multirow datatable entry reads non-belonging entry --- .../AdHocScheduledJobRunnerServiceImpl.java | 2 +- .../core/data/CommandProcessingResult.java | 8 +- .../CreateDatatableEntryCommandHandler.java | 14 +-- .../ReadWriteNonCoreDataServiceImpl.java | 94 ++++++++++++++----- .../domain/LoanApplicationTerms.java | 3 +- .../common/system/DatatableHelper.java | 17 +++- .../system/DatatableIntegrationTest.java | 50 +++++++++- 7 files changed, 140 insertions(+), 48 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java index e5f62bcc3..f2a81e238 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java @@ -82,7 +82,7 @@ public class AdHocScheduledJobRunnerServiceImpl implements AdHocScheduledJobRunn run = Math.toIntExact(ChronoUnit.YEARS.between(start, end)) >= 1; break; case CUSTOM: - next = start.plusDays((int) (long) adhoc.getReportRunEvery()); + next = start.plusDays((long) adhoc.getReportRunEvery()); run = Math.toIntExact(ChronoUnit.DAYS.between(start, end)) >= adhoc.getReportRunEvery(); break; default: diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/CommandProcessingResult.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/CommandProcessingResult.java index dad68a568..8cd248166 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/CommandProcessingResult.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/CommandProcessingResult.java @@ -38,7 +38,6 @@ public class CommandProcessingResult implements Serializable { private final String transactionId; private final Map changes; private final Map creditBureauReportData; - @SuppressWarnings("unused") private final String resourceIdentifier; private final Long productId; private final Long gsimId; @@ -52,6 +51,13 @@ public class CommandProcessingResult implements Serializable { commandResult.creditBureauReportData, commandResult.rollbackTransaction, commandResult.subResourceId); } + public static CommandProcessingResult fromCommandProcessingResult(CommandProcessingResult commandResult, final Long resourceId) { + return new CommandProcessingResult(commandResult.commandId, commandResult.officeId, commandResult.groupId, commandResult.clientId, + commandResult.loanId, commandResult.savingsId, commandResult.resourceIdentifier, resourceId, commandResult.transactionId, + commandResult.changes, commandResult.productId, commandResult.gsimId, commandResult.glimId, + commandResult.creditBureauReportData, commandResult.rollbackTransaction, commandResult.subResourceId); + } + public static CommandProcessingResult fromDetails(final Long commandId, final Long officeId, final Long groupId, final Long clientId, final Long loanId, final Long savingsId, final String resourceIdentifier, final Long entityId, final Long gsimId, final Long glimId, final Map creditBureauReportData, final String transactionId, diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/handler/CreateDatatableEntryCommandHandler.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/handler/CreateDatatableEntryCommandHandler.java index c6bfb3610..15c3f9985 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/handler/CreateDatatableEntryCommandHandler.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/handler/CreateDatatableEntryCommandHandler.java @@ -21,7 +21,6 @@ package org.apache.fineract.infrastructure.dataqueries.handler; import org.apache.fineract.commands.handler.NewCommandSourceHandler; import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; -import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder; import org.apache.fineract.infrastructure.dataqueries.service.ReadWriteNonCoreDataService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -41,17 +40,6 @@ public class CreateDatatableEntryCommandHandler implements NewCommandSourceHandl @Override public CommandProcessingResult processCommand(final JsonCommand command) { - final CommandProcessingResult commandProcessingResult = this.writePlatformService.createNewDatatableEntry(command.entityName(), - command.entityId(), command); - - return new CommandProcessingResultBuilder() // - .withCommandId(command.commandId()) // - .withEntityId(command.entityId()) // - .withOfficeId(commandProcessingResult.getOfficeId()) // - .withGroupId(commandProcessingResult.getGroupId()) // - .withClientId(commandProcessingResult.getClientId()) // - .withSavingsId(commandProcessingResult.getSavingsId()) // - .withLoanId(commandProcessingResult.getLoanId()) // - .build(); + return this.writePlatformService.createNewDatatableEntry(command.entityName(), command.entityId(), command); } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java index de7353ef2..578e2901f 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImpl.java @@ -26,6 +26,8 @@ import com.google.gson.JsonObject; import com.google.gson.reflect.TypeToken; import java.lang.reflect.Type; import java.math.BigDecimal; +import java.sql.SQLException; +import java.sql.Statement; import java.time.LocalDate; import java.time.LocalDateTime; import java.util.ArrayList; @@ -80,6 +82,8 @@ import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.jdbc.BadSqlGrammarException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.jdbc.support.GeneratedKeyHolder; +import org.springframework.jdbc.support.KeyHolder; import org.springframework.jdbc.support.rowset.SqlRowSet; import org.springframework.jdbc.support.rowset.SqlRowSetMetaData; import org.springframework.orm.jpa.JpaSystemException; @@ -374,19 +378,30 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ public CommandProcessingResult createNewDatatableEntry(final String dataTableName, final Long appTableId, final String json) { try { final String appTable = queryForApplicationTableName(dataTableName); - final CommandProcessingResult commandProcessingResult = checkMainResourceExistsWithinScope(appTable, appTableId); + CommandProcessingResult commandProcessingResult = checkMainResourceExistsWithinScope(appTable, appTableId); final List columnHeaders = this.genericDataService.fillResultsetColumnHeaders(dataTableName); + final boolean multiRow = isMultirowDatatable(columnHeaders); + final Type typeOfMap = new TypeToken>() {}.getType(); final Map dataParams = this.fromJsonHelper.extractDataMap(typeOfMap, json); final String sql = getAddSql(columnHeaders, dataTableName, getFKField(appTable), appTableId, dataParams); - this.jdbcTemplate.update(sql); + if (!multiRow) { + this.jdbcTemplate.update(sql); + commandProcessingResult = CommandProcessingResult.fromCommandProcessingResult(commandProcessingResult, appTableId); + } else { + final Long resourceId = addMultirowRecord(sql); + commandProcessingResult = CommandProcessingResult.fromCommandProcessingResult(commandProcessingResult, resourceId); + } return commandProcessingResult; // + } catch (final SQLException e) { + throw new PlatformDataIntegrityException("error.msg.unknown.data.integrity.issue", + "Unknown data integrity issue with resource.", e); } catch (final DataAccessException dve) { final Throwable cause = dve.getCause(); final Throwable realCause = dve.getMostSpecificCause(); @@ -423,7 +438,6 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ logAsErrorUnexpectedDataIntegrityException(e); throw new PlatformDataIntegrityException("error.msg.unknown.data.integrity.issue", "Unknown data integrity issue with resource.", e); - } } @@ -693,6 +707,24 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withResourceIdAsString(datatableName).build(); } + private long addMultirowRecord(String sql) throws SQLException { + KeyHolder keyHolder = new GeneratedKeyHolder(); + int insertsCount = this.jdbcTemplate.update(c -> c.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS), keyHolder); + if (insertsCount == 1) { + Number assignedKey = null; + if (keyHolder.getKeys().size() > 1) { + assignedKey = (Long) keyHolder.getKeys().get("id"); + } else { + assignedKey = keyHolder.getKey(); + } + if (assignedKey == null) { + throw new SQLException("Row id getting error."); + } + return assignedKey.longValue(); + } + throw new SQLException("Expected one inserted row."); + } + private void parseDatatableColumnForUpdate(final JsonObject column, final Map mapColumnNameDefinition, StringBuilder sqlBuilder, final String datatableName, final StringBuilder constrainBuilder, final Map codeMappings, final List removeMappings, @@ -781,7 +813,6 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ } } - @SuppressWarnings("deprecation") private int getCodeIdForColumn(final String dataTableNameAlias, final String name) { final StringBuilder checkColumnCodeMapping = new StringBuilder(); checkColumnCodeMapping.append("select ccm.code_id from x_table_column_code_mappings ccm where ccm.column_alias_name='") @@ -1277,16 +1308,15 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ final List columnHeaders = this.genericDataService.fillResultsetColumnHeaders(dataTableName); - String sql = ""; + final boolean multiRow = isMultirowDatatable(columnHeaders); - // id only used for reading a specific entry in a one to many datatable - // (when updating) - if (id == null) { - String whereClause = getFKField(appTable) + " = " + appTableId; - SQLInjectionValidator.validateSQLInput(whereClause); - sql = sql + "select * from " + sqlGenerator.escape(dataTableName) + " where " + whereClause; - } else { - sql = sql + "select * from " + sqlGenerator.escape(dataTableName) + " where id = " + id; + String whereClause = getFKField(appTable) + " = " + appTableId; + SQLInjectionValidator.validateSQLInput(whereClause); + String sql = "select * from " + sqlGenerator.escape(dataTableName) + " where " + whereClause; + + // id only used for reading a specific entry that belongs to appTableId (in a one to many datatable) + if (multiRow && id != null) { + sql = sql + " and id = " + id; } if (StringUtils.isNotBlank(order)) { @@ -1304,16 +1334,15 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ final List columnHeaders = this.genericDataService.fillResultsetColumnHeaders(dataTableName); - String sql = ""; + final boolean multiRow = isMultirowDatatable(columnHeaders); - // id only used for reading a specific entry in a one to many datatable - // (when updating) - if (id == null) { - String whereClause = getFKField(appTable) + " = " + appTableId; - SQLInjectionValidator.validateSQLInput(whereClause); - sql = sql + "select * from " + sqlGenerator.escape(dataTableName) + " where " + whereClause; - } else { - sql = sql + "select * from " + sqlGenerator.escape(dataTableName) + " where id = " + id; + String whereClause = getFKField(appTable) + " = " + appTableId; + SQLInjectionValidator.validateSQLInput(whereClause); + String sql = "select * from " + sqlGenerator.escape(dataTableName) + " where " + whereClause; + + // id only used for reading a specific entry that belongs to appTableId (in a one to many datatable) + if (multiRow && id != null) { + sql = sql + " and id = " + id; } final List result = fillDatatableResultSetDataRows(sql); @@ -1494,7 +1523,6 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ } private String getFKField(final String applicationTableName) { - return applicationTableName.substring(2) + "_id"; } @@ -1519,7 +1547,14 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ pValueWrite = "null"; } else { if ("bit".equalsIgnoreCase(pColumnHeader.getColumnType())) { - pValueWrite = BooleanUtils.toString(BooleanUtils.toBooleanObject(pValue), "1", "0", "null"); + if (databaseTypeResolver.isMySQL()) { + pValueWrite = BooleanUtils.toString(BooleanUtils.toBooleanObject(pValue), "1", "0", "null"); + } else if (databaseTypeResolver.isPostgreSQL()) { + pValueWrite = BooleanUtils.toString(BooleanUtils.toBooleanObject(pValue), "B'1'", "B'0'", "null"); + } else { + throw new IllegalStateException("Current database is not supported"); + } + } else { pValueWrite = singleQuote + this.genericDataService.replace(pValue, singleQuote, singleQuote + singleQuote) + singleQuote; @@ -1865,6 +1900,17 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ } + private boolean isMultirowDatatable(final List columnHeaders) { + boolean multiRow = false; + for (ResultsetColumnHeaderData column : columnHeaders) { + if (column.isNamed("id")) { + multiRow = true; + break; + } + } + return multiRow; + } + private boolean notTheSame(final String currValue, final String pValue, final String colType) { if (StringUtils.isEmpty(currValue) && StringUtils.isEmpty(pValue)) { return false; diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/LoanApplicationTerms.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/LoanApplicationTerms.java index 49b996356..1d1429f14 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/LoanApplicationTerms.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/LoanApplicationTerms.java @@ -1040,8 +1040,7 @@ public final class LoanApplicationTerms { break; case DAILY: // For daily work out number of days in the period - BigDecimal numberOfDaysInPeriod = BigDecimal - .valueOf(Math.toIntExact(ChronoUnit.DAYS.between(periodStartDate, periodEndDate))); + BigDecimal numberOfDaysInPeriod = BigDecimal.valueOf(ChronoUnit.DAYS.between(periodStartDate, periodEndDate)); final BigDecimal oneDayOfYearInterestRate = this.annualNominalInterestRate.divide(loanTermPeriodsInYearBigDecimal, mc) .divide(divisor, mc); diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableHelper.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableHelper.java index 8ab185f31..2adca9d32 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableHelper.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableHelper.java @@ -52,11 +52,11 @@ public class DatatableHelper { } public Integer createDatatableEntry(final String apptableName, final String datatableName, final Integer apptableId, - final boolean genericResultSet, final String dateFormat) { + final boolean genericResultSet, final String dateFormat, final String jsonAttributeToGetBack) { return Utils.performServerPost( this.requestSpec, this.responseSpec, DATATABLE_URL + "/" + datatableName + "/" + apptableId + "?genericResultSet=" + Boolean.toString(genericResultSet) + "&" + Utils.TENANT_IDENTIFIER, - getTestDatatableEntryAsJSON(dateFormat), "resourceId"); + getTestDatatableEntryAsJSON(dateFormat), jsonAttributeToGetBack); } public String readDatatableEntry(final String datatableName, final Integer resourceId, final boolean genericResultset) { @@ -65,9 +65,16 @@ public class DatatableHelper { } public List readDatatableEntry(final String datatableName, final Integer resourceId, final boolean genericResultset, - final String jsonAttributeToGetBack) { - return Utils.performServerGetList(this.requestSpec, this.responseSpec, DATATABLE_URL + "/" + datatableName + "/" + resourceId - + "?genericResultSet=" + String.valueOf(genericResultset) + "&" + Utils.TENANT_IDENTIFIER, jsonAttributeToGetBack); + final Integer datatableResourceId, final String jsonAttributeToGetBack) { + if (datatableResourceId == null) { + return Utils.performServerGetList(this.requestSpec, this.responseSpec, DATATABLE_URL + "/" + datatableName + "/" + resourceId + + "?genericResultSet=" + String.valueOf(genericResultset) + "&" + Utils.TENANT_IDENTIFIER, jsonAttributeToGetBack); + } else { + return Utils.performServerGetList( + this.requestSpec, this.responseSpec, DATATABLE_URL + "/" + datatableName + "/" + resourceId + "/" + datatableResourceId + + "?genericResultSet=" + String.valueOf(genericResultset) + "&" + Utils.TENANT_IDENTIFIER, + jsonAttributeToGetBack); + } } public Date readDatatableEntry(final String datatableName, final Integer resourceId, final boolean genericResultset, final int position, diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableIntegrationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableIntegrationTest.java index 8dd165acd..d3a127ae1 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableIntegrationTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/system/DatatableIntegrationTest.java @@ -63,11 +63,11 @@ public class DatatableIntegrationTest { // creating new client datatable entry final boolean genericResultSet = true; Integer datatableResourceID = this.datatableHelper.createDatatableEntry(CLIENT_APP_TABLE_NAME, datatableName, clientID, - genericResultSet, "yyyy-MM-dd"); + genericResultSet, "yyyy-MM-dd", "resourceId"); assertNotNull(datatableResourceID, "ERROR IN CREATING THE ENTITY DATATABLE RECORD"); // Read the Datatable entry generated with genericResultSet in true (default) - final List items = this.datatableHelper.readDatatableEntry(datatableName, clientID, genericResultSet, "data"); + final List items = this.datatableHelper.readDatatableEntry(datatableName, clientID, genericResultSet, null, "data"); assertEquals(1, items.size()); // Read the Datatable entry generated with genericResultSet in false @@ -84,4 +84,50 @@ public class DatatableIntegrationTest { assertEquals(datatableName, deletedDataTableName, "ERROR IN DELETING THE DATATABLE"); } + @Test + public void validateReadDatatableMultirow() { + // creating multirow datatable for client entity + String datatableName = this.datatableHelper.createDatatable(CLIENT_APP_TABLE_NAME, true); + DatatableHelper.verifyDatatableCreatedOnServer(this.requestSpec, this.responseSpec, datatableName); + + // creating first client with datatables + final Integer clientIdA = ClientHelper.createClientAsPerson(requestSpec, responseSpec); + + // creating second client with datatables + final Integer clientIdB = ClientHelper.createClientAsPerson(requestSpec, responseSpec); + + // creating new client datatable entry for first client + final boolean genericResultSet = true; + final Integer datatableResourceIdA = this.datatableHelper.createDatatableEntry(CLIENT_APP_TABLE_NAME, datatableName, clientIdA, + genericResultSet, "yyyy-MM-dd", "resourceId"); + assertNotNull(datatableResourceIdA, "ERROR IN CREATING THE ENTITY DATATABLE RECORD"); + + // creating new client datatable entry for second client + final Integer datatableResourceIdB = this.datatableHelper.createDatatableEntry(CLIENT_APP_TABLE_NAME, datatableName, clientIdB, + genericResultSet, "yyyy-MM-dd", "resourceId"); + assertNotNull(datatableResourceIdB, "ERROR IN CREATING THE ENTITY DATATABLE RECORD"); + + // Read the Datatable entry generated for first client + List items; + items = this.datatableHelper.readDatatableEntry(datatableName, clientIdA, genericResultSet, datatableResourceIdA, "data"); + assertEquals(1, items.size()); + + // Read the Datatable entry generated for second client + items = this.datatableHelper.readDatatableEntry(datatableName, clientIdB, genericResultSet, datatableResourceIdB, "data"); + assertEquals(1, items.size()); + + // Read the Datatable entry generated for first client and second client's record Id + items = this.datatableHelper.readDatatableEntry(datatableName, clientIdA, genericResultSet, datatableResourceIdB, "data"); + assertEquals(0, items.size()); + + // deleting datatable entries + Integer appTableIdA = this.datatableHelper.deleteDatatableEntries(datatableName, clientIdA, "clientId"); + assertEquals(clientIdA, appTableIdA, "ERROR IN DELETING THE DATATABLE ENTRIES"); + Integer appTableIdB = this.datatableHelper.deleteDatatableEntries(datatableName, clientIdB, "clientId"); + assertEquals(clientIdB, appTableIdB, "ERROR IN DELETING THE DATATABLE ENTRIES"); + + // deleting the datatable + String deletedDataTableName = this.datatableHelper.deleteDatatable(datatableName); + assertEquals(datatableName, deletedDataTableName, "ERROR IN DELETING THE DATATABLE"); + } }