From 37b20000a11b50f584f9d931e44e679fbcc23e8f Mon Sep 17 00:00:00 2001
From: Xiaxuan Gao <gaox8499@gmail.com>
Date: Thu, 19 Oct 2023 16:35:08 -0700
Subject: [PATCH] Address comments

---
 .../StoppableInstancesSelector.java                    |  5 ++---
 .../rest/server/resources/helix/InstancesAccessor.java | 10 ++++++++--
 2 files changed, 10 insertions(+), 5 deletions(-)
 rename helix-rest/src/main/java/org/apache/helix/rest/{common => clusterMaintenanceService}/StoppableInstancesSelector.java (97%)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/StoppableInstancesSelector.java b/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/StoppableInstancesSelector.java
similarity index 97%
rename from helix-rest/src/main/java/org/apache/helix/rest/common/StoppableInstancesSelector.java
rename to helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/StoppableInstancesSelector.java
index b12b3d2f14..dafe1ab2d8 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/StoppableInstancesSelector.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/StoppableInstancesSelector.java
@@ -1,4 +1,4 @@
-package org.apache.helix.rest.common;
+package org.apache.helix.rest.clusterMaintenanceService;
 
 /*
  * Licensed to the Apache Software Foundation (ASF) under one
@@ -34,7 +34,6 @@
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import org.apache.commons.lang3.NotImplementedException;
-import org.apache.helix.rest.clusterMaintenanceService.MaintenanceManagementService;
 import org.apache.helix.rest.server.json.cluster.ClusterTopology;
 import org.apache.helix.rest.server.json.instance.StoppableCheck;
 
@@ -115,7 +114,7 @@ public void getStoppableInstancesCrossZones() {
    * Determines the order of zones. If an order is provided by the user, it will be used directly.
    * Otherwise, zones will be ordered by their associated instance count in descending order.
    *
-   * If `isRandom` is true, the order of zones will be randomized regardless of any previous order.
+   * If `random` is true, the order of zones will be randomized regardless of any previous order.
    *
    * @param random Indicates whether to randomize the order of zones.
    */
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java
index 0901d30d31..8a21202704 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstancesAccessor.java
@@ -46,7 +46,7 @@
 import org.apache.helix.model.InstanceConfig;
 import org.apache.helix.rest.clusterMaintenanceService.MaintenanceManagementService;
 import org.apache.helix.rest.common.HttpConstants;
-import org.apache.helix.rest.common.StoppableInstancesSelector;
+import org.apache.helix.rest.clusterMaintenanceService.StoppableInstancesSelector;
 import org.apache.helix.rest.server.filters.ClusterAuth;
 import org.apache.helix.rest.server.json.cluster.ClusterTopology;
 import org.apache.helix.rest.server.json.instance.StoppableCheck;
@@ -149,7 +149,7 @@ public Response instancesOperations(@PathParam("clusterId") String clusterId,
       @QueryParam("continueOnFailures") boolean continueOnFailures,
       @QueryParam("skipZKRead") boolean skipZKRead,
       @QueryParam("skipHealthCheckCategories") String skipHealthCheckCategories,
-      @QueryParam("random") boolean random, String content) {
+      @DefaultValue("false") @QueryParam("random") boolean random, String content) {
     Command cmd;
     try {
       cmd = Command.valueOf(command);
@@ -233,6 +233,12 @@ private Response batchGetStoppableInstances(String clusterId, JsonNode node, boo
         orderOfZone = OBJECT_MAPPER.readValue(
             node.get(InstancesAccessor.InstancesProperties.zone_order.name()).toString(),
             OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class, String.class));
+        if (!orderOfZone.isEmpty() && random) {
+          String message =
+              "Both 'orderOfZone' and 'random' parameters are set. Please specify only one option.";
+          _logger.error(message);
+          return badRequest(message);
+        }
       }
 
       // Prepare output result