From 323d96abd1289b8510d92e12ee64c285c3c3cd38 Mon Sep 17 00:00:00 2001 From: Rishi Agarwal Date: Tue, 30 Mar 2021 19:09:37 -0400 Subject: [PATCH] Review Comments --- .../aggregation/dynamic/TableTypeTest.java | 52 +++++++++---------- .../elide/modelconfig/model/Argument.java | 9 ++-- .../elide/modelconfig/model/Dimension.java | 32 +++++++----- .../yahoo/elide/modelconfig/model/Join.java | 10 ++-- .../elide/modelconfig/model/Measure.java | 23 ++++---- .../yahoo/elide/modelconfig/model/Table.java | 42 ++++++++------- 6 files changed, 92 insertions(+), 76 deletions(-) diff --git a/elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/dynamic/TableTypeTest.java b/elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/dynamic/TableTypeTest.java index 73c9d18e04..f5615b0a91 100644 --- a/elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/dynamic/TableTypeTest.java +++ b/elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/dynamic/TableTypeTest.java @@ -47,10 +47,10 @@ public class TableTypeTest { @Test void testGetAndSetField() throws Exception { Table testTable = Table.builder() - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .name("dim1") .type(Type.BOOLEAN) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -177,7 +177,7 @@ void testMeasureAnnotations() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .measures(Arrays.asList(Measure.builder() + .measure(Measure.builder() .type(Type.MONEY) .category("category1") .definition("SUM{{ price}}") @@ -187,7 +187,7 @@ void testMeasureAnnotations() throws Exception { .readAccess("Admin") .description("A measure") .tags(tags) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -217,7 +217,7 @@ void testDimensionAnnotations() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .type(Type.TEXT) .category("category1") .definition("{{region}}") @@ -229,7 +229,7 @@ void testDimensionAnnotations() throws Exception { .tags(tags) .cardinality("small") .tableSource("region.id") - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -258,11 +258,11 @@ void testDimensionTableValues() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .type(Type.COORDINATE) .name("location") .values(values) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -281,7 +281,7 @@ void testTimeDimensionAnnotations() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .type(Type.TIME) .category("category1") .definition("{{createdOn }}") @@ -291,7 +291,7 @@ void testTimeDimensionAnnotations() throws Exception { .readAccess("Admin") .description("A time dimension") .tags(tags) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -323,19 +323,19 @@ void testMultipleTimeDimensionGrains() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .type(Type.TIME) .definition("{{createdOn}}") .name("createdOn") - .grains(Arrays.asList(Grain.builder() + .grain(Grain.builder() .sql("some sql") .type(Grain.GrainType.DAY) - .build(), - Grain.builder() + .build()) + .grain(Grain.builder() .sql("some other sql") .type(Grain.GrainType.YEAR) - .build())) - .build())) + .build()) + .build()) .build(); TableType testType = new TableType(testTable); @@ -356,21 +356,21 @@ void testMultipleTimeDimensionGrains() throws Exception { void testJoinField() throws Exception { Table testTable1 = Table.builder() .name("table1") - .joins(Arrays.asList(Join.builder() + .join(Join.builder() .definition("{{id }} = {{ table2.id}}") .kind(Join.Kind.TOONE) .type(Join.Type.INNER) .name("join1") .to("table2.dim2") - .build())) + .build()) .build(); Table testTable2 = Table.builder() .name("table2") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .name("dim2") .type(Type.BOOLEAN) - .build())) + .build()) .build(); TableType testType1 = new TableType(testTable1); @@ -397,11 +397,11 @@ void testHiddenDimension() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .dimensions(Arrays.asList(Dimension.builder() + .dimension(Dimension.builder() .name("dim1") .type(Type.BOOLEAN) .hidden(true) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -420,11 +420,11 @@ void testHiddenMeasure() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .measures(Arrays.asList(Measure.builder() + .measure(Measure.builder() .name("measure1") .type(Type.BOOLEAN) .hidden(true) - .build())) + .build()) .build(); TableType testType = new TableType(testTable); @@ -443,11 +443,11 @@ void testInvalidResolver() throws Exception { Table testTable = Table.builder() .table("table1") .name("Table") - .measures(Arrays.asList(Measure.builder() + .measure(Measure.builder() .name("measure1") .type(Type.BOOLEAN) .queryPlanResolver("does.not.exist.class") - .build())) + .build()) .build(); TableType testType = new TableType(testTable); diff --git a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Argument.java b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Argument.java index 34a17bea7d..07971808b6 100644 --- a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Argument.java +++ b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Argument.java @@ -13,7 +13,6 @@ import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; import java.util.LinkedHashSet; import java.util.Set; @@ -33,7 +32,6 @@ @Data @EqualsAndHashCode() @AllArgsConstructor -@NoArgsConstructor @Builder public class Argument implements Named { @@ -48,8 +46,7 @@ public class Argument implements Named { @JsonProperty("values") @JsonDeserialize(as = LinkedHashSet.class) - @Builder.Default - private Set values = new LinkedHashSet<>(); + private Set values; @JsonProperty("tableSource") private String tableSource; @@ -57,6 +54,10 @@ public class Argument implements Named { @JsonProperty("default") private Object defaultValue; + public Argument() { + this.values = new LinkedHashSet<>(); + } + /** * Returns description of the argument. * If null, returns the name diff --git a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Dimension.java b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Dimension.java index 461c27d235..cbe58c8998 100644 --- a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Dimension.java +++ b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Dimension.java @@ -13,7 +13,7 @@ import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; +import lombok.Singular; import java.util.ArrayList; import java.util.LinkedHashSet; @@ -44,7 +44,6 @@ @Data @EqualsAndHashCode() @AllArgsConstructor -@NoArgsConstructor @Builder public class Dimension implements Named { @@ -61,12 +60,10 @@ public class Dimension implements Named { private String category; @JsonProperty("hidden") - @Builder.Default - private Boolean hidden = false; + private Boolean hidden; @JsonProperty("readAccess") - @Builder.Default - private String readAccess = "Prefab.Role.All"; + private String readAccess; @JsonProperty("definition") private String definition; @@ -78,26 +75,33 @@ public class Dimension implements Named { private Type type; @JsonProperty("grains") - @Builder.Default - private List grains = new ArrayList<>(); + @Singular + private List grains; @JsonProperty("tags") @JsonDeserialize(as = LinkedHashSet.class) - @Builder.Default - private Set tags = new LinkedHashSet<>(); + private Set tags; @JsonProperty("arguments") - @Builder.Default - private List arguments = new ArrayList<>(); + @Singular + private List arguments; @JsonProperty("values") @JsonDeserialize(as = LinkedHashSet.class) - @Builder.Default - private Set values = new LinkedHashSet<>(); + private Set values; @JsonProperty("tableSource") private String tableSource; + public Dimension() { + this.hidden = false; + this.readAccess = "Prefab.Role.All"; + this.grains = new ArrayList<>(); + this.tags = new LinkedHashSet<>(); + this.values = new LinkedHashSet<>(); + this.arguments = new ArrayList<>(); + } + /** * Returns description of the dimension. * If null, returns the name. diff --git a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Join.java b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Join.java index 80a0c053a5..e4ffc57f72 100644 --- a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Join.java +++ b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Join.java @@ -8,11 +8,11 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; + import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; /** * Joins describe the SQL expression necessary to join two physical tables. @@ -29,7 +29,6 @@ @Data @EqualsAndHashCode() @AllArgsConstructor -@NoArgsConstructor @Builder public class Join implements Named { @@ -43,12 +42,15 @@ public class Join implements Named { private Join.Type type; @JsonProperty("kind") - @Builder.Default - private Join.Kind kind = Join.Kind.TOONE; + private Join.Kind kind; @JsonProperty("definition") private String definition; + public Join() { + this.kind = Join.Kind.TOONE; + } + public enum Kind { TOONE("toOne"), diff --git a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Measure.java b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Measure.java index e2dae62d8d..61f4937ba5 100644 --- a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Measure.java +++ b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Measure.java @@ -13,7 +13,7 @@ import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; +import lombok.Singular; import java.util.ArrayList; import java.util.LinkedHashSet; @@ -40,7 +40,6 @@ @Data @EqualsAndHashCode() @AllArgsConstructor -@NoArgsConstructor @Builder public class Measure implements Named { @@ -57,12 +56,10 @@ public class Measure implements Named { private String category; @JsonProperty("hidden") - @Builder.Default - private Boolean hidden = false; + private Boolean hidden; @JsonProperty("readAccess") - @Builder.Default - private String readAccess = "Prefab.Role.All"; + private String readAccess; @JsonProperty("definition") private String definition; @@ -75,12 +72,18 @@ public class Measure implements Named { @JsonProperty("tags") @JsonDeserialize(as = LinkedHashSet.class) - @Builder.Default - private Set tags = new LinkedHashSet<>(); + private Set tags; @JsonProperty("arguments") - @Builder.Default - private List arguments = new ArrayList<>(); + @Singular + private List arguments; + + public Measure() { + this.hidden = false; + this.readAccess = "Prefab.Role.All"; + this.tags = new LinkedHashSet<>(); + this.arguments = new ArrayList<>(); + } /** * Returns description of the measure. diff --git a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Table.java b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Table.java index b1131ab7e3..00692d725c 100644 --- a/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Table.java +++ b/elide-model-config/src/main/java/com/yahoo/elide/modelconfig/model/Table.java @@ -16,7 +16,7 @@ import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; -import lombok.NoArgsConstructor; +import lombok.Singular; import java.util.ArrayList; import java.util.LinkedHashSet; @@ -50,7 +50,6 @@ @Data @EqualsAndHashCode() @AllArgsConstructor -@NoArgsConstructor @Builder public class Table implements Named { @@ -67,12 +66,10 @@ public class Table implements Named { private String dbConnectionName; @JsonProperty("isFact") - @Builder.Default - private Boolean isFact = true; + private Boolean isFact; @JsonProperty("hidden") - @Builder.Default - private Boolean hidden = false; + private Boolean hidden; @JsonProperty("description") private String description; @@ -87,29 +84,27 @@ public class Table implements Named { private String cardinality; @JsonProperty("readAccess") - @Builder.Default - private String readAccess = "Prefab.Role.All"; + private String readAccess; @JsonProperty("joins") - @Builder.Default - private List joins = new ArrayList<>(); + @Singular + private List joins; @JsonProperty("measures") - @Builder.Default - private List measures = new ArrayList<>(); + @Singular + private List measures; @JsonProperty("dimensions") - @Builder.Default - private List dimensions = new ArrayList<>(); + @Singular + private List dimensions; @JsonProperty("tags") @JsonDeserialize(as = LinkedHashSet.class) - @Builder.Default - private Set tags = new LinkedHashSet<>(); + private Set tags; @JsonProperty("arguments") - @Builder.Default - private List arguments = new ArrayList<>(); + @Singular + private List arguments; @JsonProperty("extend") private String extend; @@ -120,6 +115,17 @@ public class Table implements Named { @JsonProperty("table") private String table; + public Table() { + this.isFact = true; + this.hidden = false; + this.readAccess = "Prefab.Role.All"; + this.joins = new ArrayList<>(); + this.measures = new ArrayList<>(); + this.dimensions = new ArrayList<>(); + this.tags = new LinkedHashSet<>(); + this.arguments = new ArrayList<>(); + } + /** * Returns description of the table object. * If null, returns the name.