Fix quality flaws

5 files changed, 130 insertions, 53 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/ProjectConfigurator.java b/sonar-batch/src/main/java/org/sonar/batch/ProjectConfigurator.java
index b12616672b7..b7c83521e58 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/ProjectConfigurator.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/ProjectConfigurator.java
@@ -84,7 +84,8 @@ public class ProjectConfigurator implements BatchComponent {
       Snapshot lastSnapshot = databaseSession.getSingleResult(Snapshot.class, "resourceId", persistedProject.getId(), "last", true);
       if (lastSnapshot != null && !lastSnapshot.getCreatedAt().before(analysisDate)) {
         throw new IllegalArgumentException(
-          "'sonar.projectDate' property cannot be older than the date of the last known quality snapshot on this project. Value: '"+ settings.getString(CoreProperties.PROJECT_DATE_PROPERTY) + "'. " +
+          "'sonar.projectDate' property cannot be older than the date of the last known quality snapshot on this project. Value: '"+
+            settings.getString(CoreProperties.PROJECT_DATE_PROPERTY) + "'. " +
             "Latest quality snapshot: '"+ DateUtils.formatDate(lastSnapshot.getCreatedAt()) +"'. This property may only be used to rebuild the past in a chronological order."
         );
       }
diff --git a/sonar-server/src/main/java/org/sonar/server/issue/Action.java b/sonar-server/src/main/java/org/sonar/server/issue/Action.java
index b110cdab2a2..07f5907b1a1 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/Action.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/Action.java
@@ -74,31 +74,6 @@ public abstract class Action implements ServerComponent {
 
   abstract boolean execute(Map<String, Object> properties, Context context);
 
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) {
-      return true;
-    }
-    if (o == null || getClass() != o.getClass()) {
-      return false;
-    }
-    Action that = (Action) o;
-    if (!key.equals(that.key)) {
-      return false;
-    }
-    return true;
-  }
-
-  @Override
-  public int hashCode() {
-    return key.hashCode();
-  }
-
-  @Override
-  public String toString() {
-    return key;
-  }
-
   interface Context {
     Issue issue();
 
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ActionTest.java
new file mode 100644
index 00000000000..57f955cbf4a
--- /dev/null
+++ b/sonar-server/src/test/java/org/sonar/server/issue/ActionTest.java
@@ -0,0 +1,70 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2013 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * SonarQube is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * SonarQube is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+package org.sonar.server.issue;
+
+import org.junit.Test;
+import org.sonar.api.issue.Issue;
+import org.sonar.server.user.UserSession;
+
+import java.util.List;
+import java.util.Map;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+
+public class ActionTest {
+
+  @Test
+  public void key_should_not_be_empty() {
+    try {
+      new Action("") {
+        @Override
+        boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
+          return false;
+        }
+        @Override
+        boolean execute(Map<String, Object> properties, Context context) {
+          return false;
+        }
+      };
+    } catch (Exception e) {
+      assertThat(e).hasMessage("Action key must be set").isInstanceOf(IllegalArgumentException.class);
+    }
+  }
+
+  @Test
+  public void key_should_not_be_null() {
+    try {
+      new Action(null) {
+        @Override
+        boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
+          return false;
+        }
+        @Override
+        boolean execute(Map<String, Object> properties, Context context) {
+          return false;
+        }
+      };
+    } catch (Exception e) {
+      assertThat(e).hasMessage("Action key must be set").isInstanceOf(IllegalArgumentException.class);
+    }
+  }
+}
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
index 09bc9e1fd26..bf81543a54b 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
@@ -26,6 +26,7 @@ import org.mockito.ArgumentCaptor;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.IssueQuery;
 import org.sonar.api.issue.IssueQueryResult;
+import org.sonar.api.issue.condition.Condition;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.IssueChangeContext;
 import org.sonar.api.web.UserRole;
@@ -59,7 +60,6 @@ public class IssueBulkChangeServiceTest {
 
   private IssueBulkChangeService service;
 
-  private Action action = mock(Action.class);
   private List<Action> actions;
 
   @Before
@@ -68,8 +68,6 @@ public class IssueBulkChangeServiceTest {
     when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) issue));
 
     actions = newArrayList();
-    when(action.key()).thenReturn("assign");
-    actions.add(action);
 
     service = new IssueBulkChangeService(finder, issueStorage, issueNotifications, actions);
   }
@@ -80,10 +78,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
-
-    when(action.supports(any(Issue.class))).thenReturn(true);
-    when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
-    when(action.execute(eq(properties), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
+    actions.add(new MockAction("assign"));
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
@@ -104,17 +99,8 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     properties.put("set_severity.severity", "MINOR");
 
-    Action setSeverityAction = mock(Action.class);
-    when(setSeverityAction.key()).thenReturn("set_severity");
-    actions.add(setSeverityAction);
-
-    when(action.supports(any(Issue.class))).thenReturn(true);
-    when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
-    when(action.execute(eq(properties), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
-
-    when(setSeverityAction.supports(any(Issue.class))).thenReturn(true);
-    when(setSeverityAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
-    when(setSeverityAction.execute(eq(properties), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
+    actions.add(new MockAction("set_severity"));
+    actions.add(new MockAction("assign"));
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
@@ -133,10 +119,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD,DEFG");
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
-
-    when(action.supports(any(Issue.class))).thenReturn(true);
-    when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
-    when(action.execute(eq(properties), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
+    actions.add(new MockAction("assign"));
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     service.execute(issueBulkChangeQuery, userSession);
@@ -155,8 +138,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
-
-    when(action.supports(any(Issue.class))).thenReturn(false);
+    actions.add(new MockAction("assign", true, true, false));
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
@@ -173,9 +155,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
-
-    when(action.supports(any(Issue.class))).thenReturn(true);
-    when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(false);
+    actions.add(new MockAction("assign", true, false, true));
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
@@ -193,6 +173,9 @@ public class IssueBulkChangeServiceTest {
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
 
+    Action action = mock(Action.class);
+    actions.add(action);
+    when(action.key()).thenReturn("assign");
     when(action.supports(any(Issue.class))).thenReturn(true);
     doThrow(new RuntimeException("Error")).when(action).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
 
@@ -241,4 +224,36 @@ public class IssueBulkChangeServiceTest {
     verifyZeroInteractions(issueNotifications);
   }
 
+
+  class MockAction extends Action {
+
+    private boolean verify;
+    private boolean execute;
+
+    public MockAction(String key, boolean verify, boolean execute, final boolean support) {
+      super(key);
+      this.verify = verify;
+      this.execute = execute;
+      setConditions(new Condition() {
+        @Override
+        public boolean matches(Issue issue) {
+          return support;
+        }
+      });
+    }
+
+    public MockAction(String key) {
+      this(key, true, true, true);
+    }
+
+    @Override
+    boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
+      return verify;
+    }
+
+    @Override
+    boolean execute(Map<String, Object> properties, Context context) {
+      return execute;
+    }
+  }
 }
diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/BulkChangeQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/BulkChangeQueryTest.java
index b764942da60..ddef62bafe8 100644
--- a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/BulkChangeQueryTest.java
+++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/BulkChangeQueryTest.java
@@ -50,4 +50,20 @@ public class BulkChangeQueryTest {
     );
   }
 
+  @Test
+  public void should_not_add_null_issues() {
+    BulkChangeQuery query = BulkChangeQuery.create()
+      .issues(null);
+
+    assertThat(query.urlParams()).isEmpty();
+  }
+
+  @Test
+  public void should_not_add_null_actions() {
+    BulkChangeQuery query = BulkChangeQuery.create()
+      .actions(null);
+
+    assertThat(query.urlParams()).isEmpty();
+  }
+
 }