chia7712 commented on code in PR #20214:
URL: https://github.com/apache/kafka/pull/20214#discussion_r2254538479


##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TasksResponse.java:
##########
@@ -32,7 +31,7 @@ public class TasksResponse extends Message {
 
     @JsonCreator
     public TasksResponse(@JsonProperty("tasks") TreeMap<String, TaskState> 
tasks) {
-        this.tasks = Collections.unmodifiableMap((tasks == null) ? new 
TreeMap<>() : tasks);
+        this.tasks = Map.copyOf((tasks == null) ? new TreeMap<>() : tasks);

Review Comment:
   ```java
   this.tasks = tasks == null ? Map.of() : Map.copyOf(tasks);
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/task/TaskSpec.java:
##########
@@ -112,6 +111,6 @@ public String toString() {
     }
 
     protected static Map<String, String> configOrEmptyMap(Map<String, String> 
config) {
-        return (config == null) ? Collections.emptyMap() : config;
+        return (config == null) ? Map.of() : config;

Review Comment:
   ```java
   return config == null ? Map.of() : config;
   ```



##########
trogdor/src/test/java/org/apache/kafka/trogdor/task/SampleTaskSpec.java:
##########
@@ -35,7 +34,7 @@ public SampleTaskSpec(@JsonProperty("startMs") long startMs,
                         @JsonProperty("error") String error) {
         super(startMs, durationMs);
         this.nodeToExitMs = nodeToExitMs == null ? new HashMap<>() :

Review Comment:
   ```java
   this.nodeToExitMs = nodeToExitMs == null ? Map.of() : 
Map.copyOf(nodeToExitMs);
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/PartitionsSpec.java:
##########
@@ -84,7 +83,9 @@ public List<Integer> partitionNumbers() {
             }
             return partitionNumbers;
         } else {
-            return new ArrayList<>(partitionAssignments.keySet());
+            ArrayList<Integer> partitionNumbers = new 
ArrayList<>(partitionAssignments.keySet());
+            partitionNumbers.sort(Integer::compareTo);

Review Comment:
   Why is it being sorted?



##########
trogdor/src/main/java/org/apache/kafka/trogdor/agent/AgentClient.java:
##########
@@ -275,8 +274,7 @@ public static void main(String[] args) throws Exception {
                     System.out.printf("\tStart time: %s%n",
                         dateString(status.serverStartMs(), localOffset));
                     List<List<String>> lines = new ArrayList<>();
-                    List<String> header = new ArrayList<>(
-                        Arrays.asList("WORKER_ID", "TASK_ID", "STATE", 
"TASK_TYPE"));
+                    List<String> header = List.of("WORKER_ID", "TASK_ID", 
"STATE", "TASK_TYPE");
                     lines.add(header);

Review Comment:
   ```java
   lines.add(List.of("WORKER_ID", "TASK_ID", "STATE", "TASK_TYPE"));
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConnectionStressSpec.java:
##########
@@ -56,7 +55,7 @@ public ConnectionStressSpec(@JsonProperty("startMs") long 
startMs,
             @JsonProperty("numThreads") int numThreads,
             @JsonProperty("action") ConnectionStressAction action) {
         super(startMs, durationMs);
-        this.clientNodes = (clientNodes == null) ? Collections.emptyList() :
+        this.clientNodes = (clientNodes == null) ? List.of() :

Review Comment:
   ```java
   this.clientNodes = clientNodes == null ? List.of() : 
List.copyOf(clientNodes);
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/basic/BasicNode.java:
##########
@@ -46,7 +45,7 @@ public BasicNode(String name, String hostname, Map<String, 
String> config,
     public BasicNode(String name, JsonNode root) {
         this.name = name;
         String hostname = "localhost";
-        Set<String> tags = Collections.emptySet();
+        Set<String> tags = Set.of();

Review Comment:
   Perhaps it could be initialized as a `HashSet` instead?



##########
trogdor/src/main/java/org/apache/kafka/trogdor/coordinator/CoordinatorClient.java:
##########
@@ -471,8 +470,7 @@ static String prettyPrintTasksResponse(TasksResponse 
response, ZoneOffset zoneOf
             return "No matching tasks found.";
         }
         List<List<String>> lines = new ArrayList<>();
-        List<String> header = new ArrayList<>(
-            Arrays.asList("ID", "TYPE", "STATE", "INFO"));
+        List<String> header = List.of("ID", "TYPE", "STATE", "INFO");

Review Comment:
   ```java
   lines.add(List.of("ID", "TYPE", "STATE", "INFO"));
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/TopicsSpec.java:
##########
@@ -68,9 +67,8 @@ public void set(String name, PartitionsSpec value) {
     }
 
     public TopicsSpec immutableCopy() {
-        HashMap<String, PartitionsSpec> mapCopy = new HashMap<>();
-        mapCopy.putAll(map);
-        return new TopicsSpec(Collections.unmodifiableMap(mapCopy));
+        HashMap<String, PartitionsSpec> mapCopy = new HashMap<>(map);
+        return new TopicsSpec(Map.copyOf(mapCopy));

Review Comment:
   ```java
   return new TopicsSpec(Map.copyOf(map));
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TaskRequest.java:
##########
@@ -23,15 +23,12 @@
 /**
  * The request to /coordinator/tasks/{taskId}
  */
-public class TaskRequest {
-    private final String taskId;
-
+public record TaskRequest(String taskId) {
     @JsonCreator
     public TaskRequest(@JsonProperty("taskId") String taskId) {
         this.taskId = taskId == null ? "" : taskId;
     }
 
-    @JsonProperty
     public String taskId() {

Review Comment:
   This method is no longer necessary after switching to a record class, right?



##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/TasksRequest.java:
##########
@@ -69,8 +68,7 @@ public TasksRequest(@JsonProperty("taskIds") 
Collection<String> taskIds,
             @JsonProperty("firstEndMs") long firstEndMs,
             @JsonProperty("lastEndMs") long lastEndMs,
             @JsonProperty("state") Optional<TaskStateType> state) {
-        this.taskIds = Collections.unmodifiableSet((taskIds == null) ?
-            new HashSet<>() : new HashSet<>(taskIds));
+        this.taskIds = Set.copyOf((taskIds == null) ? new HashSet<>() : new 
HashSet<>(taskIds));

Review Comment:
   ```java
   this.taskIds = taskIds == null ? Set.of() : Set.copyOf(taskIds);
   ```



##########
trogdor/src/main/java/org/apache/kafka/trogdor/workload/RandomComponent.java:
##########
@@ -22,25 +22,23 @@
 
 /**
  * Contains a percent value represented as an integer between 1 and 100 and a 
PayloadGenerator to specify
- * how often that PayloadGenerator should be used. 
+ * how often that PayloadGenerator should be used.
  */
-public class RandomComponent {
-    private final int percent;
-    private final PayloadGenerator component;
-    
-
+public record RandomComponent(int percent, PayloadGenerator component) {
     @JsonCreator
     public RandomComponent(@JsonProperty("percent") int percent,
                            @JsonProperty("component") PayloadGenerator 
component) {
         this.percent = percent;
         this.component = component;
     }
 
+    @Override
     @JsonProperty
     public int percent() {

Review Comment:
   ditto. are those methods necessary?



##########
trogdor/src/test/java/org/apache/kafka/trogdor/common/MiniTrogdorCluster.java:
##########
@@ -161,7 +161,7 @@ public MiniTrogdorCluster build() throws Exception {
                         Integer.toString(node.coordinatorPort));
                 }
                 node.node = new BasicNode(entry.getKey(), node.hostname, 
config,

Review Comment:
   ```java
   node.node = new BasicNode(entry.getKey(), node.hostname, config, Set.of());
   ```



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to