From 7953067436449b18d1b0cbbf69575087dece5bee Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 17 Oct 2017 16:38:09 +0200 Subject: [PATCH] SONAR-9926 CeQueue#cancel now needs CeQueuDto instead of uuid --- .../ce/queue/InternalCeQueueImplTest.java | 135 +++++++++--------- .../main/java/org/sonar/ce/queue/CeQueue.java | 8 +- .../java/org/sonar/ce/queue/CeQueueImpl.java | 31 ++-- .../org/sonar/server/ce/ws/CancelAction.java | 25 +++- .../org/sonar/ce/queue/CeQueueImplTest.java | 53 ++++--- .../sonar/server/ce/ws/CancelActionTest.java | 53 +++++-- 6 files changed, 174 insertions(+), 131 deletions(-) diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java index eb34e2cee5c6..74b2a9503f7a 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/queue/InternalCeQueueImplTest.java @@ -19,13 +19,22 @@ */ package org.sonar.ce.queue; -import com.google.common.collect.ImmutableSet; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.ce.container.ComputeEngineStatus.Status.STARTED; +import static org.sonar.ce.container.ComputeEngineStatus.Status.STOPPING; + import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.util.List; import java.util.Optional; import java.util.Random; + import javax.annotation.Nullable; + import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; @@ -50,13 +59,7 @@ import org.sonar.server.organization.DefaultOrganization; import org.sonar.server.organization.DefaultOrganizationProvider; -import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Arrays.asList; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.sonar.ce.container.ComputeEngineStatus.Status.STARTED; -import static org.sonar.ce.container.ComputeEngineStatus.Status.STOPPING; +import com.google.common.collect.ImmutableSet; public class InternalCeQueueImplTest { @@ -69,19 +72,19 @@ public class InternalCeQueueImplTest { @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule - public DbTester dbTester = DbTester.create(system2); + public DbTester db = DbTester.create(system2); - private DbSession session = dbTester.getSession(); + private DbSession session = db.getSession(); private UuidFactory uuidFactory = UuidFactoryImpl.INSTANCE; - private CEQueueStatus queueStatus = new CEQueueStatusImpl(dbTester.getDbClient()); + private CEQueueStatus queueStatus = new CEQueueStatusImpl(db.getDbClient()); private DefaultOrganizationProvider defaultOrganizationProvider = mock(DefaultOrganizationProvider.class); private ComputeEngineStatus computeEngineStatus = mock(ComputeEngineStatus.class); - private InternalCeQueue underTest = new InternalCeQueueImpl(system2, dbTester.getDbClient(), uuidFactory, queueStatus, defaultOrganizationProvider, computeEngineStatus); + private InternalCeQueue underTest = new InternalCeQueueImpl(system2, db.getDbClient(), uuidFactory, queueStatus, defaultOrganizationProvider, computeEngineStatus); @Before public void setUp() throws Exception { - OrganizationDto defaultOrganization = dbTester.getDefaultOrganization(); + OrganizationDto defaultOrganization = db.getDefaultOrganization(); when(defaultOrganizationProvider.get()).thenReturn(DefaultOrganization.newBuilder() .setUuid(defaultOrganization.getUuid()) .setKey(defaultOrganization.getKey()) @@ -172,11 +175,11 @@ public void test_remove() { underTest.remove(peek.get(), CeActivityDto.Status.SUCCESS, null, null); // queue is empty - assertThat(dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), task.getUuid()).isPresent()).isFalse(); + assertThat(db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), task.getUuid()).isPresent()).isFalse(); assertThat(underTest.peek(WORKER_UUID_2).isPresent()).isFalse(); // available in history - Optional history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), task.getUuid()); + Optional history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid()); assertThat(history.isPresent()).isTrue(); assertThat(history.get().getStatus()).isEqualTo(CeActivityDto.Status.SUCCESS); assertThat(history.get().getIsLast()).isTrue(); @@ -206,7 +209,7 @@ public void remove_does_not_set_analysisUuid_in_CeActivity_when_CeTaskResult_has underTest.remove(peek.get(), CeActivityDto.Status.SUCCESS, newTaskResult(null), null); // available in history - Optional history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), task.getUuid()); + Optional history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid()); assertThat(history.isPresent()).isTrue(); assertThat(history.get().getAnalysisUuid()).isNull(); } @@ -219,7 +222,7 @@ public void remove_sets_analysisUuid_in_CeActivity_when_CeTaskResult_has_analysi underTest.remove(peek.get(), CeActivityDto.Status.SUCCESS, newTaskResult(AN_ANALYSIS_UUID), null); // available in history - Optional history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), task.getUuid()); + Optional history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid()); assertThat(history.isPresent()).isTrue(); assertThat(history.get().getAnalysisUuid()).isEqualTo("U1"); } @@ -232,7 +235,7 @@ public void remove_saves_error_message_and_stacktrace_when_exception_is_provided Optional peek = underTest.peek(WORKER_UUID_1); underTest.remove(peek.get(), CeActivityDto.Status.FAILED, null, error); - Optional activityDto = dbTester.getDbClient().ceActivityDao().selectByUuid(session, task.getUuid()); + Optional activityDto = db.getDbClient().ceActivityDao().selectByUuid(session, task.getUuid()); assertThat(activityDto).isPresent(); assertThat(activityDto.get().getErrorMessage()).isEqualTo(error.getMessage()); @@ -248,7 +251,7 @@ public void remove_saves_error_when_TypedMessageException_is_provided() { Optional peek = underTest.peek(WORKER_UUID_1); underTest.remove(peek.get(), CeActivityDto.Status.FAILED, null, error); - CeActivityDto activityDto = dbTester.getDbClient().ceActivityDao().selectByUuid(session, task.getUuid()).get(); + CeActivityDto activityDto = db.getDbClient().ceActivityDao().selectByUuid(session, task.getUuid()).get(); assertThat(activityDto.getErrorType()).isEqualTo("aType"); assertThat(activityDto.getErrorMessage()).isEqualTo("aMessage"); assertThat(activityDto.getErrorStacktrace()).isEqualToIgnoringWhitespace(stacktraceToString(error)); @@ -270,13 +273,13 @@ public String getType() { @Test public void remove_copies_executionCount_and_workerUuid() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setWorkerUuid("Dustin") .setExecutionCount(2)); - dbTester.commit(); + db.commit(); underTest.remove(new CeTask.Builder() .setOrganizationUuid("foo") @@ -284,7 +287,7 @@ public void remove_copies_executionCount_and_workerUuid() { .setType("bar") .build(), CeActivityDto.Status.SUCCESS, null, null); - CeActivityDto dto = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), "uuid").get(); + CeActivityDto dto = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), "uuid").get(); assertThat(dto.getExecutionCount()).isEqualTo(2); assertThat(dto.getWorkerUuid()).isEqualTo("Dustin"); } @@ -316,16 +319,16 @@ public void test_peek() throws Exception { @Test public void peek_overrides_workerUuid_to_argument() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setWorkerUuid("must be overriden")); - dbTester.commit(); + db.commit(); underTest.peek(WORKER_UUID_1); - CeQueueDto ceQueueDto = dbTester.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get(); + CeQueueDto ceQueueDto = db.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get(); assertThat(ceQueueDto.getWorkerUuid()).isEqualTo(WORKER_UUID_1); } @@ -340,50 +343,50 @@ public void peek_nothing_if_application_status_stopping() throws Exception { @Test public void peek_peeks_pending_tasks_with_executionCount_equal_to_0_and_increases_it() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setExecutionCount(0)); - dbTester.commit(); + db.commit(); assertThat(underTest.peek(WORKER_UUID_1).get().getUuid()).isEqualTo("uuid"); - assertThat(dbTester.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get().getExecutionCount()).isEqualTo(1); + assertThat(db.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get().getExecutionCount()).isEqualTo(1); } @Test public void peek_peeks_pending_tasks_with_executionCount_equal_to_1_and_increases_it() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setExecutionCount(1)); - dbTester.commit(); + db.commit(); assertThat(underTest.peek(WORKER_UUID_1).get().getUuid()).isEqualTo("uuid"); - assertThat(dbTester.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get().getExecutionCount()).isEqualTo(2); + assertThat(db.getDbClient().ceQueueDao().selectByUuid(session, "uuid").get().getExecutionCount()).isEqualTo(2); } @Test public void peek_ignores_pending_tasks_with_executionCount_equal_to_2() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setExecutionCount(2)); - dbTester.commit(); + db.commit(); assertThat(underTest.peek(WORKER_UUID_1).isPresent()).isFalse(); } @Test public void peek_ignores_pending_tasks_with_executionCount_greater_than_2() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setExecutionCount(2 + Math.abs(new Random().nextInt(100)))); - dbTester.commit(); + db.commit(); assertThat(underTest.peek(WORKER_UUID_1).isPresent()).isFalse(); } @@ -451,7 +454,7 @@ public void peek_resets_to_pending_any_task_in_progress_for_specified_worker_uui } private void verifyResetTask(CeQueueDto originalDto) { - CeQueueDto dto = dbTester.getDbClient().ceQueueDao().selectByUuid(session, originalDto.getUuid()).get(); + CeQueueDto dto = db.getDbClient().ceQueueDao().selectByUuid(session, originalDto.getUuid()).get(); assertThat(dto.getStatus()).isEqualTo(CeQueueDto.Status.PENDING); assertThat(dto.getExecutionCount()).isEqualTo(originalDto.getExecutionCount()); assertThat(dto.getCreatedAt()).isEqualTo(originalDto.getCreatedAt()); @@ -459,7 +462,7 @@ private void verifyResetTask(CeQueueDto originalDto) { } private void verifyUnmodifiedTask(CeQueueDto originalDto) { - CeQueueDto dto = dbTester.getDbClient().ceQueueDao().selectByUuid(session, originalDto.getUuid()).get(); + CeQueueDto dto = db.getDbClient().ceQueueDao().selectByUuid(session, originalDto.getUuid()).get(); assertThat(dto.getStatus()).isEqualTo(originalDto.getStatus()); assertThat(dto.getExecutionCount()).isEqualTo(originalDto.getExecutionCount()); assertThat(dto.getCreatedAt()).isEqualTo(originalDto.getCreatedAt()); @@ -474,8 +477,8 @@ private CeQueueDto insertInProgress(String uuid, String workerUuid, int executio .setStatus(CeQueueDto.Status.IN_PROGRESS) .setWorkerUuid(workerUuid) .setExecutionCount(executionCount); - dbTester.getDbClient().ceQueueDao().insert(session, dto); - dbTester.commit(); + db.getDbClient().ceQueueDao().insert(session, dto); + db.commit(); return dto; } @@ -487,52 +490,50 @@ private CeQueueDto insertPending(String uuid, String workerUuid, int executionCo .setStatus(CeQueueDto.Status.PENDING) .setWorkerUuid(workerUuid) .setExecutionCount(executionCount); - dbTester.getDbClient().ceQueueDao().insert(session, dto); - dbTester.commit(); + db.getDbClient().ceQueueDao().insert(session, dto); + db.commit(); return dto; } @Test public void cancel_pending() throws Exception { CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); + CeQueueDto queueDto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), task.getUuid()).get(); - // ignore - boolean canceled = underTest.cancel("UNKNOWN"); - assertThat(canceled).isFalse(); + underTest.cancel(db.getSession(), queueDto); - canceled = underTest.cancel(task.getUuid()); - assertThat(canceled).isTrue(); - Optional activity = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), task.getUuid()); + Optional activity = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid()); assertThat(activity.isPresent()).isTrue(); assertThat(activity.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); } @Test public void cancel_copies_executionCount_and_workerUuid() { - dbTester.getDbClient().ceQueueDao().insert(session, new CeQueueDto() + CeQueueDto ceQueueDto = db.getDbClient().ceQueueDao().insert(session, new CeQueueDto() .setUuid("uuid") .setTaskType("foo") .setStatus(CeQueueDto.Status.PENDING) .setWorkerUuid("Dustin") .setExecutionCount(2)); - dbTester.commit(); + db.commit(); - underTest.cancel("uuid"); + underTest.cancel(db.getSession(), ceQueueDto); - CeActivityDto dto = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), "uuid").get(); + CeActivityDto dto = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), "uuid").get(); assertThat(dto.getExecutionCount()).isEqualTo(2); assertThat(dto.getWorkerUuid()).isEqualTo("Dustin"); } @Test public void fail_to_cancel_if_in_progress() throws Exception { - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage(Matchers.startsWith("Task is in progress and can't be canceled")); - CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); underTest.peek(WORKER_UUID_2); + CeQueueDto queueDto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), task.getUuid()).get(); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage(Matchers.startsWith("Task is in progress and can't be canceled")); - underTest.cancel(task.getUuid()); + underTest.cancel(db.getSession(), queueDto); } @Test @@ -545,11 +546,11 @@ public void cancelAll_pendings_but_not_in_progress() throws Exception { int canceledCount = underTest.cancelAll(); assertThat(canceledCount).isEqualTo(2); - Optional history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), pendingTask1.getUuid()); + Optional history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), pendingTask1.getUuid()); assertThat(history.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); - history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), pendingTask2.getUuid()); + history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), pendingTask2.getUuid()); assertThat(history.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); - history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), inProgressTask.getUuid()); + history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), inProgressTask.getUuid()); assertThat(history.isPresent()).isFalse(); } @@ -657,7 +658,7 @@ public void resetTasksWithUnknownWorkerUUIDs_with_worker_without_tasks_will_rese } private void verifyReset(CeQueueDto original) { - CeQueueDto dto = dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), original.getUuid()).get(); + CeQueueDto dto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), original.getUuid()).get(); // We do not touch ExecutionCount nor CreatedAt assertThat(dto.getExecutionCount()).isEqualTo(original.getExecutionCount()); assertThat(dto.getCreatedAt()).isEqualTo(original.getCreatedAt()); @@ -673,7 +674,7 @@ private void verifyReset(CeQueueDto original) { } private void verifyUnmodified(CeQueueDto original) { - CeQueueDto dto = dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), original.getUuid()).get(); + CeQueueDto dto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), original.getUuid()).get(); assertThat(dto.getStatus()).isEqualTo(original.getStatus()); assertThat(dto.getExecutionCount()).isEqualTo(original.getExecutionCount()); assertThat(dto.getCreatedAt()).isEqualTo(original.getCreatedAt()); @@ -681,8 +682,8 @@ private void verifyUnmodified(CeQueueDto original) { } private void verifyCanceled(CeQueueDto original) { - assertThat(dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), original.getUuid())).isEmpty(); - CeActivityDto dto = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), original.getUuid()).get(); + assertThat(db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), original.getUuid())).isEmpty(); + CeActivityDto dto = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), original.getUuid()).get(); assertThat(dto.getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); assertThat(dto.getExecutionCount()).isEqualTo(original.getExecutionCount()); assertThat(dto.getWorkerUuid()).isEqualTo(original.getWorkerUuid()); @@ -695,8 +696,8 @@ private CeQueueDto insertCeQueueDto(String uuid, CeQueueDto.Status status, int e .setStatus(status) .setExecutionCount(executionCount) .setWorkerUuid(workerUuid); - dbTester.getDbClient().ceQueueDao().insert(dbTester.getSession(), dto); - dbTester.commit(); + db.getDbClient().ceQueueDao().insert(db.getSession(), dto); + db.commit(); return dto; } @@ -729,7 +730,7 @@ private void verifyCeTask(CeTaskSubmit taskSubmit, CeTask task, @Nullable Compon } private void verifyCeQueueDtoForTaskSubmit(CeTaskSubmit taskSubmit) { - Optional queueDto = dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), taskSubmit.getUuid()); + Optional queueDto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), taskSubmit.getUuid()); assertThat(queueDto.isPresent()).isTrue(); CeQueueDto dto = queueDto.get(); assertThat(dto.getTaskType()).isEqualTo(taskSubmit.getType()); @@ -739,7 +740,7 @@ private void verifyCeQueueDtoForTaskSubmit(CeTaskSubmit taskSubmit) { } private ComponentDto newComponentDto(String uuid) { - return ComponentTesting.newPublicProjectDto(dbTester.getDefaultOrganization(), uuid).setName("name_" + uuid).setDbKey("key_" + uuid); + return ComponentTesting.newPublicProjectDto(db.getDefaultOrganization(), uuid).setName("name_" + uuid).setDbKey("key_" + uuid); } private CeTask submit(String reportType, String componentUuid) { @@ -765,7 +766,7 @@ private CeTaskResult newTaskResult(@Nullable String analysisUuid) { } private ComponentDto insertComponent(ComponentDto componentDto) { - dbTester.getDbClient().componentDao().insert(session, componentDto); + db.getDbClient().componentDao().insert(session, componentDto); session.commit(); return componentDto; } diff --git a/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueue.java b/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueue.java index 024fcfbd0f21..109e3bd22da1 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueue.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueue.java @@ -22,6 +22,9 @@ import java.util.Collection; import java.util.List; +import org.sonar.db.DbSession; +import org.sonar.db.ce.CeQueueDto; + /** * Queue of pending Compute Engine tasks. Both producer and consumer actions * are implemented. @@ -60,11 +63,8 @@ public interface CeQueue { /** * Cancels a task in status {@link org.sonar.db.ce.CeQueueDto.Status#PENDING}. An unchecked * exception is thrown if the status is not {@link org.sonar.db.ce.CeQueueDto.Status#PENDING}. - * The method does nothing and returns {@code false} if the task does not exist. - * - * @return true if the task exists and is successfully canceled. */ - boolean cancel(String taskUuid); + void cancel(DbSession dbSession, CeQueueDto ceQueueDto); /** * Removes all the tasks from the queue, except the tasks with status diff --git a/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueueImpl.java b/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueueImpl.java index 27bc6c5fc1e8..5c31cd353185 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueueImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/queue/CeQueueImpl.java @@ -19,17 +19,22 @@ */ package org.sonar.ce.queue; -import com.google.common.base.Function; -import com.google.common.collect.ImmutableMap; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.base.Predicates.notNull; +import static com.google.common.collect.FluentIterable.from; +import static java.util.Collections.singleton; +import static java.util.Objects.requireNonNull; + import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; + import javax.annotation.Nonnull; import javax.annotation.Nullable; + import org.sonar.api.ce.ComputeEngineSide; import org.sonar.core.util.UuidFactory; import org.sonar.db.DbClient; @@ -39,11 +44,8 @@ import org.sonar.db.component.ComponentDto; import org.sonar.server.organization.DefaultOrganizationProvider; -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.FluentIterable.from; -import static java.util.Collections.singleton; -import static java.util.Objects.requireNonNull; +import com.google.common.base.Function; +import com.google.common.collect.ImmutableMap; @ComputeEngineSide public class CeQueueImpl implements CeQueue { @@ -121,16 +123,9 @@ private List loadTasks(DbSession dbSession, List dtos) { } @Override - public boolean cancel(String taskUuid) { - try (DbSession dbSession = dbClient.openSession(false)) { - Optional queueDto = dbClient.ceQueueDao().selectByUuid(dbSession, taskUuid); - if (queueDto.isPresent()) { - checkState(CeQueueDto.Status.PENDING.equals(queueDto.get().getStatus()), "Task is in progress and can't be canceled [uuid=%s]", taskUuid); - cancelImpl(dbSession, queueDto.get()); - return true; - } - return false; - } + public void cancel(DbSession dbSession, CeQueueDto ceQueueDto) { + checkState(CeQueueDto.Status.PENDING.equals(ceQueueDto.getStatus()), "Task is in progress and can't be canceled [uuid=%s]", ceQueueDto.getUuid()); + cancelImpl(dbSession, ceQueueDto); } private void cancelImpl(DbSession dbSession, CeQueueDto q) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/CancelAction.java b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/CancelAction.java index 1914df86da43..4ca31bd88d88 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ce/ws/CancelAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ce/ws/CancelAction.java @@ -19,11 +19,16 @@ */ package org.sonar.server.ce.ws; +import java.util.Optional; + import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.ce.queue.CeQueue; import org.sonar.core.util.Uuids; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.ce.CeQueueDto; import org.sonar.server.user.UserSession; public class CancelAction implements CeWsAction { @@ -31,17 +36,25 @@ public class CancelAction implements CeWsAction { public static final String PARAM_TASK_ID = "id"; private final UserSession userSession; + private DbClient dbClient; private final CeQueue queue; - public CancelAction(UserSession userSession, CeQueue queue) { + public CancelAction(UserSession userSession, DbClient dbClient, CeQueue queue) { this.userSession = userSession; + this.dbClient = dbClient; this.queue = queue; } @Override public void define(WebService.NewController controller) { WebService.NewAction action = controller.createAction("cancel") - .setDescription("Cancels a pending task. Requires system administration permission. In-progress tasks cannot be canceled.") + .setDescription("Cancels a pending task.
" + + "In-progress tasks cannot be canceled.
" + + "Requires one of the following permissions:" + + "
    " + + "
  • 'Administer System'
    • " + + "
  • 'Administer' rights on the project related to the task
    • " + + "
") .setInternal(true) .setPost(true) .setSince("5.2") @@ -58,7 +71,13 @@ public void define(WebService.NewController controller) { public void handle(Request wsRequest, Response wsResponse) { userSession.checkIsSystemAdministrator(); String taskId = wsRequest.mandatoryParam(PARAM_TASK_ID); - queue.cancel(taskId); + try (DbSession dbSession = dbClient.openSession(false)) { + Optional queueDto = dbClient.ceQueueDao().selectByUuid(dbSession, taskId); + queueDto.ifPresent(dto -> { + queue.cancel(dbSession, dto); + }); + } wsResponse.noContent(); } + } diff --git a/server/sonar-server/src/test/java/org/sonar/ce/queue/CeQueueImplTest.java b/server/sonar-server/src/test/java/org/sonar/ce/queue/CeQueueImplTest.java index 8109a82d442d..fc5419594a69 100644 --- a/server/sonar-server/src/test/java/org/sonar/ce/queue/CeQueueImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/ce/queue/CeQueueImplTest.java @@ -19,9 +19,15 @@ */ package org.sonar.ce.queue; +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.startsWith; + import java.util.List; import java.util.Optional; + import javax.annotation.Nullable; + import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -39,10 +45,6 @@ import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import static java.util.Arrays.asList; -import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.startsWith; - public class CeQueueImplTest { private static final String WORKER_UUID = "workerUuid"; @@ -53,18 +55,19 @@ public class CeQueueImplTest { @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule - public DbTester dbTester = DbTester.create(system2); + public DbTester db = DbTester.create(system2); - private DbSession session = dbTester.getSession(); + private DbSession session = db.getSession(); private UuidFactory uuidFactory = UuidFactoryImpl.INSTANCE; - private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester); + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); - private CeQueue underTest = new CeQueueImpl(dbTester.getDbClient(), uuidFactory, defaultOrganizationProvider); + private CeQueue underTest = new CeQueueImpl(db.getDbClient(), uuidFactory, defaultOrganizationProvider); @Test public void submit_returns_task_populated_from_CeTaskSubmit_and_creates_CeQueue_row() { CeTaskSubmit taskSubmit = createTaskSubmit(CeTaskTypes.REPORT, "PROJECT_1", "rob"); + CeTask task = underTest.submit(taskSubmit); verifyCeTask(taskSubmit, task, null); @@ -73,7 +76,7 @@ public void submit_returns_task_populated_from_CeTaskSubmit_and_creates_CeQueue_ @Test public void submit_populates_component_name_and_key_of_CeTask_if_component_exists() { - ComponentDto componentDto = insertComponent(ComponentTesting.newPrivateProjectDto(dbTester.organizations().insert(), "PROJECT_1")); + ComponentDto componentDto = insertComponent(ComponentTesting.newPrivateProjectDto(db.organizations().insert(), "PROJECT_1")); CeTaskSubmit taskSubmit = createTaskSubmit(CeTaskTypes.REPORT, componentDto.uuid(), null); CeTask task = underTest.submit(taskSubmit); @@ -116,7 +119,7 @@ public void massSubmit_returns_tasks_for_each_CeTaskSubmit_populated_from_CeTask @Test public void massSubmit_populates_component_name_and_key_of_CeTask_if_component_exists() { - ComponentDto componentDto1 = insertComponent(ComponentTesting.newPrivateProjectDto(dbTester.getDefaultOrganization(), "PROJECT_1")); + ComponentDto componentDto1 = insertComponent(ComponentTesting.newPrivateProjectDto(db.getDefaultOrganization(), "PROJECT_1")); CeTaskSubmit taskSubmit1 = createTaskSubmit(CeTaskTypes.REPORT, componentDto1.uuid(), null); CeTaskSubmit taskSubmit2 = createTaskSubmit("something", "non existing component uuid", null); @@ -130,28 +133,24 @@ public void massSubmit_populates_component_name_and_key_of_CeTask_if_component_e @Test public void cancel_pending() throws Exception { CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); + CeQueueDto queueDto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), task.getUuid()).get(); - // ignore - boolean canceled = underTest.cancel("UNKNOWN"); - assertThat(canceled).isFalse(); + underTest.cancel(db.getSession(), queueDto); - canceled = underTest.cancel(task.getUuid()); - assertThat(canceled).isTrue(); - Optional activity = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), task.getUuid()); + Optional activity = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), task.getUuid()); assertThat(activity.isPresent()).isTrue(); assertThat(activity.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); } @Test public void fail_to_cancel_if_in_progress() throws Exception { + submit(CeTaskTypes.REPORT, "PROJECT_1"); + CeQueueDto ceQueueDto = db.getDbClient().ceQueueDao().peek(session, WORKER_UUID, MAX_EXECUTION_COUNT).get(); + expectedException.expect(IllegalStateException.class); expectedException.expectMessage(startsWith("Task is in progress and can't be canceled")); - CeTask task = submit(CeTaskTypes.REPORT, "PROJECT_1"); - - dbTester.getDbClient().ceQueueDao().peek(session, WORKER_UUID, MAX_EXECUTION_COUNT); - - underTest.cancel(task.getUuid()); + underTest.cancel(db.getSession(), ceQueueDto); } @Test @@ -160,16 +159,16 @@ public void cancelAll_pendings_but_not_in_progress() throws Exception { CeTask pendingTask1 = submit(CeTaskTypes.REPORT, "PROJECT_2"); CeTask pendingTask2 = submit(CeTaskTypes.REPORT, "PROJECT_3"); - dbTester.getDbClient().ceQueueDao().peek(session, WORKER_UUID, MAX_EXECUTION_COUNT); + db.getDbClient().ceQueueDao().peek(session, WORKER_UUID, MAX_EXECUTION_COUNT); int canceledCount = underTest.cancelAll(); assertThat(canceledCount).isEqualTo(2); - Optional history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), pendingTask1.getUuid()); + Optional history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), pendingTask1.getUuid()); assertThat(history.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); - history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), pendingTask2.getUuid()); + history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), pendingTask2.getUuid()); assertThat(history.get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); - history = dbTester.getDbClient().ceActivityDao().selectByUuid(dbTester.getSession(), inProgressTask.getUuid()); + history = db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), inProgressTask.getUuid()); assertThat(history.isPresent()).isFalse(); } @@ -202,7 +201,7 @@ private void verifyCeTask(CeTaskSubmit taskSubmit, CeTask task, @Nullable Compon } private void verifyCeQueueDtoForTaskSubmit(CeTaskSubmit taskSubmit) { - Optional queueDto = dbTester.getDbClient().ceQueueDao().selectByUuid(dbTester.getSession(), taskSubmit.getUuid()); + Optional queueDto = db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), taskSubmit.getUuid()); assertThat(queueDto.isPresent()).isTrue(); assertThat(queueDto.get().getTaskType()).isEqualTo(taskSubmit.getType()); assertThat(queueDto.get().getComponentUuid()).isEqualTo(taskSubmit.getComponentUuid()); @@ -227,7 +226,7 @@ private CeTaskSubmit createTaskSubmit(String type, @Nullable String componentUui } private ComponentDto insertComponent(ComponentDto componentDto) { - dbTester.getDbClient().componentDao().insert(session, componentDto); + db.getDbClient().componentDao().insert(session, componentDto); session.commit(); return componentDto; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/CancelActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ce/ws/CancelActionTest.java index 34a6430c0ed1..51458ced9195 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ce/ws/CancelActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ce/ws/CancelActionTest.java @@ -19,38 +19,62 @@ */ package org.sonar.server.ce.ws; +import static org.assertj.core.api.Java6Assertions.assertThat; + import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.ce.queue.CeQueue; +import org.sonar.ce.queue.CeQueueImpl; +import org.sonar.ce.queue.CeTask; +import org.sonar.ce.queue.CeTaskSubmit; +import org.sonar.core.util.UuidFactoryFast; +import org.sonar.db.DbTester; +import org.sonar.db.ce.CeActivityDto; +import org.sonar.db.ce.CeQueueDto; +import org.sonar.db.ce.CeTaskTypes; +import org.sonar.db.component.ComponentDto; import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsActionTester; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; - public class CancelActionTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); @Rule public ExpectedException expectedException = ExpectedException.none(); + @Rule + public DbTester db = DbTester.create(); + + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); + private CeQueue queue = new CeQueueImpl(db.getDbClient(), UuidFactoryFast.getInstance(), defaultOrganizationProvider); - private CeQueue queue = mock(CeQueue.class); - private CancelAction underTest = new CancelAction(userSession, queue); + private CancelAction underTest = new CancelAction(userSession, db.getDbClient(), queue); private WsActionTester tester = new WsActionTester(underTest); @Test public void cancel_pending_task() { logInAsSystemAdministrator(); + ComponentDto project = db.components().insertPrivateProject(); + CeQueueDto queue = createTaskSubmit(project); tester.newRequest() - .setParam("id", "T1") + .setParam("id", queue.getUuid()) .execute(); - verify(queue).cancel("T1"); + assertThat(db.getDbClient().ceActivityDao().selectByUuid(db.getSession(), queue.getUuid()).get().getStatus()).isEqualTo(CeActivityDto.Status.CANCELED); + } + + @Test + public void does_not_fail_on_unknown_task() { + logInAsSystemAdministrator(); + + tester.newRequest() + .setParam("id", "UNKNOWN") + .execute(); } @Test @@ -61,8 +85,6 @@ public void throw_IllegalArgumentException_if_missing_id() { expectedException.expectMessage("The 'id' parameter is missing"); tester.newRequest().execute(); - - verifyZeroInteractions(queue); } @Test @@ -75,11 +97,18 @@ public void throw_ForbiddenException_if_not_system_administrator() { tester.newRequest() .setParam("id", "T1") .execute(); - - verifyZeroInteractions(queue); } private void logInAsSystemAdministrator() { userSession.logIn().setSystemAdministrator(); } + + private CeQueueDto createTaskSubmit(ComponentDto component) { + CeTaskSubmit.Builder submission = queue.prepareSubmit(); + submission.setType(CeTaskTypes.REPORT); + submission.setComponentUuid(component.uuid()); + submission.setSubmitterLogin(null); + CeTask task = queue.submit(submission.build()); + return db.getDbClient().ceQueueDao().selectByUuid(db.getSession(), task.getUuid()).get(); + } }