Stop using Constants utility in SimpleTriggerFactoryBean

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

Hide file tree

Showing 2 changed files with 65 additions and 10 deletions.
diff --git a/...support/src/main/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBean.java b/...support/src/main/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBean.java
@@ -28,7 +28,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;
 
@@ -46,6 +45,7 @@
  * instead of registering the JobDetail separately.
  *
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.1
  * @see #setName
  * @see #setGroup
@@ -56,9 +56,26 @@
  */
 public class SimpleTriggerFactoryBean implements FactoryBean<SimpleTrigger>, BeanNameAware, InitializingBean {
 
-	/** Constants for the SimpleTrigger class. */
-	private static final Constants constants = new Constants(SimpleTrigger.class);
-
+	/**
+	 * Map of constant names to constant values for the misfire instruction constants
+	 * defined in {@link org.quartz.Trigger} and {@link org.quartz.SimpleTrigger}.
+	 */
+	private static final Map<String, Integer> constants = Map.of(
+			"MISFIRE_INSTRUCTION_SMART_POLICY",
+				SimpleTrigger.MISFIRE_INSTRUCTION_SMART_POLICY,
+			"MISFIRE_INSTRUCTION_IGNORE_MISFIRE_POLICY",
+				SimpleTrigger.MISFIRE_INSTRUCTION_IGNORE_MISFIRE_POLICY,
+			"MISFIRE_INSTRUCTION_FIRE_NOW",
+				SimpleTrigger.MISFIRE_INSTRUCTION_FIRE_NOW,
+			"MISFIRE_INSTRUCTION_RESCHEDULE_NEXT_WITH_EXISTING_COUNT",
+				SimpleTrigger.MISFIRE_INSTRUCTION_RESCHEDULE_NEXT_WITH_EXISTING_COUNT,
+			"MISFIRE_INSTRUCTION_RESCHEDULE_NEXT_WITH_REMAINING_COUNT",
+				SimpleTrigger.MISFIRE_INSTRUCTION_RESCHEDULE_NEXT_WITH_REMAINING_COUNT,
+			"MISFIRE_INSTRUCTION_RESCHEDULE_NOW_WITH_EXISTING_REPEAT_COUNT",
+				SimpleTrigger.MISFIRE_INSTRUCTION_RESCHEDULE_NOW_WITH_EXISTING_REPEAT_COUNT,
+			"MISFIRE_INSTRUCTION_RESCHEDULE_NOW_WITH_REMAINING_REPEAT_COUNT",
+				SimpleTrigger.MISFIRE_INSTRUCTION_RESCHEDULE_NOW_WITH_REMAINING_REPEAT_COUNT
+		);
 
 	@Nullable
 	private String name;
@@ -204,7 +221,10 @@ public void setMisfireInstruction(int misfireInstruction) {
 	 * @see org.quartz.SimpleTrigger#MISFIRE_INSTRUCTION_RESCHEDULE_NOW_WITH_REMAINING_REPEAT_COUNT
 	 */
 	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/...rt/src/test/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBeanTests.java b/...rt/src/test/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBeanTests.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,21 +16,32 @@
 
 package org.springframework.scheduling.quartz;
 
-import java.text.ParseException;
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
 import org.quartz.SimpleTrigger;
 
+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 SimpleTriggerFactoryBean}.
+ *
  * @author Stephane Nicoll
+ * @author Sam Brannen
  */
-public class SimpleTriggerFactoryBeanTests {
+class SimpleTriggerFactoryBeanTests {
+
+	private final SimpleTriggerFactoryBean factory = new SimpleTriggerFactoryBean();
+
 
 	@Test
-	public void createWithoutJobDetail() throws ParseException {
-		SimpleTriggerFactoryBean factory = new SimpleTriggerFactoryBean();
+	void createWithoutJobDetail() {
 		factory.setName("myTrigger");
 		factory.setRepeatCount(5);
 		factory.setRepeatInterval(1000L);
@@ -40,4 +51,28 @@ public void createWithoutJobDetail() throws ParseException {
 		assertThat(trigger.getRepeatInterval()).isEqualTo(1000L);
 	}
 
+	@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 SimpleTrigger}.
+	 */
+	@Test
+	void setMisfireInstructionNameToAllSupportedValues() {
+		streamMisfireInstructionConstants()
+				.map(Field::getName)
+				.forEach(name -> assertThatNoException().as(name).isThrownBy(() -> factory.setMisfireInstructionName(name)));
+	}
+
+	private static Stream<Field> streamMisfireInstructionConstants() {
+		return Arrays.stream(SimpleTrigger.class.getFields())
+				.filter(ReflectionUtils::isPublicStaticFinal)
+				.filter(field -> field.getName().startsWith("MISFIRE_INSTRUCTION_"));
+	}
+
 }