From: Julien Lancelot Date: Thu, 19 Mar 2015 14:18:12 +0000 (+0100) Subject: Remove useless code in PersistIssuesStep and add unit tests X-Git-Tag: 5.2-RC1~2526 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6295e7813ed56935eca2bedefdd3c6c932a1c9df;p=sonarqube.git Remove useless code in PersistIssuesStep and add unit tests --- 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 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 @@ + + + + + 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 @@ + + + + + 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 @@ + + + + + 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 @@ + + + + + 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 @@ + + + + + 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 @@ + + + + + + +