From 9c676e98c0d95bea53e7355c4f092b95b3767ab5 Mon Sep 17 00:00:00 2001 From: robokaso Date: Wed, 15 Oct 2008 08:20:29 +0000 Subject: [PATCH] REOPENED - BATCH-857: map daos need to be truly transactional for correct restart backported cloning for MapJobExecutionContextDao --- .../repository/dao/MapJobExecutionDao.java | 26 ++++++++++---- .../dao/AbstractJobExecutionDaoTests.java | 23 ++++-------- .../dao/MapJobExecutionDaoTests.java | 35 +++++++++++++++++++ 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/MapJobExecutionDao.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/MapJobExecutionDao.java index cec55a025..95a103c4f 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/MapJobExecutionDao.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/MapJobExecutionDao.java @@ -5,6 +5,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import org.apache.commons.lang.SerializationUtils; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobInstance; import org.springframework.batch.item.ExecutionContext; @@ -28,6 +29,14 @@ public class MapJobExecutionDao implements JobExecutionDao { contextsByJobExecutionId.clear(); } + private static final JobExecution copy(JobExecution original) { + return (JobExecution) SerializationUtils.deserialize(SerializationUtils.serialize(original)); + } + + private static final ExecutionContext copy(ExecutionContext original) { + return (ExecutionContext) SerializationUtils.deserialize(SerializationUtils.serialize(original)); + } + public int getJobExecutionCount(JobInstance jobInstance) { int count = 0; for (Iterator iterator = executionsById.values().iterator(); iterator.hasNext();) { @@ -44,7 +53,7 @@ public class MapJobExecutionDao implements JobExecutionDao { Long newId = new Long(currentId++); jobExecution.setId(newId); jobExecution.incrementVersion(); - executionsById.put(newId, jobExecution); + executionsById.put(newId, copy(jobExecution)); } public List findJobExecutions(JobInstance jobInstance) { @@ -52,7 +61,7 @@ public class MapJobExecutionDao implements JobExecutionDao { for (Iterator iterator = executionsById.values().iterator(); iterator.hasNext();) { JobExecution exec = (JobExecution) iterator.next(); if (exec.getJobInstance().equals(jobInstance)) { - executions.add(exec); + executions.add(copy(exec)); } } return executions; @@ -63,7 +72,7 @@ public class MapJobExecutionDao implements JobExecutionDao { Assert.notNull(id, "JobExecution is expected to have an id (should be saved already)"); Assert.notNull(executionsById.get(id), "JobExecution must already be saved"); jobExecution.incrementVersion(); - executionsById.put(id, jobExecution); + executionsById.put(id, copy(jobExecution)); } public JobExecution getLastJobExecution(JobInstance jobInstance) { @@ -80,15 +89,20 @@ public class MapJobExecutionDao implements JobExecutionDao { lastExec = exec; } } - return lastExec; + if (lastExec == null) { + return null; + } + else { + return copy(lastExec); + } } public ExecutionContext findExecutionContext(JobExecution jobExecution) { - return (ExecutionContext) contextsByJobExecutionId.get(jobExecution.getId()); + return copy((ExecutionContext) contextsByJobExecutionId.get(jobExecution.getId())); } public void saveOrUpdateExecutionContext(JobExecution jobExecution) { - contextsByJobExecutionId.put(jobExecution.getId(), jobExecution.getExecutionContext()); + contextsByJobExecutionId.put(jobExecution.getId(), copy(jobExecution.getExecutionContext())); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java index c5e6915ea..0a3129134 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java @@ -1,7 +1,6 @@ package org.springframework.batch.core.repository.dao; import java.util.Date; -import java.util.HashMap; import java.util.List; import org.springframework.batch.core.BatchStatus; @@ -91,11 +90,9 @@ public abstract class AbstractJobExecutionDaoTests extends AbstractTransactional JobExecution exec1 = new JobExecution(jobInstance); exec1.setCreateTime(new Date(0)); - ExecutionContext ctx = new ExecutionContext() { - { - put("key", "value"); - } - }; + ExecutionContext ctx = new ExecutionContext(); + ctx.put("key", "value"); + JobExecution exec2 = new JobExecution(jobInstance); exec2.setExecutionContext(ctx); exec2.setCreateTime(new Date(1)); @@ -111,11 +108,8 @@ public abstract class AbstractJobExecutionDaoTests extends AbstractTransactional public void testSaveAndFindContext() { dao.saveJobExecution(execution); - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + ExecutionContext ctx = new ExecutionContext(); + ctx.put("key", "value"); execution.setExecutionContext(ctx); dao.saveOrUpdateExecutionContext(execution); @@ -135,11 +129,8 @@ public abstract class AbstractJobExecutionDaoTests extends AbstractTransactional public void testUpdateContext() { dao.saveJobExecution(execution); - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + ExecutionContext ctx = new ExecutionContext(); + ctx.put("key", "value"); execution.setExecutionContext(ctx); dao.saveOrUpdateExecutionContext(execution); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/MapJobExecutionDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/MapJobExecutionDaoTests.java index 17ab9abc8..4906522bf 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/MapJobExecutionDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/MapJobExecutionDaoTests.java @@ -1,5 +1,12 @@ package org.springframework.batch.core.repository.dao; +import java.util.Date; + +import org.springframework.batch.core.JobExecution; +import org.springframework.batch.core.JobInstance; +import org.springframework.batch.core.JobParameters; +import org.springframework.batch.item.ExecutionContext; + public class MapJobExecutionDaoTests extends AbstractJobExecutionDaoTests { @@ -8,5 +15,33 @@ public class MapJobExecutionDaoTests extends AbstractJobExecutionDaoTests { MapJobInstanceDao.clear(); return new MapJobExecutionDao(); } + + /** + * Modifications to saved entity do not affect the persisted object. + */ + public void testPersistentCopy() { + + JobExecutionDao tested = new MapJobExecutionDao(); + JobInstance jobInstance = new JobInstance(new Long(1), new JobParameters(), "mapJob"); + JobExecution jobExecution = new JobExecution(jobInstance); + + assertNull(jobExecution.getStartTime()); + tested.saveJobExecution(jobExecution); + jobExecution.setStartTime(new Date()); + + JobExecution retrieved = tested.getLastJobExecution(jobInstance); + assertNull(retrieved.getStartTime()); + + tested.updateJobExecution(jobExecution); + jobExecution.setEndTime(new Date()); + assertNull(retrieved.getEndTime()); + + tested.saveOrUpdateExecutionContext(jobExecution); + jobExecution.getExecutionContext().put("key", "value"); + ExecutionContext stored = tested.findExecutionContext(jobExecution); + assertTrue(stored.isEmpty()); + + } + }