From 4dc406ef6083d6fdeb4ceac7fb9c4828c28b0a92 Mon Sep 17 00:00:00 2001 From: Mahmoud Ben Hassine Date: Mon, 29 Jan 2018 16:58:24 +0100 Subject: [PATCH] BATCH-2564: make AutomaticJobRegistrar implement SmartLifeCycle Before this commit, the AutomaticJobRegistrar was started on ContextRefreshedEvent event. This is sometimes too late to register jobs especially when other life cycle components that use the jobs are started before this registrar. This commit changes the AutomaticJobRegistrar to implement SmartLifeCycle and makes its autoStartup and phase properties configurable. It should be noted that the "onApplicationEvent" method has been removed even if it is a public API. This method is not intended to be used by client code and even if it was, its usage is considered wrong anyway. Resolves BATCH-2564 --- .../support/AutomaticJobRegistrar.java | 64 ++++++++++++------- .../support/AutomaticJobRegistrarTests.java | 51 ++++++++------- 2 files changed, 65 insertions(+), 50 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrar.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrar.java index 5a8fa8a61..a4eb0d82a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrar.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2018 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. @@ -25,11 +25,9 @@ import org.springframework.batch.core.configuration.JobRegistry; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.context.ApplicationListener; import org.springframework.context.Lifecycle; +import org.springframework.context.SmartLifecycle; import org.springframework.context.event.ApplicationContextEvent; -import org.springframework.context.event.ContextClosedEvent; -import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.Ordered; import org.springframework.util.Assert; @@ -40,10 +38,11 @@ import org.springframework.util.Assert; * * @author Lucas Ward * @author Dave Syer + * @author Mahmoud Ben Hassine * * @since 2.1 */ -public class AutomaticJobRegistrar implements Ordered, Lifecycle, ApplicationListener, ApplicationContextAware, +public class AutomaticJobRegistrar implements Ordered, SmartLifecycle, ApplicationContextAware, InitializingBean { private Collection applicationContextFactories = new ArrayList(); @@ -54,6 +53,10 @@ public class AutomaticJobRegistrar implements Ordered, Lifecycle, ApplicationLis private volatile boolean running = false; + private int phase = Integer.MIN_VALUE + 1000; + + private boolean autoStartup = true; + private Object lifecycleMonitor = new Object(); private int order = Ordered.LOWEST_PRECEDENCE; @@ -125,25 +128,6 @@ public class AutomaticJobRegistrar implements Ordered, Lifecycle, ApplicationLis } - /** - * Creates all the application contexts required and set up job registry entries with all the instances of - * {@link Job} found therein. Also closes the contexts when the enclosing context is closed. - * - * @see InitializingBean#afterPropertiesSet() - */ - @Override - public final void onApplicationEvent(ApplicationContextEvent event) { - // TODO: With Spring 3 a SmartLifecycle is started automatically - if (event.getSource() == applicationContext) { - if (event instanceof ContextRefreshedEvent) { - start(); - } - else if (event instanceof ContextClosedEvent) { - stop(); - } - } - } - /** * Delegates to {@link JobLoader#clear()}. * @@ -193,4 +177,36 @@ public class AutomaticJobRegistrar implements Ordered, Lifecycle, ApplicationLis } } + @Override + public boolean isAutoStartup() { + return autoStartup; + } + + /** + * @param autoStartup true for auto start. + * @see #isAutoStartup() + */ + public void setAutoStartup(boolean autoStartup) { + this.autoStartup = autoStartup; + } + + @Override + public int getPhase() { + return phase; + } + + /** + * @param phase the phase. + * @see #getPhase() + */ + public void setPhase(int phase) { + this.phase = phase; + } + + @Override + public void stop(Runnable callback) { + stop(); + callback.run(); + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrarTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrarTests.java index 956b28a61..7c4d9cfab 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrarTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2010-2014 the original author or authors. + * Copyright 2010-2018 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. @@ -25,11 +25,10 @@ import java.util.Collection; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import org.springframework.batch.core.Job; import org.springframework.beans.factory.BeanCreationException; import org.springframework.context.ApplicationContext; -import org.springframework.context.event.ContextClosedEvent; -import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.Ordered; @@ -40,6 +39,7 @@ import org.springframework.core.io.Resource; * * @author Dave Syer * @author Lucas Ward + * @author Mahmoud Ben Hassine * */ public class AutomaticJobRegistrarTests { @@ -66,6 +66,20 @@ public class AutomaticJobRegistrarTests { } + @Test + public void testDefaultAutoStartup() throws Exception { + + assertTrue(registrar.isAutoStartup()); + + } + + @Test + public void testDefaultPhase() throws Exception { + + assertEquals(Integer.MIN_VALUE + 1000, registrar.getPhase()); + + } + @Test public void testLocateJob() throws Exception { @@ -176,36 +190,21 @@ public class AutomaticJobRegistrarTests { } - @SuppressWarnings("resource") @Test - public void testInitCalledOnContextRefreshed() throws Exception { + public void testStartStopRunningWithCallback() throws Exception { + Runnable callback = Mockito.mock(Runnable.class); Resource[] jobPaths = new Resource[] { new ClassPathResource( "org/springframework/batch/core/launch/support/2jobs.xml") }; - registrar.setApplicationContext(new ClassPathXmlApplicationContext( - "/org/springframework/batch/core/launch/support/test-environment-with-registry-and-auto-register.xml")); - GenericApplicationContext applicationContext = new GenericApplicationContext(); - applicationContext.refresh(); - setUpApplicationContextFactories(jobPaths, applicationContext); - registrar.setApplicationContext(applicationContext); - registrar.onApplicationEvent(new ContextRefreshedEvent(applicationContext)); - assertEquals(2, registry.getJobNames().size()); - } - - @Test - public void testClearCalledOnContextClosed() throws Exception { - - Resource[] jobPaths = new Resource[] { new ClassPathResource( - "org/springframework/batch/core/launch/support/2jobs.xml") }; - @SuppressWarnings("resource") - GenericApplicationContext applicationContext = new GenericApplicationContext(); - applicationContext.refresh(); - setUpApplicationContextFactories(jobPaths, applicationContext); - registrar.setApplicationContext(applicationContext); + setUpApplicationContextFactories(jobPaths, null); + registrar.start(); + assertTrue(registrar.isRunning()); registrar.start(); assertEquals(2, registry.getJobNames().size()); - registrar.onApplicationEvent(new ContextClosedEvent(applicationContext)); + registrar.stop(callback); + assertFalse(registrar.isRunning()); assertEquals(0, registry.getJobNames().size()); + Mockito.verify(callback, Mockito.times(1)).run(); }