Code Review Changes

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/JdbcTaskExecutionDao.java

@@ -20,7 +20,6 @@
 import java.sql.SQLException;
 import java.sql.Types;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -29,11 +28,9 @@
 import javax.sql.DataSource;
 
 import org.springframework.cloud.task.repository.TaskExecution;
-import org.springframework.dao.DataAccessException;
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.jdbc.core.JdbcOperations;
 import org.springframework.jdbc.core.JdbcTemplate;
-import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.RowCallbackHandler;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.util.Assert;
@@ -83,7 +80,8 @@ public class JdbcTaskExecutionDao implements TaskExecutionDao {
 			+ "START_TIME, END_TIME, TASK_NAME, EXIT_CODE, "
 			+ "EXIT_MESSAGE, LAST_UPDATED, STATUS_CODE "
 			+ "from %PREFIX%EXECUTION where TASK_NAME = ? "
-			+ "order by TASK_EXECUTION_ID";
+			+ "order by TASK_EXECUTION_ID "
+			+ "LIMIT ? OFFSET ?";
 
 	final String FIND_TASK_NAMES = "SELECT distinct TASK_NAME from %PREFIX%EXECUTION order by TASK_NAME";
 
@@ -172,60 +170,22 @@ public long getTaskExecutionCount(String taskName) {
 	@Override
 	public Set<TaskExecution> findRunningTaskExecutions(String taskName) {
 		final Set<TaskExecution> result = new HashSet<TaskExecution>();
-		RowCallbackHandler handler = new RowCallbackHandler() {
-			@Override
-			public void processRow(ResultSet rs) throws SQLException {
-				TaskExecutionRowMapper mapper = new TaskExecutionRowMapper();
-				result.add(mapper.mapRow(rs, 0));
-			}
-		};
-		jdbcTemplate.query(getQuery(FIND_RUNNING_TASK_EXECUTIONS),
-				new Object[] { taskName }, handler);
-
+		List resultList = jdbcTemplate.query(getQuery(FIND_RUNNING_TASK_EXECUTIONS),
+				new Object[]{ taskName }, new TaskExecutionRowMapper());
+		result.addAll(resultList);
 		return result;
 
 	}
 
 	@Override
 	public List<TaskExecution> getTaskExecutionsByName(String taskName, final int start, final int count) {
-		ResultSetExtractor<List<TaskExecution>> extractor =
-				new ResultSetExtractor<List<TaskExecution>>() {
-
-			private List<TaskExecution> list = new ArrayList<TaskExecution>();
-
-			@Override
-			public List<TaskExecution> extractData(ResultSet rs) throws SQLException,
-					DataAccessException {
-				int rowNum = 0;
-				while (rowNum < start && rs.next()) {
-					rowNum++;
-				}
-				while (rowNum < start + count && rs.next()) {
-					RowMapper<TaskExecution> rowMapper = new TaskExecutionRowMapper();
-					list.add(rowMapper.mapRow(rs, rowNum));
-					rowNum++;
-				}
-				return list;
-			}
-
-		};
-
-		List<TaskExecution> result = jdbcTemplate.query(getQuery(FIND_TASK_EXECUTIONS),
-				new Object[] { taskName }, extractor);
-
-		return result;
+		return jdbcTemplate.query(getQuery(FIND_TASK_EXECUTIONS),
+				new Object[]{ taskName, count, start + 1 }, new TaskExecutionRowMapper());
 	}
 
 	@Override
 	public List<String> getTaskNames() {
-		return jdbcTemplate.query(getQuery(FIND_TASK_NAMES),
-				new RowMapper<String>() {
-					@Override
-					public String mapRow(ResultSet rs, int rowNum)
-							throws SQLException {
-						return rs.getString(1);
-					}
-				});
+		return jdbcTemplate.queryForList(getQuery(FIND_TASK_NAMES), String.class);
 	}
 
 	private String getQuery(String base) {
@@ -266,14 +226,12 @@ public void processRow(ResultSet rs) throws SQLException {
 
 		jdbcTemplate.query(getQuery(FIND_PARAMS_FROM_ID), new Object[] { executionId },
 				handler);
-		return Collections.unmodifiableList(params);
+		return params;
 
 	}
 	/**
 	 * Re-usable mapper for {@link TaskExecution} instances.
 	 *
-	 * @author Dave Syer
-	 *
 	 */
 	private final class TaskExecutionRowMapper implements RowMapper<TaskExecution> {
 
@@ -282,9 +240,9 @@ public TaskExecutionRowMapper() {
 
 		@Override
 		public TaskExecution mapRow(ResultSet rs, int rowNum) throws SQLException {
-			String  id = rs.getString(1);
+			String  id = rs.getString("TASK_EXECUTION_ID");
 			TaskExecution taskExecution=new TaskExecution();
-			taskExecution.setExecutionId(rs.getString(1));
+			taskExecution.setExecutionId(id);
 			taskExecution.setStartTime(rs.getTimestamp("START_TIME"));
 			taskExecution.setEndTime(rs.getTimestamp("END_TIME"));
 			taskExecution.setExitCode(rs.getInt("EXIT_CODE"));

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/dao/MapTaskExecutionDao.java

@@ -18,11 +18,11 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -69,49 +69,49 @@ public long getTaskExecutionCount(String taskName) {
 
 	@Override
 	public Set<TaskExecution> findRunningTaskExecutions(String taskName) {
-		Set<TaskExecution> result = new HashSet<>();
+		Set<TaskExecution> result = getTaskExecutionTreeSet();
 		for (Map.Entry<String, TaskExecution> entry : taskExecutions.entrySet()) {
 			if (entry.getValue().getTaskName().equals(taskName) &&
 					entry.getValue().getEndTime() == null) {
 				result.add(entry.getValue());
 			}
 		}
-		return Collections.unmodifiableSet(result);
+		return result;
 	}
 
 	@Override
 	public List<TaskExecution> getTaskExecutionsByName(String taskName, int start, int count) {
 		List<TaskExecution> result = new ArrayList<>();
-		Set<TaskExecution> filteredSet = new HashSet<>();
+		Set<TaskExecution> filteredSet = getTaskExecutionTreeSet();
 		for (Map.Entry<String, TaskExecution> entry : taskExecutions.entrySet()) {
 			if (entry.getValue().getTaskName().equals(taskName)) {
 				filteredSet.add(entry.getValue());
 			}
 		}
-		int rowNum = 0;
-		Iterator<TaskExecution> rs = filteredSet.iterator();
-		while (rowNum < start && rs.hasNext()) {
-			rs.next();
-			rowNum++;
-		}
-		while (rowNum < start + count && rs.hasNext()) {
-			result.add(rs.next());
-			rowNum++;
-		}
-
-		return Collections.unmodifiableList(result);
+		result.addAll(filteredSet);
+		return result.subList(start, start + count);
 	}
 
 	@Override
 	public List<String> getTaskNames() {
-		Set<String> result = new HashSet<>();
+		Set<String> result = new TreeSet<>();
 		for (Map.Entry<String, TaskExecution> entry : taskExecutions.entrySet()) {
 			result.add(entry.getValue().getTaskName());
 		}
-		return Collections.unmodifiableList(new ArrayList(result));
+		return new ArrayList<String>(result);
 	}
 
 	public Map<String, TaskExecution> getTaskExecutions() {
 		return Collections.unmodifiableMap(taskExecutions);
 	}
+
+	private TreeSet<TaskExecution> getTaskExecutionTreeSet() {
+		return new TreeSet<TaskExecution>(new Comparator<TaskExecution>() {
+			@Override
+			public int compare(TaskExecution e1, TaskExecution e2) {
+				return e1.getExecutionId().compareTo(e2.getExecutionId());
+			}
+		});
+	}
+
 }

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/JdbcTaskExplorerFactoryBean.java

@@ -20,6 +20,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.cloud.task.repository.TaskExplorer;
 import org.springframework.cloud.task.repository.dao.JdbcTaskExecutionDao;
 import org.springframework.cloud.task.repository.dao.TaskExecutionDao;
@@ -30,7 +31,7 @@
  *
  * @author Glenn Renfro
  */
-public class JdbcTaskExplorerFactoryBean {
+public class JdbcTaskExplorerFactoryBean implements FactoryBean<TaskExplorer>{
 
 	public static final String DEFAULT_TABLE_PREFIX = "TASK_";
 
@@ -70,6 +71,16 @@ public TaskExplorer getObject(){
 		return taskExplorer;
 	}
 
+	@Override
+	public Class<?> getObjectType() {
+		return TaskExplorer.class;
+	}
+
+	@Override
+	public boolean isSingleton() {
+		return true;
+	}
+
 	private TaskExecutionDao createJdbcTaskExecutionDao()  {
 		JdbcTaskExecutionDao dao = new JdbcTaskExecutionDao(dataSource);
 		dao.setTablePrefix(tablePrefix);

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/JdbcTaskRepositoryFactoryBean.java

@@ -20,6 +20,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.cloud.task.repository.TaskRepository;
 import org.springframework.cloud.task.repository.dao.JdbcTaskExecutionDao;
 import org.springframework.cloud.task.repository.dao.TaskExecutionDao;
@@ -31,7 +32,7 @@
  *
  * @author Glenn Renfro
  */
-public class JdbcTaskRepositoryFactoryBean {
+public class JdbcTaskRepositoryFactoryBean implements FactoryBean<TaskRepository>{
 
 	public static final String DEFAULT_TABLE_PREFIX = "TASK_";
 
@@ -71,6 +72,16 @@ public TaskRepository getObject(){
 		return taskRepository;
 	}
 
+	@Override
+	public Class<?> getObjectType() {
+		return TaskRepository.class;
+	}
+
+	@Override
+	public boolean isSingleton() {
+		return true;
+	}
+
 	private TaskExecutionDao createJdbcTaskExecutionDao()  {
 		JdbcTaskExecutionDao dao = new JdbcTaskExecutionDao(dataSource);
 		dao.setTablePrefix(tablePrefix);

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/MapTaskExplorerFactoryBean.java

@@ -18,6 +18,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.cloud.task.repository.TaskExplorer;
 import org.springframework.cloud.task.repository.dao.MapTaskExecutionDao;
 
@@ -27,7 +28,7 @@
  *
  * @author Glenn Renfro
  */
-public class MapTaskExplorerFactoryBean {
+public class MapTaskExplorerFactoryBean implements FactoryBean<TaskExplorer>{
 
 	private static final Log logger = LogFactory.getLog(MapTaskExplorerFactoryBean.class);
 
@@ -47,4 +48,14 @@ public TaskExplorer getObject(){
 		return taskExplorer;
 	}
 
+	@Override
+	public Class<?> getObjectType() {
+		return TaskExplorer.class;
+	}
+
+	@Override
+	public boolean isSingleton() {
+		return true;
+	}
+
 }

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/repository/support/MapTaskRepositoryFactoryBean.java

@@ -18,6 +18,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.springframework.beans.factory.FactoryBean;
 import org.springframework.cloud.task.repository.TaskRepository;
 import org.springframework.cloud.task.repository.dao.MapTaskExecutionDao;
 
@@ -28,7 +29,7 @@
  *
  * @author Glenn Renfro
  */
-public class MapTaskRepositoryFactoryBean {
+public class MapTaskRepositoryFactoryBean implements FactoryBean<TaskRepository>{
 
 	private static final Log logger = LogFactory.getLog(MapTaskRepositoryFactoryBean.class);
 
@@ -47,4 +48,14 @@ public MapTaskRepositoryFactoryBean(){
 	public TaskRepository getObject(){
 		return taskRepository;
 	}
+
+	@Override
+	public Class<?> getObjectType() {
+		return TaskRepository.class;
+	}
+
+	@Override
+	public boolean isSingleton() {
+		return true;
+	}
 }

spring-cloud-task-core/src/test/java/org/springframework/cloud/task/repository/support/SimpleTaskExplorerTests.java

@@ -117,6 +117,23 @@ public void getTaskExecution() {
 		}
 	}
 
+	@Test
+	public void taskExecutionNotFound() {
+		final int TEST_COUNT = 5;
+		Map<String, TaskExecution> expectedResults = new HashMap<>();
+		for (int i = 0; i < TEST_COUNT; i++) {
+			TaskExecution expectedTaskExecution = createAndSaveTaskExecution();
+			expectedResults.put(expectedTaskExecution.getExecutionId(),
+					expectedTaskExecution);
+		}
+
+		TaskExecution actualTaskExecution =
+				taskExplorer.getTaskExecution("NO_EXECUTION_PRESENT");
+			assertNull(String.format(
+					"expected null for actualTaskExecution %s", testType),
+					actualTaskExecution);
+	}
+
 	@Test
 	public void getTaskCountByTaskName() {
 		final int TEST_COUNT = 5;