Skip to content

Commit

Permalink
Modified calculating order for DefaultStateTransitionComparator
Browse files Browse the repository at this point in the history 
Resolves #3996
  • Loading branch information
@fmbenhassine
Robert McNees authored and fmbenhassine committed Jan 15, 2024
1 parent 21555c0 commit 9d139b5
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 34 deletions.
59 changes: 45 additions & 14 deletions ...ava/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,31 @@
import java.util.Comparator;

/**
* Sorts by ascending 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<StateTransition> {
Expand All @@ -34,27 +53,39 @@ public class DefaultStateTransitionComparator implements Comparator<StateTransit

@Override
public int compare(StateTransition arg0, StateTransition arg1) {
String value = arg1.getPattern();
if (arg0.getPattern().equals(value)) {
String arg0Pattern = arg0.getPattern();
String arg1Pattern = arg1.getPattern();
if (arg0.getPattern().equals(arg1Pattern)) {
return 0;
}
int patternCount = StringUtils.countOccurrencesOf(arg0.getPattern(), "*");
int valueCount = StringUtils.countOccurrencesOf(value, "*");
if (patternCount > 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);
}

}
20 changes: 20 additions & 0 deletions ...ch-core/src/test/java/org/springframework/batch/core/job/builder/FlowJobBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 testBuildWithDeciderPriorityWithEqualWildcard() {
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 testBuildWithDeciderPriority() {
JobExecutionDecider decider = (jobExecution, stepExecution) -> new FlowExecutionStatus("COMPLETED_PARTIALLY");
Expand Down
96 changes: 76 additions & 20 deletions ...rg/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

}

0 comments on commit 9d139b5

Please sign in to comment.