From d6230824cb560456ee4120acb3e9cd4f0ded3ffb Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sun, 16 Jul 2023 14:59:03 +0200 Subject: [PATCH] Stop using Constants utility in SimpleTriggerFactoryBean See gh-30851 --- .../quartz/SimpleTriggerFactoryBean.java | 30 ++++++++++--- .../quartz/SimpleTriggerFactoryBeanTests.java | 45 ++++++++++++++++--- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBean.java b/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBean.java index bab87a5a29a0..7bd1f6c7e10c 100644 --- a/spring-context-support/src/main/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBean.java +++ b/spring-context-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, 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 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/spring-context-support/src/test/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBeanTests.java b/spring-context-support/src/test/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBeanTests.java index 6db0bcc82d2d..e7dd464dcd2e 100644 --- a/spring-context-support/src/test/java/org/springframework/scheduling/quartz/SimpleTriggerFactoryBeanTests.java +++ b/spring-context-support/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 streamMisfireInstructionConstants() { + return Arrays.stream(SimpleTrigger.class.getFields()) + .filter(ReflectionUtils::isPublicStaticFinal) + .filter(field -> field.getName().startsWith("MISFIRE_INSTRUCTION_")); + } + }