Quartz Scheduler - configurable SchedulerFactory [SPR-16439]

[spring-projects-issues]

Andrej Urvantsev opened SPR-16439 and commented

I was trying to integrate quartz-mongodb with Spring, but found that it's nearly impossible to do that without ugly hacks.

What I wanted is to provide a bean that implements org.quartz.SchedulerFactory which injects MongoClient, so I can create a scheduler which can store jobs in MongoDB.

The problem is in the implementation of org.springframework.scheduling.quartz.SchedulerFactoryBean.afterPropertiesSet:

// Create SchedulerFactory instance...
SchedulerFactory schedulerFactory = BeanUtils.instantiateClass(this.schedulerFactoryClass);
initSchedulerFactory(schedulerFactory);

Here it instantiates a factory using reflection(I think). Instead I would like to have conditional scheduler factory initialization - if there is a bean of org.quartz.SchedulerFactory, then use it, otherwise use the code above.

Or even better: StdSchedulerFactory can be created as conditional bean as well - if there is not any SchedulerFactory, then create default one.

In the worst case the code above can be moved to protected method so it's possible to override it.

---

Affects: 4.3.14, 5.0.3

Issue Links:

• Quartz Scheduler -allow to use standard properties in non-standard factories [SPR-16440] #20986 Quartz Scheduler -allow to use standard properties in non-standard factories

Referenced from: commits cd57335, 857a5b0, 33d655a, c7f60d1

Backported to: 4.3.15

[spring-projects-issues]

Juergen Hoeller commented

Considering the SchedulerFactory itself as a separate bean is a bit messy since the SchedulerFactory lifecycle is being managed by SchedulerFactoryBean to some degree, in particular for a StdSchedulerFactory which will have its initialize(Properties) callback triggered in the SchedulerFactoryBean.afterPropertiesSet() phase.

That said, what we can certainly do is to open it up for overriding. We could either provide a protected createSchedulerFactory template method, or we could offer a setSchedulerFactory method where an external SchedulerFactory may be passed to SchedulerFactoryBean (potentially as an inner bean definition). In the latter case, we'd simply take the given SchedulerFactory instead of instantiating our own, nevertheless applying our local configuration settings to it if it extends StdSchedulerFactory.

[spring-projects-issues]

Juergen Hoeller commented

I went with a setSchedulerFactory(SchedulerFactory) method on SchedulerFactoryBean. If that method is being called, it overrides any class specified through setSchedulerFactoryClass.

[spring-projects-issues]

Andrej Urvantsev commented

I think setSchedulerFactory approach is the lowest hanging fruit here, and it probably fits the current design of SchedulerFactoryBean.

And, yes, if provided factory does not extend StdSchedulerFactory, then initialization should not be applied I believe. Otherwise this part looks strange:

if (this.configLocation != null || this.quartzProperties != null ||
					this.taskExecutor != null || this.dataSource != null) {
				throw new IllegalArgumentException(
						"StdSchedulerFactory required for applying Quartz properties: " + schedulerFactory);
			}

from initSchedulerFactory method - why forbid to use 'standard' properties for non-standard factory.

Or maybe factories which are set through

[spring-projects-issues]

Andrej Urvantsev commented

Could you also back-port it to 4.3 branch?

[spring-projects-issues]

Juergen Hoeller commented

We can't apply those properties to arbitrary SchedulerFactory implementations since initialize(Properties) only exists on StdSchedulerFactory. That if check simply asserts that none of those properties has been set on SchedulerFactoryBean since we can't pass them on in such a scenario. Instead of simply ignoring some of the user-specified properties, we explicitly raise an exception there. With a custom SchedulerFactory like in your case, you simply shouldn't set any of those properties on SchedulerFactoryBean but rather initialize the affected settings in the external factory yourself.

The 4.3.x branch is generally in maintenance mode already. However, this one is easy enough to backport: I'll do so later this week (along with a few other recent fixes) so that it'll appear in 4.3.15.BUILD-SNAPSHOT.

[spring-projects-issues]

Andrej Urvantsev commented

Created #20986 about initialization.

[spring-projects-issues]

Juergen Hoeller commented

I've relaxed the local properties check to only apply to schedulerFactoryClass but not to an external provided reference through the schedulerFactory property. With the latter, you may extend from StdSchedulerFactory in order to receive locally defined properties; otherwise such local properties will simply be ignored.

[spring-projects-issues]

Andrej Urvantsev commented

Sorry for being annoying and thank you for your patience :)

It looks a way better now - just a thought: when you provide schedulerFactoryClass then there is a sense in invoking initSchedulerFactory() - because it's a part of instance creation. But when you provide schedulerFactory then I would assume that factory is already initialized, so I would not init it at all. Because it might overwrite something which is already set and mess it up.

TL;DR: I would use initSchedulerFactory only when schedulerFactoryClass is used.

[spring-projects-issues]

Juergen Hoeller commented

Indeed, since we're free to define the semantics for an externally provided SchedulerFactory instance and since StdSchedulerFactory is designed with initializing constructors, I agree that it's better to never call initialize(Properties) for such an instance at all and always rely on the external instance to be fully initialized upfront.