Allow fields (constructor args, too) to be marked deprecated or obsolete.

This mirrors the current `:deprecated => "details"` and `:obsolete => "details"` in the Ruby side of Logstash.

* When a field is obsolete, using it will raise an exception.
* When a field is deprecated, using it will log a warning.

In tests, I used a trick I learned from Go to make compile-time assertions about return types. This is the reason for `Field x` in various places used in assignment but otherwise not used.

logstash-core/src/main/java/org/logstash/plugin/ConstructingObjectParser.java

@@ -1,5 +1,8 @@
 package org.logstash.plugin;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 import java.util.*;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
@@ -14,10 +17,10 @@ import java.util.stream.Collectors;
  * @param <Value> The object type to construct when `parse` is called.
  */
 public class ConstructingObjectParser<Value> implements Function<Map<String, Object>, Value> {
+    private final Logger logger = LogManager.getLogger();
     private final Function<Object[], Value> builder;
-    private final Map<String, BiConsumer<Value, Object>> parsers = new LinkedHashMap<>();
-    private final Map<String, BiConsumer<Object[], Object>> constructorArgs;
-
+    private final Map<String, FieldDefinition<Value>> parsers = new LinkedHashMap<>();
+    private final Map<String, FieldDefinition<Object[]>> constructorArgs;
 
     /**
      * @param supplier The supplier which produces an object instance.
@@ -42,7 +45,7 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
     /**
      * A function which takes an Object and returns an Integer
      *
-     * @param object
+     * @param object the object to transform to Integer
      * @return An Integer based on the given object.
      * @throws IllegalArgumentException if conversion is not possible
      */
@@ -149,8 +152,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param consumer the function to call once the value is available
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareLong(String name, BiConsumer<Value, Long> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformLong);
+    public Field declareLong(String name, BiConsumer<Value, Long> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformLong);
     }
 
     /**
@@ -159,27 +162,31 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param name the name of the field.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareLong(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformLong);
+    public Field declareLong(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformLong);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareField(String name, BiConsumer<Value, T> consumer, Function<Object, T> transform) {
+    public <T> Field declareField(String name, BiConsumer<Value, T> consumer, Function<Object, T> transform) {
         BiConsumer<Value, Object> objConsumer = (value, object) -> consumer.accept(value, transform.apply(object));
-        parsers.put(name, objConsumer);
+        FieldDefinition<Value> field = new FieldDefinition<>(objConsumer, FieldUsage.Field);
+        parsers.put(name, field);
+        return field;
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareConstructorArg(String name, Function<Object, T> transform) {
+    public <T> Field declareConstructorArg(String name, Function<Object, T> transform) {
         final int position = constructorArgs.size();
         BiConsumer<Object[], Object> objConsumer = (array, object) -> array[position] = transform.apply(object);
+        FieldDefinition<Object[]> field = new FieldDefinition<>(objConsumer, FieldUsage.Constructor);
         try {
-            constructorArgs.put(name, objConsumer);
+            constructorArgs.put(name, field);
         } catch (UnsupportedOperationException e) {
             // This will be thrown when this ConstructingObjectParser is created with a Supplier (which takes no arguments)
             // for example, new ConstructingObjectParser<>((Supplier<String>) String::new)
             throw new UnsupportedOperationException("Cannot add constructor args because the constructor doesn't take any arguments!");
         }
+        return field;
     }
 
     /**
@@ -189,8 +196,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param consumer the function to call once the value is available
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareInteger(String name, BiConsumer<Value, Integer> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformInteger);
+    public Field declareInteger(String name, BiConsumer<Value, Integer> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformInteger);
     }
 
     /**
@@ -199,8 +206,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param name the name of the field.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareInteger(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformInteger);
+    public Field declareInteger(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformInteger);
     }
 
     /**
@@ -210,8 +217,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param consumer the function to call once the value is available
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareString(String name, BiConsumer<Value, String> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformString);
+    public Field declareString(String name, BiConsumer<Value, String> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformString);
     }
 
     /**
@@ -220,8 +227,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param name the name of this field.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareString(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformString);
+    public Field declareString(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformString);
     }
 
     /**
@@ -232,8 +239,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param <T> the type stored in the List.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareList(String name, BiConsumer<Value, List<T>> consumer, Function<Object, T> transform) {
-        declareField(name, consumer, object -> transformList(object, transform));
+    public <T> Field declareList(String name, BiConsumer<Value, List<T>> consumer, Function<Object, T> transform) {
+        return declareField(name, consumer, object -> transformList(object, transform));
     }
 
     /**
@@ -244,8 +251,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param <T>       The type of object contained in the list.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareList(String name, Function<Object, T> transform) {
-        declareConstructorArg(name, (object) -> transformList(object, transform));
+    public <T> Field declareList(String name, Function<Object, T> transform) {
+        return declareConstructorArg(name, (object) -> transformList(object, transform));
     }
 
     /**
@@ -254,33 +261,33 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param name the name of the argument
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareFloat(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformFloat);
+    public Field declareFloat(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformFloat);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareFloat(String name, BiConsumer<Value, Float> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformFloat);
+    public Field declareFloat(String name, BiConsumer<Value, Float> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformFloat);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareDouble(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformDouble);
+    public Field declareDouble(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformDouble);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareDouble(String name, BiConsumer<Value, Double> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformDouble);
+    public Field declareDouble(String name, BiConsumer<Value, Double> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformDouble);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareBoolean(String name) {
-        declareConstructorArg(name, ConstructingObjectParser::transformBoolean);
+    public Field declareBoolean(String name) {
+        return declareConstructorArg(name, ConstructingObjectParser::transformBoolean);
     }
 
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public void declareBoolean(String name, BiConsumer<Value, Boolean> consumer) {
-        declareField(name, consumer, ConstructingObjectParser::transformBoolean);
+    public Field declareBoolean(String name, BiConsumer<Value, Boolean> consumer) {
+        return declareField(name, consumer, ConstructingObjectParser::transformBoolean);
     }
 
     /**
@@ -292,8 +299,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param <T> The type of object to store as the value.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareObject(String name, BiConsumer<Value, T> consumer, ConstructingObjectParser<T> parser) {
-        declareField(name, consumer, (t) -> transformObject(t, parser));
+    public <T> Field declareObject(String name, BiConsumer<Value, T> consumer, ConstructingObjectParser<T> parser) {
+        return declareField(name, consumer, (t) -> transformObject(t, parser));
     }
 
     /**
@@ -304,8 +311,8 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
      * @param <T>    The type of object created by the parser.
      */
     @SuppressWarnings("WeakerAccess") // Public Interface
-    public <T> void declareObject(String name, ConstructingObjectParser<T> parser) {
-        declareConstructorArg(name, (t) -> transformObject(t, parser));
+    public <T> Field declareObject(String name, ConstructingObjectParser<T> parser) {
+        return declareConstructorArg(name, (t) -> transformObject(t, parser));
     }
 
     /**
@@ -335,11 +342,11 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
                 continue;
             }
 
-            BiConsumer<Value, Object> parser = parsers.get(name);
-            assert parser != null;
+            FieldDefinition<Value> field = parsers.get(name);
+            assert field != null;
 
             try {
-                parser.accept(value, entry.getValue());
+                field.accept(value, entry.getValue());
             } catch (IllegalArgumentException e) {
                 throw new IllegalArgumentException("Field " + name + ": " + e.getMessage(), e);
             }
@@ -366,11 +373,18 @@ public class ConstructingObjectParser<Value> implements Function<Map<String, Obj
         Object[] args = new Object[constructorArgs.size()];
 
         // Constructor arguments. Any constructor argument is a *required* setting.
-        for (Map.Entry<String, BiConsumer<Object[], Object>> argInfo : constructorArgs.entrySet()) {
+        for (Map.Entry<String, FieldDefinition<Object[]>> argInfo : constructorArgs.entrySet()) {
             String name = argInfo.getKey();
-            BiConsumer<Object[], Object> argsBuilder = argInfo.getValue();
+            FieldDefinition<Object[]> field = argInfo.getValue();
+
             if (config.containsKey(name)) {
-                argsBuilder.accept(args, config.get(name));
+                if (field.isObsolete()) {
+                    throw new IllegalArgumentException("Field '" + name + "' is obsolete and may not be used. " + field.getDetails());
+                } else if (field.isDeprecated()) {
+                    logger.warn("Field '" + name + "' is deprecated and should be avoided. " + field.getDetails());
+                }
+
+                field.accept(args, config.get(name));
             } else {
                 throw new IllegalArgumentException("Missing required argument '" + name + "' for " + getClass());
             }

logstash-core/src/main/java/org/logstash/plugin/Field.java

@@ -0,0 +1,7 @@
+package org.logstash.plugin;
+
+public interface Field {
+    Field setDeprecated(String details);
+
+    Field setObsolete(String details);
+}

logstash-core/src/main/java/org/logstash/plugin/FieldDefinition.java

@@ -0,0 +1,53 @@
+package org.logstash.plugin;
+
+import java.util.function.BiConsumer;
+
+class FieldDefinition<Value> implements Field, BiConsumer<Value, Object> {
+    private final FieldUsage usage;
+    private final BiConsumer<Value, Object> consumer;
+
+    private String details;
+    private FieldStatus status;
+
+    FieldDefinition(BiConsumer<Value, Object> consumer, FieldUsage usage) {
+        this.consumer = consumer;
+        this.usage = usage;
+    }
+
+    @Override
+    public Field setDeprecated(String details) {
+        setStatus(FieldStatus.Deprecated, details);
+        return this;
+    }
+
+    @Override
+    public Field setObsolete(String details) {
+        if (usage == FieldUsage.Constructor) {
+            throw new IllegalArgumentException("Constructor arguments cannot be made obsolete.");
+        }
+        setStatus(FieldStatus.Obsolete, details);
+        return this;
+    }
+
+    private void setStatus(FieldStatus status, String details) {
+        this.status = status;
+        this.details = details;
+    }
+
+    @Override
+    public void accept(Value value, Object object) {
+        consumer.accept(value, object);
+    }
+
+    boolean isDeprecated() {
+        return status == FieldStatus.Deprecated;
+    }
+
+    boolean isObsolete() {
+        return status == FieldStatus.Obsolete;
+    }
+
+    String getDetails() {
+        return details;
+    }
+}

logstash-core/src/main/java/org/logstash/plugin/FieldStatus.java

@@ -0,0 +1,7 @@
+package org.logstash.plugin;
+
+enum FieldStatus {
+    Supported,
+    Deprecated,
+    Obsolete
+}

logstash-core/src/main/java/org/logstash/plugin/FieldUsage.java

@@ -0,0 +1,6 @@
+package org.logstash.plugin;
+
+public enum FieldUsage {
+    Constructor,
+    Field
+}

logstash-core/src/test/java/org/logstash/plugin/ConstructingObjectParserTest.java

@@ -1,5 +1,6 @@
 package org.logstash.plugin;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
 import org.junit.runner.RunWith;
@@ -14,31 +15,35 @@ import static org.junit.runners.Parameterized.Parameters;
 
 @RunWith(Enclosed.class)
 public class ConstructingObjectParserTest {
+    @SuppressWarnings("unused")
+    private static void check(Field field) {
+        // Exists to do return type compile-time checks.
+        // no body is needed
+    }
     public static class FieldIntegrationTest {
-        static final ConstructingObjectParser<Example> EXAMPLE_BUILDER = new ConstructingObjectParser<>(Example::new);
-        static final ConstructingObjectParser<Path> PATH_BUILDER = new ConstructingObjectParser<>(args -> Paths.get((String) args[0]));
-
-        static {
-            PATH_BUILDER.declareString("path");
-
-            EXAMPLE_BUILDER.declareFloat("float", Example::setF);
-            EXAMPLE_BUILDER.declareInteger("integer", Example::setI);
-            EXAMPLE_BUILDER.declareLong("long", Example::setL);
-            EXAMPLE_BUILDER.declareDouble("double", Example::setD);
-            EXAMPLE_BUILDER.declareBoolean("boolean", Example::setB);
-            EXAMPLE_BUILDER.declareString("string", Example::setS);
-            EXAMPLE_BUILDER.declareList("stringList", Example::setStringList, ConstructingObjectParser::transformString);
-
-            // Custom transform (Object => Path)
-            EXAMPLE_BUILDER.declareString("path", (example, path) -> example.setPath(Paths.get(path)));
-
-            // Custom nested object constructor: { "object": { "path": "some path" } }
-            EXAMPLE_BUILDER.declareObject("object", Example::setPath, PATH_BUILDER);
-        }
+        private final ConstructingObjectParser<Example> EXAMPLE_BUILDER = new ConstructingObjectParser<>(Example::new);
+        private final ConstructingObjectParser<Path> PATH_BUILDER = new ConstructingObjectParser<>(args -> Paths.get((String) args[0]));
 
         private final Map<String, Object> config = new HashMap<>();
 
-        public FieldIntegrationTest() {
+
+        @Before
+        public void setup() {
+            check(PATH_BUILDER.declareString("path"));
+            check(EXAMPLE_BUILDER.declareFloat("float", Example::setF));
+            check(EXAMPLE_BUILDER.declareInteger("integer", Example::setI));
+            check(EXAMPLE_BUILDER.declareLong("long", Example::setL));
+            check(EXAMPLE_BUILDER.declareDouble("double", Example::setD));
+            check(EXAMPLE_BUILDER.declareBoolean("boolean", Example::setB));
+            check(EXAMPLE_BUILDER.declareString("string", Example::setS));
+            check(EXAMPLE_BUILDER.declareList("stringList", Example::setStringList, ConstructingObjectParser::transformString));
+
+            // Custom transform (Object => Path)
+            check(EXAMPLE_BUILDER.declareString("path", (example, path) -> example.setPath(Paths.get(path))));
+
+            // Custom nested object constructor: { "object": { "path": "some path" } }
+            check(EXAMPLE_BUILDER.declareObject("object", Example::setPath, PATH_BUILDER));
+
             config.put("float", 1F);
             config.put("integer", 1);
             config.put("long", 1L);
@@ -78,7 +83,7 @@ public class ConstructingObjectParserTest {
         public void testInvalidConstructor() {
             // EXAMPLE_BUILDER gives a Supplier (not a Function<Object[], ...>), so constructor arguments
             // should be disabled and this call should fail.
-            EXAMPLE_BUILDER.declareString("invalid");
+            check(EXAMPLE_BUILDER.declareString("invalid"));
         }
 
         private static class Example {
@@ -93,11 +98,11 @@ public class ConstructingObjectParserTest {
             private List<String> stringList;
             private Path path;
 
-            public List<String> getStringList() {
+            List<String> getStringList() {
                 return stringList;
             }
 
-            public void setStringList(List<String> stringList) {
+            void setStringList(List<String> stringList) {
                 this.stringList = stringList;
             }
 
@@ -161,32 +166,30 @@ public class ConstructingObjectParserTest {
     }
 
     public static class ConstructorIntegrationTest {
-        static final ConstructingObjectParser<Example> EXAMPLE_BUILDER = new ConstructingObjectParser<>(args -> new Example((int) args[0], (float) args[1], (long) args[2], (double) args[3], (boolean) args[4], (String) args[5], (Path) args[6], (Path) args[7], (List<String>) args[8]));
-        static final ConstructingObjectParser<Path> PATH_BUILDER = new ConstructingObjectParser<>(args -> Paths.get((String) args[0]));
-
-        static {
-            PATH_BUILDER.declareString("path");
-
-            EXAMPLE_BUILDER.declareInteger("integer");
-            EXAMPLE_BUILDER.declareFloat("float");
-            EXAMPLE_BUILDER.declareLong("long");
-            EXAMPLE_BUILDER.declareDouble("double");
-            EXAMPLE_BUILDER.declareBoolean("boolean");
-            EXAMPLE_BUILDER.declareString("string");
-
-            // Custom transform (Object => Path)
-            EXAMPLE_BUILDER.declareConstructorArg("path", (object) -> Paths.get((String) object));
-
-            // Custom nested object constructor: { "object": { "path": "some path" } }
-            EXAMPLE_BUILDER.declareObject("object", PATH_BUILDER);
-
-            EXAMPLE_BUILDER.declareList("stringList", ConstructingObjectParser::transformString);
-
-        }
+        private final ConstructingObjectParser<Example> EXAMPLE_BUILDER = new ConstructingObjectParser<>(args -> new Example((int) args[0], (float) args[1], (long) args[2], (double) args[3], (boolean) args[4], (String) args[5], (Path) args[6], (Path) args[7], (List<String>) args[8]));
+        private final ConstructingObjectParser<Path> PATH_BUILDER = new ConstructingObjectParser<>(args -> Paths.get((String) args[0]));
 
         private final Map<String, Object> config = new LinkedHashMap<>();
 
-        public ConstructorIntegrationTest() {
+        @Before
+        public void setup() {
+            check(PATH_BUILDER.declareString("path"));
+
+            check(EXAMPLE_BUILDER.declareInteger("integer"));
+            check(EXAMPLE_BUILDER.declareFloat("float"));
+            check(EXAMPLE_BUILDER.declareLong("long"));
+            check(EXAMPLE_BUILDER.declareDouble("double"));
+            check(EXAMPLE_BUILDER.declareBoolean("boolean"));
+            check(EXAMPLE_BUILDER.declareString("string"));
+
+            // Custom transform (Object => Path)
+            check(EXAMPLE_BUILDER.declareConstructorArg("path", (object) -> Paths.get((String) object)));
+
+            // Custom nested object constructor: { "object": { "path": "some path" } }
+            check(EXAMPLE_BUILDER.declareObject("object", PATH_BUILDER));
+
+            check(EXAMPLE_BUILDER.declareList("stringList", ConstructingObjectParser::transformString));
+
             config.put("float", 1F);
             config.put("integer", 1);
             config.put("long", 1L);
@@ -225,7 +228,7 @@ public class ConstructingObjectParserTest {
             private final Path p1;
             private final Path p2;
 
-            private List<String> stringList;
+            private final List<String> stringList;
 
             Example(int i, float f, long l, double d, boolean b, String s, Path p1, Path p2, List<String> stringList) {
                 this.i = i;
@@ -328,4 +331,29 @@ public class ConstructingObjectParserTest {
             ConstructingObjectParser.transformString(input);
         }
     }
+
+    public static class DeprecationsAndObsoletes {
+        final ConstructingObjectParser<Example> c = new ConstructingObjectParser<>((args) -> new Example());
+
+        @Before
+        public void setup() {
+            c.declareInteger("deprecated").setDeprecated("This setting will warn the user when used.");
+            c.declareInteger("obsolete", (a, b) -> {
+            }).setObsolete("This setting should cause a failure when someone uses it.");
+        }
+
+        @Test
+        public void deprecatedUsageIsAllowed() {
+            // XXX: Implement a custom log appender that captures log4j logs so we can verify the warning is logged.
+            c.apply(Collections.singletonMap("deprecated", 1));
+        }
+
+        @Test(expected = IllegalArgumentException.class)
+        public void obsoletesOnConstructorArgsIsInvalid() {
+            c.declareInteger("fail").setObsolete("...");
+        }
+
+        private static class Example {
+        }
+    }
 }