Reduce memory usage of the PostAnalysisIssueVisitor

The collection of `DefaultIssue` objects hold by the `PostAnalysisIssueVisitor` can take a significant amount of memory in the CE process heap when there are numerous issues. A `DefaultIssue` alone can weigh several 10s of kilobytes.

This change introduces a `PostAnalysisIssueVisitor.LightIssue` that contains a subset of the fields from `DefaultIssue` that are required during Pull Request decoration, thereby allowing the full `DefaultIssue` instances not to be retained on the heap.

.gitignore

@@ -11,5 +11,11 @@
 /.idea/
 *.iml
 
+# Eclipse
+/.classpath
+/.project
+/.settings/
+/bin/
+
 #Project libs
-/sonarqube-lib/
+/sonarqube-lib/

build.gradle

@@ -68,6 +68,7 @@ dependencies {
  compile('io.aexp.nodes.graphql:nodes:0.5.0') {
  exclude group: 'com.fasterxml.jackson.core'
  }
+ compileOnly 'com.google.code.findbugs:jsr305:3.0.2'
 }
 
 

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetails.java

@@ -45,7 +45,6 @@
 import org.sonar.ce.task.projectanalysis.measure.Measure;
 import org.sonar.ce.task.projectanalysis.measure.MeasureRepository;
 import org.sonar.ce.task.projectanalysis.metric.MetricRepository;
-import org.sonar.core.issue.DefaultIssue;
 import org.sonar.server.measure.Rating;
 
 import java.io.UnsupportedEncodingException;
@@ -201,7 +200,7 @@ public String createAnalysisSummary(FormatterFactory formatterFactory) {
  }
 
  public String createAnalysisIssueSummary(PostAnalysisIssueVisitor.ComponentIssue componentIssue, FormatterFactory formatterFactory) {
- final DefaultIssue issue = componentIssue.getIssue();
+ final PostAnalysisIssueVisitor.LightIssue issue = componentIssue.getIssue();
 
  String baseImageUrl = getBaseImageUrl();
 

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PostAnalysisIssueVisitor.java

@@ -18,13 +18,17 @@
  */
 package com.github.mc1arke.sonarqube.plugin.ce.pullrequest;
 
+import org.sonar.api.rules.RuleType;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.issue.IssueVisitor;
 import org.sonar.core.issue.DefaultIssue;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
+
+import javax.annotation.CheckForNull;
 
 public class PostAnalysisIssueVisitor extends IssueVisitor {
 
@@ -42,20 +46,115 @@ public List<ComponentIssue> getIssues() {
  public static class ComponentIssue {
 
  private final Component component;
- private final DefaultIssue issue;
+ private final LightIssue issue;
 
  ComponentIssue(Component component, DefaultIssue issue) {
  super();
  this.component = component;
- this.issue = issue;
+ this.issue = (issue != null) ? new LightIssue(issue) : null;
+ // the null test is to please PostAnalysisIssueVisitorTest.checkAllIssuesCollected()
  }
 
  public Component getComponent() {
  return component;
  }
 
- public DefaultIssue getIssue() {
+ public LightIssue getIssue() {
  return issue;
  }
  }
+
+ /**
+ * A simple bean for holding the useful bits of a #{@link DefaultIssue}.
+ * <br>
+ * It presents a subset of the #{@link DefaultIssue} interface, hence the inconsistent getters names,
+ * and CheckForNull annotations.
+ */
+ public static class LightIssue {
+
+ private final Long effortInMinutes;
+ private final String key;
+ private final Integer line;
+ private final String message;
+ private final String resolution;
+ private final String severity;
+ private final String status;
+ private final RuleType type;
+
+ private LightIssue(DefaultIssue issue) {
+ this.effortInMinutes = issue.effortInMinutes();
+ this.key = issue.key();
+ this.line = issue.getLine();
+ this.message = issue.getMessage();
+ this.resolution = issue.resolution();
+ this.severity = issue.severity();
+ this.status = issue.status();
+ this.type = issue.type();
+ }
+
+ @CheckForNull
+ public Long effortInMinutes() {
+ return effortInMinutes;
+ }
+
+ public String key() {
+ return key;
+ }
+
+ @CheckForNull
+ public Integer getLine() {
+ return line;
+ }
+
+ @CheckForNull
+ public String getMessage() {
+ return message;
+ }
+
+ @CheckForNull
+ public String resolution() {
+ return resolution;
+ }
+
+ public String severity() {
+ return severity;
+ }
+
+ public String getStatus() {
+ return status;
+ }
+
+ public String status() {
+ return status;
+ }
+
+ public RuleType type() {
+ return type;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(effortInMinutes, key, line, message, resolution, severity, status, type);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ LightIssue other = (LightIssue) obj;
+ return Objects.equals(effortInMinutes, other.effortInMinutes)
+ && Objects.equals(key, other.key)
+ && Objects.equals(line, other.line)
+ && Objects.equals(message, other.message)
+ && Objects.equals(resolution, other.resolution)
+ && Objects.equals(severity, other.severity)
+ && Objects.equals(status, other.status)
+ && type == other.type;
+ }
+
+ }
 }

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetailsTest.java

@@ -45,7 +45,6 @@
 import org.sonar.ce.task.projectanalysis.measure.MeasureRepository;
 import org.sonar.ce.task.projectanalysis.metric.Metric;
 import org.sonar.ce.task.projectanalysis.metric.MetricRepository;
-import org.sonar.core.issue.DefaultIssue;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -193,26 +192,26 @@ public void testCreateAnalysisSummary() {
  doReturn(treeRootHolder).when(measuresHolder).getTreeRootHolder();
 
  PostAnalysisIssueVisitor postAnalysisIssueVisitor = mock(PostAnalysisIssueVisitor.class);
- DefaultIssue issue1 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue1 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_CLOSED).when(issue1).status();
 
- DefaultIssue issue2 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue2 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue2).status();
  doReturn(RuleType.BUG).when(issue2).type();
 
- DefaultIssue issue3 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue3 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue3).status();
  doReturn(RuleType.SECURITY_HOTSPOT).when(issue3).type();
 
- DefaultIssue issue4 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue4 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue4).status();
  doReturn(RuleType.CODE_SMELL).when(issue4).type();
 
- DefaultIssue issue5 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue5 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue5).status();
  doReturn(RuleType.VULNERABILITY).when(issue5).type();
 
- DefaultIssue issue6 = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue6 = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue6).status();
  doReturn(RuleType.BUG).when(issue6).type();
 
@@ -469,7 +468,7 @@ public void testCreateAnalysisSummary3() {
  AnalysisDetails.MeasuresHolder measuresHolder = mock(AnalysisDetails.MeasuresHolder.class);
  doReturn(treeRootHolder).when(measuresHolder).getTreeRootHolder();
 
- DefaultIssue issue = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue issue = mock(PostAnalysisIssueVisitor.LightIssue.class);
  doReturn(Issue.STATUS_OPEN).when(issue).status();
  doReturn(RuleType.BUG).when(issue).type();
  PostAnalysisIssueVisitor postAnalysisIssueVisitor = mock(PostAnalysisIssueVisitor.class);

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PostAnalysisIssueVisitorTest.java

@@ -19,17 +19,32 @@
 package com.github.mc1arke.sonarqube.plugin.ce.pullrequest;
 
 import org.junit.Test;
+import org.sonar.api.rules.RuleType;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.core.issue.DefaultIssue;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 public class PostAnalysisIssueVisitorTest {
 
+ private static final long EXAMPLE_ISSUE_EFFORT_IN_MINUTES = 15L;
+ private static final String EXAMPLE_ISSUE_KEY = "key";
+ private static final int EXAMPLE_ISSUE_LINE = 1000;
+ private static final String EXAMPLE_ISSUE_MESSAGE = "message";
+ private static final String EXAMPLE_ISSUE_RESOLUTION = "resolution";
+ private static final String EXAMPLE_ISSUE_SEVERITY = "severity";
+ private static final String EXAMPLE_ISSUE_STATUS = "status";
+ private static final RuleType EXAMPLE_ISSUE_TYPE = RuleType.BUG;
+
  @Test
  public void checkAllIssuesCollected() {
  PostAnalysisIssueVisitor testCase = new PostAnalysisIssueVisitor();
@@ -50,4 +65,97 @@ public void checkAllIssuesCollected() {
  assertThat(testCase.getIssues().get(i).getComponent()).isEqualTo(expected.get(i).getComponent());
  }
  }
+
+ private DefaultIssue exampleDefaultIssue() {
+ DefaultIssue defaultIssue = mock(DefaultIssue.class);
+ doReturn(EXAMPLE_ISSUE_EFFORT_IN_MINUTES).when(defaultIssue).effortInMinutes();
+ doReturn(EXAMPLE_ISSUE_KEY).when(defaultIssue).key();
+ doReturn(EXAMPLE_ISSUE_LINE).when(defaultIssue).getLine();
+ doReturn(EXAMPLE_ISSUE_MESSAGE).when(defaultIssue).getMessage();
+ doReturn(EXAMPLE_ISSUE_RESOLUTION).when(defaultIssue).resolution();
+ doReturn(EXAMPLE_ISSUE_SEVERITY).when(defaultIssue).severity();
+ doReturn(EXAMPLE_ISSUE_STATUS).when(defaultIssue).status();
+ doReturn(EXAMPLE_ISSUE_TYPE).when(defaultIssue).type();
+ return defaultIssue;
+ }
+
+ @Test
+ public void testLightIssueMapping() {
+ // mock a DefaultIssue
+ DefaultIssue defaultIssue = exampleDefaultIssue();
+
+ // map the DefaultIssue into a LightIssue (using PostAnalysisIssueVisitor to workaround private constructor)
+ PostAnalysisIssueVisitor visitor = new PostAnalysisIssueVisitor();
+ visitor.onIssue(null, defaultIssue);
+ PostAnalysisIssueVisitor.LightIssue lightIssue = visitor.getIssues().get(0).getIssue();
+
+ // check values equality, twice (see below)
+ for (int i = 0; i < 2; i++) {
+ assertThat(lightIssue.effortInMinutes()).isEqualTo(EXAMPLE_ISSUE_EFFORT_IN_MINUTES);
+ assertThat(lightIssue.key()).isEqualTo(EXAMPLE_ISSUE_KEY);
+ assertThat(lightIssue.getLine()).isEqualTo(EXAMPLE_ISSUE_LINE);
+ assertThat(lightIssue.getMessage()).isEqualTo(EXAMPLE_ISSUE_MESSAGE);
+ assertThat(lightIssue.resolution()).isEqualTo(EXAMPLE_ISSUE_RESOLUTION);
+ assertThat(lightIssue.severity()).isEqualTo(EXAMPLE_ISSUE_SEVERITY);
+ assertThat(lightIssue.status()).isEqualTo(EXAMPLE_ISSUE_STATUS);
+ assertThat(lightIssue.getStatus()).isEqualTo(EXAMPLE_ISSUE_STATUS); // alias getter
+ assertThat(lightIssue.type()).isEqualTo(EXAMPLE_ISSUE_TYPE);
+ }
+
+ // check DefaultIssue getters have been called _exactly once_
+ verify(defaultIssue).effortInMinutes();
+ verify(defaultIssue).key();
+ verify(defaultIssue).getLine();
+ verify(defaultIssue).getMessage();
+ verify(defaultIssue).resolution();
+ verify(defaultIssue).severity();
+ verify(defaultIssue).status();
+ verify(defaultIssue).type();
+ verifyNoMoreInteractions(defaultIssue);
+ }
+
+ @Test
+ public void testEqualLightIssues() {
+ DefaultIssue defaultIssue = exampleDefaultIssue();
+
+ // map the DefaultIssue into two equal LightIssues
+ PostAnalysisIssueVisitor visitor = new PostAnalysisIssueVisitor();
+ visitor.onIssue(null, defaultIssue);
+ visitor.onIssue(null, defaultIssue);
+ PostAnalysisIssueVisitor.LightIssue lightIssue1 = visitor.getIssues().get(0).getIssue();
+ PostAnalysisIssueVisitor.LightIssue lightIssue2 = visitor.getIssues().get(1).getIssue();
+
+ // assert equality
+ assertEquals(lightIssue1, lightIssue2);
+ assertEquals(lightIssue1.hashCode(), lightIssue2.hashCode());
+
+ // also assert equality on identity
+ assertEquals(lightIssue1, lightIssue1);
+ assertEquals(lightIssue1.hashCode(), lightIssue1.hashCode());
+ }
+
+ @Test
+ public void testDifferentLightIssues() {
+ DefaultIssue defaultIssue = exampleDefaultIssue();
+
+ // map the DefaultIssue into a first LightIssue
+ PostAnalysisIssueVisitor visitor = new PostAnalysisIssueVisitor();
+ visitor.onIssue(null, defaultIssue);
+ PostAnalysisIssueVisitor.LightIssue lightIssue1 = visitor.getIssues().get(0).getIssue();
+
+ // map a slightly different DefaultIssue into an other LightIssue
+ doReturn("another message").when(defaultIssue).getMessage();
+ visitor.onIssue(null, defaultIssue);
+ PostAnalysisIssueVisitor.LightIssue lightIssue2 = visitor.getIssues().get(1).getIssue();
+
+ // assert difference
+ assertNotEquals(lightIssue1, lightIssue2);
+ assertNotEquals(lightIssue1.hashCode(), lightIssue2.hashCode());
+
+ // also assert difference with unrelated objects, and null
+ assertNotEquals(lightIssue1, new Object());
+ assertNotEquals(lightIssue1, null);
+
+ }
+
 }

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecoratorTest.java

@@ -15,7 +15,6 @@
 import org.sonar.api.rules.RuleType;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.component.ReportAttributes;
-import org.sonar.core.issue.DefaultIssue;
 import org.sonar.db.alm.setting.AlmSettingDto;
 import org.sonar.db.alm.setting.ProjectAlmSettingDto;
 
@@ -104,7 +103,7 @@ private void mockValidAnalysis() {
  when(component.getType()).thenReturn(Component.Type.FILE);
  when(component.getReportAttributes()).thenReturn(reportAttributes);
 
- DefaultIssue defaultIssue = mock(DefaultIssue.class);
+ PostAnalysisIssueVisitor.LightIssue defaultIssue = mock(PostAnalysisIssueVisitor.LightIssue.class);
  when(defaultIssue.status()).thenReturn(Issue.STATUS_OPEN);
  when(defaultIssue.severity()).thenReturn(Severity.CRITICAL);
  when(defaultIssue.getLine()).thenReturn(ISSUE_LINE);