Updated thin taskexecutions by name to return 404 when definition is …

…not found. Provide task/execution and thin executions by name test when task is not defined and where it is defined. Removed excess `andDo(print())` from tests.
spring-cloud · Oct 21, 2024 · 4fd8d08 · 4fd8d08
1 parent 93c62fa
commit 4fd8d08
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 32 deletions.
diff --git a/...org/springframework/cloud/dataflow/server/config/DataFlowControllerAutoConfiguration.java b/...org/springframework/cloud/dataflow/server/config/DataFlowControllerAutoConfiguration.java
@@ -286,8 +286,8 @@ public TaskExecutionController taskExecutionController(
 		}
 
 		@Bean
-		public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer taskExplorer) {
-			return new TaskExecutionThinController(taskExplorer);
+		public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer taskExplorer, TaskDefinitionRepository taskDefinitionRepository) {
+			return new TaskExecutionThinController(taskExplorer, taskDefinitionRepository);
 		}
 
 		@Bean

diff --git a/...in/java/org/springframework/cloud/dataflow/server/controller/TaskExecutionController.java b/...in/java/org/springframework/cloud/dataflow/server/controller/TaskExecutionController.java
@@ -185,9 +185,11 @@ public PagedModel<TaskExecutionResource> retrieveTasksByName(
 			Pageable pageable,
 			PagedResourcesAssembler<TaskJobExecutionRel> assembler
 	) {
+		long tasks = this.taskDefinitionRepository.countByTaskName(taskName);
+		if(tasks == 0) {
+			throw new NoSuchTaskDefinitionException(taskName);
+		}
 		validatePageable(pageable);
-		this.taskDefinitionRepository.findById(taskName)
-				.orElseThrow(() -> new NoSuchTaskDefinitionException(taskName));
 		Page<TaskExecution> taskExecutions = this.explorer.findTaskExecutionsByName(taskName, pageable);
 		Page<TaskJobExecutionRel> result = getPageableRelationships(taskExecutions, pageable);
 		return assembler.toModel(result, this.taskAssembler);

diff --git a/...ava/org/springframework/cloud/dataflow/server/controller/TaskExecutionThinController.java b/...ava/org/springframework/cloud/dataflow/server/controller/TaskExecutionThinController.java
@@ -18,6 +18,8 @@
 
 import org.springframework.cloud.dataflow.core.ThinTaskExecution;
 import org.springframework.cloud.dataflow.rest.resource.TaskExecutionThinResource;
+import org.springframework.cloud.dataflow.server.repository.NoSuchTaskDefinitionException;
+import org.springframework.cloud.dataflow.server.repository.TaskDefinitionRepository;
 import org.springframework.cloud.dataflow.server.task.DataflowTaskExplorer;
 import org.springframework.cloud.task.repository.TaskExecution;
 import org.springframework.data.domain.Page;
@@ -47,10 +49,12 @@
 public class TaskExecutionThinController {
 
 	private final DataflowTaskExplorer explorer;
+	private final TaskDefinitionRepository taskDefinitionRepository;
 	private final TaskExecutionThinResourceAssembler resourceAssembler;
 
-	public TaskExecutionThinController(DataflowTaskExplorer explorer) {
+	public TaskExecutionThinController(DataflowTaskExplorer explorer, TaskDefinitionRepository taskDefinitionRepository) {
 		this.explorer = explorer;
+		this.taskDefinitionRepository = taskDefinitionRepository;
 		this.resourceAssembler = new TaskExecutionThinResourceAssembler();
 	}
 
@@ -70,6 +74,10 @@ public PagedModel<TaskExecutionThinResource> retrieveTasksByName(
 		Pageable pageable,
 		PagedResourcesAssembler<ThinTaskExecution> pagedAssembler
 	) {
+		long tasks = this.taskDefinitionRepository.countByTaskName(taskName);
+		if(tasks == 0) {
+			throw new NoSuchTaskDefinitionException(taskName);
+		}
 		Page<TaskExecution> page = this.explorer.findTaskExecutionsByName(taskName, pageable);
 		Page<ThinTaskExecution> thinTaskExecutions = new PageImpl<>(page.stream().map(ThinTaskExecution::new).toList(), pageable, page.getTotalElements());
 		explorer.populateCtrStatus(thinTaskExecutions.getContent());

diff --git a/...n/java/org/springframework/cloud/dataflow/server/repository/TaskDefinitionRepository.java b/...n/java/org/springframework/cloud/dataflow/server/repository/TaskDefinitionRepository.java
@@ -46,4 +46,5 @@ public interface TaskDefinitionRepository extends KeyValueRepository<TaskDefinit
 	 */
 	TaskDefinition findByTaskName(String name);
 
+    long countByTaskName(String taskName);
 }
diff --git a/...springframework/cloud/dataflow/server/task/impl/DefaultDataFlowTaskExecutionQueryDao.java b/...springframework/cloud/dataflow/server/task/impl/DefaultDataFlowTaskExecutionQueryDao.java
@@ -522,20 +522,19 @@ private List<String> getTaskArguments(long taskExecutionId) {
 
 	@Override
 	public void populateCtrStatus(Collection<ThinTaskExecution> thinTaskExecutions) {
-		Map<Long, ThinTaskExecution> taskExecutionMap = thinTaskExecutions.stream()
+		if(!thinTaskExecutions.isEmpty()) {
+			Map<Long, ThinTaskExecution> taskExecutionMap = thinTaskExecutions.stream()
 				.collect(Collectors.toMap(ThinTaskExecution::getExecutionId, Function.identity()));
-		String ids = taskExecutionMap.keySet()
-				.stream()
-				.map(Object::toString)
-				.collect(Collectors.joining(","));
-		String sql = FIND_CTR_STATUS.replace(":taskExecutionIds", ids);
-		jdbcTemplate.query(sql, rs -> {
-			Long id = rs.getLong("TASK_EXECUTION_ID");
-			String ctrStatus = rs.getString("CTR_STATUS");
-			logger.debug("populateCtrStatus:{}={}", id, ctrStatus);
-			ThinTaskExecution execution = taskExecutionMap.get(id);
-			Assert.notNull(execution, "Expected TaskExecution for " + id + " from " + ids);
-			execution.setCtrTaskStatus(ctrStatus);
-		});
+			String ids = taskExecutionMap.keySet().stream().map(Object::toString).collect(Collectors.joining(","));
+			String sql = FIND_CTR_STATUS.replace(":taskExecutionIds", ids);
+			jdbcTemplate.query(sql, rs -> {
+				Long id = rs.getLong("TASK_EXECUTION_ID");
+				String ctrStatus = rs.getString("CTR_STATUS");
+				logger.debug("populateCtrStatus:{}={}", id, ctrStatus);
+				ThinTaskExecution execution = taskExecutionMap.get(id);
+				Assert.notNull(execution, "Expected TaskExecution for " + id + " from " + ids);
+				execution.setCtrTaskStatus(ctrStatus);
+			});
+		}
 	}
 }
diff --git a/...rc/test/java/org/springframework/cloud/dataflow/server/configuration/JobDependencies.java b/...rc/test/java/org/springframework/cloud/dataflow/server/configuration/JobDependencies.java
@@ -270,8 +270,8 @@ public TaskExecutionController taskExecutionController(
 	}
 
 	@Bean
-	public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer dataflowTaskExplorer) {
-		return new TaskExecutionThinController(dataflowTaskExplorer);
+	public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer dataflowTaskExplorer, TaskDefinitionRepository taskDefinitionRepository) {
+		return new TaskExecutionThinController(dataflowTaskExplorer, taskDefinitionRepository);
 	}
 
 	@Bean

diff --git a/...c/test/java/org/springframework/cloud/dataflow/server/configuration/TestDependencies.java b/...c/test/java/org/springframework/cloud/dataflow/server/configuration/TestDependencies.java
@@ -593,8 +593,8 @@ public TaskExecutionController taskExecutionController(
 	}
 
 	@Bean
-	public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer dataflowTaskExplorer) {
-		return new TaskExecutionThinController(dataflowTaskExplorer);
+	public TaskExecutionThinController taskExecutionThinController(DataflowTaskExplorer dataflowTaskExplorer, TaskDefinitionRepository taskDefinitionRepository) {
+		return new TaskExecutionThinController(dataflowTaskExplorer, taskDefinitionRepository);
 	}
 	@Bean
 	public TasksInfoController taskExecutionsInfoController(TaskExecutionService taskExecutionService) {

diff --git a/...va/org/springframework/cloud/dataflow/server/controller/TaskExecutionControllerTests.java b/...va/org/springframework/cloud/dataflow/server/controller/TaskExecutionControllerTests.java
@@ -204,6 +204,7 @@ void setupMockMVC() throws JobInstanceAlreadyCompleteException, JobExecutionAlre
 			SAMPLE_CLEANSED_ARGUMENT_LIST.add("spring.datasource.password=******");
 
 			taskDefinitionRepository.save(new TaskDefinition(TASK_NAME_ORIG, "demo"));
+			taskDefinitionRepository.save(new TaskDefinition("nope", "demo"));
 			TaskExecution taskExecution1 =
 				taskExecutionDao.createTaskExecution(TASK_NAME_ORIG, LocalDateTime.now(), SAMPLE_ARGUMENT_LIST, "foobar");
 
@@ -324,7 +325,6 @@ void getExecutionForJob() throws Exception {
 	void getAllExecutions() throws Exception {
 		verifyTaskArgs(SAMPLE_CLEANSED_ARGUMENT_LIST, "$._embedded.taskExecutionResourceList[0].",
 				mockMvc.perform(get("/tasks/executions").accept(MediaType.APPLICATION_JSON))
-						.andDo(print())
 						.andExpect(status().isOk()))
 				.andExpect(jsonPath("$._embedded.taskExecutionResourceList[*].executionId", containsInAnyOrder(4, 3, 2, 1)))
 				.andExpect(jsonPath("$._embedded.taskExecutionResourceList[*].parentExecutionId", containsInAnyOrder(null, null, null, 1)))
@@ -335,7 +335,6 @@ void getAllExecutions() throws Exception {
 	@Test
 	void getAllThinExecutions() throws Exception {
 			mockMvc.perform(get("/tasks/thinexecutions").accept(MediaType.APPLICATION_JSON))
-				.andDo(print())
 				.andExpect(status().isOk())
 			.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList[*].executionId", containsInAnyOrder(4, 3, 2, 1)))
 			.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList[*].parentExecutionId", containsInAnyOrder(null, null, null, 1)))
@@ -345,9 +344,9 @@ void getAllThinExecutions() throws Exception {
 	@Test
 	void getThinExecutionsByName() throws Exception {
 		mockMvc.perform(get("/tasks/thinexecutions").queryParam("name", "nope").accept(MediaType.APPLICATION_JSON))
-			.andDo(print())
-			.andExpect(status().isOk())
 			.andExpect(jsonPath("$.page.totalElements", is(0)));
+		mockMvc.perform(get("/tasks/thinexecutions").queryParam("name", "none").accept(MediaType.APPLICATION_JSON))
+			.andExpect(status().is4xxClientError());
 	}
 
 	@Test
@@ -388,7 +387,6 @@ void taskExecution() throws Exception {
 		resultActions = mockMvc.perform(
 						get("/tasks/executions/" + resource.getExecutionId())
 								.accept(MediaType.APPLICATION_JSON))
-				.andDo(print())
 				.andExpect(status().isOk())
 				.andExpect(content().json("{taskName: \"timestamp3\"}"));
 		response = resultActions.andReturn().getResponse().getContentAsString();

diff --git a/...amework/cloud/dataflow/integration/test/tasks/AbstractLargeTaskExecutionDatabaseBase.java b/...amework/cloud/dataflow/integration/test/tasks/AbstractLargeTaskExecutionDatabaseBase.java
@@ -28,7 +28,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
@@ -110,35 +112,69 @@ private void addColumnTableType(Map<String, Map<String, Integer>> columnTypes, S
 
 	@Test
 	void queryWithLargeNumberOfTaskExecutions() throws Exception {
+		mockMvc.perform(post("/apps/task/nope/1.0").param("uri", "maven:io.spring:timestamp-task:3.0.0"))
+			.andExpect(status().is2xxSuccessful());
+		mockMvc.perform(post("/tasks/definitions").param("name", "nope").param("definition", "nope"))
+			.andExpect(status().is2xxSuccessful());
+		mockMvc.perform(post("/tasks/definitions").param("name", "ts-batch").param("definition", "nope"))
+			.andExpect(status().is2xxSuccessful());
 		long startTime = System.currentTimeMillis();
 		mockMvc
-			.perform(get("/tasks/executions").accept(MediaType.APPLICATION_JSON).param("size", "20").param("page", "1"))
+			.perform(get("/tasks/executions").param("size", "20").param("page", "1").accept(MediaType.APPLICATION_JSON))
 			.andExpect(status().isOk())
 			.andExpect(jsonPath("$._embedded.taskExecutionResourceList", hasSize(greaterThanOrEqualTo(20))));
 		long totalTime = System.currentTimeMillis() - startTime;
 		long startTime2 = System.currentTimeMillis();
 		mockMvc
 			.perform(
-					get("/tasks/executions").accept(MediaType.APPLICATION_JSON).param("size", "200").param("page", "2"))
+					get("/tasks/executions").param("size", "200").param("page", "2").accept(MediaType.APPLICATION_JSON))
 			.andExpect(status().isOk())
 			.andExpect(jsonPath("$._embedded.taskExecutionResourceList", hasSize(greaterThanOrEqualTo(200))));
 		long totalTime2 = System.currentTimeMillis() - startTime2;
 		long startTime3 = System.currentTimeMillis();
 		mockMvc.perform(
-				get("/tasks/thinexecutions").accept(MediaType.APPLICATION_JSON).param("size", "20").param("page", "3"))
+				get("/tasks/thinexecutions").param("size", "20").param("page", "3").accept(MediaType.APPLICATION_JSON))
 			.andExpect(status().isOk())
 			.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList", hasSize(greaterThanOrEqualTo(20))));
 		long totalTime3 = System.currentTimeMillis() - startTime3;
 		long startTime4 = System.currentTimeMillis();
 		mockMvc.perform(
-				get("/tasks/thinexecutions").accept(MediaType.APPLICATION_JSON).param("size", "200").param("page", "2"))
+				get("/tasks/thinexecutions").param("size", "200").param("page", "2").accept(MediaType.APPLICATION_JSON))
 			.andExpect(status().isOk())
 			.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList", hasSize(greaterThanOrEqualTo(200))));
 		long totalTime4 = System.currentTimeMillis() - startTime4;
+		long startTime5 = System.currentTimeMillis();
+		mockMvc.perform(
+				get("/tasks/executions").param("name", "nope").param("page", "0").param("size", "200").accept(MediaType.APPLICATION_JSON))
+			.andExpect(status().isOk())
+			.andExpect(jsonPath("$.page.totalElements", is(0)));
+		long totalTime5 = System.currentTimeMillis() - startTime5;
+		long startTime6 = System.currentTimeMillis();
+		mockMvc.perform(
+				get("/tasks/thinexecutions").param("name", "nope").param("page", "0").param("size", "200").accept(MediaType.APPLICATION_JSON))
+			.andExpect(status().isOk())
+			.andExpect(jsonPath("$.page.totalElements", is(0)));
+		long totalTime6 = System.currentTimeMillis() - startTime6;
+		long startTime7 = System.currentTimeMillis();
+		mockMvc.perform(
+				get("/tasks/executions").param("name", "ts-batch").param("page", "0").param("size", "200").accept(MediaType.APPLICATION_JSON))
+			.andExpect(status().isOk())
+			.andExpect(jsonPath("$.page.totalElements", is(83)));
+		long totalTime7 = System.currentTimeMillis() - startTime7;
+		long startTime8 = System.currentTimeMillis();
+		mockMvc.perform(
+				get("/tasks/thinexecutions").param("name", "ts-batch").param("page", "0").param("size", "200").accept(MediaType.APPLICATION_JSON))
+			.andExpect(status().isOk())
+			.andExpect(jsonPath("$.page.totalElements", is(83)));
+		long totalTime8 = System.currentTimeMillis() - startTime8;
 		logger.info("result:totalTime={}ms", totalTime);
 		logger.info("result:totalTime2={}ms", totalTime2);
 		logger.info("result:totalTime3={}ms", totalTime3);
 		logger.info("result:totalTime4={}ms", totalTime4);
+		logger.info("result:totalTime5={}ms", totalTime5);
+		logger.info("result:totalTime6={}ms", totalTime6);
+		logger.info("result:totalTime7={}ms", totalTime7);
+		logger.info("result:totalTime8={}ms", totalTime8);
 		double ratioExecution = (double) totalTime / (double) totalTime2;
 		double ratioThinExecution = (double) totalTime3 / (double) totalTime4;
 		double ratioThinToExecution = (double) totalTime2 / (double) totalTime4;
-Original file line number
+Diff line change
@@ Expand Up @@
     	 */
     	TaskDefinition findByTaskName(String name);
+        long countByTaskName(String taskName);
     }