Skip to content

Commit

Permalink
MINOR: Cleanups in Shell Module (apache#16204)
Browse files Browse the repository at this point in the history
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
  • Loading branch information
sjhajharia committed Jun 5, 2024
1 parent bd9d68f commit f2aafcc
Show file tree
Hide file tree
Showing 15 changed files with 32 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}
}

0 comments on commit f2aafcc

Please sign in to comment.