Fix Quality flaws in org.sonar.server.ce.ws

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java
index 883a92f8ca9b0d5fe575d886bc7c64d2d88c3606..cd8cda869493f4b338f7cdccd88d69e282f69a7e 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ActivityAction.java
@@ -160,8 +160,7 @@ public class ActivityAction implements CeWsAction {
   }
 
   private ActivityResponse doHandle(ActivityWsRequest request) {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       // if a task searched by uuid is found all other parameters are ignored
       Optional<WsCe.Task> taskSearchedById = searchTaskByUuid(dbSession, request);
       if (taskSearchedById.isPresent()) {
@@ -181,8 +180,6 @@ public class ActivityAction implements CeWsAction {
         queuedTasks,
         pastTasks,
         request.getPageSize());
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java
index 1c871e3331bd567c800b9f68b06901fc76fca791..1e1013b36324afc41fe16a74a94346f6bedd094e 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/ComponentAction.java
@@ -35,9 +35,9 @@ import org.sonar.db.component.ComponentDto;
 import org.sonar.server.component.ComponentFinder;
 import org.sonar.server.user.UserSession;
 import org.sonar.server.ws.KeyExamples;
-import org.sonar.server.ws.WsUtils;
 
 import static org.sonar.server.component.ComponentFinder.ParamNames.COMPONENT_ID_AND_KEY;
+import static org.sonar.server.ws.WsUtils.writeProtobuf;
 import static org.sonarqube.ws.WsCe.ProjectResponse;
 
 public class ComponentAction implements CeWsAction {
@@ -80,8 +80,7 @@ public class ComponentAction implements CeWsAction {
 
   @Override
   public void handle(Request wsRequest, Response wsResponse) throws Exception {
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       ComponentDto component = componentFinder.getByUuidOrKey(dbSession, wsRequest.param(PARAM_COMPONENT_ID), wsRequest.param(PARAM_COMPONENT_KEY), COMPONENT_ID_AND_KEY);
       userSession.checkComponentPermission(UserRole.USER, component);
       List<CeQueueDto> queueDtos = dbClient.ceQueueDao().selectByComponentUuid(dbSession, component.uuid());
@@ -95,10 +94,7 @@ public class ComponentAction implements CeWsAction {
       if (activityDtos.size() == 1) {
         wsResponseBuilder.setCurrent(formatter.formatActivity(dbSession, activityDtos.get(0)));
       }
-      WsUtils.writeProtobuf(wsResponseBuilder.build(), wsRequest, wsResponse);
-
-    } finally {
-      dbClient.closeSession(dbSession);
+      writeProtobuf(wsResponseBuilder.build(), wsRequest, wsResponse);
     }
   }
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskAction.java
index 77c95e66084744484af401ee799f9a25bba60027..e70e24cd82d5e9b9ef193a983546e290e26a8a34 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskAction.java
@@ -87,8 +87,7 @@ public class TaskAction implements CeWsAction {
   @Override
   public void handle(Request wsRequest, Response wsResponse) throws Exception {
     String taskUuid = wsRequest.mandatoryParam(PARAM_TASK_UUID);
-    DbSession dbSession = dbClient.openSession(false);
-    try {
+    try (DbSession dbSession = dbClient.openSession(false)) {
       WsCe.TaskResponse.Builder wsTaskResponse = WsCe.TaskResponse.newBuilder();
       Optional<CeQueueDto> queueDto = dbClient.ceQueueDao().selectByUuid(dbSession, taskUuid);
       if (queueDto.isPresent()) {
@@ -108,8 +107,6 @@ public class TaskAction implements CeWsAction {
         }
       }
       writeProtobuf(wsTaskResponse.build(), wsRequest, wsResponse);
-    } finally {
-      dbClient.closeSession(dbSession);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskFormatter.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskFormatter.java
index 8f1357d614a0f0a94c08f44e1c3f9e5f58eb7531..9d832a82150e6d1f747288b8d5281ed0410ab724 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskFormatter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/TaskFormatter.java
@@ -20,7 +20,6 @@
 package org.sonar.server.ce.ws;
 
 import com.google.common.base.Optional;
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import java.util.Collection;
 import java.util.Collections;
@@ -29,11 +28,11 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.System2;
+import org.sonar.core.util.stream.Collectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.ce.CeActivityDto;
@@ -42,6 +41,10 @@ import org.sonar.db.component.ComponentDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonarqube.ws.WsCe;
 
+import static com.google.common.base.Preconditions.checkState;
+import static org.sonar.api.utils.DateUtils.formatDateTime;
+import static org.sonar.core.util.Protobuf.setNullable;
+
 /**
  * Converts {@link CeActivityDto} and {@link CeQueueDto} to the protobuf objects
  * used to write WS responses (see ws-ce.proto in module sonar-ws)
@@ -58,7 +61,7 @@ public class TaskFormatter {
 
   public List<WsCe.Task> formatQueue(DbSession dbSession, List<CeQueueDto> dtos) {
     ComponentDtoCache cache = ComponentDtoCache.forQueueDtos(dbClient, dbSession, dtos);
-    return dtos.stream().map(input -> formatQueue(input, cache)).collect(Collectors.toList());
+    return dtos.stream().map(input -> formatQueue(input, cache)).collect(Collectors.toList(dtos.size()));
   }
 
   public WsCe.Task formatQueue(DbSession dbSession, CeQueueDto dto) {
@@ -69,29 +72,19 @@ public class TaskFormatter {
     WsCe.Task.Builder builder = WsCe.Task.newBuilder();
     String organizationKey = componentDtoCache.getOrganizationKey(dto.getComponentUuid());
     // FIXME organization field should be set from the CeQueueDto rather than from the ComponentDto
-    if (organizationKey != null) {
-      builder.setOrganization(organizationKey);
+    setNullable(organizationKey, builder::setOrganization);
+    if (dto.getComponentUuid() != null) {
+      builder.setComponentId(dto.getComponentUuid());
+      buildComponent(builder, componentDtoCache.getComponent(dto.getComponentUuid()));
     }
     builder.setId(dto.getUuid());
     builder.setStatus(WsCe.TaskStatus.valueOf(dto.getStatus().name()));
     builder.setType(dto.getTaskType());
     builder.setLogs(false);
-    if (dto.getComponentUuid() != null) {
-      builder.setComponentId(dto.getComponentUuid());
-      buildComponent(builder, componentDtoCache.getComponent(dto.getComponentUuid()));
-    }
-    if (dto.getSubmitterLogin() != null) {
-      builder.setSubmitterLogin(dto.getSubmitterLogin());
-    }
-    builder.setSubmittedAt(DateUtils.formatDateTime(new Date(dto.getCreatedAt())));
-    if (dto.getStartedAt() != null) {
-      builder.setStartedAt(DateUtils.formatDateTime(new Date(dto.getStartedAt())));
-    }
-    //
-    Long executionTimeMs = computeExecutionTimeMs(dto);
-    if (executionTimeMs != null) {
-      builder.setExecutionTimeMs(executionTimeMs);
-    }
+    setNullable(dto.getSubmitterLogin(), builder::setSubmitterLogin);
+    builder.setSubmittedAt(formatDateTime(new Date(dto.getCreatedAt())));
+    setNullable(dto.getStartedAt(), builder::setStartedAt, DateUtils::formatDateTime);
+    setNullable(computeExecutionTimeMs(dto), builder::setExecutionTimeMs);
     return builder.build();
   }
 
@@ -105,16 +98,16 @@ public class TaskFormatter {
 
   public List<WsCe.Task> formatActivity(DbSession dbSession, List<CeActivityDto> dtos) {
     ComponentDtoCache cache = ComponentDtoCache.forActivityDtos(dbClient, dbSession, dtos);
-    return dtos.stream().map(input -> formatActivity(input, cache, null)).collect(Collectors.toList());
+    return dtos.stream()
+      .map(input -> formatActivity(input, cache, null))
+      .collect(Collectors.toList(dtos.size()));
   }
 
   private static WsCe.Task formatActivity(CeActivityDto dto, ComponentDtoCache componentDtoCache, @Nullable String scannerContext) {
     WsCe.Task.Builder builder = WsCe.Task.newBuilder();
     String organizationKey = componentDtoCache.getOrganizationKey(dto.getComponentUuid());
     // FIXME organization field should be set from the CeActivityDto rather than from the ComponentDto
-    if (organizationKey != null) {
-      builder.setOrganization(organizationKey);
-    }
+    setNullable(organizationKey, builder::setOrganization);
     builder.setId(dto.getUuid());
     builder.setStatus(WsCe.TaskStatus.valueOf(dto.getStatus().name()));
     builder.setType(dto.getTaskType());
@@ -123,31 +116,15 @@ public class TaskFormatter {
       builder.setComponentId(dto.getComponentUuid());
       buildComponent(builder, componentDtoCache.getComponent(dto.getComponentUuid()));
     }
-    if (dto.getAnalysisUuid() != null) {
-      builder.setAnalysisId(dto.getAnalysisUuid());
-    }
-    if (dto.getSubmitterLogin() != null) {
-      builder.setSubmitterLogin(dto.getSubmitterLogin());
-    }
-    builder.setSubmittedAt(DateUtils.formatDateTime(new Date(dto.getSubmittedAt())));
-    if (dto.getStartedAt() != null) {
-      builder.setStartedAt(DateUtils.formatDateTime(new Date(dto.getStartedAt())));
-    }
-    if (dto.getExecutedAt() != null) {
-      builder.setExecutedAt(DateUtils.formatDateTime(new Date(dto.getExecutedAt())));
-    }
-    if (dto.getExecutionTimeMs() != null) {
-      builder.setExecutionTimeMs(dto.getExecutionTimeMs());
-    }
-    if (dto.getErrorMessage() != null) {
-      builder.setErrorMessage(dto.getErrorMessage());
-    }
-    if (dto.getErrorStacktrace() != null) {
-      builder.setErrorStacktrace(dto.getErrorStacktrace());
-    }
-    if (scannerContext != null) {
-      builder.setScannerContext(scannerContext);
-    }
+    setNullable(dto.getAnalysisUuid(), builder::setAnalysisId);
+    setNullable(dto.getSubmitterLogin(), builder::setSubmitterLogin);
+    builder.setSubmittedAt(formatDateTime(new Date(dto.getSubmittedAt())));
+    setNullable(dto.getStartedAt(), builder::setStartedAt, DateUtils::formatDateTime);
+    setNullable(dto.getExecutedAt(), builder::setExecutedAt, DateUtils::formatDateTime);
+    setNullable(dto.getExecutionTimeMs(), builder::setExecutionTimeMs);
+    setNullable(dto.getErrorMessage(), builder::setErrorMessage);
+    setNullable(dto.getErrorStacktrace(), builder::setErrorStacktrace);
+    setNullable(scannerContext, builder::setScannerContext);
     builder.setHasScannerContext(dto.isHasScannerContext());
     return builder.build();
   }
@@ -172,7 +149,7 @@ public class TaskFormatter {
     static ComponentDtoCache forQueueDtos(DbClient dbClient, DbSession dbSession, Collection<CeQueueDto> ceQueueDtos) {
       Map<String, ComponentDto> componentsByUuid = dbClient.componentDao().selectByUuids(dbSession, uuidOfCeQueueDtos(ceQueueDtos))
         .stream()
-        .collect(org.sonar.core.util.stream.Collectors.uniqueIndex(ComponentDto::uuid));
+        .collect(Collectors.uniqueIndex(ComponentDto::uuid));
       return new ComponentDtoCache(componentsByUuid, buildOrganizationsByUuid(dbClient, dbSession, componentsByUuid));
     }
 
@@ -181,7 +158,7 @@ public class TaskFormatter {
         .filter(Objects::nonNull)
         .map(CeQueueDto::getComponentUuid)
         .filter(Objects::nonNull)
-        .collect(org.sonar.core.util.stream.Collectors.toSet(ceQueueDtos.size()));
+        .collect(Collectors.toSet(ceQueueDtos.size()));
     }
 
     static ComponentDtoCache forActivityDtos(DbClient dbClient, DbSession dbSession, Collection<CeActivityDto> ceActivityDtos) {
@@ -189,7 +166,7 @@ public class TaskFormatter {
         dbSession,
         uuidOfCeActivityDtos(ceActivityDtos))
         .stream()
-        .collect(org.sonar.core.util.stream.Collectors.uniqueIndex(ComponentDto::uuid));
+        .collect(Collectors.uniqueIndex(ComponentDto::uuid));
       return new ComponentDtoCache(componentsByUuid, buildOrganizationsByUuid(dbClient, dbSession, componentsByUuid));
     }
 
@@ -198,7 +175,7 @@ public class TaskFormatter {
         .filter(Objects::nonNull)
         .map(CeActivityDto::getComponentUuid)
         .filter(Objects::nonNull)
-        .collect(org.sonar.core.util.stream.Collectors.toSet(ceActivityDtos.size()));
+        .collect(Collectors.toSet(ceActivityDtos.size()));
     }
 
     static ComponentDtoCache forUuid(DbClient dbClient, DbSession dbSession, String uuid) {
@@ -212,9 +189,9 @@ public class TaskFormatter {
         dbSession,
         componentsByUuid.values().stream()
           .map(ComponentDto::getOrganizationUuid)
-          .collect(Collectors.toSet()))
+          .collect(Collectors.toSet(componentsByUuid.size())))
         .stream()
-        .collect(org.sonar.core.util.stream.Collectors.uniqueIndex(OrganizationDto::getUuid));
+        .collect(Collectors.uniqueIndex(OrganizationDto::getUuid));
     }
 
     @CheckForNull
@@ -236,7 +213,7 @@ public class TaskFormatter {
       }
       String organizationUuid = componentDto.getOrganizationUuid();
       OrganizationDto organizationDto = organizationsByUuid.get(organizationUuid);
-      Preconditions.checkState(organizationDto != null, "Organization with uuid '%s' not found", organizationUuid);
+      checkState(organizationDto != null, "Organization with uuid '%s' not found", organizationUuid);
       return organizationDto.getKey();
     }
   }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java
index 48090f5a0d3698f58d6893ed4bfe884c4afc6913..27c27006da4610929c38bc7a1050517e129c98c2 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/ce/ws/ActivityActionTest.java
@@ -76,19 +76,18 @@ public class ActivityActionTest {
   @Rule
   public DbTester dbTester = DbTester.create(System2.INSTANCE);
 
-  ComponentDbTester componentDb = new ComponentDbTester(dbTester);
-
-  TaskFormatter formatter = new TaskFormatter(dbTester.getDbClient(), System2.INSTANCE);
-  ActivityAction underTest = new ActivityAction(userSession, dbTester.getDbClient(), formatter, new CeTaskProcessor[] {mock(CeTaskProcessor.class)});
-  WsActionTester ws = new WsActionTester(underTest);
+  private ComponentDbTester componentDb = new ComponentDbTester(dbTester);
+  private TaskFormatter formatter = new TaskFormatter(dbTester.getDbClient(), System2.INSTANCE);
+  private ActivityAction underTest = new ActivityAction(userSession, dbTester.getDbClient(), formatter, new CeTaskProcessor[] {mock(CeTaskProcessor.class)});
+  private WsActionTester ws = new WsActionTester(underTest);
 
   @Test
   public void get_all_past_activity() {
     globalAdmin();
-    OrganizationDto organization1Dto = dbTester.organizations().insert();
-    dbTester.components().insertProject(organization1Dto, "PROJECT_1");
-    OrganizationDto organization2Dto = dbTester.organizations().insert();
-    dbTester.components().insertProject(organization2Dto, "PROJECT_2");
+    OrganizationDto org1 = dbTester.organizations().insert();
+    dbTester.components().insertProject(org1, "PROJECT_1");
+    OrganizationDto org2 = dbTester.organizations().insert();
+    dbTester.components().insertProject(org2, "PROJECT_2");
     insertActivity("T1", "PROJECT_1", CeActivityDto.Status.SUCCESS);
     insertActivity("T2", "PROJECT_2", CeActivityDto.Status.FAILED);
 
@@ -98,19 +97,19 @@ public class ActivityActionTest {
     assertThat(activityResponse.getTasksCount()).isEqualTo(2);
     // chronological order, from newest to oldest
     WsCe.Task task = activityResponse.getTasks(0);
-    assertThat(task.getOrganization()).isEqualTo(organization2Dto.getKey());
+    assertThat(task.getOrganization()).isEqualTo(org2.getKey());
     assertThat(task.getId()).isEqualTo("T2");
     assertThat(task.getStatus()).isEqualTo(WsCe.TaskStatus.FAILED);
     assertThat(task.getComponentId()).isEqualTo("PROJECT_2");
     assertThat(task.getAnalysisId()).isEqualTo("U1");
     assertThat(task.getExecutionTimeMs()).isEqualTo(500L);
     assertThat(task.getLogs()).isFalse();
-    assertThat(activityResponse.getTasks(1).getId()).isEqualTo("T1");
-    assertThat(activityResponse.getTasks(1).getStatus()).isEqualTo(WsCe.TaskStatus.SUCCESS);
-    assertThat(activityResponse.getTasks(1).getComponentId()).isEqualTo("PROJECT_1");
-    assertThat(activityResponse.getTasks(1).getLogs()).isFalse();
-
-    assertThat(activityResponse.getTasks(1).getOrganization()).isEqualTo(organization1Dto.getKey());
+    task = activityResponse.getTasks(1);
+    assertThat(task.getId()).isEqualTo("T1");
+    assertThat(task.getStatus()).isEqualTo(WsCe.TaskStatus.SUCCESS);
+    assertThat(task.getComponentId()).isEqualTo("PROJECT_1");
+    assertThat(task.getLogs()).isFalse();
+    assertThat(task.getOrganization()).isEqualTo(org1.getKey());
   }
 
   @Test
@@ -229,7 +228,6 @@ public class ActivityActionTest {
     componentDb.insertProjectAndSnapshot(struts);
     componentDb.insertProjectAndSnapshot(zookeeper);
     componentDb.insertProjectAndSnapshot(eclipse);
-    dbTester.commit();
     globalAdmin();
     insertActivity("T1", "P1", CeActivityDto.Status.SUCCESS);
     insertActivity("T2", "P2", CeActivityDto.Status.SUCCESS);