From 1bf105238a9dc023a3fd03128ad1a8609034fd89 Mon Sep 17 00:00:00 2001 From: Corneil du Plessis Date: Wed, 22 Nov 2023 15:08:05 +0200 Subject: [PATCH] Ensure sub-query use for Job instances. Fix find by name query ordering. Fix find by name not found query. Improve test case readability. #5484 --- .../repository/AggregateJobQueryDao.java | 2 +- .../repository/JdbcAggregateJobQueryDao.java | 25 ++++---- .../service/impl/DefaultTaskJobService.java | 2 +- .../JobExecutionControllerTests.java | 9 ++- .../JobInstanceControllerTests.java | 60 +++++++++++-------- 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/AggregateJobQueryDao.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/AggregateJobQueryDao.java index f29e0fe03c..9b0ff62dec 100644 --- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/AggregateJobQueryDao.java +++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/AggregateJobQueryDao.java @@ -33,7 +33,7 @@ * @author Corneil du Plessis */ public interface AggregateJobQueryDao { - Page listJobInstances(String jobName, Pageable pageable); + Page listJobInstances(String jobName, Pageable pageable) throws NoSuchJobException; Page listJobExecutions(String jobName, BatchStatus status, Pageable pageable) throws NoSuchJobExecutionException; diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/JdbcAggregateJobQueryDao.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/JdbcAggregateJobQueryDao.java index 4e5c0df866..a923fc4c54 100644 --- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/JdbcAggregateJobQueryDao.java +++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/repository/JdbcAggregateJobQueryDao.java @@ -20,14 +20,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.time.Instant; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.TreeMap; +import java.util.*; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -238,14 +231,17 @@ public JdbcAggregateJobQueryDao(DataSource dataSource, SchemaService schemaServi byJobInstanceIdWithStepCountPagingQueryProvider = getPagingQueryProvider(FIELDS_WITH_STEP_COUNT, FROM_CLAUSE_TASK_EXEC_BATCH, JOB_INSTANCE_ID_FILTER); byTaskExecutionIdWithStepCountPagingQueryProvider = getPagingQueryProvider(FIELDS_WITH_STEP_COUNT, FROM_CLAUSE_TASK_EXEC_BATCH, TASK_EXECUTION_ID_FILTER); - jobExecutionsPagingQueryProviderByName = getPagingQueryProvider(FIND_JOBS_FIELDS, FIND_JOBS_FROM, FIND_JOBS_WHERE, Collections.singletonMap("JOB_INSTANCE_ID", Order.DESCENDING)); + jobExecutionsPagingQueryProviderByName = getPagingQueryProvider(FIND_JOBS_FIELDS, FIND_JOBS_FROM, FIND_JOBS_WHERE, Collections.singletonMap("E.JOB_EXECUTION_ID", Order.DESCENDING)); byJobExecutionIdAndSchemaPagingQueryProvider = getPagingQueryProvider(FIELDS_WITH_STEP_COUNT, FROM_CLAUSE_TASK_EXEC_BATCH, FIND_BY_ID_SCHEMA); } @Override - public Page listJobInstances(String jobName, Pageable pageable) { + public Page listJobInstances(String jobName, Pageable pageable) throws NoSuchJobException { int total = countJobExecutions(jobName); + if(total == 0) { + throw new NoSuchJobException("No Job with that name either current or historic: [" + jobName + "]"); + } List taskJobInstancesForJobName = total > 0 ? getTaskJobInstancesForJobName(jobName, pageable) : Collections.emptyList(); @@ -273,7 +269,14 @@ public JobInstanceExecutions getJobInstanceExecutions(long jobInstanceId, String } else if (executions.size() > 1) { throw new RuntimeException("Expected a single JobInstanceExecutions not " + executions.size()); } - return executions.get(0); + JobInstanceExecutions jobInstanceExecution = executions.get(0); + if(jobInstanceExecution.getTaskJobExecutions() != null && !jobInstanceExecution.getTaskJobExecutions().isEmpty()) { + for (TaskJobExecution taskJobExecution : jobInstanceExecution.getTaskJobExecutions()) { + JobService jobService = jobServiceContainer.get(taskJobExecution.getSchemaTarget()); + jobService.addStepExecutions(taskJobExecution.getJobExecution()); + } + } + return jobInstanceExecution; } @Override diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultTaskJobService.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultTaskJobService.java index 54d6f80b5b..a8bff1208a 100644 --- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultTaskJobService.java +++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultTaskJobService.java @@ -186,7 +186,7 @@ public TaskJobExecution getJobExecution(long id, String schemaTarget) throws NoS } @Override - public Page listTaskJobInstancesForJobName(Pageable pageable, String jobName) { + public Page listTaskJobInstancesForJobName(Pageable pageable, String jobName) throws NoSuchJobException { Assert.notNull(pageable, "pageable must not be null"); Assert.notNull(jobName, "jobName must not be null"); return aggregateJobQueryDao.listJobInstances(jobName, pageable); diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobExecutionControllerTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobExecutionControllerTests.java index 921be013aa..21d84580e6 100644 --- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobExecutionControllerTests.java +++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobExecutionControllerTests.java @@ -52,9 +52,7 @@ import org.springframework.web.context.WebApplicationContext; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter; -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; @@ -212,7 +210,8 @@ public void testGetExecutionWithJobProperties() throws Exception { .andExpect(jsonPath("$.executionId", is(10))) .andExpect(jsonPath("$.jobExecution.jobParameters.parameters", Matchers.hasKey(("javaUtilDate")))) .andExpect(jsonPath("$.jobExecution.stepExecutions", hasSize(1))).andReturn(); - assertThat(result.getResponse().getContentAsString()).contains("{\"identifying\":true,\"value\":\"2023-06-07T04:00:00.000+00:00\",\"type\":\"DATE\"}"); + assertThat(result.getResponse().getContentAsString()).contains("\"identifying\":true"); + assertThat(result.getResponse().getContentAsString()).contains("\"type\":\"DATE\""); } @Test @@ -231,7 +230,7 @@ public void testGetAllExecutions() throws Exception { .andDo(print()) .andExpect(status().isOk()) .andExpect(jsonPath("$._embedded.jobExecutionResourceList", hasSize(10))) - .andExpect(jsonPath("$._embedded.jobExecutionResourceList[*].executionId", containsInAnyOrder(10, 9, 8, 7, 6, 5, 4, 3, 2, 1))); + .andExpect(jsonPath("$._embedded.jobExecutionResourceList[*].executionId", containsInRelativeOrder(10, 9, 8, 7, 6, 5, 4, 3, 2, 1))); } @Test diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobInstanceControllerTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobInstanceControllerTests.java index f1468868d1..e00cb2a238 100644 --- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobInstanceControllerTests.java +++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/JobInstanceControllerTests.java @@ -16,16 +16,13 @@ package org.springframework.cloud.dataflow.server.controller; -import java.util.ArrayList; -import java.util.Date; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; - import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.JobParameters; +import org.springframework.batch.core.StepExecution; import org.springframework.batch.core.repository.JobRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.batch.BatchProperties; @@ -52,22 +49,22 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; +import java.util.ArrayList; +import java.util.Date; + +import static org.hamcrest.Matchers.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; /** * @author Glenn Renfro * @author Corneil du Plessis */ @RunWith(SpringRunner.class) -@SpringBootTest(classes = { JobDependencies.class, - PropertyPlaceholderAutoConfiguration.class, BatchProperties.class }) -@EnableConfigurationProperties({ CommonApplicationProperties.class }) +@SpringBootTest(classes = {JobDependencies.class, + PropertyPlaceholderAutoConfiguration.class, BatchProperties.class}) +@EnableConfigurationProperties({CommonApplicationProperties.class}) @DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD) @AutoConfigureTestDatabase(replace = Replace.ANY) public class JobInstanceControllerTests { @@ -105,7 +102,7 @@ public class JobInstanceControllerTests { @Before public void setupMockMVC() { this.mockMvc = MockMvcBuilders.webAppContextSetup(wac) - .defaultRequest(get("/").accept(MediaType.APPLICATION_JSON)).build(); + .defaultRequest(get("/").accept(MediaType.APPLICATION_JSON)).build(); if (!initialized) { createSampleJob(JOB_NAME_ORIG, 1); createSampleJob(JOB_NAME_FOO, 1); @@ -122,36 +119,46 @@ public void testJobInstanceControllerConstructorMissingRepository() { @Test public void testGetInstanceNotFound() throws Exception { mockMvc.perform(get("/jobs/instances/1345345345345").accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isNotFound()); + .andDo(print()) + .andExpect(status().isNotFound()); } @Test public void testGetInstance() throws Exception { - mockMvc.perform(get("/jobs/instances/1").accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk()) - .andExpect(content().json("{jobInstanceId: " + 1 + "}")); + mockMvc.perform(get("/jobs/instances/1").accept(MediaType.APPLICATION_JSON)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.jobInstanceId", equalTo(1))) + .andExpect(jsonPath("$.jobExecutions[0].stepExecutionCount", equalTo(1))) + .andExpect(jsonPath("$.jobExecutions[0].jobExecution.stepExecutions", hasSize(1))); } @Test public void testGetInstancesByName() throws Exception { - mockMvc.perform(get("/jobs/instances/").param("name", JOB_NAME_ORIG).accept(MediaType.APPLICATION_JSON)).andDo(print()) - .andExpect(status().isOk()).andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobName", is(JOB_NAME_ORIG))) - .andExpect(jsonPath("$._embedded.jobInstanceResourceList", hasSize(1))); + mockMvc.perform(get("/jobs/instances/").param("name", JOB_NAME_ORIG).accept(MediaType.APPLICATION_JSON)) + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobName", is(JOB_NAME_ORIG))) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList", hasSize(1))); } @Test public void testGetExecutionsByNameMultipleResult() throws Exception { mockMvc.perform(get("/jobs/instances/").param("name", JOB_NAME_FOOBAR).accept(MediaType.APPLICATION_JSON)) - .andExpect(status().isOk()).andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobName", is(JOB_NAME_FOOBAR))) - .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobExecutions[0].executionId", is(4))) - .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobExecutions[1].executionId", is(3))) - .andExpect(jsonPath("$._embedded.jobInstanceResourceList", hasSize(1))); + .andDo(print()) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobName", is(JOB_NAME_FOOBAR))) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobExecutions[0].executionId", is(4))) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList[0].jobExecutions[1].executionId", is(3))) + .andExpect(jsonPath("$._embedded.jobInstanceResourceList", hasSize(1))); } @Test public void testGetInstanceByNameNotFound() throws Exception { mockMvc.perform(get("/jobs/instances/").param("name", "BAZ").accept(MediaType.APPLICATION_JSON)) - .andExpect(status().is4xxClientError()).andReturn().getResponse().getContentAsString() - .contains("NoSuchJobException"); + .andDo(print()) + .andExpect(status().is4xxClientError()) + .andExpect(content().string(containsString("NoSuchJobException"))); } private void createSampleJob(String jobName, int jobExecutionCount) { @@ -165,6 +172,9 @@ private void createSampleJob(String jobName, int jobExecutionCount) { TaskBatchDao taskBatchDao = taskBatchDaoContainer.get(defaultSchemaTarget); for (int i = 0; i < jobExecutionCount; i++) { JobExecution jobExecution = jobRepository.createJobExecution(instance, new JobParameters(), null); + StepExecution stepExecution = new StepExecution("foo", jobExecution, 1L); + stepExecution.setId(null); + jobRepository.add(stepExecution); taskBatchDao.saveRelationship(taskExecution, jobExecution); } }