advancedxy commented on code in PR #891:
URL: https://github.com/apache/incubator-uniffle/pull/891#discussion_r1203790042


##########
coordinator/src/main/java/org/apache/uniffle/coordinator/web/servlet/BaseServlet.java:
##########
@@ -44,9 +44,9 @@ protected void doPost(HttpServletRequest req, 
HttpServletResponse resp) throws I
     writeJSON(resp, handlerRequest(() -> handlePost(req, resp)));
   }
 
-  private Response handlerRequest(
-      Callable<Response> function) {
-    Response response;
+  private <T> Response<T> handlerRequest(

Review Comment:
   > We can implement all REST api through one servlet.
   
   If we are going to implement all REST API through one servlet, then there's 
no point to make this `handlerRequest` method a generic method.  It should be 
simply defined as `private Response<Object> handleRequest(...)` then..
   
   I would propose to something like this
   ```
   public abstract class BaseServlet<T> extends HttpServlet {
   
     private Response<T> handlerRequest(
         Callable<Response<T>> function) {
       Response<T> response;
       try {
         // todo: Do something for authentication
         response = function.call();
       } catch (Exception e) {
         response = Response.fail(e.getMessage());
       }
       return response;
     }
   
     protected Response<T> handleGet(
         HttpServletRequest req,
         HttpServletResponse resp) throws ServletException, IOException {
       ...
     }
   
     protected Response<T> handlePost(
         HttpServletRequest req,
         HttpServletResponse resp) throws ServletException, IOException {
       ...
     }
   
   }
   ```
   
   For concrete servlet which handles and returns only one type should 
implement it with a concrete type.
   
   For servlet that might return multiple types could defined it as 
   
   ```
   public class AnyReturnServlet extends BaseServlet<Object> {
     ...
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to