MINOR: Cleanups in Shell Module (#16204)

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
apache · Jun 5, 2024 · f2aafcc · f2aafcc
1 parent bd9d68f
commit f2aafcc
Show file tree

Hide file tree

Showing 15 changed files with 32 additions and 46 deletions.
diff --git a/shell/src/main/java/org/apache/kafka/shell/MetadataShell.java b/shell/src/main/java/org/apache/kafka/shell/MetadataShell.java
@@ -101,7 +101,7 @@ static File parentParent(File file) {
      * Take the FileLock in the given directory, if it already exists. Technically, there is a
      * TOCTOU bug here where someone could create and lock the lockfile in between our check
      * and our use. However, this is very unlikely to ever be a problem in practice, and closing
-     * this hole would require the parent parent directory to always be writable when loading a
+     * this hole would require the parent directory to always be writable when loading a
      * snapshot so that we could create our .lock file there.
      */
     static FileLock takeDirectoryLockIfExists(File directory) {

diff --git a/shell/src/main/java/org/apache/kafka/shell/command/CatCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/CatCommandHandler.java
@@ -121,7 +121,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof CatCommandHandler)) return false;
         CatCommandHandler o = (CatCommandHandler) other;
-        if (!Objects.equals(o.targets, targets)) return false;
-        return true;
+        return Objects.equals(o.targets, targets);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/CdCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/CdCommandHandler.java
@@ -112,7 +112,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof CdCommandHandler)) return false;
         CdCommandHandler o = (CdCommandHandler) other;
-        if (!o.target.equals(target)) return false;
-        return true;
+        return o.target.equals(target);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/ErroneousCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/ErroneousCommandHandler.java
@@ -52,8 +52,7 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof ErroneousCommandHandler)) return false;
         ErroneousCommandHandler o = (ErroneousCommandHandler) other;
-        if (!Objects.equals(o.message, message)) return false;
-        return true;
+        return Objects.equals(o.message, message);
     }
 
     @Override

diff --git a/shell/src/main/java/org/apache/kafka/shell/command/ExitCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/ExitCommandHandler.java
@@ -89,7 +89,6 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object other) {
-        if (!(other instanceof ExitCommandHandler)) return false;
-        return true;
+        return other instanceof ExitCommandHandler;
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/FindCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/FindCommandHandler.java
@@ -127,7 +127,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof FindCommandHandler)) return false;
         FindCommandHandler o = (FindCommandHandler) other;
-        if (!Objects.equals(o.paths, paths)) return false;
-        return true;
+        return Objects.equals(o.paths, paths);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/HelpCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/HelpCommandHandler.java
@@ -89,7 +89,6 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object other) {
-        if (!(other instanceof HelpCommandHandler)) return false;
-        return true;
+        return other instanceof HelpCommandHandler;
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/LsCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/LsCommandHandler.java
@@ -115,8 +115,7 @@ public void run(
                     MetadataNodeInfo info = entryOption.get();
                     MetadataNode node = info.node();
                     if (node.isDirectory()) {
-                        List<String> children = new ArrayList<>();
-                        children.addAll(node.childNames());
+                        List<String> children = new ArrayList<>(node.childNames());
                         children.sort(String::compareTo);
                         targetDirectories.add(
                             new TargetDirectory(info.lastPathComponent(), children));
@@ -128,8 +127,7 @@ public void run(
                 }
             }));
         }
-        OptionalInt screenWidth = shell.isPresent() ?
-            OptionalInt.of(shell.get().screenWidth()) : OptionalInt.empty();
+        OptionalInt screenWidth = shell.map(interactiveShell -> OptionalInt.of(interactiveShell.screenWidth())).orElseGet(OptionalInt::empty);
         log.trace("LS : targetFiles = {}, targetDirectories = {}, screenWidth = {}",
             targetFiles, targetDirectories, screenWidth);
         printTargets(writer, screenWidth, targetFiles, targetDirectories);
@@ -271,8 +269,7 @@ public boolean equals(Object o) {
             if (!(o instanceof ColumnSchema)) return false;
             ColumnSchema other = (ColumnSchema) o;
             if (entriesPerColumn != other.entriesPerColumn) return false;
-            if (!Arrays.equals(columnWidths, other.columnWidths)) return false;
-            return true;
+            return Arrays.equals(columnWidths, other.columnWidths);
         }
 
         @Override
@@ -298,7 +295,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof LsCommandHandler)) return false;
         LsCommandHandler o = (LsCommandHandler) other;
-        if (!Objects.equals(o.targets, targets)) return false;
-        return true;
+        return Objects.equals(o.targets, targets);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/ManCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/ManCommandHandler.java
@@ -110,7 +110,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof ManCommandHandler)) return false;
         ManCommandHandler o = (ManCommandHandler) other;
-        if (!o.cmd.equals(cmd)) return false;
-        return true;
+        return o.cmd.equals(cmd);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/NoOpCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/NoOpCommandHandler.java
@@ -42,7 +42,6 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object other) {
-        if (!(other instanceof NoOpCommandHandler)) return false;
-        return true;
+        return other instanceof NoOpCommandHandler;
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/PwdCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/PwdCommandHandler.java
@@ -88,7 +88,6 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object other) {
-        if (!(other instanceof PwdCommandHandler)) return false;
-        return true;
+        return other instanceof PwdCommandHandler;
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/command/TreeCommandHandler.java b/shell/src/main/java/org/apache/kafka/shell/command/TreeCommandHandler.java
@@ -117,7 +117,6 @@ public int hashCode() {
     public boolean equals(Object other) {
         if (!(other instanceof TreeCommandHandler)) return false;
         TreeCommandHandler o = (TreeCommandHandler) other;
-        if (!Objects.equals(o.targets, targets)) return false;
-        return true;
+        return Objects.equals(o.targets, targets);
     }
 }
diff --git a/shell/src/main/java/org/apache/kafka/shell/glob/GlobVisitor.java b/shell/src/main/java/org/apache/kafka/shell/glob/GlobVisitor.java
@@ -76,8 +76,7 @@ public boolean equals(Object o) {
             if (!(o instanceof MetadataNodeInfo)) return false;
             MetadataNodeInfo other = (MetadataNodeInfo) o;
             if (!Arrays.equals(path, other.path)) return false;
-            if (!node.equals(other.node)) return false;
-            return true;
+            return node.equals(other.node);
         }
 
         @Override

diff --git a/shell/src/test/java/org/apache/kafka/shell/command/CommandTest.java b/shell/src/test/java/org/apache/kafka/shell/command/CommandTest.java
@@ -30,39 +30,39 @@
 public class CommandTest {
     @Test
     public void testParseCommands() {
-        assertEquals(new CatCommandHandler(Arrays.asList("foo")),
+        assertEquals(new CatCommandHandler(Collections.singletonList("foo")),
             new Commands(true).parseCommand(Arrays.asList("cat", "foo")));
         assertEquals(new CdCommandHandler(Optional.empty()),
-            new Commands(true).parseCommand(Arrays.asList("cd")));
+            new Commands(true).parseCommand(Collections.singletonList("cd")));
         assertEquals(new CdCommandHandler(Optional.of("foo")),
             new Commands(true).parseCommand(Arrays.asList("cd", "foo")));
         assertEquals(new ExitCommandHandler(),
-            new Commands(true).parseCommand(Arrays.asList("exit")));
+            new Commands(true).parseCommand(Collections.singletonList("exit")));
         assertEquals(new HelpCommandHandler(),
-            new Commands(true).parseCommand(Arrays.asList("help")));
+            new Commands(true).parseCommand(Collections.singletonList("help")));
         assertEquals(new HistoryCommandHandler(3),
             new Commands(true).parseCommand(Arrays.asList("history", "3")));
         assertEquals(new HistoryCommandHandler(Integer.MAX_VALUE),
-            new Commands(true).parseCommand(Arrays.asList("history")));
+            new Commands(true).parseCommand(Collections.singletonList("history")));
         assertEquals(new LsCommandHandler(Collections.emptyList()),
-            new Commands(true).parseCommand(Arrays.asList("ls")));
+            new Commands(true).parseCommand(Collections.singletonList("ls")));
         assertEquals(new LsCommandHandler(Arrays.asList("abc", "123")),
             new Commands(true).parseCommand(Arrays.asList("ls", "abc", "123")));
         assertEquals(new PwdCommandHandler(),
-            new Commands(true).parseCommand(Arrays.asList("pwd")));
+            new Commands(true).parseCommand(Collections.singletonList("pwd")));
     }
 
     @Test
     public void testParseInvalidCommand() {
         assertEquals(new ErroneousCommandHandler("invalid choice: 'blah' (choose " +
             "from 'cat', 'cd', 'exit', 'find', 'help', 'history', 'ls', 'man', 'pwd', 'tree')"),
-            new Commands(true).parseCommand(Arrays.asList("blah")));
+            new Commands(true).parseCommand(Collections.singletonList("blah")));
     }
 
     @Test
     public void testEmptyCommandLine() {
         assertEquals(new NoOpCommandHandler(),
-            new Commands(true).parseCommand(Arrays.asList("")));
+            new Commands(true).parseCommand(Collections.singletonList("")));
         assertEquals(new NoOpCommandHandler(),
             new Commands(true).parseCommand(Collections.emptyList()));
     }

diff --git a/shell/src/test/java/org/apache/kafka/shell/glob/GlobVisitorTest.java b/shell/src/test/java/org/apache/kafka/shell/glob/GlobVisitorTest.java
@@ -29,6 +29,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -99,11 +100,11 @@ public MetadataNode child(String name) {
     }
 
     static class InfoConsumer implements Consumer<Optional<MetadataNodeInfo>> {
-        private Optional<List<MetadataNodeInfo>> infos = null;
+        private Optional<List<MetadataNodeInfo>> infos = Optional.empty();
 
         @Override
         public void accept(Optional<MetadataNodeInfo> info) {
-            if (infos == null) {
+            if (!infos.isPresent()) {
                 if (info.isPresent()) {
                     infos = Optional.of(new ArrayList<>());
                     infos.get().add(info.get());
@@ -137,7 +138,7 @@ public void testDotDot() {
         InfoConsumer consumer = new InfoConsumer();
         GlobVisitor visitor = new GlobVisitor("..", consumer);
         visitor.accept(DATA);
-        assertEquals(Optional.of(Arrays.asList(
+        assertEquals(Optional.of(Collections.singletonList(
             new MetadataNodeInfo(new String[0], DATA.root()))), consumer.infos);
     }
 
@@ -146,7 +147,7 @@ public void testDoubleDotDot() {
         InfoConsumer consumer = new InfoConsumer();
         GlobVisitor visitor = new GlobVisitor("../..", consumer);
         visitor.accept(DATA);
-        assertEquals(Optional.of(Arrays.asList(
+        assertEquals(Optional.of(Collections.singletonList(
             new MetadataNodeInfo(new String[0], DATA.root()))), consumer.infos);
     }
 
@@ -189,8 +190,8 @@ public void testAbsoluteGlob() {
         InfoConsumer consumer = new InfoConsumer();
         GlobVisitor visitor = new GlobVisitor("/a?pha", consumer);
         visitor.accept(DATA);
-        assertEquals(Optional.of(Arrays.asList(
-            new MetadataNodeInfo(new String[] {"alpha"},
+        assertEquals(Optional.of(Collections.singletonList(
+            new MetadataNodeInfo(new String[]{"alpha"},
                 DATA.root().child("alpha")))), consumer.infos);
     }
 }