]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-14315 Cannot paginate /api/project_tags/search
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Thu, 18 Nov 2021 17:24:55 +0000 (11:24 -0600)
committersonartech <sonartech@sonarsource.com>
Thu, 18 Nov 2021 20:03:33 +0000 (20:03 +0000)
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RegisterRules.java
server/sonar-webserver-es/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndex.java
server/sonar-webserver-es/src/test/java/org/sonar/server/measure/index/ProjectMeasuresIndexTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/projecttag/ws/SearchAction.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/projecttag/ws/SearchActionTest.java

index bab17bc450cd92cda74b7bcb2b6e2af0208c547a..3857596e28a437b906266209f743c2707537ef2b 100644 (file)
@@ -745,8 +745,7 @@ public class RegisterRules implements Startable {
    * The side effect of this approach is that extended repositories will not be managed the same way.
    * If an extended repository do not exists anymore, then related active rules will be removed.
    */
-  private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RegisterRulesContext recorder,
-    List<RulesDefinition.Repository> context) {
+  private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession dbSession, RegisterRulesContext recorder, List<RulesDefinition.Repository> context) {
     List<String> repositoryKeys = context.stream()
       .map(RulesDefinition.ExtendedRepository::key)
       .collect(MoreCollectors.toList(context.size()));
index 2eafd49b2e604064c84c7641683cd8a6119e3c4e..5b20ee0d1ef59a3d210b7bc2c6ce49fff36921d2 100644 (file)
@@ -435,16 +435,19 @@ public class ProjectMeasuresIndex {
     }
   }
 
-  public List<String> searchTags(@Nullable String textQuery, int size) {
-    int maxPageSize = 500;
+  public List<String> searchTags(@Nullable String textQuery, int page, int size) {
+    int maxPageSize = 100;
+    int maxPage = 20;
     checkArgument(size <= maxPageSize, "Page size must be lower than or equals to " + maxPageSize);
+    checkArgument(page > 0 && page <= maxPage, "Page must be between 0 and " + maxPage);
+
     if (size <= 0) {
       return emptyList();
     }
 
     TermsAggregationBuilder tagFacet = AggregationBuilders.terms(FIELD_TAGS)
       .field(FIELD_TAGS)
-      .size(size)
+      .size(size*page)
       .minDocCount(1)
       .order(BucketOrder.key(true));
     if (textQuery != null) {
@@ -454,7 +457,6 @@ public class ProjectMeasuresIndex {
     SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder()
       .query(authorizationTypeSupport.createQueryFilter())
       .fetchSource(false)
-      .size(0)
       .aggregation(tagFacet);
 
     SearchResponse response = client.search(EsClient.prepareSearch(TYPE_PROJECT_MEASURES.getMainType())
@@ -463,6 +465,7 @@ public class ProjectMeasuresIndex {
     Terms aggregation = response.getAggregations().get(FIELD_TAGS);
 
     return aggregation.getBuckets().stream()
+      .skip((page-1) * size)
       .map(Bucket::getKeyAsString)
       .collect(MoreCollectors.toList());
   }
index 87fe834978c8f704eb83cee0c6680f8504f4c603..4510f12b08d64c50508a8ee5505bdcbe933c679d 100644 (file)
@@ -1517,7 +1517,7 @@ public class ProjectMeasuresIndexTest {
       newDoc().setTags(newArrayList("finance", "offshore")),
       newDoc().setTags(newArrayList("offshore")));
 
-    List<String> result = underTest.searchTags("off", 10);
+    List<String> result = underTest.searchTags("off", 1,10);
 
     assertThat(result).containsOnly("offshore", "official", "Madhoff");
   }
@@ -1532,7 +1532,7 @@ public class ProjectMeasuresIndexTest {
       newDoc().setTags(newArrayList("finance", "offshore")),
       newDoc().setTags(newArrayList("offshore")));
 
-    List<String> result = underTest.searchTags(null, 10);
+    List<String> result = underTest.searchTags(null, 1, 10);
 
     assertThat(result).containsOnly("offshore", "official", "Madhoff", "finance", "marketing", "java", "javascript");
   }
@@ -1547,11 +1547,49 @@ public class ProjectMeasuresIndexTest {
       newDoc().setTags(newArrayList("finance", "offshore")),
       newDoc().setTags(newArrayList("offshore")));
 
-    List<String> result = underTest.searchTags(null, 10);
+    List<String> result = underTest.searchTags(null, 1,10);
 
     assertThat(result).containsExactly("Madhoff", "finance", "java", "javascript", "marketing", "official", "offshore");
   }
 
+  @Test
+  public void search_tags_follows_paging() {
+    index(
+      newDoc().setTags(newArrayList("finance", "offshore", "java")),
+      newDoc().setTags(newArrayList("official", "javascript")),
+      newDoc().setTags(newArrayList("marketing", "official")),
+      newDoc().setTags(newArrayList("marketing", "Madhoff")),
+      newDoc().setTags(newArrayList("finance", "offshore")),
+      newDoc().setTags(newArrayList("offshore")));
+
+    List<String> result = underTest.searchTags(null, 1,3);
+    assertThat(result).containsExactly("Madhoff", "finance", "java");
+
+    result = underTest.searchTags(null, 2,3);
+    assertThat(result).containsExactly("javascript", "marketing", "official");
+
+    result = underTest.searchTags(null, 3,3);
+    assertThat(result).containsExactly("offshore");
+
+    result = underTest.searchTags(null, 3,4);
+    assertThat(result).isEmpty();
+  }
+
+  @Test
+  public void search_tags_returns_nothing_if_page_too_large() {
+    index(
+      newDoc().setTags(newArrayList("finance", "offshore", "java")),
+      newDoc().setTags(newArrayList("official", "javascript")),
+      newDoc().setTags(newArrayList("marketing", "official")),
+      newDoc().setTags(newArrayList("marketing", "Madhoff")),
+      newDoc().setTags(newArrayList("finance", "offshore")),
+      newDoc().setTags(newArrayList("offshore")));
+
+    List<String> result = underTest.searchTags(null, 10,2);
+
+    assertThat(result).isEmpty();
+  }
+
   @Test
   public void search_tags_only_of_authorized_projects() {
     indexForUser(USER1,
@@ -1562,14 +1600,14 @@ public class ProjectMeasuresIndexTest {
 
     userSession.logIn(USER1);
 
-    List<String> result = underTest.searchTags(null, 10);
+    List<String> result = underTest.searchTags(null, 1,10);
 
     assertThat(result).containsOnly("finance", "marketing");
   }
 
   @Test
   public void search_tags_with_no_tags() {
-    List<String> result = underTest.searchTags("whatever", 10);
+    List<String> result = underTest.searchTags("whatever", 1,10);
 
     assertThat(result).isEmpty();
   }
@@ -1578,7 +1616,7 @@ public class ProjectMeasuresIndexTest {
   public void search_tags_with_page_size_at_0() {
     index(newDoc().setTags(newArrayList("offshore")));
 
-    List<String> result = underTest.searchTags(null, 0);
+    List<String> result = underTest.searchTags(null, 1,0);
 
     assertThat(result).isEmpty();
   }
@@ -1650,9 +1688,17 @@ public class ProjectMeasuresIndexTest {
   @Test
   public void fail_if_page_size_greater_than_500() {
     expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Page size must be lower than or equals to 500");
+    expectedException.expectMessage("Page size must be lower than or equals to 100");
+
+    underTest.searchTags("whatever", 1, 101);
+  }
+
+  @Test
+  public void fail_if_page_greater_than_20() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Page must be between 0 and 20");
 
-    underTest.searchTags("whatever", 501);
+    underTest.searchTags("whatever", 21, 100);
   }
 
   private void index(ProjectMeasuresDoc... docs) {
index f2098f894cc920ff2b83a585b9f8f890cb7fea26..bdae1ca0eeb15cb28d820c615f0ad6b0b43a9915 100644 (file)
 package org.sonar.server.projecttag.ws;
 
 import java.util.List;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.server.measure.index.ProjectMeasuresIndex;
 import org.sonarqube.ws.ProjectTags;
 
+import static org.sonar.api.server.ws.WebService.Param.PAGE;
 import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE;
 import static org.sonar.api.server.ws.WebService.Param.TEXT_QUERY;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
@@ -43,10 +45,11 @@ public class SearchAction implements ProjectTagsWsAction {
       .setDescription("Search tags")
       .setSince("6.4")
       .setResponseExample(getClass().getResource("search-example.json"))
+      .setChangelog(new Change("9.2", "Parameter 'page' added"))
       .setHandler(this);
 
     action.addSearchQuery("off", "tags");
-    action.createPageSize(10, 100);
+    action.addPagingParams(10, 100);
   }
 
   @Override
@@ -56,7 +59,7 @@ public class SearchAction implements ProjectTagsWsAction {
   }
 
   private ProjectTags.SearchResponse doHandle(Request request) {
-    List<String> tags = index.searchTags(request.param(TEXT_QUERY), request.mandatoryParamAsInt(PAGE_SIZE));
+    List<String> tags = index.searchTags(request.param(TEXT_QUERY), request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE));
     return ProjectTags.SearchResponse.newBuilder().addAllTags(tags).build();
   }
 }
index 6edf6be148bf15a9041620537f8e0ef711f88e5c..0e330309cef09a78b90947fdbaa68d76160bb3c0 100644 (file)
@@ -102,7 +102,7 @@ public class SearchActionTest {
     assertThat(definition.isPost()).isFalse();
     assertThat(definition.responseExampleAsString()).isNotEmpty();
     assertThat(definition.since()).isEqualTo("6.4");
-    assertThat(definition.params()).extracting(WebService.Param::key).containsOnly("q", "ps");
+    assertThat(definition.params()).extracting(WebService.Param::key).containsOnly("q", "p", "ps");
   }
 
   private void index(ProjectMeasuresDoc... docs) {