Serialize string and primitive collections (#4)

src/main/java/com/kickstarter/dropwizard/metrics/influxdb/InfluxDbMeasurement.java

@@ -3,10 +3,14 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 
@@ -17,6 +21,11 @@
  */
 @AutoValue
 public abstract class InfluxDbMeasurement {
+  private static final Set<Class> VALID_FIELD_CLASSES = ImmutableSet.of(
+    Boolean.class, Byte.class, Character.class, Double.class, Float.class,
+    Integer.class, Long.class, Short.class, String.class
+  );
+
   public abstract String name();
   public abstract Map<String, String> tags();
   public abstract Map<String, String> fields();
@@ -103,8 +112,6 @@ public boolean isValid() {
 
     /**
      * Adds all key-value pairs to the tags map.
-     * @param items
-     * @return
      */
     public Builder putTags(final Map<String, String> items) {
       tags.putAll(items);
@@ -120,7 +127,7 @@ public Builder putTag(final String key, final String value) {
     }
 
     /**
-     * Adds all key-value pairs to the fields map.
+     * Adds all key-value pairs to the fields map; null, NaN, and +-Inf values are dropped.
      *
      * @throws IllegalArgumentException if any value is not a String or primitive.
      */
@@ -130,11 +137,11 @@ public <T> Builder putFields(final Map<String, T> fields) {
     }
 
     /**
-     * Adds all key-value pairs to the fields map.
+     * Adds all key-value pairs to the fields map; null, NaN, and +-Inf values are dropped.
      */
     public <T> Builder tryPutFields(final Map<String, T> fields,
                                     final Consumer<IllegalArgumentException> exceptionHandler) {
-      for(final Map.Entry<String, T> field: fields.entrySet()) {
+      for (final Map.Entry<String, T> field : fields.entrySet()) {
         try {
           putField(field.getKey(), field.getValue());
         } catch (IllegalArgumentException e) {
@@ -145,36 +152,88 @@ public <T> Builder tryPutFields(final Map<String, T> fields,
     }
 
     /**
-     * Adds the given key-value pair to the fields map.
+     * Adds the given key-value pair to the fields map if it is not one of: null, NaN, +-Inf.
      *
-     * @throws IllegalArgumentException if any value is not a String or primitive.
+     * @throws IllegalArgumentException if any value is not one of:
+     *         null, String, primitive, or a Collection of Strings and primitives.
      */
     public <T> Builder putField(final String key, final T value) {
-      if (value == null) {
-        return this;
+      if (value instanceof Collection<?>) {
+        final Collection collection = (Collection) value;
+        final String collString = validatedPrimitiveCollection(key, collection);
+        fields.put(key, collString);
+      } else if (value != null) {
+        final Optional<String> fieldString = validatedPrimitiveField(key, value);
+        fieldString.ifPresent(s -> fields.put(key, s));
+      }
+
+      return this;
+    }
+
+    /**
+     * Validates that the collection only contains null values, Strings, and primitives.
+     *
+     * @return the collection as a string, if valid.
+     * @throws IllegalArgumentException if any collection value is not a null value, string or primitive.
+     */
+    private static String validatedPrimitiveCollection(final String key, final Collection collection) {
+      for (final Object value : collection) {
+        if (!isValidField(value)) {
+          throw new IllegalArgumentException(
+            String.format(
+              "InfluxDbMeasurement collection field '%s' must contain only Strings and primitives: invalid field '%s'",
+              key, value
+            )
+          );
+        }
       }
 
+      return collection.toString();
+    }
+
+    /**
+     * Validates that the field is a null value, String, or primitive.
+     *
+     * @return true if the field is a null value, String, or primitive.
+     */
+    private static <T> boolean isValidField(final T value) {
+      return value == null || VALID_FIELD_CLASSES.contains(value.getClass());
+    }
+
+    /**
+     * Validates that the field is a String or primitive.
+     *
+     * @return an Optional containing the stringified field, or Optional.empty()
+     *         if the field is an invalid primitive field.
+     *
+     * @throws IllegalArgumentException if the field is not a string or primitive.
+     */
+    private static <T> Optional<String> validatedPrimitiveField(final String key, final T value) {
       if (value instanceof Float) {
         final float f = (Float) value;
         if (!Float.isNaN(f) && !Float.isInfinite(f)) {
-          fields.put(key, String.valueOf(f));
+          return Optional.of(String.valueOf(f));
         }
       } else if (value instanceof Double) {
         final double d = (Double) value;
         if (!Double.isNaN(d) && !Double.isInfinite(d)) {
-          fields.put(key, String.valueOf(d));
+          return Optional.of(String.valueOf(d));
         }
-      } else if (value instanceof Integer || value instanceof Long) {
-        fields.put(key, String.format("%di", ((Number) value).longValue()));
-      } else if (value instanceof String || value instanceof Boolean) {
-        fields.put(key, value.toString());
+      } else if (value instanceof Number) {
+        // Serialize Byte, Short, Integer, and Long values as integers.
+        return Optional.of(String.format("%di", ((Number) value).longValue()));
+      } else if (value instanceof String || value instanceof Character || value instanceof Boolean) {
+        return Optional.of(value.toString());
       } else {
         throw new IllegalArgumentException(
-          String.format("InfluxDbMeasurement field '%s' must be String or primitive: %s", key, value)
+          String.format(
+            "InfluxDbMeasurement field '%s' must be a String, primitive, or Collection: invalid field '%s'",
+            key, value
+          )
         );
       }
 
-      return this;
+      return Optional.empty();
     }
 
     public InfluxDbMeasurement build() {

src/test/java/com/kickstarter/dropwizard/metrics/influxdb/InfluxDbMeasurementTest.java

@@ -2,7 +2,6 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import org.junit.Test;
 
 import java.util.ArrayList;
@@ -244,14 +243,39 @@ public void testBuilder_PutBooleanField() {
       measurement);
   }
 
+  @Test
+  public void testBuilder_PutStringAndPrimitiveCollection() {
+    final InfluxDbMeasurement measurement = new InfluxDbMeasurement.Builder("Measurement", 90210L)
+      .putField("eating", Arrays.asList(1, 2, 3))
+      .build();
+
+    assertEquals("should add Collection field to the measurement",
+      InfluxDbMeasurement.create("Measurement", ImmutableMap.of(), ImmutableMap.of("eating", "[1, 2, 3]"), 90210L),
+      measurement);
+  }
+
+  @Test
+  public void testBuilder_PutNonStringOrPrimitiveCollection() {
+    try {
+      new InfluxDbMeasurement.Builder("Measurement", 90210L)
+        .putField("val", Arrays.asList(ImmutableMap.of("okay", "then")));
+      fail("Expected an Exception when adding a non-String, -primitive, or Collection field");
+    } catch (final Exception thrown) {
+      assertEquals("Expected an IllegalArgumentException", IllegalArgumentException.class, thrown.getClass());
+      assertEquals("InfluxDbMeasurement collection field 'val' must contain " +
+        "only Strings and primitives: invalid field '{okay=then}'", thrown.getMessage());
+    }
+  }
+
   @Test
   public void testBuilder_PutNonStringOrPrimitiveField() {
     try {
-      new InfluxDbMeasurement.Builder("Measurement", 90210L).putField("val", Arrays.asList(1, 2, 3));
+      new InfluxDbMeasurement.Builder("Measurement", 90210L).putField("val", ImmutableMap.of("okay", "then"));
       fail("Expected an Exception when adding a non-String or -primitive field");
     } catch (final Exception thrown) {
       assertEquals("Expected an IllegalArgumentException", IllegalArgumentException.class, thrown.getClass());
-      assertEquals("InfluxDbMeasurement field 'val' must be String or primitive: [1, 2, 3]", thrown.getMessage());
+      assertEquals("InfluxDbMeasurement field 'val' must be a String, primitive, or Collection: " +
+        "invalid field '{okay=then}'", thrown.getMessage());
     }
   }
 
@@ -260,18 +284,18 @@ public void testBuilder_TryPutFields() {
     final ArrayList<Exception> exceptions = new ArrayList<>();
 
     new InfluxDbMeasurement.Builder("Measurement", 90210L).tryPutFields(ImmutableMap.of(
-      "a", ImmutableSet.of(),
+      "a", ImmutableMap.of("well", "okay"),
       "c", "d",
-      "e", Arrays.asList(1, 2, 3)
+      "e", ImmutableMap.of("not", 1, "good", 2)
     ), exceptions::add);
 
     assertEquals("should catch two exceptions", 2, exceptions.size());
     assertEquals("should catch IllegalArgumentExceptions", IllegalArgumentException.class, exceptions.get(0).getClass());
     assertEquals("should catch IllegalArgumentExceptions", IllegalArgumentException.class, exceptions.get(1).getClass());
     assertEquals("should describe the invalid key-value pair",
       ImmutableList.of(
-        "InfluxDbMeasurement field 'a' must be String or primitive: []",
-        "InfluxDbMeasurement field 'e' must be String or primitive: [1, 2, 3]"
+        "InfluxDbMeasurement field 'a' must be a String, primitive, or Collection: invalid field '{well=okay}'",
+        "InfluxDbMeasurement field 'e' must be a String, primitive, or Collection: invalid field '{not=1, good=2}'"
       ),
       exceptions.stream().map(Exception::getMessage).collect(toList())
     );

src/test/java/com/kickstarter/dropwizard/metrics/influxdb/transformer/DropwizardTransformerTest.java

@@ -430,11 +430,13 @@ public void testFromValueGroup_InvalidField() {
     final Map<String, Object> objFields = ImmutableMap.of(
       "some", true,
       "bad", Arrays.asList(1, 2, 3),
-      "fields", 5
+      "fields", 5,
+      "NOW", ImmutableMap.of("ha", "ha")
     );
 
     final Map<String, String> strFields = ImmutableMap.of(
       "some", "true",
+      "bad", "[1, 2, 3]",
       "fields", "5i"
     );
 
@@ -451,7 +453,7 @@ public void testFromValueGroup_InvalidMeasurement() {
 
     final Map<String, Object> objFields = ImmutableMap.of(
       "all", Float.NaN,
-      "bad", Arrays.asList(1, 2, 3),
+      "bad", ImmutableMap.of("oh", "no"),
       "fields", Float.NaN
     );