Skip to content

Commit

Permalink
Fixes shell pagination (apache#5251)
Browse files Browse the repository at this point in the history 
* Remove -np from all tests

* Check conditions for pagination

Adds a boolean to track if the shell is able to paginate and adds
specific conditions that invalidate pagination

* Removes unneeded check and mockshell terminal size

Removes the terminal size from MockShell so pagination does not occur.
  • Loading branch information
@ddanielr
ddanielr authored Jan 14, 2025
1 parent b140460 commit f3bd1b8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 36 deletions.
5 changes: 4 additions & 1 deletion shell/src/main/java/org/apache/accumulo/shell/Shell.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public class Shell extends ShellOptions implements KeywordExecutable {
protected String execCommand = null;
protected boolean verbose = true;

private boolean canPaginate = false;
private boolean tabCompletion;
private boolean disableAuthTimeout;
private long authTimeout;
Expand Down Expand Up @@ -383,6 +384,8 @@ public boolean config(String... args) throws IOException {
}
}

canPaginate = terminal.getSize().getRows() > 0;

// decide whether to execute commands from a file and quit
if (options.getExecFile() != null) {
execFile = options.getExecFile();
Expand Down Expand Up @@ -1066,7 +1069,7 @@ public final void printLines(Iterator<String> lines, boolean paginate, PrintLine
if (out == null) {
if (peek != null) {
writer.println(peek);
if (paginate) {
if (canPaginate && paginate) {
linesPrinted += peek.isEmpty() ? 0 : Math.ceil(peek.length() * 1.0 / termWidth);

// check if displaying the next line would result in
Expand Down
2 changes: 0 additions & 2 deletions test/src/main/java/org/apache/accumulo/test/shell/MockShell.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.apache.accumulo.shell.Shell;
import org.jline.reader.LineReader;
import org.jline.reader.LineReaderBuilder;
import org.jline.terminal.Size;
import org.jline.terminal.Terminal;
import org.jline.terminal.impl.DumbTerminal;
import org.slf4j.Logger;
Expand All @@ -53,7 +52,6 @@ public class MockShell {
output = new TestOutputStream();
input = new StringInputStream();
terminal = new DumbTerminal(input, output);
terminal.setSize(new Size(80, 24));
reader = LineReaderBuilder.builder().terminal(terminal).build();
shell = new Shell(reader);
shell.setLogErrorsToConsole();
Expand Down
65 changes: 32 additions & 33 deletions test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public void exporttableImporttable() throws Exception {
}
Thread.sleep(20);
ts.exec("importtable " + table2 + " " + import_, true);
ts.exec("config -t " + table2 + " -np", true, "345M", true);
ts.exec("config -t " + table2, true, "345M", true);
ts.exec("getsplits -t " + table2, true, "row5", true);
ts.exec("constraint --list -t " + table2, true, "VisibilityConstraint=2", true);
ts.exec("online " + table, true);
Expand All @@ -249,14 +249,14 @@ public void propStressTest() throws Exception {
for (int i = 0; i < 50; i++) {
String expected = (100 + i) + "M";
ts.exec("config -t " + table + " -s table.split.threshold=" + expected, true);
ts.exec("config -t " + table + " -np -f table.split.threshold", true, expected, true);
ts.exec("config -t " + table + " -f table.split.threshold", true, expected, true);

ts.exec("config -t " + table + " -s table.scan.max.memory=" + expected, true);
ts.exec("config -t " + table + " -np -f table.scan.max.memory", true, expected, true);
ts.exec("config -t " + table + " -f table.scan.max.memory", true, expected, true);

String bExpected = ((i % 2) == 0) ? "true" : "false";
ts.exec("config -t " + table + " -s table.bloom.enabled=" + bExpected, true);
ts.exec("config -t " + table + " -np -f table.bloom.enabled", true, bExpected, true);
ts.exec("config -t " + table + " -f table.bloom.enabled", true, bExpected, true);
}
}
}
Expand Down Expand Up @@ -688,7 +688,7 @@ public void clonetable() throws Exception {
ts.exec("table " + clone);
ts.exec("scan", true, "value", true);
ts.exec("constraint --list -t " + clone, true, "VisibilityConstraint=2", true);
ts.exec("config -t " + clone + " -np", true, "123M", true);
ts.exec("config -t " + clone, true, "123M", true);
ts.exec("getsplits -t " + clone, true, "a\nb\nc\n");
ts.exec("deletetable -f " + table);
ts.exec("deletetable -f " + clone);
Expand Down Expand Up @@ -717,7 +717,7 @@ public void clonetableOffline() throws Exception {
ts.exec("table " + clone);
ts.exec("scan", false, "TableOfflineException", true);
ts.exec("constraint --list -t " + clone, true, "VisibilityConstraint=2", true);
ts.exec("config -t " + clone + " -np", true, "123M", true);
ts.exec("config -t " + clone, true, "123M", true);
ts.exec("getsplits -t " + clone, true, "a\nb\nc\n");
ts.exec("deletetable -f " + table);
ts.exec("deletetable -f " + clone);
Expand Down Expand Up @@ -1080,7 +1080,7 @@ public void deletemany() throws Exception {
assertEquals(10, countkeys(table));
ts.exec("deletemany -f -b row8");
assertEquals(8, countkeys(table));
ts.exec("scan -t " + table + " -np", true, "row8", false);
ts.exec("scan -t " + table, true, "row8", false);
make10();
ts.exec("deletemany -f -b row4 -e row5");
assertEquals(8, countkeys(table));
Expand Down Expand Up @@ -1165,7 +1165,7 @@ public void formatter() {
expectedDefault.add("row2 cf1:cq []\t2468ace");
ArrayList<String> actualDefault = new ArrayList<>(4);
boolean isFirst = true;
for (String s : ts.exec("scan -np", true).split("[\n\r]+")) {
for (String s : ts.exec("scan", true).split("[\n\r]+")) {
if (isFirst) {
isFirst = false;
} else {
Expand All @@ -1181,7 +1181,7 @@ public void formatter() {
ts.exec("formatter -t formatter_test -f " + HexFormatter.class.getName(), true);
ArrayList<String> actualFormatted = new ArrayList<>(4);
isFirst = true;
for (String s : ts.exec("scan -np", true).split("[\n\r]+")) {
for (String s : ts.exec("scan", true).split("[\n\r]+")) {
if (isFirst) {
isFirst = false;
} else {
Expand Down Expand Up @@ -1292,7 +1292,7 @@ public void grep() throws Exception {

@Test
public void help() throws Exception {
ts.exec("help -np", true, "Help Commands", true);
ts.exec("help", true, "Help Commands", true);
ts.exec("?", true, "Help Commands", true);
for (String c : ("bye exit quit about help info ? "
+ "deleteiter deletescaniter listiter setiter setscaniter "
Expand Down Expand Up @@ -1376,7 +1376,7 @@ public void importDirectoryWithOptions() throws Exception {
ts.exec("createtable " + table, true);
ts.exec("notable", true);
ts.exec("importdirectory -t " + table + " -i " + importDir + " true", true);
ts.exec("scan -t " + table + " -np -b 0 -e 2", true, "0-->" + nonce, true);
ts.exec("scan -t " + table + " -b 0 -e 2", true, "0-->" + nonce, true);
ts.exec("scan -t " + table + " -b 00000098 -e 00000100", true, "99-->" + nonce, true);
// Attempt to re-import without -i option, error should occur
ts.exec("importdirectory -t " + table + " " + importDir + " true", false);
Expand Down Expand Up @@ -1661,7 +1661,7 @@ public void verifyPerTableClasspath(final String table, final File fooConstraint

sleepUninterruptibly(250, TimeUnit.MILLISECONDS);

ts.exec("scan -np", true, "foo", false);
ts.exec("scan ", true, "foo", false);

ts.exec("constraint -a FooConstraint", true);

Expand Down Expand Up @@ -1771,25 +1771,25 @@ public void namespaces() throws Exception {
}

private int countkeys(String table) {
ts.exec("scan -np -t " + table);
ts.exec("scan -t " + table);
return ts.output.get().split("\n").length - 1;
}

@Test
public void scans() throws Exception {
ts.exec("createtable t");
make10();
String result = ts.exec("scan -np -b row1 -e row1");
String result = ts.exec("scan -b row1 -e row1");
assertEquals(2, result.split("\n").length);
result = ts.exec("scan -np -b row3 -e row5");
result = ts.exec("scan -b row3 -e row5");
assertEquals(4, result.split("\n").length);
result = ts.exec("scan -np -r row3");
result = ts.exec("scan -r row3");
assertEquals(2, result.split("\n").length);
result = ts.exec("scan -np -b row:");
result = ts.exec("scan -b row:");
assertEquals(1, result.split("\n").length);
result = ts.exec("scan -np -b row");
result = ts.exec("scan -b row");
assertEquals(11, result.split("\n").length);
result = ts.exec("scan -np -e row:");
result = ts.exec("scan -e row:");
assertEquals(11, result.split("\n").length);
ts.exec("deletetable -f t");
}
Expand Down Expand Up @@ -1836,21 +1836,21 @@ public void scansWithClassLoaderContext() throws IOException {

// The implementation of ValueReversingIterator in the FAKE context does nothing, the value is
// not reversed.
result = ts.exec("scan -pn baz -np -b row1 -e row1");
result = ts.exec("scan -pn baz -b row1 -e row1");
assertEquals(2, result.split("\n").length);
assertTrue(result.contains("value"));
result = ts.exec("scan -pn baz -np -b row3 -e row5");
result = ts.exec("scan -pn baz -b row3 -e row5");
assertEquals(4, result.split("\n").length);
assertTrue(result.contains("value"));
result = ts.exec("scan -pn baz -np -r row3");
result = ts.exec("scan -pn baz -r row3");
assertEquals(2, result.split("\n").length);
assertTrue(result.contains("value"));
result = ts.exec("scan -pn baz -np -b row:");
result = ts.exec("scan -pn baz -b row:");
assertEquals(1, result.split("\n").length);
result = ts.exec("scan -pn baz -np -b row");
result = ts.exec("scan -pn baz -b row");
assertEquals(11, result.split("\n").length);
assertTrue(result.contains("value"));
result = ts.exec("scan -pn baz -np -e row:");
result = ts.exec("scan -pn baz -e row:");
assertEquals(11, result.split("\n").length);
assertTrue(result.contains("value"));

Expand All @@ -1862,25 +1862,25 @@ public void scansWithClassLoaderContext() throws IOException {
+ REAL_CONTEXT + "=" + REAL_CONTEXT_CLASSPATH + "\n", result);
// Override the table classloader context with the REAL implementation of
// ValueReversingIterator, which does reverse the value.
result = ts.exec("scan -pn baz -np -b row1 -e row1 -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -b row1 -e row1 -cc " + REAL_CONTEXT);
assertEquals(2, result.split("\n").length);
assertTrue(result.contains("eulav"));
assertFalse(result.contains("value"));
result = ts.exec("scan -pn baz -np -b row3 -e row5 -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -b row3 -e row5 -cc " + REAL_CONTEXT);
assertEquals(4, result.split("\n").length);
assertTrue(result.contains("eulav"));
assertFalse(result.contains("value"));
result = ts.exec("scan -pn baz -np -r row3 -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -r row3 -cc " + REAL_CONTEXT);
assertEquals(2, result.split("\n").length);
assertTrue(result.contains("eulav"));
assertFalse(result.contains("value"));
result = ts.exec("scan -pn baz -np -b row: -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -b row: -cc " + REAL_CONTEXT);
assertEquals(1, result.split("\n").length);
result = ts.exec("scan -pn baz -np -b row -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -b row -cc " + REAL_CONTEXT);
assertEquals(11, result.split("\n").length);
assertTrue(result.contains("eulav"));
assertFalse(result.contains("value"));
result = ts.exec("scan -pn baz -np -e row: -cc " + REAL_CONTEXT);
result = ts.exec("scan -pn baz -e row: -cc " + REAL_CONTEXT);
assertEquals(11, result.split("\n").length);
assertTrue(result.contains("eulav"));
assertFalse(result.contains("value"));
Expand Down Expand Up @@ -2046,8 +2046,7 @@ private void make10() {
private List<String> getFiles(String tableId) {
ts.output.clear();

ts.exec(
"scan -t " + MetadataTable.NAME + " -np -c file -b " + tableId + " -e " + tableId + "~");
ts.exec("scan -t " + MetadataTable.NAME + " -c file -b " + tableId + " -e " + tableId + "~");

log.debug("countFiles(): {}", ts.output.get());

Expand Down

0 comments on commit f3bd1b8

Please sign in to comment.