adoroszlai commented on code in PR #9126:
URL: https://github.com/apache/ozone/pull/9126#discussion_r2416016569


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -69,57 +79,133 @@ public void doGet(HttpServletRequest request, 
HttpServletResponse response)
       return;
     }
 
-    String format = parseAcceptHeader(request);
-    if (FORMAT_XML.equals(format)) {
-      response.setContentType("text/xml; charset=utf-8");
-    } else if (FORMAT_JSON.equals(format)) {
+    ResponseFormat format = parseAcceptHeader(request);
+    switch (format) {
+    case JSON:
       response.setContentType("application/json; charset=utf-8");
+      break;
+    case XML:
+    default:
+      response.setContentType("text/xml; charset=utf-8");
+      break;
     }

Review Comment:
   I suggest adding `getContentType()` instance method in `ResponseFormat`, 
returning `application/json` and `text/xml` for `JSON` and `XML` respectively.  
Then we can change this to:
   
   ```suggestion
       response.setContentType(format.getContentType() + "; charset=utf-8");
   ```



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -69,57 +79,133 @@ public void doGet(HttpServletRequest request, 
HttpServletResponse response)
       return;
     }
 
-    String format = parseAcceptHeader(request);
-    if (FORMAT_XML.equals(format)) {
-      response.setContentType("text/xml; charset=utf-8");
-    } else if (FORMAT_JSON.equals(format)) {
+    ResponseFormat format = parseAcceptHeader(request);
+    switch (format) {
+    case JSON:
       response.setContentType("application/json; charset=utf-8");
+      break;
+    case XML:
+    default:
+      response.setContentType("text/xml; charset=utf-8");
+      break;
     }
 
     String name = request.getParameter("name");
     Writer out = response.getWriter();
     String cmd = request.getParameter(COMMAND);
 
     processCommand(cmd, format, request, response, out, name);
-    out.close();
   }
 
-  private void processCommand(String cmd, String format,
-      HttpServletRequest request, HttpServletResponse response, Writer out,
-      String name)
+  private void processCommand(String cmd, ResponseFormat format,
+                              HttpServletRequest request, HttpServletResponse 
response, Writer out,
+                              String name)

Review Comment:
   nit: Please do not format method signature like this. Whenever visibility / 
return type / method name / other modifiers are changed, we would have to 
reindent all parameters.



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -136,7 +222,7 @@ public BadFormatException(String msg) {
   }
 
   private void processConfigTagRequest(HttpServletRequest request, String cmd,
-      Writer out) throws IOException {
+                                       Writer out) throws IOException {

Review Comment:
   Please do not format method signature like this.



-- 
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