Skip to content

Commit 9d139b5

Browse files
Robert McNeesfmbenhassine
Robert McNees
authored and
fmbenhassine
committed
Modified calculating order for DefaultStateTransitionComparator
Resolves #3996
1 parent 21555c0 commit 9d139b5

File tree

3 files changed

+141
-34
lines changed

3 files changed

+141
-34
lines changed

‎spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,31 @@
2020
import java.util.Comparator;
2121

2222
/**
23-
* Sorts by ascending specificity of pattern, based on just counting wildcards (with *
24-
* taking precedence over ?). If wildcard counts are equal then falls back to alphabetic
25-
* comparison. Hence * > foo* > ??? > fo? > foo.
23+
* Sorts by ascending specificity of pattern, based on counting wildcards (with * taking
24+
* precedence over ?). Hence * > foo* > ??? > fo? > foo.
25+
*
26+
* For more complex comparisons, any string containing at least one * token will be
27+
* considered more generic than any string that has no * token. If both strings have at
28+
* least one * token, then the string with fewer * tokens will be considered the most
29+
* generic. If both strings have the same number of * tokens, then the comparison will
30+
* fall back to length of the overall string with the shortest value being the most
31+
* generic. Finally, if the * token count is equal and the string length is equal then the
32+
* final comparison will be alphabetic.
33+
*
34+
* When two strings have ? tokens, then the string with the most ? tokens will be
35+
* considered the most generic. If both strings have the same number of ? tokens, then the
36+
* comparison will fall back to length of the overall string with the shortest value being
37+
* the most generic. Finally, if the ? token count is equal and the string length is equal
38+
* then the final comparison will be alphabetic
39+
*
40+
* If the strings contain neither * nor ? tokens then alphabetic comparison will be used.
41+
*
42+
* Hence * > foo* > *f* > *foo* > ??? > ?o? > foo?? > bar?? > fo?
43+
* > foo > bar
2644
*
2745
* @see Comparator
2846
* @author Michael Minella
47+
* @author Robert McNees
2948
* @since 3.0
3049
*/
3150
public class DefaultStateTransitionComparator implements Comparator<StateTransition> {
@@ -34,27 +53,39 @@ public class DefaultStateTransitionComparator implements Comparator<StateTransit
3453

3554
@Override
3655
public int compare(StateTransition arg0, StateTransition arg1) {
37-
String value = arg1.getPattern();
38-
if (arg0.getPattern().equals(value)) {
56+
String arg0Pattern = arg0.getPattern();
57+
String arg1Pattern = arg1.getPattern();
58+
if (arg0.getPattern().equals(arg1Pattern)) {
3959
return 0;
4060
}
41-
int patternCount = StringUtils.countOccurrencesOf(arg0.getPattern(), "*");
42-
int valueCount = StringUtils.countOccurrencesOf(value, "*");
43-
if (patternCount > valueCount) {
61+
int arg0AsteriskCount = StringUtils.countOccurrencesOf(arg0Pattern, "*");
62+
int arg1AsteriskCount = StringUtils.countOccurrencesOf(arg1Pattern, "*");
63+
if (arg0AsteriskCount > 0 && arg1AsteriskCount == 0) {
4464
return 1;
4565
}
46-
if (patternCount < valueCount) {
66+
if (arg0AsteriskCount == 0 && arg1AsteriskCount > 0) {
4767
return -1;
4868
}
49-
patternCount = StringUtils.countOccurrencesOf(arg0.getPattern(), "?");
50-
valueCount = StringUtils.countOccurrencesOf(value, "?");
51-
if (patternCount > valueCount) {
69+
if (arg0AsteriskCount > 0 && arg1AsteriskCount > 0) {
70+
if (arg0AsteriskCount < arg1AsteriskCount) {
71+
return 1;
72+
}
73+
if (arg0AsteriskCount > arg1AsteriskCount) {
74+
return -1;
75+
}
76+
}
77+
int arg0WildcardCount = StringUtils.countOccurrencesOf(arg0Pattern, "?");
78+
int arg1WildcardCount = StringUtils.countOccurrencesOf(arg1Pattern, "?");
79+
if (arg0WildcardCount > arg1WildcardCount) {
5280
return 1;
5381
}
54-
if (patternCount < valueCount) {
82+
if (arg0WildcardCount < arg1WildcardCount) {
5583
return -1;
5684
}
57-
return arg0.getPattern().compareTo(value);
85+
if (arg0Pattern.length() != arg1Pattern.length() && (arg0AsteriskCount > 0 || arg0WildcardCount > 0)) {
86+
return Integer.compare(arg1Pattern.length(), arg0Pattern.length());
87+
}
88+
return arg0.getPattern().compareTo(arg1Pattern);
5889
}
5990

6091
}

‎spring-batch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,26 @@ public FlowExecutionStatus decide(JobExecution jobExecution, @Nullable StepExecu
262262
assertEquals(1, execution.getStepExecutions().size());
263263
}
264264

265+
@Test
266+
void testBuildWithDeciderPriorityOnWildcardCount() {
267+
JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY");
268+
JobFlowBuilder builder = new JobBuilder("flow_priority", jobRepository).start(decider);
269+
builder.on("**").end();
270+
builder.on("*").fail();
271+
builder.build().preventRestart().build().execute(execution);
272+
assertEquals(BatchStatus.COMPLETED, execution.getStatus());
273+
}
274+
275+
@Test
276+
void testBuildWithDeciderPriorityWithEqualWildcard() {
277+
JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY");
278+
JobFlowBuilder builder = new JobBuilder("flow_priority", jobRepository).start(decider);
279+
builder.on("COMPLETED*").end();
280+
builder.on("*").fail();
281+
builder.build().preventRestart().build().execute(execution);
282+
assertEquals(BatchStatus.COMPLETED, execution.getStatus());
283+
}
284+
265285
@Test
266286
void testBuildWithDeciderPriority() {
267287
JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY");

‎spring-batch-core/src/test/java/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,42 +37,98 @@ void testSimpleOrderingEqual() {
3737

3838
@Test
3939
void testSimpleOrderingMoreGeneral() {
40-
StateTransition transition = StateTransition.createStateTransition(state, "CONTIN???LE", "start");
41-
StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
42-
assertEquals(1, comparator.compare(transition, other));
43-
assertEquals(-1, comparator.compare(other, transition));
40+
StateTransition generic = StateTransition.createStateTransition(state, "CONTIN???LE", "start");
41+
StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
42+
assertEquals(1, comparator.compare(generic, specific));
43+
assertEquals(-1, comparator.compare(specific, generic));
4444
}
4545

4646
@Test
4747
void testSimpleOrderingMostGeneral() {
48-
StateTransition transition = StateTransition.createStateTransition(state, "*", "start");
49-
StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
50-
assertEquals(1, comparator.compare(transition, other));
51-
assertEquals(-1, comparator.compare(other, transition));
48+
StateTransition generic = StateTransition.createStateTransition(state, "*", "start");
49+
StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
50+
assertEquals(1, comparator.compare(generic, specific));
51+
assertEquals(-1, comparator.compare(specific, generic));
5252
}
5353

5454
@Test
5555
void testSubstringAndWildcard() {
56-
StateTransition transition = StateTransition.createStateTransition(state, "CONTIN*", "start");
57-
StateTransition other = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
58-
assertEquals(1, comparator.compare(transition, other));
59-
assertEquals(-1, comparator.compare(other, transition));
56+
StateTransition generic = StateTransition.createStateTransition(state, "CONTIN*", "start");
57+
StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE", "start");
58+
assertEquals(1, comparator.compare(generic, specific));
59+
assertEquals(-1, comparator.compare(specific, generic));
6060
}
6161

6262
@Test
6363
void testSimpleOrderingMostToNextGeneral() {
64-
StateTransition transition = StateTransition.createStateTransition(state, "*", "start");
65-
StateTransition other = StateTransition.createStateTransition(state, "C?", "start");
66-
assertEquals(1, comparator.compare(transition, other));
67-
assertEquals(-1, comparator.compare(other, transition));
64+
StateTransition generic = StateTransition.createStateTransition(state, "*", "start");
65+
StateTransition specific = StateTransition.createStateTransition(state, "C?", "start");
66+
assertEquals(1, comparator.compare(generic, specific));
67+
assertEquals(-1, comparator.compare(specific, generic));
6868
}
6969

7070
@Test
7171
void testSimpleOrderingAdjacent() {
72-
StateTransition transition = StateTransition.createStateTransition(state, "CON*", "start");
73-
StateTransition other = StateTransition.createStateTransition(state, "CON?", "start");
74-
assertEquals(1, comparator.compare(transition, other));
75-
assertEquals(-1, comparator.compare(other, transition));
72+
StateTransition generic = StateTransition.createStateTransition(state, "CON*", "start");
73+
StateTransition specific = StateTransition.createStateTransition(state, "CON?", "start");
74+
assertEquals(1, comparator.compare(generic, specific));
75+
assertEquals(-1, comparator.compare(specific, generic));
76+
}
77+
78+
@Test
79+
void testOrderByNumberOfGenericWildcards() {
80+
StateTransition generic = StateTransition.createStateTransition(state, "*", "start");
81+
StateTransition specific = StateTransition.createStateTransition(state, "**", "start");
82+
assertEquals(1, comparator.compare(generic, specific));
83+
assertEquals(-1, comparator.compare(specific, generic));
84+
}
85+
86+
@Test
87+
void testOrderByNumberOfSpecificWildcards() {
88+
StateTransition generic = StateTransition.createStateTransition(state, "CONTI??ABLE", "start");
89+
StateTransition specific = StateTransition.createStateTransition(state, "CONTI?UABLE", "start");
90+
assertEquals(1, comparator.compare(generic, specific));
91+
assertEquals(-1, comparator.compare(specific, generic));
92+
}
93+
94+
@Test
95+
void testOrderByLengthWithAsteriskEquality() {
96+
StateTransition generic = StateTransition.createStateTransition(state, "CON*", "start");
97+
StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE*", "start");
98+
assertEquals(1, comparator.compare(generic, specific));
99+
assertEquals(-1, comparator.compare(specific, generic));
100+
}
101+
102+
@Test
103+
void testOrderByLengthWithWildcardEquality() {
104+
StateTransition generic = StateTransition.createStateTransition(state, "CON??", "start");
105+
StateTransition specific = StateTransition.createStateTransition(state, "CONTINUABLE??", "start");
106+
assertEquals(1, comparator.compare(generic, specific));
107+
assertEquals(-1, comparator.compare(specific, generic));
108+
}
109+
110+
@Test
111+
void testOrderByAlphaWithAsteriskEquality() {
112+
StateTransition generic = StateTransition.createStateTransition(state, "DOG**", "start");
113+
StateTransition specific = StateTransition.createStateTransition(state, "CAT**", "start");
114+
assertEquals(1, comparator.compare(generic, specific));
115+
assertEquals(-1, comparator.compare(specific, generic));
116+
}
117+
118+
@Test
119+
void testOrderByAlphaWithWildcardEquality() {
120+
StateTransition generic = StateTransition.createStateTransition(state, "DOG??", "start");
121+
StateTransition specific = StateTransition.createStateTransition(state, "CAT??", "start");
122+
assertEquals(1, comparator.compare(generic, specific));
123+
assertEquals(-1, comparator.compare(specific, generic));
124+
}
125+
126+
@Test
127+
void testPriorityOrderingWithAlphabeticComparison() {
128+
StateTransition generic = StateTransition.createStateTransition(state, "DOG", "start");
129+
StateTransition specific = StateTransition.createStateTransition(state, "CAT", "start");
130+
assertEquals(1, comparator.compare(generic, specific));
131+
assertEquals(-1, comparator.compare(specific, generic));
76132
}
77133

78134
}

0 commit comments

Comments
 (0)