From 0b64b086b87bf8b046eb2b5cda5c25206e46a2f7 Mon Sep 17 00:00:00 2001 From: pancx Date: Thu, 20 Feb 2025 12:03:52 +0800 Subject: [PATCH] [#6481] improve(CLI): Refactor table output format fix some code. --- .../apache/gravitino/cli/CommandContext.java | 12 --- .../gravitino/cli/commands/ListCatalogs.java | 4 + .../gravitino/cli/commands/ListMetalakes.java | 4 + .../cli/outputs/BaseOutputFormat.java | 7 +- .../gravitino/cli/outputs/PlainFormat.java | 12 +-- .../gravitino/cli/outputs/TableFormat.java | 74 +++---------------- .../gravitino/cli/output/TestPlainFormat.java | 1 - .../gravitino/cli/output/TestTableFormat.java | 35 --------- 8 files changed, 22 insertions(+), 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 index e6e767739cc..6f41e3697eb 100644 --- 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 index 70301a131a0..6dc8b4cea3b 100644 --- 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 index ef2214e05ec..0a030a34788 100644 --- 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 index 12ec6aa5b0f..a233fe87512 100644 --- 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 implements OutputFormat { - protected int limit; protected CommandContext context; /** @@ -43,9 +40,7 @@ public abstract class BaseOutputFormat implements OutputFormat { * @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 index 967df71a676..f61578ffc6f 100644 --- 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 metalakeNames = - Arrays.stream(metalakes).map(Metalake::name).collect(Collectors.toList()); - return NEWLINE_JOINER.join(metalakeNames); - } + List 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 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 index d609663ef96..6d08f73edf0 100644 --- 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 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 index 8a71061d915..300a12e9d57 100644 --- 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 index 0709259f554..64d5ea4987b 100644 --- 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 tableFormat = - new TableFormat(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; }