]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4776 Load issues changelog at the begin of each module analysis
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 8 Nov 2013 14:59:29 +0000 (15:59 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 8 Nov 2013 14:59:29 +0000 (15:59 +0100)
21 files changed:
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesStack.java
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecorator.java
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesStackTest.java
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/NewTechnicalDebtDecoratorTest.java
sonar-core/src/main/java/org/sonar/core/issue/IssueChangelogFinder.java [deleted file]
sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDao.java
sonar-core/src/main/java/org/sonar/core/issue/db/IssueChangeDto.java
sonar-core/src/main/resources/org/sonar/core/issue/db/IssueChangeMapper.xml
sonar-core/src/test/java/org/sonar/core/issue/IssueChangelogFinderTest.java [deleted file]
sonar-core/src/test/java/org/sonar/core/issue/db/IssueChangeDaoTest.java
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module.xml [new file with mode: 0644]
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module_are_sorted_by_created_date.xml [new file with mode: 0644]
sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/shared.xml
sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/DefaultIssue.java
sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/DefaultIssueTest.java
sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/FieldDiffsTest.java

index 265dcba65df2ee1d593e95e040e4ecff84b8df79..cd2f6d12bed9402fed7a0f0ca07182c66d0f86b2 100644 (file)
@@ -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()
-      );
+    );
   }
 
 }
index cff5ec73080a1bf0337b985b486595268fcabce2..1a333be0f5ab62d0ad2d4151ad27b8c00b09c311 100644 (file)
@@ -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
index d3a4c969c27b2f8f0eed3f5b3237e0e822d53f38..4646224dd03822debd0b43bfbe4466469b2799b7 100644 (file)
 
 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<String, IssueDto> cache;
+  private final Cache<String, IssueDto> issuesCache;
+  private final Cache<String, ArrayList<IssueChangeDto>> 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<IssueDto> selectAndRemove(String componentKey) {
-    Iterable<IssueDto> issues = cache.values(componentKey);
-    List<IssueDto> result = Lists.newArrayList();
+  public List<IssueDto> selectAndRemoveIssues(String componentKey) {
+    Iterable<IssueDto> issues = issuesCache.values(componentKey);
+    List<IssueDto> result = newArrayList();
     for (IssueDto issue : issues) {
       result.add(issue);
     }
-    cache.clear(componentKey);
+    issuesCache.clear(componentKey);
     return result;
   }
 
-  public Iterable<IssueDto> selectAll() {
-    return cache.allValues();
+  public Iterable<IssueDto> selectAllIssues() {
+    return issuesCache.allValues();
+  }
+
+  public InitialOpenIssuesStack addChangelog(IssueChangeDto issueChangeDto) {
+    ArrayList<IssueChangeDto> changeDtos = issuesChangelogCache.get(issueChangeDto.getIssueKey());
+    if (changeDtos == null) {
+      changeDtos = newArrayList();
+    }
+    changeDtos.add(issueChangeDto);
+    issuesChangelogCache.put(issueChangeDto.getIssueKey(), changeDtos);
+    return this;
+  }
+
+  public List<IssueChangeDto> selectChangelog(String issueKey) {
+    List<IssueChangeDto> changeDtos = issuesChangelogCache.get(issueKey);
+    return changeDtos != null ? changeDtos : Collections.emptyList();
   }
 
   public void clear() {
-    cache.clearAll();
+    issuesCache.clearAll();
+    issuesChangelogCache.clearAll();
   }
 }
index d244dce55016dd2f7565dd7531f202731edb7d88..90603e96ce7ba1c1ebe3684a8e322a7b14338d11 100644 (file)
@@ -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<IssueDto> dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getEffectiveKey());
+    Collection<IssueDto> 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<IssueChangeDto> 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<DefaultIssue> issues) {
-    for (IssueDto deadDto : initialOpenIssues.selectAll()) {
+    for (IssueDto deadDto : initialOpenIssues.selectAllIssues()) {
       DefaultIssue dead = deadDto.toDefaultIssue();
       updateUnmatchedIssue(dead, true);
       issues.add(dead);
index 7e003efd5c6eda5172b14a2bf4aa64f419261fa8..3f22b68a80a654ebb59b950d691a4d69c2b907a1 100644 (file)
@@ -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<Issue> issues = newArrayList(issuable.issues());
-      Multimap<Issue, FieldDiffs> changelogList = changelogFinder.findByIssues(issues);
-      saveMeasures(context, issues, changelogList);
+      saveMeasures(context, issues);
     }
   }
 
-  private void saveMeasures(DecoratorContext context, Collection<Issue> issues, Multimap<Issue, FieldDiffs> changelogList) {
+  private void saveMeasures(DecoratorContext context, Collection<Issue> 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<Measure> 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<Issue> issues, Multimap<Issue, FieldDiffs> changelogList, @Nullable Date periodDate){
+  private Double calculateNewTechnicalDebtValue(Collection<Issue> issues, @Nullable Date periodDate) {
     double value = 0;
     for (Issue issue : issues) {
       WorkDayDuration currentTechnicalDebt = ((DefaultIssue) issue).technicalDebt();
-      List<FieldDiffs> technicalDebtChangelog = changesOnField(IssueUpdater.TECHNICAL_DEBT, changelogList.get(issue));
+      List<FieldDiffs> 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<FieldDiffs> 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<FieldDiffs>() {
+      @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);
index 814da65f220e315c3b1a55614cca7785777edee3..e6c1b2c30d7163638d15acc4c7febef4611792d5 100644 (file)
@@ -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");
index c6fbeaf5cb54ce741ac29449c72f7dd73fa61fc3..3f9144bd82497e7fd191ff67b5a13efe01a078ba 100644 (file)
@@ -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<IssueDto> issueDtos = stack.selectAndRemove("org.struts.Action");
+    List<IssueDto> 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<IssueDto> issueDtos = stack.selectAndRemove("org.struts.Action");
+    List<IssueDto> 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<IssueDto> issueDtos = stack.selectAndRemove("Other");
+    List<IssueDto> 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<IssueChangeDto> 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();
   }
 }
index ddf259cacd801d31dfe7dbb4fa5b0a9981870e3f..9a573b3d62aae5993373f857299ef2409afd69d3 100644 (file)
@@ -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<IssueDto> 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);
+  }
+
 }
index 12fe60f2ff38048495c4e6252531262584e0efad..309dd77b5849900446e1b6cea6f6e6300c07e086 100644 (file)
@@ -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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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<Issue, FieldDiffs> 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 (file)
index 717d1b6..0000000
+++ /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<Issue, FieldDiffs> findByIssues(Collection<Issue> issues) {
-    Multimap<Issue, FieldDiffs> changelogList = ArrayListMultimap.create();
-
-    List<FieldDiffs> changelog = dao.selectChangelogByIssues(issueKey(issues));
-    for (FieldDiffs fieldDiff : changelog) {
-      changelogList.put(findIssueByKey(fieldDiff.issueKey(), issues), fieldDiff);
-    }
-
-    return changelogList;
-  }
-
-  private Collection<String> issueKey(Collection<Issue> issues) {
-    return newArrayList(Iterables.transform(issues, new Function<Issue, String>() {
-      @Override
-      public String apply(Issue input) {
-        return input.key();
-      }
-    }));
-  }
-
-  private Issue findIssueByKey(final String issueKey, Collection<Issue> issues){
-    return Iterables.find(issues, new Predicate<Issue>() {
-      @Override
-      public boolean apply(@Nullable Issue input) {
-        return issueKey.equals(input.key());
-      }
-    });
-  }
-
-}
index f40ab874b1b650413fd92dbc89650d1fa264853f..2db58779d1b30f1076a9659dd6bbee83f83794af 100644 (file)
@@ -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<FieldDiffs> selectChangelogByIssues(Collection<String> issueKeys) {
+  public void selectChangelogOnNonClosedIssuesByModuleAndType(Integer componentId, ResultHandler handler) {
     SqlSession session = mybatis.openSession();
     try {
-      List<FieldDiffs> result = Lists.newArrayList();
-      for (IssueChangeDto dto : selectByIssuesAndType(session, issueKeys, IssueChangeDto.TYPE_FIELD_CHANGE)) {
-        result.add(dto.toFieldDiffs());
-      }
-      return result;
+      Map<String, Object> 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);
     }
index 5687251f1a386ed92717ff1583bfc6aec22e3706..646fb02b330fb9779a93816b8ef93a9a8f92208f 100644 (file)
@@ -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";
index 7e3cf4cb6f93473c8ea546e29781037640f3cb99..ce9cb26ce66f100a34acb23ae8e7268564951340 100644 (file)
     order by c.created_at
   </select>
 
+  <select id="selectChangelogOnNonClosedIssuesByModuleAndType" parameterType="map" resultType="IssueChange">
+    select
+    <include refid="issueChangeColumns"/>
+    from issue_changes c
+    inner join issues i on i.kee = c.issue_key
+    inner join (select p.id,p.kee from projects p where (p.root_id=#{componentId} and p.qualifier &lt;&gt; 'BRC') or (p.id=#{componentId})) p on p.id=i.component_id
+    <where>
+      and c.change_type=#{changeType}
+      and i.status &lt;&gt; 'CLOSED'
+    </where>
+    order by c.created_at asc
+  </select>
+
   <select id="selectByKeyAndType" parameterType="map" resultType="IssueChange">
     select
     <include refid="issueChangeColumns"/>
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueChangelogFinderTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueChangelogFinderTest.java
deleted file mode 100644 (file)
index fc975e4..0000000
+++ /dev/null
@@ -1,58 +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.collect.Multimap;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
-import org.sonar.api.issue.Issue;
-import org.sonar.api.issue.internal.DefaultIssue;
-import org.sonar.api.issue.internal.FieldDiffs;
-import org.sonar.core.issue.db.IssueChangeDao;
-
-import static com.google.common.collect.Lists.newArrayList;
-import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.when;
-
-@RunWith(MockitoJUnitRunner.class)
-public class IssueChangelogFinderTest {
-
-  @Mock
-  private IssueChangeDao dao;
-
-  private IssueChangelogFinder finder;
-
-  @Before
-  public void before() {
-    finder = new IssueChangelogFinder(dao);
-  }
-
-  @Test
-  public void find_by_issues() {
-    when(dao.selectChangelogByIssues(newArrayList("ABCD"))).thenReturn(newArrayList(new FieldDiffs().setIssueKey("ABCD")));
-
-    Multimap<Issue, FieldDiffs> result = finder.findByIssues(newArrayList((Issue) new DefaultIssue().setKey("ABCD")));
-    assertThat(result.entries()).hasSize(1);
-  }
-}
index 47227227cd1e9f22708872b1728b5fde1d8c8d53..7ab6b3960717cbb617138cc165ed88361dc9e166 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.core.issue.db;
 
+import org.apache.ibatis.executor.result.DefaultResultHandler;
 import org.apache.ibatis.session.SqlSession;
 import org.junit.Before;
 import org.junit.Test;
@@ -106,20 +107,45 @@ public class IssueChangeDaoTest extends AbstractDaoTestCase {
   }
 
   @Test
-  public void select_issue_changelog_from_issue_keys() {
-    setupData("shared");
+  public void select_issue_changelog_by_module() {
+    setupData("select_issue_changelog_by_module");
+
+    // 400 is a non-root module, we should find 2 + 1 changelog from classes and one on itself
+    DefaultResultHandler handler = new DefaultResultHandler();
+    dao.selectChangelogOnNonClosedIssuesByModuleAndType(400, handler);
+    assertThat(handler.getResultList()).hasSize(4);
+
+    IssueChangeDto issueChangeDto = (IssueChangeDto) handler.getResultList().get(0);
+    assertThat(issueChangeDto.getId()).isNotNull();
+    assertThat(issueChangeDto.getKey()).isNotNull();
+    assertThat(issueChangeDto.getIssueKey()).isNotNull();
+    assertThat(issueChangeDto.getUserLogin()).isNotNull();
+    assertThat(issueChangeDto.getChangeType()).isNotNull();
+    assertThat(issueChangeDto.getChangeData()).isNotNull();
+    assertThat(issueChangeDto.getCreatedAt()).isNotNull();
+    assertThat(issueChangeDto.getUpdatedAt()).isNotNull();
+
+    for (Object changeDtoObject : handler.getResultList()) {
+      IssueChangeDto changeDto = (IssueChangeDto) changeDtoObject;
+      assertThat(changeDto.getChangeType()).isEqualTo(IssueChangeDto.TYPE_FIELD_CHANGE);
+    }
 
-    List<FieldDiffs> result = dao.selectChangelogByIssues(newArrayList("1001"));
-    assertThat(result).hasSize(2);
-    assertThat(result.get(0).diffs()).hasSize(1);
-    assertThat(result.get(0).issueKey()).isEqualTo("1001");
-    assertThat(result.get(0).get("actionPlan").newValue()).isEqualTo("1.1");
-    assertThat(result.get(0).get("actionPlan").oldValue()).isEqualTo("1.0");
-
-    assertThat(result.get(1).diffs()).hasSize(1);
-    assertThat(result.get(1).issueKey()).isEqualTo("1001");
-    assertThat(result.get(1).get("severity").newValue()).isEqualTo("BLOCKER");
-    assertThat(result.get(1).get("severity").oldValue()).isEqualTo("MAJOR");
+    // 399 is the root module, we should only find 1 changelog on itself
+    handler = new DefaultResultHandler();
+    dao.selectChangelogOnNonClosedIssuesByModuleAndType(399, handler);
+    assertThat(handler.getResultList()).hasSize(1);
+  }
+
+  @Test
+  public void select_issue_changelog_by_module_should_be_sorted_by_created_date() {
+    setupData("select_issue_changelog_by_module_are_sorted_by_created_date");
+
+    DefaultResultHandler handler = new DefaultResultHandler();
+    dao.selectChangelogOnNonClosedIssuesByModuleAndType(399, handler);
+    assertThat(handler.getResultList()).hasSize(3);
+    assertThat(((IssueChangeDto) handler.getResultList().get(0)).getId()).isEqualTo(1001);
+    assertThat(((IssueChangeDto) handler.getResultList().get(1)).getId()).isEqualTo(1002);
+    assertThat(((IssueChangeDto) handler.getResultList().get(2)).getId()).isEqualTo(1000);
   }
 
   @Test
diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module.xml
new file mode 100644 (file)
index 0000000..85d5f5b
--- /dev/null
@@ -0,0 +1,215 @@
+<dataset>
+
+  <projects id="399" kee="struts" root_id="[null]" qualifier="TRK" scope="PRJ" />
+  <projects id="400" kee="struts-core" root_id="399" qualifier="BRC" scope="PRJ" />
+  <projects id="401" kee="Action.java" root_id="400" qualifier="CLA" scope="PRJ" />
+  <projects id="402" kee="Filter.java" root_id="400" qualifier="CLA" scope="PRJ" />
+
+  <!-- Open Issue on a file -->
+  <issues
+      id="100"
+      kee="100"
+      component_id="401"
+      root_component_id="399"
+      rule_id="500"
+      severity="BLOCKER"
+      manual_severity="[false]"
+      message="[null]"
+      line="200"
+      effort_to_fix="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="user"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-16"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="100"
+      kee="100"
+      issue_key="100"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <issue_changes
+      id="1000"
+      kee="1000"
+      issue_key="100"
+      user_login="arthur"
+      change_type="diff"
+      change_data="actionPlan=1.0|1.1"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <issue_changes
+      id="1001"
+      kee="1001"
+      issue_key="100"
+      user_login="arthur"
+      change_type="comment"
+      change_data="recent comment"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <!-- Open Issue on a file -->
+  <issues
+      id="101"
+      kee="101"
+      component_id="402"
+      root_component_id="399"
+      rule_id="501"
+      severity="MAJOR"
+      manual_severity="[false]"
+      message="[null]"
+      line="120"
+      effort_to_fix="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-10"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="101"
+      kee="101"
+      issue_key="101"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <!-- Closed Issue on a file -->
+  <issues
+      id="102"
+      kee="102"
+      component_id="402"
+      root_component_id="399"
+      rule_id="501"
+      severity="MAJOR"
+      manual_severity="[false]"
+      message="[null]"
+      line="120"
+      effort_to_fix="[null]"
+      status="CLOSED"
+      resolution="FIXED"
+      checksum="[null]"
+      reporter="[null]"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-10"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="102"
+      kee="102"
+      issue_key="102"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <!-- Open Issue on a sub module -->
+  <issues
+      id="103"
+      kee="103"
+      component_id="400"
+      root_component_id="399"
+      rule_id="501"
+      severity="MAJOR"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-10"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="103"
+      kee="103"
+      issue_key="103"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <!-- Open Issue on a root module -->
+  <issues
+      id="104"
+      kee="104"
+      component_id="399"
+      root_component_id="399"
+      rule_id="501"
+      severity="MAJOR"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-10"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="104"
+      kee="104"
+      issue_key="104"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+</dataset>
diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module_are_sorted_by_created_date.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueChangeDaoTest/select_issue_changelog_by_module_are_sorted_by_created_date.xml
new file mode 100644 (file)
index 0000000..11ad40b
--- /dev/null
@@ -0,0 +1,64 @@
+<dataset>
+
+  <projects id="399" kee="struts" root_id="[null]" qualifier="TRK" scope="PRJ" />
+
+  <!-- Open Issue on a root module -->
+  <issues
+      id="104"
+      kee="104"
+      component_id="399"
+      root_component_id="399"
+      rule_id="501"
+      severity="MAJOR"
+      manual_severity="[false]"
+      message="[null]"
+      line="[null]"
+      effort_to_fix="[null]"
+      status="OPEN"
+      resolution="[null]"
+      checksum="[null]"
+      reporter="[null]"
+      assignee="user"
+      author_login="[null]"
+      issue_attributes="[null]"
+      issue_creation_date="2013-04-16"
+      issue_update_date="2013-04-16"
+      issue_close_date="2013-04-16"
+      created_at="2013-04-10"
+      updated_at="[null]"
+      />
+
+  <issue_changes
+      id="1000"
+      kee="1000"
+      issue_key="104"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-18"
+      updated_at="2013-04-18"
+      />
+
+  <issue_changes
+      id="1001"
+      kee="1001"
+      issue_key="104"
+      user_login="arthur"
+      change_type="diff"
+      change_data="severity=MAJOR|BLOCKER"
+      created_at="2013-04-16"
+      updated_at="2013-04-16"
+      />
+
+  <issue_changes
+      id="1002"
+      kee="1002"
+      issue_key="104"
+      user_login="arthur"
+      change_type="diff"
+      change_data="actionPlan=1.0|1.1"
+      created_at="2013-04-17"
+      updated_at="2013-04-17"
+      />
+
+</dataset>
index 5d6f87b50bd02ce2ebc703bf512919fdca19e65e..6f960b43d3c22d601672605052ae8317bba6cd7f 100644 (file)
@@ -40,8 +40,8 @@
       user_login="arthur"
       change_type="diff"
       change_data="actionPlan=1.0|1.1"
-      created_at="2013-02-01"
-      updated_at="2013-02-01"
+      created_at="2013-02-02"
+      updated_at="2013-02-02"
       />
 
   <issue_changes
@@ -51,8 +51,8 @@
       user_login="henry"
       change_type="diff"
       change_data="severity=MAJOR|BLOCKER"
-      created_at="2013-02-02"
-      updated_at="2013-02-02"
+      created_at="2013-02-01"
+      updated_at="2013-02-01"
       />
 
 </dataset>
index bae84f2e21e84a754cc14c650ff9502c346ca6a4..ea505af1b48fe79a73ecdb7de052db48e9050b0f 100644 (file)
@@ -24,7 +24,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ToStringBuilder;
@@ -41,6 +40,8 @@ import javax.annotation.Nullable;
 import java.io.Serializable;
 import java.util.*;
 
+import static com.google.common.collect.Lists.newArrayList;
+
 /**
  * PLUGINS MUST NOT BE USED THIS CLASS, EXCEPT FOR UNIT TESTING.
  *
@@ -78,6 +79,9 @@ public class DefaultIssue implements Issue {
   // Current changes
   private FieldDiffs currentChange = null;
 
+  // all changes
+  private List<FieldDiffs> changes = null;
+
   // true if the the issue did not exist in the previous scan.
   private boolean isNew = true;
 
@@ -393,17 +397,39 @@ public class DefaultIssue implements Issue {
       }
       currentChange.setDiff(field, oldValue, newValue);
     }
+    addChange(currentChange);
     return this;
   }
 
+
   @CheckForNull
   public FieldDiffs currentChange() {
     return currentChange;
   }
 
+  public DefaultIssue addChange(FieldDiffs change) {
+    if (changes == null) {
+      changes = newArrayList();
+    }
+    changes.add(change);
+    return this;
+  }
+
+  public DefaultIssue setChanges(List<FieldDiffs> changes) {
+    this.changes = changes;
+    return this;
+  }
+
+  public List<FieldDiffs> changes() {
+    if (changes == null) {
+      return Collections.emptyList();
+    }
+    return ImmutableList.copyOf(changes);
+  }
+
   public DefaultIssue addComment(DefaultIssueComment comment) {
     if (comments == null) {
-      comments = Lists.newArrayList();
+      comments = newArrayList();
     }
     comments.add(comment);
     return this;
index 2cd8cdbdfda789d0758ebc795050079ce89b37ff..0e464d40ec5940940fc544a339d0f26aac0b2d82 100644 (file)
@@ -32,6 +32,7 @@ import java.util.List;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 import static org.fest.assertions.MapAssert.entry;
+import static org.mockito.Mockito.mock;
 
 public class DefaultIssueTest {
 
@@ -208,4 +209,12 @@ public class DefaultIssueTest {
       fail("Unexpected exception: " + e);
     }
   }
+
+  @Test
+  public void all_changes_contain_current_change() throws Exception {
+    IssueChangeContext issueChangeContext = mock(IssueChangeContext.class);
+    DefaultIssue issue = new DefaultIssue().setKey("AAA").setFieldChange(issueChangeContext, "actionPlan", "1.0", "1.1");
+
+    assertThat(issue.changes()).hasSize(1);
+  }
 }
index e6f31fafb69c1ce6ee70579f2b87eae8240e6023..6a07ac3cffb19fa1f3899c18e26a3e976b5e81dc 100644 (file)
@@ -48,6 +48,22 @@ public class FieldDiffsTest {
     assertThat(diff.newValue()).isEqualTo("FIXED");
   }
 
+  @Test
+  public void test_diff_by_key() throws Exception {
+    diffs.setDiff("severity", "BLOCKER", "INFO");
+    diffs.setDiff("resolution", "OPEN", "FIXED");
+
+    assertThat(diffs.diffs()).hasSize(2);
+
+    FieldDiffs.Diff diff = diffs.diffs().get("severity");
+    assertThat(diff.oldValue()).isEqualTo("BLOCKER");
+    assertThat(diff.newValue()).isEqualTo("INFO");
+
+    diff = diffs.diffs().get("resolution");
+    assertThat(diff.oldValue()).isEqualTo("OPEN");
+    assertThat(diff.newValue()).isEqualTo("FIXED");
+  }
+
   @Test
   public void should_keep_old_value() throws Exception {
     diffs.setDiff("severity", "BLOCKER", "INFO");