Remove useless code in PersistIssuesStep and add unit tests

8 files changed, 334 insertions, 21 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistIssuesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistIssuesStep.java
index 569341eda11..139280a3b9a 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistIssuesStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistIssuesStep.java
@@ -26,12 +26,7 @@ import org.sonar.api.issue.internal.DefaultIssueComment;
 import org.sonar.api.issue.internal.FieldDiffs;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.utils.System2;
-import org.sonar.core.issue.db.IssueChangeDto;
-import org.sonar.core.issue.db.IssueChangeMapper;
-import org.sonar.core.issue.db.IssueDto;
-import org.sonar.core.issue.db.IssueMapper;
-import org.sonar.core.issue.db.UpdateConflictResolver;
-import org.sonar.core.persistence.BatchSession;
+import org.sonar.core.issue.db.*;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.MyBatis;
 import org.sonar.server.computation.ComputationContext;
@@ -67,7 +62,6 @@ public class PersistIssuesStep implements ComputationStep {
     DbSession session = dbClient.openSession(true);
     IssueMapper mapper = session.getMapper(IssueMapper.class);
     IssueChangeMapper changeMapper = session.getMapper(IssueChangeMapper.class);
-    int count = 0;
 
     CloseableIterator<DefaultIssue> issues = issueCache.traverse();
     try {
@@ -77,17 +71,14 @@ public class PersistIssuesStep implements ComputationStep {
         if (issue.isNew()) {
           Integer ruleId = ruleCache.get(issue.ruleKey()).getId();
           mapper.insert(IssueDto.toDtoForComputationInsert(issue, ruleId, system2.now()));
-          count++;
           saved = true;
         } else if (issue.isChanged()) {
           IssueDto dto = IssueDto.toDtoForUpdate(issue, system2.now());
           if (Issue.STATUS_CLOSED.equals(issue.status()) || issue.selectedAt() == null) {
             // Issue is closed by scan or changed by end-user
             mapper.update(dto);
-            count++;
           } else {
             int updateCount = mapper.updateIfBeforeSelectedDate(dto);
-            count++;
             if (updateCount == 0) {
               // End-user and scan changed the issue at the same time.
               // See https://jira.codehaus.org/browse/SONAR-4309
@@ -97,12 +88,7 @@ public class PersistIssuesStep implements ComputationStep {
           saved = true;
         }
         if (saved) {
-          count += insertChanges(changeMapper, issue);
-          if (count > BatchSession.MAX_BATCH_SIZE) {
-            session.flushStatements();
-            session.commit();
-            count = 0;
-          }
+          insertChanges(changeMapper, issue);
         }
       }
       session.flushStatements();
@@ -113,23 +99,19 @@ public class PersistIssuesStep implements ComputationStep {
     }
   }
 
-  private int insertChanges(IssueChangeMapper mapper, DefaultIssue issue) {
-    int count = 0;
+  private void insertChanges(IssueChangeMapper mapper, DefaultIssue issue) {
     for (IssueComment comment : issue.comments()) {
       DefaultIssueComment c = (DefaultIssueComment) comment;
       if (c.isNew()) {
         IssueChangeDto changeDto = IssueChangeDto.of(c);
         mapper.insert(changeDto);
-        count++;
       }
     }
     FieldDiffs diffs = issue.currentChange();
     if (!issue.isNew() && diffs != null) {
       IssueChangeDto changeDto = IssueChangeDto.of(issue.key(), diffs);
       mapper.insert(changeDto);
-      count++;
     }
-    return count;
   }
 
   @Override
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistIssuesStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistIssuesStepTest.java
new file mode 100644
index 00000000000..717f8a61eda
--- /dev/null
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistIssuesStepTest.java
@@ -0,0 +1,181 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 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.computation.step;
+
+import org.junit.*;
+import org.junit.rules.TemporaryFolder;
+import org.sonar.api.issue.Issue;
+import org.sonar.api.issue.internal.DefaultIssue;
+import org.sonar.api.issue.internal.DefaultIssueComment;
+import org.sonar.api.issue.internal.FieldDiffs;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.api.rule.Severity;
+import org.sonar.api.utils.System2;
+import org.sonar.core.issue.db.UpdateConflictResolver;
+import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.DbTester;
+import org.sonar.server.computation.issue.IssueCache;
+import org.sonar.server.computation.issue.RuleCache;
+import org.sonar.server.computation.issue.RuleCacheLoader;
+import org.sonar.server.db.DbClient;
+import org.sonar.server.issue.db.IssueDao;
+import org.sonar.server.rule.db.RuleDao;
+
+import java.io.IOException;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class PersistIssuesStepTest extends BaseStepTest {
+
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+
+  @ClassRule
+  public static DbTester dbTester = new DbTester();
+
+  DbSession session;
+
+  DbClient dbClient;
+
+  System2 system2;
+
+  IssueCache issueCache;
+
+  ComputationStep step;
+
+  @Override
+  protected ComputationStep step() throws IOException {
+    return step;
+  }
+
+  @Before
+  public void setup() throws Exception {
+    dbTester.truncateTables();
+    session = dbTester.myBatis().openSession(false);
+    dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new IssueDao(dbTester.myBatis()), new RuleDao());
+
+    issueCache = new IssueCache(temp.newFile(), System2.INSTANCE);
+    system2 = mock(System2.class);
+    when(system2.now()).thenReturn(1400000000000L);
+    step = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleCache(new RuleCacheLoader(dbClient)), issueCache);
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    session.close();
+  }
+
+  @Test
+  public void insert_new_issue() throws Exception {
+    dbTester.prepareDbUnit(getClass(), "insert_new_issue.xml");
+
+    issueCache.newAppender().append(new DefaultIssue()
+        .setKey("ISSUE")
+        .setRuleKey(RuleKey.of("xoo", "S01"))
+        .setComponentUuid("COMPONENT")
+        .setProjectUuid("PROJECT")
+        .setSeverity(Severity.BLOCKER)
+        .setStatus(Issue.STATUS_OPEN)
+        .setNew(true)
+    ).close();
+
+    step.execute(null);
+
+    dbTester.assertDbUnit(getClass(), "insert_new_issue-result.xml", new String[]{"id"}, "issues");
+  }
+
+  @Test
+  public void close_issue() throws Exception {
+    dbTester.prepareDbUnit(getClass(), "shared.xml");
+
+    issueCache.newAppender().append(new DefaultIssue()
+        .setKey("ISSUE")
+        .setRuleKey(RuleKey.of("xoo", "S01"))
+        .setComponentUuid("COMPONENT")
+        .setProjectUuid("PROJECT")
+        .setSeverity(Severity.BLOCKER)
+        .setStatus(Issue.STATUS_CLOSED)
+        .setResolution(Issue.RESOLUTION_FIXED)
+        .setNew(false)
+        .setChanged(true)
+    ).close();
+
+    step.execute(null);
+
+    dbTester.assertDbUnit(getClass(), "close_issue-result.xml", "issues");
+  }
+
+  @Test
+  public void add_comment() throws Exception {
+    dbTester.prepareDbUnit(getClass(), "shared.xml");
+
+    issueCache.newAppender().append(new DefaultIssue()
+        .setKey("ISSUE")
+        .setRuleKey(RuleKey.of("xoo", "S01"))
+        .setComponentUuid("COMPONENT")
+        .setProjectUuid("PROJECT")
+        .setSeverity(Severity.BLOCKER)
+        .setStatus(Issue.STATUS_CLOSED)
+        .setResolution(Issue.RESOLUTION_FIXED)
+        .setNew(false)
+        .setChanged(true)
+        .addComment(new DefaultIssueComment()
+            .setKey("COMMENT")
+            .setIssueKey("ISSUE")
+            .setUserLogin("john")
+            .setMarkdownText("Some text")
+            .setNew(true)
+        )
+    ).close();
+
+    step.execute(null);
+
+    dbTester.assertDbUnit(getClass(), "add_comment-result.xml", new String[]{"id", "created_at", "updated_at"}, "issue_changes");
+  }
+
+  @Test
+  public void add_change() throws Exception {
+    dbTester.prepareDbUnit(getClass(), "shared.xml");
+
+    issueCache.newAppender().append(new DefaultIssue()
+        .setKey("ISSUE")
+        .setRuleKey(RuleKey.of("xoo", "S01"))
+        .setComponentUuid("COMPONENT")
+        .setProjectUuid("PROJECT")
+        .setSeverity(Severity.BLOCKER)
+        .setStatus(Issue.STATUS_CLOSED)
+        .setResolution(Issue.RESOLUTION_FIXED)
+        .setNew(false)
+        .setChanged(true)
+        .setCurrentChange(new FieldDiffs()
+            .setIssueKey("ISSUE")
+            .setUserLogin("john")
+            .setDiff("technicalDebt", null, 1L)
+        )
+    ).close();
+
+    step.execute(null);
+
+    dbTester.assertDbUnit(getClass(), "add_change-result.xml", new String[]{"id", "created_at", "updated_at"}, "issue_changes");
+  }
+
+}
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_change-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_change-result.xml
new file mode 100644
index 00000000000..78e771f0988
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_change-result.xml
@@ -0,0 +1,15 @@
+<dataset>
+
+  <issue_changes
+      id="1"
+      kee="[null]"
+      issue_key="ISSUE"
+      user_login="john"
+      change_type="diff"
+      change_data="technicalDebt=1"
+      created_at="1400000000000"
+      updated_at="1400000000000"
+      issue_change_creation_date="[null]"
+      />
+
+</dataset>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_comment-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_comment-result.xml
new file mode 100644
index 00000000000..c95cb878bca
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/add_comment-result.xml
@@ -0,0 +1,15 @@
+<dataset>
+
+  <issue_changes
+      id="1"
+      kee="COMMENT"
+      issue_key="ISSUE"
+      user_login="john"
+      change_type="comment"
+      change_data="Some text"
+      created_at="1400000000000"
+      updated_at="1400000000000"
+      issue_change_creation_date="[null]"
+      />
+
+</dataset>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/close_issue-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/close_issue-result.xml
new file mode 100644
index 00000000000..e15b08c3e25
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/close_issue-result.xml
@@ -0,0 +1,31 @@
+<dataset>
+
+  <issues
+      id="1"
+      kee="ISSUE"
+      component_uuid="COMPONENT"
+      project_uuid="PROJECT"
+      rule_id="1"
+      severity="BLOCKER"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      technical_debt="[null]"
+      status="CLOSED"
+      resolution="FIXED"
+      checksum="[null]"
+      reporter="[null]"
+      author_login="[null]"
+      assignee="[null]"
+      action_plan_key="[null]"
+      tags="[null]"
+      issue_attributes=""
+      issue_creation_date="[null]"
+      issue_update_date="[null]"
+      issue_close_date="[null]"
+      created_at="1100000000000"
+      updated_at="1400000000000"
+      />
+
+</dataset>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue-result.xml
new file mode 100644
index 00000000000..7c0c5c10d55
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue-result.xml
@@ -0,0 +1,31 @@
+<dataset>
+
+  <issues
+      id="1"
+      kee="ISSUE"
+      component_uuid="COMPONENT"
+      project_uuid="PROJECT"
+      rule_id="1"
+      severity="BLOCKER"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      technical_debt="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      author_login="[null]"
+      assignee="[null]"
+      action_plan_key="[null]"
+      tags="[null]"
+      issue_attributes=""
+      issue_creation_date="[null]"
+      issue_update_date="[null]"
+      issue_close_date="[null]"
+      created_at="1400000000000"
+      updated_at="1400000000000"
+      />
+
+</dataset>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue.xml
new file mode 100644
index 00000000000..0d56aa5ec07
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/insert_new_issue.xml
@@ -0,0 +1,15 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="S01" plugin_name="xoo" name="name" description="description" status="READY"
+         plugin_config_key="[null]" priority="0" is_template="[true]" language="xoo"  template_id="[null]"
+         note_data="[null]" note_user_login="[null]" note_created_at="[null]" note_updated_at="[null]"
+         characteristic_id="100" default_characteristic_id="101"
+         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
+         remediation_coeff="1h" default_remediation_coeff="5d"
+         remediation_offset="5min" default_remediation_offset="10h"
+         effort_to_fix_description="[null]" description_format="MARKDOWN"
+         tags="[null]" system_tags="[null]"
+         created_at="2013-12-16" updated_at="2013-12-16"
+      />
+
+</dataset>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/shared.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/shared.xml
new file mode 100644
index 00000000000..23f7ab20677
--- /dev/null
+++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/step/PersistIssuesStepTest/shared.xml
@@ -0,0 +1,43 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="S01" plugin_name="xoo" name="name" description="description" status="READY"
+         plugin_config_key="[null]" priority="0" is_template="[true]" language="xoo"  template_id="[null]"
+         note_data="[null]" note_user_login="[null]" note_created_at="[null]" note_updated_at="[null]"
+         characteristic_id="100" default_characteristic_id="101"
+         remediation_function="LINEAR" default_remediation_function="LINEAR_OFFSET"
+         remediation_coeff="1h" default_remediation_coeff="5d"
+         remediation_offset="5min" default_remediation_offset="10h"
+         effort_to_fix_description="[null]" description_format="MARKDOWN"
+         tags="[null]" system_tags="[null]"
+         created_at="2013-12-16" updated_at="2013-12-16"
+      />
+
+  <issues
+      id="1"
+      kee="ISSUE"
+      component_uuid="COMPONENT"
+      project_uuid="PROJECT"
+      rule_id="1"
+      severity="BLOCKER"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      technical_debt="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      author_login="[null]"
+      assignee="[null]"
+      action_plan_key="[null]"
+      tags="[null]"
+      issue_attributes=""
+      issue_creation_date="[null]"
+      issue_update_date="[null]"
+      issue_close_date="[null]"
+      created_at="1100000000000"
+      updated_at="1100000000000"
+      />
+
+</dataset>