From 0103ef025d1893cc6edb39684c4e00abd6779cef Mon Sep 17 00:00:00 2001 From: Mahmoud Ben Hassine Date: Mon, 20 Feb 2023 09:57:08 +0100 Subject: [PATCH] Fix object graph deletion in SimpleJobRepository#deleteJobInstance Before this commit, SimpleJobRepository#deleteJobInstance was only deleting the given job instance and not the entire object graph, which leads to data inconsistency (orphan job executions and invalid foreign keys). This commit fixes the contract and the implementation of the method to delete the entire object graph. Resolves #4250 --- .../batch/core/repository/JobRepository.java | 6 ++++-- .../core/repository/dao/JobInstanceDao.java | 5 +++-- .../support/SimpleJobRepository.java | 4 ++++ .../support/SimpleJobRepositoryTests.java | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java index ec1589452..6d7bcf03e 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -244,7 +244,9 @@ public interface JobRepository { } /** - * Delete the job instance. + * Delete the job instance object graph (ie the job instance with all associated job + * executions along with their respective object graphs as specified in + * {@link #deleteJobExecution(JobExecution)}). * @param jobInstance the job instance to delete * @since 5.0 */ diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JobInstanceDao.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JobInstanceDao.java index 074e4a187..ea857a99d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JobInstanceDao.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JobInstanceDao.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -131,7 +131,8 @@ public interface JobInstanceDao { long getJobInstanceCount(@Nullable String jobName) throws NoSuchJobException; /** - * Delete the job instance. + * Delete the job instance. This method is not expected to delete the associated job + * executions. If this is needed, clients of this method should do that manually. * @param jobInstance the job instance to delete * @since 5.0 */ diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java index 00a63a90f..960c25bc5 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java @@ -328,6 +328,10 @@ public class SimpleJobRepository implements JobRepository { @Override public void deleteJobInstance(JobInstance jobInstance) { + List jobExecutions = this.jobExecutionDao.findJobExecutions(jobInstance); + for (JobExecution jobExecution : jobExecutions) { + deleteJobExecution(jobExecution); + } this.jobInstanceDao.deleteJobInstance(jobInstance); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java index da5967961..826e5a295 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java @@ -366,4 +366,22 @@ class SimpleJobRepositoryTests { verify(this.jobExecutionDao).deleteJobExecution(jobExecution); } + @Test + void testDeleteJobInstance() { + // given + JobExecution jobExecution1 = mock(JobExecution.class); + JobExecution jobExecution2 = mock(JobExecution.class); + JobInstance jobInstance = mock(JobInstance.class); + when(this.jobExecutionDao.findJobExecutions(jobInstance)) + .thenReturn(Arrays.asList(jobExecution1, jobExecution2)); + + // when + this.jobRepository.deleteJobInstance(jobInstance); + + // then + verify(this.jobExecutionDao).deleteJobExecution(jobExecution1); + verify(this.jobExecutionDao).deleteJobExecution(jobExecution2); + verify(this.jobInstanceDao).deleteJobInstance(jobInstance); + } + }