From 201fc06f8d7e28a0940365314a786ac6230f7ace Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Wed, 23 Mar 2016 17:12:30 -0400 Subject: [PATCH] Fix TaskId#isSet to return true when id is set and not other way around During refactoring the name was changed, but the logic wasn't. This commit fixes the logic to match the name. --- .../node/tasks/cancel/TransportCancelTasksAction.java | 2 +- .../action/admin/cluster/node/tasks/list/TaskInfo.java | 2 +- .../action/support/tasks/BaseTasksRequest.java | 6 +++--- .../action/support/tasks/TransportTasksAction.java | 6 +++--- core/src/main/java/org/elasticsearch/tasks/TaskId.java | 6 +++--- core/src/main/java/org/elasticsearch/tasks/TaskManager.java | 4 ++-- .../action/admin/cluster/node/tasks/TasksIT.java | 6 +++--- .../admin/cluster/node/tasks/TransportTasksActionTests.java | 2 +- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/cancel/TransportCancelTasksAction.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/cancel/TransportCancelTasksAction.java index 336f4c84596..9dbe4ee1aeb 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/cancel/TransportCancelTasksAction.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/cancel/TransportCancelTasksAction.java @@ -84,7 +84,7 @@ public class TransportCancelTasksAction extends TransportTasksAction operation) { - if (request.getTaskId().isSet() == false) { + if (request.getTaskId().isSet()) { // we are only checking one task, we can optimize it CancellableTask task = taskManager.getCancellableTask(request.getTaskId().getId()); if (task != null) { diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/list/TaskInfo.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/list/TaskInfo.java index c027bfa7ab4..ad9fa9509e8 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/list/TaskInfo.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/list/TaskInfo.java @@ -178,7 +178,7 @@ public class TaskInfo implements Writeable, ToXContent { } builder.dateValueField("start_time_in_millis", "start_time", startTime); builder.timeValueField("running_time_in_nanos", "running_time", runningTimeNanos, TimeUnit.NANOSECONDS); - if (parentTaskId.isSet() == false) { + if (parentTaskId.isSet()) { builder.field("parent_task_id", parentTaskId.toString()); } return builder; diff --git a/core/src/main/java/org/elasticsearch/action/support/tasks/BaseTasksRequest.java b/core/src/main/java/org/elasticsearch/action/support/tasks/BaseTasksRequest.java index f1045387259..dc296a84720 100644 --- a/core/src/main/java/org/elasticsearch/action/support/tasks/BaseTasksRequest.java +++ b/core/src/main/java/org/elasticsearch/action/support/tasks/BaseTasksRequest.java @@ -60,7 +60,7 @@ public class BaseTasksRequest> extends @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = null; - if (taskId.isSet() == false && nodesIds.length > 0) { + if (taskId.isSet() && nodesIds.length > 0) { validationException = addValidationError("task id cannot be used together with node ids", validationException); } @@ -165,12 +165,12 @@ public class BaseTasksRequest> extends if (getActions() != null && getActions().length > 0 && Regex.simpleMatch(getActions(), task.getAction()) == false) { return false; } - if (getTaskId().isSet() == false) { + if (getTaskId().isSet()) { if(getTaskId().getId() != task.getId()) { return false; } } - if (parentTaskId.isSet() == false) { + if (parentTaskId.isSet()) { if (parentTaskId.equals(task.getParentTaskId()) == false) { return false; } diff --git a/core/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java b/core/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java index a14c6e00e14..97678e6c060 100644 --- a/core/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java +++ b/core/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java @@ -125,14 +125,14 @@ public abstract class TransportTasksAction< protected String[] resolveNodes(TasksRequest request, ClusterState clusterState) { if (request.getTaskId().isSet()) { - return clusterState.nodes().resolveNodesIds(request.getNodesIds()); - } else { return new String[]{request.getTaskId().getNodeId()}; + } else { + return clusterState.nodes().resolveNodesIds(request.getNodesIds()); } } protected void processTasks(TasksRequest request, Consumer operation) { - if (request.getTaskId().isSet() == false) { + if (request.getTaskId().isSet()) { // we are only checking one task, we can optimize it Task task = taskManager.getTask(request.getTaskId().getId()); if (task != null) { diff --git a/core/src/main/java/org/elasticsearch/tasks/TaskId.java b/core/src/main/java/org/elasticsearch/tasks/TaskId.java index 5c5ad36cc17..d1f29e65226 100644 --- a/core/src/main/java/org/elasticsearch/tasks/TaskId.java +++ b/core/src/main/java/org/elasticsearch/tasks/TaskId.java @@ -73,15 +73,15 @@ public final class TaskId implements Writeable { } public boolean isSet() { - return id == -1L; + return id != -1L; } @Override public String toString() { if (isSet()) { - return "unset"; - } else { return nodeId + ":" + id; + } else { + return "unset"; } } diff --git a/core/src/main/java/org/elasticsearch/tasks/TaskManager.java b/core/src/main/java/org/elasticsearch/tasks/TaskManager.java index 0c785573c99..c881e1756e8 100644 --- a/core/src/main/java/org/elasticsearch/tasks/TaskManager.java +++ b/core/src/main/java/org/elasticsearch/tasks/TaskManager.java @@ -76,7 +76,7 @@ public class TaskManager extends AbstractComponent implements ClusterStateListen CancellableTaskHolder oldHolder = cancellableTasks.put(task.getId(), holder); assert oldHolder == null; // Check if this task was banned before we start it - if (task.getParentTaskId().isSet() == false && banedParents.isEmpty() == false) { + if (task.getParentTaskId().isSet() && banedParents.isEmpty() == false) { String reason = banedParents.get(task.getParentTaskId()); if (reason != null) { try { @@ -241,7 +241,7 @@ public class TaskManager extends AbstractComponent implements ClusterStateListen CancellableTaskHolder holder = taskEntry.getValue(); CancellableTask task = holder.getTask(); TaskId parentTaskId = task.getParentTaskId(); - if (parentTaskId.isSet() == false && lastDiscoveryNodes.nodeExists(parentTaskId.getNodeId()) == false) { + if (parentTaskId.isSet() && lastDiscoveryNodes.nodeExists(parentTaskId.getNodeId()) == false) { if (task.cancelOnParentLeaving()) { holder.cancel("Coordinating node [" + parentTaskId.getNodeId() + "] left the cluster"); } diff --git a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java index b22d93ef6b2..90dbfaa8c60 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java +++ b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TasksIT.java @@ -118,9 +118,9 @@ public class TasksIT extends ESIntegTestCase { // Verify that one of these tasks is a parent of another task if (tasks.get(0).getParentTaskId().isSet()) { - assertParentTask(Collections.singletonList(tasks.get(1)), tasks.get(0)); - } else { assertParentTask(Collections.singletonList(tasks.get(0)), tasks.get(1)); + } else { + assertParentTask(Collections.singletonList(tasks.get(1)), tasks.get(0)); } } @@ -474,7 +474,7 @@ public class TasksIT extends ESIntegTestCase { */ private void assertParentTask(List tasks, TaskInfo parentTask) { for (TaskInfo task : tasks) { - assertFalse(task.getParentTaskId().isSet()); + assertTrue(task.getParentTaskId().isSet()); assertEquals(parentTask.getNode().getId(), task.getParentTaskId().getNodeId()); assertTrue(Strings.hasLength(task.getParentTaskId().getNodeId())); assertEquals(parentTask.getId(), task.getParentTaskId().getId()); diff --git a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java index 64d69a4864f..4b478b52bd0 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/cluster/node/tasks/TransportTasksActionTests.java @@ -604,7 +604,7 @@ public class TransportTasksActionTests extends TaskManagerTestCase { @Override protected TestTaskResponse taskOperation(TestTasksRequest request, Task task) { logger.info("Task action on node {}", node); - if (failTaskOnNode == node && task.getParentTaskId().isSet() == false) { + if (failTaskOnNode == node && task.getParentTaskId().isSet()) { logger.info("Failing on node {}", node); throw new RuntimeException("Task level failure"); }