From c706b21c621cca6c04f5658abfba5bc4f06415c7 Mon Sep 17 00:00:00 2001 From: liwenhe1993 <32166572+liwenhe1993@users.noreply.github.com> Date: Sat, 28 Mar 2020 10:42:46 +0800 Subject: [PATCH] =?UTF-8?q?taskProps.getScheduleTime()=20may=20be=20null,?= =?UTF-8?q?=20but=20there=20is=20no=20check=20if=20it=20=E2=80=A6=20(#2256?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * taskProps.getScheduleTime() may be null, but there is no check if it is null or not * Add unit test --- .../server/worker/task/shell/ShellTask.java | 17 +++--- .../worker/task/shell/ShellTaskTest.java | 60 ++++++++++++++++--- 2 files changed, 63 insertions(+), 14 deletions(-) diff --git a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTask.java b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTask.java index 165430b5fd..fae514c03d 100644 --- a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTask.java +++ b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTask.java @@ -143,13 +143,16 @@ public class ShellTask extends AbstractTask { taskProps.getCmdTypeIfComplement(), taskProps.getScheduleTime()); -// replace variable TIME with $[YYYYmmddd...] in shell file when history run job and batch complement job - if(paramsMap != null && taskProps.getScheduleTime()!=null) { - String dateTime = DateUtils.format(taskProps.getScheduleTime(), Constants.PARAMETER_FORMAT_TIME); - Property p = new Property(); - p.setValue(dateTime); - p.setProp(Constants.PARAMETER_SHECDULE_TIME); - paramsMap.put(Constants.PARAMETER_SHECDULE_TIME, p); + // new + // replace variable TIME with $[YYYYmmddd...] in shell file when history run job and batch complement job + if (paramsMap != null) { + if (taskProps.getScheduleTime() != null) { + String dateTime = DateUtils.format(taskProps.getScheduleTime(), Constants.PARAMETER_FORMAT_TIME); + Property p = new Property(); + p.setValue(dateTime); + p.setProp(Constants.PARAMETER_SHECDULE_TIME); + paramsMap.put(Constants.PARAMETER_SHECDULE_TIME, p); + } script = ParameterUtils.convertParameterPlaceholders2(script, ParamUtils.convert(paramsMap)); } diff --git a/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTaskTest.java b/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTaskTest.java index ebe90147d1..387c7c5e53 100644 --- a/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTaskTest.java +++ b/dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/shell/ShellTaskTest.java @@ -25,13 +25,10 @@ import org.apache.dolphinscheduler.server.worker.task.ShellCommandExecutor; import org.apache.dolphinscheduler.server.worker.task.TaskProps; import org.apache.dolphinscheduler.service.bean.SpringApplicationContext; import org.apache.dolphinscheduler.service.process.ProcessService; -import org.junit.After; -import org.junit.Assert; -import org.junit.Assume; -import org.junit.Before; -import org.junit.Test; +import org.junit.*; import org.junit.runner.RunWith; import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; import org.slf4j.Logger; @@ -45,6 +42,7 @@ import java.util.Date; */ @RunWith(PowerMockRunner.class) @PrepareForTest(OSUtils.class) +@PowerMockIgnore({"javax.management.*"}) public class ShellTaskTest { private static final Logger logger = LoggerFactory.getLogger(ShellTaskTest.class); @@ -136,6 +134,28 @@ public class ShellTaskTest { } } + @Test + public void testInitException() { + TaskProps props = new TaskProps(); + props.setTaskDir("/tmp"); + props.setTaskAppId(String.valueOf(System.currentTimeMillis())); + props.setTaskInstId(1); + props.setTenantCode("1"); + props.setEnvFile(".dolphinscheduler_env.sh"); + props.setTaskStartTime(new Date()); + props.setTaskTimeout(0); + props.setTaskParams("{\"rawScript\": \"\"}"); + ShellTask shellTask = new ShellTask(props, logger); + try { + shellTask.init(); + } catch (Exception e) { + logger.info(e.getMessage(), e); + if (e.getMessage().contains("shell task params is not valid")) { + Assert.assertTrue(true); + } + } + } + /** * Method: init for Windows */ @@ -157,7 +177,20 @@ public class ShellTaskTest { public void testHandleForUnix() throws Exception { try { PowerMockito.when(OSUtils.isWindows()).thenReturn(false); - shellTask.handle(); + TaskProps props = new TaskProps(); + props.setTaskDir("/tmp"); + props.setTaskAppId(String.valueOf(System.currentTimeMillis())); + props.setTaskInstId(1); + props.setTenantCode("1"); + props.setEnvFile(".dolphinscheduler_env.sh"); + props.setTaskStartTime(new Date()); + props.setTaskTimeout(0); + props.setScheduleTime(new Date()); + props.setCmdTypeIfComplement(CommandType.START_PROCESS); + props.setTaskParams("{\"rawScript\": \" echo ${test}\", \"localParams\": [{\"prop\":\"test\", \"direct\":\"IN\", \"type\":\"VARCHAR\", \"value\":\"123\"}]}"); + ShellTask shellTask1 = new ShellTask(props, logger); + shellTask1.init(); + shellTask1.handle(); Assert.assertTrue(true); } catch (Error | Exception e) { if (!e.getMessage().contains("process error . exitCode is : -1") @@ -174,7 +207,20 @@ public class ShellTaskTest { public void testHandleForWindows() throws Exception { try { Assume.assumeTrue(OSUtils.isWindows()); - shellTask.handle(); + TaskProps props = new TaskProps(); + props.setTaskDir("/tmp"); + props.setTaskAppId(String.valueOf(System.currentTimeMillis())); + props.setTaskInstId(1); + props.setTenantCode("1"); + props.setEnvFile(".dolphinscheduler_env.sh"); + props.setTaskStartTime(new Date()); + props.setTaskTimeout(0); + props.setScheduleTime(new Date()); + props.setCmdTypeIfComplement(CommandType.START_PROCESS); + props.setTaskParams("{\"rawScript\": \" echo ${test}\", \"localParams\": [{\"prop\":\"test\", \"direct\":\"IN\", \"type\":\"VARCHAR\", \"value\":\"123\"}]}"); + ShellTask shellTask1 = new ShellTask(props, logger); + shellTask1.init(); + shellTask1.handle(); Assert.assertTrue(true); } catch (Error | Exception e) { if (!e.getMessage().contains("process error . exitCode is : -1")) {