This is an automated email from the ASF dual-hosted git repository.

adamsaghy pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git


The following commit(s) were added to refs/heads/develop by this push:
     new 31f41558f FINERACT-1724: Datatables query API fix - invalid query 
filter
31f41558f is described below

commit 31f41558fc9a80b7d8ada7f3140598da126ce3c2
Author: abraham.menyhart <[email protected]>
AuthorDate: Tue May 30 16:43:39 2023 +0200

    FINERACT-1724: Datatables query API fix - invalid query filter
---
 .../service/ReadWriteNonCoreDataServiceImpl.java   | 134 +++++----------------
 .../ReadWriteNonCoreDataServiceImplTest.java       |  65 ++++++----
 2 files changed, 67 insertions(+), 132 deletions(-)

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 fac9c6e39..fc981e2b8 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
@@ -19,6 +19,7 @@
 package org.apache.fineract.infrastructure.dataqueries.service;
 
 import static java.util.Arrays.asList;
+import static java.util.Locale.ENGLISH;
 import static 
org.apache.fineract.infrastructure.core.data.ApiParameterError.parameterErrorWithValue;
 
 import com.google.common.base.Splitter;
@@ -33,18 +34,14 @@ import java.sql.Date;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.sql.Timestamp;
-import java.sql.Types;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
-import java.time.format.DateTimeParseException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Stream;
 import javax.persistence.PersistenceException;
@@ -84,7 +81,6 @@ import 
org.apache.fineract.infrastructure.security.service.SqlInjectionPreventer
 import org.apache.fineract.infrastructure.security.utils.ColumnValidator;
 import org.apache.fineract.infrastructure.security.utils.SQLInjectionValidator;
 import org.apache.fineract.useradministration.domain.AppUser;
-import org.jetbrains.annotations.NotNull;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.dao.EmptyResultDataAccessException;
@@ -113,21 +109,15 @@ public class ReadWriteNonCoreDataServiceImpl implements 
ReadWriteNonCoreDataServ
             .put("number", "INT").put("boolean", "boolean").put("decimal", 
"DECIMAL").put("date", "DATE").put("datetime", "TIMESTAMP")
             .put("text", "TEXT").put("dropdown", "INT").build();
 
-    private static final List<String> stringDataTypes = Arrays.asList("char", 
"varchar", "blob", "text", "tinyblob", "tinytext",
-            "mediumblob", "mediumtext", "longblob", "longtext");
-    private static final DateTimeFormatter DATA_TABLE_DATE_FORMAT = 
DateTimeFormatter.ofPattern("yyyy-MM-dd");
-    private static final DateTimeFormatter DATA_TABLE_DATETIME_FORMAT = 
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");
-    public static final String PG_BOOLEAN_TYPE = "boolean";
-    public static final String PG_BOOL_TYPE = "bool";
-    public static final String PG_INT_TYPE = "integer";
-    public static final String PG_BIGINT_TYPE = "bigint";
-    public static final String PG_DATE_TYPE = "date";
-    public static final String PG_NUMERIC_TYPE = "numeric";
-    public static final String PG_TEXT_TYPE = "text";
-    public static final String PG_VARCHAR_TYPE = "character varying";
-    public static final String BIT_TYPE = "bit";
+    private static final List<String> stringDataTypes = asList("char", 
"varchar", "blob", "text", "tinyblob", "tinytext", "mediumblob",
+            "mediumtext", "longblob", "longtext");
+    private static final String DATE_FORMAT = "yyyy-MM-dd";
+    public static final String DATE_TIME_FORMAT = "yyyy-MM-dd HH:mm:ss";
+    private static final DateTimeFormatter DATA_TABLE_DATE_FORMAT = 
DateTimeFormatter.ofPattern(DATE_FORMAT);
+    private static final DateTimeFormatter DATA_TABLE_DATETIME_FORMAT = 
DateTimeFormatter.ofPattern(DATE_TIME_FORMAT);
+
     public static final String MYSQL_DATETIME_TYPE = "datetime";
-    public static final String MYSQL_DATE_TYPE = "date";
+    public static final String POSTGRES_TIMESTAMP_TYPE = "timestamp without 
time zone";
 
     private final JdbcTemplate jdbcTemplate;
     private final DatabaseTypeResolver databaseTypeResolver;
@@ -205,24 +195,15 @@ public class ReadWriteNonCoreDataServiceImpl implements 
ReadWriteNonCoreDataServ
 
     @Override
     public List<JsonObject> queryDataTable(String datatable, final String 
columnFilter, String valueFilter, String resultColumns) {
-        Arrays.asList(datatable, columnFilter, valueFilter, 
resultColumns).forEach(SQLInjectionValidator::validateDynamicQuery);
+        asList(datatable, columnFilter, valueFilter, 
resultColumns).forEach(SQLInjectionValidator::validateDynamicQuery);
 
         List<ResultsetColumnHeaderData> resultsetColumnHeaderData = 
genericDataService.fillResultsetColumnHeaders(datatable);
         validateRequestParams(columnFilter, valueFilter, resultColumns, 
resultsetColumnHeaderData);
-        String filterColumnType = 
resultsetColumnHeaderData.stream().filter(column -> 
Objects.equals(columnFilter, column.getColumnName()))
-                
.findFirst().map(ResultsetColumnHeaderData::getColumnType).orElse(columnFilter 
+ " does not exist in datatable");
-
-        String sql = "select " + resultColumns + " from " + datatable + " 
where " + columnFilter + " = ?";
-        SqlRowSet rowSet = null;
-        if (databaseTypeResolver.isPostgreSQL()) {
-            sql = "select " + escapeFieldNames(resultColumns) + " from " + 
sqlGenerator.escape(datatable) + " where "
-                    + escapeFieldNames(columnFilter) + " = ?";
-            rowSet = callFilteredPgSql(sql, valueFilter, filterColumnType);
-        } else if (databaseTypeResolver.isMySQL()) {
-            rowSet = callFilteredMysql(sql, valueFilter, filterColumnType);
-        } else {
-            throw new IllegalStateException("Database type is not supported");
-        }
+
+        Object validatedValueFilter = getValidatedValueFilter(columnFilter, 
valueFilter, resultsetColumnHeaderData);
+        String sql = "select " + escapeFieldNames(resultColumns) + " from " + 
sqlGenerator.escape(datatable) + " where "
+                + escapeFieldNames(columnFilter) + " = ?";
+        SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql, 
validatedValueFilter);
 
         String[] resultColumnNames = resultColumns.split(",");
         List<JsonObject> results = new ArrayList<>();
@@ -233,6 +214,18 @@ public class ReadWriteNonCoreDataServiceImpl implements 
ReadWriteNonCoreDataServ
         return results;
     }
 
+    private Object getValidatedValueFilter(String columnFilter, String 
valueFilter,
+            List<ResultsetColumnHeaderData> resultsetColumnHeaderData) {
+        ResultsetColumnHeaderData columnFilterHeader = 
resultsetColumnHeaderData.stream()
+                .filter(headerData -> 
headerData.getColumnName().equals(columnFilter)).findAny().orElseThrow();
+        if (MYSQL_DATETIME_TYPE.equals(columnFilterHeader.getColumnType())
+                || 
POSTGRES_TIMESTAMP_TYPE.equals(columnFilterHeader.getColumnType())) {
+            return validateColumn(columnFilterHeader, valueFilter, 
DATE_TIME_FORMAT, ENGLISH);
+        } else {
+            return validateColumn(columnFilterHeader, valueFilter, 
DATE_FORMAT, ENGLISH);
+        }
+    }
+
     private void extractResults(SqlRowSet rowSet, String[] resultColumnNames, 
List<JsonObject> results) {
         JsonObject json = new JsonObject();
         for (String rcn : resultColumnNames) {
@@ -264,77 +257,6 @@ public class ReadWriteNonCoreDataServiceImpl implements 
ReadWriteNonCoreDataServ
         }
     }
 
-    @NotNull
-    private SqlRowSet callFilteredMysql(String sql, String valueFilter, String 
filterColumnType) {
-        Object finalValueFilter = valueFilter;
-        SqlRowSet rowSet;
-        if (BIT_TYPE.equalsIgnoreCase(filterColumnType)) {
-            int[] argType = new int[1];
-            argType[0] = Types.BIT;
-            finalValueFilter = 
BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "1", "0", 
"null");
-            rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { 
finalValueFilter }, argType);
-        } else if (MYSQL_DATE_TYPE.equalsIgnoreCase(filterColumnType)) {
-            int[] argType = new int[1];
-            argType[0] = Types.DATE;
-            try {
-                rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { 
LocalDate.parse(valueFilter, DATA_TABLE_DATE_FORMAT) }, argType);
-            } catch (DateTimeParseException e) {
-                List<ApiParameterError> paramErrors = new ArrayList<>();
-                paramErrors.add(parameterErrorWithValue("400",
-                        "Unsupported input type for datatable query! Use 
format: 'yyyy-MM-dd'. Column filter: " + filterColumnType,
-                        "valueFilter", valueFilter));
-                throw new PlatformApiDataValidationException(paramErrors, e);
-            }
-        } else if (MYSQL_DATETIME_TYPE.equals(filterColumnType)) {
-            int[] argType = new int[1];
-            argType[0] = Types.TIMESTAMP;
-            try {
-                rowSet = jdbcTemplate.queryForRowSet(sql, new Object[] { 
LocalDateTime.parse(valueFilter, DATA_TABLE_DATETIME_FORMAT) },
-                        argType);
-            } catch (DateTimeParseException e) {
-                List<ApiParameterError> paramErrors = new ArrayList<>();
-                paramErrors.add(parameterErrorWithValue("400",
-                        "Unsupported input type for datatable query! Use 
format: 'yyyy-MM-dd HH:mm:ss'. Column filter: " + filterColumnType,
-                        "valueFilter", valueFilter));
-                throw new PlatformApiDataValidationException(paramErrors, e);
-            }
-        } else {
-            rowSet = jdbcTemplate.queryForRowSet(sql, finalValueFilter);
-        }
-        return rowSet;
-    }
-
-    @NotNull
-    private SqlRowSet callFilteredPgSql(String sql, String valueFilter, String 
filterColumnType) {
-        Object finalValueFilter = valueFilter;
-        int[] argType = new int[1];
-        if (BIT_TYPE.equalsIgnoreCase(filterColumnType)) {
-            finalValueFilter = 
BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "1", "0", 
"null");
-            argType[0] = Types.BIT;
-        } else if (PG_BOOLEAN_TYPE.equalsIgnoreCase(filterColumnType) || 
PG_BOOL_TYPE.equalsIgnoreCase(filterColumnType)) {
-            finalValueFilter = 
BooleanUtils.toString(BooleanUtils.toBooleanObject(valueFilter), "true", 
"false", "null");
-            argType[0] = Types.BOOLEAN;
-        } else if (PG_INT_TYPE.equalsIgnoreCase(filterColumnType)) {
-            argType[0] = Types.INTEGER;
-        } else if (PG_BIGINT_TYPE.equalsIgnoreCase(filterColumnType)) {
-            argType[0] = Types.BIGINT;
-        } else if (PG_DATE_TYPE.equalsIgnoreCase(filterColumnType)) {
-            argType[0] = Types.DATE;
-        } else if (filterColumnType.toLowerCase().contains("timestamp")) {
-            argType[0] = Types.TIMESTAMP;
-        } else if (PG_NUMERIC_TYPE.equalsIgnoreCase(filterColumnType)) {
-            argType[0] = Types.DECIMAL;
-        } else if (PG_TEXT_TYPE.equalsIgnoreCase(filterColumnType) || 
PG_VARCHAR_TYPE.equalsIgnoreCase(filterColumnType)) {
-            argType[0] = Types.VARCHAR;
-        } else {
-            List<ApiParameterError> paramErrors = new ArrayList<>();
-            paramErrors.add(parameterErrorWithValue("400", "Unsupported input 
type for datatable query! Column filter: " + filterColumnType,
-                    "valueFilter", valueFilter));
-            throw new PlatformApiDataValidationException(paramErrors);
-        }
-        return jdbcTemplate.queryForRowSet(sql, new Object[] { 
finalValueFilter }, argType);
-    }
-
     private String escapeFieldNames(final String inputFields) {
         final String delimiter = ",";
         if (StringUtils.isBlank(inputFields)) {
@@ -348,7 +270,7 @@ public class ReadWriteNonCoreDataServiceImpl implements 
ReadWriteNonCoreDataServ
         return String.join(",", outputFields);
     }
 
-    private static void validateRequestParams(String columnFilter, String 
valueFilter, String resultColumns,
+    private void validateRequestParams(String columnFilter, String 
valueFilter, String resultColumns,
             List<ResultsetColumnHeaderData> resultsetColumnHeaderData) {
         List<ApiParameterError> paramErrors = new ArrayList<>();
         List<String> dataTableColumnNames = 
resultsetColumnHeaderData.stream().map(ResultsetColumnHeaderData::getColumnName).toList();
diff --git 
a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java
 
b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java
index b3d6b96a8..fb1bc4b25 100644
--- 
a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java
+++ 
b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/dataqueries/service/ReadWriteNonCoreDataServiceImplTest.java
@@ -18,23 +18,27 @@
  */
 package org.apache.fineract.infrastructure.dataqueries.service;
 
+import static java.util.Collections.emptyList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 
 import com.google.gson.JsonObject;
-import java.util.Collections;
 import java.util.List;
+import java.util.stream.Stream;
 import 
org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
 import 
org.apache.fineract.infrastructure.core.service.database.DatabaseSpecificSQLGenerator;
 import 
org.apache.fineract.infrastructure.core.service.database.DatabaseTypeResolver;
 import 
org.apache.fineract.infrastructure.dataqueries.data.ResultsetColumnHeaderData;
 import org.apache.fineract.infrastructure.security.utils.SQLInjectionException;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.mockito.ArgumentMatchers;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
@@ -81,44 +85,53 @@ public class ReadWriteNonCoreDataServiceImplTest {
     @Test
     public void testQueryDataTableSuccess() {
         SqlRowSet sqlRS = Mockito.mock(SqlRowSet.class);
-        when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where 
cf1 = ?"), any(Object[].class), any(int[].class)))
-                .thenReturn(sqlRS);
+        when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where 
cf1 = ?"), any(Object.class))).thenReturn(sqlRS);
         when(sqlRS.next()).thenReturn(true).thenReturn(false);
-        
when(sqlRS.getObject(ArgumentMatchers.anyString())).thenReturn("value1").thenReturn("value2");
-        
when(sqlGenerator.escape(ArgumentMatchers.anyString())).thenReturn("rc1").thenReturn("rc2").thenReturn("table").thenReturn("cf1");
+        
when(sqlRS.getObject(anyString())).thenReturn("value1").thenReturn("value2");
+        
when(sqlGenerator.escape(anyString())).thenReturn("rc1").thenReturn("rc2").thenReturn("table").thenReturn("cf1");
         when(databaseTypeResolver.isPostgreSQL()).thenReturn(true);
 
-        ResultsetColumnHeaderData cf1 = 
ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, null, 
null, false, false);
-        ResultsetColumnHeaderData rc1 = 
ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, null, 
null, false, false);
-        ResultsetColumnHeaderData rc2 = 
ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, null, 
null, false, false);
+        ResultsetColumnHeaderData cf1 = 
ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, 
emptyList(), null, false,
+                false);
+        ResultsetColumnHeaderData rc1 = 
ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, 
emptyList(), null, false,
+                false);
+        ResultsetColumnHeaderData rc2 = 
ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, 
emptyList(), null, false,
+                false);
         
when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(List.of(cf1,
 rc1, rc2));
 
         List<JsonObject> results = underTest.queryDataTable("table", "cf1", 
"vf1", "rc1,rc2");
 
-        Assertions.assertEquals("value1", 
results.get(0).get("rc1").getAsString());
-        Assertions.assertEquals("value2", 
results.get(0).get("rc2").getAsString());
+        assertEquals("value1", results.get(0).get("rc1").getAsString());
+        assertEquals("value2", results.get(0).get("rc2").getAsString());
     }
 
     @Test
     public void testQueryDataTableValidationError() {
-        
when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(Collections.emptyList());
+        
when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(emptyList());
         assertThrows(PlatformApiDataValidationException.class, () -> 
underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2"));
     }
 
-    @Test
-    public void testInvalidDatabase() {
-        SqlRowSet sqlRS = Mockito.mock(SqlRowSet.class);
-        when(jdbcTemplate.queryForRowSet(eq("select rc1,rc2 from table where 
cf1 = ?"), any(Object[].class), any(int[].class)))
-                .thenReturn(sqlRS);
-        when(sqlRS.next()).thenReturn(true).thenReturn(false);
-        
when(sqlRS.getObject(ArgumentMatchers.anyString())).thenReturn("value1").thenReturn("value2");
-        when(databaseTypeResolver.isPostgreSQL()).thenReturn(false);
-        when(databaseTypeResolver.isMySQL()).thenReturn(false);
-        ResultsetColumnHeaderData cf1 = 
ResultsetColumnHeaderData.detailed("cf1", "text", 10L, false, false, null, 
null, false, false);
-        ResultsetColumnHeaderData rc1 = 
ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, null, 
null, false, false);
-        ResultsetColumnHeaderData rc2 = 
ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, null, 
null, false, false);
+    @ParameterizedTest
+    @MethodSource("provideParameters")
+    public void testQueryDataTableInvalidParameterError(String columnType, 
String errorMessage) {
+        ResultsetColumnHeaderData cf1 = 
ResultsetColumnHeaderData.detailed("cf1", columnType, 10L, false, false, 
emptyList(), null, false,
+                false);
+        ResultsetColumnHeaderData rc1 = 
ResultsetColumnHeaderData.detailed("rc1", "text", 10L, false, false, 
emptyList(), null, false,
+                false);
+        ResultsetColumnHeaderData rc2 = 
ResultsetColumnHeaderData.detailed("rc2", "text", 10L, false, false, 
emptyList(), null, false,
+                false);
         
when(genericDataService.fillResultsetColumnHeaders("table")).thenReturn(List.of(cf1,
 rc1, rc2));
 
-        assertThrows(IllegalStateException.class, () -> 
underTest.queryDataTable("table", "cf1", "vf1", "rc1,rc2"));
+        PlatformApiDataValidationException thrown = 
assertThrows(PlatformApiDataValidationException.class,
+                () -> underTest.queryDataTable("table", "cf1", "vf1", 
"rc1,rc2"));
+
+        assertEquals(1, thrown.getErrors().size());
+        assertEquals(errorMessage, 
thrown.getErrors().get(0).getUserMessageGlobalisationCode());
+    }
+
+    private static Stream<Arguments> provideParameters() {
+        return Stream.of(Arguments.of("timestamp without time zone", 
"validation.msg.invalid.dateFormat.format"),
+                Arguments.of("INTEGER", 
"validation.msg.invalid.integer.format"),
+                Arguments.of("BOOLEAN", 
"validation.msg.invalid.boolean.format"));
     }
 }

Reply via email to