Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ public ArrayNode filterDataNew(

validateProjectionColumns(uqiDataFilterParams.getProjectionColumns(), schema);
validateSortByColumns(uqiDataFilterParams.getSortBy(), schema);
validateConditionColumns(uqiDataFilterParams.getCondition(), schema);

String tableName = generateTable(schema);
try {
Expand Down Expand Up @@ -867,6 +868,30 @@ public boolean validConditionList(List<Condition> conditionList, Map<String, Dat
return true;
}

public void validateConditionColumns(Condition condition, Map<String, DataType> schema) {
if (condition == null) {
return;
}

ConditionalOperator operator = condition.getOperator();
if (operator == ConditionalOperator.AND || operator == ConditionalOperator.OR) {
Object value = condition.getValue();
if (value instanceof List) {
List<Condition> childConditions = (List<Condition>) value;
for (Condition child : childConditions) {
validateConditionColumns(child, schema);
}
}
} else {
String path = condition.getPath();
if (StringUtils.isNotEmpty(path) && !schema.containsKey(path)) {
throw new AppsmithPluginException(
AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR,
path + " not found in the known column names :" + schema.keySet());
}
}
}

public String generateLogicalExpression(
List<Condition> conditions,
List<PreparedStatementValueDTO> values,
Expand Down Expand Up @@ -898,9 +923,10 @@ public String generateLogicalExpression(
sb.append(" " + logicOp);
}
if (StringUtils.isNotEmpty(path)) {
String escapedPath = path.replace("\"", "\"\"");
if (value == null || value.equals(StringUtils.EMPTY)) {
sb.append(" ( ");
sb.append("\"" + path + "\"");
sb.append("\"" + escapedPath + "\"");
sb.append(" ");
if (Set.of(
ConditionalOperator.EQ,
Expand All @@ -926,7 +952,7 @@ public String generateLogicalExpression(
operator + " is not supported currently for filtering.");
}
sb.append(" ( ");
sb.append("\"" + path + "\"");
sb.append("\"" + escapedPath + "\"");
sb.append(" ");
sb.append(sqlOp);
sb.append(" ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.appsmith.external.constants.ConditionalOperator;
import com.appsmith.external.constants.DataType;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.appsmith.external.models.Condition;
import com.appsmith.external.models.UQIDataFilterParams;
Expand Down Expand Up @@ -1504,4 +1505,68 @@ public void testInvalidProjectionDoesNotLeakTable() throws IOException {
filterDataService.filterDataNew(items, new UQIDataFilterParams(null, List.of("id"), null, null));
assertEquals(2, result.size());
}

private Condition whereClause(Map<String, Object>... children) {
return parseWhereClause(Map.of("condition", "AND", "children", List.of(children)));
}

private void assertUnknownColumnRejected(Condition condition, String columnName) throws IOException {
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);

AppsmithPluginException exception = assertThrows(
AppsmithPluginException.class,
() -> filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null)));

assertEquals(AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, exception.getError());
assertThat(exception.getMessage()).startsWith(columnName + " not found in the known column names");
}

@Test
public void testWhereWithUnknownColumn_throwsException() throws IOException {
Condition condition = whereClause(Map.of("condition", "EQ", "key", "unknownColumn", "value", "1"));

assertUnknownColumnRejected(condition, "unknownColumn");
}

@Test
public void testWhereWithUnknownColumnInNestedGroup_throwsException() throws IOException {
Condition condition = whereClause(
Map.of("condition", "EQ", "key", "id", "value", "1"),
Map.of(
"condition",
"OR",
"children",
List.of(Map.of("condition", "EQ", "key", "unknownColumn", "value", ""))));

assertUnknownColumnRejected(condition, "unknownColumn");
}

@Test
public void testWhereWithColumnNameContainingQuote_throwsException() throws IOException {
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", ""));

assertUnknownColumnRejected(condition, "na\"me");
}

@Test
public void testWhereWithKnownColumn_filtersRows() throws IOException {
ArrayNode items = (ArrayNode) objectMapper.readTree(SIMPLE_DATA);
Condition condition = whereClause(Map.of("condition", "EQ", "key", "id", "value", "1"));

ArrayNode result = filterDataService.filterDataNew(items, new UQIDataFilterParams(condition, null, null, null));

assertEquals(1, result.size());
assertEquals(1, result.get(0).get("id").asInt());
}

@Test
public void testGenerateLogicalExpression_quotesColumnNameContainingQuote() {
Map<String, DataType> schema = Map.of("na\"me", DataType.STRING);
Condition condition = whereClause(Map.of("condition", "EQ", "key", "na\"me", "value", "Alice"));

String expression = filterDataService.generateLogicalExpression(
(List<Condition>) condition.getValue(), new ArrayList<>(), schema, condition.getOperator());

assertThat(expression).isEqualTo(" ( \"na\"\"me\" = ? ) ");
}
}
Loading