From 324abd4e5c9f8ca272bce660d952f3d91401bdb7 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 ++- .../task/repository/dao/JdbcTaskExecutionDao.java | 7 +++++-- .../repository/support/SimpleTaskRepository.java | 4 ++-- .../task/listener/TaskLifecycleListenerTests.java | 15 ++++++++++----- .../support/SimpleTaskRepositoryJdbcTests.java | 6 ++++++ .../support/SimpleTaskRepositoryMapTests.java | 6 ++++-- .../cloud/task/util/TestDBUtils.java | 4 +++- .../cloud/task/util/TestVerifierUtils.java | 2 +- .../task/launcher/TaskLauncherSinkTests.java | 15 ++++++++++++++- 9 files changed, 47 insertions(+), 15 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 b920cb19..eabb442e 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 4e04e378..75e1a80e 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 @@ -112,7 +112,7 @@ public class TaskLifecycleListenerTests { context.publishEvent(new ApplicationReadyEvent(new SpringApplication(), new String[0], context)); - verifyTaskExecution(0, true); + verifyTaskExecution(0, true, 0); } @Test @@ -198,7 +198,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 @@ -212,12 +212,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, @@ -255,7 +260,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 f410e780..f61d9023 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 @@ -44,6 +44,7 @@ import static org.junit.Assert.assertEquals; * * @author Glenn Renfro. * @author Michael Minella + * @author Ilayaperumal Gopinathan */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(classes = {EmbeddedDataSourceConfiguration.class, @@ -195,6 +196,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()); } @@ -207,6 +209,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()); } @@ -217,6 +220,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()); } @@ -229,6 +233,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()); } @@ -316,6 +321,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 f3de4e5b..7c049787 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 @@ -34,7 +34,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 { @@ -158,7 +160,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 f8c6a156..f72ec558 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 @@ -128,8 +128,8 @@ public class TaskLauncherSinkTests { assertTrue(waitForDBToBePopulated()); Page taskExecutions = taskExplorer.findAll(PageRequest.of(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()); } @@ -156,6 +156,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,