Skip to content

Commit

Permalink
Change DefaultStateTransitionComparator ordering to match logical flo…
Browse files Browse the repository at this point in the history 
…w execution

Resolves #4527
  • Loading branch information
@fmbenhassine
Robert McNees authored and fmbenhassine committed Mar 14, 2024
1 parent a4a8ab7 commit d779cad
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 40 deletions.
28 changes: 15 additions & 13 deletions ...ava/org/springframework/batch/core/job/flow/support/DefaultStateTransitionComparator.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 the original author or authors.
* Copyright 2013-2024 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.
Expand All @@ -20,8 +20,10 @@
import java.util.Comparator;

/**
* Sorts by ascending specificity of pattern, based on counting wildcards (with * taking
* precedence over ?). Hence * > foo* > ??? > fo? > foo.
* Sorts by descending specificity of pattern, based on counting wildcards (with ? being
* considered more specific than *). This means that more specific patterns will be
* considered greater than less specific patterns. 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
Expand All @@ -39,8 +41,8 @@
*
* If the strings contain neither * nor ? tokens then alphabetic comparison will be used.
*
* Hence * > foo* > *f* > *foo* > ??? > ?o? > foo?? > bar?? > fo?
* > foo > bar
* Hence bar > foo > fo? > bar?? > foo?? > ?0? > ??? > *foo* > *f*
* > foo* > *
*
* @see Comparator
* @author Michael Minella
Expand All @@ -61,31 +63,31 @@ public int compare(StateTransition arg0, StateTransition arg1) {
int arg0AsteriskCount = StringUtils.countOccurrencesOf(arg0Pattern, "*");
int arg1AsteriskCount = StringUtils.countOccurrencesOf(arg1Pattern, "*");
if (arg0AsteriskCount > 0 && arg1AsteriskCount == 0) {
return 1;
return -1;
}
if (arg0AsteriskCount == 0 && arg1AsteriskCount > 0) {
return -1;
return 1;
}
if (arg0AsteriskCount > 0 && arg1AsteriskCount > 0) {
if (arg0AsteriskCount < arg1AsteriskCount) {
return 1;
return -1;
}
if (arg0AsteriskCount > arg1AsteriskCount) {
return -1;
return 1;
}
}
int arg0WildcardCount = StringUtils.countOccurrencesOf(arg0Pattern, "?");
int arg1WildcardCount = StringUtils.countOccurrencesOf(arg1Pattern, "?");
if (arg0WildcardCount > arg1WildcardCount) {
return 1;
return -1;
}
if (arg0WildcardCount < arg1WildcardCount) {
return -1;
return 1;
}
if (arg0Pattern.length() != arg1Pattern.length() && (arg0AsteriskCount > 0 || arg0WildcardCount > 0)) {
return Integer.compare(arg1Pattern.length(), arg0Pattern.length());
return Integer.compare(arg0Pattern.length(), arg1Pattern.length());
}
return arg0.getPattern().compareTo(arg1Pattern);
return arg1.getPattern().compareTo(arg0Pattern);
}

}
4 changes: 2 additions & 2 deletions ...-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/SimpleFlow.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2006-2023 the original author or authors.
* Copyright 2006-2024 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.
Expand Down Expand Up @@ -313,7 +313,7 @@ private void initializeTransitions() {
set = new LinkedHashSet<>();
}
else {
set = new TreeSet<>(stateTransitionComparator);
set = new TreeSet<>(stateTransitionComparator).descendingSet();
}

transitionMap.put(name, set);
Expand Down
50 changes: 25 additions & 25 deletions ...rg/springframework/batch/core/job/flow/support/DefaultStateTransitionComparatorTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2024 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.
Expand Down Expand Up @@ -39,96 +39,96 @@ void testSimpleOrderingEqual() {
void testSimpleOrderingMoreGeneral() {
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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@Test
void testSimpleOrderingMostGeneral() {
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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@Test
void testSubstringAndWildcard() {
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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@Test
void testSimpleOrderingMostToNextGeneral() {
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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@Test
void testSimpleOrderingAdjacent() {
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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

@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));
assertEquals(1, comparator.compare(specific, generic));
assertEquals(-1, comparator.compare(generic, specific));
}

}

0 comments on commit d779cad

Please sign in to comment.