From 3a86b9a72a32eed7205df7fc89cef32321c4f550 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 16 Sep 2014 13:16:09 +0200 Subject: [PATCH] SONAR-5531 Update IssueStorage to allow issue index modification on server side --- .../issue/InitialOpenIssuesStackTest.java | 10 +- .../issue/IssueTrackingDecoratorTest.java | 28 ++-- .../plugins/core/issue/IssueTrackingTest.java | 5 +- .../server/batch/UploadReportAction.java | 4 + .../issue/InternalRubyIssueService.java | 1 + .../org/sonar/server/issue/IssueService.java | 12 +- .../server/issue/ServerIssueStorage.java | 32 +++-- .../sonar/server/issue/index/IssueIndex.java | 11 ++ .../server/qualityprofile/QProfileLoader.java | 2 +- .../sonar/server/rule/DefaultRuleFinder.java | 2 +- .../org/sonar/server/rule/RuleService.java | 4 +- .../org/sonar/server/search/BaseIndex.java | 29 ++-- .../batch/UploadReportActionMediumTest.java | 14 +- .../ComponentCleanerServiceMediumTest.java | 4 +- .../server/issue/DefaultIssueFinderTest.java | 96 +++++++------- .../server/issue/ServerIssueStorageTest.java | 91 ++++++++++++- .../actionplan/ActionPlanServiceTest.java | 2 +- .../issue/db/IssueBackendMediumTest.java | 12 +- .../IssueAuthorizationIndexMediumTest.java | 8 +- .../issue/index/IssueIndexMediumTest.java | 17 ++- .../InternalPermissionServiceMediumTest.java | 4 +- .../ActiveRuleBackendMediumTest.java | 10 +- .../RegisterQualityProfilesMediumTest.java | 8 +- .../rule/index/RuleIndexMediumTest.java | 2 +- .../should_insert_new_issues-result.xml | 26 ++++ .../should_insert_new_issues.xml | 4 + .../should_update_issues-result.xml | 32 +++++ .../should_update_issues.xml | 31 +++++ .../sonar/batch/issue/ScanIssueStorage.java | 36 ++++- .../batch/issue/ScanIssueStorageTest.java | 125 ++++++++++++++++++ .../should_insert_new_issues-result.xml | 26 ++++ .../should_insert_new_issues.xml | 4 + ...ld_resolve_conflicts_on_updates-result.xml | 33 +++++ .../should_resolve_conflicts_on_updates.xml | 54 ++++++++ .../should_update_issues-result.xml | 32 +++++ .../should_update_issues.xml | 31 +++++ .../org/sonar/core/issue/db/IssueDto.java | 19 ++- .../org/sonar/core/issue/db/IssueStorage.java | 40 +----- .../core/issue/db/UpdateConflictResolver.java | 2 +- .../org/sonar/core/issue/db/IssueDtoTest.java | 6 +- .../sonar/core/issue/db/IssueStorageTest.java | 53 ++------ .../issue/db/UpdateConflictResolverTest.java | 8 +- 42 files changed, 734 insertions(+), 236 deletions(-) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues-result.xml create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues.xml create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues-result.xml create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues-result.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates-result.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues-result.xml create mode 100644 sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues.xml diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java index aa81851122d..08cb3cfc3d6 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java @@ -70,7 +70,7 @@ public class InitialOpenIssuesStackTest { @Test public void get_and_remove_issues() { - IssueDto issueDto = new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1"); + IssueDto issueDto = new IssueDto().setComponentKey("org.struts.Action").setKee("ISSUE-1"); stack.addIssue(issueDto); List issueDtos = stack.selectAndRemoveIssues("org.struts.Action"); @@ -82,8 +82,8 @@ public class InitialOpenIssuesStackTest { @Test public void get_and_remove_with_many_issues_on_same_resource() { - stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); - stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-2")); + stack.addIssue(new IssueDto().setComponentKey("org.struts.Action").setKee("ISSUE-1")); + stack.addIssue(new IssueDto().setComponentKey("org.struts.Action").setKee("ISSUE-2")); List issueDtos = stack.selectAndRemoveIssues("org.struts.Action"); assertThat(issueDtos).hasSize(2); @@ -93,7 +93,7 @@ public class InitialOpenIssuesStackTest { @Test public void get_and_remove_do_nothing_if_resource_not_found() { - stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); + stack.addIssue(new IssueDto().setComponentKey("org.struts.Action").setKee("ISSUE-1")); List issueDtos = stack.selectAndRemoveIssues("Other"); assertThat(issueDtos).hasSize(0); @@ -119,7 +119,7 @@ public class InitialOpenIssuesStackTest { @Test public void clear_issues() { - stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); + stack.addIssue(new IssueDto().setComponentKey("org.struts.Action").setKee("ISSUE-1")); assertThat(stack.selectAllIssues()).hasSize(1); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index 121b473d2d8..c2f20fc26cc 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -137,7 +137,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey("squid", "AvoidCycle"); IssueTrackingResult trackingResult = new IssueTrackingResult(); trackingResult.addUnmatched(unmatchedIssue); @@ -163,7 +163,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -209,7 +209,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("CLOSED").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("CLOSED").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -243,7 +243,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(null).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(null).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -279,7 +279,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { // INPUT : one issue existing during previous scan final int issueOnLine = 6; - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -327,7 +327,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { // INPUT : one issue existing during previous scan final int issueOnLine = 3; - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(issueOnLine).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -375,7 +375,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance").setStatus(Rule.STATUS_REMOVED)); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -408,7 +408,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(null); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -441,7 +441,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan - IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey_unit_test_only("manual", "Performance"); + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(6).setStatus("OPEN").setRuleKey("manual", "Performance"); when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(null); IssueTrackingResult trackingResult = new IssueTrackingResult(); @@ -486,7 +486,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { Project project = new Project("struts"); DefaultIssue openIssue = new DefaultIssue(); when(issueCache.byComponent("struts")).thenReturn(Arrays.asList(openIssue)); - IssueDto deadIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle"); + IssueDto deadIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey("squid", "AvoidCycle"); when(initialOpenIssues.selectAllIssues()).thenReturn(Arrays.asList(deadIssue)); decorator.doDecorate(project); @@ -507,8 +507,8 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { @Test public void merge_matched_issue() throws Exception { - IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle") - .setLine(10).setSeverity("MAJOR").setMessage("Message").setEffortToFix(1.5).setDebt(1L).setRootComponentKey_unit_test_only("sample"); + IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey("squid", "AvoidCycle") + .setLine(10).setSeverity("MAJOR").setMessage("Message").setEffortToFix(1.5).setDebt(1L).setRootComponentKey("sample"); DefaultIssue issue = new DefaultIssue(); IssueTrackingResult trackingResult = mock(IssueTrackingResult.class); @@ -526,7 +526,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { @Test public void merge_matched_issue_on_manual_severity() throws Exception { - IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle") + IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey("squid", "AvoidCycle") .setLine(10).setManualSeverity(true).setSeverity("MAJOR").setMessage("Message").setEffortToFix(1.5).setDebt(1L); DefaultIssue issue = new DefaultIssue(); @@ -544,7 +544,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { public void merge_issue_changelog_with_previous_changelog() throws Exception { when(initialOpenIssues.selectChangelog("ABCDE")).thenReturn(newArrayList(new IssueChangeDto().setIssueKey("ABCD"))); - IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle") + IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey("squid", "AvoidCycle") .setLine(10).setMessage("Message").setEffortToFix(1.5).setDebt(1L); DefaultIssue issue = new DefaultIssue(); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java index 15064b8a9da..761ca65210e 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java @@ -20,12 +20,11 @@ package org.sonar.plugins.core.issue; -import org.sonar.api.batch.SonarIndex; - import com.google.common.base.Charsets; import com.google.common.io.Resources; import org.junit.Before; import org.junit.Test; +import org.sonar.api.batch.SonarIndex; import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.resources.Project; @@ -355,7 +354,7 @@ public class IssueTrackingTest { referenceIssue.setKee(Long.toString(id)); referenceIssue.setLine(lineId); referenceIssue.setMessage(message); - referenceIssue.setRuleKey_unit_test_only(ruleRepo, ruleKey); + referenceIssue.setRuleKey(ruleRepo, ruleKey); referenceIssue.setChecksum(lineChecksum); referenceIssue.setResolution(null); referenceIssue.setStatus(Issue.STATUS_OPEN); diff --git a/server/sonar-server/src/main/java/org/sonar/server/batch/UploadReportAction.java b/server/sonar-server/src/main/java/org/sonar/server/batch/UploadReportAction.java index a893c68ada2..a35ffcb46ad 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/batch/UploadReportAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/batch/UploadReportAction.java @@ -93,6 +93,10 @@ public class UploadReportAction implements RequestHandler { dbClient.issueDao().synchronizeAfter(session, index.get(IssueIndex.class).getLastSynchronization(), ImmutableMap.of("project", projectKey)); + + // Index project's permissions indexes + permissionService.synchronizePermissions(session, project.key()); + session.commit(); } finally { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 9ed1662e746..1fa162a5dd7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -459,6 +459,7 @@ public class InternalRubyIssueService implements ServerComponent { }); } + /** * Execute issue filter from parameters */ diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 0dd4724da90..6ef2e3b252f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -63,11 +63,8 @@ import org.sonar.server.search.QueryContext; import org.sonar.server.user.UserSession; import javax.annotation.Nullable; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Date; -import java.util.List; + +import java.util.*; /** * @since 3.6 @@ -295,6 +292,8 @@ public class IssueService implements ServerComponent { } public IssueQueryResult loadIssue(String issueKey) { + // TODO use IssueIndex for ACL + // TODO load DTO IssueQueryResult result = finder.find(IssueQuery.builder().issueKeys(Arrays.asList(issueKey)).requiredRole(UserRole.USER).build()); if (result.issues().size() != 1) { // TODO throw 404 @@ -337,10 +336,7 @@ public class IssueService implements ServerComponent { } public org.sonar.server.search.Result search(IssueQuery query, QueryContext options) { - IssueIndex issueIndex = indexClient.get(IssueIndex.class); - return issueIndex.search(query, options); - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueStorage.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueStorage.java index 90ccf5f4762..bdc93fbc925 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueStorage.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueStorage.java @@ -29,8 +29,8 @@ import org.sonar.core.persistence.MyBatis; import org.sonar.core.resource.ResourceDto; import org.sonar.core.resource.ResourceQuery; import org.sonar.server.db.DbClient; -import org.sonar.server.search.IndexDefinition; -import org.sonar.server.search.action.UpsertDto; + +import java.util.Date; /** * @since 3.6 @@ -44,22 +44,21 @@ public class ServerIssueStorage extends IssueStorage implements ServerComponent this.dbClient = dbClient; } - @Override - public void save(Iterable issues) { - super.save(issues); - DbSession session = dbClient.openSession(false); - try { - for (DefaultIssue issue : issues) { - IssueDto issueDto = dbClient.issueDao().getByKey(session, issue.key()); - session.enqueue(new UpsertDto(IndexDefinition.ISSUES.getIndexType(), issueDto)); - } - session.commit(); - } finally { - session.close(); - } + protected void doInsert(DbSession session, Date now, DefaultIssue issue) { + long componentId = componentId(issue); + long projectId = projectId(issue); + int ruleId = ruleId(issue); + IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, projectId, ruleId, now); + + dbClient.issueDao().insert(session, dto); + } + + protected void doUpdate(DbSession session, Date now, DefaultIssue issue) { + IssueDto dto = IssueDto.toDtoForUpdate(issue, projectId(issue), now); + + dbClient.issueDao().update(session, dto); } - @Override protected long componentId(DefaultIssue issue) { // TODO should be using ComponentDao ResourceDto resourceDto = dbClient.resourceDao().getResource(ResourceQuery.create().setKey(issue.componentKey())); @@ -69,7 +68,6 @@ public class ServerIssueStorage extends IssueStorage implements ServerComponent return resourceDto.getId(); } - @Override protected long projectId(DefaultIssue issue) { // TODO should be using ComponentDao ResourceDto resourceDto = dbClient.resourceDao().getResource(ResourceQuery.create().setKey(issue.projectKey())); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 3d7f1803797..d34959bdb44 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -41,6 +41,8 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import static com.google.common.collect.Lists.newArrayList; + public class IssueIndex extends BaseIndex { public IssueIndex(IssueNormalizer normalizer, SearchClient client) { @@ -110,6 +112,15 @@ public class IssueIndex extends BaseIndex { return new IssueDoc(fields); } + @Override + public Issue getNullableByKey(String key) { + Result result = search(IssueQuery.builder().issueKeys(newArrayList(key)).build(), new QueryContext()); + if (result.getTotal() == 1) { + return result.getHits().get(0); + } + return null; + } + public Result search(IssueQuery query, QueryContext options) { SearchRequestBuilder esSearch = getClient() diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLoader.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLoader.java index 1460a8950b4..20b689a13b9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLoader.java @@ -86,7 +86,7 @@ public class QProfileLoader implements ServerComponent { @CheckForNull public ActiveRule getActiveRule(ActiveRuleKey key) { - return index.get(ActiveRuleIndex.class).getByKey(key); + return index.get(ActiveRuleIndex.class).getNullableByKey(key); } public List findActiveRulesByRule(RuleKey key) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java b/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java index bb96fd210b6..319e0b8e6b3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/DefaultRuleFinder.java @@ -73,7 +73,7 @@ public class DefaultRuleFinder implements RuleFinder { @CheckForNull public org.sonar.api.rules.Rule findByKey(RuleKey key) { - Rule rule = index.getByKey(key); + Rule rule = index.getNullableByKey(key); if (rule != null && rule.status() != RuleStatus.REMOVED) { return toRule(rule); } else { diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java index 45850dd4dcd..e10e619b656 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RuleService.java @@ -55,7 +55,7 @@ public class RuleService implements ServerComponent { @CheckForNull public Rule getByKey(RuleKey key) { - return index.getByKey(key); + return index.getNullableByKey(key); } public List getByKeys(Collection keys) { @@ -63,7 +63,7 @@ public class RuleService implements ServerComponent { } public Rule getNonNullByKey(RuleKey key) { - Rule rule = index.getByKey(key); + Rule rule = index.getNullableByKey(key); if (rule == null) { throw new NotFoundException("Rule not found: " + key); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java b/server/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java index 53e2fdfae8b..6faafd538df 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java @@ -26,12 +26,7 @@ import com.google.common.collect.Multimap; import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsResponse; import org.elasticsearch.action.count.CountRequestBuilder; import org.elasticsearch.action.count.CountResponse; -import org.elasticsearch.action.get.GetRequestBuilder; -import org.elasticsearch.action.get.GetResponse; -import org.elasticsearch.action.get.MultiGetItemResponse; -import org.elasticsearch.action.get.MultiGetRequest; -import org.elasticsearch.action.get.MultiGetRequestBuilder; -import org.elasticsearch.action.get.MultiGetResponse; +import org.elasticsearch.action.get.*; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchScrollRequestBuilder; @@ -54,21 +49,14 @@ import org.joda.time.DateTime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.core.persistence.Dto; +import org.sonar.server.exceptions.NotFoundException; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.io.IOException; import java.io.Serializable; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Queue; +import java.util.*; public abstract class BaseIndex, KEY extends Serializable> implements Index { @@ -386,6 +374,15 @@ public abstract class BaseIndex, KEY extends Serial protected abstract DOMAIN toDoc(Map fields); public DOMAIN getByKey(KEY key) { + DOMAIN value = getNullableByKey(key); + if (value == null) { + throw new NotFoundException(String.format("Key '%s' not found", key)); + } + return value; + } + + @CheckForNull + public DOMAIN getNullableByKey(KEY key) { GetRequestBuilder request = client.prepareGet() .setType(this.getIndexType()) .setIndex(this.getIndexName()) diff --git a/server/sonar-server/src/test/java/org/sonar/server/batch/UploadReportActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/batch/UploadReportActionMediumTest.java index 1f8deae5242..0e39c7af09e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/batch/UploadReportActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/batch/UploadReportActionMediumTest.java @@ -48,6 +48,8 @@ import org.sonar.server.tester.ServerTester; import org.sonar.server.user.MockUserSession; import org.sonar.server.ws.WsTester; +import java.util.Date; + import static org.fest.assertions.Assertions.assertThat; public class UploadReportActionMediumTest { @@ -99,7 +101,7 @@ public class UploadReportActionMediumTest { session.commit(); - assertThat(tester.get(IssueAuthorizationIndex.class).getByKey(project.getKey())).isNull(); + assertThat(tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.getKey())).isNull(); MockUserSession.set().setLogin("john").setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION); WsTester.TestRequest request = wsTester.newGetRequest(BatchWs.API_ENDPOINT, UploadReportAction.UPLOAD_REPORT_ACTION); @@ -108,7 +110,7 @@ public class UploadReportActionMediumTest { request.execute(); // Check that issue authorization index has been created - assertThat(tester.get(IssueAuthorizationIndex.class).getByKey(project.getKey())).isNotNull(); + assertThat(tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.getKey())).isNotNull(); } @Test(expected = ForbiddenException.class) @@ -135,6 +137,10 @@ public class UploadReportActionMediumTest { .setProjectId(1L); db.componentDao().insert(session, project); + // project can be seen by anyone + tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), DefaultGroups.ANYONE, UserRole.USER, session); + db.issueAuthorizationDao().synchronizeAfter(session, new Date(0)); + ComponentDto resource = new ComponentDto() .setProjectId(1L) .setKey("MyComponent") @@ -162,7 +168,7 @@ public class UploadReportActionMediumTest { // Clear issue index to simulate that the issue has been inserted by the batch, so that it's not yet index in E/S clearIssueIndex(); assertThat(db.issueDao().getByKey(session, issue.getKey())).isNotNull(); - assertThat(tester.get(IssueIndex.class).getByKey(issue.getKey())).isNull(); + assertThat(tester.get(IssueIndex.class).getNullableByKey(issue.getKey())).isNull(); MockUserSession.set().setLogin("john").setGlobalPermissions(GlobalPermissions.SCAN_EXECUTION); @@ -171,7 +177,7 @@ public class UploadReportActionMediumTest { request.execute(); // Check that the issue has well be indexed in E/S - assertThat(tester.get(IssueIndex.class).getByKey(issue.getKey())).isNotNull(); + assertThat(tester.get(IssueIndex.class).getNullableByKey(issue.getKey())).isNotNull(); } private void clearIssueIndex(){ diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentCleanerServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentCleanerServiceMediumTest.java index f87765e1147..0f9ed0d3483 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentCleanerServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentCleanerServiceMediumTest.java @@ -95,11 +95,11 @@ public class ComponentCleanerServiceMediumTest { session.commit(); - assertThat(tester.get(IssueAuthorizationIndex.class).getByKey(project.getKey())).isNotNull(); + assertThat(tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.getKey())).isNotNull(); service.delete(project.getKey()); - assertThat(tester.get(IssueAuthorizationIndex.class).getByKey(project.getKey())).isNull(); + assertThat(tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.getKey())).isNull(); } @Test(expected = IllegalArgumentException.class) diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueFinderTest.java index 663e57a5420..738de5c4c63 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueFinderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/DefaultIssueFinderTest.java @@ -41,9 +41,9 @@ import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.persistence.MyBatis; import org.sonar.core.resource.ResourceDao; -import org.sonar.server.rule.DefaultRuleFinder; import org.sonar.core.user.DefaultUser; import org.sonar.server.issue.actionplan.ActionPlanService; +import org.sonar.server.rule.DefaultRuleFinder; import java.util.Collections; import java.util.List; @@ -84,14 +84,14 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -111,14 +111,14 @@ public class IssueQuery query = IssueQuery.builder().pageSize(1).pageIndex(1).build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(135l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Phases.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Phases.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectIssueIds(eq(query), anyInt(), any(SqlSession.class))).thenReturn(dtoList); @@ -136,9 +136,9 @@ public class @Test public void find_by_key() { IssueDto issueDto = new IssueDto().setId(1L).setRuleId(1).setComponentId(1l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); when(issueDao.selectByKey("ABCDE")).thenReturn(issueDto); @@ -156,14 +156,14 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -184,9 +184,9 @@ public class IssueQuery query = IssueQuery.builder().hideRules(true).build(); IssueDto issue = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(newArrayList(issue)); @@ -204,14 +204,14 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -231,14 +231,14 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -258,14 +258,14 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l).setKee("ABC").setActionPlanKey("A") - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l).setKee("DEF").setActionPlanKey("B") - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -288,10 +288,10 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l).setKee("ABC").setAssignee("perceval") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setComponentId(123l).setRootComponentId(100l).setKee("DEF").setReporter("arthur") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN"); List dtoList = newArrayList(issue1, issue2); when(issueDao.selectByIds(anyCollection(), any(SqlSession.class))).thenReturn(dtoList); @@ -322,9 +322,9 @@ public class IssueQuery query = IssueQuery.builder().build(); IssueDto issue = new IssueDto().setId(1L).setRuleId(50).setComponentId(123l).setRootComponentId(100l) - .setComponentKey_unit_test_only("Action.java") - .setRootComponentKey_unit_test_only("struts") - .setRuleKey_unit_test_only("squid", "AvoidCycle") + .setComponentKey("Action.java") + .setRootComponentKey("struts") + .setRuleKey("squid", "AvoidCycle") .setStatus("OPEN").setResolution("OPEN") .setDebt(10L); List dtoList = newArrayList(issue); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueStorageTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueStorageTest.java index 9a2cd6f98cf..1ce4db1a1b7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueStorageTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ServerIssueStorageTest.java @@ -23,17 +23,23 @@ package org.sonar.server.issue; import org.junit.Before; import org.junit.Test; import org.sonar.api.issue.internal.DefaultIssue; +import org.sonar.api.issue.internal.DefaultIssueComment; +import org.sonar.api.issue.internal.IssueChangeContext; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.RuleQuery; +import org.sonar.api.utils.DateUtils; +import org.sonar.api.utils.Duration; import org.sonar.api.utils.System2; import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.core.resource.ResourceDao; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.db.DbClient; +import org.sonar.server.issue.db.IssueDao; import java.util.Collection; +import java.util.Date; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; @@ -42,18 +48,22 @@ public class ServerIssueStorageTest extends AbstractDaoTestCase { DbClient dbClient; + ServerIssueStorage storage; + @Before public void setupDbClient() { dbClient = new DbClient(getDatabase(), getMyBatis(), new ComponentDao(System2.INSTANCE), + new IssueDao(System2.INSTANCE), new ResourceDao(getMyBatis(), System2.INSTANCE)); + + storage = new ServerIssueStorage(getMyBatis(), new FakeRuleFinder(), dbClient); } @Test public void load_component_id_from_db() throws Exception { setupData("load_component_id_from_db"); - ServerIssueStorage storage = new ServerIssueStorage(getMyBatis(), new FakeRuleFinder(), dbClient); long componentId = storage.componentId(new DefaultIssue().setComponentKey("struts:Action.java")); assertThat(componentId).isEqualTo(123); @@ -63,7 +73,6 @@ public class ServerIssueStorageTest extends AbstractDaoTestCase { public void fail_to_load_component_id_if_unknown_component() throws Exception { setupData("empty"); - ServerIssueStorage storage = new ServerIssueStorage(getMyBatis(), new FakeRuleFinder(), dbClient); try { storage.componentId(new DefaultIssue().setComponentKey("struts:Action.java")); fail(); @@ -76,7 +85,6 @@ public class ServerIssueStorageTest extends AbstractDaoTestCase { public void load_project_id_from_db() throws Exception { setupData("load_project_id_from_db"); - ServerIssueStorage storage = new ServerIssueStorage(getMyBatis(), new FakeRuleFinder(), dbClient); long projectId = storage.projectId(new DefaultIssue().setProjectKey("struts")); assertThat(projectId).isEqualTo(1); @@ -86,7 +94,6 @@ public class ServerIssueStorageTest extends AbstractDaoTestCase { public void fail_to_load_project_id_if_unknown_component() throws Exception { setupData("empty"); - ServerIssueStorage storage = new ServerIssueStorage(getMyBatis(), new FakeRuleFinder(), dbClient); try { storage.projectId(new DefaultIssue().setProjectKey("struts")); fail(); @@ -95,6 +102,82 @@ public class ServerIssueStorageTest extends AbstractDaoTestCase { } } + @Test + public void should_insert_new_issues() throws Exception { + setupData("should_insert_new_issues"); + + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); + // override generated key + comment.setKey("FGHIJ"); + + Date date = DateUtils.parseDate("2013-05-18"); + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setNew(true) + + .setRuleKey(RuleKey.of("squid", "AvoidCycle")) + .setLine(5000) + .setDebt(Duration.create(10L)) + .setReporter("emmerik") + .setResolution("OPEN") + .setStatus("OPEN") + .setSeverity("BLOCKER") + .setAttribute("foo", "bar") + .addComment(comment) + .setCreationDate(date) + .setUpdateDate(date) + .setCloseDate(date) + + .setComponentKey("struts:Action"); + + storage.save(issue); + + checkTables("should_insert_new_issues", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues", "issue_changes"); + } + + @Test + public void should_update_issues() throws Exception { + setupData("should_update_issues"); + + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "emmerik"); + + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); + // override generated key + comment.setKey("FGHIJ"); + + Date date = DateUtils.parseDate("2013-05-18"); + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setNew(false) + .setChanged(true) + + // updated fields + .setLine(5000) + .setDebt(Duration.create(10L)) + .setChecksum("FFFFF") + .setAuthorLogin("simon") + .setAssignee("loic") + .setFieldChange(context, "severity", "INFO", "BLOCKER") + .setReporter("emmerik") + .setResolution("FIXED") + .setStatus("RESOLVED") + .setSeverity("BLOCKER") + .setAttribute("foo", "bar") + .addComment(comment) + .setCreationDate(date) + .setUpdateDate(date) + .setCloseDate(date) + + // unmodifiable fields + .setRuleKey(RuleKey.of("xxx", "unknown")) + .setComponentKey("struts:Action") + .setProjectKey("struts"); + + storage.save(issue); + + checkTables("should_update_issues", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues", "issue_changes"); + } + static class FakeRuleFinder implements RuleFinder { @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/actionplan/ActionPlanServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/actionplan/ActionPlanServiceTest.java index a7bb7ec50f3..be984e7cf0a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/actionplan/ActionPlanServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/actionplan/ActionPlanServiceTest.java @@ -144,7 +144,7 @@ public class ActionPlanServiceTest { when(actionPlanDao.findByKey("ABCD")).thenReturn(new ActionPlanDto().setKey("ABCD").setProjectKey_unit_test_only(projectKey)); when(resourceDao.getResource(any(ResourceQuery.class))).thenReturn(new ResourceDto().setKey(projectKey).setId(1l)); - IssueDto issueDto = new IssueDto().setId(100L).setStatus(Issue.STATUS_OPEN).setRuleKey_unit_test_only("squid", "s100"); + IssueDto issueDto = new IssueDto().setId(100L).setStatus(Issue.STATUS_OPEN).setRuleKey("squid", "s100"); when(issueDao.selectIssues(any(IssueQuery.class))).thenReturn(newArrayList(issueDto)); when(issueUpdater.plan(any(DefaultIssue.class), eq((ActionPlan) null), any(IssueChangeContext.class))).thenReturn(true); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueBackendMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueBackendMediumTest.java index 80fe1f10446..b2a231eb694 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueBackendMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueBackendMediumTest.java @@ -26,8 +26,11 @@ import org.junit.ClassRule; import org.junit.Test; import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; +import org.sonar.api.security.DefaultGroups; +import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.db.IssueDto; +import org.sonar.core.permission.PermissionFacade; import org.sonar.core.persistence.DbSession; import org.sonar.core.rule.RuleDto; import org.sonar.server.component.db.ComponentDao; @@ -81,6 +84,10 @@ public class IssueBackendMediumTest { .setProjectId(1L); tester.get(ComponentDao.class).insert(dbSession, project); + // project can be seen by anyone + tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), DefaultGroups.ANYONE, UserRole.USER, dbSession); + dbClient.issueAuthorizationDao().synchronizeAfter(dbSession, new Date(0)); + ComponentDto resource = new ComponentDto() .setProjectId(1L) .setKey("MyComponent") @@ -105,7 +112,6 @@ public class IssueBackendMediumTest { // should find by key Issue issueDoc = indexClient.get(IssueIndex.class).getByKey(issue.getKey()); - assertThat(issueDoc).isNotNull(); // Check all normalized fields assertThat(issueDoc.actionPlanKey()).isEqualTo(issue.getActionPlanKey()); @@ -150,9 +156,9 @@ public class IssueBackendMediumTest { IssueDto issue = new IssueDto().setId(1L) .setRuleId(rule.getId()) .setRootComponentId(project.getId()) - .setRootComponentKey_unit_test_only(project.key()) + .setRootComponentKey(project.key()) .setComponentId(resource.getId()) - .setComponentKey_unit_test_only(resource.key()) + .setComponentKey(resource.key()) .setStatus("OPEN").setResolution("OPEN") .setKee(UUID.randomUUID().toString()); dbClient.issueDao().insert(dbSession, issue); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java index 6d3ca263618..0895eedaa5b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java @@ -82,7 +82,7 @@ public class IssueAuthorizationIndexMediumTest { session.commit(); - assertThat(index.getByKey(project.getKey())).isNull(); + assertThat(index.getNullableByKey(project.getKey())).isNull(); db.issueAuthorizationDao().synchronizeAfter(session, new Date(0)); session.commit(); @@ -96,7 +96,7 @@ public class IssueAuthorizationIndexMediumTest { tester.clearIndexes(); tester.get(Platform.class).executeStartupTasks(); - assertThat(index.getByKey(project.getKey())).isNotNull(); + assertThat(index.getNullableByKey(project.getKey())).isNotNull(); } @Test @@ -120,12 +120,12 @@ public class IssueAuthorizationIndexMediumTest { db.issueAuthorizationDao().synchronizeAfter(session, new Date(0)); session.commit(); - assertThat(index.getByKey(project.getKey())).isNotNull(); + assertThat(index.getNullableByKey(project.getKey())).isNotNull(); db.issueAuthorizationDao().deleteByKey(session, project.key()); session.commit(); - assertThat(index.getByKey(project.getKey())).isNull(); + assertThat(index.getNullableByKey(project.getKey())).isNull(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java index fcdc2b7e3fd..fe9bd465261 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexMediumTest.java @@ -38,6 +38,7 @@ import org.sonar.core.user.GroupDto; import org.sonar.core.user.UserDto; import org.sonar.server.component.db.ComponentDao; import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.RuleTesting; import org.sonar.server.rule.db.RuleDao; import org.sonar.server.search.QueryContext; @@ -101,7 +102,21 @@ public class IssueIndexMediumTest { } @Test - public void filter_by_actionPlan() throws Exception { + public void get_by_key() throws Exception { + IssueDto issue = createIssue(); + db.issueDao().insert(session, issue); + session.commit(); + + assertThat(index.getByKey(issue.getKey())).isNotNull(); + } + + @Test(expected = NotFoundException.class) + public void fail_to_get_unknown_key() throws Exception { + index.getByKey("unknown"); + } + + @Test + public void filter_by_action_plan() throws Exception { String plan1 = "plan1"; String plan2 = "plan2"; IssueDto issue1 = createIssue() diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceMediumTest.java index 1aaa07ce015..0ac268f91e8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceMediumTest.java @@ -84,13 +84,13 @@ public class InternalPermissionServiceMediumTest { MockUserSession.set().setLogin("admin").addProjectPermissions(UserRole.ADMIN, project.key()); assertThat(tester.get(RoleDao.class).selectUserPermissions(session, user.getLogin(), project.getId())).isEmpty(); - assertThat(index.getByKey(project.getKey())).isNull(); + assertThat(index.getNullableByKey(project.getKey())).isNull(); service.addPermission(params(user.getLogin(), null, project.key(), UserRole.USER)); session.commit(); assertThat(tester.get(RoleDao.class).selectUserPermissions(session, user.getLogin(), project.getId())).hasSize(1); - assertThat(index.getByKey(project.getKey())).isNotNull(); + assertThat(index.getNullableByKey(project.getKey())).isNotNull(); } private Map params(@Nullable String login, @Nullable String group, @Nullable String component, String permission) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleBackendMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleBackendMediumTest.java index a730ff72db1..81907de87f2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleBackendMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleBackendMediumTest.java @@ -64,23 +64,23 @@ public class ActiveRuleBackendMediumTest extends SearchMediumTest { // 1. Synchronize since 0 tester.clearIndexes(); - assertThat(index.get(ActiveRuleIndex.class).getByKey(activeRule.getKey())).isNull(); + assertThat(index.get(ActiveRuleIndex.class).getNullableByKey(activeRule.getKey())).isNull(); db.activeRuleDao().synchronizeAfter(dbSession, new Date(0L)); dbSession.commit(); - assertThat(index.get(ActiveRuleIndex.class).getByKey(activeRule.getKey())).isNotNull(); + assertThat(index.get(ActiveRuleIndex.class).getNullableByKey(activeRule.getKey())).isNotNull(); // 2. Synchronize since beginning tester.clearIndexes(); - assertThat(index.get(ActiveRuleIndex.class).getByKey(activeRule.getKey())).isNull(); + assertThat(index.get(ActiveRuleIndex.class).getNullableByKey(activeRule.getKey())).isNull(); db.activeRuleDao().synchronizeAfter(dbSession, beginning); dbSession.commit(); - assertThat(index.get(ActiveRuleIndex.class).getByKey(activeRule.getKey())).isNotNull(); + assertThat(index.get(ActiveRuleIndex.class).getNullableByKey(activeRule.getKey())).isNotNull(); // 3. Assert startup picks it up tester.clearIndexes(); Date before = index.get(ActiveRuleIndex.class).getLastSynchronization(); tester.get(Platform.class).executeStartupTasks(); - assertThat(index.get(ActiveRuleIndex.class).getByKey(activeRule.getKey())).isNotNull(); + assertThat(index.get(ActiveRuleIndex.class).getNullableByKey(activeRule.getKey())).isNotNull(); assertThat(before.before(index.get(ActiveRuleIndex.class).getLastSynchronization())).isTrue(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java index 1c22edba15b..39d73517b8e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java @@ -82,14 +82,14 @@ public class RegisterQualityProfilesMediumTest { ActiveRuleKey activeRuleKey = ActiveRuleKey.of(profile.getKey(), ruleKey); // 0. Check and clear ES - assertThat(tester.get(ActiveRuleIndex.class).getByKey(activeRuleKey)).isNotNull(); + assertThat(tester.get(ActiveRuleIndex.class).getNullableByKey(activeRuleKey)).isNotNull(); tester.clearIndexes(); - assertThat(tester.get(ActiveRuleIndex.class).getByKey(activeRuleKey)).isNull(); + assertThat(tester.get(ActiveRuleIndex.class).getNullableByKey(activeRuleKey)).isNull(); tester.get(Platform.class).restart(); - assertThat(tester.get(ActiveRuleIndex.class).getByKey(activeRuleKey)).isNotNull(); + assertThat(tester.get(ActiveRuleIndex.class).getNullableByKey(activeRuleKey)).isNotNull(); // Check ActiveRules in ES - org.sonar.server.qualityprofile.ActiveRule activeRule = tester.get(ActiveRuleIndex.class).getByKey(activeRuleKey); + org.sonar.server.qualityprofile.ActiveRule activeRule = tester.get(ActiveRuleIndex.class).getNullableByKey(activeRuleKey); assertThat(activeRule.key().qProfile()).isEqualTo(profile.getKee()); assertThat(activeRule.key().ruleKey()).isEqualTo(ruleKey); assertThat(activeRule.severity()).isEqualTo(Severity.CRITICAL); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java index 4a0e0631132..0f0808ae47b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java @@ -82,7 +82,7 @@ public class RuleIndexMediumTest extends SearchMediumTest { @Test public void getByKey_null_if_not_found() throws InterruptedException { - Rule rule = index.getByKey(RuleKey.of("javascript", "unknown")); + Rule rule = index.getNullableByKey(RuleKey.of("javascript", "unknown")); assertThat(rule).isNull(); } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues-result.xml new file mode 100644 index 00000000000..59d288be419 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues-result.xml @@ -0,0 +1,26 @@ + + + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues.xml b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues.xml new file mode 100644 index 00000000000..683ead42b1f --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_insert_new_issues.xml @@ -0,0 +1,4 @@ + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues-result.xml b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues-result.xml new file mode 100644 index 00000000000..82db8eecb31 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues-result.xml @@ -0,0 +1,32 @@ + + + + + + diff --git a/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues.xml b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues.xml new file mode 100644 index 00000000000..8b552f4bd7f --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/issue/ServerIssueStorageTest/should_update_issues.xml @@ -0,0 +1,31 @@ + + + + + + + diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueStorage.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueStorage.java index 723082eb335..9f3ae235db8 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueStorage.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueStorage.java @@ -21,21 +21,29 @@ package org.sonar.batch.issue; import org.sonar.api.BatchComponent; import org.sonar.api.database.model.Snapshot; +import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.rules.RuleFinder; import org.sonar.batch.ProjectTree; import org.sonar.batch.index.SnapshotCache; +import org.sonar.core.issue.db.IssueDto; +import org.sonar.core.issue.db.IssueMapper; import org.sonar.core.issue.db.IssueStorage; +import org.sonar.core.issue.db.UpdateConflictResolver; +import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; import org.sonar.core.resource.ResourceDao; import org.sonar.core.resource.ResourceDto; import org.sonar.core.resource.ResourceQuery; +import java.util.Date; + public class ScanIssueStorage extends IssueStorage implements BatchComponent { private final SnapshotCache snapshotCache; private final ResourceDao resourceDao; private final ProjectTree projectTree; + private final UpdateConflictResolver conflictResolver = new UpdateConflictResolver(); public ScanIssueStorage(MyBatis mybatis, RuleFinder ruleFinder, SnapshotCache snapshotCache, ResourceDao resourceDao, ProjectTree projectTree) { super(mybatis, ruleFinder); @@ -44,7 +52,32 @@ public class ScanIssueStorage extends IssueStorage implements BatchComponent { this.projectTree = projectTree; } - @Override + protected void doInsert(DbSession session, Date now, DefaultIssue issue) { + IssueMapper issueMapper = session.getMapper(IssueMapper.class); + long componentId = componentId(issue); + long projectId = projectId(issue); + int ruleId = ruleId(issue); + IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, projectId, ruleId, now); + issueMapper.insert(dto); + } + + protected void doUpdate(DbSession session, Date now, DefaultIssue issue) { + IssueMapper issueMapper = session.getMapper(IssueMapper.class); + IssueDto dto = IssueDto.toDtoForUpdate(issue, projectId(issue), now); + if (Issue.STATUS_CLOSED.equals(issue.status()) || issue.selectedAt() == null) { + // Issue is closed by scan or changed by end-user + issueMapper.update(dto); + + } else { + int count = issueMapper.updateIfBeforeSelectedDate(dto); + if (count == 0) { + // End-user and scan changed the issue at the same time. + // See https://jira.codehaus.org/browse/SONAR-4309 + conflictResolver.resolve(issue, issueMapper); + } + } + } + protected long componentId(DefaultIssue issue) { Snapshot snapshot = snapshotCache.get(issue.componentKey()); if (snapshot != null) { @@ -59,7 +92,6 @@ public class ScanIssueStorage extends IssueStorage implements BatchComponent { return resourceDto.getId(); } - @Override protected long projectId(DefaultIssue issue) { return projectTree.getRootProject().getId(); } diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueStorageTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueStorageTest.java index 1d0144ce4ee..983d560d70d 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueStorageTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueStorageTest.java @@ -26,11 +26,15 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.sonar.api.database.model.Snapshot; import org.sonar.api.issue.internal.DefaultIssue; +import org.sonar.api.issue.internal.DefaultIssueComment; +import org.sonar.api.issue.internal.IssueChangeContext; import org.sonar.api.resources.Project; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.RuleQuery; +import org.sonar.api.utils.DateUtils; +import org.sonar.api.utils.Duration; import org.sonar.api.utils.System2; import org.sonar.batch.ProjectTree; import org.sonar.batch.index.SnapshotCache; @@ -38,6 +42,7 @@ import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.core.resource.ResourceDao; import java.util.Collection; +import java.util.Date; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; @@ -100,6 +105,126 @@ public class ScanIssueStorageTest extends AbstractDaoTestCase { assertThat(projectId).isEqualTo(100); } + @Test + public void should_insert_new_issues() throws Exception { + setupData("should_insert_new_issues"); + + Project project = new Project("struts"); + project.setId(10); + when(projectTree.getRootProject()).thenReturn(project); + + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); + // override generated key + comment.setKey("FGHIJ"); + + Date date = DateUtils.parseDate("2013-05-18"); + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setNew(true) + + .setRuleKey(RuleKey.of("squid", "AvoidCycle")) + .setLine(5000) + .setDebt(Duration.create(10L)) + .setReporter("emmerik") + .setResolution("OPEN") + .setStatus("OPEN") + .setSeverity("BLOCKER") + .setAttribute("foo", "bar") + .addComment(comment) + .setCreationDate(date) + .setUpdateDate(date) + .setCloseDate(date) + + .setComponentKey("struts:Action"); + + storage.save(issue); + + checkTables("should_insert_new_issues", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues", "issue_changes"); + } + + @Test + public void should_update_issues() throws Exception { + setupData("should_update_issues"); + + IssueChangeContext context = IssueChangeContext.createUser(new Date(), "emmerik"); + + Project project = new Project("struts"); + project.setId(10); + when(projectTree.getRootProject()).thenReturn(project); + + DefaultIssueComment comment = DefaultIssueComment.create("ABCDE", "emmerik", "the comment"); + // override generated key + comment.setKey("FGHIJ"); + + Date date = DateUtils.parseDate("2013-05-18"); + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setNew(false) + .setChanged(true) + + // updated fields + .setLine(5000) + .setDebt(Duration.create(10L)) + .setChecksum("FFFFF") + .setAuthorLogin("simon") + .setAssignee("loic") + .setFieldChange(context, "severity", "INFO", "BLOCKER") + .setReporter("emmerik") + .setResolution("FIXED") + .setStatus("RESOLVED") + .setSeverity("BLOCKER") + .setAttribute("foo", "bar") + .addComment(comment) + .setCreationDate(date) + .setUpdateDate(date) + .setCloseDate(date) + + // unmodifiable fields + .setRuleKey(RuleKey.of("xxx", "unknown")) + .setComponentKey("not:a:component"); + + storage.save(issue); + + checkTables("should_update_issues", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues", "issue_changes"); + } + + @Test + public void should_resolve_conflicts_on_updates() throws Exception { + setupData("should_resolve_conflicts_on_updates"); + + Project project = new Project("struts"); + project.setId(10); + when(projectTree.getRootProject()).thenReturn(project); + + Date date = DateUtils.parseDate("2013-05-18"); + DefaultIssue issue = new DefaultIssue() + .setKey("ABCDE") + .setNew(false) + .setChanged(true) + .setCreationDate(DateUtils.parseDate("2005-05-12")) + .setUpdateDate(date) + .setRuleKey(RuleKey.of("squid", "AvoidCycles")) + .setComponentKey("struts:Action") + + // issue in database has been updated in 2013, after the loading by scan + .setSelectedAt(DateUtils.parseDate("2005-01-01")) + + // fields to be updated + .setLine(444) + .setSeverity("BLOCKER") + .setChecksum("FFFFF") + .setAttribute("JIRA", "http://jira.com") + + // fields overridden by end-user -> do not save + .setAssignee("looser") + .setResolution(null) + .setStatus("REOPEN"); + + storage.save(issue); + + checkTables("should_resolve_conflicts_on_updates", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues"); + } + static class FakeRuleFinder implements RuleFinder { @Override diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues-result.xml new file mode 100644 index 00000000000..59d288be419 --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues-result.xml @@ -0,0 +1,26 @@ + + + + + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues.xml new file mode 100644 index 00000000000..683ead42b1f --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_insert_new_issues.xml @@ -0,0 +1,4 @@ + + + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates-result.xml new file mode 100644 index 00000000000..26b0dd5f351 --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates-result.xml @@ -0,0 +1,33 @@ + + + + + + + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates.xml new file mode 100644 index 00000000000..c1058c76ee8 --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_resolve_conflicts_on_updates.xml @@ -0,0 +1,54 @@ + + + + + + + + + + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues-result.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues-result.xml new file mode 100644 index 00000000000..82db8eecb31 --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues-result.xml @@ -0,0 +1,32 @@ + + + + + + diff --git a/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues.xml b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues.xml new file mode 100644 index 00000000000..8b552f4bd7f --- /dev/null +++ b/sonar-batch/src/test/resources/org/sonar/batch/issue/ScanIssueStorageTest/should_update_issues.xml @@ -0,0 +1,31 @@ + + + + + + + diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java index 58f7f2a5807..63278903bf2 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java @@ -34,6 +34,7 @@ import org.sonar.core.rule.RuleDto; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + import java.io.Serializable; import java.util.Date; import java.util.UUID; @@ -367,26 +368,26 @@ public final class IssueDto extends Dto implements Serializable { } /** - * Only for unit tests + * Should only be used to persist in E/S */ - public IssueDto setRuleKey_unit_test_only(String repo, String rule) { + public IssueDto setRuleKey(String repo, String rule) { this.ruleRepo = repo; this.ruleKey = rule; return this; } /** - * Only for unit tests + * Should only be used to persist in E/S */ - public IssueDto setComponentKey_unit_test_only(String componentKey) { + public IssueDto setComponentKey(String componentKey) { this.componentKey = componentKey; return this; } /** - * Only for unit tests + * Should only be used to persist in E/S */ - public IssueDto setRootComponentKey_unit_test_only(String rootComponentKey) { + public IssueDto setRootComponentKey(String rootComponentKey) { this.rootComponentKey = rootComponentKey; return this; } @@ -411,8 +412,11 @@ public final class IssueDto extends Dto implements Serializable { .setReporter(issue.reporter()) .setAssignee(issue.assignee()) .setRuleId(ruleId) + .setRuleKey(issue.ruleKey().repository(), issue.ruleKey().rule()) .setComponentId(componentId) + .setComponentKey(issue.componentKey()) .setRootComponentId(rootComponentId) + .setRootComponentKey(issue.projectKey()) .setActionPlanKey(issue.actionPlanKey()) .setIssueAttributes(KeyValueFormat.format(issue.attributes())) .setAuthorLogin(issue.authorLogin()) @@ -442,6 +446,9 @@ public final class IssueDto extends Dto implements Serializable { .setActionPlanKey(issue.actionPlanKey()) .setIssueAttributes(KeyValueFormat.format(issue.attributes())) .setAuthorLogin(issue.authorLogin()) + .setRuleKey(issue.ruleKey().repository(), issue.ruleKey().rule()) + .setComponentKey(issue.componentKey()) + .setRootComponentKey(issue.projectKey()) .setRootComponentId(rootComponentId) .setIssueCreationDate(issue.creationDate()) .setIssueCloseDate(issue.closeDate()) diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java index 0de970e0f8d..ea5a1f2ed96 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java @@ -20,7 +20,6 @@ package org.sonar.core.issue.db; import com.google.common.collect.Lists; -import org.apache.ibatis.session.SqlSession; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueComment; import org.sonar.api.issue.internal.DefaultIssue; @@ -49,7 +48,6 @@ public abstract class IssueStorage { private final MyBatis mybatis; private final RuleFinder ruleFinder; - private final UpdateConflictResolver conflictResolver = new UpdateConflictResolver(); protected IssueStorage(MyBatis mybatis, RuleFinder ruleFinder) { this.mybatis = mybatis; @@ -72,12 +70,11 @@ public abstract class IssueStorage { List toBeUpdated = Lists.newArrayList(); DbSession batchSession = mybatis.openSession(true); int count = 0; - IssueMapper issueMapper = batchSession.getMapper(IssueMapper.class); IssueChangeMapper issueChangeMapper = batchSession.getMapper(IssueChangeMapper.class); try { for (DefaultIssue issue : issues) { if (issue.isNew()) { - insert(issueMapper, now, issue); + doInsert(batchSession, now, issue); insertChanges(issueChangeMapper, issue); if (count > BatchSession.MAX_BATCH_SIZE) { batchSession.commit(); @@ -94,22 +91,15 @@ public abstract class IssueStorage { return toBeUpdated; } - private void insert(IssueMapper issueMapper, Date now, DefaultIssue issue) { - long componentId = componentId(issue); - long projectId = projectId(issue); - int ruleId = ruleId(issue); - IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, projectId, ruleId, now); - issueMapper.insert(dto); - } + protected abstract void doInsert(DbSession batchSession, Date now, DefaultIssue issue); private void update(List toBeUpdated, Date now) { if (!toBeUpdated.isEmpty()) { - SqlSession session = mybatis.openSession(false); + DbSession session = mybatis.openSession(false); try { - IssueMapper issueMapper = session.getMapper(IssueMapper.class); IssueChangeMapper issueChangeMapper = session.getMapper(IssueChangeMapper.class); for (DefaultIssue issue : toBeUpdated) { - update(issueMapper, now, issue); + doUpdate(session, now, issue); insertChanges(issueChangeMapper, issue); } session.commit(); @@ -119,21 +109,7 @@ public abstract class IssueStorage { } } - private void update(IssueMapper issueMapper, Date now, DefaultIssue issue) { - IssueDto dto = IssueDto.toDtoForUpdate(issue, projectId(issue), now); - if (Issue.STATUS_CLOSED.equals(issue.status()) || issue.selectedAt() == null) { - // Issue is closed by scan or changed by end-user - issueMapper.update(dto); - - } else { - int count = issueMapper.updateIfBeforeSelectedDate(dto); - if (count == 0) { - // End-user and scan changed the issue at the same time. - // See https://jira.codehaus.org/browse/SONAR-4309 - conflictResolver.resolve(issue, issueMapper); - } - } - } + protected abstract void doUpdate(DbSession batchSession, Date now, DefaultIssue issue); private void insertChanges(IssueChangeMapper mapper, DefaultIssue issue) { for (IssueComment comment : issue.comments()) { @@ -150,11 +126,7 @@ public abstract class IssueStorage { } } - protected abstract long componentId(DefaultIssue issue); - - protected abstract long projectId(DefaultIssue issue); - - private int ruleId(Issue issue) { + protected int ruleId(Issue issue) { Rule rule = ruleFinder.findByKey(issue.ruleKey()); if (rule == null) { throw new IllegalStateException("Rule not found: " + issue.ruleKey()); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java b/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java index 73370a7face..883d85d9ea8 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/UpdateConflictResolver.java @@ -31,7 +31,7 @@ import java.util.Date; * * @since 3.6 */ -class UpdateConflictResolver { +public class UpdateConflictResolver { private static final Logger LOG = LoggerFactory.getLogger(IssueStorage.class); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDtoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDtoTest.java index d0bfa5dcb5e..ddda3b57f71 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDtoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDtoTest.java @@ -58,9 +58,9 @@ public class IssueDtoTest { IssueDto dto = new IssueDto() .setKee("100") .setRuleId(1) - .setRuleKey_unit_test_only("squid", "AvoidCycle") - .setComponentKey_unit_test_only("org.sonar.sample:Sample") - .setRootComponentKey_unit_test_only("org.sonar.sample") + .setRuleKey("squid", "AvoidCycle") + .setComponentKey("org.sonar.sample:Sample") + .setRootComponentKey("org.sonar.sample") .setComponentId(1l) .setRootComponentId(1l) .setStatus(Issue.STATUS_CLOSED) diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java index 0f696f87310..c4b20282ff1 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueStorageTest.java @@ -30,6 +30,7 @@ import org.sonar.api.rules.RuleQuery; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.Duration; import org.sonar.core.persistence.AbstractDaoTestCase; +import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; import java.util.Collection; @@ -63,7 +64,9 @@ public class IssueStorageTest extends AbstractDaoTestCase { .addComment(comment) .setCreationDate(date) .setUpdateDate(date) - .setCloseDate(date); + .setCloseDate(date) + + .setComponentKey("struts:Action"); saver.save(issue); @@ -112,54 +115,24 @@ public class IssueStorageTest extends AbstractDaoTestCase { checkTables("should_update_issues", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues", "issue_changes"); } - @Test - public void should_resolve_conflicts_on_updates() throws Exception { - setupData("should_resolve_conflicts_on_updates"); - - FakeSaver saver = new FakeSaver(getMyBatis(), new FakeRuleFinder()); - - Date date = DateUtils.parseDate("2013-05-18"); - DefaultIssue issue = new DefaultIssue() - .setKey("ABCDE") - .setNew(false) - .setChanged(true) - .setCreationDate(DateUtils.parseDate("2005-05-12")) - .setUpdateDate(date) - .setRuleKey(RuleKey.of("squid", "AvoidCycles")) - .setComponentKey("struts:Action") - - // issue in database has been updated in 2013, after the loading by scan - .setSelectedAt(DateUtils.parseDate("2005-01-01")) - - // fields to be updated - .setLine(444) - .setSeverity("BLOCKER") - .setChecksum("FFFFF") - .setAttribute("JIRA", "http://jira.com") - - // fields overridden by end-user -> do not save - .setAssignee("looser") - .setResolution(null) - .setStatus("REOPEN"); - - saver.save(issue); - - checkTables("should_resolve_conflicts_on_updates", new String[]{"id", "created_at", "updated_at", "issue_change_creation_date"}, "issues"); - } - static class FakeSaver extends IssueStorage { + protected FakeSaver(MyBatis mybatis, RuleFinder ruleFinder) { super(mybatis, ruleFinder); } @Override - protected long componentId(DefaultIssue issue) { - return 100l; + protected void doInsert(DbSession session, Date now, DefaultIssue issue) { + int ruleId = ruleId(issue); + IssueDto dto = IssueDto.toDtoForInsert(issue, 100l, 10l, ruleId, now); + + session.getMapper(IssueMapper.class).insert(dto); } @Override - protected long projectId(DefaultIssue issue) { - return 10l; + protected void doUpdate(DbSession session, Date now, DefaultIssue issue) { + IssueDto dto = IssueDto.toDtoForUpdate(issue, 10l, now); + session.getMapper(IssueMapper.class).update(dto); } } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java index 4dbbcb85c2f..87e1a5c971e 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/UpdateConflictResolverTest.java @@ -47,9 +47,9 @@ public class UpdateConflictResolverTest { new IssueDto() .setKee("ABCDE") .setRuleId(10) - .setRuleKey_unit_test_only("squid", "AvoidCycles") + .setRuleKey("squid", "AvoidCycles") .setComponentId(100L) - .setComponentKey_unit_test_only("struts:org.apache.struts.Action") + .setComponentKey("struts:org.apache.struts.Action") .setLine(10) .setStatus(Issue.STATUS_OPEN) @@ -93,9 +93,9 @@ public class UpdateConflictResolverTest { IssueDto dbIssue = new IssueDto() .setKee("ABCDE") .setRuleId(10) - .setRuleKey_unit_test_only("squid", "AvoidCycles") + .setRuleKey("squid", "AvoidCycles") .setComponentId(100L) - .setComponentKey_unit_test_only("struts:org.apache.struts.Action") + .setComponentKey("struts:org.apache.struts.Action") .setLine(10) .setResolution(Issue.RESOLUTION_FALSE_POSITIVE) .setStatus(Issue.STATUS_RESOLVED) -- 2.39.5