Skip to content

Commit

Permalink
Ensure sub-query use for Job instances. (#5563)
Browse files Browse the repository at this point in the history 
* Ensure sub-query use for Job instances.
 Fix find by name query ordering.
 Fix find by name not found query.
 Improve test case readability.

* Fix imports and spacing.
Removed .andDo(print())

* Fixed wildcard imports.
 #5484
  • Loading branch information
@corneil
corneil authored Nov 28, 2023
1 parent 1430b41 commit 90cd253
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .../main/java/org/springframework/cloud/dataflow/server/repository/AggregateJobQueryDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* @author Corneil du Plessis
*/
public interface AggregateJobQueryDao {
Page<JobInstanceExecutions> listJobInstances(String jobName, Pageable pageable);
Page<JobInstanceExecutions> listJobInstances(String jobName, Pageable pageable) throws NoSuchJobException;

Page<TaskJobExecution> listJobExecutions(String jobName, BatchStatus status, Pageable pageable) throws NoSuchJobExecutionException;

Expand Down
22 changes: 15 additions & 7 deletions ...n/java/org/springframework/cloud/dataflow/server/repository/JdbcAggregateJobQueryDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.springframework.cloud.dataflow.server.repository;

import javax.sql.DataSource;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.time.Instant;
Expand All @@ -29,6 +28,7 @@
import java.util.Map;
import java.util.TreeMap;
import java.util.stream.Collectors;
import javax.sql.DataSource;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -69,6 +69,7 @@
import org.springframework.jdbc.core.RowCallbackHandler;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -238,17 +239,18 @@ 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<JobInstanceExecutions> listJobInstances(String jobName, Pageable pageable) {
public Page<JobInstanceExecutions> listJobInstances(String jobName, Pageable pageable) throws NoSuchJobException {
int total = countJobExecutions(jobName);
List<JobInstanceExecutions> taskJobInstancesForJobName = total > 0
? getTaskJobInstancesForJobName(jobName, pageable)
: Collections.emptyList();
if (total == 0) {
throw new NoSuchJobException("No Job with that name either current or historic: [" + jobName + "]");
}
List<JobInstanceExecutions> taskJobInstancesForJobName = getTaskJobInstancesForJobName(jobName, pageable);
return new PageImpl<>(taskJobInstancesForJobName, pageable, total);

}
Expand All @@ -273,7 +275,13 @@ 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 (!ObjectUtils.isEmpty(jobInstanceExecution.getTaskJobExecutions())) {
jobInstanceExecution.getTaskJobExecutions().forEach((execution) ->
jobServiceContainer.get(execution.getSchemaTarget()).addStepExecutions(execution.getJobExecution())
);
}
return jobInstanceExecution;
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...in/java/org/springframework/cloud/dataflow/server/service/impl/DefaultTaskJobService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public TaskJobExecution getJobExecution(long id, String schemaTarget) throws NoS
}

@Override
public Page<JobInstanceExecutions> listTaskJobInstancesForJobName(Pageable pageable, String jobName) {
public Page<JobInstanceExecutions> 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);
Expand Down
11 changes: 6 additions & 5 deletions ...ava/org/springframework/cloud/dataflow/server/controller/JobExecutionControllerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.hamcrest.Matchers.containsInRelativeOrder;
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.put;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;

/**
* @author Glenn Renfro
Expand Down Expand Up @@ -212,7 +212,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
Expand All @@ -231,7 +232,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
Expand Down
44 changes: 27 additions & 17 deletions ...java/org/springframework/cloud/dataflow/server/controller/JobInstanceControllerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand All @@ -52,10 +53,11 @@
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
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.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;
Expand All @@ -65,9 +67,9 @@
* @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 {
Expand Down Expand Up @@ -105,7 +107,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);
Expand All @@ -122,36 +124,41 @@ public void testJobInstanceControllerConstructorMissingRepository() {
@Test
public void testGetInstanceNotFound() throws Exception {
mockMvc.perform(get("/jobs/instances/1345345345345").accept(MediaType.APPLICATION_JSON))
.andExpect(status().isNotFound());
.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))
.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))
.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)));
.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");
.andExpect(status().is4xxClientError())
.andExpect(content().string(containsString("NoSuchJobException")));
}

private void createSampleJob(String jobName, int jobExecutionCount) {
Expand All @@ -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);
}
}
Expand Down

0 comments on commit 90cd253

Please sign in to comment.