SONAR-8081 Http error 400 is sent when trying an incorrect issue transition

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 60510a4b77f9d378869af3be8ff9b8f5d39f2177..bf5deaa141c3e5f77975484898bcc5a41fbdd83b 100644 (file)
--- 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<String> keys = Sets.newHashSet();
-    for (Transition transition : transitions) {
-      if (keys.contains(transition.key())) {
+    Set<String> 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<Transition> outManualTransitions(Issue issue) {
-    List<Transition> 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<String> 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 7677c3e39544d6aef56cf2f91885cc96224bdcb8..f09b22f323acfa11bfd1fa233d672322257088cd 100644 (file)
--- 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");
   }
 }