SONAR-8368 closed issues max age can be set by property

and effectively disabled with value 0

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java

@@ -29,9 +29,11 @@ import java.util.Map;
 import java.util.Optional;
 import org.apache.ibatis.session.ResultContext;
 import org.apache.ibatis.session.ResultHandler;
+import org.sonar.api.config.Configuration;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.utils.System2;
+import org.sonar.api.utils.log.Loggers;
 import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.FieldDiffs;
@@ -49,17 +51,34 @@ import static org.sonar.api.issue.Issue.STATUS_CLOSED;
 
 public class ComponentIssuesLoader {
   private static final int DEFAULT_CLOSED_ISSUES_MAX_AGE = 30;
+  private static final String PROPERTY_CLOSED_ISSUE_MAX_AGE = "sonar.issuetracking.closedissues.maxage";
 
   private final DbClient dbClient;
   private final RuleRepository ruleRepository;
   private final ActiveRulesHolder activeRulesHolder;
   private final System2 system2;
+  private final int closedIssueMaxAge;
 
-  public ComponentIssuesLoader(DbClient dbClient, RuleRepository ruleRepository, ActiveRulesHolder activeRulesHolder, System2 system2) {
+  public ComponentIssuesLoader(DbClient dbClient, RuleRepository ruleRepository, ActiveRulesHolder activeRulesHolder,
+    Configuration configuration, System2 system2) {
     this.dbClient = dbClient;
     this.activeRulesHolder = activeRulesHolder;
     this.ruleRepository = ruleRepository;
     this.system2 = system2;
+    this.closedIssueMaxAge = configuration.get(PROPERTY_CLOSED_ISSUE_MAX_AGE)
+      .map(ComponentIssuesLoader::safelyParseClosedIssueMaxAge)
+      .filter(i -> i >= 0)
+      .orElse(DEFAULT_CLOSED_ISSUES_MAX_AGE);
+  }
+
+  private static Integer safelyParseClosedIssueMaxAge(String str) {
+    try {
+      return Integer.parseInt(str);
+    } catch (NumberFormatException e) {
+      Loggers.get(ComponentIssuesLoader.class)
+        .warn("Value of property {} should be an integer >= 0: {}", PROPERTY_CLOSED_ISSUE_MAX_AGE, str);
+      return null;
+    }
   }
 
   public List<DefaultIssue> loadOpenIssues(String componentUuid) {
@@ -148,9 +167,13 @@ public class ComponentIssuesLoader {
    * 00H00 are returned.
    */
   public List<DefaultIssue> loadClosedIssues(String componentUuid) {
+    if (closedIssueMaxAge == 0) {
+      return emptyList();
+    }
+
     Date date = new Date(system2.now());
     long closeDateAfter = date.toInstant()
-      .minus(DEFAULT_CLOSED_ISSUES_MAX_AGE, ChronoUnit.DAYS)
+      .minus(closedIssueMaxAge, ChronoUnit.DAYS)
       .truncatedTo(ChronoUnit.DAYS)
       .toEpochMilli();
     try (DbSession dbSession = dbClient.openSession(false)) {

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java

@@ -19,6 +19,9 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
@@ -26,6 +29,10 @@ import java.util.Random;
 import javax.annotation.Nullable;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.sonar.api.config.Configuration;
+import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.System2;
@@ -39,24 +46,27 @@ import org.sonar.db.issue.IssueDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.rule.RuleDefinitionDto;
 
+import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 import static org.sonar.api.issue.Issue.STATUS_CLOSED;
 import static org.sonar.api.utils.DateUtils.addDays;
 import static org.sonar.api.utils.DateUtils.parseDateTime;
 
+@RunWith(DataProviderRunner.class)
 public class ComponentIssuesLoaderTest {
   private static final Date NOW = parseDateTime("2018-08-17T13:44:53+0000");
   private static final Date DATE_LIMIT_30_DAYS_BACK_MIDNIGHT = parseDateTime("2018-07-18T00:00:00+0000");
 
   @Rule
   public DbTester dbTester = DbTester.create(System2.INSTANCE);
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   private DbClient dbClient = dbTester.getDbClient();
   private System2 system2 = mock(System2.class);
-  private ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
-    null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, system2);
 
   @Test
   public void loadClosedIssues_returns_single_DefaultIssue_by_issue_based_on_first_row() {
@@ -71,6 +81,7 @@ public class ComponentIssuesLoaderTest {
     dbTester.issues().insertFieldDiffs(issue, newToClosedDiffsWithLine(addDays(issueDate, 1), 30));
     when(system2.now()).thenReturn(NOW.getTime());
 
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(newEmptySettings());
     List<DefaultIssue> defaultIssues = underTest.loadClosedIssues(file.uuid());
 
     assertThat(defaultIssues).hasSize(1);
@@ -90,6 +101,7 @@ public class ComponentIssuesLoaderTest {
     dbTester.issues().insertFieldDiffs(issue, newToClosedDiffsWithLine(addDays(issueDate, 1), 30));
     when(system2.now()).thenReturn(NOW.getTime());
 
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(newEmptySettings());
     List<DefaultIssue> defaultIssues = underTest.loadClosedIssues(file.uuid());
 
     assertThat(defaultIssues).hasSize(1);
@@ -109,6 +121,7 @@ public class ComponentIssuesLoaderTest {
     dbTester.issues().insertFieldDiffs(issueNoCloseDate, newToClosedDiffsWithLine(issueDate, 10));
     when(system2.now()).thenReturn(NOW.getTime());
 
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(newEmptySettings());
     List<DefaultIssue> defaultIssues = underTest.loadClosedIssues(file.uuid());
 
     assertThat(defaultIssues)
@@ -118,6 +131,54 @@ public class ComponentIssuesLoaderTest {
 
   @Test
   public void loadClosedIssues_returns_only_closed_issues_which_close_date_is_from_day_30_days_ago() {
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(newEmptySettings());
+    loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(underTest);
+  }
+
+  @Test
+  public void loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago_if_property_is_empty() {
+    Configuration configuration = newConfiguration(null);
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(configuration);
+
+    loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(underTest);
+  }
+
+  @Test
+  public void loadClosedIssues_returns_only_closed_with_close_date_is_from_30_days_ago_if_property_is_less_than_0() {
+    Configuration configuration = newConfiguration(String.valueOf(-(1 + new Random().nextInt(10))));
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(configuration);
+
+    loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(underTest);
+  }
+
+  @Test
+  public void loadClosedIssues_returns_only_closed_with_close_date_is_from_30_days_ago_if_property_is_30() {
+    Configuration configuration = newConfiguration("30");
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(configuration);
+
+    loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(underTest);
+  }
+
+  @Test
+  @UseDataProvider("notAnIntegerPropertyValues")
+  public void loadClosedIssues_returns_only_closed_with_close_date_is_from_30_days_ago_if_property_is_not_an_integer(String notAnInteger) {
+    Configuration configuration = newConfiguration(notAnInteger);
+    ComponentIssuesLoader underTest = newComponentIssuesLoader(configuration);
+
+    loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(underTest);
+  }
+
+  @DataProvider
+  public static Object[][] notAnIntegerPropertyValues() {
+    return new Object[][] {
+      {"foo"},
+      {"1,3"},
+      {"1.3"},
+      {"-3.14"}
+    };
+  }
+
+  private void loadClosedIssues_returns_only_closed_issues_with_close_date_is_from_30_days_ago(ComponentIssuesLoader underTest) {
     OrganizationDto organization = dbTester.organizations().insert();
     ComponentDto project = dbTester.components().insertPublicProject(organization);
     ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project));
@@ -146,6 +207,20 @@ public class ComponentIssuesLoaderTest {
       .containsOnly(issues[0].getKey(), issues[2].getKey(), issues[3].getKey(), issues[4].getKey());
   }
 
+  @Test
+  public void loadClosedIssues_returns_empty_without_querying_DB_if_property_is_0() {
+    System2 system2 = mock(System2.class);
+    DbClient dbClient = mock(DbClient.class);
+    Configuration configuration = newConfiguration("0");
+    String componentUuid = randomAlphabetic(15);
+    ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
+      null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, configuration, system2);
+
+    assertThat(underTest.loadClosedIssues(componentUuid)).isEmpty();
+
+    verifyZeroInteractions(dbClient, system2);
+  }
+
   private static FieldDiffs newToClosedDiffsWithLine(Date creationDate, @Nullable Integer oldLineValue) {
     FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(addDays(creationDate, -5))
       .setDiff("status", randomNonCloseStatus(), STATUS_CLOSED);
@@ -161,4 +236,19 @@ public class ComponentIssuesLoaderTest {
       .toArray(String[]::new);
     return nonCloseStatuses[new Random().nextInt(nonCloseStatuses.length)];
   }
+
+  private ComponentIssuesLoader newComponentIssuesLoader(Configuration configuration) {
+    return new ComponentIssuesLoader(dbClient,
+      null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, configuration, system2);
+  }
+
+  private static Configuration newEmptySettings() {
+    return new MapSettings().asConfig();
+  }
+
+  private static Configuration newConfiguration(@Nullable String maxAge) {
+    MapSettings settings = new MapSettings();
+    settings.setProperty("sonar.issuetracking.closedissues.maxage", maxAge);
+    return settings.asConfig();
+  }
 }

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java

@@ -27,6 +27,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.ArgumentCaptor;
+import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
@@ -119,7 +120,7 @@ public class IntegrateIssuesVisitorTest {
 
   private ArgumentCaptor<DefaultIssue> defaultIssueCaptor;
 
-  private ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, System2.INSTANCE);
+  private ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(), System2.INSTANCE);
   private IssueTrackingDelegator trackingDelegator;
   private TrackerExecution tracker;
   private ShortBranchTrackerExecution shortBranchTracker;

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssueMergerTest.java

@@ -30,6 +30,7 @@ import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
+import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.utils.System2;
@@ -95,7 +96,8 @@ public class ShortBranchIssueMergerTest {
   public void setUp() {
     MockitoAnnotations.initMocks(this);
     DbClient dbClient = db.getDbClient();
-    copier = new ShortBranchIssueMerger(new ShortBranchIssuesLoader(new ShortBranchComponentsWithIssues(treeRootHolder, dbClient), dbClient, new ComponentIssuesLoader(dbClient, null, null, System2.INSTANCE)), tracker,
+    ComponentIssuesLoader componentIssuesLoader = new ComponentIssuesLoader(dbClient, null, null, new MapSettings().asConfig(), System2.INSTANCE);
+    copier = new ShortBranchIssueMerger(new ShortBranchIssuesLoader(new ShortBranchComponentsWithIssues(treeRootHolder, dbClient), dbClient, componentIssuesLoader), tracker,
       issueLifecycle);
     projectDto = db.components().insertMainBranch(p -> p.setDbKey(PROJECT_KEY).setUuid(PROJECT_UUID));
     branch1Dto = db.components().insertProjectBranch(projectDto, b -> b.setKey("myBranch1")