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