From d27d70998b977ccdd57bcfeb41347e58af368b48 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 8 Nov 2013 15:59:29 +0100 Subject: [PATCH] SONAR-4776 Load issues changelog at the begin of each module analysis --- .../org/sonar/plugins/core/CorePlugin.java | 4 +- .../core/issue/InitialOpenIssuesSensor.java | 14 +- .../core/issue/InitialOpenIssuesStack.java | 44 +++- .../core/issue/IssueTrackingDecorator.java | 11 +- .../NewTechnicalDebtDecorator.java | 29 ++- .../issue/InitialOpenIssuesSensorTest.java | 15 +- .../issue/InitialOpenIssuesStackTest.java | 53 +++-- .../issue/IssueTrackingDecoratorTest.java | 22 +- .../NewTechnicalDebtDecoratorTest.java | 124 ++++------ .../core/issue/IssueChangelogFinder.java | 77 ------- .../sonar/core/issue/db/IssueChangeDao.java | 19 +- .../sonar/core/issue/db/IssueChangeDto.java | 3 +- .../sonar/core/issue/db/IssueChangeMapper.xml | 13 ++ .../core/issue/IssueChangelogFinderTest.java | 58 ----- .../core/issue/db/IssueChangeDaoTest.java | 52 +++-- .../select_issue_changelog_by_module.xml | 215 ++++++++++++++++++ ...g_by_module_are_sorted_by_created_date.xml | 64 ++++++ .../issue/db/IssueChangeDaoTest/shared.xml | 8 +- .../api/issue/internal/DefaultIssue.java | 30 ++- .../api/issue/internal/DefaultIssueTest.java | 9 + .../api/issue/internal/FieldDiffsTest.java | 16 ++ 21 files changed, 590 insertions(+), 290 deletions(-) delete mode 100644 sonar-core/src/main/java/org/sonar/core/issue/IssueChangelogFinder.java delete mode 100644 sonar-core/src/test/java/org/sonar/core/issue/IssueChangelogFinderTest.java create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module.xml create mode 100644 sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module_are_sorted_by_created_date.xml diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java index 265dcba65df..cd2f6d12bed 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java @@ -26,7 +26,6 @@ import org.sonar.api.config.PropertyDefinition; import org.sonar.api.resources.Java; import org.sonar.api.resources.Qualifiers; import org.sonar.batch.components.PastSnapshotFinder; -import org.sonar.core.issue.IssueChangelogFinder; import org.sonar.core.technicaldebt.TechnicalDebtCalculator; import org.sonar.core.technicaldebt.TechnicalDebtConverter; import org.sonar.core.timemachine.Periods; @@ -277,7 +276,6 @@ public final class CorePlugin extends SonarPlugin { UnresolvedIssuesStatusesWidget.class, IssueFilterWidget.class, org.sonar.api.issue.NoSonarFilter.class, - IssueChangelogFinder.class, // issue notifications SendIssueNotificationsPostJob.class, @@ -396,7 +394,7 @@ public final class CorePlugin extends SonarPlugin { .type(PropertyType.BOOLEAN) .category(CoreProperties.CATEGORY_SECURITY) .build() - ); + ); } } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java index cff5ec73080..1a333be0f5a 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java @@ -25,6 +25,8 @@ import org.apache.ibatis.session.ResultHandler; import org.sonar.api.batch.Sensor; import org.sonar.api.batch.SensorContext; import org.sonar.api.resources.Project; +import org.sonar.core.issue.db.IssueChangeDao; +import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.db.IssueDao; import org.sonar.core.issue.db.IssueDto; @@ -38,10 +40,12 @@ public class InitialOpenIssuesSensor implements Sensor { private final InitialOpenIssuesStack initialOpenIssuesStack; private final IssueDao issueDao; + private final IssueChangeDao issueChangeDao; - public InitialOpenIssuesSensor(InitialOpenIssuesStack initialOpenIssuesStack, IssueDao issueDao) { + public InitialOpenIssuesSensor(InitialOpenIssuesStack initialOpenIssuesStack, IssueDao issueDao, IssueChangeDao issueChangeDao) { this.initialOpenIssuesStack = initialOpenIssuesStack; this.issueDao = issueDao; + this.issueChangeDao = issueChangeDao; } @Override @@ -63,6 +67,14 @@ public class InitialOpenIssuesSensor implements Sensor { initialOpenIssuesStack.addIssue(dto); } }); + + issueChangeDao.selectChangelogOnNonClosedIssuesByModuleAndType(project.getId(), new ResultHandler() { + @Override + public void handleResult(ResultContext rc) { + IssueChangeDto dto = (IssueChangeDto) rc.getResultObject(); + initialOpenIssuesStack.addChangelog(dto); + } + }); } @Override diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java index d3a4c969c27..4646224dd03 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java @@ -20,44 +20,66 @@ package org.sonar.plugins.core.issue; -import com.google.common.collect.Lists; +import edu.emory.mathcs.backport.java.util.Collections; import org.sonar.api.BatchExtension; import org.sonar.api.batch.InstantiationStrategy; import org.sonar.batch.index.Cache; import org.sonar.batch.index.Caches; +import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.db.IssueDto; +import java.util.ArrayList; import java.util.List; +import static com.google.common.collect.Lists.newArrayList; + @InstantiationStrategy(InstantiationStrategy.PER_BATCH) public class InitialOpenIssuesStack implements BatchExtension { - private final Cache cache; + private final Cache issuesCache; + private final Cache> issuesChangelogCache; public InitialOpenIssuesStack(Caches caches) { - cache = caches.createCache("last-open-issues"); + issuesCache = caches.createCache("last-open-issues"); + issuesChangelogCache = caches.createCache("issues-changelog"); } public InitialOpenIssuesStack addIssue(IssueDto issueDto) { - cache.put(issueDto.getComponentKey(), issueDto.getKee(), issueDto); + issuesCache.put(issueDto.getComponentKey(), issueDto.getKee(), issueDto); return this; } - public List selectAndRemove(String componentKey) { - Iterable issues = cache.values(componentKey); - List result = Lists.newArrayList(); + public List selectAndRemoveIssues(String componentKey) { + Iterable issues = issuesCache.values(componentKey); + List result = newArrayList(); for (IssueDto issue : issues) { result.add(issue); } - cache.clear(componentKey); + issuesCache.clear(componentKey); return result; } - public Iterable selectAll() { - return cache.allValues(); + public Iterable selectAllIssues() { + return issuesCache.allValues(); + } + + public InitialOpenIssuesStack addChangelog(IssueChangeDto issueChangeDto) { + ArrayList changeDtos = issuesChangelogCache.get(issueChangeDto.getIssueKey()); + if (changeDtos == null) { + changeDtos = newArrayList(); + } + changeDtos.add(issueChangeDto); + issuesChangelogCache.put(issueChangeDto.getIssueKey(), changeDtos); + return this; + } + + public List selectChangelog(String issueKey) { + List changeDtos = issuesChangelogCache.get(issueKey); + return changeDtos != null ? changeDtos : Collections.emptyList(); } public void clear() { - cache.clearAll(); + issuesCache.clearAll(); + issuesChangelogCache.clearAll(); } } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index d244dce5501..90603e96ce7 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -43,6 +43,7 @@ import org.sonar.api.utils.KeyValueFormat; import org.sonar.batch.issue.IssueCache; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.IssueUpdater; +import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.issue.workflow.IssueWorkflow; @@ -110,7 +111,7 @@ public class IssueTrackingDecorator implements Decorator { // issues = all the issues created by rule engines during this module scan and not excluded by filters // all the issues that are not closed in db before starting this module scan, including manual issues - Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getEffectiveKey()); + Collection dbOpenIssues = initialOpenIssues.selectAndRemoveIssues(resource.getEffectiveKey()); SourceHashHolder sourceHashHolder = new SourceHashHolder(index, lastSnapshots, resource); @@ -160,6 +161,12 @@ public class IssueTrackingDecorator implements Decorator { issue.setAttributes(KeyValueFormat.parse(ref.getIssueAttributes())); } + // populate existing changelog + Collection issueChangeDtos = initialOpenIssues.selectChangelog(issue.key()); + for (IssueChangeDto issueChangeDto : issueChangeDtos) { + issue.addChange(issueChangeDto.toFieldDiffs()); + } + // fields to update with current values if (ref.isManualSeverity()) { issue.setManualSeverity(true); @@ -188,7 +195,7 @@ public class IssueTrackingDecorator implements Decorator { } private void addIssuesOnDeletedComponents(Collection issues) { - for (IssueDto deadDto : initialOpenIssues.selectAll()) { + for (IssueDto deadDto : initialOpenIssues.selectAllIssues()) { DefaultIssue dead = deadDto.toDefaultIssue(); updateUnmatchedIssue(dead, true); issues.add(dead); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecorator.java index 7e003efd5c6..3f22b68a80a 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecorator.java @@ -21,8 +21,6 @@ package org.sonar.plugins.core.technicaldebt; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; import org.apache.commons.lang.time.DateUtils; import org.sonar.api.batch.*; import org.sonar.api.component.ResourcePerspectives; @@ -39,7 +37,6 @@ import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; import org.sonar.batch.components.Period; import org.sonar.batch.components.TimeMachineConfiguration; -import org.sonar.core.issue.IssueChangelogFinder; import org.sonar.core.issue.IssueUpdater; import org.sonar.core.technicaldebt.TechnicalDebtConverter; @@ -58,14 +55,11 @@ public final class NewTechnicalDebtDecorator implements Decorator { private final ResourcePerspectives perspectives; private final TimeMachineConfiguration timeMachineConfiguration; - private final IssueChangelogFinder changelogFinder; private final TechnicalDebtConverter technicalDebtConverter; - public NewTechnicalDebtDecorator(ResourcePerspectives perspectives, TimeMachineConfiguration timeMachineConfiguration, IssueChangelogFinder changelogFinder, - TechnicalDebtConverter technicalDebtConverter) { + public NewTechnicalDebtDecorator(ResourcePerspectives perspectives, TimeMachineConfiguration timeMachineConfiguration, TechnicalDebtConverter technicalDebtConverter) { this.perspectives = perspectives; this.timeMachineConfiguration = timeMachineConfiguration; - this.changelogFinder = changelogFinder; this.technicalDebtConverter = technicalDebtConverter; } @@ -84,16 +78,15 @@ public final class NewTechnicalDebtDecorator implements Decorator { Issuable issuable = perspectives.as(Issuable.class, resource); if (issuable != null && shouldSaveNewMetrics(context)) { List issues = newArrayList(issuable.issues()); - Multimap changelogList = changelogFinder.findByIssues(issues); - saveMeasures(context, issues, changelogList); + saveMeasures(context, issues); } } - private void saveMeasures(DecoratorContext context, Collection issues, Multimap changelogList) { + private void saveMeasures(DecoratorContext context, Collection issues) { Measure measure = new Measure(CoreMetrics.NEW_TECHNICAL_DEBT); for (Period period : timeMachineConfiguration.periods()) { Date periodDate = period.getDate() != null ? DateUtils.addSeconds(period.getDate(), 1) : null; - double value = calculateNewTechnicalDebtValue(issues, changelogList, periodDate); + double value = calculateNewTechnicalDebtValue(issues, periodDate); Collection children = context.getChildrenMeasures(measure.getMetric()); double sum = MeasureUtils.sumOnVariation(true, period.getIndex(), children) + value; measure.setVariation(period.getIndex(), sum); @@ -101,11 +94,11 @@ public final class NewTechnicalDebtDecorator implements Decorator { context.saveMeasure(measure); } - private Double calculateNewTechnicalDebtValue(Collection issues, Multimap changelogList, @Nullable Date periodDate){ + private Double calculateNewTechnicalDebtValue(Collection issues, @Nullable Date periodDate) { double value = 0; for (Issue issue : issues) { WorkDayDuration currentTechnicalDebt = ((DefaultIssue) issue).technicalDebt(); - List technicalDebtChangelog = changesOnField(IssueUpdater.TECHNICAL_DEBT, changelogList.get(issue)); + List technicalDebtChangelog = changesOnField(IssueUpdater.TECHNICAL_DEBT, ((DefaultIssue) issue).changes()); if (technicalDebtChangelog.isEmpty()) { if (isAfter(issue.creationDate(), periodDate)) { value += technicalDebtConverter.toDays(currentTechnicalDebt); @@ -120,8 +113,14 @@ public final class NewTechnicalDebtDecorator implements Decorator { private Double calculateNewTechnicalDebtValueFromChangelog(WorkDayDuration currentTechnicalDebt, List technicalDebtChangelog, Date periodDate) { double currentTechnicalDebtValue = technicalDebtConverter.toDays(currentTechnicalDebt); - // Changelog have to be sorted from oldest to newest to catch oldest value just before the period date -> By default it's sorted from newest to oldest - for (FieldDiffs fieldDiffs : Lists.reverse(technicalDebtChangelog)) { + // Changelog have to be sorted from oldest to newest to catch oldest value just before the period date + Collections.sort(technicalDebtChangelog, new Comparator() { + @Override + public int compare(FieldDiffs fieldDiffs, FieldDiffs fieldDiffs2) { + return fieldDiffs.createdAt().compareTo(fieldDiffs2.createdAt()); + } + }); + for (FieldDiffs fieldDiffs : technicalDebtChangelog) { if (isAfter(fieldDiffs.createdAt(), periodDate)) { WorkDayDuration pastTechnicalDebt = newValue(IssueUpdater.TECHNICAL_DEBT, fieldDiffs); double pastTechnicalDebtValue = technicalDebtConverter.toDays(pastTechnicalDebt); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java index 814da65f220..e6c1b2c30d7 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java @@ -22,8 +22,8 @@ package org.sonar.plugins.core.issue; import org.apache.ibatis.session.ResultHandler; import org.junit.Test; import org.sonar.api.resources.Project; +import org.sonar.core.issue.db.IssueChangeDao; import org.sonar.core.issue.db.IssueDao; -import org.sonar.core.issue.db.IssueDto; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.any; @@ -35,7 +35,9 @@ public class InitialOpenIssuesSensorTest { InitialOpenIssuesStack stack = mock(InitialOpenIssuesStack.class); IssueDao issueDao = mock(IssueDao.class); - InitialOpenIssuesSensor sensor = new InitialOpenIssuesSensor(stack, issueDao); + IssueChangeDao issueChangeDao = mock(IssueChangeDao.class); + + InitialOpenIssuesSensor sensor = new InitialOpenIssuesSensor(stack, issueDao, issueChangeDao); @Test public void should_select_module_open_issues() { @@ -46,6 +48,15 @@ public class InitialOpenIssuesSensorTest { verify(issueDao).selectNonClosedIssuesByModule(eq(1), any(ResultHandler.class)); } + @Test + public void should_select_module_open_issues_changelog() { + Project project = new Project("key"); + project.setId(1); + sensor.analyse(project, null); + + verify(issueChangeDao).selectChangelogOnNonClosedIssuesByModuleAndType(eq(1), any(ResultHandler.class)); + } + @Test public void test_toString() throws Exception { assertThat(sensor.toString()).isEqualTo("InitialOpenIssuesSensor"); 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 c6fbeaf5cb5..3f9144bd824 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 @@ -31,6 +31,7 @@ import org.sonar.batch.bootstrap.BootstrapProperties; import org.sonar.batch.bootstrap.BootstrapSettings; import org.sonar.batch.bootstrap.TempFolderProvider; import org.sonar.batch.index.Caches; +import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.db.IssueDto; import java.io.IOException; @@ -69,48 +70,74 @@ public class InitialOpenIssuesStackTest { } @Test - public void should_get_and_remove() { + public void get_and_remove_issues() { IssueDto issueDto = new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1"); stack.addIssue(issueDto); - List issueDtos = stack.selectAndRemove("org.struts.Action"); + List issueDtos = stack.selectAndRemoveIssues("org.struts.Action"); assertThat(issueDtos).hasSize(1); assertThat(issueDtos.get(0).getKee()).isEqualTo("ISSUE-1"); - assertThat(stack.selectAll()).isEmpty(); + assertThat(stack.selectAllIssues()).isEmpty(); } @Test - public void should_get_and_remove_with_many_issues_on_same_resource() { + 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")); - List issueDtos = stack.selectAndRemove("org.struts.Action"); + List issueDtos = stack.selectAndRemoveIssues("org.struts.Action"); assertThat(issueDtos).hasSize(2); - assertThat(stack.selectAll()).isEmpty(); + assertThat(stack.selectAllIssues()).isEmpty(); } @Test - public void should_do_nothing_if_resource_not_found() { + 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")); - List issueDtos = stack.selectAndRemove("Other"); + List issueDtos = stack.selectAndRemoveIssues("Other"); assertThat(issueDtos).hasSize(0); - assertThat(stack.selectAll()).hasSize(1); + assertThat(stack.selectAllIssues()).hasSize(1); } @Test - public void should_clear() { + public void select_changelog() { + stack.addChangelog(new IssueChangeDto().setKey("CHANGE-1").setIssueKey("ISSUE-1")); + stack.addChangelog(new IssueChangeDto().setKey("CHANGE-2").setIssueKey("ISSUE-1")); + + List issueChangeDtos = stack.selectChangelog("ISSUE-1"); + assertThat(issueChangeDtos).hasSize(2); + assertThat(issueChangeDtos.get(0).getKey()).isEqualTo("CHANGE-1"); + assertThat(issueChangeDtos.get(1).getKey()).isEqualTo("CHANGE-2"); + } + + @Test + public void return_empty_changelog() { + assertThat(stack.selectChangelog("ISSUE-1")).isEmpty(); + } + + @Test + public void clear_issues() { stack.addIssue(new IssueDto().setComponentKey_unit_test_only("org.struts.Action").setKee("ISSUE-1")); - assertThat(stack.selectAll()).hasSize(1); + assertThat(stack.selectAllIssues()).hasSize(1); // issues are not removed - assertThat(stack.selectAll()).hasSize(1); + assertThat(stack.selectAllIssues()).hasSize(1); + + stack.clear(); + assertThat(stack.selectAllIssues()).hasSize(0); + } + + @Test + public void clear_issues_changelog() { + stack.addChangelog(new IssueChangeDto().setKey("CHANGE-1").setIssueKey("ISSUE-1")); + + assertThat(stack.selectChangelog("ISSUE-1")).hasSize(1); stack.clear(); - assertThat(stack.selectAll()).hasSize(0); + assertThat(stack.selectChangelog("ISSUE-1")).isNull(); } } 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 ddf259cacd8..9a573b3d62a 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 @@ -40,6 +40,7 @@ import org.sonar.api.rules.RuleFinder; import org.sonar.batch.issue.IssueCache; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.IssueUpdater; +import org.sonar.core.issue.db.IssueChangeDto; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.core.persistence.AbstractDaoTestCase; @@ -113,7 +114,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { // INPUT : one issue, no open issues during previous scan, no filtering when(issueCache.byComponent("struts:Action.java")).thenReturn(Arrays.asList(issue)); List dbIssues = Collections.emptyList(); - when(initialOpenIssues.selectAndRemove("struts:Action.java")).thenReturn(dbIssues); + when(initialOpenIssues.selectAndRemoveIssues("struts:Action.java")).thenReturn(dbIssues); decorator.doDecorate(file); @@ -486,7 +487,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { 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"); - when(initialOpenIssues.selectAll()).thenReturn(Arrays.asList(deadIssue)); + when(initialOpenIssues.selectAllIssues()).thenReturn(Arrays.asList(deadIssue)); decorator.doDecorate(project); @@ -537,4 +538,21 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { assertThat(issue.severity()).isEqualTo("MAJOR"); verify(updater, never()).setPastSeverity(eq(issue), anyString(), any(IssueChangeContext.class)); } + + @Test + 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") + .setLine(10).setMessage("Message").setEffortToFix(1.5).setTechnicalDebt(1L); + DefaultIssue issue = new DefaultIssue(); + + IssueTrackingResult trackingResult = mock(IssueTrackingResult.class); + when(trackingResult.matched()).thenReturn(newArrayList(issue)); + when(trackingResult.matching(eq(issue))).thenReturn(previousIssue); + decorator.mergeMatched(trackingResult); + + assertThat(issue.changes()).hasSize(1); + } + } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecoratorTest.java index 12fe60f2ff3..309dd77b584 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecoratorTest.java @@ -43,7 +43,6 @@ import org.sonar.api.resources.Resource; import org.sonar.api.test.IsMeasure; import org.sonar.batch.components.Period; import org.sonar.batch.components.TimeMachineConfiguration; -import org.sonar.core.issue.IssueChangelogFinder; import org.sonar.core.technicaldebt.TechnicalDebtConverter; import java.util.Calendar; @@ -51,7 +50,6 @@ import java.util.Date; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Matchers.anyCollection; import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.*; @@ -72,9 +70,6 @@ public class NewTechnicalDebtDecoratorTest { @Mock DecoratorContext context; - @Mock - IssueChangelogFinder changelogFinder; - @Mock TechnicalDebtConverter technicalDebtConverter; @@ -109,7 +104,7 @@ public class NewTechnicalDebtDecoratorTest { when(timeMachineConfiguration.periods()).thenReturn(newArrayList(new Period(1, fiveDaysAgo, fiveDaysAgo), new Period(2, tenDaysAgo, tenDaysAgo))); - decorator = new NewTechnicalDebtDecorator(perspectives, timeMachineConfiguration, changelogFinder, technicalDebtConverter); + decorator = new NewTechnicalDebtDecorator(perspectives, timeMachineConfiguration, technicalDebtConverter); } @Test @@ -124,17 +119,16 @@ public class NewTechnicalDebtDecoratorTest { @Test public void save_on_one_issue_with_changelog() { - Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt); + Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); when(issuable.issues()).thenReturn(newArrayList(issue)); Multimap changelogList = ArrayListMultimap.create(); - // Changes should be sorted from newest to oldest - changelogList.putAll(issue, newArrayList( - new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); decorator.decorate(resource, context); @@ -144,18 +138,15 @@ public class NewTechnicalDebtDecoratorTest { @Test public void save_on_one_issue_with_changelog_having_null_value() { - Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt); + Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", null, fiveDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, null).setCreatedAt(fourDaysAgo), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); when(issuable.issues()).thenReturn(newArrayList(issue)); - Multimap changelogList = ArrayListMultimap.create(); - // Changes should be sorted from newest to oldest - changelogList.putAll(issue, newArrayList( - new FieldDiffs().setDiff("technicalDebt", null, fiveDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, null).setCreatedAt(fourDaysAgo), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -166,18 +157,15 @@ public class NewTechnicalDebtDecoratorTest { public void save_on_one_issue_with_changelog_and_periods_have_no_dates() { when(timeMachineConfiguration.periods()).thenReturn(newArrayList(new Period(1, null, null), new Period(2, null, null))); - Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt); + Issue issue = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", null, fiveDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, null).setCreatedAt(fourDaysAgo), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); when(issuable.issues()).thenReturn(newArrayList(issue)); - Multimap changelogList = ArrayListMultimap.create(); - // Changes should be sorted from newest to oldest - changelogList.putAll(issue, newArrayList( - new FieldDiffs().setDiff("technicalDebt", null, fiveDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, null).setCreatedAt(fourDaysAgo), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -186,23 +174,21 @@ public class NewTechnicalDebtDecoratorTest { @Test public void save_on_issues_with_changelog() { - Issue issue1 = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt); - Issue issue2 = new DefaultIssue().setKey("B").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt); + Issue issue1 = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); + Issue issue2 = new DefaultIssue().setKey("B").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); when(issuable.issues()).thenReturn(newArrayList(issue1, issue2)); - Multimap changelogList = ArrayListMultimap.create(); - changelogList.putAll(issue1, newArrayList( - new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - changelogList.putAll(issue2, newArrayList( - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -215,9 +201,6 @@ public class NewTechnicalDebtDecoratorTest { (Issue) new DefaultIssue().setKey("A").setCreationDate(nineDaysAgo).setTechnicalDebt(fiveDaysDebt)) ); - Multimap changelogList = ArrayListMultimap.create(); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -230,9 +213,6 @@ public class NewTechnicalDebtDecoratorTest { (Issue) new DefaultIssue().setKey("A").setCreationDate(nineDaysAgo).setTechnicalDebt(null)) ); - Multimap changelogList = ArrayListMultimap.create(); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -247,9 +227,6 @@ public class NewTechnicalDebtDecoratorTest { (Issue) new DefaultIssue().setKey("A").setCreationDate(nineDaysAgo).setTechnicalDebt(fiveDaysDebt)) ); - Multimap changelogList = ArrayListMultimap.create(); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is null, period2 is null @@ -263,9 +240,6 @@ public class NewTechnicalDebtDecoratorTest { new DefaultIssue().setKey("B").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt) )); - Multimap changelogList = ArrayListMultimap.create(); - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo @@ -274,25 +248,23 @@ public class NewTechnicalDebtDecoratorTest { @Test public void save_on_issues_with_changelog_and_issues_without_changelog() { - Issue issue1 = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt); - Issue issue2 = new DefaultIssue().setKey("B").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt); + Issue issue1 = new DefaultIssue().setKey("A").setCreationDate(fiveDaysAgo).setTechnicalDebt(fiveDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); + Issue issue2 = new DefaultIssue().setKey("B").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt).setChanges( + newArrayList( + new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(rightNow), + new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) + ) + ); Issue issue3 = new DefaultIssue().setKey("C").setCreationDate(nineDaysAgo).setTechnicalDebt(fiveDaysDebt); Issue issue4 = new DefaultIssue().setKey("D").setCreationDate(fiveDaysAgo).setTechnicalDebt(twoDaysDebt); when(issuable.issues()).thenReturn(newArrayList(issue1, issue2, issue3, issue4)); - Multimap changelogList = ArrayListMultimap.create(); - changelogList.putAll(issue1, newArrayList( - new FieldDiffs().setDiff("technicalDebt", twoDaysDebt, fiveDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(fourDaysAgo), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - changelogList.putAll(issue2, newArrayList( - new FieldDiffs().setDiff("technicalDebt", oneDaysDebt, twoDaysDebt).setCreatedAt(rightNow), - new FieldDiffs().setDiff("technicalDebt", null, oneDaysDebt).setCreatedAt(nineDaysAgo) - )); - - when(changelogFinder.findByIssues(anyCollection())).thenReturn(changelogList); - decorator.decorate(resource, context); // remember : period1 is 5daysAgo, period2 is 10daysAgo diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueChangelogFinder.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueChangelogFinder.java deleted file mode 100644 index 717d1b6c0fe..00000000000 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueChangelogFinder.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2013 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -package org.sonar.core.issue; - -import com.google.common.base.Function; -import com.google.common.base.Predicate; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Iterables; -import com.google.common.collect.Multimap; -import org.sonar.api.BatchExtension; -import org.sonar.api.issue.Issue; -import org.sonar.api.issue.internal.FieldDiffs; -import org.sonar.core.issue.db.IssueChangeDao; - -import javax.annotation.Nullable; - -import java.util.Collection; -import java.util.List; - -import static com.google.common.collect.Lists.newArrayList; - -public class IssueChangelogFinder implements BatchExtension { - - private final IssueChangeDao dao; - - public IssueChangelogFinder(IssueChangeDao dao) { - this.dao = dao; - } - - public Multimap findByIssues(Collection issues) { - Multimap changelogList = ArrayListMultimap.create(); - - List changelog = dao.selectChangelogByIssues(issueKey(issues)); - for (FieldDiffs fieldDiff : changelog) { - changelogList.put(findIssueByKey(fieldDiff.issueKey(), issues), fieldDiff); - } - - return changelogList; - } - - private Collection issueKey(Collection issues) { - return newArrayList(Iterables.transform(issues, new Function() { - @Override - public String apply(Issue input) { - return input.key(); - } - })); - } - - private Issue findIssueByKey(final String issueKey, Collection issues){ - return Iterables.find(issues, new Predicate() { - @Override - public boolean apply(@Nullable Issue input) { - return issueKey.equals(input.key()); - } - }); - } - -} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java index f40ab874b1b..2db58779d1b 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java @@ -21,6 +21,7 @@ package org.sonar.core.issue.db; import com.google.common.collect.Lists; +import org.apache.ibatis.session.ResultHandler; import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; @@ -30,12 +31,10 @@ import org.sonar.core.persistence.MyBatis; import javax.annotation.CheckForNull; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; +import java.util.*; import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; /** * @since 3.6 @@ -69,14 +68,14 @@ public class IssueChangeDao implements BatchComponent, ServerComponent { } } - public List selectChangelogByIssues(Collection issueKeys) { + public void selectChangelogOnNonClosedIssuesByModuleAndType(Integer componentId, ResultHandler handler) { SqlSession session = mybatis.openSession(); try { - List result = Lists.newArrayList(); - for (IssueChangeDto dto : selectByIssuesAndType(session, issueKeys, IssueChangeDto.TYPE_FIELD_CHANGE)) { - result.add(dto.toFieldDiffs()); - } - return result; + Map params = newHashMap(); + params.put("componentId", componentId); + params.put("changeType", IssueChangeDto.TYPE_FIELD_CHANGE); + session.select("org.sonar.core.issue.db.IssueChangeMapper.selectChangelogOnNonClosedIssuesByModuleAndType", params, handler); + } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDto.java index 5687251f1a3..646fb02b330 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDto.java @@ -27,12 +27,13 @@ import org.sonar.api.issue.internal.FieldDiffs; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import java.io.Serializable; import java.util.Date; /** * @since 3.6 */ -public final class IssueChangeDto { +public final class IssueChangeDto implements Serializable { public static final String TYPE_FIELD_CHANGE = "diff"; public static final String TYPE_COMMENT = "comment"; diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml index 7e3cf4cb6f9..ce9cb26ce66 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml @@ -39,6 +39,19 @@ order by c.created_at + +