Repository: skiptests/astraea
Pull request #342 (2022-05-15 11:06:37)

Fix flaky AdminTest#testOffsets
```
AdminTest > testOffsets() FAILED
[30](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:30)
    org.opentest4j.AssertionFailedError: expected: <6> but was: <3>
[31](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:31)
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
[32](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:32)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
[33](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:33)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
[34](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:34)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
[35](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:35)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:527)
[36](https://github.com/skiptests/astraea/runs/6440503201?check_suite_focus=true#step:5:36)
        at app//org.astraea.admin.AdminTest.testOffsets(AdminTest.java:137)
```


Patch:
diff --git a/app/src/main/java/org/astraea/web/BrokerHandler.java b/app/src/main/java/org/astraea/web/BrokerHandler.java
index d723b7fb..6084eda2 100644
--- a/app/src/main/java/org/astraea/web/BrokerHandler.java
+++ b/app/src/main/java/org/astraea/web/BrokerHandler.java
@@ -27,7 +27,7 @@ class BrokerHandler implements Handler {
   }
 
   @Override
-  public JsonObject response(Optional<String> target, Map<String, String> queries) {
+  public JsonObject get(Optional<String> target, Map<String, String> queries) {
     mustBeNumber(target);
     Predicate<Map.Entry<Integer, ?>> brokerId =
         e -> target.stream().map(Integer::valueOf).allMatch(t -> t.equals(e.getKey()));
diff --git a/app/src/main/java/org/astraea/web/ErrorObject.java b/app/src/main/java/org/astraea/web/ErrorObject.java
index 0820f70f..585d4e3f 100644
--- a/app/src/main/java/org/astraea/web/ErrorObject.java
+++ b/app/src/main/java/org/astraea/web/ErrorObject.java
@@ -3,6 +3,11 @@ package org.astraea.web;
 import java.util.NoSuchElementException;
 
 class ErrorObject implements JsonObject {
+
+  static ErrorObject for404(String message) {
+    return new ErrorObject(404, message);
+  }
+
   final int code;
   final String message;
 
@@ -10,4 +15,9 @@ class ErrorObject implements JsonObject {
     this.code = exception instanceof NoSuchElementException ? 404 : 400;
     this.message = exception.getMessage();
   }
+
+  ErrorObject(int code, String message) {
+    this.code = code;
+    this.message = message;
+  }
 }
diff --git a/app/src/main/java/org/astraea/web/GroupHandler.java b/app/src/main/java/org/astraea/web/GroupHandler.java
index d579d50c..026e0ede 100644
--- a/app/src/main/java/org/astraea/web/GroupHandler.java
+++ b/app/src/main/java/org/astraea/web/GroupHandler.java
@@ -17,7 +17,7 @@ public class GroupHandler implements Handler {
   }
 
   @Override
-  public JsonObject response(Optional<String> target, Map<String, String> queries) {
+  public JsonObject get(Optional<String> target, Map<String, String> queries) {
     Predicate<Map.Entry<String, ?>> groupFilter =
         e -> target.stream().allMatch(t -> t.equals(e.getKey()));
     var topics = admin.topicNames();
diff --git a/app/src/main/java/org/astraea/web/Handler.java b/app/src/main/java/org/astraea/web/Handler.java
index b7769d28..2cc608aa 100644
--- a/app/src/main/java/org/astraea/web/Handler.java
+++ b/app/src/main/java/org/astraea/web/Handler.java
@@ -6,17 +6,29 @@ import java.io.IOException;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
 interface Handler extends HttpHandler {
 
-  default JsonObject response(HttpExchange exchange) {
+  default JsonObject process(HttpExchange exchange) {
+    var method = exchange.getRequestMethod().toUpperCase(Locale.ROOT);
     var start = System.currentTimeMillis();
+    JsonObject response;
     try {
-      return response(
-          parseTarget(exchange.getRequestURI()), parseQueries(exchange.getRequestURI()));
+      switch (method) {
+        case "GET":
+          return get(parseTarget(exchange.getRequestURI()), parseQueries(exchange.getRequestURI()));
+        case "POST":
+          return post(PostRequest.of(exchange));
+        case "DELETE":
+          var target = parseTarget(exchange.getRequestURI());
+          if (target.isPresent()) return delete(target.get());
+        default:
+          return ErrorObject.for404(method + " is not supported yet");
+      }
     } catch (Exception e) {
       e.printStackTrace();
       return new ErrorObject(e);
@@ -31,7 +43,8 @@ interface Handler extends HttpHandler {
 
   @Override
   default void handle(HttpExchange exchange) throws IOException {
-    var response = response(exchange);
+    JsonObject response = process(exchange);
+
     var responseData = response.json().getBytes(StandardCharsets.UTF_8);
     exchange
         .getResponseHeaders()
@@ -62,5 +75,32 @@ interface Handler extends HttpHandler {
         .collect(Collectors.toMap(p -> p.split("=")[0], p -> p.split("=")[1]));
   }
 
-  JsonObject response(Optional<String> target, Map<String, String> queries);
+  /**
+   * handle the get request.
+   *
+   * @param target the last keyword of url
+   * @param queries queries from url
+   * @return json object to return
+   */
+  JsonObject get(Optional<String> target, Map<String, String> queries);
+
+  /**
+   * handle the post request.
+   *
+   * @param request the last keyword of url
+   * @return json object to return
+   */
+  default JsonObject post(PostRequest request) {
+    return ErrorObject.for404("POST is not supported yet");
+  }
+
+  /**
+   * handle the delete request.
+   *
+   * @param target the last keyword of url
+   * @return json object to return
+   */
+  default JsonObject delete(String target) {
+    return ErrorObject.for404("DELETE is not supported yet");
+  }
 }
diff --git a/app/src/main/java/org/astraea/web/PostRequest.java b/app/src/main/java/org/astraea/web/PostRequest.java
new file mode 100644
index 00000000..962f0cc9
--- /dev/null
+++ b/app/src/main/java/org/astraea/web/PostRequest.java
@@ -0,0 +1,45 @@
+package org.astraea.web;
+
+import com.google.gson.Gson;
+import com.sun.net.httpserver.HttpExchange;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.stream.Collectors;
+
+public interface PostRequest {
+
+  @SuppressWarnings("unchecked")
+  static PostRequest of(HttpExchange exchange) throws IOException {
+    var jsonString = new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8);
+    var raw =
+        ((Map<String, Object>) new Gson().fromJson(jsonString, Map.class))
+            .entrySet().stream()
+                .filter(e -> e.getValue() != null)
+                .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().toString()));
+    return () -> raw;
+  }
+
+  /** @return body represented by key-value */
+  Map<String, String> raw();
+
+  default String value(String key) {
+    var value = raw().get(key);
+    if (value == null) throw new NoSuchElementException("the value for " + key + " is nonexistent");
+    return value;
+  }
+
+  default double doubleValue(String key) {
+    try {
+      return Double.parseDouble(value(key));
+    } catch (NumberFormatException e) {
+      throw new IllegalArgumentException(e);
+    }
+  }
+
+  default int intValue(String key) {
+    // the number in GSON is always DOUBLE>
+    return (int) doubleValue(key);
+  }
+}
diff --git a/app/src/main/java/org/astraea/web/TopicHandler.java b/app/src/main/java/org/astraea/web/TopicHandler.java
index 01da67a6..d143711b 100644
--- a/app/src/main/java/org/astraea/web/TopicHandler.java
+++ b/app/src/main/java/org/astraea/web/TopicHandler.java
@@ -20,7 +20,7 @@ class TopicHandler implements Handler {
   }
 
   @Override
-  public JsonObject response(Optional<String> target, Map<String, String> queries) {
+  public JsonObject get(Optional<String> target, Map<String, String> queries) {
     Predicate<Map.Entry<String, ?>> topicFilter =
         e -> target.stream().allMatch(t -> t.equals(e.getKey()));
     var topics = admin.topics();


Test patch:
diff --git a/app/src/test/java/org/astraea/admin/AdminTest.java b/app/src/test/java/org/astraea/admin/AdminTest.java
index 3ef6f46f..17b48a89 100644
--- a/app/src/test/java/org/astraea/admin/AdminTest.java
+++ b/app/src/test/java/org/astraea/admin/AdminTest.java
@@ -122,7 +122,7 @@ public class AdminTest extends RequireBrokerCluster {
     try (var admin = Admin.of(bootstrapServers())) {
       admin.creator().topic(topicName).numberOfPartitions(3).create();
       // wait for syncing topic creation
-      TimeUnit.SECONDS.sleep(5);
+      TimeUnit.SECONDS.sleep(3);
       var offsets = admin.offsets(Set.of(topicName));
       Assertions.assertEquals(3, offsets.size());
       offsets
@@ -134,6 +134,8 @@ public class AdminTest extends RequireBrokerCluster {
               });
 
       admin.creator().topic("a").numberOfPartitions(3).create();
+      // wait for syncing topic creation
+      TimeUnit.SECONDS.sleep(3);
       Assertions.assertEquals(6, admin.offsets().size());
     }
   }
diff --git a/app/src/test/java/org/astraea/web/BrokerHandlerTest.java b/app/src/test/java/org/astraea/web/BrokerHandlerTest.java
index 3275ed63..5867c9c2 100644
--- a/app/src/test/java/org/astraea/web/BrokerHandlerTest.java
+++ b/app/src/test/java/org/astraea/web/BrokerHandlerTest.java
@@ -16,7 +16,7 @@ public class BrokerHandlerTest extends RequireBrokerCluster {
       var handler = new BrokerHandler(admin);
       var response =
           Assertions.assertInstanceOf(
-              BrokerHandler.Brokers.class, handler.response(Optional.empty(), Map.of()));
+              BrokerHandler.Brokers.class, handler.get(Optional.empty(), Map.of()));
       Assertions.assertEquals(brokerIds().size(), response.brokers.size());
       brokerIds()
           .forEach(
@@ -32,7 +32,7 @@ public class BrokerHandlerTest extends RequireBrokerCluster {
       var handler = new BrokerHandler(admin);
       var exception =
           Assertions.assertThrows(
-              NoSuchElementException.class, () -> handler.response(Optional.of("99999"), Map.of()));
+              NoSuchElementException.class, () -> handler.get(Optional.of("99999"), Map.of()));
       Assertions.assertTrue(exception.getMessage().contains("99999"));
     }
   }
@@ -43,7 +43,7 @@ public class BrokerHandlerTest extends RequireBrokerCluster {
       var handler = new BrokerHandler(admin);
       var exception =
           Assertions.assertThrows(
-              NoSuchElementException.class, () -> handler.response(Optional.of("abc"), Map.of()));
+              NoSuchElementException.class, () -> handler.get(Optional.of("abc"), Map.of()));
       Assertions.assertTrue(exception.getMessage().contains("abc"));
     }
   }
@@ -56,7 +56,7 @@ public class BrokerHandlerTest extends RequireBrokerCluster {
       var broker =
           Assertions.assertInstanceOf(
               BrokerHandler.Broker.class,
-              handler.response(Optional.of(String.valueOf(brokerId)), Map.of()));
+              handler.get(Optional.of(String.valueOf(brokerId)), Map.of()));
       Assertions.assertEquals(brokerId, broker.id);
       Assertions.assertNotEquals(0, broker.configs.size());
     }
diff --git a/app/src/test/java/org/astraea/web/ErrorObjectTest.java b/app/src/test/java/org/astraea/web/ErrorObjectTest.java
new file mode 100644
index 00000000..44404c1e
--- /dev/null
+++ b/app/src/test/java/org/astraea/web/ErrorObjectTest.java
@@ -0,0 +1,14 @@
+package org.astraea.web;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class ErrorObjectTest {
+
+  @Test
+  void test404() {
+    var error = ErrorObject.for404("this");
+    Assertions.assertEquals(404, error.code);
+    Assertions.assertEquals("this", error.message);
+  }
+}
diff --git a/app/src/test/java/org/astraea/web/GroupHandlerTest.java b/app/src/test/java/org/astraea/web/GroupHandlerTest.java
index 9434c6a6..c840f76b 100644
--- a/app/src/test/java/org/astraea/web/GroupHandlerTest.java
+++ b/app/src/test/java/org/astraea/web/GroupHandlerTest.java
@@ -33,7 +33,7 @@ public class GroupHandlerTest extends RequireBrokerCluster {
         var handler = new GroupHandler(admin);
         var response =
             Assertions.assertInstanceOf(
-                GroupHandler.Groups.class, handler.response(Optional.empty(), Map.of()));
+                GroupHandler.Groups.class, handler.get(Optional.empty(), Map.of()));
         Assertions.assertEquals(1, response.groups.size());
         Assertions.assertEquals(groupId, response.groups.iterator().next().groupId);
         Assertions.assertEquals(1, response.groups.iterator().next().members.size());
@@ -53,8 +53,7 @@ public class GroupHandlerTest extends RequireBrokerCluster {
       var handler = new GroupHandler(admin);
       var exception =
           Assertions.assertThrows(
-              NoSuchElementException.class,
-              () -> handler.response(Optional.of("unknown"), Map.of()));
+              NoSuchElementException.class, () -> handler.get(Optional.of("unknown"), Map.of()));
       Assertions.assertTrue(exception.getMessage().contains("unknown"));
     }
   }
@@ -75,7 +74,7 @@ public class GroupHandlerTest extends RequireBrokerCluster {
         Assertions.assertEquals(0, consumer.poll(Duration.ofSeconds(3)).size());
         var group =
             Assertions.assertInstanceOf(
-                GroupHandler.Group.class, handler.response(Optional.of(groupId), Map.of()));
+                GroupHandler.Group.class, handler.get(Optional.of(groupId), Map.of()));
         Assertions.assertEquals(groupId, group.groupId);
         Assertions.assertEquals(1, group.members.size());
       }
diff --git a/app/src/test/java/org/astraea/web/HandlerTest.java b/app/src/test/java/org/astraea/web/HandlerTest.java
index 6e0420f2..f5e1c9f4 100644
--- a/app/src/test/java/org/astraea/web/HandlerTest.java
+++ b/app/src/test/java/org/astraea/web/HandlerTest.java
@@ -16,9 +16,10 @@ public class HandlerTest {
           throw exception;
         };
 
-    var httpExchange = Mockito.mock(HttpExchange.class);
-    Mockito.when(httpExchange.getRequestURI()).thenReturn(URI.create("http://localhost:8888/abc"));
-    var r = Assertions.assertInstanceOf(ErrorObject.class, handler.response(httpExchange));
+    var exchange = Mockito.mock(HttpExchange.class);
+    Mockito.when(exchange.getRequestURI()).thenReturn(URI.create("http://localhost:8888/abc"));
+    Mockito.when(exchange.getRequestMethod()).thenReturn("get");
+    var r = Assertions.assertInstanceOf(ErrorObject.class, handler.process(exchange));
     Assertions.assertNotEquals(200, r.code);
     Assertions.assertEquals(exception.getMessage(), r.message);
   }
diff --git a/app/src/test/java/org/astraea/web/PostRequestTest.java b/app/src/test/java/org/astraea/web/PostRequestTest.java
new file mode 100644
index 00000000..cf3193be
--- /dev/null
+++ b/app/src/test/java/org/astraea/web/PostRequestTest.java
@@ -0,0 +1,25 @@
+package org.astraea.web;
+
+import com.sun.net.httpserver.HttpExchange;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+public class PostRequestTest {
+
+  @Test
+  void testParse() throws IOException {
+    var input =
+        new ByteArrayInputStream("{\"a\":\"b\",\"c\":123}".getBytes(StandardCharsets.UTF_8));
+    var exchange = Mockito.mock(HttpExchange.class);
+    Mockito.when(exchange.getRequestBody()).thenReturn(input);
+
+    var request = PostRequest.of(exchange);
+    Assertions.assertEquals(2, request.raw().size());
+    Assertions.assertEquals("b", request.raw().get("a"));
+    Assertions.assertEquals(123, request.intValue("c"));
+  }
+}
diff --git a/app/src/test/java/org/astraea/web/TopicHandlerTest.java b/app/src/test/java/org/astraea/web/TopicHandlerTest.java
index b7b500c6..d324eccf 100644
--- a/app/src/test/java/org/astraea/web/TopicHandlerTest.java
+++ b/app/src/test/java/org/astraea/web/TopicHandlerTest.java
@@ -21,7 +21,7 @@ public class TopicHandlerTest extends RequireBrokerCluster {
       var handler = new TopicHandler(admin);
       var response =
           Assertions.assertInstanceOf(
-              TopicHandler.Topics.class, handler.response(Optional.empty(), Map.of()));
+              TopicHandler.Topics.class, handler.get(Optional.empty(), Map.of()));
       Assertions.assertEquals(1, response.topics.size());
       Assertions.assertEquals(topicName, response.topics.iterator().next().name);
       Assertions.assertNotEquals(0, response.topics.iterator().next().configs.size());
@@ -34,8 +34,7 @@ public class TopicHandlerTest extends RequireBrokerCluster {
       var handler = new TopicHandler(admin);
       var exception =
           Assertions.assertThrows(
-              NoSuchElementException.class,
-              () -> handler.response(Optional.of("unknown"), Map.of()));
+              NoSuchElementException.class, () -> handler.get(Optional.of("unknown"), Map.of()));
       Assertions.assertTrue(exception.getMessage().contains("unknown"));
     }
   }
@@ -49,7 +48,7 @@ public class TopicHandlerTest extends RequireBrokerCluster {
       var handler = new TopicHandler(admin);
       var topicInfo =
           Assertions.assertInstanceOf(
-              TopicHandler.TopicInfo.class, handler.response(Optional.of(topicName), Map.of()));
+              TopicHandler.TopicInfo.class, handler.get(Optional.of(topicName), Map.of()));
       Assertions.assertEquals(topicName, topicInfo.name);
       Assertions.assertNotEquals(0, topicInfo.configs.size());
     }