Browse Source

[Bug][WorkerServer] SqlTask NullPointerException (#5556)

* [Bug][WorkerServer] add null checks to sqlParamsMap and sqlParamsMap.get(i), Add test to verify it

* Change Test Name

* Fix Code style issues, Modify checking null for Sql params in sqlParamsMap with clearer logging.
2.0.7-release
LOUKHNATI Mohamed Khalil 3 years ago committed by GitHub
parent
commit
86ffc1f3f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 21
      dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
  2. 33
      dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java

21
dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java

@ -473,8 +473,16 @@ public class SqlTask extends AbstractTask {
String paramName = m.group(1); String paramName = m.group(1);
Property prop = paramsPropsMap.get(paramName); Property prop = paramsPropsMap.get(paramName);
sqlParamsMap.put(index, prop); if (prop == null) {
index++; logger.error("setSqlParamsMap: No Property with paramName: {} is found in paramsPropsMap of task instance"
+ " with id: {}. So couldn't put Property in sqlParamsMap.", paramName, taskExecutionContext.getTaskInstanceId());
}
else {
sqlParamsMap.put(index, prop);
index++;
logger.info("setSqlParamsMap: Property with paramName: {} put in sqlParamsMap of content {} successfully.", paramName, content);
}
} }
} }
@ -490,8 +498,13 @@ public class SqlTask extends AbstractTask {
//parameter print style //parameter print style
logger.info("after replace sql , preparing : {}", formatSql); logger.info("after replace sql , preparing : {}", formatSql);
StringBuilder logPrint = new StringBuilder("replaced sql , parameters:"); StringBuilder logPrint = new StringBuilder("replaced sql , parameters:");
for (int i = 1; i <= sqlParamsMap.size(); i++) { if (sqlParamsMap == null) {
logPrint.append(sqlParamsMap.get(i).getValue() + "(" + sqlParamsMap.get(i).getType() + ")"); logger.info("printReplacedSql: sqlParamsMap is null.");
}
else {
for (int i = 1; i <= sqlParamsMap.size(); i++) {
logPrint.append(sqlParamsMap.get(i).getValue() + "(" + sqlParamsMap.get(i).getType() + ")");
}
} }
logger.info("Sql Params are {}", logPrint); logger.info("Sql Params are {}", logPrint);
} }

33
dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java

@ -17,8 +17,13 @@
package org.apache.dolphinscheduler.server.worker.task.sql; package org.apache.dolphinscheduler.server.worker.task.sql;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.Constants;
import org.apache.dolphinscheduler.common.datasource.DatasourceUtil; import org.apache.dolphinscheduler.common.datasource.DatasourceUtil;
import org.apache.dolphinscheduler.common.process.Property;
import org.apache.dolphinscheduler.common.task.sql.SqlParameters; import org.apache.dolphinscheduler.common.task.sql.SqlParameters;
import org.apache.dolphinscheduler.common.utils.ParameterUtils; import org.apache.dolphinscheduler.common.utils.ParameterUtils;
import org.apache.dolphinscheduler.dao.AlertDao; import org.apache.dolphinscheduler.dao.AlertDao;
@ -34,6 +39,8 @@ import java.sql.PreparedStatement;
import java.sql.ResultSet; import java.sql.ResultSet;
import java.sql.ResultSetMetaData; import java.sql.ResultSetMetaData;
import java.util.Date; import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
@ -118,7 +125,7 @@ public class SqlTaskTest {
PowerMockito.when(DatasourceUtil.getConnection(Mockito.any(), Mockito.any())).thenReturn(connection); PowerMockito.when(DatasourceUtil.getConnection(Mockito.any(), Mockito.any())).thenReturn(connection);
sqlTask.handle(); sqlTask.handle();
Assert.assertEquals(Constants.EXIT_CODE_SUCCESS, sqlTask.getExitStatusCode()); assertEquals(Constants.EXIT_CODE_SUCCESS, sqlTask.getExitStatusCode());
} }
@Test @Test
@ -150,6 +157,30 @@ public class SqlTaskTest {
Assert.assertNotNull(result); Assert.assertNotNull(result);
} }
@Test
public void shouldntThrowNullPointerException_When_SqlParamsMapIsNull_printReplacedSql() {
try {
sqlTask.printReplacedSql("", "", "", null);
assertTrue(true);
} catch (NullPointerException err) {
fail();
}
}
@Test
public void shouldntPutPropertyInSqlParamsMap_When_paramNameIsNotFoundInparamsPropsMap_setSqlParamsMap() {
Map<Integer, Property> sqlParamsMap = new HashMap<>();
Map<String, Property> paramsPropsMap = new HashMap<>();
paramsPropsMap.put("validPropertyName", new Property());
taskExecutionContext = PowerMockito.mock(TaskExecutionContext.class);
PowerMockito.when(taskExecutionContext.getTaskInstanceId()).thenReturn(1);
sqlTask.setSqlParamsMap("notValidPropertyName", "(notValidPropertyName)", sqlParamsMap, paramsPropsMap);
assertEquals(0, sqlParamsMap.size());
}
@Test @Test
public void testQueryBySQLUsingLimit() throws Exception { public void testQueryBySQLUsingLimit() throws Exception {
TaskExecutionContext localTaskExecutionContext; TaskExecutionContext localTaskExecutionContext;

Loading…
Cancel
Save