From fc8e38bb33581bba60be1ef1403488051c0686fb Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Wed, 14 Sep 2016 19:05:46 +0200 Subject: [PATCH] SONAR-8081 Http error 400 is sent when trying an incorrect issue transition --- .../sonar/server/issue/workflow/State.java | 64 +++++++++---------- .../server/issue/workflow/StateTest.java | 48 ++++++++------ 2 files changed, 59 insertions(+), 53 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/State.java b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/State.java index 60510a4b77f..bf5deaa141c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/State.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/workflow/State.java @@ -21,10 +21,11 @@ package org.sonar.server.issue.workflow; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.CheckForNull; import org.apache.commons.lang.StringUtils; import org.sonar.api.issue.Issue; @@ -43,47 +44,46 @@ public class State { } private static void checkDuplications(Transition[] transitions, String stateKey) { - Set keys = Sets.newHashSet(); - for (Transition transition : transitions) { - if (keys.contains(transition.key())) { + Set keys = new HashSet<>(); + + Arrays.stream(transitions) + .filter(transition -> !keys.add(transition.key())) + .findAny() + .ifPresent(transition -> { throw new IllegalArgumentException("Transition '" + transition.key() + "' is declared several times from the originating state '" + stateKey + "'"); - } - keys.add(transition.key()); - } + }); } public List outManualTransitions(Issue issue) { - List result = Lists.newArrayList(); - for (Transition transition : outTransitions) { - if (!transition.automatic() && transition.supports(issue)) { - result.add(transition); - } - } - return result; + return Arrays.stream(outTransitions) + .filter(transition -> !transition.automatic()) + .filter(transition -> transition.supports(issue)) + .collect(Collectors.toList()); } @CheckForNull public Transition outAutomaticTransition(Issue issue) { - Transition result = null; - for (Transition transition : outTransitions) { - if (transition.automatic() && transition.supports(issue)) { - if (result == null) { - result = transition; - } else { - throw new IllegalStateException("Several automatic transitions are available for issue: " + issue); - } - } - } - return result; + final Transition[] result = new Transition[1]; + Set keys = new HashSet<>(); + + Arrays.stream(outTransitions) + .filter(Transition::automatic) + .filter(transition -> transition.supports(issue)) + .peek(transition -> result[0] = transition) + .filter(transition -> !keys.add(transition.key())) + .findAny() + .ifPresent(transition -> { + throw new IllegalArgumentException("Several automatic transitions are available for issue: " + issue); + }); + + return result[0]; } Transition transition(String transitionKey) { - for (Transition transition : outTransitions) { - if (transitionKey.equals(transition.key())) { - return transition; - } - } - throw new IllegalStateException("Transition from state " + key + " does not exist: " + transitionKey); + return Arrays.stream(outTransitions) + .filter(transition -> transitionKey.equals(transition.key())) + .findAny() + .orElseThrow(() -> new IllegalArgumentException("Transition from state " + key + " does not exist: " + transitionKey)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/StateTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/StateTest.java index 7677c3e3954..f09b22f323a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/StateTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/workflow/StateTest.java @@ -19,42 +19,48 @@ */ package org.sonar.server.issue.workflow; +import org.junit.Rule; import org.junit.Test; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; +import org.junit.rules.ExpectedException; public class StateTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + Transition t1 = Transition.builder("close").from("OPEN").to("CLOSED").build(); @Test public void key_should_be_set() { - try { - new State("", new Transition[0]); - fail(); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("State key must be set"); - } + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("State key must be set"); + + new State("", new Transition[0]); } @Test public void key_should_be_upper_case() { - try { - new State("close", new Transition[0]); - fail(); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("State key must be upper-case"); - } + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("State key must be upper-case"); + + new State("close", new Transition[0]); } @Test public void no_duplicated_out_transitions() { - try { - new State("CLOSE", new Transition[] {t1, t1}); - fail(); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("Transition 'close' is declared several times from the originating state 'CLOSE'"); - } + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Transition 'close' is declared several times from the originating state 'CLOSE'"); + + new State("CLOSE", new Transition[] {t1, t1}); + } + + @Test + public void fail_when_transition_is_unknown() { + State state = new State("VALIDATED", new Transition[0]); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Transition from state VALIDATED does not exist: Unknown Transition"); + + state.transition("Unknown Transition"); } } -- 2.39.5