Skip to content

Commit d6cbb31

Browse files
RobWinRobWin
authored
Issue ReactiveX#562: State transition must be validated before the transition happens
* Issue ReactiveX#562: State transition must be validated before the transition happens.
1 parent d0cf318 commit d6cbb31

5 files changed

Lines changed: 87 additions & 10 deletions

File tree

‎resilience4j-circuitbreaker/src/main/java/io/github/resilience4j/circuitbreaker/CircuitBreaker.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,11 +470,10 @@ enum StateTransition {
470470
private static final Map<Tuple2<State, State>, StateTransition> STATE_TRANSITION_MAP = Arrays.stream(StateTransition.values())
471471
.collect(Collectors.toMap(v -> Tuple.of(v.fromState, v.toState), Function.identity()));
472472

473-
public static StateTransition transitionBetween(State fromState, State toState){
473+
public static StateTransition transitionBetween(String name, State fromState, State toState){
474474
final StateTransition stateTransition = STATE_TRANSITION_MAP.get(Tuple.of(fromState, toState));
475475
if(stateTransition == null) {
476-
throw new IllegalStateException(
477-
String.format("Illegal state transition from %s to %s", fromState.toString(), toState.toString()));
476+
throw new IllegalStateTransitionException(name, fromState, toState);
478477
}
479478
return stateTransition;
480479
}

‎resilience4j-circuitbreaker/src/main/java/io/github/resilience4j/circuitbreaker/IllegalStateTransitionException.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package io.github.resilience4j.circuitbreaker;
2+
3+
/**
4+
* A {@link IllegalStateTransitionException} signals that someone tried to trigger an illegal state transition..
5+
*/
6+
public class IllegalStateTransitionException extends RuntimeException {
7+
8+
private final String name;
9+
private final CircuitBreaker.State fromState;
10+
private final CircuitBreaker.State toState;
11+
12+
IllegalStateTransitionException(String name, CircuitBreaker.State fromState, CircuitBreaker.State toState) {
13+
super(String.format("CircuitBreaker '%s' tried an illegal state transition from %s to %s", name, fromState.toString(), toState.toString()));
14+
this.name = name;
15+
this.fromState = fromState;
16+
this.toState = toState;
17+
}
18+
19+
public CircuitBreaker.State getFromState() {
20+
return fromState;
21+
}
22+
23+
public CircuitBreaker.State getToState() {
24+
return toState;
25+
}
26+
27+
public String getName() {
28+
return name;
29+
}
30+
}

‎resilience4j-circuitbreaker/src/main/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachine.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,21 +228,17 @@ public String toString() {
228228
public void reset() {
229229
CircuitBreakerState previousState = stateReference.getAndUpdate(currentState -> new ClosedState());
230230
if (previousState.getState() != CLOSED) {
231-
publishStateTransitionEvent(StateTransition.transitionBetween(previousState.getState(), CLOSED));
231+
publishStateTransitionEvent(StateTransition.transitionBetween(getName(), previousState.getState(), CLOSED));
232232
}
233233
publishResetEvent();
234234
}
235235

236236
private void stateTransition(State newState, UnaryOperator<CircuitBreakerState> newStateGenerator) {
237237
CircuitBreakerState previousState = stateReference.getAndUpdate(currentState -> {
238-
if (currentState.getState() == newState) {
239-
return currentState;
240-
}
238+
StateTransition.transitionBetween(getName(), currentState.getState(), newState);
241239
return newStateGenerator.apply(currentState);
242240
});
243-
if (previousState.getState() != newState) {
244-
publishStateTransitionEvent(StateTransition.transitionBetween(previousState.getState(), newState));
245-
}
241+
publishStateTransitionEvent(StateTransition.transitionBetween(getName(), previousState.getState(), newState));
246242
}
247243

248244
@Override

‎resilience4j-circuitbreaker/src/test/java/io/github/resilience4j/circuitbreaker/IllegalStateTransitionExceptionTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.github.resilience4j.circuitbreaker;
2+
3+
import org.junit.Test;
4+
5+
import static org.assertj.core.api.Assertions.assertThat;
6+
7+
public class IllegalStateTransitionExceptionTest {
8+
9+
@Test
10+
public void shouldReturnCorrectMessage(){
11+
IllegalStateTransitionException illegalStateTransitionException = new IllegalStateTransitionException("testName", CircuitBreaker.State.OPEN,
12+
CircuitBreaker.State.CLOSED);
13+
assertThat(illegalStateTransitionException.getMessage()).isEqualTo("CircuitBreaker 'testName' tried an illegal state transition from OPEN to CLOSED");
14+
assertThat(illegalStateTransitionException.getFromState()).isEqualTo(CircuitBreaker.State.OPEN);
15+
assertThat(illegalStateTransitionException.getToState()).isEqualTo(CircuitBreaker.State.CLOSED);
16+
}
17+
}

‎resilience4j-circuitbreaker/src/test/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachineTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.statemachinesystems.mockclock.MockClock;
2222
import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
2323
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
24+
import io.github.resilience4j.circuitbreaker.IllegalStateTransitionException;
2425
import org.junit.Before;
2526
import org.junit.Test;
2627

@@ -420,6 +421,40 @@ public void shouldReleasePermissionWhenExceptionIgnored() {
420421
assertCircuitBreakerMetricsEqualTo(-1f, 0, 0, 0, 0L);
421422
}
422423

424+
@Test
425+
public void shouldNotAllowTransitionFromClosedToHalfOpen() {
426+
assertThatThrownBy(() -> circuitBreaker.transitionToHalfOpenState()).isInstanceOf(IllegalStateTransitionException.class)
427+
.hasMessage("CircuitBreaker 'testName' tried an illegal state transition from CLOSED to HALF_OPEN");
428+
assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.CLOSED);
429+
}
430+
431+
@Test
432+
public void shouldNotAllowTransitionFromClosedToClosed() {
433+
assertThatThrownBy(() -> circuitBreaker.transitionToClosedState()).isInstanceOf(IllegalStateTransitionException.class)
434+
.hasMessage("CircuitBreaker 'testName' tried an illegal state transition from CLOSED to CLOSED");
435+
assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.CLOSED);
436+
}
437+
438+
@Test
439+
public void shouldResetToClosedState() {
440+
circuitBreaker.transitionToOpenState();
441+
circuitBreaker.reset();
442+
assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.CLOSED);
443+
}
444+
445+
@Test
446+
public void shouldResetClosedState() {
447+
circuitBreaker.onSuccess(0, TimeUnit.NANOSECONDS);
448+
circuitBreaker.onSuccess(0, TimeUnit.NANOSECONDS);
449+
assertThat(circuitBreaker.getMetrics().getNumberOfSuccessfulCalls()).isEqualTo(2);
450+
451+
circuitBreaker.reset();
452+
assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.CLOSED);
453+
assertThat(circuitBreaker.getMetrics().getNumberOfSuccessfulCalls()).isEqualTo(0);
454+
}
455+
456+
457+
423458
private void assertCircuitBreakerMetricsEqualTo(Float expectedFailureRate, Integer expectedSuccessCalls, Integer expectedBufferedCalls, Integer expectedFailedCalls, Long expectedNotPermittedCalls) {
424459
final CircuitBreaker.Metrics metrics = circuitBreaker.getMetrics();
425460
assertThat(metrics.getFailureRate()).isEqualTo(expectedFailureRate);

0 commit comments

Comments
 (0)