[#6481] improve(CLI): Refactor table output format

fix some code.
apache · Feb 20, 2025 · 0b64b08 · 0b64b08
1 parent 9f9cbd8
commit 0b64b08
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 127 deletions.
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java b/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java
@@ -32,7 +32,6 @@ public class CommandContext {
   private final boolean quiet;
   private final CommandLine line;
   private final String auth;
-  private final int outputLimit;
 
   private String ignoreEnv;
   private boolean ignoreSet = false;
@@ -56,8 +55,6 @@ public CommandContext(CommandLine line) {
             ? line.getOptionValue(GravitinoOptions.OUTPUT)
             : Command.OUTPUT_FORMAT_PLAIN;
     this.quiet = line.hasOption(GravitinoOptions.QUIET);
-    // TODO add limit option to CLI
-    this.outputLimit = -1;
 
     this.url = getUrl();
     this.ignoreVersions = getIgnore();
@@ -118,15 +115,6 @@ public String auth() {
     return auth;
   }
 
-  /**
-   * Returns the output limit.
-   *
-   * @return The output limit.
-   */
-  public int outputLimit() {
-    return outputLimit;
-  }
-
   /**
    * Retrieves the Gravitino URL from the command line options or the GRAVITINO_URL environment
    * variable or the Gravitino config file.

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListCatalogs.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListCatalogs.java
@@ -48,6 +48,10 @@ public void handle() {
     try {
       GravitinoClient client = buildClient(metalake);
       catalogs = client.listCatalogsInfo();
+      if (catalogs.length == 0) {
+        printInformation("No catalogs exist.");
+        return;
+      }
       printResults(catalogs);
     } catch (NoSuchMetalakeException err) {
       exitWithError(ErrorMessages.UNKNOWN_METALAKE);

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListMetalakes.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListMetalakes.java
@@ -42,6 +42,10 @@ public void handle() {
     try {
       GravitinoAdminClient client = buildAdminClient();
       metalakes = client.listMetalakes();
+      if (metalakes.length == 0) {
+        printInformation("No metalakes exist.");
+        return;
+      }
       printResults(metalakes);
     } catch (Exception exp) {
       exitWithError(exp.getMessage());

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/BaseOutputFormat.java b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/BaseOutputFormat.java
@@ -19,7 +19,6 @@
 
 package org.apache.gravitino.cli.outputs;
 
-import com.google.common.base.Preconditions;
 import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -30,11 +29,9 @@
 
 /**
  * Abstract base implementation of {@link OutputFormat} interface providing common functionality for
- * various output format implementations. This class handles basic output operations and provides
- * configurable behavior for quiet mode, output limiting.
+ * various output format implementations.
  */
 public abstract class BaseOutputFormat<T> implements OutputFormat<T> {
-  protected int limit;
   protected CommandContext context;
 
   /**
@@ -43,9 +40,7 @@ public abstract class BaseOutputFormat<T> implements OutputFormat<T> {
    * @param context the command context, must not be null;
    */
   public BaseOutputFormat(CommandContext context) {
-    Preconditions.checkNotNull(context, "CommandContext cannot be null");
     this.context = context;
-    this.limit = context.outputLimit();
   }
 
   /**

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/PlainFormat.java b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/PlainFormat.java
@@ -89,13 +89,9 @@ public MetalakeListPlainFormat(CommandContext context) {
     /** {@inheritDoc} */
     @Override
     public String getOutput(Metalake[] metalakes) {
-      if (metalakes == null || metalakes.length == 0) {
-        return null;
-      } else {
-        List<String> metalakeNames =
-            Arrays.stream(metalakes).map(Metalake::name).collect(Collectors.toList());
-        return NEWLINE_JOINER.join(metalakeNames);
-      }
+      List<String> metalakeNames =
+          Arrays.stream(metalakes).map(Metalake::name).collect(Collectors.toList());
+      return NEWLINE_JOINER.join(metalakeNames);
     }
   }
 
@@ -129,7 +125,7 @@ public CatalogListPlainFormat(CommandContext context) {
     @Override
     public String getOutput(Catalog[] catalogs) {
       if (catalogs == null || catalogs.length == 0) {
-        output("No catalogs exists.", System.err);
+        output("No catalogs exist.", System.out);
         return null;
       } else {
         List<String> catalogNames =

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java b/clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java
@@ -38,7 +38,6 @@
 import static org.apache.gravitino.cli.outputs.OutputConstant.TABLE_UPPER_BORDER_MIDDLE_IDX;
 import static org.apache.gravitino.cli.outputs.OutputConstant.TABLE_UPPER_BORDER_RIGHT_IDX;
 
-import com.google.common.base.Preconditions;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
@@ -98,7 +97,6 @@ public TableFormat(CommandContext context) {
    * @return the table formatted output string.
    */
   public String getTableFormat(Column... columns) {
-    checkColumns(columns);
     ByteArrayOutputStream baos = new ByteArrayOutputStream();
 
     String[] headers =
@@ -108,16 +106,11 @@ public String getTableFormat(Column... columns) {
             .toArray(String[]::new);
 
     List<Character> borders = OutputConstant.BASIC_ASCII;
-    checkHeaders(headers, columns);
 
     if (headers.length != columns.length) {
       throw new IllegalArgumentException("Headers must be provided for all columns");
     }
 
-    if (limit != -1) {
-      columns = getLimitedColumns(columns);
-    }
-
     try (OutputStreamWriter osw = new OutputStreamWriter(baos, StandardCharsets.UTF_8)) {
       writeUpperBorder(osw, borders, System.lineSeparator(), columns);
       writeHeader(osw, borders, System.lineSeparator(), columns);
@@ -132,46 +125,6 @@ public String getTableFormat(Column... columns) {
     return new String(baos.toByteArray(), StandardCharsets.UTF_8);
   }
 
-  private void checkColumns(Column... columns) {
-    Preconditions.checkArgument(columns.length > 0, "At least one column must be provided");
-    int cellCount = columns[0].getCellCount();
-    for (Column column : columns) {
-      Preconditions.checkArgument(
-          column.getCellCount() == cellCount, "All columns must have the same cell count");
-    }
-  }
-
-  private void checkHeaders(String[] headers, Column[] columns) {
-    Preconditions.checkArgument(
-        headers.length == columns.length, "Headers must be provided for all columns");
-    for (String header : headers) {
-      Preconditions.checkArgument(header != null, "Headers must not be null");
-    }
-  }
-
-  /**
-   * Limits the number of rows in the table columns based on a predefined limit. If the current cell
-   * count is below the limit, returns the original columns unchanged. Otherwise, creates new
-   * columns with data truncated to the limit.
-   *
-   * @param columns The array of columns to potentially limit
-   * @return A new array of columns with limited rows, or the original array if no limiting is
-   *     needed
-   * @throws IllegalArgumentException If the columns array is null or empty
-   */
-  private Column[] getLimitedColumns(Column[] columns) {
-    if (columns[0].getCellCount() < limit) {
-      return columns;
-    }
-
-    Column[] limitedColumns = new Column[columns.length];
-    for (int i = 0; i < columns.length; i++) {
-      limitedColumns[i] = columns[i].getLimitedColumn(limit);
-    }
-
-    return limitedColumns;
-  }
-
   /**
    * Writes the top border of the table using specified border characters.
    *
@@ -533,15 +486,11 @@ public MetalakeListTableFormat(CommandContext context) {
     /** {@inheritDoc} */
     @Override
     public String getOutput(Metalake[] metalakes) {
-      if (metalakes.length == 0) {
-        output("No metalakes exist.", System.err);
-        return null;
-      } else {
-        Column columnName = new Column(context, "metalake");
-        Arrays.stream(metalakes).forEach(metalake -> columnName.addCell(metalake.name()));
-
-        return getTableFormat(columnName);
-      }
+
+      Column columnName = new Column(context, "metalake");
+      Arrays.stream(metalakes).forEach(metalake -> columnName.addCell(metalake.name()));
+
+      return getTableFormat(columnName);
     }
   }
 
@@ -585,15 +534,10 @@ public CatalogListTableFormat(CommandContext context) {
     /** {@inheritDoc} */
     @Override
     public String getOutput(Catalog[] catalogs) {
-      if (catalogs.length == 0) {
-        output("No metalakes exist.", System.err);
-        return null;
-      } else {
-        Column columnName = new Column(context, "catalog");
-        Arrays.stream(catalogs).forEach(metalake -> columnName.addCell(metalake.name()));
-
-        return getTableFormat(columnName);
-      }
+      Column columnName = new Column(context, "catalog");
+      Arrays.stream(catalogs).forEach(metalake -> columnName.addCell(metalake.name()));
+
+      return getTableFormat(columnName);
     }
   }
 }
diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestPlainFormat.java b/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestPlainFormat.java
@@ -116,7 +116,6 @@ void testOutputWithUnsupportType() {
 
   private CommandContext getMockContext() {
     CommandContext mockContext = mock(CommandContext.class);
-    when(mockContext.outputLimit()).thenReturn(-1);
 
     return mockContext;
   }

diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java b/clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java
@@ -224,40 +224,6 @@ public String getOutput(String entity) {
         outputString);
   }
 
-  @Test
-  void testTableOutputWithLimit() {
-    CommandContext mockContext = mock(CommandContext.class);
-    when(mockContext.outputLimit()).thenReturn(5);
-
-    Column columnA = new Column(mockContext, "metalake");
-    Column columnB = new Column(mockContext, "comment");
-
-    addRepeatedCells(columnA, 10);
-    addRepeatedCells(columnB, 10);
-
-    TableFormat<String> tableFormat =
-        new TableFormat<String>(mockContext) {
-          @Override
-          public String getOutput(String entity) {
-            return null;
-          }
-        };
-
-    String outputString = tableFormat.getTableFormat(columnA, columnB).trim();
-    Assertions.assertEquals(
-        "+------------+-----------+\n"
-            + "|  Metalake  |  Comment  |\n"
-            + "+------------+-----------+\n"
-            + "| Metalake-1 | Comment-1 |\n"
-            + "| Metalake-2 | Comment-2 |\n"
-            + "| Metalake-3 | Comment-3 |\n"
-            + "| Metalake-4 | Comment-4 |\n"
-            + "| Metalake-5 | Comment-5 |\n"
-            + "| …          | …         |\n"
-            + "+------------+-----------+",
-        outputString);
-  }
-
   @Test
   void testMetalakeDetailsWithTableFormat() {
     CommandContext mockContext = getMockContext();
@@ -349,7 +315,6 @@ private void addRepeatedCells(Column column, int count) {
 
   private CommandContext getMockContext() {
     CommandContext mockContext = mock(CommandContext.class);
-    when(mockContext.outputLimit()).thenReturn(-1);
 
     return mockContext;
   }