SONAR-13196 Search issues since leak period returns all issues if no leak period exists

3 files changed, 39 insertions, 31 deletions

diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java
index d9b1ecadc96..58c00a5fa8d 100644
--- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java
+++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java
@@ -638,7 +638,7 @@ public class IssueIndex {
   }
 
   private void validateCreationDateBounds(@Nullable Date createdBefore, @Nullable Date createdAfter) {
-    Preconditions.checkArgument(createdAfter == null || createdAfter.before(new Date(system.now())),
+    Preconditions.checkArgument(createdAfter == null || createdAfter.compareTo(new Date(system.now())) <= 0,
       "Start bound cannot be in the future");
     Preconditions.checkArgument(createdAfter == null || createdBefore == null || createdAfter.before(createdBefore),
       "Start bound cannot be larger or equal to end bound");
diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java
index 7dd6221826f..d6c7f69d8c3 100644
--- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java
+++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java
@@ -95,7 +95,7 @@ public class IssueQueryFactory {
     .map(Enum::name)
     .collect(MoreCollectors.toSet(RuleType.values().length - 1));
   private static final ComponentDto UNKNOWN_COMPONENT = new ComponentDto().setUuid(UNKNOWN).setProjectUuid(UNKNOWN);
-
+  private static final Set<String> QUALIFIERS_WITHOUT_LEAK_PERIOD = new HashSet<>(Arrays.asList(Qualifiers.APP, Qualifiers.VIEW, Qualifiers.SUBVIEW));
   private final DbClient dbClient;
   private final Clock clock;
   private final UserSession userSession;
@@ -145,8 +145,6 @@ public class IssueQueryFactory {
   }
 
   private void setCreatedAfterFromDates(IssueQuery.Builder builder, @Nullable Date createdAfter, @Nullable String createdInLast, boolean createdAfterInclusive) {
-    checkArgument(createdAfter == null || createdInLast == null, format("Parameters %s and %s cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_CREATED_IN_LAST));
-
     Date actualCreatedAfter = createdAfter;
     if (createdInLast != null) {
       actualCreatedAfter = Date.from(
@@ -171,20 +169,26 @@ public class IssueQueryFactory {
     String createdInLast = request.getCreatedInLast();
 
     if (request.getSinceLeakPeriod() == null || !request.getSinceLeakPeriod()) {
+      checkArgument(createdAfter == null || createdInLast == null, format("Parameters %s and %s cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_CREATED_IN_LAST));
       setCreatedAfterFromDates(builder, createdAfter, createdInLast, true);
     } else {
       checkArgument(createdAfter == null, "Parameters '%s' and '%s' cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_SINCE_LEAK_PERIOD);
+      checkArgument(createdInLast == null, format("Parameters %s and %s cannot be set simultaneously", PARAM_CREATED_IN_LAST, PARAM_SINCE_LEAK_PERIOD));
+
       checkArgument(componentUuids.size() == 1, "One and only one component must be provided when searching since leak period");
       ComponentDto component = componentUuids.iterator().next();
-      Date createdAfterFromSnapshot = findCreatedAfterFromComponentUuid(dbSession, component);
-      setCreatedAfterFromDates(builder, createdAfterFromSnapshot, createdInLast, false);
+
+      if (!QUALIFIERS_WITHOUT_LEAK_PERIOD.contains(component.qualifier()) && request.getPullRequest() == null) {
+        Date createdAfterFromSnapshot = findCreatedAfterFromComponentUuid(dbSession, component);
+        setCreatedAfterFromDates(builder, createdAfterFromSnapshot, null, false);
+      }
     }
   }
 
-  @CheckForNull
   private Date findCreatedAfterFromComponentUuid(DbSession dbSession, ComponentDto component) {
     Optional<SnapshotDto> snapshot = dbClient.snapshotDao().selectLastAnalysisByComponentUuid(dbSession, component.uuid());
-    return snapshot.map(s -> longToDate(s.getPeriodDate())).orElse(null);
+    // if last analysis has no period date, then no issue should be considered new.
+    return snapshot.map(s -> longToDate(s.getPeriodDate())).orElseGet(() -> new Date(clock.millis()));
   }
 
   private boolean mergeDeprecatedComponentParameters(DbSession session, SearchRequest request, List<ComponentDto> allComponents) {
@@ -319,7 +323,7 @@ public class IssueQueryFactory {
   }
 
   private void addCreatedAfterByProjects(IssueQuery.Builder builder, DbSession dbSession, SearchRequest request, Set<String> applicationUuids) {
-    if (request.getSinceLeakPeriod() == null || !request.getSinceLeakPeriod()) {
+    if (request.getSinceLeakPeriod() == null || !request.getSinceLeakPeriod() || request.getPullRequest() != null) {
       return;
     }
 
@@ -331,6 +335,7 @@ public class IssueQueryFactory {
       .stream()
       .filter(s -> s.getPeriodDate() != null)
       .collect(uniqueIndex(SnapshotDto::getComponentUuid, s -> new PeriodStart(longToDate(s.getPeriodDate()), false)));
+
     builder.createdAfterByProjectUuids(leakByProjects);
   }
 
diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java
index 050eb283ae7..c86fb938571 100644
--- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java
+++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java
@@ -24,6 +24,7 @@ import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -43,6 +44,7 @@ import org.sonar.server.tester.UserSessionRule;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.tuple;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -276,6 +278,7 @@ public class IssueQueryFactoryTest {
   @Test
   public void application_search_project_issues_on_leak() {
     Date now = new Date();
+    when(clock.millis()).thenReturn(now.getTime());
     ComponentDto project1 = db.components().insertPublicProject();
     SnapshotDto analysis1 = db.components().insertSnapshot(project1, s -> s.setPeriodDate(addDays(now, -14).getTime()));
     ComponentDto project2 = db.components().insertPublicProject();
@@ -292,8 +295,8 @@ public class IssueQueryFactoryTest {
       .setSinceLeakPeriod(true));
 
     assertThat(result.createdAfterByProjectUuids()).hasSize(1);
-    assertThat(result.createdAfterByProjectUuids().get(project1.uuid()).date().getTime()).isEqualTo(analysis1.getPeriodDate());
-    assertThat(result.createdAfterByProjectUuids().get(project1.uuid()).inclusive()).isFalse();
+    assertThat(result.createdAfterByProjectUuids().entrySet()).extracting(Map.Entry::getKey, e -> e.getValue().date(), e -> e.getValue().inclusive()).containsOnly(
+      tuple(project1.uuid(), new Date(analysis1.getPeriodDate()), false));
     assertThat(result.viewUuids()).containsExactlyInAnyOrder(application.uuid());
   }
 
@@ -392,14 +395,14 @@ public class IssueQueryFactoryTest {
     assertThat(underTest.create(new SearchRequest()
       .setProjectKeys(singletonList(branch.getKey()))
       .setBranch(branch.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(project.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(project.uuid()), false);
 
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(branch.getKey()))
       .setBranch(branch.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(project.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(project.uuid()), false);
   }
 
   @Test
@@ -411,22 +414,22 @@ public class IssueQueryFactoryTest {
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(file.getKey()))
       .setBranch(branch.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
 
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(branch.getKey()))
       .setFileUuids(singletonList(file.uuid()))
       .setBranch(branch.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
 
     assertThat(underTest.create(new SearchRequest()
       .setProjectKeys(singletonList(branch.getKey()))
       .setFileUuids(singletonList(file.uuid()))
       .setBranch(branch.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.fileUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
   }
 
   @Test
@@ -439,8 +442,8 @@ public class IssueQueryFactoryTest {
       .setComponentKeys(singletonList(file.getKey()))
       .setBranch(branch.getBranch())
       .setOnComponentOnly(true)))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.componentUuids()), IssueQuery::isMainBranch)
-        .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.componentUuids()), IssueQuery::isMainBranch)
+      .containsOnly(branch.uuid(), singletonList(file.uuid()), false);
   }
 
   @Test
@@ -451,13 +454,13 @@ public class IssueQueryFactoryTest {
     assertThat(underTest.create(new SearchRequest()
       .setProjectKeys(singletonList(project.getKey()))
       .setBranch("master")))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(project.uuid(), singletonList(project.uuid()), true);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(project.uuid(), singletonList(project.uuid()), true);
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(project.getKey()))
       .setBranch("master")))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(project.uuid(), singletonList(project.uuid()), true);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(project.uuid(), singletonList(project.uuid()), true);
   }
 
   @Test
@@ -492,16 +495,16 @@ public class IssueQueryFactoryTest {
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(applicationBranch1.getKey()))
       .setBranch(applicationBranch1.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(applicationBranch1.uuid(), Collections.emptyList(), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(applicationBranch1.uuid(), Collections.emptyList(), false);
 
     // Search on project1Branch1
     assertThat(underTest.create(new SearchRequest()
       .setComponentKeys(singletonList(applicationBranch1.getKey()))
       .setProjectKeys(singletonList(project1.getKey()))
       .setBranch(applicationBranch1.getBranch())))
-        .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
-        .containsOnly(applicationBranch1.uuid(), singletonList(project1.uuid()), false);
+      .extracting(IssueQuery::branchUuid, query -> new ArrayList<>(query.projectUuids()), IssueQuery::isMainBranch)
+      .containsOnly(applicationBranch1.uuid(), singletonList(project1.uuid()), false);
   }
 
   @Test