From 5b579b45ca90397d3a393c3225ee176800c75b79 Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Fri, 17 Nov 2017 17:12:28 -0800 Subject: [PATCH 1/6] First cut of config and validation --- .../java/com/yahoo/bullet/BulletConfig.java | 223 +++++++++++++++++- src/main/java/com/yahoo/bullet/Config.java | 11 +- src/main/java/com/yahoo/bullet/Utilities.java | 28 ++- src/main/java/com/yahoo/bullet/Validator.java | 198 ++++++++++++++++ .../aggregations/CountDistinct.java | 9 +- .../operations/aggregations/Distribution.java | 19 +- .../operations/aggregations/GroupBy.java | 12 +- .../operations/aggregations/KMVStrategy.java | 13 +- .../bullet/operations/aggregations/Raw.java | 13 +- .../aggregations/SketchingStrategy.java | 12 +- .../bullet/operations/aggregations/TopK.java | 19 +- .../aggregations/grouping/GroupOperation.java | 2 +- .../com/yahoo/bullet/parsing/Aggregation.java | 19 +- .../yahoo/bullet/parsing/Configurable.java | 8 +- .../java/com/yahoo/bullet/parsing/Parser.java | 10 +- .../yahoo/bullet/parsing/Specification.java | 22 +- .../yahoo/bullet/querying/AbstractQuery.java | 8 +- .../bullet/querying/AggregationQuery.java | 9 +- .../yahoo/bullet/querying/FilterQuery.java | 9 +- .../com/yahoo/bullet/result/Metadata.java | 39 +-- .../bullet/parsing/FilterClauseTest.java | 2 +- .../bullet/parsing/LogicalClauseTest.java | 2 +- .../yahoo/bullet/parsing/ProjectionTest.java | 2 +- 23 files changed, 518 insertions(+), 171 deletions(-) create mode 100644 src/main/java/com/yahoo/bullet/Validator.java diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index 28497874..587eddff 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -5,12 +5,22 @@ */ package com.yahoo.bullet; +import com.yahoo.bullet.result.Metadata; import lombok.extern.slf4j.Slf4j; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + @Slf4j public class BulletConfig extends Config { + // Field names public static final String SPECIFICATION_DEFAULT_DURATION = "bullet.query.default.duration"; public static final String SPECIFICATION_MAX_DURATION = "bullet.query.max.duration"; + public static final String RECORD_INJECT_TIMESTAMP = "bullet.record.inject.timestamp.enable"; + public static final String RECORD_INJECT_TIMESTAMP_KEY = "bullet.record.inject.timestamp.key"; + public static final String AGGREGATION_DEFAULT_SIZE = "bullet.query.aggregation.default.size"; public static final String AGGREGATION_MAX_SIZE = "bullet.query.aggregation.max.size"; public static final String AGGREGATION_COMPOSITE_FIELD_SEPARATOR = "bullet.query.aggregation.composite.field.separator"; @@ -34,19 +44,178 @@ public class BulletConfig extends Config { public static final String TOP_K_AGGREGATION_SKETCH_ENTRIES = "bullet.query.aggregation.top.k.sketch.entries"; public static final String TOP_K_AGGREGATION_SKETCH_ERROR_TYPE = "bullet.query.aggregation.top.k.sketch.error.type"; - public static final String RECORD_INJECT_TIMESTAMP = "bullet.record.inject.timestamp.enable"; - public static final String RECORD_INJECT_TIMESTAMP_KEY = "bullet.record.inject.timestamp.key"; - public static final String RESULT_METADATA_ENABLE = "bullet.result.metadata.enable"; public static final String RESULT_METADATA_METRICS = "bullet.result.metadata.metrics"; public static final String RESULT_METADATA_METRICS_CONCEPT_KEY = "name"; public static final String RESULT_METADATA_METRICS_NAME_KEY = "key"; - public static final String RESULT_METADATA_METRICS_MAPPING = "bullet.result.metadata.metrics.mapping"; - public static final String PUBSUB_CONTEXT_NAME = "bullet.pubsub.context.name"; public static final String PUBSUB_CLASS_NAME = "bullet.pubsub.class.name"; + // Defaults + public static final int DEFAULT_SPECIFICATION_DURATION = 30 * 1000; + public static final int DEFAULT_SPECIFICATION_MAX_DURATION = 120 * 1000; + public static final boolean DEFAULT_RECORD_INJECT_TIMESTAMP = false; + public static final String DEFAULT_RECORD_INJECT_TIMESTAMP_KEY = "bullet_receive_timestamp"; + + public static final int DEFAULT_AGGREGATION_SIZE = 1; + public static final int DEFAULT_AGGREGATION_MAX_SIZE = 512; + public static final String DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR = "|"; + + public static final int DEFAULT_RAW_AGGREGATION_MAX_SIZE = 30; + public static final int DEFAULT_RAW_AGGREGATION_MICRO_BATCH_SIZE = 1; + + public static final int DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES = 16384; + public static final float DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING = 1.0f; + public static final String DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY = "Alpha"; + public static final int DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR = 8; + + public static final int DEFAULT_GROUP_AGGREGATION_SKETCH_ENTRIES = 512; + public static final float DEFAULT_GROUP_AGGREGATION_SKETCH_SAMPLING = 1.0f; + public static final int DEFAULT_GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR = 8; + + public static final int DEFAULT_DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES = 1024; + public static final int DEFAULT_DISTRIBUTION_AGGREGATION_MAX_POINTS = 100; + public static final int DEFAULT_DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING = 6; + + public static final int DEFAULT_TOP_K_AGGREGATION_SKETCH_ENTRIES = 1024; + public static final String DEFAULT_TOP_K_AGGREGATION_SKETCH_ERROR_TYPE = "NFN"; + + public static final boolean DEFAULT_RESULT_METADATA_ENABLE = true; + // This is a Map for simplicity. The YAML is a list. We ensure that the names are unique. The + // final Metadata in the result is a map so if the names are not unique, they get overridden. + public static final Map DEFAULT_RESULT_METADATA_METRICS = new HashMap<>(); + static { + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_ID.getName(), "query_id"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_BODY.getName(), "query"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_CREATION_TIME.getName(), "query_receive_time"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_TERMINATION_TIME.getName(), "query_finish_time"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.SKETCH_METADATA.getName(), "sketches"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ESTIMATED_RESULT.getName(), "was_estimated"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.STANDARD_DEVIATIONS.getName(), "standard_deviations"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.FAMILY.getName(), "family"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.SIZE.getName(), "size"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.THETA.getName(), "theta"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.UNIQUES_ESTIMATE.getName(), "uniques_estimate"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MINIMUM_VALUE.getName(), "minimum_value"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MAXIMUM_VALUE.getName(), "maximum_value"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ITEMS_SEEN.getName(), "items_seen"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.NORMALIZED_RANK_ERROR.getName(), "normalized_rank_error"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MAXIMUM_COUNT_ERROR.getName(), "maximum_count_error"); + DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ACTIVE_ITEMS.getName(), "active_items"); + } + + private static final Validator VALIDATOR = new Validator(); + static { + VALIDATOR.define(SPECIFICATION_DEFAULT_DURATION) + .defaultTo(DEFAULT_SPECIFICATION_DURATION) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(SPECIFICATION_MAX_DURATION) + .defaultTo(DEFAULT_SPECIFICATION_MAX_DURATION) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(RECORD_INJECT_TIMESTAMP) + .defaultTo(DEFAULT_RECORD_INJECT_TIMESTAMP) + .checkIf(Validator::isBoolean); + VALIDATOR.define(RECORD_INJECT_TIMESTAMP_KEY) + .defaultTo(DEFAULT_RECORD_INJECT_TIMESTAMP_KEY) + .checkIf(Validator::isNotNull) + .castTo(Validator::asInt); + + VALIDATOR.define(AGGREGATION_DEFAULT_SIZE) + .defaultTo(DEFAULT_AGGREGATION_SIZE) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(AGGREGATION_MAX_SIZE) + .defaultTo(DEFAULT_AGGREGATION_MAX_SIZE) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(AGGREGATION_COMPOSITE_FIELD_SEPARATOR) + .defaultTo(DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR) + .checkIf(Validator::isString); + + VALIDATOR.define(RAW_AGGREGATION_MAX_SIZE) + .defaultTo(DEFAULT_RAW_AGGREGATION_MAX_SIZE) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(RAW_AGGREGATION_MICRO_BATCH_SIZE) + .defaultTo(DEFAULT_RAW_AGGREGATION_MICRO_BATCH_SIZE) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + + VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES) + .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING) + .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING) + .checkIf(Validator::isPositive) + .checkIf(Validator::isFloat) + .castTo(Validator::asFloat); + VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY) + .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY) + .checkIf(Validator::isString); + VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR) + .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + + VALIDATOR.define(GROUP_AGGREGATION_SKETCH_ENTRIES) + .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_ENTRIES) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(GROUP_AGGREGATION_SKETCH_SAMPLING) + .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_SAMPLING) + .checkIf(Validator::isPositive) + .checkIf(Validator::isFloat) + .castTo(Validator::asFloat); + VALIDATOR.define(GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR) + .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + + VALIDATOR.define(DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES) + .defaultTo(DEFAULT_DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(DISTRIBUTION_AGGREGATION_MAX_POINTS) + .defaultTo(DEFAULT_DISTRIBUTION_AGGREGATION_MAX_POINTS) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + VALIDATOR.define(DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING) + .defaultTo(DEFAULT_DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING) + .checkIf(Validator::isPositiveInt) + .castTo(Validator::asInt); + + VALIDATOR.define(TOP_K_AGGREGATION_SKETCH_ENTRIES) + .defaultTo(DEFAULT_TOP_K_AGGREGATION_SKETCH_ENTRIES) + .checkIf(Validator::isPowerOfTwo) + .castTo(Validator::asInt); + VALIDATOR.define(TOP_K_AGGREGATION_SKETCH_ERROR_TYPE) + .defaultTo(DEFAULT_TOP_K_AGGREGATION_SKETCH_ERROR_TYPE) + .checkIf(Validator::isString); + + VALIDATOR.define(RESULT_METADATA_ENABLE) + .defaultTo(DEFAULT_RESULT_METADATA_ENABLE) + .checkIf(Validator::isBoolean); + VALIDATOR.define(RESULT_METADATA_METRICS) + .defaultTo(DEFAULT_RESULT_METADATA_METRICS) + .checkIf(Validator::isList) + .castTo(BulletConfig::mapifyMetadata); + + VALIDATOR.relatesTo("Max should be less or equal to default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) + .checkIf(Validator::isGreaterOrEqual); + VALIDATOR.relatesTo("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) + .checkIf(Validator::isGreaterOrEqual); + VALIDATOR.relatesTo("Metadata is enabled and keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + .checkIf(BulletConfig::isMetadataNotConfigured); + VALIDATOR.relatesTo("Metadata is disabled and keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + .checkIf(BulletConfig::isMetadataUnnecessary) + .orElseUse(false, Collections.emptyMap()); + } + + // Members public static final String DEFAULT_CONFIGURATION_NAME = "bullet_defaults.yaml"; /** @@ -64,4 +233,48 @@ public BulletConfig(String file) { public BulletConfig() { super(DEFAULT_CONFIGURATION_NAME); } + + /** + * Validates and fixes configuration for this config. If there are undefaulted or wrongly typed elements, you + * should use a {@link Validator} to define the appropriate definitions, casters and defaults to use. You + * should call this method before you use the config to ensure that all configurations are valid. + */ + public void validate() { + validate(this); + } + + /** + * Validates and fixes configuration for a given {@link BulletConfig}. This method checks, defaults and fixes the + * various settings defined in this class. + * + * @param config The {@link BulletConfig} to normalize. + */ + public static void validate(BulletConfig config) { + VALIDATOR.normalize(config); + } + + @SuppressWarnings("unchecked") + private static Object mapifyMetadata(Object metadata) { + List keys = (List) metadata; + Map mapping = new HashMap<>(); + // For each metric configured, load the name of the field to add it to the metadata as. + for (Map m : keys) { + String concept = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY); + String name = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY); + if (Metadata.KNOWN_CONCEPTS.contains(Metadata.Concept.from(concept))) { + mapping.put(concept, name); + } + } + return mapping; + } + + @SuppressWarnings("unchecked") + private static boolean isMetadataNotConfigured(Object enabled, Object keys) { + return (Boolean) enabled && keys == null; + } + + private static boolean isMetadataUnnecessary(Object enabled, Object keys) { + return !((Boolean) enabled) && keys != null; + } } + diff --git a/src/main/java/com/yahoo/bullet/Config.java b/src/main/java/com/yahoo/bullet/Config.java index c425732a..96d081da 100644 --- a/src/main/java/com/yahoo/bullet/Config.java +++ b/src/main/java/com/yahoo/bullet/Config.java @@ -5,6 +5,8 @@ */ package com.yahoo.bullet; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import lombok.extern.slf4j.Slf4j; import org.jvyaml.YAML; @@ -24,7 +26,9 @@ @Slf4j public class Config implements Serializable { private Map data; + public static final String DELIMITER = "."; + private static Gson GSON = new GsonBuilder().serializeNulls().setPrettyPrinting().disableHtmlEscaping().create(); /** * Constructor that loads a specific file and loads the settings in that file. @@ -33,7 +37,6 @@ public class Config implements Serializable { */ public Config(String file) { data = readYAML(file); - log.info("Final Configuration:\n{} ", data); } /** @@ -47,7 +50,6 @@ public Config(String file, String defaultConfigurationFile) { // Override Map specificConf = readYAML(file); data.putAll(specificConf); - log.info("Final Configuration with defaults:\n{} ", data); } /** @@ -207,4 +209,9 @@ protected Map readYAML(String yamlFile) { return new HashMap<>(); } } + + @Override + public String toString() { + return GSON.toJson(data); + } } diff --git a/src/main/java/com/yahoo/bullet/Utilities.java b/src/main/java/com/yahoo/bullet/Utilities.java index e4771221..11ead899 100644 --- a/src/main/java/com/yahoo/bullet/Utilities.java +++ b/src/main/java/com/yahoo/bullet/Utilities.java @@ -10,9 +10,27 @@ import java.util.Map; public class Utilities { + /** + * Tries to get the object casted as the target type. If it is generic, the captured types cannot not be + * validated. Only the base object type is validated. + * + * @param entry The object to cast. + * @param clazz The class of the U. + * @param The type to get the object as. + * @return The casted object of type U or null if the cast could not be done. + */ + @SuppressWarnings("unchecked") + public static U getCasted(Object entry, Class clazz) { + try { + return clazz.cast(entry); + } catch (ClassCastException ignored) { + } + return null; + } + /** - * Tries to get a key from a map as the target type. If it is a composite type, the internal types are not - * validated. Only the outermost object type is validated. + * Tries to get a key from a map as the target type. If it is a generic type, the captured types are not + * validated. Only the base object type is validated. * * @param map The non-null map that possibly contains the key. * @param key The String key to get the value for from the map. @@ -23,11 +41,7 @@ public class Utilities { */ @SuppressWarnings("unchecked") public static U getCasted(Map map, String key, Class clazz) { - try { - return clazz.cast(map.get(key)); - } catch (ClassCastException ignored) { - } - return null; + return getCasted(map.get(key), clazz); } /** diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java new file mode 100644 index 00000000..a14d921f --- /dev/null +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -0,0 +1,198 @@ +package com.yahoo.bullet; + +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.BiPredicate; +import java.util.function.Function; +import java.util.function.Predicate; + +@Slf4j +public class Validator { + public static final Predicate UNARY_IDENTITY = o -> true; + public static final BiPredicate BINARY_IDENTITY = (oA, oB) -> true; + + public class Entry { + private String key; + private Predicate validation; + @Getter + private Object defaultValue; + private Function adapter; + + public Entry(String key) { + this.validation = UNARY_IDENTITY; + this.key = key; + } + + public Entry checkIf(Predicate validator) { + this.validation.and(validator); + return this; + } + + public Entry defaultTo(Object defaultValue) { + this.defaultValue = defaultValue; + return this; + } + + public Entry castTo(Function adapter) { + this.adapter = adapter; + return this; + } + + public void normalize(BulletConfig config) { + Object value = config.get(key); + boolean isValid = validation.test(value); + if (!isValid) { + log.warn("Key: {} had an invalid value: {}. Using default: {}", key, value, defaultValue); + value = defaultValue; + } + if (adapter != null) { + value = adapter.apply(value); + log.info("Changed the type for {}: {}", key, value); + } + config.set(key, value); + } + } + + public class Relationship { + @Getter + private String keyA; + @Getter + private String keyB; + private String description; + private BiPredicate binaryRelation; + private Object defaultA; + private Object defaultB; + + public Relationship(String description, String keyA, String keyB) { + this.description = description; + this.keyA = keyA; + this.keyB = keyB; + // These are guaranteed to be present. + this.defaultA = entries.get(keyA).getDefaultValue(); + this.defaultB = entries.get(keyB).getDefaultValue(); + this.binaryRelation = BINARY_IDENTITY; + } + + public Relationship checkIf(BiPredicate binaryRelation) { + this.binaryRelation.and(binaryRelation); + return this; + } + + public void orElseUse(Object objectA, Object objectB) { + this.defaultA = objectA; + this.defaultB = objectB; + } + + public void normalize(BulletConfig config) { + Object objectA = config.get(keyA); + Object objectB = config.get(keyB); + boolean result = binaryRelation.test(objectA, objectB); + if (!result) { + log.warn("{}: {} and {}: {} do not satisfy: {}. Using their defaults", keyA, objectA, keyB, objectB, description); + log.warn("Using default {} for {}", defaultA, keyA); + log.warn("Using default {} for {}", defaultB, keyB); + config.set(keyA, defaultA); + config.set(keyB, defaultB); + } + } + } + + private Map entries = new HashMap<>(); + private List relations = new ArrayList<>(); + + public Entry define(String key) { + Entry entry = new Entry(key); + entries.put(key, entry); + return entry; + } + + public Relationship relatesTo(String description, String keyA, String keyB) { + Objects.requireNonNull(entries.get(keyA), "You cannot add a relationship for " + keyA + " before defining it"); + Objects.requireNonNull(entries.get(keyB), "You cannot add a relationship for " + keyB + " before defining it"); + + Relationship relation = new Relationship(description, keyA, keyB); + relations.add(relation); + return relation; + } + + public void normalize(BulletConfig config) { + entries.values().forEach(e -> e.normalize(config)); + relations.stream().forEach(r -> r.normalize(config)); + } + + // Type Adapters + + public static Object asInt(Object value) { + return ((Number) value).intValue(); + } + + public static Object asFloat(Object value) { + return ((Number) value).floatValue(); + } + + // Validators + + public static boolean isNotNull(Object value) { + return value != null; + } + + public static boolean isType(Object value, Class clazz) { + return isNotNull(value) && clazz.isInstance(value); + } + + public static boolean isBoolean(Object value) { + return isType(value, Boolean.class); + } + + public static boolean isString(Object value) { + return isType(value, String.class); + } + + public static boolean isList(Object value) { + return isType(value, List.class); + } + + public static boolean isMap(Object value) { + return isType(value, Map.class); + } + + public static boolean isNumber(Object value) { + return isType(value, Number.class); + } + + public static boolean isInt(Object value) { + return isType(value, Long.class) || isType(value, Integer.class); + } + + public static boolean isFloat(Object value) { + return isType(value, Double.class) || isType(value, Float.class); + } + + public static boolean isPositive(Object value) { + return isNumber(value) && ((Number) value).doubleValue() > 0; + } + + public static boolean isPositiveInt(Object value) { + return isPositive(value) && isInt(value); + } + + public static boolean isPowerOfTwo(Object value) { + if (!isPositiveInt(value)) { + return false; + } + int toCheck = ((Number) value).intValue(); + return (toCheck & toCheck - 1) == 0; + } + + // Binary Validators + + public static boolean isGreaterOrEqual(Object first, Object second) { + return ((Number) first).doubleValue() >= ((Number) second).doubleValue(); + } +} diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/CountDistinct.java b/src/main/java/com/yahoo/bullet/operations/aggregations/CountDistinct.java index 6a1d03e6..07721247 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/CountDistinct.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/CountDistinct.java @@ -44,12 +44,9 @@ public CountDistinct(Aggregation aggregation) { Map attributes = aggregation.getAttributes(); ResizeFactor resizeFactor = getResizeFactor(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR); - float samplingProbability = ((Number) config.getOrDefault(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING, - DEFAULT_SAMPLING_PROBABILITY)).floatValue(); - Family family = getFamily(config.getOrDefault(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY, - DEFAULT_UPDATE_SKETCH_FAMILY).toString()); - int nominalEntries = ((Number) config.getOrDefault(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES, - DEFAULT_NOMINAL_ENTRIES)).intValue(); + float samplingProbability = config.getAs(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING, Float.class); + Family family = getFamily(config.getAs(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY, String.class)); + int nominalEntries = config.getAs(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES, Integer.class); newName = attributes == null ? DEFAULT_NEW_NAME : attributes.getOrDefault(NEW_NAME_FIELD, DEFAULT_NEW_NAME).toString(); diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/Distribution.java b/src/main/java/com/yahoo/bullet/operations/aggregations/Distribution.java index 65ff061c..cb87bcec 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/Distribution.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/Distribution.java @@ -30,12 +30,6 @@ * configured for the sketch, the normalized rank error can be determined and tightly bound. */ public class Distribution extends SketchingStrategy { - public static final int DEFAULT_ENTRIES = 1024; - - public static final int DEFAULT_MAX_POINTS = 100; - public static final int DEFAULT_POINTS = 1; - public static final int DEFAULT_ROUNDING = 6; - // Distribution fields public static final String TYPE = "type"; public static final String POINTS = "points"; @@ -82,14 +76,11 @@ public class Distribution extends SketchingStrategy { @SuppressWarnings("unchecked") public Distribution(Aggregation aggregation) { super(aggregation); - entries = ((Number) config.getOrDefault(BulletConfig.DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES, - DEFAULT_ENTRIES)).intValue(); - rounding = ((Number) config.getOrDefault(BulletConfig.DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING, - DEFAULT_ROUNDING)).intValue(); - int pointLimit = ((Number) config.getOrDefault(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, - DEFAULT_MAX_POINTS)).intValue(); - // The max gets rid of negative sizes if accidentally configured. - maxPoints = Math.max(DEFAULT_POINTS, Math.min(pointLimit, aggregation.getSize())); + entries = config.getAs(BulletConfig.DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES, Integer.class); + rounding = config.getAs(BulletConfig.DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING, Integer.class); + + int pointLimit = config.getAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, Integer.class); + maxPoints = Math.min(pointLimit, aggregation.getSize()); this.aggregation = aggregation; // The sketch is initialized in initialize! diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/GroupBy.java b/src/main/java/com/yahoo/bullet/operations/aggregations/GroupBy.java index c29313ed..cd39969e 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/GroupBy.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/GroupBy.java @@ -34,9 +34,6 @@ public class GroupBy extends KMVStrategy { // This is reused for the duration of the strategy. private final CachingGroupData container; - // 13.27% error rate at 99.73% confidence (3 SD). Irrelevant since we are using this to cap the number of groups. - public static final int DEFAULT_NOMINAL_ENTRIES = 512; - private final Set operations; /** @@ -54,10 +51,11 @@ public GroupBy(Aggregation aggregation) { container = new CachingGroupData(null, metrics); ResizeFactor resizeFactor = getResizeFactor(BulletConfig.GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR); - float samplingProbability = ((Number) config.getOrDefault(BulletConfig.GROUP_AGGREGATION_SKETCH_SAMPLING, - DEFAULT_SAMPLING_PROBABILITY)).floatValue(); - int nominalEntries = ((Number) config.getOrDefault(BulletConfig.GROUP_AGGREGATION_SKETCH_ENTRIES, - DEFAULT_NOMINAL_ENTRIES)).intValue(); + float samplingProbability = config.getAs(BulletConfig.GROUP_AGGREGATION_SKETCH_SAMPLING, Float.class); + + // Default at 512 gives a 13.27% error rate at 99.73% confidence (3 SD). Irrelevant since we are using this to + // mostly cap the number of groups. You can use the Sketch theta to extrapolate the aggregation for all the data. + int nominalEntries = config.getAs(BulletConfig.GROUP_AGGREGATION_SKETCH_ENTRIES, Integer.class); int size = aggregation.getSize(); sketch = new TupleSketch(resizeFactor, samplingProbability, nominalEntries, size); diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/KMVStrategy.java b/src/main/java/com/yahoo/bullet/operations/aggregations/KMVStrategy.java index 51977be9..9998cb65 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/KMVStrategy.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/KMVStrategy.java @@ -13,12 +13,6 @@ * The parent class for {@link SketchingStrategy} that use the KMV type of Sketch - Theta and Tuple. */ public abstract class KMVStrategy extends SketchingStrategy { - // Common defaults for KMV type sketches - // No Sampling - public static final float DEFAULT_SAMPLING_PROBABILITY = 1.0f; - // Sketch * 8 its size upto 2 * nominal entries everytime it reaches cap - public static final int DEFAULT_RESIZE_FACTOR = ResizeFactor.X8.lg(); - /** * Constructor that requires an {@link Aggregation}. * @@ -37,7 +31,7 @@ public KMVStrategy(Aggregation aggregation) { */ @SuppressWarnings("unchecked") public ResizeFactor getResizeFactor(String key) { - return getResizeFactor((Number) config.getOrDefault(key, DEFAULT_RESIZE_FACTOR)); + return getResizeFactor(config.getAs(key, Integer.class)); } /** * Converts a integer representing the resizing for Sketches into a {@link ResizeFactor}. @@ -45,9 +39,8 @@ public ResizeFactor getResizeFactor(String key) { * @param factor An int representing the scaling when the Sketch reaches its threshold. Supports 1, 2, 4 and 8. * @return A {@link ResizeFactor} represented by the integer or {@link ResizeFactor#X8} otherwise. */ - public static ResizeFactor getResizeFactor(Number factor) { - int resizeFactor = factor.intValue(); - switch (resizeFactor) { + public static ResizeFactor getResizeFactor(int factor) { + switch (factor) { case 1: return ResizeFactor.X1; case 2: diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/Raw.java b/src/main/java/com/yahoo/bullet/operations/aggregations/Raw.java index 889e9249..84297453 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/Raw.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/Raw.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Map; /** * Implements the LIMIT operation on multiple raw {@link BulletRecord}. @@ -30,14 +29,12 @@ */ @Slf4j public class Raw implements Strategy { - public static final Integer DEFAULT_MAX_SIZE = 30; - public static final Integer DEFAULT_MICRO_BATCH_SIZE = 1; private ArrayList aggregate = new ArrayList<>(); private Integer size; private int consumed = 0; private int combined = 0; - private int microBatchSize = DEFAULT_MICRO_BATCH_SIZE; + private int microBatchSize; /** * Constructor that takes in an {@link Aggregation}. The size of the aggregation is used as a LIMIT @@ -47,13 +44,11 @@ public class Raw implements Strategy { */ @SuppressWarnings("unchecked") public Raw(Aggregation aggregation) { - Map config = aggregation.getConfiguration(); - int maximumSize = ((Number) config.getOrDefault(BulletConfig.RAW_AGGREGATION_MAX_SIZE, - DEFAULT_MAX_SIZE)).intValue(); + BulletConfig config = aggregation.getConfiguration(); + int maximumSize = config.getAs(BulletConfig.RAW_AGGREGATION_MAX_SIZE, Integer.class); size = Math.min(aggregation.getSize(), maximumSize); - microBatchSize = ((Number) aggregation.getConfiguration().getOrDefault(BulletConfig.RAW_AGGREGATION_MICRO_BATCH_SIZE, - DEFAULT_MICRO_BATCH_SIZE)).intValue(); + microBatchSize = config.getAs(BulletConfig.RAW_AGGREGATION_MICRO_BATCH_SIZE, Integer.class); } @Override diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/SketchingStrategy.java b/src/main/java/com/yahoo/bullet/operations/aggregations/SketchingStrategy.java index 366b7d7a..b3361973 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/SketchingStrategy.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/SketchingStrategy.java @@ -32,8 +32,8 @@ public abstract class SketchingStrategy implements Strategy { // The metadata concept to key mapping protected final Map metadataKeys; - // A copy of the configuration - protected final Map config; + // A copy of the configuration + protected final BulletConfig config; // Separator for multiple fields when inserting into the Sketch protected final String separator; @@ -53,10 +53,10 @@ public abstract class SketchingStrategy implements Strategy { @SuppressWarnings("unchecked") public SketchingStrategy(Aggregation aggregation) { config = aggregation.getConfiguration(); - metadataKeys = (Map) config.getOrDefault(BulletConfig.RESULT_METADATA_METRICS_MAPPING, - Collections.emptyMap()); - separator = config.getOrDefault(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, - Aggregation.DEFAULT_FIELD_SEPARATOR).toString(); + boolean shouldMeta = config.getAs(BulletConfig.RESULT_METADATA_ENABLE, Boolean.class); + metadataKeys = shouldMeta ? (Map) config.getAs(BulletConfig.RESULT_METADATA_METRICS, Map.class) : + Collections.emptyMap(); + separator = config.getAs(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, String.class); fieldsToNames = aggregation.getFields(); fields = Utilities.isEmpty(fieldsToNames) ? Collections.emptyList() : new ArrayList<>(fieldsToNames.keySet()); diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/TopK.java b/src/main/java/com/yahoo/bullet/operations/aggregations/TopK.java index a777fb41..1bf189a5 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/TopK.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/TopK.java @@ -23,11 +23,8 @@ public class TopK extends SketchingStrategy { public static final String NEW_NAME_FIELD = "newName"; public static final String DEFAULT_NEW_NAME = "COUNT"; - public static final int DEFAULT_MAX_MAP_ENTRIES = 1024; - public static final String NO_FALSE_NEGATIVES = "NFN"; public static final String NO_FALSE_POSITIVES = "NFP"; - public static final String DEFAULT_ERROR_TYPE = NO_FALSE_NEGATIVES; public static final String THRESHOLD_FIELD = "threshold"; @@ -42,8 +39,7 @@ public class TopK extends SketchingStrategy { public TopK(Aggregation aggregation) { super(aggregation); - String errorConfiguration = (config.getOrDefault(BulletConfig.TOP_K_AGGREGATION_SKETCH_ERROR_TYPE, - DEFAULT_ERROR_TYPE)).toString(); + String errorConfiguration = config.getAs(BulletConfig.TOP_K_AGGREGATION_SKETCH_ERROR_TYPE, String.class); ErrorType errorType = getErrorType(errorConfiguration); @@ -52,7 +48,7 @@ public TopK(Aggregation aggregation) { newName = attributes == null ? DEFAULT_NEW_NAME : attributes.getOrDefault(NEW_NAME_FIELD, DEFAULT_NEW_NAME).toString(); - int maxMapSize = getMaxMapEntries(config); + int maxMapSize = config.getAs(BulletConfig.TOP_K_AGGREGATION_SKETCH_ENTRIES, Integer.class); Number threshold = getThreshold(attributes); int size = aggregation.getSize(); sketch = threshold != null ? new FrequentItemsSketch(errorType, maxMapSize, threshold.longValue(), size) : @@ -90,17 +86,6 @@ private void splitFields(BulletRecord record) { record.rename(FrequentItemsSketch.COUNT_FIELD, newName); } - @SuppressWarnings("unchecked") - private static int getMaxMapEntries(Map config) { - int entries = ((Number) config.getOrDefault(BulletConfig.TOP_K_AGGREGATION_SKETCH_ENTRIES, - DEFAULT_MAX_MAP_ENTRIES)).intValue(); - // If non-positive or not a power of 2 - if (entries <= 0 || (entries & entries - 1) != 0) { - return DEFAULT_MAX_MAP_ENTRIES; - } - return entries; - } - private static Number getThreshold(Map attributes) { if (Utilities.isEmpty(attributes)) { return null; diff --git a/src/main/java/com/yahoo/bullet/operations/aggregations/grouping/GroupOperation.java b/src/main/java/com/yahoo/bullet/operations/aggregations/grouping/GroupOperation.java index 55aeff03..c54a4139 100644 --- a/src/main/java/com/yahoo/bullet/operations/aggregations/grouping/GroupOperation.java +++ b/src/main/java/com/yahoo/bullet/operations/aggregations/grouping/GroupOperation.java @@ -91,7 +91,7 @@ public static boolean hasOperations(Map attributes) { /** * Validates whether the provided {@link Collection} of {@link GroupOperation} is valid. * - * @param operations The non-null operations to validate. + * @param operations The non-null operations to normalize. * @return An {@link List} of {@link Error} if any operations were invalid or null if valid. */ public static List checkOperations(Collection operations) { diff --git a/src/main/java/com/yahoo/bullet/parsing/Aggregation.java b/src/main/java/com/yahoo/bullet/parsing/Aggregation.java index 452b6453..a47b1961 100644 --- a/src/main/java/com/yahoo/bullet/parsing/Aggregation.java +++ b/src/main/java/com/yahoo/bullet/parsing/Aggregation.java @@ -45,7 +45,7 @@ public class Aggregation implements Configurable, Validatable { private Strategy strategy; // In case any strategies need it. - private Map configuration; + private BulletConfig configuration; public static final Set SUPPORTED_AGGREGATION_TYPES = new HashSet<>(asList(AggregationType.GROUP, AggregationType.COUNT_DISTINCT, AggregationType.RAW, @@ -54,12 +54,6 @@ public class Aggregation implements Configurable, Validatable { makeError("Unknown aggregation type", "Current supported aggregation types are: RAW (or LIMIT), " + "GROUP (or DISTINCT), COUNT DISTINCT, DISTRIBUTION, TOP K"); - public static final Integer DEFAULT_SIZE = 1; - public static final Integer DEFAULT_MAX_SIZE = 512; - - public static final String DEFAULT_FIELD_SEPARATOR = "|"; - - /** * Default constructor. GSON recommended */ @@ -69,13 +63,11 @@ public Aggregation() { @Override @SuppressWarnings("unchecked") - public void configure(Map configuration) { - this.configuration = configuration; + public void configure(BulletConfig config) { + this.configuration = config; - Number defaultSize = (Number) configuration.getOrDefault(BulletConfig.AGGREGATION_DEFAULT_SIZE, DEFAULT_SIZE); - Number maximumSize = (Number) configuration.getOrDefault(BulletConfig.AGGREGATION_MAX_SIZE, DEFAULT_MAX_SIZE); - int sizeDefault = defaultSize.intValue(); - int sizeMaximum = maximumSize.intValue(); + int sizeDefault = config.getAs(BulletConfig.AGGREGATION_DEFAULT_SIZE, Integer.class); + int sizeMaximum = config.getAs(BulletConfig.AGGREGATION_MAX_SIZE, Integer.class); // Null or negative, then default, else min of size and max size = (size == null || size < 0) ? sizeDefault : Math.min(size, sizeMaximum); @@ -120,5 +112,4 @@ Strategy findStrategy() { public String toString() { return "{size: " + size + ", type: " + type + ", fields: " + fields + ", attributes: " + attributes + "}"; } - } diff --git a/src/main/java/com/yahoo/bullet/parsing/Configurable.java b/src/main/java/com/yahoo/bullet/parsing/Configurable.java index 032ddb34..1729b77b 100644 --- a/src/main/java/com/yahoo/bullet/parsing/Configurable.java +++ b/src/main/java/com/yahoo/bullet/parsing/Configurable.java @@ -5,15 +5,15 @@ */ package com.yahoo.bullet.parsing; -import java.util.Map; +import com.yahoo.bullet.BulletConfig; public interface Configurable { /** - * Takes a map containing configuration and applies it to itself. + * Takes a {@link BulletConfig} containing configuration and applies it to itself. * - * @param configuration A Map of configuration key values. + * @param configuration The configuration containing the settings. */ - default void configure(Map configuration) { + default void configure(BulletConfig configuration) { // Do nothing } } diff --git a/src/main/java/com/yahoo/bullet/parsing/Parser.java b/src/main/java/com/yahoo/bullet/parsing/Parser.java index abe8779f..5a1d1d85 100644 --- a/src/main/java/com/yahoo/bullet/parsing/Parser.java +++ b/src/main/java/com/yahoo/bullet/parsing/Parser.java @@ -7,10 +7,9 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.FilterOperations.FilterType; -import java.util.Map; - public class Parser { private static final FieldTypeAdapterFactory CLAUSE_FACTORY = FieldTypeAdapterFactory.of(Clause.class, t -> t.getAsJsonObject().get(Clause.OPERATION_FIELD).getAsString()) @@ -25,15 +24,14 @@ public class Parser { * Parses a Specification out of the query string. * * @param queryString The String version of the query. - * @param configuration Additional configuration for the specification. + * @param config Additional configuration for the specification. * * @return The parsed, configured Specification. */ - public static Specification parse(String queryString, Map configuration) { + public static Specification parse(String queryString, BulletConfig config) { Specification specification = GSON.fromJson(queryString, Specification.class); - specification.configure(configuration); + specification.configure(config); return specification; } - } diff --git a/src/main/java/com/yahoo/bullet/parsing/Specification.java b/src/main/java/com/yahoo/bullet/parsing/Specification.java index dc7136d3..7ad73047 100644 --- a/src/main/java/com/yahoo/bullet/parsing/Specification.java +++ b/src/main/java/com/yahoo/bullet/parsing/Specification.java @@ -19,7 +19,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Optional; /** @@ -39,9 +38,6 @@ public class Specification implements Configurable, Validatable { private Boolean shouldInjectTimestamp; private String timestampKey; - public static final String DEFAULT_RECEIVE_TIMESTAMP_KEY = "bullet_receive_timestamp"; - public static final Integer DEFAULT_DURATION_MS = 30 * 1000; - public static final Integer DEFAULT_MAX_DURATION_MS = 120 * 1000; public static final String SUB_KEY_SEPERATOR = "\\."; public static final String AGGREGATION_FAILURE_RESOLUTION = "Please try again later"; @@ -159,26 +155,24 @@ public boolean isMicroBatch() { @Override @SuppressWarnings("unchecked") - public void configure(Map configuration) { + public void configure(BulletConfig config) { if (filters != null) { - filters.forEach(f -> f.configure(configuration)); + filters.forEach(f -> f.configure(config)); } if (projection != null) { - projection.configure(configuration); + projection.configure(config); } // Must have an aggregation if (aggregation == null) { aggregation = new Aggregation(); } - aggregation.configure(configuration); + aggregation.configure(config); - shouldInjectTimestamp = (Boolean) configuration.getOrDefault(BulletConfig.RECORD_INJECT_TIMESTAMP, false); - timestampKey = (String) configuration.getOrDefault(BulletConfig.RECORD_INJECT_TIMESTAMP_KEY, DEFAULT_RECEIVE_TIMESTAMP_KEY); + shouldInjectTimestamp = config.getAs(BulletConfig.RECORD_INJECT_TIMESTAMP, Boolean.class); + timestampKey = config.getAs(BulletConfig.RECORD_INJECT_TIMESTAMP_KEY, String.class); - Number defaultDuration = (Number) configuration.getOrDefault(BulletConfig.SPECIFICATION_DEFAULT_DURATION, DEFAULT_DURATION_MS); - Number maxDuration = (Number) configuration.getOrDefault(BulletConfig.SPECIFICATION_MAX_DURATION, DEFAULT_MAX_DURATION_MS); - int durationDefault = defaultDuration.intValue(); - int durationMax = maxDuration.intValue(); + int durationDefault = config.getAs(BulletConfig.SPECIFICATION_DEFAULT_DURATION, Integer.class); + int durationMax = config.getAs(BulletConfig.SPECIFICATION_MAX_DURATION, Integer.class); // Null or negative, then default, else min of duration and max. duration = (duration == null || duration < 0) ? durationDefault : Math.min(duration, durationMax); diff --git a/src/main/java/com/yahoo/bullet/querying/AbstractQuery.java b/src/main/java/com/yahoo/bullet/querying/AbstractQuery.java index d2d790b3..04f768f1 100644 --- a/src/main/java/com/yahoo/bullet/querying/AbstractQuery.java +++ b/src/main/java/com/yahoo/bullet/querying/AbstractQuery.java @@ -6,6 +6,7 @@ package com.yahoo.bullet.querying; import com.google.gson.JsonParseException; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.parsing.Error; import com.yahoo.bullet.parsing.Parser; import com.yahoo.bullet.parsing.ParsingException; @@ -13,7 +14,6 @@ import lombok.Getter; import java.util.List; -import java.util.Map; import java.util.Optional; public abstract class AbstractQuery implements Query { @@ -27,12 +27,12 @@ public abstract class AbstractQuery implements Query { * Constructor that takes a String representation of the query and a configuration to use. * * @param queryString The query as a string. - * @param configuration The configuration to use. + * @param config The validated {@link BulletConfig} configuration to use. * @throws ParsingException if there was an issue. */ - public AbstractQuery(String queryString, Map configuration) throws JsonParseException, ParsingException { + public AbstractQuery(String queryString, BulletConfig config) throws JsonParseException, ParsingException { this.queryString = queryString; - specification = Parser.parse(queryString, configuration); + specification = Parser.parse(queryString, config); Optional> errors = specification.validate(); if (errors.isPresent()) { throw new ParsingException(errors.get()); diff --git a/src/main/java/com/yahoo/bullet/querying/AggregationQuery.java b/src/main/java/com/yahoo/bullet/querying/AggregationQuery.java index cf2a0230..01ffc97c 100644 --- a/src/main/java/com/yahoo/bullet/querying/AggregationQuery.java +++ b/src/main/java/com/yahoo/bullet/querying/AggregationQuery.java @@ -5,12 +5,11 @@ */ package com.yahoo.bullet.querying; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.parsing.ParsingException; import com.yahoo.bullet.result.Clip; import lombok.Getter; -import java.util.Map; - public class AggregationQuery extends AbstractQuery { @Getter protected long lastAggregationTime = 0L; @@ -19,11 +18,11 @@ public class AggregationQuery extends AbstractQuery { * Constructor that takes a String representation of the query. * * @param queryString The query as a string. - * @param configuration A map of configurations to use. + * @param config A {@link BulletConfig} configuration that has been validated. * @throws ParsingException if there was an issue. */ - public AggregationQuery(String queryString, Map configuration) throws ParsingException { - super(queryString, configuration); + public AggregationQuery(String queryString, BulletConfig config) throws ParsingException { + super(queryString, config); } @Override diff --git a/src/main/java/com/yahoo/bullet/querying/FilterQuery.java b/src/main/java/com/yahoo/bullet/querying/FilterQuery.java index 2bca92e7..440d7c79 100644 --- a/src/main/java/com/yahoo/bullet/querying/FilterQuery.java +++ b/src/main/java/com/yahoo/bullet/querying/FilterQuery.java @@ -5,21 +5,20 @@ */ package com.yahoo.bullet.querying; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.parsing.ParsingException; import com.yahoo.bullet.record.BulletRecord; -import java.util.Map; - public class FilterQuery extends AbstractQuery { /** * Default constructor. * * @param input The query as a String. - * @param configuration A map of configurations to use. + * @param config A {@link BulletConfig} configuration that has been validated. * @throws ParsingException if there was an issue. */ - public FilterQuery(String input, Map configuration) throws ParsingException { - super(input, configuration); + public FilterQuery(String input, BulletConfig config) throws ParsingException { + super(input, config); } /** diff --git a/src/main/java/com/yahoo/bullet/result/Metadata.java b/src/main/java/com/yahoo/bullet/result/Metadata.java index 37aac48e..1107d6ac 100644 --- a/src/main/java/com/yahoo/bullet/result/Metadata.java +++ b/src/main/java/com/yahoo/bullet/result/Metadata.java @@ -5,17 +5,14 @@ */ package com.yahoo.bullet.result; -import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.parsing.Error; import lombok.Getter; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import static java.util.Arrays.asList; @@ -77,6 +74,7 @@ public static Concept from(String concept) { /** * Returns a backing view of the meta information as a Map. + * * @return A Map of keys to objects that denote the meta information. */ public Map asMap() { @@ -85,7 +83,8 @@ public Map asMap() { /** * Add a piece of meta information. - * @param key The name of the meta tag + * + * @param key The name of the meta tag * @param information An object that represents the information. * @return This object for chaining. */ @@ -96,6 +95,7 @@ public Metadata add(String key, Object information) { /** * Add an error to the Metadata. + * * @param errors Error objects to add. * @return This object for chaining. */ @@ -113,6 +113,7 @@ public Metadata addErrors(List errors) { /** * Static construction of Metadata with some errors. + * * @param errors A non-null list of Error objects. * @return The Metadata object with the errors. */ @@ -124,6 +125,7 @@ public static Metadata of(Error... errors) { /** * Static construction of Metadata with some errors. + * * @param errors A non-null list of Error objects. * @return The Metadata object with the errors. */ @@ -135,6 +137,7 @@ public static Metadata of(List errors) { /** * Merge another Metadata into this Metadata. + * * @param metadata A Metadata to merge. * @return This Object after the merge. */ @@ -144,32 +147,4 @@ public Metadata merge(Metadata metadata) { } return this; } - - /** - * For each {@link Concept} in a given {@link Set}, return a mapping of the {@link Concept} to its name. - * - * @param configuration The configuration that contains the metadata configuration. - * @param concepts A {@link Set} of {@link Concept} to get the mappings for. - * - * @return A mapping of the names or an empty mapping if metadata is not enabled or none of the concepts were found. - */ - @SuppressWarnings("unchecked") - public static Map getConceptNames(Map configuration, Set concepts) { - boolean isMetadataEnabled = (Boolean) configuration.getOrDefault(BulletConfig.RESULT_METADATA_ENABLE, false); - if (!isMetadataEnabled) { - return Collections.emptyMap(); - } - List keys = (List) configuration.getOrDefault(BulletConfig.RESULT_METADATA_METRICS, - Collections.emptyList()); - Map mapping = new HashMap<>(); - // For each metric configured, load the name of the field to add it to the metadata as. - for (Map m : keys) { - String concept = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY); - String name = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY); - if (concepts.contains(Concept.from(concept))) { - mapping.put(concept, name); - } - } - return mapping; - } } diff --git a/src/test/java/com/yahoo/bullet/parsing/FilterClauseTest.java b/src/test/java/com/yahoo/bullet/parsing/FilterClauseTest.java index 2ad24807..194f6b0d 100644 --- a/src/test/java/com/yahoo/bullet/parsing/FilterClauseTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/FilterClauseTest.java @@ -218,7 +218,7 @@ public void testToString() { public void testValidate() { FilterClause filterClause = new FilterClause(); Optional> errors = filterClause.validate(); - // currently FilterClause.validate() does nothing + // currently FilterClause.normalize() does nothing Assert.assertFalse(errors.isPresent()); } diff --git a/src/test/java/com/yahoo/bullet/parsing/LogicalClauseTest.java b/src/test/java/com/yahoo/bullet/parsing/LogicalClauseTest.java index a66e290d..99d383db 100644 --- a/src/test/java/com/yahoo/bullet/parsing/LogicalClauseTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/LogicalClauseTest.java @@ -239,7 +239,7 @@ public void testToString() { public void testValidate() { LogicalClause logicalClause = new LogicalClause(); Optional> errors = logicalClause.validate(); - // currently LogicalClause.validate() does nothing + // currently LogicalClause.normalize() does nothing Assert.assertFalse(errors.isPresent()); } } diff --git a/src/test/java/com/yahoo/bullet/parsing/ProjectionTest.java b/src/test/java/com/yahoo/bullet/parsing/ProjectionTest.java index caf8069a..afe6e43f 100644 --- a/src/test/java/com/yahoo/bullet/parsing/ProjectionTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/ProjectionTest.java @@ -118,7 +118,7 @@ public void testNullFieldName() { public void testValidate() { Projection projection = new Projection(); Optional> errors = projection.validate(); - // currently Projection.validate() does nothing + // currently Projection.normalize() does nothing Assert.assertFalse(errors.isPresent()); } From 9dc337918907a513cf2a1812e1fb5f6d9056f984 Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Mon, 20 Nov 2017 13:32:52 -0800 Subject: [PATCH 2/6] Docs --- pom.xml | 4 +- .../java/com/yahoo/bullet/BulletConfig.java | 15 +- src/main/java/com/yahoo/bullet/Validator.java | 212 +++++++++++++++++- 3 files changed, 215 insertions(+), 16 deletions(-) diff --git a/pom.xml b/pom.xml index e9c4dac9..6da8ae80 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ com.yahoo.bullet bullet-core - 0.2.6-SNAPSHOT + 0.3.0-SNAPSHOT jar bullet-core @@ -193,7 +193,7 @@ maven-javadoc-plugin - 2.9.1 + 2.10.4 ${project.basedir}/src/main/java diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index 587eddff..d69007bf 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -105,6 +105,8 @@ public class BulletConfig extends Config { DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ACTIVE_ITEMS.getName(), "active_items"); } + // It is ok for this to be static since the VALIDATOR itself does not change for different values for fields + // in the BulletConfig. private static final Validator VALIDATOR = new Validator(); static { VALIDATOR.define(SPECIFICATION_DEFAULT_DURATION) @@ -204,13 +206,13 @@ public class BulletConfig extends Config { .checkIf(Validator::isList) .castTo(BulletConfig::mapifyMetadata); - VALIDATOR.relatesTo("Max should be less or equal to default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) + VALIDATOR.relate("Max should be less or equal to default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) .checkIf(Validator::isGreaterOrEqual); - VALIDATOR.relatesTo("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) + VALIDATOR.relate("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) .checkIf(Validator::isGreaterOrEqual); - VALIDATOR.relatesTo("Metadata is enabled and keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + VALIDATOR.relate("Metadata is enabled and keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) .checkIf(BulletConfig::isMetadataNotConfigured); - VALIDATOR.relatesTo("Metadata is disabled and keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + VALIDATOR.relate("Metadata is disabled and keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) .checkIf(BulletConfig::isMetadataUnnecessary) .orElseUse(false, Collections.emptyMap()); } @@ -237,7 +239,10 @@ public BulletConfig() { /** * Validates and fixes configuration for this config. If there are undefaulted or wrongly typed elements, you * should use a {@link Validator} to define the appropriate definitions, casters and defaults to use. You - * should call this method before you use the config to ensure that all configurations are valid. + * should call this method before you use the config to ensure that all configurations are valid. This class + * defines a validator for all the fields it knows about. If you subclass it and define your own fields, you should + * create your own Validator and define entries and relationships that you need to validate. Make sure to call this + * method in your override if you wish validate the fields defined by this config. */ public void validate() { validate(this); diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java index a14d921f..ef35fd9d 100644 --- a/src/main/java/com/yahoo/bullet/Validator.java +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -12,11 +12,21 @@ import java.util.function.Function; import java.util.function.Predicate; +/** + * This class validates instances of {@link BulletConfig}. Use {@link com.yahoo.bullet.Validator.Entry} to define + * fields and {@link com.yahoo.bullet.Validator.Relationship} to define relationships between them. + */ @Slf4j public class Validator { - public static final Predicate UNARY_IDENTITY = o -> true; - public static final BiPredicate BINARY_IDENTITY = (oA, oB) -> true; - + private static final Predicate UNARY_IDENTITY = o -> true; + private static final BiPredicate BINARY_IDENTITY = (oA, oB) -> true; + + /** + * This represents a field in the Validator. It applies a {@link Predicate} to the value of the field and uses a + * default value (see {@link Entry#defaultTo(Object)} if the predicate fails. It can also apply an arbitrary + * conversion using {@link Entry#castTo(Function)}. These are all applied when you call + * {@link Entry#normalize(BulletConfig)} with a {@link BulletConfig} containing a field that matches the Entry. + */ public class Entry { private String key; private Predicate validation; @@ -24,26 +34,57 @@ public class Entry { private Object defaultValue; private Function adapter; - public Entry(String key) { + private Entry(String key) { this.validation = UNARY_IDENTITY; this.key = key; } + /** + * Add a {@link Predicate} to check for the field represented by the entry. This predicate should take the + * value of the field and return true to represent a successful validation and false otherwise. You can add + * more checks by repeatedly calling this method. All your predicates added so far will be ANDed to the latest + * check. + * + * @param validator The non-null validator to use for this Entry. + * @return This Entry for chaining. + */ public Entry checkIf(Predicate validator) { + Objects.requireNonNull(validator); this.validation.and(validator); return this; } + /** + * Use a default value for the field represented by this Entry. This is used if the validation fails. Note that + * the {@link Entry#castTo(Function)} will be applied to this if present. + * + * @param defaultValue The value to use for the field in the {@link BulletConfig} if validation fails. + * @return This Entry for chaining. + */ public Entry defaultTo(Object defaultValue) { this.defaultValue = defaultValue; return this; } + /** + * Apply a cast to the value in the {@link BulletConfig} after validation and defaults are applied. Use this to + * convert values in the config to their final types if you find yourself type-casting or checking for their + * types repeatedly. + * + * @param adapter The function that takes the field (or the default value) represented and converts it to another. + * @return This Entry for chaining. + */ public Entry castTo(Function adapter) { this.adapter = adapter; return this; } + /** + * Normalizes a {@link BulletConfig} by validating, apply defaults and converting the object represented by the + * field in this Entry. + * + * @param config The config to normalize. + */ public void normalize(BulletConfig config) { Object value = config.get(key); boolean isValid = validation.test(value); @@ -59,6 +100,12 @@ public void normalize(BulletConfig config) { } } + /** + * This represents a binary relationship between two fields in a {@link BulletConfig}. You should have defined + * {@link Entry} for these fields before you try to define relationships between them. You can use this apply a + * {@link BiPredicate} to these fields and provide or use their defined defaults (defined using + * {@link Entry#defaultTo(Object)}) if the check fails. + */ public class Relationship { @Getter private String keyA; @@ -69,26 +116,47 @@ public class Relationship { private Object defaultA; private Object defaultB; - public Relationship(String description, String keyA, String keyB) { + private Relationship(String description, String keyA, String keyB) { this.description = description; this.keyA = keyA; this.keyB = keyB; - // These are guaranteed to be present. + // These keys in entries are guaranteed to be present. this.defaultA = entries.get(keyA).getDefaultValue(); this.defaultB = entries.get(keyB).getDefaultValue(); this.binaryRelation = BINARY_IDENTITY; } + /** + * Provide the {@link BiPredicate} that acts as the check for this relationship. You can provide more checks + * and they will be ANDed on the existing checks. + * + * @param binaryRelation A check for this relationship. + * @return This Relationship for chaining. + */ public Relationship checkIf(BiPredicate binaryRelation) { this.binaryRelation.and(binaryRelation); return this; } + /** + * Provide custom defaults for this Relationship if you do not want to use the defaults provided in their + * Entries. + * + * @param objectA The default for the first field. + * @param objectB The default for the second field. + */ public void orElseUse(Object objectA, Object objectB) { this.defaultA = objectA; this.defaultB = objectB; } + /** + * Normalize the given {@link BulletConfig} for the fields defined by this relationship. This applies the check + * and uses the defaults (provided using {@link Relationship#orElseUse(Object, Object)} or the Entry defaults + * for these fields if the check fails. + * + * @param config The config to normalize. + */ public void normalize(BulletConfig config) { Object objectA = config.get(keyA); Object objectB = config.get(keyB); @@ -106,13 +174,31 @@ public void normalize(BulletConfig config) { private Map entries = new HashMap<>(); private List relations = new ArrayList<>(); + /** + * Creates an instance of the Entry using the name of the field. This field by default will pass the + * {@link Predicate} unless you add a check using {@link Entry#checkIf(Predicate)}. + * + * @param key The name of the field. + * @return The created {@link Entry}. + */ public Entry define(String key) { Entry entry = new Entry(key); entries.put(key, entry); return entry; } - public Relationship relatesTo(String description, String keyA, String keyB) { + /** + * Create a relationship with a description for it for the given fields. By default, the relationship will + * hold true unless you provide a custom check using {@link Relationship#checkIf(BiPredicate)}. By default, + * if the relationship fails to hold, the defaults defined by the Entries for these fields will be used unless + * you provide new ones using {@link Relationship#orElseUse(Object, Object)}. + * + * @param description A string description of this relationship. + * @param keyA The first field in the relationship. + * @param keyB The second field in the relationship. + * @return The created {@link Relationship}. + */ + public Relationship relate(String description, String keyA, String keyB) { Objects.requireNonNull(entries.get(keyA), "You cannot add a relationship for " + keyA + " before defining it"); Objects.requireNonNull(entries.get(keyB), "You cannot add a relationship for " + keyB + " before defining it"); @@ -121,6 +207,12 @@ public Relationship relatesTo(String description, String keyA, String keyB) { return relation; } + /** + * Validate and normalize the provided {@link BulletConfig} for the defined entries and relationships. Then entries + * are used to normalize the config first. + * + * @param config The config containing fields to normalize. + */ public void normalize(BulletConfig config) { entries.values().forEach(e -> e.normalize(config)); relations.stream().forEach(r -> r.normalize(config)); @@ -128,60 +220,155 @@ public void normalize(BulletConfig config) { // Type Adapters + /** + * This casts a {@link Number} Object to an {@link Integer}. + * + * @param value The value to cast. + * @return The converted Integer object. + */ public static Object asInt(Object value) { return ((Number) value).intValue(); } + /** + * This casts a {@link Number} Object to an {@link Float}. + * + * @param value The value to cast. + * @return The converted Float object. + */ public static Object asFloat(Object value) { return ((Number) value).floatValue(); } - // Validators + /** + * This casts a {@link Number} Object to an {@link Double}. + * + * @param value The value to cast. + * @return The converted Double object. + */ + public static Object asDouble(Object value) { + return ((Number) value).doubleValue(); + } + + // Unary Predicates + /** + * Checks to see if the value is null or not. + * + * @param value The object to check. + * @return A boolean denoting if the value was null. + */ public static boolean isNotNull(Object value) { return value != null; } + /** + * Checks to see if the value is of the provided type or not. + * + * @param value The object to check type for. + * @param clazz The supposed class of the value. + * @return A boolean denoting if the value was of the provided class. + */ public static boolean isType(Object value, Class clazz) { return isNotNull(value) && clazz.isInstance(value); } + /** + * Checks to see if the value is a {@link Boolean}. + * + * @param value The object to check. + * @return A boolean denoting if the value was a boolean. + */ public static boolean isBoolean(Object value) { return isType(value, Boolean.class); } + /** + * Checks to see if the value is a {@link String}. + * + * @param value The object to check. + * @return A boolean denoting if the value was a String. + */ public static boolean isString(Object value) { return isType(value, String.class); } + /** + * Checks to see if the value is a {@link List}. + * + * @param value The object to check. + * @return A boolean denoting if the value was a List. + */ public static boolean isList(Object value) { return isType(value, List.class); } + /** + * Checks to see if the value is a {@link Map}. + * + * @param value The object to check. + * @return A boolean denoting if the value was a Map. + */ public static boolean isMap(Object value) { return isType(value, Map.class); } + /** + * Checks to see if the value is a {@link Number}. + * + * @param value The object to check. + * @return A boolean denoting if the value was a Number. + */ public static boolean isNumber(Object value) { return isType(value, Number.class); } + /** + * Checks to see if the value is an integer type. Both {@link Integer} and {@link Long} qualify. + * + * @param value The object to check. + * @return A boolean denoting if the value was an integer. + */ public static boolean isInt(Object value) { return isType(value, Long.class) || isType(value, Integer.class); } + /** + * Checks to see if the value is an floating-point. Both {@link Float} and {@link Double} qualify. + * + * @param value The object to check. + * @return A boolean denoting if the value was a floating-point. + */ public static boolean isFloat(Object value) { return isType(value, Double.class) || isType(value, Float.class); } + /** + * Checks to see if the value was positive. + * + * @param value The object to check. + * @return A boolean denoting whether the given number value was positive. + */ public static boolean isPositive(Object value) { return isNumber(value) && ((Number) value).doubleValue() > 0; } + /** + * Checks to see if the value was a positive integer ({@link Integer} or {@link Long}). + * + * @param value The object to check. + * @return A boolean denoting whether the given number value was a positive integer type. + */ public static boolean isPositiveInt(Object value) { return isPositive(value) && isInt(value); } + /** + * Checks to see if the value was a positive integer ({@link Integer} or {@link Long}) and a power of 2. + * + * @param value The object to check. + * @return A boolean denoting whether the given number value was a positive, power of 2 integer type. + */ public static boolean isPowerOfTwo(Object value) { if (!isPositiveInt(value)) { return false; @@ -190,8 +377,15 @@ public static boolean isPowerOfTwo(Object value) { return (toCheck & toCheck - 1) == 0; } - // Binary Validators + // Binary Predicates. + /** + * Checks to see if the first numeric object is greater than or equal to the second numeric object. + * + * @param first The first numeric object. + * @param second The second numeric object. + * @return A boolean denoting whether the first object is greater or equal to the second. + */ public static boolean isGreaterOrEqual(Object first, Object second) { return ((Number) first).doubleValue() >= ((Number) second).doubleValue(); } From 1ac0e1598d1f998bee877f745dfe782e7bbb3fd4 Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Mon, 20 Nov 2017 17:03:37 -0800 Subject: [PATCH 3/6] Fixing most tests --- .../java/com/yahoo/bullet/BulletConfig.java | 27 +++--- src/main/java/com/yahoo/bullet/Validator.java | 8 +- src/main/resources/log4j.properties | 2 +- .../com/yahoo/bullet/BulletConfigTest.java | 34 +++++++ .../aggregations/CountDistinctTest.java | 49 +++++----- .../aggregations/DistributionTest.java | 26 +++--- .../operations/aggregations/GroupAllTest.java | 7 +- .../operations/aggregations/GroupByTest.java | 29 +++--- .../operations/aggregations/RawTest.java | 12 +-- .../operations/aggregations/TopKTest.java | 23 ++--- .../aggregations/grouping/GroupDataTest.java | 5 +- .../yahoo/bullet/parsing/AggregationTest.java | 72 +++++++------- .../bullet/parsing/AggregationUtils.java | 41 ++++---- .../com/yahoo/bullet/parsing/QueryUtils.java | 17 +++- .../bullet/parsing/SpecificationTest.java | 93 ++++++++++--------- .../bullet/querying/AggregationQueryTest.java | 16 ++-- .../bullet/querying/FilterQueryTest.java | 6 +- .../com/yahoo/bullet/result/MetadataTest.java | 35 ------- src/test/resources/log4j.properties | 2 +- 19 files changed, 262 insertions(+), 242 deletions(-) diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index d69007bf..cadb5d97 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -56,7 +56,7 @@ public class BulletConfig extends Config { public static final int DEFAULT_SPECIFICATION_DURATION = 30 * 1000; public static final int DEFAULT_SPECIFICATION_MAX_DURATION = 120 * 1000; public static final boolean DEFAULT_RECORD_INJECT_TIMESTAMP = false; - public static final String DEFAULT_RECORD_INJECT_TIMESTAMP_KEY = "bullet_receive_timestamp"; + public static final String DEFAULT_RECORD_INJECT_TIMESTAMP_KEY = "bullet_project_timestamp"; public static final int DEFAULT_AGGREGATION_SIZE = 1; public static final int DEFAULT_AGGREGATION_MAX_SIZE = 512; @@ -122,8 +122,7 @@ public class BulletConfig extends Config { .checkIf(Validator::isBoolean); VALIDATOR.define(RECORD_INJECT_TIMESTAMP_KEY) .defaultTo(DEFAULT_RECORD_INJECT_TIMESTAMP_KEY) - .checkIf(Validator::isNotNull) - .castTo(Validator::asInt); + .checkIf(Validator::isString); VALIDATOR.define(AGGREGATION_DEFAULT_SIZE) .defaultTo(DEFAULT_AGGREGATION_SIZE) @@ -211,9 +210,9 @@ public class BulletConfig extends Config { VALIDATOR.relate("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) .checkIf(Validator::isGreaterOrEqual); VALIDATOR.relate("Metadata is enabled and keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) - .checkIf(BulletConfig::isMetadataNotConfigured); + .checkIf(BulletConfig::isMetadataConfigured); VALIDATOR.relate("Metadata is disabled and keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) - .checkIf(BulletConfig::isMetadataUnnecessary) + .checkIf(BulletConfig::isMetadataNecessary) .orElseUse(false, Collections.emptyMap()); } @@ -243,9 +242,12 @@ public BulletConfig() { * defines a validator for all the fields it knows about. If you subclass it and define your own fields, you should * create your own Validator and define entries and relationships that you need to validate. Make sure to call this * method in your override if you wish validate the fields defined by this config. + * + * @return This config for chaining. */ - public void validate() { + public BulletConfig validate() { validate(this); + return this; } /** @@ -273,13 +275,16 @@ private static Object mapifyMetadata(Object metadata) { return mapping; } - @SuppressWarnings("unchecked") - private static boolean isMetadataNotConfigured(Object enabled, Object keys) { - return (Boolean) enabled && keys == null; + private static boolean isMetadataConfigured(Object enabled, Object keys) { + // This function should return false when metadata is enabled but keys are not set. + boolean isMetadataOff = !((Boolean) enabled); + return isMetadataOff || keys != null; } - private static boolean isMetadataUnnecessary(Object enabled, Object keys) { - return !((Boolean) enabled) && keys != null; + private static boolean isMetadataNecessary(Object enabled, Object keys) { + // This function should return false when metadata is disabled but keys are set. + boolean isMetadataOn = (Boolean) enabled; + return isMetadataOn || keys == null; } } diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java index ef35fd9d..c41b84b0 100644 --- a/src/main/java/com/yahoo/bullet/Validator.java +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -50,7 +50,7 @@ private Entry(String key) { */ public Entry checkIf(Predicate validator) { Objects.requireNonNull(validator); - this.validation.and(validator); + this.validation = this.validation.and(validator); return this; } @@ -134,7 +134,7 @@ private Relationship(String description, String keyA, String keyB) { * @return This Relationship for chaining. */ public Relationship checkIf(BiPredicate binaryRelation) { - this.binaryRelation.and(binaryRelation); + this.binaryRelation = this.binaryRelation.and(binaryRelation); return this; } @@ -160,8 +160,8 @@ public void orElseUse(Object objectA, Object objectB) { public void normalize(BulletConfig config) { Object objectA = config.get(keyA); Object objectB = config.get(keyB); - boolean result = binaryRelation.test(objectA, objectB); - if (!result) { + boolean isValid = binaryRelation.test(objectA, objectB); + if (!isValid) { log.warn("{}: {} and {}: {} do not satisfy: {}. Using their defaults", keyA, objectA, keyB, objectB, description); log.warn("Using default {} for {}", defaultA, keyA); log.warn("Using default {} for {}", defaultB, keyB); diff --git a/src/main/resources/log4j.properties b/src/main/resources/log4j.properties index 2ca597cd..8e37df86 100644 --- a/src/main/resources/log4j.properties +++ b/src/main/resources/log4j.properties @@ -1,5 +1,5 @@ # Root logger option -log4j.rootLogger=INFO, stdout +log4j.rootLogger=INFO,stdout # Direct log messages to stdout log4j.appender.stdout=org.apache.log4j.ConsoleAppender diff --git a/src/test/java/com/yahoo/bullet/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/BulletConfigTest.java index 40a56ca4..fe5eb45a 100644 --- a/src/test/java/com/yahoo/bullet/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/BulletConfigTest.java @@ -206,4 +206,38 @@ public void testMissingRequiredConfig() { config.getRequiredConfigAs("does.not.exist", Long.class); } + + /* + @Test + public void testConceptKeyExtractionWithMetadataNotEnabled() { + Map configuration = new HashMap<>(); + configuration.put(BulletConfig.RESULT_METADATA_METRICS, asMetadataEntries(Pair.of("Estimated Result", "foo"))); + + Set concepts = new HashSet<>(singletonList(Concept.ESTIMATED_RESULT)); + + Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), Collections.emptyMap()); + } + + @Test + public void testConceptKeyExtraction() { + Map configuration = new HashMap<>(); + configuration.put(BulletConfig.RESULT_METADATA_ENABLE, true); + configuration.put(BulletConfig.RESULT_METADATA_METRICS, + asMetadataEntries(Pair.of("Estimated Result", "foo"), + Pair.of("Sketch Metadata", "bar"), + Pair.of("Non Existent", "bar"), + Pair.of("Standard Deviations", "baz"))); + + Set concepts = new HashSet<>(asList(Concept.ESTIMATED_RESULT, + Concept.SKETCH_METADATA, + Concept.STANDARD_DEVIATIONS)); + + Map expectedMap = new HashMap<>(); + expectedMap.put(Concept.ESTIMATED_RESULT.getName(), "foo"); + expectedMap.put(Concept.SKETCH_METADATA.getName(), "bar"); + expectedMap.put(Concept.STANDARD_DEVIATIONS.getName(), "baz"); + + Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), expectedMap); + } + */ } diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/CountDistinctTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/CountDistinctTest.java index 380e6f09..75634e80 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/CountDistinctTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/CountDistinctTest.java @@ -18,25 +18,24 @@ import org.testng.Assert; import org.testng.annotations.Test; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.IntStream; -import static com.yahoo.bullet.parsing.AggregationUtils.addParsedMetadata; +import static com.yahoo.bullet.parsing.AggregationUtils.addMetadata; import static com.yahoo.bullet.parsing.AggregationUtils.makeAttributes; import static com.yahoo.bullet.parsing.AggregationUtils.makeGroupFields; import static java.util.Arrays.asList; public class CountDistinctTest { @SafeVarargs - public static CountDistinct makeCountDistinct(Map configuration, Map attributes, + public static CountDistinct makeCountDistinct(BulletConfig configuration, Map attributes, List fields, Map.Entry... metadata) { Aggregation aggregation = new Aggregation(); Map asMap = makeGroupFields(fields); aggregation.setFields(asMap); aggregation.setAttributes(attributes); - aggregation.setConfiguration(addParsedMetadata(configuration, metadata)); + aggregation.setConfiguration(addMetadata(configuration, metadata).validate()); CountDistinct countDistinct = new CountDistinct(aggregation); countDistinct.initialize(); @@ -57,24 +56,25 @@ public static CountDistinct makeCountDistinct(List fields) { } @SafeVarargs - public static CountDistinct makeCountDistinct(Map configuration, List fields, + public static CountDistinct makeCountDistinct(BulletConfig configuration, List fields, Map.Entry... metadata) { return makeCountDistinct(configuration, null, fields, metadata); } - public static Map makeConfiguration(int resizeFactor, float sampling, String family, String separator, int k) { - Map config = new HashMap<>(); - config.put(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES, k); - config.put(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR, resizeFactor); - config.put(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY, family); - config.put(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING, sampling); - config.put(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, separator); + public static BulletConfig makeConfiguration(int resizeFactor, float sampling, String family, String separator, int k) { + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES, k); + config.set(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR, resizeFactor); + config.set(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY, family); + config.set(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING, sampling); + config.set(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, separator); return config; } - public static Map makeConfiguration(int resizeFactor, int k) { - return makeConfiguration(resizeFactor, CountDistinct.DEFAULT_SAMPLING_PROBABILITY, - CountDistinct.DEFAULT_UPDATE_SKETCH_FAMILY, Aggregation.DEFAULT_FIELD_SEPARATOR, k); + public static BulletConfig makeConfiguration(int resizeFactor, int k) { + return makeConfiguration(resizeFactor, BulletConfig.DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING, + BulletConfig.DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY, + BulletConfig.DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR, k); } @Test @@ -98,10 +98,6 @@ public void testResizeFactorConversion() { Assert.assertEquals(CountDistinct.getResizeFactor(3), ResizeFactor.X8); Assert.assertEquals(CountDistinct.getResizeFactor(17), ResizeFactor.X8); Assert.assertEquals(CountDistinct.getResizeFactor(-10), ResizeFactor.X8); - - Assert.assertEquals(CountDistinct.getResizeFactor(4.9), ResizeFactor.X4); - Assert.assertEquals(CountDistinct.getResizeFactor(8.9), ResizeFactor.X8); - Assert.assertEquals(CountDistinct.getResizeFactor(-10.9), ResizeFactor.X8); } @Test @@ -116,7 +112,6 @@ public void testNoRecordCount() { BulletRecord actual = aggregate.get(0); BulletRecord expected = RecordBox.get().add(CountDistinct.DEFAULT_NEW_NAME, 0.0).getRecord(); Assert.assertEquals(actual, expected); - } @Test @@ -157,7 +152,7 @@ public void testSingleFieldExactCountDistinctWithDuplicates() { @Test public void testSingleFieldApproximateCountDistinctWithMetadata() { - Map config = makeConfiguration(4, 512); + BulletConfig config = makeConfiguration(4, 512); CountDistinct countDistinct = makeCountDistinct(config, asList("field"), Pair.of(Concept.SKETCH_METADATA, "aggregate_stats"), Pair.of(Concept.FAMILY, "family"), @@ -213,7 +208,7 @@ public void testSingleFieldApproximateCountDistinctWithMetadata() { @Test public void testNewNamingOfResult() { - Map config = makeConfiguration(4, 1024); + BulletConfig config = makeConfiguration(4, 1024); CountDistinct countDistinct = makeCountDistinct(config, makeAttributes("myCount"), asList("field"), Pair.of(Concept.SKETCH_METADATA, "stats"), Pair.of(Concept.ESTIMATED_RESULT, "est")); @@ -238,7 +233,7 @@ public void testNewNamingOfResult() { @Test public void testCombiningExact() { - Map config = makeConfiguration(4, 1024); + BulletConfig config = makeConfiguration(4, 1024); CountDistinct countDistinct = makeCountDistinct(config, makeAttributes("myCount"), asList("field")); IntStream.range(0, 512).mapToObj(i -> RecordBox.get().add("field", i).getRecord()) @@ -279,7 +274,7 @@ public void testCombiningExact() { @Test public void testCombiningAndConsuming() { - Map config = makeConfiguration(4, 1024); + BulletConfig config = makeConfiguration(4, 1024); CountDistinct countDistinct = makeCountDistinct(config, makeAttributes("myCount"), asList("field")); IntStream.range(0, 256).mapToObj(i -> RecordBox.get().add("field", i).getRecord()) @@ -309,7 +304,7 @@ public void testCombiningAndConsuming() { @Test public void testMultipleFieldsCountDistinct() { - Map config = makeConfiguration(4, 512); + BulletConfig config = makeConfiguration(4, 512); CountDistinct countDistinct = makeCountDistinct(config, makeAttributes("myCount"), asList("fieldA", "fieldB")); IntStream.range(0, 256).mapToObj(i -> RecordBox.get().add("fieldA", i).add("fieldB", 255 - i).getRecord()) .forEach(countDistinct::consume); @@ -325,9 +320,9 @@ public void testMultipleFieldsCountDistinct() { @Test public void testMultipleFieldsCountDistinctAmbiguity() { - Map config = makeConfiguration(4, 512); + BulletConfig config = makeConfiguration(4, 512); - String s = Aggregation.DEFAULT_FIELD_SEPARATOR; + String s = BulletConfig.DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR; CountDistinct countDistinct = makeCountDistinct(config, makeAttributes("myCount"), asList("fieldA", "fieldB")); BulletRecord first = RecordBox.get().add("fieldA", s).add("fieldB", s + s).getRecord(); BulletRecord second = RecordBox.get().add("fieldA", s + s).add("fieldB", s).getRecord(); diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java index 29d30441..fc95c400 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java @@ -40,7 +40,7 @@ import static com.yahoo.bullet.operations.aggregations.sketches.QuantileSketch.SEPARATOR; import static com.yahoo.bullet.operations.aggregations.sketches.QuantileSketch.START_INCLUSIVE; import static com.yahoo.bullet.operations.aggregations.sketches.QuantileSketch.VALUE_FIELD; -import static com.yahoo.bullet.parsing.AggregationUtils.addParsedMetadata; +import static com.yahoo.bullet.parsing.AggregationUtils.addMetadata; import static com.yahoo.bullet.parsing.AggregationUtils.makeAttributes; import static java.util.Arrays.asList; @@ -55,13 +55,13 @@ public class DistributionTest { Pair.of(Concept.MAXIMUM_VALUE, "max"), Pair.of(Concept.SKETCH_METADATA, "meta")); - public static Distribution makeDistribution(Map configuration, Map attributes, + public static Distribution makeDistribution(BulletConfig configuration, Map attributes, String field, int size, List> metadata) { Aggregation aggregation = new Aggregation(); aggregation.setFields(Collections.singletonMap(field, field)); aggregation.setSize(size); aggregation.setAttributes(attributes); - aggregation.configure(addParsedMetadata(configuration, metadata)); + aggregation.setConfiguration(addMetadata(configuration, metadata).validate()); Distribution distribution = new Distribution(aggregation); distribution.initialize(); @@ -81,15 +81,15 @@ public static Distribution makeDistribution(DistributionType type, List return makeDistribution(makeConfiguration(10, 128), makeAttributes(type, points), "field", 20, ALL_METADATA); } - public static Map makeConfiguration(int maxPoints, int k, int rounding) { - Map config = new HashMap<>(); - config.put(BulletConfig.DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES, k); - config.put(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, maxPoints); - config.put(BulletConfig.DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING, rounding); + public static BulletConfig makeConfiguration(int maxPoints, int k, int rounding) { + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES, k); + config.set(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, maxPoints); + config.set(BulletConfig.DISTRIBUTION_AGGREGATION_GENERATED_POINTS_ROUNDING, rounding); return config; } - public static Map makeConfiguration(int maxPoints, int k) { + public static BulletConfig makeConfiguration(int maxPoints, int k) { return makeConfiguration(maxPoints, k, 4); } @@ -98,7 +98,7 @@ public void testInitialize() { List errors; Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(Collections.emptyMap()); + aggregation.setConfiguration(new BulletConfig().validate()); Distribution distribution = new Distribution(aggregation); errors = distribution.initialize(); @@ -128,7 +128,7 @@ public void testInitialize() { public void testRangeInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(Collections.emptyMap()); + aggregation.setConfiguration(new BulletConfig().validate()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; @@ -191,7 +191,7 @@ public void testRangeInitialization() { public void testNumberOfPointsInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(Collections.emptyMap()); + aggregation.setConfiguration(new BulletConfig().validate()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; @@ -230,7 +230,7 @@ public void testNumberOfPointsInitialization() { public void testProvidedPointsInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(Collections.emptyMap()); + aggregation.setConfiguration(new BulletConfig().validate()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java index 7cda462f..a31ef07a 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java @@ -5,6 +5,7 @@ */ package com.yahoo.bullet.operations.aggregations; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.AggregationType; import com.yahoo.bullet.operations.AggregationOperations.GroupOperationType; import com.yahoo.bullet.operations.aggregations.grouping.GroupOperation; @@ -36,7 +37,7 @@ public static GroupAll makeGroupAll(Map... groupOperations) { // Does not matter aggregation.setSize(1); aggregation.setAttributes(makeAttributes(groupOperations)); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); GroupAll all = new GroupAll(aggregation); all.initialize(); return all; @@ -123,14 +124,14 @@ public void testCountingMoreThanMaximum() { GroupAll groupAll = makeGroupAll(makeGroupOperation(GroupOperationType.COUNT, null, null)); BulletRecord someRecord = RecordBox.get().add("foo", 1).getRecord(); - IntStream.range(0, 2 * Aggregation.DEFAULT_MAX_SIZE).forEach(i -> groupAll.consume(someRecord)); + IntStream.range(0, 2 * BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE).forEach(i -> groupAll.consume(someRecord)); Assert.assertNotNull(groupAll.getSerializedAggregation()); List aggregate = groupAll.getAggregation().getRecords(); Assert.assertEquals(aggregate.size(), 1); BulletRecord actual = aggregate.get(0); BulletRecord expected = RecordBox.get().add(GroupOperationType.COUNT.getName(), - 2L * Aggregation.DEFAULT_MAX_SIZE).getRecord(); + 2L * BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE).getRecord(); Assert.assertEquals(actual, expected); } diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupByTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupByTest.java index cf72f899..820824cc 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupByTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupByTest.java @@ -20,7 +20,6 @@ import org.testng.Assert; import org.testng.annotations.Test; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -31,7 +30,7 @@ import static com.yahoo.bullet.operations.AggregationOperations.GroupOperationType.AVG; import static com.yahoo.bullet.operations.AggregationOperations.GroupOperationType.COUNT; import static com.yahoo.bullet.operations.AggregationOperations.GroupOperationType.SUM; -import static com.yahoo.bullet.parsing.AggregationUtils.addParsedMetadata; +import static com.yahoo.bullet.parsing.AggregationUtils.addMetadata; import static com.yahoo.bullet.parsing.AggregationUtils.makeAttributes; import static com.yahoo.bullet.parsing.AggregationUtils.makeGroupFields; import static com.yahoo.bullet.parsing.AggregationUtils.makeGroupOperation; @@ -47,7 +46,7 @@ public class GroupByTest { Pair.of(Concept.UNIQUES_ESTIMATE, "uniquesApprox"), Pair.of(Concept.STANDARD_DEVIATIONS, "stddev")); - public static GroupBy makeGroupBy(Map configuration, Map fields, int size, + public static GroupBy makeGroupBy(BulletConfig configuration, Map fields, int size, List> operations, List> metadata) { Aggregation aggregation = new Aggregation(); @@ -57,14 +56,15 @@ public static GroupBy makeGroupBy(Map configuration, Map configuration, Map fields, int size, + public static GroupBy makeGroupBy(BulletConfig configuration, Map fields, int size, Map... operations) { return makeGroupBy(configuration, fields, size, asList(operations), ALL_METADATA); } @@ -87,18 +87,19 @@ public static GroupBy makeDistinct(List fields, int size) { return makeDistinct(makeGroupFields(fields), size); } - public static Map makeConfiguration(int resizeFactor, float sampling, String separator, int k) { - Map config = new HashMap<>(); - config.put(BulletConfig.GROUP_AGGREGATION_SKETCH_ENTRIES, k); - config.put(BulletConfig.GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR, resizeFactor); - config.put(BulletConfig.GROUP_AGGREGATION_SKETCH_SAMPLING, sampling); - config.put(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, separator); + public static BulletConfig makeConfiguration(int resizeFactor, float sampling, String separator, int k) { + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.GROUP_AGGREGATION_SKETCH_ENTRIES, k); + config.set(BulletConfig.GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR, resizeFactor); + config.set(BulletConfig.GROUP_AGGREGATION_SKETCH_SAMPLING, sampling); + config.set(BulletConfig.AGGREGATION_COMPOSITE_FIELD_SEPARATOR, separator); return config; } - public static Map makeConfiguration(int k) { - return makeConfiguration(GroupBy.DEFAULT_RESIZE_FACTOR, GroupBy.DEFAULT_SAMPLING_PROBABILITY, - Aggregation.DEFAULT_FIELD_SEPARATOR, k); + public static BulletConfig makeConfiguration(int k) { + return makeConfiguration(BulletConfig.DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR, + BulletConfig.DEFAULT_GROUP_AGGREGATION_SKETCH_SAMPLING, + BulletConfig.DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR, k); } @Test diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/RawTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/RawTest.java index b7fd44af..ff46ce66 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/RawTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/RawTest.java @@ -17,9 +17,7 @@ import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -40,17 +38,17 @@ private void readObject(java.io.ObjectInputStream in) throws IOException, ClassN public static Raw makeRaw(int size, int microBatchSize, int maxSize) { Aggregation aggregation = new Aggregation(); aggregation.setSize(size); - Map config = new HashMap<>(); - config.put(BulletConfig.RAW_AGGREGATION_MAX_SIZE, maxSize); - config.put(BulletConfig.RAW_AGGREGATION_MICRO_BATCH_SIZE, microBatchSize); - aggregation.setConfiguration(config); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RAW_AGGREGATION_MAX_SIZE, maxSize); + config.set(BulletConfig.RAW_AGGREGATION_MICRO_BATCH_SIZE, microBatchSize); + aggregation.setConfiguration(config.validate()); Raw raw = new Raw(aggregation); raw.initialize(); return raw; } public static Raw makeRaw(int size, int microBatchSize) { - return makeRaw(size, microBatchSize, Raw.DEFAULT_MAX_SIZE); + return makeRaw(size, microBatchSize, BulletConfig.DEFAULT_RAW_AGGREGATION_MAX_SIZE); } public static Raw makeRaw(int size) { diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/TopKTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/TopKTest.java index 10965531..10586526 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/TopKTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/TopKTest.java @@ -26,7 +26,7 @@ import java.util.Set; import java.util.stream.IntStream; -import static com.yahoo.bullet.parsing.AggregationUtils.addParsedMetadata; +import static com.yahoo.bullet.parsing.AggregationUtils.addMetadata; import static com.yahoo.bullet.parsing.AggregationUtils.makeAttributes; import static com.yahoo.bullet.parsing.AggregationUtils.makeGroupFields; import static java.util.Arrays.asList; @@ -44,14 +44,15 @@ public class TopKTest { Pair.of(Concept.SKETCH_METADATA, "meta")); - public static TopK makeTopK(Map configuration, Map attributes, + public static TopK makeTopK(BulletConfig configuration, Map attributes, Map fields, int size, List> metadata) { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.TOP_K); aggregation.setFields(fields); aggregation.setAttributes(attributes); aggregation.setSize(size); - aggregation.configure(addParsedMetadata(configuration, metadata)); + aggregation.setConfiguration(addMetadata(configuration, metadata).validate()); + TopK topK = new TopK(aggregation); topK.initialize(); return topK; @@ -67,11 +68,11 @@ public static TopK makeTopK(List fields, int maxMapSize, int size) { return makeTopK(ErrorType.NO_FALSE_NEGATIVES, fields, null, maxMapSize, size, null); } - public static Map makeConfiguration(ErrorType errorType, int maxMapSize) { - Map config = new HashMap<>(); - config.put(BulletConfig.TOP_K_AGGREGATION_SKETCH_ENTRIES, maxMapSize); - config.put(BulletConfig.TOP_K_AGGREGATION_SKETCH_ERROR_TYPE, - errorType == ErrorType.NO_FALSE_POSITIVES ? TopK.NO_FALSE_POSITIVES : TopK.NO_FALSE_NEGATIVES); + public static BulletConfig makeConfiguration(ErrorType errorType, int maxMapSize) { + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.TOP_K_AGGREGATION_SKETCH_ENTRIES, maxMapSize); + config.set(BulletConfig.TOP_K_AGGREGATION_SKETCH_ERROR_TYPE, + errorType == ErrorType.NO_FALSE_POSITIVES ? TopK.NO_FALSE_POSITIVES : TopK.NO_FALSE_NEGATIVES); return config; } @@ -273,8 +274,8 @@ public void testNullAttributes() { @Test public void testBadMaxMapEntries() { TopK topK = makeTopK(makeConfiguration(ErrorType.NO_FALSE_NEGATIVES, -1), null, singletonMap("A", "foo"), - TopK.DEFAULT_MAX_MAP_ENTRIES, null); - int uniqueGroups = TopK.DEFAULT_MAX_MAP_ENTRIES / 4; + BulletConfig.DEFAULT_TOP_K_AGGREGATION_SKETCH_ENTRIES, null); + int uniqueGroups = BulletConfig.DEFAULT_TOP_K_AGGREGATION_SKETCH_ENTRIES / 4; IntStream.range(0, uniqueGroups).mapToObj(i -> RecordBox.get().add("A", i).getRecord()) .forEach(topK::consume); @@ -293,7 +294,7 @@ public void testBadMaxMapEntries() { // Not a power of 2 TopK another = makeTopK(makeConfiguration(ErrorType.NO_FALSE_NEGATIVES, 5), null, singletonMap("A", "foo"), - TopK.DEFAULT_MAX_MAP_ENTRIES, null); + BulletConfig.DEFAULT_TOP_K_AGGREGATION_SKETCH_ENTRIES, null); IntStream.range(0, uniqueGroups).mapToObj(i -> RecordBox.get().add("A", i).getRecord()) .forEach(another::consume); diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java index ace6e458..1c691253 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java @@ -5,6 +5,7 @@ */ package com.yahoo.bullet.operations.aggregations.grouping; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.GroupOperationType; import com.yahoo.bullet.operations.SerializerDeserializer; import com.yahoo.bullet.parsing.Aggregation; @@ -98,10 +99,10 @@ public void testCountingMoreThanMaximum() { GroupData data = make(new GroupOperation(GroupOperationType.COUNT, null, null)); BulletRecord someRecord = RecordBox.get().add("foo", 1).getRecord(); - IntStream.range(0, 2 * Aggregation.DEFAULT_MAX_SIZE).forEach(i -> data.consume(someRecord)); + IntStream.range(0, 2 * BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE).forEach(i -> data.consume(someRecord)); BulletRecord expected = RecordBox.get().add(GroupOperationType.COUNT.getName(), - 2L * Aggregation.DEFAULT_MAX_SIZE).getRecord(); + 2L * BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE).getRecord(); Assert.assertEquals(data.getMetricsAsBulletRecord(), expected); } diff --git a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java index 6d542097..6a7f825e 100644 --- a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java @@ -18,10 +18,7 @@ import org.testng.Assert; import org.testng.annotations.Test; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; import static com.yahoo.bullet.operations.AggregationOperations.AggregationType.COUNT_DISTINCT; @@ -32,7 +29,6 @@ import static com.yahoo.bullet.parsing.AggregationUtils.makeAttributes; import static com.yahoo.bullet.parsing.AggregationUtils.makeGroupOperation; import static java.util.Arrays.asList; -import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -40,37 +36,39 @@ public class AggregationTest { @Test public void testSize() { Aggregation aggregation = new Aggregation(); + BulletConfig config = new BulletConfig().validate(); aggregation.setSize(null); - aggregation.configure(emptyMap()); - Assert.assertEquals(aggregation.getSize(), Aggregation.DEFAULT_SIZE); + aggregation.configure(config); + Assert.assertEquals((Object) aggregation.getSize(), BulletConfig.DEFAULT_AGGREGATION_SIZE); aggregation.setSize(-10); - aggregation.configure(emptyMap()); - Assert.assertEquals(aggregation.getSize(), Aggregation.DEFAULT_SIZE); + aggregation.configure(config); + Assert.assertEquals((Object) aggregation.getSize(), BulletConfig.DEFAULT_AGGREGATION_SIZE); aggregation.setSize(0); - aggregation.configure(emptyMap()); + aggregation.configure(config); Assert.assertEquals(aggregation.getSize(), (Integer) 0); aggregation.setSize(1); - aggregation.configure(emptyMap()); + aggregation.configure(config); Assert.assertEquals(aggregation.getSize(), (Integer) 1); - aggregation.setSize(Aggregation.DEFAULT_SIZE); - aggregation.configure(emptyMap()); - Assert.assertEquals(aggregation.getSize(), Aggregation.DEFAULT_SIZE); + aggregation.setSize(BulletConfig.DEFAULT_AGGREGATION_SIZE); + aggregation.configure(config); + Assert.assertEquals((Object) aggregation.getSize(), BulletConfig.DEFAULT_AGGREGATION_SIZE); - aggregation.setSize(Aggregation.DEFAULT_MAX_SIZE + 1); - aggregation.configure(emptyMap()); - Assert.assertEquals(aggregation.getSize(), Aggregation.DEFAULT_MAX_SIZE); + aggregation.setSize(BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE + 1); + aggregation.configure(config); + Assert.assertEquals((Object) aggregation.getSize(), BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE); } @Test public void testConfiguredSize() { - Map config = new HashMap<>(); - config.put(BulletConfig.AGGREGATION_DEFAULT_SIZE, 10); - config.put(BulletConfig.AGGREGATION_MAX_SIZE, 200); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.AGGREGATION_DEFAULT_SIZE, 10); + config.set(BulletConfig.AGGREGATION_MAX_SIZE, 200); + config.validate(); Aggregation aggregation = new Aggregation(); @@ -117,8 +115,8 @@ public void testSuccessfulValidate() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT, null, "count"))); + aggregation.configure(new BulletConfig().validate()); - aggregation.configure(emptyMap()); Assert.assertFalse(aggregation.validate().isPresent()); } @@ -127,7 +125,7 @@ public void testValidateNoField() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(SUM, null, null))); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -139,7 +137,7 @@ public void testUnsupportedOperation() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT_FIELD, "someField", "myCountField"))); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -151,7 +149,7 @@ public void testAttributeOperationMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, null)); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); // Missing attribute operations should be silently validated List errors = aggregation.validate().get(); @@ -164,7 +162,7 @@ public void testAttributeOperationBadFormat() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, asList("foo"))); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); // Bad attribute operations should be silently validated List errors = aggregation.validate().get(); @@ -178,7 +176,7 @@ public void testAttributeOperationsUnknownOperation() { aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT, null, "bar"), makeGroupOperation(COUNT_FIELD, "foo", "foo_avg"))); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); // The bad operation should have been thrown out. Assert.assertEquals(aggregation.validate(), Optional.>empty()); @@ -195,7 +193,7 @@ public void testAttributeOperationsDuplicateOperation() { makeGroupOperation(COUNT, "bar", null), makeGroupOperation(COUNT, "foo", null), makeGroupOperation(COUNT, "bar", null))); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); // The bad ones should be removed. Assert.assertEquals(aggregation.validate(), Optional.>empty()); } @@ -204,7 +202,7 @@ public void testAttributeOperationsDuplicateOperation() { public void testFailValidateOnCountDistinctFieldsMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(COUNT_DISTINCT); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -215,7 +213,7 @@ public void testFailValidateOnCountDistinctFieldsMissing() { public void testFailValidateOnGroupFieldsAndOperationsMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -225,7 +223,7 @@ public void testFailValidateOnGroupFieldsAndOperationsMissing() { @Test public void testToString() { Aggregation aggregation = new Aggregation(); - aggregation.configure(emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.toString(), "{size: 1, type: RAW, fields: null, attributes: null}"); @@ -244,16 +242,16 @@ public void testToString() { @Test public void testUnimplementedStrategies() { Aggregation aggregation = new Aggregation(); - aggregation.setType(null); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); + Assert.assertNull(aggregation.getStrategy()); } @Test public void testRawStrategy() { Aggregation aggregation = new Aggregation(); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.findStrategy().getClass(), Raw.class); } @@ -265,7 +263,7 @@ public void testGroupAllStrategy() { aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, singletonList(singletonMap(GroupOperation.OPERATION_TYPE, GroupOperationType.COUNT.getName())))); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupAll.class); } @@ -275,7 +273,7 @@ public void testCountDistinctStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.COUNT_DISTINCT); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.getStrategy().getClass(), CountDistinct.class); } @@ -285,7 +283,7 @@ public void testDistinctStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.GROUP); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupBy.class); } @@ -298,7 +296,7 @@ public void testGroupByStrategy() { aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, singletonList(singletonMap(GroupOperation.OPERATION_TYPE, GroupOperationType.COUNT.getName())))); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupBy.class); } @@ -308,7 +306,7 @@ public void testDistributionStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.DISTRIBUTION); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(Collections.emptyMap()); + aggregation.configure(new BulletConfig().validate()); Assert.assertEquals(aggregation.getStrategy().getClass(), Distribution.class); } diff --git a/src/test/java/com/yahoo/bullet/parsing/AggregationUtils.java b/src/test/java/com/yahoo/bullet/parsing/AggregationUtils.java index 7d90bc26..db71d502 100644 --- a/src/test/java/com/yahoo/bullet/parsing/AggregationUtils.java +++ b/src/test/java/com/yahoo/bullet/parsing/AggregationUtils.java @@ -14,6 +14,8 @@ import com.yahoo.bullet.operations.aggregations.grouping.GroupOperation; import com.yahoo.bullet.result.Metadata; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -23,6 +25,28 @@ import static java.util.Arrays.asList; public class AggregationUtils { + public static BulletConfig addMetadata(BulletConfig config, List> metadata) { + if (metadata == null) { + config.set(BulletConfig.RESULT_METADATA_ENABLE, false); + return config; + } + List> metadataList = new ArrayList<>(); + for (Map.Entry meta : metadata) { + Map entry = new HashMap<>(); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, meta.getKey().getName()); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, meta.getValue()); + metadataList.add(entry); + } + config.set(BulletConfig.RESULT_METADATA_METRICS, metadataList); + config.set(BulletConfig.RESULT_METADATA_ENABLE, true); + return config; + } + + @SafeVarargs + public static BulletConfig addMetadata(BulletConfig config, Map.Entry... metadata) { + return addMetadata(config, metadata == null ? null : Arrays.asList(metadata)); + } + public static Map makeGroupFields(List fields) { if (fields != null) { return fields.stream().collect(Collectors.toMap(Function.identity(), Function.identity())); @@ -60,23 +84,6 @@ public static Map makeAttributes(Map... maps) { return makeAttributes(asList(maps)); } - @SafeVarargs - public static Map addParsedMetadata(Map configuration, Map.Entry... metadata) { - return addParsedMetadata(configuration, metadata != null ? asList(metadata) : null); - } - - public static Map addParsedMetadata(Map configuration, List> metadata) { - if (metadata != null) { - Map metadataKeys = new HashMap<>(); - for (Map.Entry e : metadata) { - metadataKeys.put(e.getKey().getName(), e.getValue()); - } - configuration.put(BulletConfig.RESULT_METADATA_METRICS_MAPPING, metadataKeys); - configuration.put(BulletConfig.RESULT_METADATA_METRICS_MAPPING, metadataKeys); - } - return configuration; - } - public static Map makeAttributes(List> maps) { Map map = new HashMap<>(); map.put(GroupOperation.OPERATIONS, maps); diff --git a/src/test/java/com/yahoo/bullet/parsing/QueryUtils.java b/src/test/java/com/yahoo/bullet/parsing/QueryUtils.java index d7d45a50..455529ec 100644 --- a/src/test/java/com/yahoo/bullet/parsing/QueryUtils.java +++ b/src/test/java/com/yahoo/bullet/parsing/QueryUtils.java @@ -5,6 +5,7 @@ */ package com.yahoo.bullet.parsing; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.AggregationType; import com.yahoo.bullet.operations.AggregationOperations.DistributionType; import com.yahoo.bullet.operations.FilterOperations.FilterType; @@ -379,17 +380,25 @@ public static String getOperationFor(AggregationType operation) { } } - public static AggregationQuery getAggregationQuery(String queryString, Map configuration) { + public static AggregationQuery getAggregationQuery(String queryString, Map configuration) { try { - return new AggregationQuery(queryString, configuration); + BulletConfig config = new BulletConfig(); + configuration.forEach(config::set); + config.validate(); + + return new AggregationQuery(queryString, config); } catch (ParsingException e) { throw new RuntimeException(e); } } - public static FilterQuery getFilterQuery(String input, Map configuration) { + public static FilterQuery getFilterQuery(String input, Map configuration) { try { - return new FilterQuery(input, configuration); + BulletConfig config = new BulletConfig(); + configuration.forEach(config::set); + config.validate(); + + return new FilterQuery(input, config); } catch (ParsingException e) { throw new RuntimeException(e); } diff --git a/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java b/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java index 317ffa13..d67e1114 100644 --- a/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java @@ -18,8 +18,6 @@ import org.testng.annotations.Test; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -29,7 +27,6 @@ import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.mockito.Mockito.mock; @@ -92,13 +89,15 @@ public static int size(BulletRecord record) { @Test public void testDefaults() { Specification specification = new Specification(); - specification.configure(emptyMap()); + BulletConfig config = new BulletConfig(); + config.validate(); + specification.configure(config); Assert.assertNull(specification.getProjection()); Assert.assertNull(specification.getFilters()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_DURATION_MS); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_DURATION); Assert.assertEquals(specification.getAggregation().getType(), AggregationType.RAW); - Assert.assertEquals(specification.getAggregation().getSize(), Aggregation.DEFAULT_SIZE); + Assert.assertEquals((Object) specification.getAggregation().getSize(), BulletConfig.DEFAULT_AGGREGATION_SIZE); Assert.assertTrue(specification.isAcceptingData()); Assert.assertEquals(specification.getAggregate().getRecords(), emptyList()); } @@ -139,47 +138,51 @@ public void testAggregationForced() { Assert.assertNull(specification.getFilters()); // If you had null for aggregation Assert.assertNull(specification.getAggregation()); - specification.configure(Collections.emptyMap()); + specification.configure(new BulletConfig().validate()); + Assert.assertTrue(specification.isAcceptingData()); Assert.assertEquals(specification.getAggregate().getRecords(), emptyList()); } @Test public void testDuration() { + BulletConfig config = new BulletConfig().validate(); + Specification specification = new Specification(); - specification.configure(emptyMap()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_DURATION_MS); + specification.configure(config); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_DURATION); specification.setDuration(-1000); - specification.configure(emptyMap()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_DURATION_MS); + specification.configure(config); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_DURATION); specification.setDuration(0); - specification.configure(emptyMap()); + specification.configure(config); Assert.assertEquals(specification.getDuration(), (Integer) 0); specification.setDuration(1); - specification.configure(emptyMap()); + specification.configure(config); Assert.assertEquals(specification.getDuration(), (Integer) 1); - specification.setDuration(Specification.DEFAULT_DURATION_MS); - specification.configure(emptyMap()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_DURATION_MS); + specification.setDuration(BulletConfig.DEFAULT_SPECIFICATION_DURATION); + specification.configure(config); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_DURATION); - specification.setDuration(Specification.DEFAULT_MAX_DURATION_MS); - specification.configure(emptyMap()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_MAX_DURATION_MS); + specification.setDuration(BulletConfig.DEFAULT_SPECIFICATION_MAX_DURATION); + specification.configure(config); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_MAX_DURATION); - specification.setDuration(Specification.DEFAULT_MAX_DURATION_MS * 2); - specification.configure(emptyMap()); - Assert.assertEquals(specification.getDuration(), Specification.DEFAULT_MAX_DURATION_MS); + specification.setDuration(BulletConfig.DEFAULT_SPECIFICATION_MAX_DURATION * 2); + specification.configure(config); + Assert.assertEquals((Object) specification.getDuration(), BulletConfig.DEFAULT_SPECIFICATION_MAX_DURATION); } @Test public void testCustomDuration() { - Map config = new HashMap<>(); - config.put(BulletConfig.SPECIFICATION_DEFAULT_DURATION, 200); - config.put(BulletConfig.SPECIFICATION_MAX_DURATION, 1000); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.SPECIFICATION_DEFAULT_DURATION, 200); + config.set(BulletConfig.SPECIFICATION_MAX_DURATION, 1000); + config.validate(); Specification specification = new Specification(); @@ -216,7 +219,7 @@ public void testCustomDuration() { public void testFiltering() { Specification specification = new Specification(); specification.setFilters(singletonList(FilterClauseTest.getFieldFilter(FilterType.EQUALS, "foo", "bar"))); - specification.configure(emptyMap()); + specification.configure(new BulletConfig().validate()); Assert.assertTrue(specification.filter(RecordBox.get().add("field", "foo").getRecord())); Assert.assertTrue(specification.filter(RecordBox.get().add("field", "bar").getRecord())); @@ -227,10 +230,11 @@ public void testFiltering() { public void testReceiveTimestampNoProjection() { Long start = System.currentTimeMillis(); - Map config = new HashMap<>(); - config.put(BulletConfig.RECORD_INJECT_TIMESTAMP, true); Specification specification = new Specification(); specification.setProjection(null); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RECORD_INJECT_TIMESTAMP, true); + config.validate(); specification.configure(config); BulletRecord input = RecordBox.get().add("field", "foo").add("mid", "123").getRecord(); @@ -242,7 +246,7 @@ public void testReceiveTimestampNoProjection() { Assert.assertEquals(actual.get("field"), "foo"); Assert.assertEquals(actual.get("mid"), "123"); - Long recordedTimestamp = (Long) actual.get(Specification.DEFAULT_RECEIVE_TIMESTAMP_KEY); + Long recordedTimestamp = (Long) actual.get(BulletConfig.DEFAULT_RECORD_INJECT_TIMESTAMP_KEY); Assert.assertTrue(recordedTimestamp >= start); Assert.assertTrue(recordedTimestamp <= end); } @@ -251,12 +255,13 @@ public void testReceiveTimestampNoProjection() { public void testReceiveTimestamp() { Long start = System.currentTimeMillis(); - Map config = new HashMap<>(); - config.put(BulletConfig.RECORD_INJECT_TIMESTAMP, true); Specification specification = new Specification(); Projection projection = new Projection(); projection.setFields(singletonMap("field", "bid")); specification.setProjection(projection); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RECORD_INJECT_TIMESTAMP, true); + config.validate(); specification.configure(config); BulletRecord input = RecordBox.get().add("field", "foo").add("mid", "123").getRecord(); @@ -267,7 +272,7 @@ public void testReceiveTimestamp() { Assert.assertEquals(size(actual), 2); Assert.assertEquals(actual.get("bid"), "foo"); - Long recordedTimestamp = (Long) actual.get(Specification.DEFAULT_RECEIVE_TIMESTAMP_KEY); + Long recordedTimestamp = (Long) actual.get(BulletConfig.DEFAULT_RECORD_INJECT_TIMESTAMP_KEY); Assert.assertTrue(recordedTimestamp >= start); Assert.assertTrue(recordedTimestamp <= end); } @@ -277,25 +282,26 @@ public void testAggregationDefault() { Specification specification = new Specification(); Aggregation aggregation = new Aggregation(); aggregation.setType(null); - aggregation.setSize(Aggregation.DEFAULT_MAX_SIZE - 1); + aggregation.setSize(BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE - 1); specification.setAggregation(aggregation); Assert.assertNull(aggregation.getType()); - specification.configure(emptyMap()); + specification.configure(new BulletConfig().validate()); // Specification no longer fixes type Assert.assertNull(aggregation.getType()); - Assert.assertEquals(aggregation.getSize(), new Integer(Aggregation.DEFAULT_MAX_SIZE - 1)); + Assert.assertEquals(aggregation.getSize(), new Integer(BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE - 1)); } @Test public void testMeetingDefaultSpecification() { Specification specification = new Specification(); - specification.configure(emptyMap()); - Assert.assertTrue(makeStream(Aggregation.DEFAULT_SIZE - 1).map(specification::filter).allMatch(x -> x)); + specification.configure(new BulletConfig().validate()); + + Assert.assertTrue(makeStream(BulletConfig.DEFAULT_AGGREGATION_SIZE - 1).map(specification::filter).allMatch(x -> x)); // Check that we only get the default number out - makeList(Aggregation.DEFAULT_SIZE + 2).forEach(specification::aggregate); - Assert.assertEquals((Integer) specification.getAggregate().getRecords().size(), Aggregation.DEFAULT_SIZE); + makeList(BulletConfig.DEFAULT_AGGREGATION_SIZE + 2).forEach(specification::aggregate); + Assert.assertEquals((Object) specification.getAggregate().getRecords().size(), BulletConfig.DEFAULT_AGGREGATION_SIZE); } @Test @@ -367,18 +373,19 @@ public void testAggregationExceptions() { @Test public void testToString() { + BulletConfig config = new BulletConfig().validate(); Specification specification = new Specification(); - specification.configure(emptyMap()); + specification.configure(config); Assert.assertEquals(specification.toString(), - "{filters: null, projection: null, " + - "aggregation: {size: 1, type: RAW, fields: null, attributes: null}, duration: 30000}"); + "{filters: null, projection: null, " + + "aggregation: {size: 1, type: RAW, fields: null, attributes: null}, duration: 30000}"); specification.setFilters(singletonList(FilterClauseTest.getFieldFilter(FilterType.EQUALS, "foo", "bar"))); Projection projection = new Projection(); projection.setFields(singletonMap("field", "bid")); specification.setProjection(projection); - specification.configure(emptyMap()); + specification.configure(config); Assert.assertEquals(specification.toString(), "{filters: [{operation: EQUALS, field: field, values: [foo, bar]}], " + diff --git a/src/test/java/com/yahoo/bullet/querying/AggregationQueryTest.java b/src/test/java/com/yahoo/bullet/querying/AggregationQueryTest.java index 1637e067..ef4a04ca 100644 --- a/src/test/java/com/yahoo/bullet/querying/AggregationQueryTest.java +++ b/src/test/java/com/yahoo/bullet/querying/AggregationQueryTest.java @@ -8,8 +8,6 @@ import com.google.gson.JsonParseException; import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.AggregationType; -import com.yahoo.bullet.operations.aggregations.Raw; -import com.yahoo.bullet.parsing.Aggregation; import com.yahoo.bullet.record.BulletRecord; import org.testng.Assert; import org.testng.annotations.Test; @@ -64,8 +62,8 @@ public void testAggregationTime() { AggregationQuery query = getAggregationQuery("{'aggregation' : {}}", emptyMap()); long creationTime = query.getStartTime(); byte[] record = getListBytes(new BulletRecord()); - IntStream.range(0, Aggregation.DEFAULT_SIZE).forEach((x) -> query.consume(record)); - Assert.assertEquals(query.getData().getRecords().size(), (int) Aggregation.DEFAULT_SIZE); + IntStream.range(0, BulletConfig.DEFAULT_AGGREGATION_SIZE).forEach((x) -> query.consume(record)); + Assert.assertEquals(query.getData().getRecords().size(), (int) BulletConfig.DEFAULT_AGGREGATION_SIZE); long lastAggregationTime = query.getLastAggregationTime(); Assert.assertTrue(creationTime <= lastAggregationTime); } @@ -74,9 +72,9 @@ public void testAggregationTime() { public void testDefaultLimiting() { AggregationQuery query = getAggregationQuery("{'aggregation' : {}}", emptyMap()); byte[] record = getListBytes(new BulletRecord()); - IntStream.range(0, Aggregation.DEFAULT_SIZE - 1).forEach(x -> Assert.assertFalse(query.consume(record))); + IntStream.range(0, BulletConfig.DEFAULT_AGGREGATION_SIZE - 1).forEach(x -> Assert.assertFalse(query.consume(record))); Assert.assertTrue(query.consume(record)); - Assert.assertEquals((Integer) query.getData().getRecords().size(), Aggregation.DEFAULT_SIZE); + Assert.assertEquals((Object) query.getData().getRecords().size(), BulletConfig.DEFAULT_AGGREGATION_SIZE); } @Test @@ -92,9 +90,9 @@ public void testCustomLimiting() { public void testSizeUpperBound() { AggregationQuery query = getAggregationQuery(makeAggregationQuery(AggregationType.RAW, 1000), emptyMap()); byte[] record = getListBytes(new BulletRecord()); - IntStream.range(0, Raw.DEFAULT_MAX_SIZE - 1).forEach(x -> Assert.assertFalse(query.consume(record))); + IntStream.range(0, BulletConfig.DEFAULT_RAW_AGGREGATION_MAX_SIZE - 1).forEach(x -> Assert.assertFalse(query.consume(record))); Assert.assertTrue(query.consume(record)); - Assert.assertEquals((Integer) query.getData().getRecords().size(), Raw.DEFAULT_MAX_SIZE); + Assert.assertEquals((Object) query.getData().getRecords().size(), BulletConfig.DEFAULT_RAW_AGGREGATION_MAX_SIZE); } @Test @@ -102,8 +100,8 @@ public void testConfiguredUpperBound() { Map config = new HashMap<>(); config.put(BulletConfig.AGGREGATION_MAX_SIZE, 2000); config.put(BulletConfig.RAW_AGGREGATION_MAX_SIZE, 200); - AggregationQuery query = getAggregationQuery(makeAggregationQuery(AggregationType.RAW, 1000), config); + byte[] record = getListBytes(new BulletRecord()); IntStream.range(0, 199).forEach(x -> Assert.assertFalse(query.consume(record))); Assert.assertTrue(query.consume(record)); diff --git a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java index 8384e5f8..073e5c51 100644 --- a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java +++ b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java @@ -5,9 +5,9 @@ */ package com.yahoo.bullet.querying; +import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.AggregationType; import com.yahoo.bullet.operations.FilterOperations.FilterType; -import com.yahoo.bullet.parsing.Aggregation; import com.yahoo.bullet.parsing.ParsingException; import com.yahoo.bullet.result.RecordBox; import org.apache.commons.lang3.tuple.Pair; @@ -42,7 +42,7 @@ public void testFilteringProjection() { @Test public void testNoAggregationAttempted() { FilterQuery query = getFilterQuery(makeRawFullQuery("map_field.id", Arrays.asList("1", "23"), FilterType.EQUALS, - AggregationType.RAW, Aggregation.DEFAULT_MAX_SIZE, + AggregationType.RAW, BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE, Pair.of("map_field.id", "mid")), emptyMap()); @@ -106,6 +106,6 @@ public void testMaximumEmittedWithNonMatchingRecords() { @Test(expectedExceptions = ParsingException.class) public void testValidationFail() throws ParsingException { - new FilterQuery("{ 'aggregation': { 'type': null } }", emptyMap()); + new FilterQuery("{ 'aggregation': { 'type': null } }", new BulletConfig().validate()); } } diff --git a/src/test/java/com/yahoo/bullet/result/MetadataTest.java b/src/test/java/com/yahoo/bullet/result/MetadataTest.java index ba737c61..0df0616e 100644 --- a/src/test/java/com/yahoo/bullet/result/MetadataTest.java +++ b/src/test/java/com/yahoo/bullet/result/MetadataTest.java @@ -8,7 +8,6 @@ import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.parsing.Error; import com.yahoo.bullet.result.Metadata.Concept; -import org.apache.commons.lang3.tuple.Pair; import org.testng.Assert; import org.testng.annotations.Test; @@ -16,10 +15,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; @@ -119,36 +116,4 @@ public void testMerging() { Assert.assertEquals(metaA.asMap(), expected); } - - @Test - public void testConceptKeyExtractionWithMetadataNotEnabled() { - Map configuration = new HashMap<>(); - configuration.put(BulletConfig.RESULT_METADATA_METRICS, asMetadataEntries(Pair.of("Estimated Result", "foo"))); - - Set concepts = new HashSet<>(singletonList(Concept.ESTIMATED_RESULT)); - - Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), Collections.emptyMap()); - } - - @Test - public void testConceptKeyExtraction() { - Map configuration = new HashMap<>(); - configuration.put(BulletConfig.RESULT_METADATA_ENABLE, true); - configuration.put(BulletConfig.RESULT_METADATA_METRICS, - asMetadataEntries(Pair.of("Estimated Result", "foo"), - Pair.of("Sketch Metadata", "bar"), - Pair.of("Non Existent", "bar"), - Pair.of("Standard Deviations", "baz"))); - - Set concepts = new HashSet<>(asList(Concept.ESTIMATED_RESULT, - Concept.SKETCH_METADATA, - Concept.STANDARD_DEVIATIONS)); - - Map expectedMap = new HashMap<>(); - expectedMap.put(Concept.ESTIMATED_RESULT.getName(), "foo"); - expectedMap.put(Concept.SKETCH_METADATA.getName(), "bar"); - expectedMap.put(Concept.STANDARD_DEVIATIONS.getName(), "baz"); - - Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), expectedMap); - } } diff --git a/src/test/resources/log4j.properties b/src/test/resources/log4j.properties index f4ff15dc..1a7614c0 100644 --- a/src/test/resources/log4j.properties +++ b/src/test/resources/log4j.properties @@ -1,5 +1,5 @@ # Root logger option -log4j.rootLogger=FATAL, stdout +log4j.rootLogger=FATAL,stdout # Direct log messages to stdout log4j.appender.stdout=org.apache.log4j.ConsoleAppender From 39a6cab4acd744fd5af0320f4f82c809efc6116a Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Mon, 20 Nov 2017 19:24:48 -0800 Subject: [PATCH 4/6] Fixing all tests --- .../java/com/yahoo/bullet/BulletConfig.java | 2 +- src/main/java/com/yahoo/bullet/Utilities.java | 2 +- src/main/java/com/yahoo/bullet/Validator.java | 5 +++++ .../aggregations/DistributionTest.java | 5 +++-- .../aggregations/grouping/GroupDataTest.java | 1 - .../yahoo/bullet/querying/FilterQueryTest.java | 17 +++++++++++------ 6 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index cadb5d97..9d24d6cb 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -62,7 +62,7 @@ public class BulletConfig extends Config { public static final int DEFAULT_AGGREGATION_MAX_SIZE = 512; public static final String DEFAULT_AGGREGATION_COMPOSITE_FIELD_SEPARATOR = "|"; - public static final int DEFAULT_RAW_AGGREGATION_MAX_SIZE = 30; + public static final int DEFAULT_RAW_AGGREGATION_MAX_SIZE = 100; public static final int DEFAULT_RAW_AGGREGATION_MICRO_BATCH_SIZE = 1; public static final int DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES = 16384; diff --git a/src/main/java/com/yahoo/bullet/Utilities.java b/src/main/java/com/yahoo/bullet/Utilities.java index 11ead899..c67c96c8 100644 --- a/src/main/java/com/yahoo/bullet/Utilities.java +++ b/src/main/java/com/yahoo/bullet/Utilities.java @@ -19,7 +19,7 @@ public class Utilities { * @param The type to get the object as. * @return The casted object of type U or null if the cast could not be done. */ - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") public static U getCasted(Object entry, Class clazz) { try { return clazz.cast(entry); diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java index c41b84b0..308ecc22 100644 --- a/src/main/java/com/yahoo/bullet/Validator.java +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -1,3 +1,8 @@ +/* + * Copyright 2017, Yahoo Inc. + * Licensed under the terms of the Apache License, Version 2.0. + * See the LICENSE file associated with the project for terms. + */ package com.yahoo.bullet; import lombok.Getter; diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java index fc95c400..6761b2a1 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java @@ -451,9 +451,10 @@ public void testCasting() { @Test public void testNegativeSize() { - // We will default to MAX_POINTS is configured to -1 and we will default to Distribution.DEFAULT_SIZE which is 1 + // MAX_POINTS is configured to -1 and we will use the min BulletConfig.DEFAULT_DISTRIBUTION_AGGREGATION_MAX_POINTS + // and aggregation size, which is 1 Distribution distribution = makeDistribution(makeConfiguration(-1, 128), makeAttributes(DistributionType.PMF, 10L), - "field", 10, ALL_METADATA); + "field", 1, ALL_METADATA); IntStream.range(0, 100).mapToDouble(i -> i).mapToObj(d -> RecordBox.get().add("field", d).getRecord()) .forEach(distribution::consume); diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java index 1c691253..1a06c5ec 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/grouping/GroupDataTest.java @@ -8,7 +8,6 @@ import com.yahoo.bullet.BulletConfig; import com.yahoo.bullet.operations.AggregationOperations.GroupOperationType; import com.yahoo.bullet.operations.SerializerDeserializer; -import com.yahoo.bullet.parsing.Aggregation; import com.yahoo.bullet.record.BulletRecord; import com.yahoo.bullet.result.RecordBox; import org.apache.commons.lang3.tuple.Pair; diff --git a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java index 073e5c51..749f0a88 100644 --- a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java +++ b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java @@ -15,20 +15,25 @@ import org.testng.annotations.Test; import java.util.Arrays; +import java.util.Collections; +import java.util.Map; import static com.yahoo.bullet.TestHelpers.getListBytes; import static com.yahoo.bullet.parsing.QueryUtils.getFilterQuery; import static com.yahoo.bullet.parsing.QueryUtils.makeAggregationQuery; import static com.yahoo.bullet.parsing.QueryUtils.makeProjectionFilterQuery; import static com.yahoo.bullet.parsing.QueryUtils.makeRawFullQuery; -import static java.util.Collections.emptyMap; public class FilterQueryTest { + private static Map configWithNoTimestamp() { + return Collections.singletonMap(BulletConfig.RECORD_INJECT_TIMESTAMP, false); + } + @Test public void testFilteringProjection() { FilterQuery query = getFilterQuery(makeProjectionFilterQuery("map_field.id", Arrays.asList("1", "23"), FilterType.EQUALS, Pair.of("map_field.id", "mid")), - emptyMap()); + configWithNoTimestamp()); RecordBox boxA = RecordBox.get().addMap("map_field", Pair.of("id", "3")); Assert.assertFalse(query.consume(boxA.getRecord())); Assert.assertNull(query.getData()); @@ -43,8 +48,7 @@ public void testFilteringProjection() { public void testNoAggregationAttempted() { FilterQuery query = getFilterQuery(makeRawFullQuery("map_field.id", Arrays.asList("1", "23"), FilterType.EQUALS, AggregationType.RAW, BulletConfig.DEFAULT_AGGREGATION_MAX_SIZE, - Pair.of("map_field.id", "mid")), - emptyMap()); + Pair.of("map_field.id", "mid")), configWithNoTimestamp()); RecordBox boxA = RecordBox.get().addMap("map_field", Pair.of("id", "23")); RecordBox expectedA = RecordBox.get().add("mid", "23"); @@ -63,7 +67,7 @@ public void testNoAggregationAttempted() { @Test public void testMaximumEmitted() { - FilterQuery query = getFilterQuery(makeAggregationQuery(AggregationType.RAW, 2), emptyMap()); + FilterQuery query = getFilterQuery(makeAggregationQuery(AggregationType.RAW, 2), configWithNoTimestamp()); RecordBox box = RecordBox.get(); Assert.assertTrue(query.consume(box.getRecord())); Assert.assertEquals(query.getData(), getListBytes(box.getRecord())); @@ -78,7 +82,8 @@ public void testMaximumEmitted() { @Test public void testMaximumEmittedWithNonMatchingRecords() { FilterQuery query = getFilterQuery(makeRawFullQuery("mid", Arrays.asList("1", "23"), FilterType.EQUALS, - AggregationType.RAW, 2, Pair.of("mid", "mid")), emptyMap()); + AggregationType.RAW, 2, Pair.of("mid", "mid")), + configWithNoTimestamp()); RecordBox boxA = RecordBox.get().add("mid", "23"); RecordBox expectedA = RecordBox.get().add("mid", "23"); From 9967fc4cfebee48f8dca710203587a19a1e06c26 Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Tue, 21 Nov 2017 19:37:48 -0800 Subject: [PATCH 5/6] Tests --- .../java/com/yahoo/bullet/BulletConfig.java | 103 ++++-- src/main/java/com/yahoo/bullet/Validator.java | 53 ++- .../com/yahoo/bullet/BulletConfigTest.java | 192 ++++++++-- .../java/com/yahoo/bullet/ValidatorTest.java | 337 ++++++++++++++++++ .../yahoo/bullet/parsing/AggregationTest.java | 11 + 5 files changed, 637 insertions(+), 59 deletions(-) create mode 100644 src/test/java/com/yahoo/bullet/ValidatorTest.java diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index 9d24d6cb..5b3dd544 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -5,9 +5,14 @@ */ package com.yahoo.bullet; +import com.yahoo.bullet.pubsub.PubSub.Context; import com.yahoo.bullet.result.Metadata; +import com.yahoo.bullet.result.Metadata.Concept; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -82,28 +87,27 @@ public class BulletConfig extends Config { public static final String DEFAULT_TOP_K_AGGREGATION_SKETCH_ERROR_TYPE = "NFN"; public static final boolean DEFAULT_RESULT_METADATA_ENABLE = true; - // This is a Map for simplicity. The YAML is a list. We ensure that the names are unique. The - // final Metadata in the result is a map so if the names are not unique, they get overridden. - public static final Map DEFAULT_RESULT_METADATA_METRICS = new HashMap<>(); - static { - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_ID.getName(), "query_id"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_BODY.getName(), "query"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_CREATION_TIME.getName(), "query_receive_time"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.QUERY_TERMINATION_TIME.getName(), "query_finish_time"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.SKETCH_METADATA.getName(), "sketches"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ESTIMATED_RESULT.getName(), "was_estimated"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.STANDARD_DEVIATIONS.getName(), "standard_deviations"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.FAMILY.getName(), "family"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.SIZE.getName(), "size"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.THETA.getName(), "theta"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.UNIQUES_ESTIMATE.getName(), "uniques_estimate"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MINIMUM_VALUE.getName(), "minimum_value"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MAXIMUM_VALUE.getName(), "maximum_value"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ITEMS_SEEN.getName(), "items_seen"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.NORMALIZED_RANK_ERROR.getName(), "normalized_rank_error"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.MAXIMUM_COUNT_ERROR.getName(), "maximum_count_error"); - DEFAULT_RESULT_METADATA_METRICS.put(Metadata.Concept.ACTIVE_ITEMS.getName(), "active_items"); - } + public static final List> DEFAULT_RESULT_METADATA_METRICS = + makeMetadata(ImmutablePair.of(Concept.QUERY_ID, "query_id"), + ImmutablePair.of(Concept.QUERY_BODY, "query"), + ImmutablePair.of(Concept.QUERY_CREATION_TIME, "query_receive_time"), + ImmutablePair.of(Concept.QUERY_TERMINATION_TIME, "query_finish_time"), + ImmutablePair.of(Concept.SKETCH_METADATA, "sketches"), + ImmutablePair.of(Concept.ESTIMATED_RESULT, "was_estimated"), + ImmutablePair.of(Concept.STANDARD_DEVIATIONS, "standard_deviations"), + ImmutablePair.of(Concept.FAMILY, "family"), + ImmutablePair.of(Concept.SIZE, "size"), + ImmutablePair.of(Concept.THETA, "theta"), + ImmutablePair.of(Concept.UNIQUES_ESTIMATE, "uniques_estimate"), + ImmutablePair.of(Concept.MINIMUM_VALUE, "minimum_value"), + ImmutablePair.of(Concept.MAXIMUM_VALUE, "maximum_value"), + ImmutablePair.of(Concept.ITEMS_SEEN, "items_seen"), + ImmutablePair.of(Concept.NORMALIZED_RANK_ERROR, "normalized_rank_error"), + ImmutablePair.of(Concept.MAXIMUM_COUNT_ERROR, "maximum_count_error"), + ImmutablePair.of(Concept.ACTIVE_ITEMS, "active_items")); + + public static final String DEFAULT_PUBSUB_CONTEXT_NAME = Context.QUERY_PROCESSING.name(); + public static final String DEFAULT_PUBSUB_CLASS_NAME = "com.yahoo.bullet.pubsub.MockPubSub"; // It is ok for this to be static since the VALIDATOR itself does not change for different values for fields // in the BulletConfig. @@ -203,8 +207,16 @@ public class BulletConfig extends Config { VALIDATOR.define(RESULT_METADATA_METRICS) .defaultTo(DEFAULT_RESULT_METADATA_METRICS) .checkIf(Validator::isList) + .checkIf(BulletConfig::isMetadata) .castTo(BulletConfig::mapifyMetadata); + VALIDATOR.define(PUBSUB_CONTEXT_NAME) + .defaultTo(DEFAULT_PUBSUB_CONTEXT_NAME) + .checkIf(Validator.isIn(String.class, Context.QUERY_PROCESSING.name(), Context.QUERY_SUBMISSION.name())); + VALIDATOR.define(PUBSUB_CLASS_NAME) + .defaultTo(DEFAULT_PUBSUB_CLASS_NAME) + .checkIf(Validator::isString); + VALIDATOR.relate("Max should be less or equal to default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) .checkIf(Validator::isGreaterOrEqual); VALIDATOR.relate("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) @@ -262,19 +274,39 @@ public static void validate(BulletConfig config) { @SuppressWarnings("unchecked") private static Object mapifyMetadata(Object metadata) { - List keys = (List) metadata; + List entries = (List) metadata; Map mapping = new HashMap<>(); - // For each metric configured, load the name of the field to add it to the metadata as. - for (Map m : keys) { - String concept = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY); - String name = (String) m.get(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY); - if (Metadata.KNOWN_CONCEPTS.contains(Metadata.Concept.from(concept))) { - mapping.put(concept, name); - } + // For each metadata entry that is configured, load the name of the field to add it to the metadata as. + for (Map m : entries) { + mapping.put((String) m.get(RESULT_METADATA_METRICS_CONCEPT_KEY), + (String) m.get(RESULT_METADATA_METRICS_NAME_KEY)); } return mapping; } + @SuppressWarnings("unchecked") + private static boolean isMetadata(Object metadata) { + try { + for (Map m : (List) metadata) { + if (m.size() != 2) { + log.warn("Metadata should only contain the keys {}, {}. Found {}", RESULT_METADATA_METRICS_CONCEPT_KEY, + RESULT_METADATA_METRICS_NAME_KEY, m); + return false; + } + String concept = (String) m.get(RESULT_METADATA_METRICS_CONCEPT_KEY); + String name = (String) m.get(RESULT_METADATA_METRICS_CONCEPT_KEY); + if (!Metadata.KNOWN_CONCEPTS.contains(Concept.from(concept))) { + log.warn("Unknown metadata concept: {}", concept); + return false; + } + } + } catch (ClassCastException e) { + log.warn("Metadata should be a list containing maps of string keys and values. Found {}", metadata); + return false; + } + return true; + } + private static boolean isMetadataConfigured(Object enabled, Object keys) { // This function should return false when metadata is enabled but keys are not set. boolean isMetadataOff = !((Boolean) enabled); @@ -286,5 +318,16 @@ private static boolean isMetadataNecessary(Object enabled, Object keys) { boolean isMetadataOn = (Boolean) enabled; return isMetadataOn || keys == null; } + + private static List> makeMetadata(Pair... entries) { + List> metadataList = new ArrayList<>(); + for (Pair entry : entries) { + Map metadata = new HashMap<>(); + metadata.put(RESULT_METADATA_METRICS_CONCEPT_KEY, entry.getKey().getName()); + metadata.put(RESULT_METADATA_METRICS_NAME_KEY, entry.getValue()); + metadataList.add(metadata); + } + return metadataList; + } } diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java index 308ecc22..e998b020 100644 --- a/src/main/java/com/yahoo/bullet/Validator.java +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -5,14 +5,16 @@ */ package com.yahoo.bullet; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.function.BiPredicate; import java.util.function.Function; import java.util.function.Predicate; @@ -20,6 +22,9 @@ /** * This class validates instances of {@link BulletConfig}. Use {@link com.yahoo.bullet.Validator.Entry} to define * fields and {@link com.yahoo.bullet.Validator.Relationship} to define relationships between them. + * + * It also provides a bunch of useful {@link Predicate} and {@link BiPredicate} for use in the checks for the entries + * and relationships and type converting {@link Function} for converting types of entries. */ @Slf4j public class Validator { @@ -35,7 +40,6 @@ public class Validator { public class Entry { private String key; private Predicate validation; - @Getter private Object defaultValue; private Function adapter; @@ -84,6 +88,18 @@ public Entry castTo(Function adapter) { return this; } + /** + * Get the defaultValue in the Entry after applying the cast, if present. + * + * @return The defaultValue after any casts. + */ + public Object getDefaultValue() { + if (adapter == null) { + return defaultValue; + } + return adapter.apply(defaultValue); + } + /** * Normalizes a {@link BulletConfig} by validating, apply defaults and converting the object represented by the * field in this Entry. @@ -112,9 +128,7 @@ public void normalize(BulletConfig config) { * {@link Entry#defaultTo(Object)}) if the check fails. */ public class Relationship { - @Getter private String keyA; - @Getter private String keyB; private String description; private BiPredicate binaryRelation; @@ -176,8 +190,8 @@ public void normalize(BulletConfig config) { } } - private Map entries = new HashMap<>(); - private List relations = new ArrayList<>(); + private final Map entries = new HashMap<>(); + private final List relations = new ArrayList<>(); /** * Creates an instance of the Entry using the name of the field. This field by default will pass the @@ -255,6 +269,16 @@ public static Object asDouble(Object value) { return ((Number) value).doubleValue(); } + /** + * This casts an Object to an {@link String}. + * + * @param value The value to cast. + * @return The converted String object. + */ + public static Object asString(Object value) { + return value.toString(); + } + // Unary Predicates /** @@ -382,6 +406,23 @@ public static boolean isPowerOfTwo(Object value) { return (toCheck & toCheck - 1) == 0; } + // Unary Predicate Generators + + /** + * Creates a {@link Predicate} that checks to see if the given object is in the list of values. + * + * @param type The class of the object whose membership is being tested. + * @param values The values that the object could be equal to that is being tested. + * @param The type of the values and the object. + * @return A predicate that checks to see if the object provided to it is of the given type and is in the given values. + */ + public static Predicate isIn(Class type, T... values) { + Objects.requireNonNull(type); + Objects.requireNonNull(values); + Set set = new HashSet<>(Arrays.asList(values)); + return o -> isType(o, type) && set.contains(o); + } + // Binary Predicates. /** diff --git a/src/test/java/com/yahoo/bullet/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/BulletConfigTest.java index fe5eb45a..465677b4 100644 --- a/src/test/java/com/yahoo/bullet/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/BulletConfigTest.java @@ -5,17 +5,46 @@ */ package com.yahoo.bullet; +import com.yahoo.bullet.result.Metadata; +import com.yahoo.bullet.result.Metadata.Concept; import org.testng.Assert; import org.testng.annotations.Test; +import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import static com.yahoo.bullet.TestHelpers.assertJSONEquals; + public class BulletConfigTest { + private static Map allMetadataAsMap() { + Map meta = new HashMap<>(); + for (Map m : BulletConfig.DEFAULT_RESULT_METADATA_METRICS) { + meta.put(m.get(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY), m.get(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY)); + } + return meta; + } + + private static class CustomConfig extends BulletConfig { + private static final Validator CUSTOM = new Validator(); + static { + CUSTOM.define("foo").defaultTo(42).checkIf(Validator::isPositiveInt); + CUSTOM.define("bar").defaultTo(0.4).checkIf(Validator::isPositive); + CUSTOM.relate("foo > bar", "foo", "bar").checkIf(Validator::isGreaterOrEqual); + } + + @Override + public BulletConfig validate() { + CUSTOM.normalize(this); + return super.validate(); + } + } + @Test public void testNoFiles() { BulletConfig config = new BulletConfig(); @@ -207,37 +236,154 @@ public void testMissingRequiredConfig() { config.getRequiredConfigAs("does.not.exist", Long.class); } - /* @Test - public void testConceptKeyExtractionWithMetadataNotEnabled() { - Map configuration = new HashMap<>(); - configuration.put(BulletConfig.RESULT_METADATA_METRICS, asMetadataEntries(Pair.of("Estimated Result", "foo"))); + public void testMetadataConversion() { + List> metadata = new ArrayList<>(); + Map expected = new HashMap<>(); + for (Concept concept : Metadata.KNOWN_CONCEPTS) { + Map entry = new HashMap<>(); + String name = concept.getName(); + String key = concept.getName().substring(0, 3); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, name); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, key); + metadata.add(entry); + expected.put(name, key); + } + + BulletConfig config = new BulletConfig().validate(); + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), allMetadataAsMap()); + + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), expected); + } - Set concepts = new HashSet<>(singletonList(Concept.ESTIMATED_RESULT)); + @Test + public void testUnknownMetadata() { + List> metadata = new ArrayList<>(); + Map expected = new HashMap<>(); + for (Concept concept : Arrays.asList(Concept.QUERY_ID, Concept.ITEMS_SEEN)) { + Map entry = new HashMap<>(); + String name = concept.getName(); + String key = concept.getName().substring(0, 3); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, name); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, key); + metadata.add(entry); + expected.put(name, key); + } - Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), Collections.emptyMap()); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), expected); + + // Add an unknown one + Map entry = new HashMap<>(); + entry.put("foo", "bar"); + entry.put("baz", "qux"); + metadata.add(entry); + + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + // Now it's all defaults + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), allMetadataAsMap()); } @Test - public void testConceptKeyExtraction() { - Map configuration = new HashMap<>(); - configuration.put(BulletConfig.RESULT_METADATA_ENABLE, true); - configuration.put(BulletConfig.RESULT_METADATA_METRICS, - asMetadataEntries(Pair.of("Estimated Result", "foo"), - Pair.of("Sketch Metadata", "bar"), - Pair.of("Non Existent", "bar"), - Pair.of("Standard Deviations", "baz"))); + public void testBadMetadata() { + List> metadata = new ArrayList<>(); + Map expected = new HashMap<>(); + for (Concept concept : Arrays.asList(Concept.QUERY_ID, Concept.ITEMS_SEEN)) { + Map entry = new HashMap<>(); + String name = concept.getName(); + String key = concept.getName().substring(0, 3); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, name); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, key); + metadata.add(entry); + expected.put(name, key); + } - Set concepts = new HashSet<>(asList(Concept.ESTIMATED_RESULT, - Concept.SKETCH_METADATA, - Concept.STANDARD_DEVIATIONS)); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), expected); + + // Add a badly typed one + Map entry = new HashMap<>(); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, new ArrayList<>()); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, new ArrayList<>()); + metadata.add(entry); + + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + // Now it's all defaults + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), allMetadataAsMap()); + } - Map expectedMap = new HashMap<>(); - expectedMap.put(Concept.ESTIMATED_RESULT.getName(), "foo"); - expectedMap.put(Concept.SKETCH_METADATA.getName(), "bar"); - expectedMap.put(Concept.STANDARD_DEVIATIONS.getName(), "baz"); + @Test + public void testIncompleteMetadata() { + List> metadata = new ArrayList<>(); + Map expected = new HashMap<>(); + for (Concept concept : Arrays.asList(Concept.QUERY_ID, Concept.ITEMS_SEEN)) { + Map entry = new HashMap<>(); + String name = concept.getName(); + String key = concept.getName().substring(0, 3); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, name); + entry.put(BulletConfig.RESULT_METADATA_METRICS_NAME_KEY, key); + metadata.add(entry); + expected.put(name, key); + } - Assert.assertEquals(Metadata.getConceptNames(configuration, concepts), expectedMap); + BulletConfig config = new BulletConfig(); + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), expected); + + // Add only one entry + Map entry = new HashMap<>(); + entry.put(BulletConfig.RESULT_METADATA_METRICS_CONCEPT_KEY, Concept.QUERY_ID.getName()); + metadata.add(entry); + + config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); + config.validate(); + // Now it's all defaults + Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), allMetadataAsMap()); + } + + @Test + public void testStringification() { + BulletConfig config = new BulletConfig(); + config.clear(); + String key = BulletConfig.AGGREGATION_DEFAULT_SIZE; + config.set(key, 20); + assertJSONEquals(config.toString(), "{'" + key + "': 20 }"); + } + + @Test + public void testCustomConfigValidation() { + CustomConfig config = new CustomConfig(); + Assert.assertNull(config.get("foo")); + Assert.assertNull(config.get("bar")); + Assert.assertEquals(config.get(BulletConfig.AGGREGATION_DEFAULT_SIZE), (long) BulletConfig.DEFAULT_AGGREGATION_SIZE); + + config.set("foo", 42); + config.set("bar", 10.1); + config.validate(); + Assert.assertEquals(config.get("foo"), 42); + Assert.assertEquals(config.get("bar"), 10.1); + + config.set("foo", 4.2); + config.set("bar", 12); + config.validate(); + // Entry defaults before relationship + Assert.assertEquals(config.get("foo"), 42); + Assert.assertEquals(config.get("bar"), 12); + + config.set("foo", 13); + config.set("bar", 16); + config.validate(); + // Relationship defaults both + Assert.assertEquals(config.get("foo"), 42); + Assert.assertEquals(config.get("bar"), 0.4); } - */ } diff --git a/src/test/java/com/yahoo/bullet/ValidatorTest.java b/src/test/java/com/yahoo/bullet/ValidatorTest.java new file mode 100644 index 00000000..94b2abe0 --- /dev/null +++ b/src/test/java/com/yahoo/bullet/ValidatorTest.java @@ -0,0 +1,337 @@ +/* + * Copyright 2017, Yahoo Inc. + * Licensed under the terms of the Apache License, Version 2.0. + * See the LICENSE file associated with the project for terms. + */ +package com.yahoo.bullet; + +import com.yahoo.bullet.Validator.Entry; +import com.yahoo.bullet.Validator.Relationship; +import org.testng.Assert; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.Collections; +import java.util.List; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; + +public class ValidatorTest { + private BulletConfig empty; + + @BeforeMethod + public void setup() { + empty = new BulletConfig(); + empty.clear(); + } + + @Test + public void testDefaultEntry() { + Validator validator = new Validator(); + Entry entry = validator.define("foo"); + empty.set("foo", "bar"); + entry.normalize(empty); + + Assert.assertEquals(empty.get("foo"), "bar"); + Assert.assertNull(entry.getDefaultValue()); + } + + @Test + public void testEntryDefaulting() { + Validator validator = new Validator(); + Entry entry = validator.define("foo").checkIf(Validator::isNotNull).defaultTo("baz").castTo(Validator::asString); + + Assert.assertNull(empty.get("foo")); + entry.normalize(empty); + Assert.assertEquals(empty.get("foo"), "baz"); + Assert.assertEquals(entry.getDefaultValue(), "baz"); + + // Check to see if type conversion is done for default of the non-final type. + entry.defaultTo(1); + Assert.assertEquals(entry.getDefaultValue(), "1"); + } + + @Test + public void testEntryCasting() { + Validator validator = new Validator(); + Entry entry = validator.define("foo").castTo(Validator::asInt); + empty.set("foo", 1.035f); + entry.normalize(empty); + + Assert.assertEquals(empty.get("foo").getClass(), Integer.class); + Assert.assertEquals(empty.get("foo"), 1); + } + + @Test + public void testEntryMultipleChecks() { + Validator validator = new Validator(); + Entry entry = validator.define("foo") + .checkIf(Validator::isNotNull).checkIf(Validator.isIn(String.class, "baz", "bar")) + .defaultTo("qux"); + + Assert.assertNull(empty.get("foo")); + + entry.normalize(empty); + Assert.assertEquals(empty.get("foo"), "qux"); + + empty.set("foo", "baz"); + entry.normalize(empty); + Assert.assertEquals(empty.get("foo"), "baz"); + + empty.set("foo", "bar"); + entry.normalize(empty); + Assert.assertEquals(empty.get("foo"), "bar"); + + empty.set("foo", "foo"); + entry.normalize(empty); + Assert.assertEquals(empty.get("foo"), "qux"); + } + + @Test + public void testIsNotNull() { + Assert.assertTrue(Validator.isNotNull("foo")); + Assert.assertFalse(Validator.isNotNull(null)); + } + + @Test + public void testisType() { + Assert.assertTrue(Validator.isType("foo", String.class)); + Assert.assertTrue(Validator.isType(1, Integer.class)); + Assert.assertTrue(Validator.isType(1L, Long.class)); + Assert.assertTrue(Validator.isType(asList("foo", "bar"), List.class)); + Assert.assertFalse(Validator.isType(0, Float.class)); + } + + @Test + public void testIsBoolean() { + Assert.assertTrue(Validator.isBoolean(true)); + Assert.assertTrue(Validator.isBoolean(false)); + Assert.assertFalse(Validator.isBoolean("foo")); + } + + @Test + public void testIsString() { + Assert.assertTrue(Validator.isString("foo")); + Assert.assertFalse(Validator.isString(null)); + Assert.assertFalse(Validator.isString(1.0)); + } + + @Test + public void testIsFloat() { + Assert.assertTrue(Validator.isFloat(1.34)); + Assert.assertTrue(Validator.isFloat(1.34f)); + Assert.assertFalse(Validator.isFloat(2)); + } + + @Test + public void testIsInt() { + Assert.assertTrue(Validator.isInt(1)); + Assert.assertTrue(Validator.isInt(3L)); + Assert.assertFalse(Validator.isInt(2.3)); + } + + @Test + public void testIsNumber() { + Assert.assertTrue(Validator.isNumber(1.3)); + Assert.assertTrue(Validator.isNumber(1)); + Assert.assertTrue(Validator.isNumber(42L)); + Assert.assertTrue(Validator.isNumber(4.2f)); + Assert.assertFalse(Validator.isNumber("foo")); + } + + @Test + public void testIsPositive() { + Assert.assertTrue(Validator.isPositive(2.4)); + Assert.assertTrue(Validator.isPositive(2)); + Assert.assertTrue(Validator.isPositive(0.02)); + Assert.assertFalse(Validator.isPositive(-0.3)); + } + + @Test + public void testIsPositiveInt() { + Assert.assertTrue(Validator.isPositiveInt(1)); + Assert.assertTrue(Validator.isPositiveInt(2L)); + Assert.assertFalse(Validator.isPositiveInt(0.3)); + Assert.assertFalse(Validator.isPositiveInt(0L)); + Assert.assertFalse(Validator.isPositiveInt(0)); + Assert.assertFalse(Validator.isPositiveInt(-10)); + Assert.assertFalse(Validator.isPositiveInt(-10.3)); + } + + @Test + public void testIsPowerOfTwo() { + Assert.assertTrue(Validator.isPowerOfTwo(1)); + Assert.assertTrue(Validator.isPowerOfTwo(2)); + Assert.assertTrue(Validator.isPowerOfTwo(4)); + Assert.assertTrue(Validator.isPowerOfTwo(16384)); + Assert.assertFalse(Validator.isPowerOfTwo(3)); + Assert.assertFalse(Validator.isPowerOfTwo(2.0)); + Assert.assertFalse(Validator.isPowerOfTwo(-4)); + } + + @Test + public void testIsMap() { + Assert.assertTrue(Validator.isMap(Collections.singletonMap("foo", "bar"))); + Assert.assertFalse(Validator.isMap("foo")); + } + + @Test + public void testIsIn() { + Assert.assertTrue(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("foo")); + Assert.assertTrue(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("qux")); + Assert.assertFalse(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("f")); + Assert.assertFalse(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test(1.0)); + + Assert.assertTrue(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(asList("foo", "bar"))); + Assert.assertTrue(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(asList("baz", "qux"))); + Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(singletonList("baz"))); + Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(singletonList("foo"))); + Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(null)); + Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test("foo")); + } + + @Test + public void testIntegerConversion() { + Assert.assertEquals(Validator.asInt(3.4), 3); + Assert.assertEquals(Validator.asInt(3), 3); + Assert.assertEquals(Validator.asInt(3L), 3); + Assert.assertEquals(Validator.asInt(3.4f), 3); + } + + @Test(expectedExceptions = ClassCastException.class) + public void testIntegerConversionFailure() { + Validator.asInt("foo"); + } + + @Test + public void testFloatConversion() { + Assert.assertEquals(Validator.asFloat(3.0), 3.0f); + Assert.assertEquals(Validator.asFloat(3), 3.0f); + Assert.assertEquals(Validator.asFloat(3L), 3.0f); + Assert.assertEquals(Validator.asFloat(3.0f), 3.0f); + } + + @Test(expectedExceptions = ClassCastException.class) + public void testFloatConversionFailure() { + Validator.asFloat("foo"); + } + + @Test + public void testDoubleConversion() { + Assert.assertEquals(Validator.asDouble(3.0), 3.0); + Assert.assertEquals(Validator.asDouble(3), 3.0); + Assert.assertEquals(Validator.asDouble(3L), 3.0); + Assert.assertEquals(Validator.asDouble(3.0f), 3.0); + } + + @Test(expectedExceptions = ClassCastException.class) + public void testDoubleConversionFailure() { + Validator.asDouble("foo"); + } + + @Test + public void testStringConversion() { + Assert.assertEquals(Validator.asString("foo"), "foo"); + Assert.assertEquals(Validator.asString(1.0), "1.0"); + Assert.assertEquals(Validator.asString(asList("foo", "bar")), asList("foo", "bar").toString()); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testStringConversionFailure() { + Validator.asString(null); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testRelationshipWithoutEntry() { + Validator validator = new Validator(); + validator.relate("Won't be created", "foo", "bar"); + } + + @Test + public void testRelationshipDefaulting() { + Validator validator = new Validator(); + validator.define("foo"); + validator.define("bar"); + + Relationship relation = validator.relate("Test", "foo", "bar"); + + // Nothing happens + empty.set("foo", 0.2); + empty.set("bar", "baz"); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 0.2); + Assert.assertEquals(empty.get("bar"), "baz"); + } + + @Test + public void testRelationshipFailureUsingEntryDefaults() { + Validator validator = new Validator(); + validator.define("foo").defaultTo(0); + validator.define("bar").defaultTo(42); + + Relationship relation = validator.relate("Test", "foo", "bar").checkIf(Validator::isGreaterOrEqual); + + empty.set("foo", -1L); + empty.set("bar", 0.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 0); + Assert.assertEquals(empty.get("bar"), 42); + + empty.set("foo", 42); + empty.set("bar", 4.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 4.2); + } + + @Test + public void testRelationshipWithMultipleChecks() { + Validator validator = new Validator(); + validator.define("foo").defaultTo(42); + validator.define("bar").defaultTo(11).castTo(Validator::asDouble); + + Relationship relation = validator.relate("Test", "foo", "bar").checkIf(Validator::isGreaterOrEqual) + .checkIf((a, b) -> ((Number) b).intValue() > 10); + + empty.set("foo", -1L); + empty.set("bar", 0.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 11.0); + + empty.set("foo", 42); + empty.set("bar", 4.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 11.0); + + empty.set("foo", 42); + empty.set("bar", 40.3); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 40.3); + } + + @Test + public void testRelationshipFailureUsesCustomDefaults() { + Validator validator = new Validator(); + validator.define("foo").defaultTo(0); + validator.define("bar").defaultTo(42); + + Relationship relation = validator.relate("Test", "foo", "bar").checkIf(Validator::isGreaterOrEqual); + relation.orElseUse("qux", "norf"); + + empty.set("foo", 42); + empty.set("bar", 4.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), 42); + Assert.assertEquals(empty.get("bar"), 4.2); + + empty.set("foo", -1L); + empty.set("bar", 0.2); + relation.normalize(empty); + Assert.assertEquals(empty.get("foo"), "qux"); + Assert.assertEquals(empty.get("bar"), "norf"); + } +} diff --git a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java index 6a7f825e..53dc26c9 100644 --- a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java @@ -14,6 +14,7 @@ import com.yahoo.bullet.operations.aggregations.GroupBy; import com.yahoo.bullet.operations.aggregations.Raw; import com.yahoo.bullet.operations.aggregations.Strategy; +import com.yahoo.bullet.operations.aggregations.TopK; import com.yahoo.bullet.operations.aggregations.grouping.GroupOperation; import org.testng.Assert; import org.testng.annotations.Test; @@ -310,4 +311,14 @@ public void testDistributionStrategy() { Assert.assertEquals(aggregation.getStrategy().getClass(), Distribution.class); } + + @Test + public void testTopKStrategy() { + Aggregation aggregation = new Aggregation(); + aggregation.setType(AggregationOperations.AggregationType.TOP_K); + aggregation.setFields(singletonMap("field", "foo")); + aggregation.configure(new BulletConfig().validate()); + + Assert.assertEquals(aggregation.getStrategy().getClass(), TopK.class); + } } From ebdb69cb2ce8dc618ce76ff9c3295014ff343a45 Mon Sep 17 00:00:00 2001 From: Akshai Sarma Date: Wed, 22 Nov 2017 14:17:55 -0800 Subject: [PATCH 6/6] Addressing review comments --- .../java/com/yahoo/bullet/BulletConfig.java | 56 +++++++++++-------- src/main/java/com/yahoo/bullet/Validator.java | 42 +++++++++----- .../com/yahoo/bullet/result/Metadata.java | 2 +- .../com/yahoo/bullet/BulletConfigTest.java | 42 +++++++------- .../java/com/yahoo/bullet/ValidatorTest.java | 40 +++++++++---- .../aggregations/DistributionTest.java | 8 +-- .../operations/aggregations/GroupAllTest.java | 2 +- .../yahoo/bullet/parsing/AggregationTest.java | 38 ++++++------- .../bullet/parsing/SpecificationTest.java | 12 ++-- .../bullet/querying/FilterQueryTest.java | 2 +- src/test/resources/test_config.yaml | 1 + 11 files changed, 143 insertions(+), 102 deletions(-) diff --git a/src/main/java/com/yahoo/bullet/BulletConfig.java b/src/main/java/com/yahoo/bullet/BulletConfig.java index 5b3dd544..9d27fcdc 100644 --- a/src/main/java/com/yahoo/bullet/BulletConfig.java +++ b/src/main/java/com/yahoo/bullet/BulletConfig.java @@ -109,6 +109,7 @@ public class BulletConfig extends Config { public static final String DEFAULT_PUBSUB_CONTEXT_NAME = Context.QUERY_PROCESSING.name(); public static final String DEFAULT_PUBSUB_CLASS_NAME = "com.yahoo.bullet.pubsub.MockPubSub"; + // Validator definitions for the configs in this class. // It is ok for this to be static since the VALIDATOR itself does not change for different values for fields // in the BulletConfig. private static final Validator VALIDATOR = new Validator(); @@ -151,38 +152,40 @@ public class BulletConfig extends Config { VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES) .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES) - .checkIf(Validator::isPositiveInt) + .checkIf(Validator::isPowerOfTwo) .castTo(Validator::asInt); VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING) .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_SAMPLING) - .checkIf(Validator::isPositive) .checkIf(Validator::isFloat) + .checkIf(Validator.isInRange(0.0, 1.0)) .castTo(Validator::asFloat); VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY) .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_FAMILY) .checkIf(Validator::isString); VALIDATOR.define(COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR) .defaultTo(DEFAULT_COUNT_DISTINCT_AGGREGATION_SKETCH_RESIZE_FACTOR) - .checkIf(Validator::isPositiveInt) + .checkIf(Validator::isPowerOfTwo) + .checkIf(Validator.isInRange(1, 8)) .castTo(Validator::asInt); VALIDATOR.define(GROUP_AGGREGATION_SKETCH_ENTRIES) - .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_ENTRIES) - .checkIf(Validator::isPositiveInt) - .castTo(Validator::asInt); + .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_ENTRIES) + .checkIf(Validator::isPowerOfTwo) + .castTo(Validator::asInt); VALIDATOR.define(GROUP_AGGREGATION_SKETCH_SAMPLING) - .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_SAMPLING) - .checkIf(Validator::isPositive) - .checkIf(Validator::isFloat) - .castTo(Validator::asFloat); + .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_SAMPLING) + .checkIf(Validator::isFloat) + .checkIf(Validator.isInRange(0.0, 1.0)) + .castTo(Validator::asFloat); VALIDATOR.define(GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR) .defaultTo(DEFAULT_GROUP_AGGREGATION_SKETCH_RESIZE_FACTOR) - .checkIf(Validator::isPositiveInt) + .checkIf(Validator::isPowerOfTwo) + .checkIf(Validator.isInRange(1, 8)) .castTo(Validator::asInt); VALIDATOR.define(DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES) .defaultTo(DEFAULT_DISTRIBUTION_AGGREGATION_SKETCH_ENTRIES) - .checkIf(Validator::isPositiveInt) + .checkIf(Validator::isPowerOfTwo) .castTo(Validator::asInt); VALIDATOR.define(DISTRIBUTION_AGGREGATION_MAX_POINTS) .defaultTo(DEFAULT_DISTRIBUTION_AGGREGATION_MAX_POINTS) @@ -217,13 +220,13 @@ public class BulletConfig extends Config { .defaultTo(DEFAULT_PUBSUB_CLASS_NAME) .checkIf(Validator::isString); - VALIDATOR.relate("Max should be less or equal to default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) + VALIDATOR.relate("Max should be >= default", SPECIFICATION_MAX_DURATION, SPECIFICATION_DEFAULT_DURATION) .checkIf(Validator::isGreaterOrEqual); - VALIDATOR.relate("Max should be less or equal to default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) + VALIDATOR.relate("Max should be >= default", AGGREGATION_MAX_SIZE, AGGREGATION_DEFAULT_SIZE) .checkIf(Validator::isGreaterOrEqual); - VALIDATOR.relate("Metadata is enabled and keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + VALIDATOR.relate("If metadata is enabled, keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) .checkIf(BulletConfig::isMetadataConfigured); - VALIDATOR.relate("Metadata is disabled and keys are defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) + VALIDATOR.relate("If metadata is disabled, keys are not defined", RESULT_METADATA_ENABLE, RESULT_METADATA_METRICS) .checkIf(BulletConfig::isMetadataNecessary) .orElseUse(false, Collections.emptyMap()); } @@ -232,28 +235,32 @@ public class BulletConfig extends Config { public static final String DEFAULT_CONFIGURATION_NAME = "bullet_defaults.yaml"; /** - * Constructor that loads specific file augmented with defaults. + * Constructor that loads specific file augmented with defaults and performs a {@link BulletConfig#validate()}. * * @param file YAML file to load. */ public BulletConfig(String file) { super(file, DEFAULT_CONFIGURATION_NAME); + validate(); } /** - * Constructor that loads just the defaults. + * Constructor that loads just the defaults and performs a {@link BulletConfig#validate()}. */ public BulletConfig() { super(DEFAULT_CONFIGURATION_NAME); + validate(); } /** * Validates and fixes configuration for this config. If there are undefaulted or wrongly typed elements, you * should use a {@link Validator} to define the appropriate definitions, casters and defaults to use. You - * should call this method before you use the config to ensure that all configurations are valid. This class - * defines a validator for all the fields it knows about. If you subclass it and define your own fields, you should - * create your own Validator and define entries and relationships that you need to validate. Make sure to call this - * method in your override if you wish validate the fields defined by this config. + * should call this method before you use the config if you set additional settings to ensure that all configurations + * are valid. + * + * This class defines a validator for all the fields it knows about. If you subclass it and define your own fields, + * you should create your own Validator and define entries and relationships that you need to validate. Make sure + * to call this method from your override if you wish re-validate the fields defined by this config. * * @return This config for chaining. */ @@ -266,10 +273,10 @@ public BulletConfig validate() { * Validates and fixes configuration for a given {@link BulletConfig}. This method checks, defaults and fixes the * various settings defined in this class. * - * @param config The {@link BulletConfig} to normalize. + * @param config The {@link BulletConfig} to validate. */ public static void validate(BulletConfig config) { - VALIDATOR.normalize(config); + VALIDATOR.validate(config); } @SuppressWarnings("unchecked") @@ -319,6 +326,7 @@ private static boolean isMetadataNecessary(Object enabled, Object keys) { return isMetadataOn || keys == null; } + @SafeVarargs private static List> makeMetadata(Pair... entries) { List> metadataList = new ArrayList<>(); for (Pair entry : entries) { diff --git a/src/main/java/com/yahoo/bullet/Validator.java b/src/main/java/com/yahoo/bullet/Validator.java index e998b020..f3caa2c3 100644 --- a/src/main/java/com/yahoo/bullet/Validator.java +++ b/src/main/java/com/yahoo/bullet/Validator.java @@ -104,9 +104,9 @@ public Object getDefaultValue() { * Normalizes a {@link BulletConfig} by validating, apply defaults and converting the object represented by the * field in this Entry. * - * @param config The config to normalize. + * @param config The config to validate. */ - public void normalize(BulletConfig config) { + void normalize(BulletConfig config) { Object value = config.get(key); boolean isValid = validation.test(value); if (!isValid) { @@ -123,7 +123,7 @@ public void normalize(BulletConfig config) { /** * This represents a binary relationship between two fields in a {@link BulletConfig}. You should have defined - * {@link Entry} for these fields before you try to define relationships between them. You can use this apply a + * {@link Entry} for these fields before you try to define relationships between them. You can use this to apply a * {@link BiPredicate} to these fields and provide or use their defined defaults (defined using * {@link Entry#defaultTo(Object)}) if the check fails. */ @@ -174,9 +174,9 @@ public void orElseUse(Object objectA, Object objectB) { * and uses the defaults (provided using {@link Relationship#orElseUse(Object, Object)} or the Entry defaults * for these fields if the check fails. * - * @param config The config to normalize. + * @param config The config to validate. */ - public void normalize(BulletConfig config) { + void normalize(BulletConfig config) { Object objectA = config.get(keyA); Object objectB = config.get(keyB); boolean isValid = binaryRelation.test(objectA, objectB); @@ -228,11 +228,11 @@ public Relationship relate(String description, String keyA, String keyB) { /** * Validate and normalize the provided {@link BulletConfig} for the defined entries and relationships. Then entries - * are used to normalize the config first. + * are used to validate the config first. * - * @param config The config containing fields to normalize. + * @param config The config containing fields to validate. */ - public void normalize(BulletConfig config) { + public void validate(BulletConfig config) { entries.values().forEach(e -> e.normalize(config)); relations.stream().forEach(r -> r.normalize(config)); } @@ -411,16 +411,32 @@ public static boolean isPowerOfTwo(Object value) { /** * Creates a {@link Predicate} that checks to see if the given object is in the list of values. * - * @param type The class of the object whose membership is being tested. * @param values The values that the object could be equal to that is being tested. * @param The type of the values and the object. - * @return A predicate that checks to see if the object provided to it is of the given type and is in the given values. + * @return A predicate that checks to see if the object provided to it is in the given values. */ - public static Predicate isIn(Class type, T... values) { - Objects.requireNonNull(type); + @SuppressWarnings("unchecked") + public static Predicate isIn(T... values) { Objects.requireNonNull(values); Set set = new HashSet<>(Arrays.asList(values)); - return o -> isType(o, type) && set.contains(o); + return set::contains; + } + + /** + * Creates a {@link Predicate} that checks to see if the given object is a {@link Number} in the proper range. + * + * @param min The smallest this number value could be. + * @param max The largest this number value could be. + * @param The type of the value, min, and max. + * @return A predicate that checks to see if the provided object is a Number and is in the proper range. + */ + @SuppressWarnings("unchecked") + public static Predicate isInRange(T min, T max) { + Objects.requireNonNull(min); + Objects.requireNonNull(max); + double minimum = min.doubleValue(); + double maximum = max.doubleValue(); + return o -> isNumber(o) && ((T) o).doubleValue() >= minimum && ((T) o).doubleValue() <= maximum; } // Binary Predicates. diff --git a/src/main/java/com/yahoo/bullet/result/Metadata.java b/src/main/java/com/yahoo/bullet/result/Metadata.java index 1107d6ac..62a715f4 100644 --- a/src/main/java/com/yahoo/bullet/result/Metadata.java +++ b/src/main/java/com/yahoo/bullet/result/Metadata.java @@ -84,7 +84,7 @@ public Map asMap() { /** * Add a piece of meta information. * - * @param key The name of the meta tag + * @param key The name of the meta tag * @param information An object that represents the information. * @return This object for chaining. */ diff --git a/src/test/java/com/yahoo/bullet/BulletConfigTest.java b/src/test/java/com/yahoo/bullet/BulletConfigTest.java index 465677b4..380b4fe3 100644 --- a/src/test/java/com/yahoo/bullet/BulletConfigTest.java +++ b/src/test/java/com/yahoo/bullet/BulletConfigTest.java @@ -40,7 +40,7 @@ private static class CustomConfig extends BulletConfig { @Override public BulletConfig validate() { - CUSTOM.normalize(this); + CUSTOM.validate(this); return super.validate(); } } @@ -48,27 +48,27 @@ public BulletConfig validate() { @Test public void testNoFiles() { BulletConfig config = new BulletConfig(); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000); config = new BulletConfig(null); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000); config = new BulletConfig(""); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000); } @Test public void testMissingFile() { BulletConfig config = new BulletConfig("/path/to/non/existant/file"); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 120000); } @Test public void testCustomConfig() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 10000L); - Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100L); - Assert.assertEquals(config.get(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES), 16384L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 10000); + Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100); + Assert.assertEquals(config.get(BulletConfig.COUNT_DISTINCT_AGGREGATION_SKETCH_ENTRIES), 16384); } @Test @@ -138,12 +138,12 @@ public void testMerging() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); int configSize = config.getAll(Optional.empty()).size(); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 10000L); - Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 10000); + Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100); Config another = new BulletConfig(null); another.clear(); - another.set(BulletConfig.SPECIFICATION_MAX_DURATION, 42L); + another.set(BulletConfig.SPECIFICATION_MAX_DURATION, 42); config.set("pi", 3.14); config.merge(another); @@ -152,8 +152,8 @@ public void testMerging() { config.merge(null); Assert.assertEquals(config.getAll(Optional.empty()).size(), configSize + 1); - Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 42L); - Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100L); + Assert.assertEquals(config.get(BulletConfig.SPECIFICATION_MAX_DURATION), 42); + Assert.assertEquals(config.get(BulletConfig.AGGREGATION_MAX_SIZE), 100); Assert.assertEquals(config.get("pi"), 3.14); } @@ -189,7 +189,7 @@ public void testPropertiesStripPrefix() { public void testGetAsAGivenType() { BulletConfig config = new BulletConfig("src/test/resources/custom_config.yaml"); - long defaulted = config.getAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, Long.class); + int defaulted = config.getAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, Integer.class); Assert.assertEquals(defaulted, 100); Map customMap = config.getAs("my.custom.map", Map.class); @@ -210,7 +210,7 @@ public void testGetOrDefaultAsAGivenType() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); - long notDefaulted = config.getOrDefaultAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, 42L, Long.class); + int notDefaulted = config.getOrDefaultAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, 42, Integer.class); Assert.assertEquals(notDefaulted, 100); String defaulted = config.getOrDefaultAs("foo", "value", String.class); @@ -225,7 +225,7 @@ public void testGettingRequiredConfig() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); - long present = config.getRequiredConfigAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, Long.class); + int present = config.getRequiredConfigAs(BulletConfig.DISTRIBUTION_AGGREGATION_MAX_POINTS, Integer.class); Assert.assertEquals(present, 100); } @@ -233,7 +233,7 @@ public void testGettingRequiredConfig() { public void testMissingRequiredConfig() { BulletConfig config = new BulletConfig("src/test/resources/test_config.yaml"); - config.getRequiredConfigAs("does.not.exist", Long.class); + config.getRequiredConfigAs("does.not.exist", Integer.class); } @Test @@ -250,7 +250,7 @@ public void testMetadataConversion() { expected.put(name, key); } - BulletConfig config = new BulletConfig().validate(); + BulletConfig config = new BulletConfig(); Assert.assertEquals(config.get(BulletConfig.RESULT_METADATA_METRICS), allMetadataAsMap()); config.set(BulletConfig.RESULT_METADATA_METRICS, metadata); @@ -362,9 +362,9 @@ public void testStringification() { @Test public void testCustomConfigValidation() { CustomConfig config = new CustomConfig(); - Assert.assertNull(config.get("foo")); - Assert.assertNull(config.get("bar")); - Assert.assertEquals(config.get(BulletConfig.AGGREGATION_DEFAULT_SIZE), (long) BulletConfig.DEFAULT_AGGREGATION_SIZE); + Assert.assertEquals(config.get("foo"), 42); + Assert.assertEquals(config.get("bar"), 0.4); + Assert.assertEquals(config.get(BulletConfig.AGGREGATION_DEFAULT_SIZE), BulletConfig.DEFAULT_AGGREGATION_SIZE); config.set("foo", 42); config.set("bar", 10.1); diff --git a/src/test/java/com/yahoo/bullet/ValidatorTest.java b/src/test/java/com/yahoo/bullet/ValidatorTest.java index 94b2abe0..b5f8f492 100644 --- a/src/test/java/com/yahoo/bullet/ValidatorTest.java +++ b/src/test/java/com/yahoo/bullet/ValidatorTest.java @@ -67,7 +67,8 @@ public void testEntryCasting() { public void testEntryMultipleChecks() { Validator validator = new Validator(); Entry entry = validator.define("foo") - .checkIf(Validator::isNotNull).checkIf(Validator.isIn(String.class, "baz", "bar")) + .checkIf(Validator::isNotNull) + .checkIf(Validator.isIn("baz", "bar")) .defaultTo("qux"); Assert.assertNull(empty.get("foo")); @@ -176,19 +177,34 @@ public void testIsMap() { Assert.assertFalse(Validator.isMap("foo")); } + @SuppressWarnings("unchecked") @Test public void testIsIn() { - Assert.assertTrue(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("foo")); - Assert.assertTrue(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("qux")); - Assert.assertFalse(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test("f")); - Assert.assertFalse(Validator.isIn(String.class, "foo", "bar", "baz", "qux").test(1.0)); - - Assert.assertTrue(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(asList("foo", "bar"))); - Assert.assertTrue(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(asList("baz", "qux"))); - Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(singletonList("baz"))); - Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(singletonList("foo"))); - Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test(null)); - Assert.assertFalse(Validator.isIn(List.class, asList("foo", "bar"), asList("baz", "qux")).test("foo")); + Assert.assertTrue(Validator.isIn("foo", "bar", "baz", "qux").test("foo")); + Assert.assertTrue(Validator.isIn("foo", "bar", "baz", "qux").test("qux")); + Assert.assertFalse(Validator.isIn("foo", "bar", "baz", "qux").test("f")); + Assert.assertFalse(Validator.isIn("foo", "bar", "baz", "qux").test(1.0)); + + Assert.assertTrue(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test(asList("foo", "bar"))); + Assert.assertTrue(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test(asList("baz", "qux"))); + Assert.assertFalse(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test(singletonList("baz"))); + Assert.assertFalse(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test(singletonList("foo"))); + Assert.assertFalse(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test(null)); + Assert.assertFalse(Validator.isIn(asList("foo", "bar"), asList("baz", "qux")).test("foo")); + } + + @Test + public void testIsInRange() { + Assert.assertTrue(Validator.isInRange(-1, 2.0).test(-1)); + Assert.assertTrue(Validator.isInRange(-1, 2.0).test(0)); + Assert.assertTrue(Validator.isInRange(-1, 2.0).test(2.0)); + Assert.assertTrue(Validator.isInRange(-1.0, 2.0).test(0.1)); + Assert.assertTrue(Validator.isInRange(-1.0, 2.0).test(0.0f)); + Assert.assertTrue(Validator.isInRange(-1.0, 2.0).test(1L)); + Assert.assertFalse(Validator.isInRange(-1, 2.0).test("0")); + Assert.assertFalse(Validator.isInRange(-1, 2.0).test(-1.1)); + Assert.assertFalse(Validator.isInRange(-1, 2.0).test(2.1)); + Assert.assertFalse(Validator.isInRange(-1, 2.0).test(null)); } @Test diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java index 6761b2a1..7a63ef9e 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/DistributionTest.java @@ -98,7 +98,7 @@ public void testInitialize() { List errors; Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(new BulletConfig().validate()); + aggregation.setConfiguration(new BulletConfig()); Distribution distribution = new Distribution(aggregation); errors = distribution.initialize(); @@ -128,7 +128,7 @@ public void testInitialize() { public void testRangeInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(new BulletConfig().validate()); + aggregation.setConfiguration(new BulletConfig()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; @@ -191,7 +191,7 @@ public void testRangeInitialization() { public void testNumberOfPointsInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(new BulletConfig().validate()); + aggregation.setConfiguration(new BulletConfig()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; @@ -230,7 +230,7 @@ public void testNumberOfPointsInitialization() { public void testProvidedPointsInitialization() { Aggregation aggregation = new Aggregation(); aggregation.setSize(20); - aggregation.setConfiguration(new BulletConfig().validate()); + aggregation.setConfiguration(new BulletConfig()); aggregation.setFields(Collections.singletonMap("foo", "bar")); Distribution distribution = new Distribution(aggregation); List errors; diff --git a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java index a31ef07a..3597400f 100644 --- a/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java +++ b/src/test/java/com/yahoo/bullet/operations/aggregations/GroupAllTest.java @@ -37,7 +37,7 @@ public static GroupAll makeGroupAll(Map... groupOperations) { // Does not matter aggregation.setSize(1); aggregation.setAttributes(makeAttributes(groupOperations)); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); GroupAll all = new GroupAll(aggregation); all.initialize(); return all; diff --git a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java index 53dc26c9..004beefe 100644 --- a/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/AggregationTest.java @@ -37,7 +37,7 @@ public class AggregationTest { @Test public void testSize() { Aggregation aggregation = new Aggregation(); - BulletConfig config = new BulletConfig().validate(); + BulletConfig config = new BulletConfig(); aggregation.setSize(null); aggregation.configure(config); @@ -116,7 +116,7 @@ public void testSuccessfulValidate() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT, null, "count"))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertFalse(aggregation.validate().isPresent()); } @@ -126,7 +126,7 @@ public void testValidateNoField() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(SUM, null, null))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -138,7 +138,7 @@ public void testUnsupportedOperation() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT_FIELD, "someField", "myCountField"))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -150,7 +150,7 @@ public void testAttributeOperationMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, null)); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); // Missing attribute operations should be silently validated List errors = aggregation.validate().get(); @@ -163,7 +163,7 @@ public void testAttributeOperationBadFormat() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, asList("foo"))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); // Bad attribute operations should be silently validated List errors = aggregation.validate().get(); @@ -177,7 +177,7 @@ public void testAttributeOperationsUnknownOperation() { aggregation.setType(GROUP); aggregation.setAttributes(makeAttributes(makeGroupOperation(COUNT, null, "bar"), makeGroupOperation(COUNT_FIELD, "foo", "foo_avg"))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); // The bad operation should have been thrown out. Assert.assertEquals(aggregation.validate(), Optional.>empty()); @@ -194,7 +194,7 @@ public void testAttributeOperationsDuplicateOperation() { makeGroupOperation(COUNT, "bar", null), makeGroupOperation(COUNT, "foo", null), makeGroupOperation(COUNT, "bar", null))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); // The bad ones should be removed. Assert.assertEquals(aggregation.validate(), Optional.>empty()); } @@ -203,7 +203,7 @@ public void testAttributeOperationsDuplicateOperation() { public void testFailValidateOnCountDistinctFieldsMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(COUNT_DISTINCT); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -214,7 +214,7 @@ public void testFailValidateOnCountDistinctFieldsMissing() { public void testFailValidateOnGroupFieldsAndOperationsMissing() { Aggregation aggregation = new Aggregation(); aggregation.setType(GROUP); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); List errors = aggregation.validate().get(); Assert.assertEquals(errors.size(), 1); @@ -224,7 +224,7 @@ public void testFailValidateOnGroupFieldsAndOperationsMissing() { @Test public void testToString() { Aggregation aggregation = new Aggregation(); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.toString(), "{size: 1, type: RAW, fields: null, attributes: null}"); @@ -244,7 +244,7 @@ public void testToString() { public void testUnimplementedStrategies() { Aggregation aggregation = new Aggregation(); aggregation.setType(null); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertNull(aggregation.getStrategy()); } @@ -252,7 +252,7 @@ public void testUnimplementedStrategies() { @Test public void testRawStrategy() { Aggregation aggregation = new Aggregation(); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.findStrategy().getClass(), Raw.class); } @@ -264,7 +264,7 @@ public void testGroupAllStrategy() { aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, singletonList(singletonMap(GroupOperation.OPERATION_TYPE, GroupOperationType.COUNT.getName())))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupAll.class); } @@ -274,7 +274,7 @@ public void testCountDistinctStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.COUNT_DISTINCT); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), CountDistinct.class); } @@ -284,7 +284,7 @@ public void testDistinctStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.GROUP); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupBy.class); } @@ -297,7 +297,7 @@ public void testGroupByStrategy() { aggregation.setAttributes(singletonMap(GroupOperation.OPERATIONS, singletonList(singletonMap(GroupOperation.OPERATION_TYPE, GroupOperationType.COUNT.getName())))); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), GroupBy.class); } @@ -307,7 +307,7 @@ public void testDistributionStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.DISTRIBUTION); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), Distribution.class); } @@ -317,7 +317,7 @@ public void testTopKStrategy() { Aggregation aggregation = new Aggregation(); aggregation.setType(AggregationOperations.AggregationType.TOP_K); aggregation.setFields(singletonMap("field", "foo")); - aggregation.configure(new BulletConfig().validate()); + aggregation.configure(new BulletConfig()); Assert.assertEquals(aggregation.getStrategy().getClass(), TopK.class); } diff --git a/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java b/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java index d67e1114..29726904 100644 --- a/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java +++ b/src/test/java/com/yahoo/bullet/parsing/SpecificationTest.java @@ -138,7 +138,7 @@ public void testAggregationForced() { Assert.assertNull(specification.getFilters()); // If you had null for aggregation Assert.assertNull(specification.getAggregation()); - specification.configure(new BulletConfig().validate()); + specification.configure(new BulletConfig()); Assert.assertTrue(specification.isAcceptingData()); Assert.assertEquals(specification.getAggregate().getRecords(), emptyList()); @@ -146,7 +146,7 @@ public void testAggregationForced() { @Test public void testDuration() { - BulletConfig config = new BulletConfig().validate(); + BulletConfig config = new BulletConfig(); Specification specification = new Specification(); specification.configure(config); @@ -219,7 +219,7 @@ public void testCustomDuration() { public void testFiltering() { Specification specification = new Specification(); specification.setFilters(singletonList(FilterClauseTest.getFieldFilter(FilterType.EQUALS, "foo", "bar"))); - specification.configure(new BulletConfig().validate()); + specification.configure(new BulletConfig()); Assert.assertTrue(specification.filter(RecordBox.get().add("field", "foo").getRecord())); Assert.assertTrue(specification.filter(RecordBox.get().add("field", "bar").getRecord())); @@ -286,7 +286,7 @@ public void testAggregationDefault() { specification.setAggregation(aggregation); Assert.assertNull(aggregation.getType()); - specification.configure(new BulletConfig().validate()); + specification.configure(new BulletConfig()); // Specification no longer fixes type Assert.assertNull(aggregation.getType()); @@ -296,7 +296,7 @@ public void testAggregationDefault() { @Test public void testMeetingDefaultSpecification() { Specification specification = new Specification(); - specification.configure(new BulletConfig().validate()); + specification.configure(new BulletConfig()); Assert.assertTrue(makeStream(BulletConfig.DEFAULT_AGGREGATION_SIZE - 1).map(specification::filter).allMatch(x -> x)); // Check that we only get the default number out @@ -373,7 +373,7 @@ public void testAggregationExceptions() { @Test public void testToString() { - BulletConfig config = new BulletConfig().validate(); + BulletConfig config = new BulletConfig(); Specification specification = new Specification(); specification.configure(config); diff --git a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java index 749f0a88..02117438 100644 --- a/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java +++ b/src/test/java/com/yahoo/bullet/querying/FilterQueryTest.java @@ -111,6 +111,6 @@ public void testMaximumEmittedWithNonMatchingRecords() { @Test(expectedExceptions = ParsingException.class) public void testValidationFail() throws ParsingException { - new FilterQuery("{ 'aggregation': { 'type': null } }", new BulletConfig().validate()); + new FilterQuery("{ 'aggregation': { 'type': null } }", new BulletConfig()); } } diff --git a/src/test/resources/test_config.yaml b/src/test/resources/test_config.yaml index 6d1339a1..f799ed07 100644 --- a/src/test/resources/test_config.yaml +++ b/src/test/resources/test_config.yaml @@ -1,4 +1,5 @@ bullet.query.max.duration: 10000 +bullet.query.default.duration: 1000 bullet.query.aggregation.max.size: 100 # Some random fake setting fake.setting: null