diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java index 1d43ccc1ac..39d3e54fd4 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java @@ -20,12 +20,31 @@ import java.util.Comparator; /** - * Sorts by decreasing specificity of pattern, based on just counting wildcards (with * - * taking precedence over ?). If wildcard counts are equal then falls back to alphabetic - * comparison. Hence * > foo* > ??? > fo? > foo. + * Sorts by ascending specificity of pattern, based on counting wildcards (with * taking + * precedence over ?). Hence * > foo* > ??? > fo? > foo. + * + * For more complex comparisons, any string containing at least one * token will be + * considered more generic than any string that has no * token. If both strings have at + * least one * token, then the string with fewer * tokens will be considered the most + * generic. If both strings have the same number of * tokens, then the comparison will + * fall back to length of the overall string with the shortest value being the most + * generic. Finally, if the * token count is equal and the string length is equal then the + * final comparison will be alphabetic. + * + * When two strings have ? tokens, then the string with the most ? tokens will be + * considered the most generic. If both strings have the same number of ? tokens, then the + * comparison will fall back to length of the overall string with the shortest value being + * the most generic. Finally, if the ? token count is equal and the string length is equal + * then the final comparison will be alphabetic + * + * If the strings contain neither * nor ? tokens then alphabetic comparison will be used. + * + * Hence * > foo* > *f* > *foo* > ??? > ?o? > foo?? > bar?? > fo? + * > foo > bar * * @see Comparator * @author Michael Minella + * @author Robert McNees * @since 3.0 */ public class DefaultStateTransitionComparator implements Comparator { @@ -39,27 +58,39 @@ public class DefaultStateTransitionComparator implements Comparator valueCount) { + int arg0AsteriskCount = StringUtils.countOccurrencesOf(arg0Pattern, "*"); + int arg1AsteriskCount = StringUtils.countOccurrencesOf(arg1Pattern, "*"); + if (arg0AsteriskCount > 0 && arg1AsteriskCount == 0) { return 1; } - if (patternCount < valueCount) { + if (arg0AsteriskCount == 0 && arg1AsteriskCount > 0) { return -1; } - patternCount = StringUtils.countOccurrencesOf(arg0.getPattern(), "?"); - valueCount = StringUtils.countOccurrencesOf(value, "?"); - if (patternCount > valueCount) { + if (arg0AsteriskCount > 0 && arg1AsteriskCount > 0) { + if (arg0AsteriskCount < arg1AsteriskCount) { + return 1; + } + if (arg0AsteriskCount > arg1AsteriskCount) { + return -1; + } + } + int arg0WildcardCount = StringUtils.countOccurrencesOf(arg0Pattern, "?"); + int arg1WildcardCount = StringUtils.countOccurrencesOf(arg1Pattern, "?"); + if (arg0WildcardCount > arg1WildcardCount) { return 1; } - if (patternCount < valueCount) { + if (arg0WildcardCount < arg1WildcardCount) { return -1; } - return arg0.getPattern().compareTo(value); + if (arg0Pattern.length() != arg1Pattern.length() && (arg0AsteriskCount > 0 || arg0WildcardCount > 0)) { + return Integer.compare(arg1Pattern.length(), arg0Pattern.length()); + } + return arg0.getPattern().compareTo(arg1Pattern); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java index 14e61a58a0..557152c0be 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java @@ -262,6 +262,26 @@ public FlowExecutionStatus decide(JobExecution jobExecution, @Nullable StepExecu assertEquals(1, execution.getStepExecutions().size()); } + @Test + void testBuildWithDeciderPriorityOnWildcardCount() { + JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY"); + JobFlowBuilder builder = new JobBuilder("flow_priority", jobRepository).start(decider); + builder.on("**").end(); + builder.on("*").fail(); + builder.build().preventRestart().build().execute(execution); + assertEquals(BatchStatus.COMPLETED, execution.getStatus()); + } + + @Test + void testBuildWithDeciderPriority() { + JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY"); + JobFlowBuilder builder = new JobBuilder("flow_priority", jobRepository).start(decider); + builder.on("COMPLETED*").end(); + builder.on("*").fail(); + builder.build().preventRestart().build().execute(execution); + assertEquals(BatchStatus.COMPLETED, execution.getStatus()); + } + @Test void testBuildWithIntermediateSimpleJob() { SimpleJobBuilder builder = new JobBuilder("flow", jobRepository).start(step1); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java index 3bb7759a38..d71e4813b2 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java @@ -37,42 +37,98 @@ void testSimpleOrderingEqual() { @Test void testSimpleOrderingMoreGeneral() { - StateTransition transition = StateTransition.createStateTransition(state, "CONTIN???LE", "start"); - StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); - assertEquals(1, comparator.compare(transition, other)); - assertEquals(-1, comparator.compare(other, transition)); + StateTransition generic = StateTransition.createStateTransition(state, "CONTIN???LE", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); } @Test void testSimpleOrderingMostGeneral() { - StateTransition transition = StateTransition.createStateTransition(state, "*", "start"); - StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); - assertEquals(1, comparator.compare(transition, other)); - assertEquals(-1, comparator.compare(other, transition)); + StateTransition generic = StateTransition.createStateTransition(state, "*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); } @Test void testSubstringAndWildcard() { - StateTransition transition = StateTransition.createStateTransition(state, "CONTIN*", "start"); - StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); - assertEquals(1, comparator.compare(transition, other)); - assertEquals(-1, comparator.compare(other, transition)); + StateTransition generic = StateTransition.createStateTransition(state, "CONTIN*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); } @Test void testSimpleOrderingMostToNextGeneral() { - StateTransition transition = StateTransition.createStateTransition(state, "*", "start"); - StateTransition other = StateTransition.createStateTransition(state, "C?", "start"); - assertEquals(1, comparator.compare(transition, other)); - assertEquals(-1, comparator.compare(other, transition)); + StateTransition generic = StateTransition.createStateTransition(state, "*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "C?", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); } @Test void testSimpleOrderingAdjacent() { - StateTransition transition = StateTransition.createStateTransition(state, "CON*", "start"); - StateTransition other = StateTransition.createStateTransition(state, "CON?", "start"); - assertEquals(1, comparator.compare(transition, other)); - assertEquals(-1, comparator.compare(other, transition)); + StateTransition generic = StateTransition.createStateTransition(state, "CON*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CON?", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByNumberOfGenericWildcards() { + StateTransition generic = StateTransition.createStateTransition(state, "*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "**", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByNumberOfSpecificWildcards() { + StateTransition generic = StateTransition.createStateTransition(state, "CONTI??ABLE", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTI?UABLE", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByLengthWithAsteriskEquality() { + StateTransition generic = StateTransition.createStateTransition(state, "CON*", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE*", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByLengthWithWildcardEquality() { + StateTransition generic = StateTransition.createStateTransition(state, "CON??", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE??", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByAlphaWithAsteriskEquality() { + StateTransition generic = StateTransition.createStateTransition(state, "DOG**", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CAT**", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testOrderByAlphaWithWildcardEquality() { + StateTransition generic = StateTransition.createStateTransition(state, "DOG??", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CAT??", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); + } + + @Test + void testPriorityOrderingWithAlphabeticComparison() { + StateTransition generic = StateTransition.createStateTransition(state, "DOG", "start"); + StateTransition specific = StateTransition.createStateTransition(state, "CAT", "start"); + assertEquals(1, comparator.compare(generic, specific)); + assertEquals(-1, comparator.compare(specific, generic)); } }