diff --git a/execution/src/main/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacade.java b/execution/src/main/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacade.java index 2eedf55bb..d8b594bcc 100644 --- a/execution/src/main/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacade.java +++ b/execution/src/main/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacade.java @@ -18,8 +18,10 @@ package org.springframework.batch.execution.facade; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Properties; import org.springframework.batch.core.configuration.JobConfiguration; @@ -30,7 +32,6 @@ import org.springframework.batch.core.domain.JobIdentifier; import org.springframework.batch.core.domain.JobInstance; import org.springframework.batch.core.executor.JobExecutor; import org.springframework.batch.core.repository.JobRepository; -import org.springframework.batch.core.runtime.JobExecutionRegistry; import org.springframework.batch.execution.job.DefaultJobExecutor; import org.springframework.batch.repeat.ExitStatus; import org.springframework.batch.repeat.RepeatContext; @@ -54,11 +55,11 @@ import org.springframework.util.Assert; public class SimpleJobExecutorFacade implements JobExecutorFacade, JobExecutionListener, StatisticsProvider { - private JobExecutor jobExecutor; + private JobExecutor jobExecutor = new DefaultJobExecutor(); private JobRepository jobRepository; - private JobExecutionRegistry jobExecutionRegistry = new VolatileJobExecutionRegistry(); + private Map jobExecutionRegistry = new HashMap(); // there is no sensible default for this private JobConfigurationLocator jobConfigurationLocator; @@ -90,22 +91,6 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, } } - public SimpleJobExecutorFacade() { - jobExecutor = new DefaultJobExecutor(); - } - - /** - * Setter for the job execution registry. The default should be adequate so - * this setter method is mainly used for testing. - * - * @param jobExecutionRegistry - * the jobExecutionRegistry to set - */ - public void setJobExecutionRegistry( - JobExecutionRegistry jobExecutionRegistry) { - this.jobExecutionRegistry = jobExecutionRegistry; - } - /** * Setter for injection of {@link JobConfigurationLocator}. * @@ -143,7 +128,7 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, "JobIdentifier name must not be null."); Assert - .state(!jobExecutionRegistry.isRegistered(jobIdentifier), + .state(!jobExecutionRegistry.containsKey(jobIdentifier), "A job with this JobRuntimeInformation is already executing in this container"); Assert.state(jobExecutor != null, "JobExecutor must be provided."); @@ -155,7 +140,8 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, JobInstance job = jobRepository.findOrCreateJob(jobConfiguration, jobIdentifier); - JobExecution jobExecution = jobExecutionRegistry.register(job); + JobExecution jobExecution = new JobExecution(job); + jobExecutionRegistry.put(jobIdentifier, jobExecution); try { @@ -166,6 +152,7 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, } finally { this.after(jobExecution); + jobExecutionRegistry.remove(jobIdentifier); } @@ -210,7 +197,6 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, // not running any more running--; } - jobExecutionRegistry.unregister(execution.getJobIdentifier()); } /* @@ -220,7 +206,7 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, */ public void stop(JobIdentifier runtimeInformation) throws NoSuchJobExecutionException { - JobExecution jobExecutionContext = jobExecutionRegistry + JobExecution jobExecutionContext = (JobExecution) jobExecutionRegistry .get(runtimeInformation); if (jobExecutionContext == null) { throw new NoSuchJobExecutionException("No such Job is executing: [" @@ -262,7 +248,7 @@ public class SimpleJobExecutorFacade implements JobExecutorFacade, public Properties getStatistics() { int i = 0; Properties props = new Properties(); - for (Iterator iter = jobExecutionRegistry.findAll().iterator(); iter + for (Iterator iter = jobExecutionRegistry.values().iterator(); iter .hasNext();) { JobExecution element = (JobExecution) iter.next(); i++; diff --git a/execution/src/main/java/org/springframework/batch/execution/facade/VolatileJobExecutionRegistry.java b/execution/src/main/java/org/springframework/batch/execution/facade/VolatileJobExecutionRegistry.java deleted file mode 100644 index cff2848f8..000000000 --- a/execution/src/main/java/org/springframework/batch/execution/facade/VolatileJobExecutionRegistry.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright 2006-2007 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.batch.execution.facade; - -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; - -import org.springframework.batch.core.domain.JobExecution; -import org.springframework.batch.core.domain.JobIdentifier; -import org.springframework.batch.core.domain.JobInstance; -import org.springframework.batch.core.runtime.JobExecutionRegistry; - -/** - * Simple in-memory implementation of {@link JobExecutionRegistry}. - * Synchronizes all access to the underlying storage. Good for most purposes. - * - * @author Dave Syer - * - */ -public class VolatileJobExecutionRegistry implements JobExecutionRegistry { - - private Map contexts = new HashMap(); - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#findByName(java.lang.String) - */ - public Collection findByName(String name) { - Set values = new HashSet(); - HashMap contexts; - synchronized (this.contexts) { - contexts = new HashMap(this.contexts); - } - for (Iterator iter = contexts.entrySet().iterator(); iter.hasNext();) { - Map.Entry entry = (Map.Entry) iter.next(); - String runtimeName = ((JobIdentifier) entry.getKey()).getName(); - if ((name == null && runtimeName == null) || name.equals(runtimeName)) { - values.add(entry.getValue()); - } - } - return values; - } - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#findAll() - */ - public Collection findAll() { - - synchronized (this.contexts) { - return new HashSet(contexts.values()); - } - - } - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#findByRuntimeInformation(org.springframework.batch.container.common.runtime.JobRuntimeInformation) - */ - public JobExecution get(JobIdentifier runtimeInformation) { - - synchronized (this.contexts) { - return (JobExecution) contexts.get(runtimeInformation); - } - - } - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#isRegistered(org.springframework.batch.container.common.runtime.JobRuntimeInformation) - */ - public boolean isRegistered(JobIdentifier runtimeInformation) { - - synchronized (this.contexts) { - return contexts.containsKey(runtimeInformation); - } - } - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#register(org.springframework.batch.container.common.runtime.JobRuntimeInformation, - * org.springframework.batch.container.common.domain.JobExecution) - */ - public JobExecution register(JobInstance job) { - - JobIdentifier jobIdentifier = job.getIdentifier(); - - if (isRegistered(jobIdentifier)) { - return get(jobIdentifier); - } - JobExecution context = new JobExecution(job); - - synchronized (this.contexts) { - contexts.put(jobIdentifier, context); - } - - return context; - } - - /* - * (non-Javadoc) - * @see org.springframework.batch.container.common.executor.JobExecutionRegistry#unregister(org.springframework.batch.container.common.runtime.JobRuntimeInformation) - */ - public void unregister(JobIdentifier runtimeInformation) { - - synchronized (this.contexts) { - contexts.remove(runtimeInformation); - } - - } - -} diff --git a/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacadeTests.java b/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacadeTests.java index 1356e500c..1596f518f 100644 --- a/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacadeTests.java +++ b/execution/src/test/java/org/springframework/batch/execution/facade/SimpleJobExecutorFacadeTests.java @@ -16,9 +16,10 @@ package org.springframework.batch.execution.facade; +import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Properties; import junit.framework.TestCase; @@ -31,11 +32,11 @@ import org.springframework.batch.core.domain.JobExecution; import org.springframework.batch.core.domain.JobInstance; import org.springframework.batch.core.executor.JobExecutor; import org.springframework.batch.core.repository.JobRepository; -import org.springframework.batch.core.runtime.JobExecutionRegistry; import org.springframework.batch.core.runtime.SimpleJobIdentifier; import org.springframework.batch.io.exception.BatchCriticalException; import org.springframework.batch.repeat.ExitStatus; import org.springframework.batch.repeat.context.RepeatContextSupport; +import org.springframework.util.ReflectionUtils; /** * SimpleBatchContainer unit tests. @@ -169,12 +170,7 @@ public class SimpleJobExecutorFacadeTests extends TestCase { } public void testStopWithNoJob() throws Exception { - MockControl control = MockControl.createControl(JobExecutionRegistry.class); - JobExecutionRegistry jobExecutionRegistry = (JobExecutionRegistry) control.getMock(); - jobExecutorFacade.setJobExecutionRegistry(jobExecutionRegistry); SimpleJobIdentifier runtimeInformation = new SimpleJobIdentifier("TestJob"); - control.expectAndReturn(jobExecutionRegistry.get(runtimeInformation), null); - control.replay(); try { jobExecutorFacade.stop(runtimeInformation); fail("Expected NoSuchJobExecutionException"); @@ -182,25 +178,19 @@ public class SimpleJobExecutorFacadeTests extends TestCase { // expected assertTrue(e.getMessage().indexOf("TestJob")>=0); } - control.verify(); } public void testStop() throws Exception { - JobExecutionRegistry jobExecutionRegistry = new VolatileJobExecutionRegistry(); - jobExecutorFacade.setJobExecutionRegistry(jobExecutionRegistry); SimpleJobIdentifier runtimeInformation = new SimpleJobIdentifier("TestJob"); - JobExecution context = jobExecutionRegistry.register(new JobInstance(runtimeInformation, new Long(0))); + JobExecution execution = new JobExecution(new JobInstance(runtimeInformation, new Long(0))); + registerExecution(runtimeInformation, execution); RepeatContextSupport stepContext = new RepeatContextSupport(null); RepeatContextSupport chunkContext = new RepeatContextSupport(stepContext); - context.registerStepContext(stepContext); - context.registerChunkContext(chunkContext); + execution.registerStepContext(stepContext); + execution.registerChunkContext(chunkContext); jobExecutorFacade.stop(runtimeInformation); - // It is only unregistered when the start method finishes, and it hasn't - // been called. - assertTrue(jobExecutionRegistry.isRegistered(runtimeInformation)); - assertTrue(stepContext.isCompleteOnly()); assertTrue(chunkContext.isCompleteOnly()); } @@ -210,19 +200,13 @@ public class SimpleJobExecutorFacadeTests extends TestCase { } public void testStatisticsWithContext() throws Exception { - MockControl control = MockControl.createControl(JobExecutionRegistry.class); - JobExecutionRegistry jobExecutionRegistry = (JobExecutionRegistry) control.getMock(); - jobExecutorFacade.setJobExecutionRegistry(jobExecutionRegistry); SimpleJobIdentifier runtimeInformation = new SimpleJobIdentifier("TestJob"); - JobExecution jobExecutionContext = new JobExecution(new JobInstance(runtimeInformation, new Long(0))); - jobExecutionContext.registerStepContext(new RepeatContextSupport(null)); - jobExecutionContext.registerChunkContext(new RepeatContextSupport(null)); - control.expectAndReturn(jobExecutionRegistry.findAll(), Collections.singleton(jobExecutionContext)); - control.replay(); + JobExecution execution = new JobExecution(new JobInstance(runtimeInformation, new Long(0))); + registerExecution(runtimeInformation, execution); + execution.registerStepContext(new RepeatContextSupport(null)); Properties statistics = jobExecutorFacade.getStatistics(); assertNotNull(statistics); assertTrue(statistics.containsKey("job1.step1")); - control.verify(); } public void testListenersCalledLastOnAfter() throws Exception { @@ -261,5 +245,15 @@ public class SimpleJobExecutorFacadeTests extends TestCase { assertEquals("two", list.get(1)); } + private void registerExecution(SimpleJobIdentifier runtimeInformation, + JobExecution execution) throws NoSuchFieldException, + IllegalAccessException { + Field field = SimpleJobExecutorFacade.class.getDeclaredField("jobExecutionRegistry"); + ReflectionUtils.makeAccessible(field); + Map map = (Map) field.get(jobExecutorFacade); + map.put(runtimeInformation, execution); + } + } +