From abdd2337b144df149a2031c3b26862ae41b4e936 Mon Sep 17 00:00:00 2001 From: Wenjun <861923274@qq.com> Date: Thu, 22 Apr 2021 12:02:32 +0800 Subject: [PATCH] [Bug][worker] Fix sql task query result is empty (#5322) (#5324) * [Bug][worker] Fix sql task query result is empty (#5322) * add ut * remove duplicate * add ut --- .../server/worker/task/sql/SqlTask.java | 47 ++++++++++--------- .../server/worker/task/sql/SqlTaskTest.java | 39 ++++++++++++++- pom.xml | 1 + 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java index 06df56491d..ed803fc952 100644 --- a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java +++ b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java @@ -304,34 +304,37 @@ public class SqlTask extends AbstractTask { */ private String resultProcess(ResultSet resultSet) throws Exception { ArrayNode resultJSONArray = JSONUtils.createArrayNode(); - ResultSetMetaData md = resultSet.getMetaData(); - int num = md.getColumnCount(); - - int rowCount = 0; - - while (rowCount < LIMIT && resultSet.next()) { - ObjectNode mapOfColValues = JSONUtils.createObjectNode(); - for (int i = 1; i <= num; i++) { - mapOfColValues.set(md.getColumnLabel(i), JSONUtils.toJsonNode(resultSet.getObject(i))); + if (resultSet != null) { + ResultSetMetaData md = resultSet.getMetaData(); + int num = md.getColumnCount(); + + int rowCount = 0; + + while (rowCount < LIMIT && resultSet.next()) { + ObjectNode mapOfColValues = JSONUtils.createObjectNode(); + for (int i = 1; i <= num; i++) { + mapOfColValues.set(md.getColumnLabel(i), JSONUtils.toJsonNode(resultSet.getObject(i))); + } + resultJSONArray.add(mapOfColValues); + rowCount++; } - resultJSONArray.add(mapOfColValues); - rowCount++; - } - String result = JSONUtils.toJsonString(resultJSONArray); - logger.debug("execute sql result : {}", result); - int displayRows = sqlParameters.getDisplayRows() > 0 ? sqlParameters.getDisplayRows() : Constants.DEFAULT_DISPLAY_ROWS; - displayRows = Math.min(displayRows, resultJSONArray.size()); - logger.info("display sql result {} rows as follows:", displayRows); - for (int i = 0; i < displayRows; i++) { - String row = JSONUtils.toJsonString(resultJSONArray.get(i)); - logger.info("row {} : {}", i + 1, row); + int displayRows = sqlParameters.getDisplayRows() > 0 ? sqlParameters.getDisplayRows() : Constants.DEFAULT_DISPLAY_ROWS; + displayRows = Math.min(displayRows, resultJSONArray.size()); + logger.info("display sql result {} rows as follows:", displayRows); + for (int i = 0; i < displayRows; i++) { + String row = JSONUtils.toJsonString(resultJSONArray.get(i)); + logger.info("row {} : {}", i + 1, row); + } } + String result = JSONUtils.toJsonString(resultJSONArray); if (sqlParameters.getSendEmail() == null || sqlParameters.getSendEmail()) { - sendAttachment(sqlParameters.getGroupId(), StringUtils.isNotEmpty(sqlParameters.getTitle()) ? sqlParameters.getTitle() : taskExecutionContext.getTaskName() + " query result sets", - JSONUtils.toJsonString(resultJSONArray)); + sendAttachment(sqlParameters.getGroupId(), StringUtils.isNotEmpty(sqlParameters.getTitle()) + ? sqlParameters.getTitle() + : taskExecutionContext.getTaskName() + " query result sets", result); } + logger.debug("execute sql result : {}", result); return result; } diff --git a/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java b/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java index 2abb91c6b6..29964ca8c7 100644 --- a/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java +++ b/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTaskTest.java @@ -20,6 +20,8 @@ package org.apache.dolphinscheduler.server.worker.task.sql; import org.apache.dolphinscheduler.common.Constants; import org.apache.dolphinscheduler.common.utils.ParameterUtils; import org.apache.dolphinscheduler.dao.AlertDao; +import org.apache.dolphinscheduler.dao.datasource.BaseDataSource; +import org.apache.dolphinscheduler.remote.command.alert.AlertSendResponseCommand; import org.apache.dolphinscheduler.server.entity.SQLTaskExecutionContext; import org.apache.dolphinscheduler.server.entity.TaskExecutionContext; import org.apache.dolphinscheduler.server.worker.task.TaskProps; @@ -29,6 +31,8 @@ import org.apache.dolphinscheduler.service.bean.SpringApplicationContext; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; import java.util.Date; import org.junit.Assert; @@ -39,6 +43,7 @@ import org.mockito.Mockito; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,7 +51,8 @@ import org.slf4j.LoggerFactory; * sql task test */ @RunWith(PowerMockRunner.class) -@PrepareForTest(value = {SqlTask.class, DriverManager.class, SpringApplicationContext.class, ParameterUtils.class}) +@PrepareForTest(value = {SqlTask.class, DriverManager.class, SpringApplicationContext.class, + ParameterUtils.class, AlertSendResponseCommand.class, BaseDataSource.class}) public class SqlTaskTest { private static final Logger logger = LoggerFactory.getLogger(SqlTaskTest.class); @@ -112,6 +118,35 @@ public class SqlTaskTest { PowerMockito.when(ParameterUtils.replaceScheduleTime(Mockito.any(), Mockito.any())).thenReturn("insert into tb_1 values('1','2')"); sqlTask.handle(); - Assert.assertEquals(Constants.EXIT_CODE_SUCCESS,sqlTask.getExitStatusCode()); + Assert.assertEquals(Constants.EXIT_CODE_SUCCESS, sqlTask.getExitStatusCode()); + } + + @Test + public void testResultProcess() throws Exception { + // test input null and will not throw a exception + AlertSendResponseCommand mockResponseCommand = PowerMockito.mock(AlertSendResponseCommand.class); + PowerMockito.when(mockResponseCommand.getResStatus()).thenReturn(true); + PowerMockito.when(alertClientService.sendAlert(0, "null query result sets", "[]")).thenReturn(mockResponseCommand); + String result = Whitebox.invokeMethod(sqlTask, "resultProcess", null); + Assert.assertNotNull(result); + } + + @Test + public void testResultProcess02() throws Exception { + // test input not null + ResultSet resultSet = PowerMockito.mock(ResultSet.class); + ResultSetMetaData mockResultMetaData = PowerMockito.mock(ResultSetMetaData.class); + PowerMockito.when(resultSet.getMetaData()).thenReturn(mockResultMetaData); + PowerMockito.when(mockResultMetaData.getColumnCount()).thenReturn(2); + PowerMockito.when(resultSet.next()).thenReturn(true); + PowerMockito.when(resultSet.getObject(Mockito.anyInt())).thenReturn(1); + PowerMockito.when(mockResultMetaData.getColumnLabel(Mockito.anyInt())).thenReturn("a"); + + AlertSendResponseCommand mockResponseCommand = PowerMockito.mock(AlertSendResponseCommand.class); + PowerMockito.when(mockResponseCommand.getResStatus()).thenReturn(true); + PowerMockito.when(alertClientService.sendAlert(Mockito.anyInt(), Mockito.anyString(), Mockito.anyString())).thenReturn(mockResponseCommand); + + String result = Whitebox.invokeMethod(sqlTask, "resultProcess", resultSet); + Assert.assertNotNull(result); } } diff --git a/pom.xml b/pom.xml index 3be973330d..06633472a0 100644 --- a/pom.xml +++ b/pom.xml @@ -963,6 +963,7 @@ **/server/worker/task/TaskManagerTest.java **/server/worker/task/AbstractCommandExecutorTest.java **/server/worker/task/ShellTaskReturnTest.java + **/server/worker/task/sql/SqlTaskTest.java **/server/worker/EnvFileTest.java **/server/worker/runner/TaskExecuteThreadTest.java **/server/worker/runner/WorkerManagerThreadTest.java