diff --git a/src/main/java/com/yahoo/bullet/aggregations/GroupBy.java b/src/main/java/com/yahoo/bullet/aggregations/GroupBy.java index 16024507..8ecc0797 100644 --- a/src/main/java/com/yahoo/bullet/aggregations/GroupBy.java +++ b/src/main/java/com/yahoo/bullet/aggregations/GroupBy.java @@ -24,8 +24,6 @@ import java.util.Optional; import java.util.Set; -import static com.yahoo.bullet.common.Utilities.extractField; - /** * This {@link Strategy} implements a Tuple Sketch based approach to doing a group by. In particular, it * provides a uniform sample of the groups if the number of unique groups exceed the Sketch size. Metrics like @@ -104,7 +102,7 @@ private Map getFields(BulletRecord record) { Map fieldValues = new HashMap<>(); for (String field : fields) { // This explicitly does not do a TypedObject checking. Nulls (and everything else) turn into Strings - String value = Objects.toString(extractField(field, record)); + String value = Objects.toString(record.extractField(field)); fieldValues.put(field, value); } return fieldValues; diff --git a/src/main/java/com/yahoo/bullet/aggregations/SketchingStrategy.java b/src/main/java/com/yahoo/bullet/aggregations/SketchingStrategy.java index 8b17145d..68cc5f65 100644 --- a/src/main/java/com/yahoo/bullet/aggregations/SketchingStrategy.java +++ b/src/main/java/com/yahoo/bullet/aggregations/SketchingStrategy.java @@ -23,8 +23,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.bullet.common.Utilities.extractField; - /** * The parent class for all {@link Strategy} that use Sketches. * @@ -99,7 +97,7 @@ public void reset() { * @return A string representing the composite field. */ String composeField(BulletRecord record) { - return composeField(fields.stream().map(field -> Objects.toString(extractField(field, record)))); + return composeField(fields.stream().map(field -> Objects.toString(record.extractField(field)))); } /** diff --git a/src/main/java/com/yahoo/bullet/common/Utilities.java b/src/main/java/com/yahoo/bullet/common/Utilities.java index a2a827f5..8bf3065f 100644 --- a/src/main/java/com/yahoo/bullet/common/Utilities.java +++ b/src/main/java/com/yahoo/bullet/common/Utilities.java @@ -94,44 +94,25 @@ public static double round(double value, int places) { } /** - * Extracts the field from the given {@link BulletRecord}. + * Extracts this identifier as a {@link TypedObject}. * - * @param field The field to get. It can be "." separated to look inside maps. - * @param record The record containing the field. - * @return The extracted field or null if error or not found. - */ - public static Object extractField(String field, BulletRecord record) { - if (field == null) { - return null; - } - String[] split = splitField(field); - try { - return split.length > 1 ? record.get(split[0], split[1]) : record.get(field); - } catch (ClassCastException cce) { - return null; - } - } - - /** - * Extracts this field as a {@link TypedObject}. - * - * @param field The field name to extract. + * @param identifier The identifier name to extract. It can be "." separated to look inside maps and arrays. * @param record The {@link BulletRecord} to extract it from. - * @return The created TypedObject from the value for the field in the record. + * @return The created TypedObject from the value for the identifier in the record. */ - public static TypedObject extractTypedObject(String field, BulletRecord record) { - return new TypedObject(extractField(field, record)); + public static TypedObject extractTypedObject(String identifier, BulletRecord record) { + return new TypedObject(record.extractField(identifier)); } /** - * Extracts the field from the given (@link BulletRecord} as a {@link Number}, if possible. + * Extracts the identifier from the given (@link BulletRecord} as a {@link Number}, if possible. * - * @param field The field containing a numeric value to get. It can be "." separated to look inside maps. - * @param record The record containing the field. - * @return The value of the field as a {@link Number} or null if it cannot be forced to one. + * @param identifier The identifier of a number to get. It can be "." separated to look inside maps and arrays. + * @param record The record containing the identifier. + * @return The value of the identifier as a {@link Number} or null if it cannot be forced to one. */ - public static Number extractFieldAsNumber(String field, BulletRecord record) { - Object value = extractField(field, record); + public static Number extractFieldAsNumber(String identifier, BulletRecord record) { + Object value = record.extractField(identifier); // Also checks for null if (value instanceof Number) { return (Number) value; diff --git a/src/main/java/com/yahoo/bullet/querying/operations/FilterOperations.java b/src/main/java/com/yahoo/bullet/querying/operations/FilterOperations.java index 7149d882..7f178650 100644 --- a/src/main/java/com/yahoo/bullet/querying/operations/FilterOperations.java +++ b/src/main/java/com/yahoo/bullet/querying/operations/FilterOperations.java @@ -23,7 +23,6 @@ import java.util.regex.Pattern; import java.util.stream.Stream; -import static com.yahoo.bullet.common.Utilities.extractField; import static com.yahoo.bullet.common.Utilities.extractTypedObject; import static com.yahoo.bullet.common.Utilities.isEmpty; import static com.yahoo.bullet.typesystem.TypedObject.IS_NOT_NULL; @@ -110,7 +109,7 @@ private static TypedObject getTypedValue(BulletRecord record, Type type, Value v Type newType = value.getType(); switch (value.getKind()) { case FIELD: - result = newType == null ? TypedObject.typeCastFromObject(type, extractField(valueString, record)) + result = newType == null ? TypedObject.typeCastFromObject(type, record.extractField(valueString)) : extractTypedObject(valueString, record).forceCast(newType); break; case VALUE: diff --git a/src/test/java/com/yahoo/bullet/common/UtilitiesTest.java b/src/test/java/com/yahoo/bullet/common/UtilitiesTest.java index b2463224..9a7c9e15 100644 --- a/src/test/java/com/yahoo/bullet/common/UtilitiesTest.java +++ b/src/test/java/com/yahoo/bullet/common/UtilitiesTest.java @@ -79,21 +79,6 @@ public void testRounding() { Assert.assertEquals(String.valueOf(Utilities.round(1.4499999999999, 1)), "1.4"); } - - @Test - public void testExtractField() { - BulletRecord record = RecordBox.get().add("field", "foo").add("map_field.foo", "bar") - .addMap("map_field", Pair.of("foo", "baz")) - .addList("list_field", singletonMap("foo", "baz")) - .getRecord(); - - Assert.assertNull(Utilities.extractField(null, record)); - Assert.assertNull(Utilities.extractField("", record)); - Assert.assertNull(Utilities.extractField("id", record)); - Assert.assertEquals(Utilities.extractField("map_field.foo", record), "baz"); - Assert.assertNull(Utilities.extractField("list_field.bar", record)); - } - @Test public void testNumericExtraction() { BulletRecord record = RecordBox.get().add("foo", "1.20").add("bar", 42L) diff --git a/src/test/java/com/yahoo/bullet/querying/operations/FilterOperationsTest.java b/src/test/java/com/yahoo/bullet/querying/operations/FilterOperationsTest.java index 89bd4b19..455324e1 100644 --- a/src/test/java/com/yahoo/bullet/querying/operations/FilterOperationsTest.java +++ b/src/test/java/com/yahoo/bullet/querying/operations/FilterOperationsTest.java @@ -369,9 +369,9 @@ public void testSizeOf() { Assert.assertTrue(FilterOperations.perform(RecordBox.get().add("id", "12").getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().add("id", "123").getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setListOfStringMap("id", new ArrayList<>()), clause)); - Assert.assertTrue(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", 1)).getRecord(), clause)); - Assert.assertTrue(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", 1), singletonMap("2", 2)).getRecord(), clause)); - Assert.assertFalse(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", 1), singletonMap("2", 2), singletonMap("3", 3)).getRecord(), clause)); + Assert.assertTrue(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", 1)).getRecord(), clause)); + Assert.assertTrue(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", 1), singletonMap("2", 2)).getRecord(), clause)); + Assert.assertFalse(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", 1), singletonMap("2", 2), singletonMap("3", 3)).getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setStringMap("id", new HashMap<>()), clause)); Assert.assertTrue(FilterOperations.perform(RecordBox.get().addMap("id", Pair.of("1", 1), Pair.of("2", 2)).getRecord(), clause)); } @@ -381,8 +381,8 @@ public void testContainsKey() { FilterClause clause = getFieldFilter("id", CONTAINS_KEY, "1", "2"); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().add("id", "1").getRecord(), clause)); - Assert.assertTrue(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", 1)).getRecord(), clause)); - Assert.assertFalse(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("3", 1)).getRecord(), clause)); + Assert.assertTrue(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", 1)).getRecord(), clause)); + Assert.assertFalse(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("3", 1)).getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setListOfStringMap("id", new ArrayList<>()), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setStringMap("id", new HashMap<>()), clause)); Assert.assertTrue(FilterOperations.perform(RecordBox.get().addMap("id", Pair.of("1", 1), Pair.of("2", 2)).getRecord(), clause)); @@ -395,9 +395,9 @@ public void testContainsValue() { FilterClause clause = getFieldFilter("id", CONTAINS_VALUE, "1", "2"); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().add("id", "1").getRecord(), clause)); - Assert.assertTrue(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", "1")).getRecord(), clause)); - Assert.assertTrue(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("1", 1)).getRecord(), clause)); - Assert.assertFalse(FilterOperations.perform(RecordBox.get().addList("id", singletonMap("3", "3")).getRecord(), clause)); + Assert.assertTrue(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", "1")).getRecord(), clause)); + Assert.assertTrue(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("1", 1)).getRecord(), clause)); + Assert.assertFalse(FilterOperations.perform(RecordBox.get().addListOfMaps("id", singletonMap("3", "3")).getRecord(), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setListOfStringMap("id", new ArrayList<>()), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setListOfStringMap("id", singletonList(new HashMap<>())), clause)); Assert.assertFalse(FilterOperations.perform(RecordBox.get().getRecord().setStringMap("id", new HashMap<>()), clause)); diff --git a/src/test/java/com/yahoo/bullet/querying/operations/ProjectionOperationsTest.java b/src/test/java/com/yahoo/bullet/querying/operations/ProjectionOperationsTest.java index 4ea56711..7927b308 100644 --- a/src/test/java/com/yahoo/bullet/querying/operations/ProjectionOperationsTest.java +++ b/src/test/java/com/yahoo/bullet/querying/operations/ProjectionOperationsTest.java @@ -46,7 +46,7 @@ public void testProjection() { public void testUnsupportedProjection() { Projection projection = makeProjection(ImmutablePair.of("list_field.1.foo", "bar"), ImmutablePair.of("field", "foo")); - BulletRecord record = RecordBox.get().addList("list_field", emptyMap(), singletonMap("foo", "bar")) + BulletRecord record = RecordBox.get().addListOfMaps("list_field", emptyMap(), singletonMap("foo", "bar")) .add("field", "123") .getRecord(); BulletRecord actual = ProjectionOperations.project(record, projection, null, provider); @@ -58,9 +58,9 @@ public void testUnsupportedProjection() { public void testMapList() { Projection projection = makeProjection("list_field", "foo"); - BulletRecord record = RecordBox.get().addList("list_field", emptyMap(), singletonMap("foo", "baz")).getRecord(); + BulletRecord record = RecordBox.get().addListOfMaps("list_field", emptyMap(), singletonMap("foo", "baz")).getRecord(); - BulletRecord expected = RecordBox.get().addList("foo", emptyMap(), singletonMap("foo", "baz")).getRecord(); + BulletRecord expected = RecordBox.get().addListOfMaps("foo", emptyMap(), singletonMap("foo", "baz")).getRecord(); BulletRecord actual = ProjectionOperations.project(record, projection, null, provider); Assert.assertEquals(actual, expected); diff --git a/src/test/java/com/yahoo/bullet/result/ClipTest.java b/src/test/java/com/yahoo/bullet/result/ClipTest.java index 767487e9..3ff00cdd 100644 --- a/src/test/java/com/yahoo/bullet/result/ClipTest.java +++ b/src/test/java/com/yahoo/bullet/result/ClipTest.java @@ -53,7 +53,7 @@ public void testNullValueInRecord() { @Test public void testRecordAddition() { BulletRecord record = new RecordBox().add("field", "sample").addMap("map_field", Pair.of("foo", "bar")) - .addList("list_field", new HashMap<>(), singletonMap("foo", 1L)) + .addListOfMaps("list_field", new HashMap<>(), singletonMap("foo", 1L)) .getRecord(); assertJSONEquals(Clip.of(record).asJSON(), makeJSON("[{'list_field':[{},{'foo':1}],'field':'sample','map_field':{'foo':'bar'}}]")); } @@ -61,7 +61,7 @@ public void testRecordAddition() { @Test public void testRecordsAddition() { BulletRecord record = new RecordBox().add("field", "sample").addMap("map_field", Pair.of("foo", "bar")) - .addList("list_field", new HashMap<>(), singletonMap("foo", 1L)) + .addListOfMaps("list_field", new HashMap<>(), singletonMap("foo", 1L)) .getRecord(); BulletRecord another = new RecordBox().add("field", "another").getRecord(); diff --git a/src/test/java/com/yahoo/bullet/result/RecordBox.java b/src/test/java/com/yahoo/bullet/result/RecordBox.java index c84a199a..bd5db762 100644 --- a/src/test/java/com/yahoo/bullet/result/RecordBox.java +++ b/src/test/java/com/yahoo/bullet/result/RecordBox.java @@ -11,6 +11,7 @@ import org.apache.commons.lang3.tuple.Pair; import java.util.ArrayList; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -78,21 +79,71 @@ public final RecordBox addMap(String name, Pair... entries) { } @SafeVarargs - public final RecordBox addList(String name, Map... entries) { + public final RecordBox addMapOfMaps(String name, Pair>... entries) { + if (entries != null && entries.length != 0) { + Map[] sampleEntries = (Map[]) Arrays.stream(entries).map(Pair::getRight).toArray(); + Object value = findObject(sampleEntries); + if (value instanceof Boolean) { + record.setMapOfBooleanMap(name, asMapOfMaps(Boolean.class, entries)); + } else if (value instanceof Integer) { + record.setMapOfIntegerMap(name, asMapOfMaps(Integer.class, entries)); + } else if (value instanceof Long) { + record.setMapOfLongMap(name, asMapOfMaps(Long.class, entries)); + } else if (value instanceof Float) { + record.setMapOfFloatMap(name, asMapOfMaps(Float.class, entries)); + } else if (value instanceof Double) { + record.setMapOfDoubleMap(name, asMapOfMaps(Double.class, entries)); + } else if (value instanceof String) { + record.setMapOfStringMap(name, asMapOfMaps(String.class, entries)); + } else if (value == null) { + record.setMapOfStringMap(name, null); + } else { + throw new RuntimeException("Unsupported type cannot be added in test code to BulletRecord " + value); + } + } + return this; + } + + public final RecordBox addList(String name, Object... entries) { + if (entries != null && entries.length != 0) { + Object value = entries[0]; + if (value instanceof Boolean) { + record.setBooleanList(name, asList(Boolean.class, entries)); + } else if (value instanceof Integer) { + record.setIntegerList(name, asList(Integer.class, entries)); + } else if (value instanceof Long) { + record.setLongList(name, asList(Long.class, entries)); + } else if (value instanceof Float) { + record.setFloatList(name, asList(Float.class, entries)); + } else if (value instanceof Double) { + record.setDoubleList(name, asList(Double.class, entries)); + } else if (value instanceof String) { + record.setStringList(name, asList(String.class, entries)); + } else if (value == null) { + record.setStringList(name, null); + } else { + throw new RuntimeException("Unsupported type cannot be added in test code to BulletRecord " + value); + } + } + return this; + } + + @SafeVarargs + public final RecordBox addListOfMaps(String name, Map... entries) { if (entries != null && entries.length != 0) { Object value = findObject(entries); if (value instanceof Boolean) { - record.setListOfBooleanMap(name, asList(Boolean.class, entries)); + record.setListOfBooleanMap(name, asListOfMaps(Boolean.class, entries)); } else if (value instanceof Integer) { - record.setListOfIntegerMap(name, asList(Integer.class, entries)); + record.setListOfIntegerMap(name, asListOfMaps(Integer.class, entries)); } else if (value instanceof Long) { - record.setListOfLongMap(name, asList(Long.class, entries)); + record.setListOfLongMap(name, asListOfMaps(Long.class, entries)); } else if (value instanceof Float) { - record.setListOfFloatMap(name, asList(Float.class, entries)); + record.setListOfFloatMap(name, asListOfMaps(Float.class, entries)); } else if (value instanceof Double) { - record.setListOfDoubleMap(name, asList(Double.class, entries)); + record.setListOfDoubleMap(name, asListOfMaps(Double.class, entries)); } else if (value instanceof String) { - record.setListOfStringMap(name, asList(String.class, entries)); + record.setListOfStringMap(name, asListOfMaps(String.class, entries)); } else if (value == null) { record.setListOfStringMap(name, null); } else { @@ -133,7 +184,49 @@ private Map asMap(Class clazz, Map.Entry... en return newMap; } - private List> asList(Class clazz, Map... entries) { + private Map asMap(Class clazz, Map map) { + Objects.requireNonNull(clazz); + Objects.requireNonNull(map); + Map newMap = new LinkedHashMap<>(map.size()); + for (Map.Entry entry : map.entrySet()) { + Object object = entry.getValue(); + if (!clazz.isInstance(object)) { + throw new RuntimeException("Object " + object + " is not an instance of class " + clazz.getName()); + } + newMap.put(entry.getKey(), (T) entry.getValue()); + } + return newMap; + } + + private Map> asMapOfMaps(Class clazz, Pair>... entries) { + Objects.requireNonNull(clazz); + Objects.requireNonNull(entries); + Map> newMap = new LinkedHashMap<>(entries.length); + for (Pair> entry : entries) { + String key = entry.getKey(); + Map casted = asMap(clazz, entry.getValue()); + if (casted == null) { + throw new RuntimeException("Object " + casted + " is not an instance of class " + clazz.getName()); + } + newMap.put(key, casted); + } + return newMap; + } + + private List asList(Class clazz, Object... entries) { + Objects.requireNonNull(clazz); + Objects.requireNonNull(entries); + List newList = new ArrayList<>(entries.length); + for (Object object : entries) { + if (!clazz.isInstance(object)) { + throw new RuntimeException("Object " + object + " is not an instance of class " + clazz.getName()); + } + newList.add((T) object); + } + return newList; + } + + private List> asListOfMaps(Class clazz, Map... entries) { Objects.requireNonNull(clazz); Objects.requireNonNull(entries); List> newList = new ArrayList<>(entries.length); @@ -144,5 +237,4 @@ private List> asList(Class clazz, Map... e } return newList; } - }