From 829df7dbdbbfa9cee5da12a7ab796be22fbd4dbe Mon Sep 17 00:00:00 2001 From: Almog Gavra Date: Thu, 29 Aug 2019 10:27:03 -0700 Subject: [PATCH] feat: fix rohans comments --- .../table/builder/TypeListTableBuilder.java | 3 ++ .../ksql/cli/console/ConsoleTest.java | 28 +++++------ .../ksql/parser/tree/AstVisitor.java | 4 ++ .../confluent/ksql/parser/tree/ListTypes.java | 5 ++ .../ksql/rest/entity/SchemaInfo.java | 47 ++++++++++--------- 5 files changed, 52 insertions(+), 35 deletions(-) diff --git a/ksql-cli/src/main/java/io/confluent/ksql/cli/console/table/builder/TypeListTableBuilder.java b/ksql-cli/src/main/java/io/confluent/ksql/cli/console/table/builder/TypeListTableBuilder.java index 4bf935f3a290..1f33201cdb16 100644 --- a/ksql-cli/src/main/java/io/confluent/ksql/cli/console/table/builder/TypeListTableBuilder.java +++ b/ksql-cli/src/main/java/io/confluent/ksql/cli/console/table/builder/TypeListTableBuilder.java @@ -18,7 +18,9 @@ import com.google.common.collect.ImmutableList; import io.confluent.ksql.cli.console.table.Table; import io.confluent.ksql.rest.entity.TypeList; +import java.util.Comparator; import java.util.List; +import java.util.Map.Entry; public class TypeListTableBuilder implements TableBuilder { @@ -32,6 +34,7 @@ public Table buildTable(final TypeList entity) { .getTypes() .entrySet() .stream() + .sorted(Comparator.comparing(Entry::getKey)) .map(entry -> ImmutableList.of(entry.getKey(), entry.getValue().toTypeString()))) .build(); } diff --git a/ksql-cli/src/test/java/io/confluent/ksql/cli/console/ConsoleTest.java b/ksql-cli/src/test/java/io/confluent/ksql/cli/console/ConsoleTest.java index c2f3b12b816d..dfae6cd08353 100644 --- a/ksql-cli/src/test/java/io/confluent/ksql/cli/console/ConsoleTest.java +++ b/ksql-cli/src/test/java/io/confluent/ksql/cli/console/ConsoleTest.java @@ -868,15 +868,15 @@ public void shouldPrintTypesList() throws IOException { // Given: final KsqlEntityList entities = new KsqlEntityList(ImmutableList.of( new TypeList("statement", ImmutableMap.of( + "typeB", new SchemaInfo( + SqlBaseType.ARRAY, + null, + new SchemaInfo(SqlBaseType.STRING, null, null)), "typeA", new SchemaInfo( SqlBaseType.STRUCT, ImmutableList.of( new FieldInfo("f1", new SchemaInfo(SqlBaseType.STRING, null, null))), - null), - "typeB", new SchemaInfo( - SqlBaseType.ARRAY, - null, - new SchemaInfo(SqlBaseType.STRING, null, null)) + null) )) )); @@ -890,6 +890,15 @@ public void shouldPrintTypesList() throws IOException { + " \"@type\" : \"type_list\",\n" + " \"statementText\" : \"statement\",\n" + " \"types\" : {\n" + + " \"typeB\" : {\n" + + " \"type\" : \"ARRAY\",\n" + + " \"fields\" : null,\n" + + " \"memberSchema\" : {\n" + + " \"type\" : \"STRING\",\n" + + " \"fields\" : null,\n" + + " \"memberSchema\" : null\n" + + " }\n" + + " },\n" + " \"typeA\" : {\n" + " \"type\" : \"STRUCT\",\n" + " \"fields\" : [ {\n" @@ -901,15 +910,6 @@ public void shouldPrintTypesList() throws IOException { + " }\n" + " } ],\n" + " \"memberSchema\" : null\n" - + " },\n" - + " \"typeB\" : {\n" - + " \"type\" : \"ARRAY\",\n" - + " \"fields\" : null,\n" - + " \"memberSchema\" : {\n" - + " \"type\" : \"STRING\",\n" - + " \"fields\" : null,\n" - + " \"memberSchema\" : null\n" - + " }\n" + " }\n" + " },\n" + " \"warnings\" : [ ]\n" diff --git a/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/AstVisitor.java b/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/AstVisitor.java index e9d0be196410..51198b985892 100644 --- a/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/AstVisitor.java +++ b/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/AstVisitor.java @@ -167,6 +167,10 @@ protected R visitListTables(final ListTables node, final C context) { return visitStatement(node, context); } + protected R visitListTypes(final ListTypes listTypes, final C context) { + return visitStatement(listTypes, context); + } + protected R visitUnsetProperty(final UnsetProperty node, final C context) { return visitStatement(node, context); } diff --git a/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/ListTypes.java b/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/ListTypes.java index 2f214adb72b4..5828116460fd 100644 --- a/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/ListTypes.java +++ b/ksql-parser/src/main/java/io/confluent/ksql/parser/tree/ListTypes.java @@ -27,6 +27,11 @@ public ListTypes(final Optional location) { super(location); } + @Override + public R accept(final AstVisitor visitor, final C context) { + return visitor.visitListTypes(this, context); + } + @Override public int hashCode() { return Objects.hashCode(getClass()); diff --git a/ksql-rest-app/src/main/java/io/confluent/ksql/rest/entity/SchemaInfo.java b/ksql-rest-app/src/main/java/io/confluent/ksql/rest/entity/SchemaInfo.java index a89ec0581b45..3106ba71ea56 100644 --- a/ksql-rest-app/src/main/java/io/confluent/ksql/rest/entity/SchemaInfo.java +++ b/ksql-rest-app/src/main/java/io/confluent/ksql/rest/entity/SchemaInfo.java @@ -19,11 +19,14 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.errorprone.annotations.Immutable; import io.confluent.ksql.schema.ksql.SqlBaseType; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.function.Function; import java.util.stream.Collectors; @Immutable @@ -77,27 +80,29 @@ public int hashCode() { return Objects.hash(type, fields, memberSchema); } + private static final Map> TO_TYPE_STRING = + ImmutableMap.>builder() + .put(SqlBaseType.STRING, si -> "VARCHAR(STRING)") + .put( + SqlBaseType.ARRAY, + si -> SqlBaseType.ARRAY + "<" + si.memberSchema.toTypeString() + ">") + .put( + SqlBaseType.MAP, + si -> SqlBaseType.MAP + + "<" + SqlBaseType.STRING + + "," + si.memberSchema.toTypeString() + + ">") + .put( + SqlBaseType.STRUCT, + si -> si.fields + .stream() + .map(f -> f.getName() + " " + f.getSchema().toTypeString()) + .collect(Collectors.joining(", ", SqlBaseType.STRUCT + "<", ">"))) + .build(); + public String toTypeString() { - switch (getType()) { - case ARRAY: - return SqlBaseType.ARRAY + "<" - + memberSchema.toTypeString() - + ">"; - case MAP: - return SqlBaseType.MAP - + "<" - + SqlBaseType.STRING + ", " - + memberSchema.toTypeString() - + ">"; - case STRUCT: - return fields - .stream() - .map(f -> f.getName() + " " + f.getSchema().toTypeString()) - .collect(Collectors.joining(", ", SqlBaseType.STRUCT + "<", ">")); - case STRING: - return "VARCHAR(STRING)"; - default: - return type.name(); - } + // needs a map instead of switch because for some reason switch creates an + // internal class with no annotations that messes up EntityTest + return TO_TYPE_STRING.getOrDefault(type, si -> si.type.name()).apply(this); } }