Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Return hint message on identical columns (#3656)

Browse Source 
- Return hint message if identical column names are found in SQL query for postgres, MySQL, mssql, redshift plugin.
- Add a PluginUtils class to hold general utility functions for plugins.
This commit is contained in:
Sumit Kumar 2021-03-24 08:22:49 +05:30 committed by GitHub
parent 992da806bf
commit 56f22edbe8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 358 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/PluginUtils.java vendored Normal file
View File

@ -0,0 +1,50 @@
package com.appsmith.external.helpers;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
public class PluginUtils {
public static List<String> getColumnsListForJdbcPlugin(ResultSetMetaData metaData) throws SQLException {
List<String> columnsList = IntStream
.range(1, metaData.getColumnCount()+1) // JDBC column indexes start from 1
.mapToObj(i -> {
try {
return metaData.getColumnName(i);
} catch (SQLException exception) {
/*
* - Need suggestions on alternative ways of handling this exception.
*/
throw new RuntimeException(exception);
}
})
.collect(Collectors.toList());
return columnsList;
}
public static List<String> getIdenticalColumns(List<String> columnNames) {
/*
* - Get frequency of each column name
*/
Map<String, Long> columnFrequencies = columnNames
.stream()
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
/*
* - Filter only the inputs which have frequency greater than 1
*/
List<String> identicalColumns = columnFrequencies.entrySet().stream()
.filter(entry -> entry.getValue() > 1)
.map(entry -> entry.getKey())
.collect(Collectors.toList());
return identicalColumns;
}
}

8
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/ActionExecutionResult.java vendored
View File

@ -6,6 +6,8 @@ import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import java.util.Set;
@Getter
@Setter
@ToString
@ -17,6 +19,12 @@ public class ActionExecutionResult {
Object body;
Boolean isExecutionSuccess = false;
/*
* - To return useful hints to the user.
* - E.g. if sql query result has identical columns
*/
Set<String> messages;
ActionExecutionRequest request;
}

19
app/server/appsmith-plugins/mssqlPlugin/src/main/java/com/external/plugins/MssqlPlugin.java vendored
View File

@ -54,6 +54,8 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
import static com.appsmith.external.helpers.PluginUtils.getColumnsListForJdbcPlugin;
import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns;
import static com.appsmith.external.models.Connection.Mode.READ_ONLY;
import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;
@ -167,6 +169,7 @@ public class MssqlPlugin extends BasePlugin {
}
List<Map<String, Object>> rowsList = new ArrayList<>(50);
final List<String> columnsList = new ArrayList<>();
Statement statement = null;
PreparedStatement preparedQuery = null;
@ -208,6 +211,7 @@ public class MssqlPlugin extends BasePlugin {
} else {
ResultSetMetaData metaData = resultSet.getMetaData();
int colCount = metaData.getColumnCount();
columnsList.addAll(getColumnsListForJdbcPlugin(metaData));
while (resultSet.next()) {
// Use `LinkedHashMap` here so that the column ordering is preserved in the response.
@ -287,6 +291,7 @@ public class MssqlPlugin extends BasePlugin {
ActionExecutionResult result = new ActionExecutionResult();
result.setBody(objectMapper.valueToTree(rowsList));
result.setMessages(populateHintMessages(columnsList));
result.setIsExecutionSuccess(true);
System.out.println(Thread.currentThread().getName() + ": In the MssqlPlugin, got action execution result");
return Mono.just(result);
@ -316,6 +321,20 @@ public class MssqlPlugin extends BasePlugin {
.subscribeOn(scheduler);
}
private Set<String> populateHintMessages(List<String> columnNames) {
Set<String> messages = new HashSet<>();
List<String> identicalColumns = getIdenticalColumns(columnNames);
if(!CollectionUtils.isEmpty(identicalColumns)) {
messages.add("Your MsSQL query result may not have all the columns because duplicate column names " +
"were found for the column(s): " + String.join(", ", identicalColumns) + ". You may use the " +
"SQL keyword 'as' to rename the duplicate column name(s) and resolve this issue.");
}
return messages;
}
@Override
public Mono<Connection> datasourceCreate(DatasourceConfiguration datasourceConfiguration) {

44
app/server/appsmith-plugins/mssqlPlugin/src/test/java/com/external/plugins/MssqlPluginTest.java vendored
View File

@ -27,11 +27,17 @@ import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@ -505,4 +511,42 @@ public class MssqlPluginTest {
.verifyComplete();
}
@Test
public void testDuplicateColumnNames() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<Connection> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setBody("SELECT id, username as id, password, email as password FROM users WHERE id = 1");
Mono<ActionExecutionResult> executeMono = dsConnectionMono
.flatMap(conn -> pluginExecutor.executeParameterized(conn, new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(result -> {
assertNotEquals(0, result.getMessages().size());
String expectedMessage = "Your MsSQL query result may not have all the columns because duplicate " +
"column names were found for the column(s)";
assertTrue(
result.getMessages().stream()
.anyMatch(message -> message.contains(expectedMessage))
);
/*
* - Check if all of the duplicate column names are reported.
*/
Set<String> expectedColumnNames = Stream.of("id", "password")
.collect(Collectors.toCollection(HashSet::new));
Set<String> foundColumnNames = new HashSet<>();
result.getMessages().stream()
.filter(message -> message.contains(expectedMessage))
.forEach(message -> {
Arrays.stream(message.split(":")[1].split("\\.")[0].split(","))
.forEach(columnName -> foundColumnNames.add(columnName.trim()));
});
assertTrue(expectedColumnNames.equals(foundColumnNames));
})
.verifyComplete();
}
}

22
app/server/appsmith-plugins/mysqlPlugin/src/main/java/com/external/plugins/MySqlPlugin.java vendored
View File

@ -57,6 +57,7 @@ import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns;
import static io.r2dbc.spi.ConnectionFactoryOptions.SSL;
import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;
@ -212,6 +213,7 @@ public class MySqlPlugin extends BasePlugin {
boolean isSelectOrShowQuery = getIsSelectOrShowQuery(query);
final List<Map<String, Object>> rowsList = new ArrayList<>(50);
final List<String> columnsList = new ArrayList<>();
Flux<Result> resultFlux = Mono.from(connection.validate(ValidationDepth.REMOTE))
.flatMapMany(isValid -> {
@ -233,6 +235,11 @@ public class MySqlPlugin extends BasePlugin {
.flatMap(result ->
result.map((row, meta) -> {
rowsList.add(getRow(row, meta));
if(columnsList.isEmpty()) {
columnsList.addAll(meta.getColumnNames());
}
return result;
}
)
@ -259,6 +266,7 @@ public class MySqlPlugin extends BasePlugin {
.map(res -> {
ActionExecutionResult result = new ActionExecutionResult();
result.setBody(objectMapper.valueToTree(rowsList));
result.setMessages(populateHintMessages(columnsList));
result.setIsExecutionSuccess(true);
System.out.println(Thread.currentThread().getName() + " In the MySqlPlugin, got action " +
"execution result");
@ -333,6 +341,20 @@ public class MySqlPlugin extends BasePlugin {
}
private Set<String> populateHintMessages(List<String> columnNames) {
Set<String> messages = new HashSet<>();
List<String> identicalColumns = getIdenticalColumns(columnNames);
if(!CollectionUtils.isEmpty(identicalColumns)) {
messages.add("Your MySQL query result may not have all the columns because duplicate column names " +
"were found for the column(s): " + String.join(", ", identicalColumns) + ". You may use the " +
"SQL keyword 'as' to rename the duplicate column name(s) and resolve this issue.");
}
return messages;
}
/**
* 1. Parse the actual row objects returned by r2dbc driver for mysql statements.
* 2. Return the row as a map {column_name -> column_value}.

41
app/server/appsmith-plugins/mysqlPlugin/src/test/java/com/external/plugins/MySqlPluginTest.java vendored
View File

@ -28,10 +28,14 @@ import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
@ -696,4 +700,41 @@ public class MySqlPluginTest {
.verifyComplete();
}
public void testDuplicateColumnNames() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<Connection> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setBody("SELECT id, username as id, password, email as password FROM users WHERE id = 1");
Mono<ActionExecutionResult> executeMono = dsConnectionMono
.flatMap(conn -> pluginExecutor.executeParameterized(conn, new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(result -> {
assertNotEquals(0, result.getMessages().size());
String expectedMessage = "Your MySQL query result may not have all the columns because duplicate column names " +
"were found for the column(s)";
assertTrue(
result.getMessages().stream()
.anyMatch(message -> message.contains(expectedMessage))
);
/*
* - Check if all of the duplicate column names are reported.
*/
Set<String> expectedColumnNames = Stream.of("id", "password")
.collect(Collectors.toCollection(HashSet::new));
Set<String> foundColumnNames = new HashSet<>();
result.getMessages().stream()
.filter(message -> message.contains(expectedMessage))
.forEach(message -> {
Arrays.stream(message.split(":")[1].split("\\.")[0].split(","))
.forEach(columnName -> foundColumnNames.add(columnName.trim()));
});
assertTrue(expectedColumnNames.equals(foundColumnNames));
})
.verifyComplete();
}
}

20
app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java vendored
View File

@ -59,6 +59,8 @@ import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static com.appsmith.external.helpers.PluginUtils.getColumnsListForJdbcPlugin;
import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns;
import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;
@ -212,6 +214,7 @@ public class PostgresPlugin extends BasePlugin {
}
List<Map<String, Object>> rowsList = new ArrayList<>(50);
final List<String> columnsList = new ArrayList<>();
Statement statement = null;
ResultSet resultSet = null;
@ -272,8 +275,10 @@ public class PostgresPlugin extends BasePlugin {
ResultSetMetaData metaData = resultSet.getMetaData();
int colCount = metaData.getColumnCount();
columnsList.addAll(getColumnsListForJdbcPlugin(metaData));
while (resultSet.next()) {
// Use `LinkedHashMap` here so that the column ordering is preserved in the response.
Map<String, Object> row = new LinkedHashMap<>(colCount);
@ -374,6 +379,7 @@ public class PostgresPlugin extends BasePlugin {
ActionExecutionResult result = new ActionExecutionResult();
result.setBody(objectMapper.valueToTree(rowsList));
result.setMessages(populateHintMessages(columnsList));
result.setIsExecutionSuccess(true);
System.out.println(Thread.currentThread().getName() + ": In the PostgresPlugin, got action execution result");
return Mono.just(result);
@ -405,6 +411,20 @@ public class PostgresPlugin extends BasePlugin {
}
private Set<String> populateHintMessages(List<String> columnNames) {
Set<String> messages = new HashSet<>();
List<String> identicalColumns = getIdenticalColumns(columnNames);
if(!CollectionUtils.isEmpty(identicalColumns)) {
messages.add("Your PostgreSQL query result may not have all the columns because duplicate column " +
"names were found for the column(s): " + String.join(", ", identicalColumns) + ". You may use" +
" the SQL keyword 'as' to rename the duplicate column name(s) and resolve this issue.");
}
return messages;
}
@Override
public Mono<ActionExecutionResult> execute(HikariDataSource connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) {
// Unused function

47
app/server/appsmith-plugins/postgresPlugin/src/test/java/com/external/plugins/PostgresPluginTest.java vendored
View File

@ -28,14 +28,19 @@ import java.sql.DriverManager;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@ -855,4 +860,46 @@ public class PostgresPluginTest {
assertTrue(error.getMessage().contains("The server does not support SSL"));
});
}
public void testDuplicateColumnNames() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<HikariDataSource> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setBody("SELECT id, username as id, password, email as password FROM users WHERE id = 1");
List<Property> pluginSpecifiedTemplates = new ArrayList<>();
pluginSpecifiedTemplates.add(new Property("preparedStatement", "false"));
actionConfiguration.setPluginSpecifiedTemplates(pluginSpecifiedTemplates);
Mono<ActionExecutionResult> executeMono = dsConnectionMono
.flatMap(conn -> pluginExecutor.executeParameterized(conn, new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(result -> {
assertNotEquals(0, result.getMessages().size());
String expectedMessage = "Your PostgreSQL query result may not have all the columns because " +
"duplicate column names were found for the column(s)";
assertTrue(
result.getMessages().stream()
.anyMatch(message -> message.contains(expectedMessage))
);
/*
* - Check if all of the duplicate column names are reported.
*/
Set<String> expectedColumnNames = Stream.of("id", "password")
.collect(Collectors.toCollection(HashSet::new));
Set<String> foundColumnNames = new HashSet<>();
result.getMessages().stream()
.filter(message -> message.contains(expectedMessage))
.forEach(message -> {
Arrays.stream(message.split(":")[1].split("\\.")[0].split(","))
.forEach(columnName -> foundColumnNames.add(columnName.trim()));
});
assertTrue(expectedColumnNames.equals(foundColumnNames));
})
.verifyComplete();
}
}

20
app/server/appsmith-plugins/redshiftPlugin/src/main/java/com/external/plugins/RedshiftPlugin.java vendored
View File

@ -45,6 +45,8 @@ import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;
import static com.appsmith.external.helpers.PluginUtils.getColumnsListForJdbcPlugin;
import static com.appsmith.external.helpers.PluginUtils.getIdenticalColumns;
import static com.appsmith.external.models.Connection.Mode.READ_ONLY;
@ -238,6 +240,7 @@ public class RedshiftPlugin extends BasePlugin {
}
List<Map<String, Object>> rowsList = new ArrayList<>(50);
final List<String> columnsList = new ArrayList<>();
Statement statement = null;
ResultSet resultSet = null;
@ -247,6 +250,8 @@ public class RedshiftPlugin extends BasePlugin {
if (isResultSet) {
resultSet = statement.getResultSet();
ResultSetMetaData metaData = resultSet.getMetaData();
columnsList.addAll(getColumnsListForJdbcPlugin(metaData));
while (resultSet.next()) {
Map<String, Object> row = getRow(resultSet);
@ -281,6 +286,7 @@ public class RedshiftPlugin extends BasePlugin {
ActionExecutionResult result = new ActionExecutionResult();
result.setBody(objectMapper.valueToTree(rowsList));
result.setMessages(populateHintMessages(columnsList));
result.setIsExecutionSuccess(true);
System.out.println(
Thread.currentThread().getName() + ": " +
@ -320,6 +326,20 @@ public class RedshiftPlugin extends BasePlugin {
.subscribeOn(scheduler);
}
private Set<String> populateHintMessages(List<String> columnNames) {
Set<String> messages = new HashSet<>();
List<String> identicalColumns = getIdenticalColumns(columnNames);
if(!CollectionUtils.isEmpty(identicalColumns)) {
messages.add("Your Redshift query result may not have all the columns because duplicate column names " +
"were found for the column(s): " + String.join(", ", identicalColumns) + ". You may use the " +
"SQL keyword 'as' to rename the duplicate column name(s) and resolve this issue.");
}
return messages;
}
@Override
public Mono<Connection> datasourceCreate(DatasourceConfiguration datasourceConfiguration) {
try {

88
app/server/appsmith-plugins/redshiftPlugin/src/test/java/com/external/plugins/RedshiftPluginTest.java vendored
View File

@ -1,5 +1,6 @@
package com.external.plugins;
import com.appsmith.external.helpers.PluginUtils;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.ActionExecutionResult;
import com.appsmith.external.models.DBAuth;
@ -30,12 +31,17 @@ import java.sql.Statement;
import java.sql.Time;
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doNothing;
@ -222,7 +228,7 @@ public class RedshiftPluginTest {
*/
ResultSetMetaData mockResultSetMetaData = mock(ResultSetMetaData.class);
when(mockResultSet.getMetaData()).thenReturn(mockResultSetMetaData);
when(mockResultSetMetaData.getColumnCount()).thenReturn(10);
when(mockResultSetMetaData.getColumnCount()).thenReturn(0).thenReturn(10);
when(mockResultSetMetaData.getColumnTypeName(Mockito.anyInt())).thenReturn("int4", "varchar", "varchar",
"varchar", "date", "date", "time", "timetz", "timestamp", "timestamptz");
when(mockResultSetMetaData.getColumnName(Mockito.anyInt())).thenReturn("id", "username", "password", "email",
@ -450,4 +456,84 @@ public class RedshiftPluginTest {
})
.verifyComplete();
}
@Test
public void testDuplicateColumnNames() throws SQLException {
/* Mock java.sql.Connection:
* a. isClosed()
* b. isValid()
*/
Connection mockConnection = mock(Connection.class);
when(mockConnection.isClosed()).thenReturn(false);
when(mockConnection.isValid(Mockito.anyInt())).thenReturn(true);
/* Mock java.sql.Statement:
* a. execute(...)
* b. close()
*/
Statement mockStatement = mock(Statement.class);
when(mockConnection.createStatement()).thenReturn(mockStatement);
when(mockStatement.execute(Mockito.any())).thenReturn(true);
doNothing().when(mockStatement).close();
/* Mock java.sql.ResultSet:
* a. getObject(...)
* d. next()
* e. close()
*/
ResultSet mockResultSet = mock(ResultSet.class);
when(mockStatement.getResultSet()).thenReturn(mockResultSet);
when(mockResultSet.getObject(Mockito.anyInt())).thenReturn("", 1, "", 1, "", "jill", "", "jill");
when(mockResultSet.next()).thenReturn(true).thenReturn(false);
doNothing().when(mockResultSet).close();
/* Mock java.sql.ResultSetMetaData:
* a. getColumnCount()
* b. getColumnTypeName(...)
* c. getColumnName(...)
*/
ResultSetMetaData mockResultSetMetaData = mock(ResultSetMetaData.class);
when(mockResultSet.getMetaData()).thenReturn(mockResultSetMetaData);
when(mockResultSetMetaData.getColumnCount()).thenReturn(4);
when(mockResultSetMetaData.getColumnTypeName(Mockito.anyInt())).thenReturn("int4", "int4", "varchar",
"varchar");
when(mockResultSetMetaData.getColumnName(Mockito.anyInt())).thenReturn("id", "id", "username", "username");
ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setBody("SELECT id, id, username, username FROM users WHERE id = 1");
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<Connection> dsConnectionMono = Mono.just(mockConnection);
Mono<ActionExecutionResult> executeMono = dsConnectionMono
.flatMap(conn -> pluginExecutor.execute(conn, dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(result -> {
assertNotNull(result);
assertTrue(result.getIsExecutionSuccess());
assertNotEquals(0, result.getMessages().size());
String expectedMessage = "Your Redshift query result may not have all the columns because " +
"duplicate column names were found for the column(s)";
assertTrue(
result.getMessages().stream()
.anyMatch(message -> message.contains(expectedMessage))
);
/*
* - Check if all of the duplicate column names are reported.
*/
Set<String> expectedColumnNames = Stream.of("id", "username")
.collect(Collectors.toCollection(HashSet::new));
Set<String> foundColumnNames = new HashSet<>();
result.getMessages().stream()
.filter(message -> message.contains(expectedMessage))
.forEach(message -> {
Arrays.stream(message.split(":")[1].split("\\.")[0].split(","))
.forEach(columnName -> foundColumnNames.add(columnName.trim()));
});
assertTrue(expectedColumnNames.equals(foundColumnNames));
})
.verifyComplete();
}
}