From 04393bca17d0c77aef252f202e1ec0a3f61e975e Mon Sep 17 00:00:00 2001 From: Ilayaperumal Gopinathan Date: Mon, 22 Oct 2018 15:38:30 +0530 Subject: [PATCH] Exit code is nullable - Make exit code nullable and only update of execution status can make it a valid integer - Update JDBC create/start task execution queries to have exitCode as `null` values - Update tests to validate/verify the appropirate exit code values for create/start/complete task executions On merge had to add a pause to the TaskLauncherSinkTests to wait for the task to complete successfully Before we assumed that 0 was a satisfactory result meaning it was either running or completed. Now with the null being returned it could be zero or null. So we have to wait for the task to complete. --- .../cloud/task/repository/TaskExecution.java | 3 ++- .../repository/dao/JdbcTaskExecutionDao.java | 7 +++++-- .../support/SimpleTaskRepository.java | 4 ++-- .../listener/TaskLifecycleListenerTests.java | 15 ++++++++++----- .../SimpleTaskRepositoryJdbcTests.java | 6 ++++++ .../support/SimpleTaskRepositoryMapTests.java | 6 ++++-- .../cloud/task/util/TestDBUtils.java | 4 +++- .../cloud/task/util/TestVerifierUtils.java | 2 +- .../task/launcher/TaskLauncherSinkTests.java | 19 ++++++++++++++++--- 9 files changed, 49 insertions(+), 17 deletions(-) diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/TaskExecution.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/TaskExecution.java index 7409000c..97a35de1 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/TaskExecution.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/TaskExecution.java @@ -27,6 +27,7 @@ import org.springframework.util.Assert; * * @author Glenn Renfro * @author Michael Minella + * @author Ilayaperumal Gopinathan */ public class TaskExecution { @@ -122,7 +123,7 @@ public class TaskExecution { } public Integer getExitCode() { - return (exitCode == null) ? 0 : exitCode; + return this.exitCode; } public void setExitCode(Integer exitCode) { diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/JdbcTaskExecutionDao.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/JdbcTaskExecutionDao.java index 6101ae2d..094d4599 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/JdbcTaskExecutionDao.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/JdbcTaskExecutionDao.java @@ -58,6 +58,7 @@ import org.springframework.util.StringUtils; * @author Glenn Renfro * @author Gunnar Hillert * @author David Turanski + * @author Ilayaperumal Gopinathan */ public class JdbcTaskExecutionDao implements TaskExecutionDao { @@ -75,8 +76,8 @@ public class JdbcTaskExecutionDao implements TaskExecutionDao { public static final String TASK_NAME_WHERE_CLAUSE = "where TASK_NAME = :taskName "; private static final String SAVE_TASK_EXECUTION = "INSERT into %PREFIX%EXECUTION" - + "(TASK_EXECUTION_ID, START_TIME, TASK_NAME, LAST_UPDATED, EXTERNAL_EXECUTION_ID, PARENT_EXECUTION_ID)" - + "values (:taskExecutionId, :startTime, :taskName, :lastUpdated, :externalExecutionId, :parentExecutionId)"; + + "(TASK_EXECUTION_ID, EXIT_CODE, START_TIME, TASK_NAME, LAST_UPDATED, EXTERNAL_EXECUTION_ID, PARENT_EXECUTION_ID)" + + "values (:taskExecutionId, :exitCode, :startTime, :taskName, :lastUpdated, :externalExecutionId, :parentExecutionId)"; private static final String CREATE_TASK_ARGUMENT = "INSERT into " + "%PREFIX%EXECUTION_PARAMS(TASK_EXECUTION_ID, TASK_PARAM ) values (:taskExecutionId, :taskParam)"; @@ -192,6 +193,7 @@ public class JdbcTaskExecutionDao implements TaskExecutionDao { final MapSqlParameterSource queryParameters = new MapSqlParameterSource() .addValue("taskExecutionId", nextExecutionId, Types.BIGINT) + .addValue("exitCode", null, Types.INTEGER) .addValue("startTime", startTime, Types.TIMESTAMP) .addValue("taskName", taskName, Types.VARCHAR) .addValue("lastUpdated", new Date(), Types.TIMESTAMP) @@ -222,6 +224,7 @@ public class JdbcTaskExecutionDao implements TaskExecutionDao { final MapSqlParameterSource queryParameters = new MapSqlParameterSource() .addValue("startTime", startTime, Types.TIMESTAMP) + .addValue("exitCode", null, Types.INTEGER) .addValue("taskName", taskName, Types.VARCHAR) .addValue("lastUpdated", new Date(), Types.TIMESTAMP) .addValue("parentExecutionId", parentExecutionId, Types.BIGINT) diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/SimpleTaskRepository.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/SimpleTaskRepository.java index c0d0e087..2b600bcb 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/SimpleTaskRepository.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/SimpleTaskRepository.java @@ -84,7 +84,7 @@ public class SimpleTaskRepository implements TaskRepository { String exitMessage, String errorMessage) { initialize(); - validateExitInformation(executionId, exitCode, endTime); + validateCompletedTaskExitInformation(executionId, exitCode, endTime); exitMessage = trimMessage(exitMessage, this.maxExitMessageSize); errorMessage = trimMessage(errorMessage, this.maxErrorMessageSize); taskExecutionDao.completeTaskExecution(executionId, exitCode, endTime, exitMessage, errorMessage); @@ -189,7 +189,7 @@ public class SimpleTaskRepository implements TaskRepository { } } - private void validateExitInformation(long executionId, Integer exitCode, Date endTime){ + private void validateCompletedTaskExitInformation(long executionId, Integer exitCode, Date endTime){ Assert.notNull(exitCode, "exitCode should not be null"); Assert.isTrue(exitCode >= 0, "exit code must be greater than or equal to zero"); Assert.notNull(endTime, "TaskExecution endTime cannot be null."); diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskLifecycleListenerTests.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskLifecycleListenerTests.java index 8d08205b..838e71e2 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskLifecycleListenerTests.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskLifecycleListenerTests.java @@ -108,7 +108,7 @@ public class TaskLifecycleListenerTests { context.publishEvent(new ApplicationReadyEvent(new SpringApplication(), new String[0], context)); - verifyTaskExecution(0, true); + verifyTaskExecution(0, true, 0); } @Test @@ -183,7 +183,7 @@ public class TaskLifecycleListenerTests { context.refresh(); this.taskExplorer = context.getBean(TaskExplorer.class); - verifyTaskExecution(0, false, 0, null, "myid"); + verifyTaskExecution(0, false, null, null, "myid"); } @Test @@ -197,12 +197,17 @@ public class TaskLifecycleListenerTests { context.refresh(); this.taskExplorer = context.getBean(TaskExplorer.class); - verifyTaskExecution(0, false, 0, null, null, 789L); + verifyTaskExecution(0, false, null, null, null, 789L); + } + + private void verifyTaskExecution(int numberOfParams, boolean update, Integer exitCode) { + verifyTaskExecution(numberOfParams, update, exitCode, null, null); } private void verifyTaskExecution(int numberOfParams, boolean update) { - verifyTaskExecution(numberOfParams, update, 0, null, null); + verifyTaskExecution(numberOfParams, update, null, null, null); } + private void verifyTaskExecution(int numberOfParams, boolean update, Integer exitCode, Throwable exception, String externalExecutionId) { verifyTaskExecution(numberOfParams, update, exitCode, exception, @@ -240,7 +245,7 @@ public class TaskLifecycleListenerTests { } else { assertNull(taskExecution.getEndTime()); - assertTrue(taskExecution.getExitCode() == 0); + assertTrue(taskExecution.getExitCode() == null); } assertEquals("testTask", taskExecution.getTaskName()); diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryJdbcTests.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryJdbcTests.java index 74b0c15d..f03e3479 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryJdbcTests.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryJdbcTests.java @@ -46,6 +46,7 @@ import static org.junit.Assert.assertEquals; * * @author Glenn Renfro. * @author Michael Minella + * @author Ilayaperumal Gopinathan */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(classes = {EmbeddedDataSourceConfiguration.class, @@ -200,6 +201,7 @@ public class SimpleTaskRepositoryJdbcTests { TaskExecution expectedTaskExecution = TaskExecutionCreator.createAndStoreTaskExecutionNoParams(taskRepository); expectedTaskExecution.setExitMessage(new String(new char[SimpleTaskRepository.MAX_EXIT_MESSAGE_SIZE+1])); expectedTaskExecution.setEndTime(new Date()); + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = completeTaskExecution(expectedTaskExecution, taskRepository); assertEquals(SimpleTaskRepository.MAX_EXIT_MESSAGE_SIZE, actualTaskExecution.getExitMessage().length()); } @@ -212,6 +214,7 @@ public class SimpleTaskRepositoryJdbcTests { TaskExecution expectedTaskExecution = TaskExecutionCreator.createAndStoreTaskExecutionNoParams(simpleTaskRepository); expectedTaskExecution.setExitMessage(new String(new char[SimpleTaskRepository.MAX_EXIT_MESSAGE_SIZE + 1])); expectedTaskExecution.setEndTime(new Date()); + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = completeTaskExecution(expectedTaskExecution, simpleTaskRepository); assertEquals(5, actualTaskExecution.getExitMessage().length()); } @@ -222,6 +225,7 @@ public class SimpleTaskRepositoryJdbcTests { TaskExecution expectedTaskExecution = TaskExecutionCreator.createAndStoreTaskExecutionNoParams(taskRepository); expectedTaskExecution.setErrorMessage(new String(new char[SimpleTaskRepository.MAX_ERROR_MESSAGE_SIZE+1])); expectedTaskExecution.setEndTime(new Date()); + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = completeTaskExecution(expectedTaskExecution, taskRepository); assertEquals(SimpleTaskRepository.MAX_ERROR_MESSAGE_SIZE, actualTaskExecution.getErrorMessage().length()); } @@ -234,6 +238,7 @@ public class SimpleTaskRepositoryJdbcTests { TaskExecution expectedTaskExecution = TaskExecutionCreator.createAndStoreTaskExecutionNoParams(simpleTaskRepository); expectedTaskExecution.setErrorMessage(new String(new char[SimpleTaskRepository.MAX_ERROR_MESSAGE_SIZE + 1])); expectedTaskExecution.setEndTime(new Date()); + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = completeTaskExecution(expectedTaskExecution, simpleTaskRepository); assertEquals(5, actualTaskExecution.getErrorMessage().length()); } @@ -321,6 +326,7 @@ public class SimpleTaskRepositoryJdbcTests { expectedTaskExecution.setErrorMessage(new String(new char[maxErrorMessage+ 1])); expectedTaskExecution.setExitMessage(new String(new char[maxExitMessage + 1])); expectedTaskExecution.setEndTime(new Date()); + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = completeTaskExecution(expectedTaskExecution, taskRepository); assertEquals(maxErrorMessage.intValue(), actualTaskExecution.getErrorMessage().length()); diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryMapTests.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryMapTests.java index 79a9dde3..6e94c976 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryMapTests.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskRepositoryMapTests.java @@ -35,7 +35,9 @@ import static org.springframework.test.util.AssertionErrors.assertTrue; /** * Tests for the SimpleTaskRepository that uses Map as a datastore. - * @author Glenn Renfro. + * + * @author Glenn Renfro + * @author Ilayaperumal Gopinathan */ public class SimpleTaskRepositoryMapTests { @@ -159,7 +161,7 @@ public class SimpleTaskRepositoryMapTests { TaskExecution expectedTaskExecution = TaskExecutionCreator.createAndStoreTaskExecutionNoParams(taskRepository); expectedTaskExecution.setEndTime(new Date()); - + expectedTaskExecution.setExitCode(0); TaskExecution actualTaskExecution = TaskExecutionCreator.completeExecution(taskRepository, expectedTaskExecution); TestVerifierUtils.verifyTaskExecution(expectedTaskExecution, actualTaskExecution); } diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDBUtils.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDBUtils.java index 706bfe1f..76205564 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDBUtils.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDBUtils.java @@ -38,6 +38,7 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.support.MetaDataAccessException; import org.springframework.jdbc.support.incrementer.DataFieldMaxValueIncrementer; +import org.springframework.util.StringUtils; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; @@ -48,6 +49,7 @@ import static org.mockito.Mockito.when; * relational database. * * @author Glenn Renfro + * @author Ilayaperumal Gopinathan */ public class TestDBUtils { @@ -68,7 +70,7 @@ public class TestDBUtils { @Override public TaskExecution mapRow(ResultSet rs, int rownumber) throws SQLException { TaskExecution taskExecution=new TaskExecution(rs.getLong("TASK_EXECUTION_ID"), - rs.getInt("EXIT_CODE"), + StringUtils.hasText(rs.getString("EXIT_CODE")) ? Integer.valueOf(rs.getString("EXIT_CODE")) : null, rs.getString("TASK_NAME"), rs.getTimestamp("START_TIME"), rs.getTimestamp("END_TIME"), diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestVerifierUtils.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestVerifierUtils.java index 08a4b260..c6424daf 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestVerifierUtils.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestVerifierUtils.java @@ -89,7 +89,7 @@ public class TestVerifierUtils { long executionId = randomGenerator.nextLong(); String taskName = UUID.randomUUID().toString(); - return new TaskExecution(executionId, 0, taskName, + return new TaskExecution(executionId, null, taskName, startTime, null, null, new ArrayList(), null, null); } diff --git a/spring-cloud-task-integration-tests/src/test/java/org/springframework/cloud/task/launcher/TaskLauncherSinkTests.java b/spring-cloud-task-integration-tests/src/test/java/org/springframework/cloud/task/launcher/TaskLauncherSinkTests.java index 7033c594..d5d9112e 100644 --- a/spring-cloud-task-integration-tests/src/test/java/org/springframework/cloud/task/launcher/TaskLauncherSinkTests.java +++ b/spring-cloud-task-integration-tests/src/test/java/org/springframework/cloud/task/launcher/TaskLauncherSinkTests.java @@ -63,8 +63,8 @@ public class TaskLauncherSinkTests { private final static int WAIT_INTERVAL = 500; private final static int MAX_WAIT_TIME = 10000; - private final static String URL = "maven://io.spring.cloud:" - + "timestamp-task:jar:1.1.0.RELEASE"; + private final static String URL = "maven://org.springframework.cloud.task.app:" + + "timestamp-task:jar:1.3.0.RELEASE"; private final static String DATASOURCE_URL; private final static String DATASOURCE_USER_NAME = "SA"; private final static String DATASOURCE_USER_PASSWORD = ""; @@ -140,8 +140,8 @@ public class TaskLauncherSinkTests { assertTrue(waitForDBToBePopulated()); Page taskExecutions = taskExplorer.findAll(new PageRequest(0, 10)); - TaskExecution te = taskExecutions.iterator().next(); assertEquals("Only one row is expected", 1, taskExecutions.getTotalElements()); + assertTrue(waitForTaskToComplete()); assertEquals("return code should be 0", 0, taskExecutions.iterator().next().getExitCode().intValue()); } @@ -168,6 +168,19 @@ public class TaskLauncherSinkTests { return isDbPopulated; } + private boolean waitForTaskToComplete() throws Exception { + boolean istTaskComplete = false; + for (int waitTime = 0; waitTime <= MAX_WAIT_TIME; waitTime += WAIT_INTERVAL) { + Thread.sleep(WAIT_INTERVAL); + TaskExecution taskExecution = taskExplorer.getTaskExecution(1); + if (taskExecution.getExitCode() != null) { + istTaskComplete = true; + break; + } + } + return istTaskComplete; + } + private void launchTask(String artifactURL) { TaskLaunchRequest request = new TaskLaunchRequest(artifactURL, null, this.properties, null, null);