SONAR-13665 Fix api/webhooks/deliveries to return appropriate 'componentKey' in response

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookDeliveriesAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookDeliveriesAction.java
index 7567211391c23a46d49f3182955edfb76f8c12ee..af45ff0cd5e5eb5286229621aca1cb896dc4646d 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookDeliveriesAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/webhook/ws/WebhookDeliveriesAction.java
@@ -19,8 +19,13 @@
  */
 package org.sonar.server.webhook.ws;
 
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
-import java.util.Optional;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
@@ -32,12 +37,12 @@ import org.sonar.db.DbSession;
 import org.sonar.db.project.ProjectDto;
 import org.sonar.db.webhook.WebhookDeliveryLiteDto;
 import org.sonar.server.component.ComponentFinder;
+import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.user.UserSession;
 import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Webhooks;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static java.util.Objects.requireNonNull;
 import static org.apache.commons.lang.StringUtils.isNotBlank;
 import static org.sonar.api.server.ws.WebService.Param.PAGE;
 import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE;
@@ -110,60 +115,67 @@ public class WebhookDeliveriesAction implements WebhooksWsAction {
   }
 
   private Data loadFromDatabase(@Nullable String webhookUuid, @Nullable String ceTaskId, @Nullable String projectKey, int page, int pageSize) {
-    ProjectDto project;
+    Map<String, ProjectDto> projectUuidMap;
     List<WebhookDeliveryLiteDto> deliveries;
     int totalElements;
     try (DbSession dbSession = dbClient.openSession(false)) {
       if (isNotBlank(webhookUuid)) {
         totalElements = dbClient.webhookDeliveryDao().countDeliveriesByWebhookUuid(dbSession, webhookUuid);
         deliveries = dbClient.webhookDeliveryDao().selectByWebhookUuid(dbSession, webhookUuid, offset(page, pageSize), pageSize);
-        project = getProjectDto(dbSession, deliveries);
+        projectUuidMap = getProjectsDto(dbSession, deliveries);
       } else if (projectKey != null) {
-        project = componentFinder.getProjectByKey(dbSession, projectKey);
+        ProjectDto project = componentFinder.getProjectByKey(dbSession, projectKey);
+        projectUuidMap = new HashMap<>();
+        projectUuidMap.put(project.getUuid(), project);
         totalElements = dbClient.webhookDeliveryDao().countDeliveriesByComponentUuid(dbSession, project.getUuid());
         deliveries = dbClient.webhookDeliveryDao().selectOrderedByComponentUuid(dbSession, project.getUuid(), offset(page, pageSize), pageSize);
       } else {
         totalElements = dbClient.webhookDeliveryDao().countDeliveriesByCeTaskUuid(dbSession, ceTaskId);
         deliveries = dbClient.webhookDeliveryDao().selectOrderedByCeTaskUuid(dbSession, ceTaskId, offset(page, pageSize), pageSize);
-        project = getProjectDto(dbSession, deliveries);
+        projectUuidMap = getProjectsDto(dbSession, deliveries);
       }
     }
-    return new Data(project, deliveries).withPagingInfo(page, pageSize, totalElements);
+    return new Data(projectUuidMap, deliveries).withPagingInfo(page, pageSize, totalElements);
   }
 
-  private ProjectDto getProjectDto(DbSession dbSession, List<WebhookDeliveryLiteDto> deliveries) {
-    Optional<String> deliveredComponentUuid = deliveries
+  private Map<String, ProjectDto> getProjectsDto(DbSession dbSession, List<WebhookDeliveryLiteDto> deliveries) {
+    Map<String, String> deliveredComponentUuid = deliveries
       .stream()
-      .map(WebhookDeliveryLiteDto::getComponentUuid)
-      .findFirst();
+      .collect(Collectors.toMap(WebhookDeliveryLiteDto::getUuid, WebhookDeliveryLiteDto::getComponentUuid));
 
-    if (deliveredComponentUuid.isPresent()) {
-      return componentFinder.getProjectByUuid(dbSession, deliveredComponentUuid.get());
+    if (!deliveredComponentUuid.isEmpty()) {
+      return dbClient.projectDao().selectByUuids(dbSession, new HashSet<>(deliveredComponentUuid.values()))
+        .stream()
+        .collect(Collectors.toMap(ProjectDto::getUuid, Function.identity()));
     } else {
-      return null;
+      return Collections.emptyMap();
     }
   }
 
   private static class Data {
-    private final ProjectDto project;
+    private final Map<String, ProjectDto> projectUuidMap;
     private final List<WebhookDeliveryLiteDto> deliveryDtos;
 
     private int pageIndex;
     private int pageSize;
     private int totalElements;
 
-    Data(@Nullable ProjectDto project, List<WebhookDeliveryLiteDto> deliveries) {
+    Data(Map<String, ProjectDto> projectUuidMap, List<WebhookDeliveryLiteDto> deliveries) {
       this.deliveryDtos = deliveries;
       if (deliveries.isEmpty()) {
-        this.project = null;
+        this.projectUuidMap = projectUuidMap;
       } else {
-        this.project = requireNonNull(project);
+        checkArgument(!projectUuidMap.isEmpty());
+        this.projectUuidMap = projectUuidMap;
       }
     }
 
     void ensureAdminPermission(UserSession userSession) {
-      if (project != null) {
-        userSession.checkProjectPermission(UserRole.ADMIN, project);
+      if (!projectUuidMap.isEmpty()) {
+        List<ProjectDto> projectsUserHasAccessTo = userSession.keepAuthorizedProjects(UserRole.ADMIN, projectUuidMap.values());
+        if (projectsUserHasAccessTo.size() != projectUuidMap.size()) {
+          throw new ForbiddenException("Insufficient privileges");
+        }
       }
     }
 
@@ -171,6 +183,7 @@ public class WebhookDeliveriesAction implements WebhooksWsAction {
       Webhooks.DeliveriesWsResponse.Builder responseBuilder = Webhooks.DeliveriesWsResponse.newBuilder();
       Webhooks.Delivery.Builder deliveryBuilder = Webhooks.Delivery.newBuilder();
       for (WebhookDeliveryLiteDto dto : deliveryDtos) {
+        ProjectDto project = projectUuidMap.get(dto.getComponentUuid());
         copyDtoToProtobuf(project, dto, deliveryBuilder);
         responseBuilder.addDeliveries(deliveryBuilder);
       }

diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/WebhookDeliveriesActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/WebhookDeliveriesActionTest.java
index 00346d79ebc2951e2e70057e8760f1316df0465d..a67677fdb0bcd3155f4c28eab82a06e88fbfaf5e 100644 (file)
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/WebhookDeliveriesActionTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/webhook/ws/WebhookDeliveriesActionTest.java
@@ -22,7 +22,6 @@ package org.sonar.server.webhook.ws;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
 import org.sonar.api.web.UserRole;
@@ -36,19 +35,18 @@ import org.sonar.server.component.TestComponentFinder;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.UnauthorizedException;
 import org.sonar.server.tester.UserSessionRule;
+import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 import org.sonarqube.ws.Webhooks;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.db.component.ComponentTesting.newPrivateProjectDto;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.tuple;
 import static org.sonar.db.webhook.WebhookDeliveryTesting.newDto;
 import static org.sonar.test.JsonAssert.assertJson;
 
 public class WebhookDeliveriesActionTest {
 
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
-
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
 
@@ -60,6 +58,7 @@ public class WebhookDeliveriesActionTest {
 
   private WsActionTester ws;
   private ComponentDto project;
+  private ComponentDto otherProject;
 
   @Before
   public void setUp() {
@@ -67,6 +66,7 @@ public class WebhookDeliveriesActionTest {
     WebhookDeliveriesAction underTest = new WebhookDeliveriesAction(dbClient, userSession, componentFinder);
     ws = new WsActionTester(underTest);
     project = db.components().insertPrivateProject(c -> c.setDbKey("my-project"));
+    otherProject = db.components().insertPrivateProject(c -> c.setDbKey("other-project"));
   }
 
   @Test
@@ -79,9 +79,9 @@ public class WebhookDeliveriesActionTest {
 
   @Test
   public void throw_UnauthorizedException_if_anonymous() {
-    expectedException.expect(UnauthorizedException.class);
-
-    ws.newRequest().execute();
+    TestRequest request = ws.newRequest();
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(UnauthorizedException.class);
   }
 
   @Test
@@ -164,17 +164,30 @@ public class WebhookDeliveriesActionTest {
     WebhookDeliveryDto dto1 = newDto().setComponentUuid(project.uuid()).setCeTaskUuid("t1").setWebhookUuid("wh-1-uuid");
     WebhookDeliveryDto dto2 = newDto().setComponentUuid(project.uuid()).setCeTaskUuid("t1").setWebhookUuid("wh-1-uuid");
     WebhookDeliveryDto dto3 = newDto().setComponentUuid(project.uuid()).setCeTaskUuid("t2").setWebhookUuid("wh-2-uuid");
+
+    WebhookDeliveryDto dto4 = newDto().setComponentUuid(otherProject.uuid()).setCeTaskUuid("t4").setWebhookUuid("wh-1-uuid");
+    WebhookDeliveryDto dto5 = newDto().setComponentUuid(otherProject.uuid()).setCeTaskUuid("t5").setWebhookUuid("wh-1-uuid");
+
     dbClient.webhookDeliveryDao().insert(db.getSession(), dto1);
     dbClient.webhookDeliveryDao().insert(db.getSession(), dto2);
     dbClient.webhookDeliveryDao().insert(db.getSession(), dto3);
+    dbClient.webhookDeliveryDao().insert(db.getSession(), dto4);
+    dbClient.webhookDeliveryDao().insert(db.getSession(), dto5);
     db.commit();
-    userSession.logIn().addProjectPermission(UserRole.ADMIN, project);
+    userSession.logIn().addProjectPermission(UserRole.ADMIN, project, otherProject);
 
     Webhooks.DeliveriesWsResponse response = ws.newRequest()
       .setParam("webhook", "wh-1-uuid")
       .executeProtobuf(Webhooks.DeliveriesWsResponse.class);
-    assertThat(response.getDeliveriesCount()).isEqualTo(2);
-    assertThat(response.getDeliveriesList()).extracting(Webhooks.Delivery::getId).containsOnly(dto1.getUuid(), dto2.getUuid());
+    assertThat(response.getDeliveriesCount()).isEqualTo(4);
+    assertThat(response.getDeliveriesList()).extracting(Webhooks.Delivery::getId)
+      .containsOnly(dto1.getUuid(), dto2.getUuid(), dto4.getUuid(), dto5.getUuid());
+    assertThat(response.getDeliveriesList()).extracting(Webhooks.Delivery::getId, Webhooks.Delivery::getComponentKey)
+      .containsOnly(
+        tuple(dto1.getUuid(), project.getDbKey()),
+        tuple(dto2.getUuid(), project.getDbKey()),
+        tuple(dto4.getUuid(), otherProject.getDbKey()),
+        tuple(dto5.getUuid(), otherProject.getDbKey()));
   }
 
   @Test
@@ -242,12 +255,11 @@ public class WebhookDeliveriesActionTest {
     db.commit();
     userSession.logIn().addProjectPermission(UserRole.USER, project);
 
-    expectedException.expect(ForbiddenException.class);
-    expectedException.expectMessage("Insufficient privileges");
-
-    ws.newRequest()
-      .setParam("componentKey", project.getDbKey())
-      .execute();
+    TestRequest request = ws.newRequest()
+      .setParam("componentKey", project.getDbKey());
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(ForbiddenException.class)
+      .hasMessage("Insufficient privileges");
   }
 
   @Test
@@ -258,37 +270,34 @@ public class WebhookDeliveriesActionTest {
     db.commit();
     userSession.logIn().addProjectPermission(UserRole.USER, project);
 
-    expectedException.expect(ForbiddenException.class);
-    expectedException.expectMessage("Insufficient privileges");
-
-    ws.newRequest()
-      .setParam("ceTaskId", dto.getCeTaskUuid())
-      .execute();
+    TestRequest request = ws.newRequest()
+      .setParam("ceTaskId", dto.getCeTaskUuid());
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(ForbiddenException.class)
+      .hasMessage("Insufficient privileges");
   }
 
   @Test
   public void throw_IAE_if_both_component_and_task_parameters_are_set() {
     userSession.logIn();
 
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Either 'ceTaskId' or 'componentKey' or 'webhook' must be provided");
-
-    ws.newRequest()
+    TestRequest request = ws.newRequest()
       .setParam("componentKey", project.getDbKey())
-      .setParam("ceTaskId", "t1")
-      .execute();
+      .setParam("ceTaskId", "t1");
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Either 'ceTaskId' or 'componentKey' or 'webhook' must be provided");
   }
 
   @Test
   public void throw_IAE_if_both_component_and_webhook_are_set() {
     userSession.logIn();
 
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Either 'ceTaskId' or 'componentKey' or 'webhook' must be provided");
-
-    ws.newRequest()
+    TestRequest request = ws.newRequest()
       .setParam("componentKey", project.getDbKey())
-      .setParam("webhook", "wh-uuid")
-      .execute();
+      .setParam("webhook", "wh-uuid");
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Either 'ceTaskId' or 'componentKey' or 'webhook' must be provided");
   }
 }