From f14146982f403cb7d80ed17f2b290cd1d99c693f Mon Sep 17 00:00:00 2001
From: Tanguy Leroux <tlrx.dev@gmail.com>
Date: Fri, 16 Mar 2018 10:20:56 +0100
Subject: [PATCH] Use removeTask instead of finishTask in
 PersistentTasksClusterService (#29055)

The method `PersistentTasksClusterService.finishTask()` has been
modified since it was added and does not use any `removeOncompletion`
flag anymore. Its behavior is now similar to `removeTask()` and can be
replaced by this one. When a non existing task is removed, the cluster
state update task will fail and its `source` will still indicate
`finish persistent task`/`remove persistent task`.
---
 .../PersistentTasksClusterService.java        |  2 +-
 .../PersistentTasksCustomMetaData.java        | 22 +++----------------
 .../PersistentTasksCustomMetaDataTests.java   | 11 +---------
 3 files changed, 5 insertions(+), 30 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java b/server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java
index 24d8c5f7be3..7c395365c1b 100644
--- a/server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java
+++ b/server/src/main/java/org/elasticsearch/persistent/PersistentTasksClusterService.java
@@ -117,7 +117,7 @@ public class PersistentTasksClusterService extends AbstractComponent implements
             public ClusterState execute(ClusterState currentState) throws Exception {
                 PersistentTasksCustomMetaData.Builder tasksInProgress = builder(currentState);
                 if (tasksInProgress.hasTask(id, allocationId)) {
-                    tasksInProgress.finishTask(id);
+                    tasksInProgress.removeTask(id);
                     return update(currentState, tasksInProgress);
                 } else {
                     if (tasksInProgress.hasTask(id)) {
diff --git a/server/src/main/java/org/elasticsearch/persistent/PersistentTasksCustomMetaData.java b/server/src/main/java/org/elasticsearch/persistent/PersistentTasksCustomMetaData.java
index 25b3567ac39..ee45eb8ffad 100644
--- a/server/src/main/java/org/elasticsearch/persistent/PersistentTasksCustomMetaData.java
+++ b/server/src/main/java/org/elasticsearch/persistent/PersistentTasksCustomMetaData.java
@@ -609,7 +609,7 @@ public final class PersistentTasksCustomMetaData extends AbstractNamedDiffable<M
                 changed = true;
                 tasks.put(taskId, new PersistentTask<>(taskInProgress, getNextAllocationId(), assignment));
             } else {
-                throw new ResourceNotFoundException("cannot reassign task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot reassign task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }
@@ -623,7 +623,7 @@ public final class PersistentTasksCustomMetaData extends AbstractNamedDiffable<M
                 changed = true;
                 tasks.put(taskId, new PersistentTask<>(taskInProgress, status));
             } else {
-                throw new ResourceNotFoundException("cannot update task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot update task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }
@@ -635,23 +635,7 @@ public final class PersistentTasksCustomMetaData extends AbstractNamedDiffable<M
             if (tasks.remove(taskId) != null) {
                 changed = true;
             } else {
-                throw new ResourceNotFoundException("cannot remove task with id {" + taskId + "}, the task no longer exits");
-            }
-            return this;
-        }
-
-        /**
-         * Finishes the task
-         * <p>
-         * If the task is marked with removeOnCompletion flag, it is removed from the list, otherwise it is stopped.
-         */
-        public Builder finishTask(String taskId) {
-            PersistentTask<?> taskInProgress = tasks.get(taskId);
-            if (taskInProgress != null) {
-                changed = true;
-                tasks.remove(taskId);
-            } else {
-                throw new ResourceNotFoundException("cannot finish task with id {" + taskId + "}, the task no longer exits");
+                throw new ResourceNotFoundException("cannot remove task with id {" + taskId + "}, the task no longer exists");
             }
             return this;
         }
diff --git a/server/src/test/java/org/elasticsearch/persistent/PersistentTasksCustomMetaDataTests.java b/server/src/test/java/org/elasticsearch/persistent/PersistentTasksCustomMetaDataTests.java
index 537fc21ed43..67962b800d2 100644
--- a/server/src/test/java/org/elasticsearch/persistent/PersistentTasksCustomMetaDataTests.java
+++ b/server/src/test/java/org/elasticsearch/persistent/PersistentTasksCustomMetaDataTests.java
@@ -191,7 +191,7 @@ public class PersistentTasksCustomMetaDataTests extends AbstractDiffableSerializ
             }
             boolean changed = false;
             for (int j = 0; j < randomIntBetween(1, 10); j++) {
-                switch (randomInt(4)) {
+                switch (randomInt(3)) {
                     case 0:
                         lastKnownTask = addRandomTask(builder);
                         changed = true;
@@ -223,15 +223,6 @@ public class PersistentTasksCustomMetaDataTests extends AbstractDiffableSerializ
                             expectThrows(ResourceNotFoundException.class, () -> builder.removeTask(fLastKnownTask));
                         }
                         break;
-                    case 4:
-                        if (builder.hasTask(lastKnownTask)) {
-                            changed = true;
-                            builder.finishTask(lastKnownTask);
-                        } else {
-                            String fLastKnownTask = lastKnownTask;
-                            expectThrows(ResourceNotFoundException.class, () -> builder.finishTask(fLastKnownTask));
-                        }
-                        break;
                 }
             }
             assertEquals(changed, builder.isChanged());