Stop using Constants utility in CronTriggerFactoryBean

See spring-projectsgh-30851
sbrannen · Jul 16, 2023 · edba535 · edba535
1 parent 79cf532
commit edba535
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 8 deletions.
diff --git a/...t-support/src/main/java/org/springframework/scheduling/quartz/CronTriggerFactoryBean.java b/...t-support/src/main/java/org/springframework/scheduling/quartz/CronTriggerFactoryBean.java
@@ -30,7 +30,6 @@
 import org.springframework.beans.factory.BeanNameAware;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.InitializingBean;
-import org.springframework.core.Constants;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -48,6 +47,7 @@
  * instead of registering the JobDetail separately.
  *
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.1
  * @see #setName
  * @see #setGroup
@@ -58,8 +58,16 @@
  */
 public class CronTriggerFactoryBean implements FactoryBean<CronTrigger>, BeanNameAware, InitializingBean {
 
-	/** Constants for the CronTrigger class. */
-	private static final Constants constants = new Constants(CronTrigger.class);
+	/**
+	 * Map of constant names to constant values for the misfire instruction constants
+	 * defined in {@link org.quartz.Trigger} and {@link org.quartz.CronTrigger}.
+	 */
+	private static final Map<String, Integer> constants = Map.of(
+			"MISFIRE_INSTRUCTION_SMART_POLICY", CronTrigger.MISFIRE_INSTRUCTION_SMART_POLICY,
+			"MISFIRE_INSTRUCTION_IGNORE_MISFIRE_POLICY", CronTrigger.MISFIRE_INSTRUCTION_IGNORE_MISFIRE_POLICY,
+			"MISFIRE_INSTRUCTION_FIRE_ONCE_NOW", CronTrigger.MISFIRE_INSTRUCTION_FIRE_ONCE_NOW,
+			"MISFIRE_INSTRUCTION_DO_NOTHING", CronTrigger.MISFIRE_INSTRUCTION_DO_NOTHING
+		);
 
 
 	@Nullable
@@ -213,7 +221,10 @@ public void setMisfireInstruction(int misfireInstruction) {
 	 * @see org.quartz.CronTrigger#MISFIRE_INSTRUCTION_DO_NOTHING
 	 */
 	public void setMisfireInstructionName(String constantName) {
-		this.misfireInstruction = constants.asNumber(constantName).intValue();
+		Assert.hasText(constantName, "'constantName' must not be null or blank");
+		Integer misfireInstruction = constants.get(constantName);
+		Assert.notNull(misfireInstruction, "Only misfire instruction constants allowed");
+		this.misfireInstruction = misfireInstruction;
 	}
 
 	/**

diff --git a/...port/src/test/java/org/springframework/scheduling/quartz/CronTriggerFactoryBeanTests.java b/...port/src/test/java/org/springframework/scheduling/quartz/CronTriggerFactoryBeanTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-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.
@@ -16,26 +16,62 @@
 
 package org.springframework.scheduling.quartz;
 
+import java.lang.reflect.Field;
 import java.text.ParseException;
+import java.util.Arrays;
+import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
 import org.quartz.CronTrigger;
 
+import org.springframework.util.ReflectionUtils;
+
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 
 /**
+ * Tests for {@link CronTriggerFactoryBean}.
+ *
  * @author Stephane Nicoll
+ * @author Sam Brannen
  */
-public class CronTriggerFactoryBeanTests {
+class CronTriggerFactoryBeanTests {
+
+	private final CronTriggerFactoryBean factory = new CronTriggerFactoryBean();
+
 
 	@Test
-	public void createWithoutJobDetail() throws ParseException {
-		CronTriggerFactoryBean factory = new CronTriggerFactoryBean();
+	void createWithoutJobDetail() throws ParseException {
 		factory.setName("myTrigger");
 		factory.setCronExpression("0 15 10 ? * *");
 		factory.afterPropertiesSet();
 		CronTrigger trigger = factory.getObject();
 		assertThat(trigger.getCronExpression()).isEqualTo("0 15 10 ? * *");
 	}
 
+	@Test
+	void setMisfireInstructionNameToUnsupportedValues() {
+		assertThatIllegalArgumentException().isThrownBy(() -> factory.setMisfireInstructionName(null));
+		assertThatIllegalArgumentException().isThrownBy(() -> factory.setMisfireInstructionName("   "));
+		assertThatIllegalArgumentException().isThrownBy(() -> factory.setMisfireInstructionName("bogus"));
+	}
+
+	/**
+	 * This test effectively verifies that the internal 'constants' map is properly
+	 * configured for all MISFIRE_INSTRUCTION_ constants defined in {@link CronTrigger}.
+	 */
+	@Test
+	void setMisfireInstructionNameToAllSupportedValues() {
+		streamMisfireInstructionConstants()
+				.map(Field::getName)
+				.forEach(name -> assertThatNoException().as(name).isThrownBy(() -> factory.setMisfireInstructionName(name)));
+	}
+
+	private static Stream<Field> streamMisfireInstructionConstants() {
+		return Arrays.stream(CronTrigger.class.getFields())
+				.filter(ReflectionUtils::isPublicStaticFinal)
+				.filter(field -> field.getName().startsWith("MISFIRE_INSTRUCTION_"));
+	}
+
 }