[#999] improvement: apply REST principles in the urls of the http interfaces

common/src/test/java/org/apache/uniffle/common/metrics/TestUtils.java

@@ -42,15 +42,21 @@ public static String httpGet(String urlString) throws IOException {
     return content.toString();
   }
 
+  public static String httpPost(String urlString) throws IOException {
+    return httpPost(urlString, null);
+  }
+
   public static String httpPost(String urlString, String postData) throws IOException {
     URL url = new URL(urlString);
     HttpURLConnection con = (HttpURLConnection) url.openConnection();
     con.setDoOutput(true);
     con.setRequestMethod("POST");
     con.setRequestProperty("Content-type", "application/json");
     StringBuilder content = new StringBuilder();
-    try (OutputStream outputStream = con.getOutputStream(); ) {
-      outputStream.write(postData.getBytes());
+    try (OutputStream outputStream = con.getOutputStream()) {
+      if (postData != null) {
+        outputStream.write(postData.getBytes());
+      }
       try (BufferedReader in = new BufferedReader(new InputStreamReader(con.getInputStream())); ) {
         String inputLine;
         while ((inputLine = in.readLine()) != null) {

coordinator/src/main/java/org/apache/uniffle/coordinator/web/resource/AdminResource.java

@@ -19,7 +19,6 @@
 
 import java.util.List;
 import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
 
 import org.apache.hbase.thirdparty.javax.ws.rs.GET;
 import org.apache.hbase.thirdparty.javax.ws.rs.Path;
@@ -35,20 +34,22 @@
 import org.apache.uniffle.coordinator.web.Response;
 
 @Produces({MediaType.APPLICATION_JSON})
-public class AdminResource {
+public class AdminResource extends BaseResource {
   private static final Logger LOG = LoggerFactory.getLogger(AdminResource.class);
-  @Context private HttpServletRequest httpRequest;
   @Context protected ServletContext servletContext;
 
   @GET
   @Path("/refreshChecker")
   public Response<List<ServerNode>> refreshChecker() {
-    List<AccessChecker> accessCheckers = getAccessManager().getAccessCheckers();
-    LOG.info(
-        "The access checker {} has been refreshed, you can add the checker via rss.coordinator.access.checkers.",
-        accessCheckers);
-    accessCheckers.forEach(AccessChecker::refreshAccessChecker);
-    return Response.success(null);
+    return execute(
+        () -> {
+          List<AccessChecker> accessCheckers = getAccessManager().getAccessCheckers();
+          LOG.info(
+              "The access checker {} has been refreshed, you can add the checker via rss.coordinator.access.checkers.",
+              accessCheckers);
+          accessCheckers.forEach(AccessChecker::refreshAccessChecker);
+          return null;
+        });
   }
 
   private AccessManager getAccessManager() {

coordinator/src/main/java/org/apache/uniffle/coordinator/web/resource/BaseResource.java

@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.uniffle.coordinator.web.resource;
+
+import java.util.concurrent.Callable;
+
+import org.apache.uniffle.coordinator.web.Response;
+
+public abstract class BaseResource {
+  protected <T> Response<T> execute(Callable<T> callable) {
+    try {
+      return Response.success(callable.call());
+    } catch (Throwable e) {
+      return Response.fail(e.getMessage());
+    }
+  }
+}

coordinator/src/main/java/org/apache/uniffle/coordinator/web/resource/ServerResource.java

@@ -27,6 +27,7 @@
 import org.apache.hbase.thirdparty.javax.ws.rs.GET;
 import org.apache.hbase.thirdparty.javax.ws.rs.POST;
 import org.apache.hbase.thirdparty.javax.ws.rs.Path;
+import org.apache.hbase.thirdparty.javax.ws.rs.PathParam;
 import org.apache.hbase.thirdparty.javax.ws.rs.Produces;
 import org.apache.hbase.thirdparty.javax.ws.rs.QueryParam;
 import org.apache.hbase.thirdparty.javax.ws.rs.core.Context;
@@ -40,13 +41,18 @@
 import org.apache.uniffle.coordinator.web.request.DecommissionRequest;
 
 @Produces({MediaType.APPLICATION_JSON})
-public class ServerResource {
+public class ServerResource extends BaseResource {
   @Context protected ServletContext servletContext;
 
+  @GET
+  @Path("/nodes/{id}")
+  public Response<ServerNode> node(@PathParam("id") String id) {
+    return execute(() -> getClusterManager().getServerNodeById(id));
+  }
+
   @GET
   @Path("/nodes")
-  public Response<List<ServerNode>> nodes(
-      @QueryParam("id") String id, @QueryParam("status") String status) {
+  public Response<List<ServerNode>> nodes(@QueryParam("status") String status) {
     ClusterManager clusterManager = getClusterManager();
     List<ServerNode> serverList;
     if (ServerStatus.UNHEALTHY.name().equalsIgnoreCase(status)) {
@@ -60,9 +66,6 @@ public Response<List<ServerNode>> nodes(
         serverList.stream()
             .filter(
                 server -> {
-                  if (id != null && !id.equals(server.getId())) {
-                    return false;
-                  }
                   if (status != null && !server.getStatus().toString().equals(status)) {
                     return false;
                   }
@@ -75,34 +78,47 @@ public Response<List<ServerNode>> nodes(
 
   @POST
   @Path("/cancelDecommission")
-  @Produces({MediaType.APPLICATION_JSON})
   public Response<Object> cancelDecommission(CancelDecommissionRequest params) {
-    if (CollectionUtils.isEmpty(params.getServerIds())) {
-      return Response.fail("Parameter[serverIds] should not be null!");
-    }
-    ClusterManager clusterManager = getClusterManager();
-    try {
-      params.getServerIds().forEach(clusterManager::cancelDecommission);
-    } catch (Exception e) {
-      return Response.fail(e.getMessage());
-    }
-    return Response.success(null);
+    return execute(
+        () -> {
+          assert CollectionUtils.isNotEmpty(params.getServerIds())
+              : "Parameter[serverIds] should not be null!";
+          params.getServerIds().forEach(getClusterManager()::cancelDecommission);
+          return null;
+        });
+  }
+
+  @POST
+  @Path("/{id}/cancelDecommission")
+  public Response<Object> cancelDecommission(@PathParam("id") String serverId) {
+    return execute(
+        () -> {
+          getClusterManager().cancelDecommission(serverId);
+          return null;
+        });
   }
 
   @POST
   @Path("/decommission")
-  @Produces({MediaType.APPLICATION_JSON})
   public Response<Object> decommission(DecommissionRequest params) {
-    if (CollectionUtils.isEmpty(params.getServerIds())) {
-      return Response.fail("Parameter[serverIds] should not be null!");
-    }
-    ClusterManager clusterManager = getClusterManager();
-    try {
-      params.getServerIds().forEach(clusterManager::decommission);
-    } catch (Exception e) {
-      return Response.fail(e.getMessage());
-    }
-    return Response.success(null);
+    return execute(
+        () -> {
+          assert CollectionUtils.isNotEmpty(params.getServerIds())
+              : "Parameter[serverIds] should not be null!";
+          params.getServerIds().forEach(getClusterManager()::decommission);
+          return null;
+        });
+  }
+
+  @POST
+  @Path("/{id}/decommission")
+  @Produces({MediaType.APPLICATION_JSON})
+  public Response<Object> decommission(@PathParam("id") String serverId) {
+    return execute(
+        () -> {
+          getClusterManager().decommission(serverId);
+          return null;
+        });
   }
 
   private ClusterManager getClusterManager() {

docs/coordinator_guide.md

@@ -128,9 +128,27 @@ AccessQuotaChecker is a checker when the number of concurrent tasks submitted by
 |rss.coordinator.quota.default.app.num|5|Default number of apps at user level.|
 
 
-## RESTful API(beta)
+## RESTful API
 
-### Fetch Shuffle servers
+### Fetch single shuffle server
+
+<details>
+ <summary><code>GET</code> <code><b>/api/server/nodes/{id}</b></code> </summary>
+
+##### Parameters
+
+> |name|type|data type|description|
+> |----|----|---------|-----------|
+> |id|required|string|shuffle server id, eg:127.0.0.1-19999|
+##### Example cURL
+
+> ```bash
+>  curl -X GET http://localhost:19998/api/server/nodes/127.0.0.1-19999
+> ```
+</details>
+
+
+### Fetch shuffle servers
 
 <details>
  <summary><code>GET</code> <code><b>/api/server/nodes</b></code> </summary>
@@ -139,13 +157,13 @@ AccessQuotaChecker is a checker when the number of concurrent tasks submitted by
 
 > |name|type|data type|description|
 > |----|----|---------|-----------|
-> |id|required|string|shuffle server id, eg:127.0.0.1:19999|
 > |status|optional|string|Shuffle server status, eg:ACTIVE, DECOMMISSIONING, DECOMMISSIONED|
 
 ##### Example cURL
 
 > ```bash
 >  curl -X GET http://localhost:19998/api/server/nodes
+>  curl -X GET http://localhost:19998/api/server/nodes?status=ACTIVE
 > ```
 </details>
 
@@ -158,7 +176,7 @@ AccessQuotaChecker is a checker when the number of concurrent tasks submitted by
 
 > |name|type| data type         |description|
 > |----|-------------------|---------|-----------|
-> |serverIds|required| array |Shuffle server array, eg:["127.0.0.1:19999"]|
+> |serverIds|required| array |Shuffle server array, eg:["127.0.0.1-19999"]|
 > 
 ##### Example cURL
 
@@ -168,6 +186,25 @@ AccessQuotaChecker is a checker when the number of concurrent tasks submitted by
 </details>
 
 
+### Decommission single shuffle server
+
+<details>
+ <summary><code>POST</code> <code><b>/api/server/{id}/decommission</b></code> </summary>
+
+##### Parameters
+
+> | name |type| data type | description                          |
+> |------|-------------------|-----------|--------------------------------------|
+> | id   |required| string    | Shuffle server id, eg:127.0.0.1-19999 |
+>
+##### Example cURL
+
+> ```bash
+>  curl -X POST -H "Content-Type: application/json" http://localhost:19998/api/server/127.0.0.1-19999/decommission
+> ```
+</details>
+
+
 ### Cancel decommission shuffle servers
 
 <details>
@@ -184,4 +221,23 @@ AccessQuotaChecker is a checker when the number of concurrent tasks submitted by
 > ```bash
 >  curl -X POST -H "Content-Type: application/json" http://localhost:19998/api/server/cancelDecommission  -d '{"serverIds:": ["127.0.0.1:19999"]}'
 > ```
+</details>
+
+
+### Cancel decommission single shuffle server
+
+<details>
+ <summary><code>POST</code> <code><b>/api/server/{id}/cancelDecommission</b></code> </summary>
+
+##### Parameters
+
+> |name|type| data type | description                             |
+> |----|-------------------|--------|-----------------------------------------|
+> |serverIds|required| string | Shuffle server id, eg:"127.0.0.1-19999" |
+>
+##### Example cURL
+
+> ```bash
+>  curl -X POST -H "Content-Type: application/json" http://localhost:19998/api/server/127.0.0.1-19999/cancelDecommission
+> ```
 </details>