feat: fix rohans comments

confluentinc · Aug 29, 2019 · 829df7d · 829df7d
1 parent 9195d0c
commit 829df7d
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 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
@@ -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<TypeList> {
 
@@ -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
@@ -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
@@ -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
@@ -27,6 +27,11 @@ public ListTypes(final Optional<NodeLocation> location) {
     super(location);
   }
 
+  @Override
+  public <R, C> R accept(final AstVisitor<R, C> 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
@@ -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<SqlBaseType, Function<SchemaInfo, String>> TO_TYPE_STRING =
+      ImmutableMap.<SqlBaseType, Function<SchemaInfo, String>>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);
   }
 }