Reimplement LogStash::Numeric setting in Java (elastic#17127)

Reimplements `LogStash::Setting::Numeric` Ruby setting class into the `org.logstash.settings.NumericSetting` and exposes it through `java_import` as `LogStash::Setting::NumericSetting`.
Updates the rspec tests:
- verifies `java.lang.IllegalArgumentException` instead of `ArgumentError` is thrown because the kind of exception thrown by Java code, during verification.

Author: andsel

logstash-core/lib/logstash/environment.rb

@@ -48,7 +48,7 @@ module Environment
            Setting::Boolean.new("pipeline.system", false),
    Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum),
    Setting::PositiveInteger.new("pipeline.batch.size", 125),
-           Setting::Numeric.new("pipeline.batch.delay", 50), # in milliseconds
+           Setting::SettingNumeric.new("pipeline.batch.delay", 50), # in milliseconds
            Setting::Boolean.new("pipeline.unsafe_shutdown", false),
            Setting::Boolean.new("pipeline.reloadable", true),
            Setting::Boolean.new("pipeline.plugin_classloaders", false),
@@ -72,7 +72,7 @@ module Environment
             Setting::SettingString.new("api.auth.basic.username", nil, false).nullable,
           Setting::Password.new("api.auth.basic.password", nil, false).nullable,
             Setting::SettingString.new("api.auth.basic.password_policy.mode", "WARN", true, %w[WARN ERROR]),
-           Setting::Numeric.new("api.auth.basic.password_policy.length.minimum", 8),
+           Setting::SettingNumeric.new("api.auth.basic.password_policy.length.minimum", 8),
             Setting::SettingString.new("api.auth.basic.password_policy.include.upper", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
             Setting::SettingString.new("api.auth.basic.password_policy.include.lower", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
             Setting::SettingString.new("api.auth.basic.password_policy.include.digit", "REQUIRED", true, %w[REQUIRED OPTIONAL]),
@@ -85,14 +85,14 @@ module Environment
            Setting::Boolean.new("queue.drain", false),
              Setting::Bytes.new("queue.page_capacity", "64mb"),
              Setting::Bytes.new("queue.max_bytes", "1024mb"),
-           Setting::Numeric.new("queue.max_events", 0), # 0 is unlimited
-           Setting::Numeric.new("queue.checkpoint.acks", 1024), # 0 is unlimited
-           Setting::Numeric.new("queue.checkpoint.writes", 1024), # 0 is unlimited
-           Setting::Numeric.new("queue.checkpoint.interval", 1000), # 0 is no time-based checkpointing
+           Setting::SettingNumeric.new("queue.max_events", 0), # 0 is unlimited
+           Setting::SettingNumeric.new("queue.checkpoint.acks", 1024), # 0 is unlimited
+           Setting::SettingNumeric.new("queue.checkpoint.writes", 1024), # 0 is unlimited
+           Setting::SettingNumeric.new("queue.checkpoint.interval", 1000), # 0 is no time-based checkpointing
            Setting::Boolean.new("queue.checkpoint.retry", true),
            Setting::Boolean.new("dead_letter_queue.enable", false),
              Setting::Bytes.new("dead_letter_queue.max_bytes", "1024mb"),
-           Setting::Numeric.new("dead_letter_queue.flush_interval", 5000),
+           Setting::SettingNumeric.new("dead_letter_queue.flush_interval", 5000),
             Setting::SettingString.new("dead_letter_queue.storage_policy", "drop_newer", true, ["drop_newer", "drop_older"]),
     Setting::SettingNullableString.new("dead_letter_queue.retain.age"), # example 5d
          Setting::TimeValue.new("slowlog.threshold.warn", "-1"),

logstash-core/lib/logstash/settings.rb

@@ -413,26 +413,7 @@ def coerce(value)
     ### Specific settings #####
 
     java_import org.logstash.settings.Boolean
-
-    class Numeric < Coercible
-      def initialize(name, default = nil, strict = true)
-        super(name, ::Numeric, default, strict)
-      end
-
-      def coerce(v)
-        return v if v.is_a?(::Numeric)
-
-        # I hate these "exceptions as control flow" idioms
-        # but Ruby's `"a".to_i => 0` makes it hard to do anything else.
-        coerced_value = (Integer(v) rescue nil) || (Float(v) rescue nil)
-
-        if coerced_value.nil?
-          raise ArgumentError.new("Failed to coerce value to Numeric. Received #{v} (#{v.class})")
-        else
-          coerced_value
-        end
-      end
-    end
+    java_import org.logstash.settings.SettingNumeric
 
     class Integer < Coercible
       def initialize(name, default = nil, strict = true)

logstash-core/spec/logstash/queue_factory_spec.rb

@@ -26,10 +26,10 @@
       LogStash::Setting::SettingString.new("queue.type", "memory", true, ["persisted", "memory"]),
       LogStash::Setting::Bytes.new("queue.page_capacity", "8mb"),
       LogStash::Setting::Bytes.new("queue.max_bytes", "64mb"),
-      LogStash::Setting::Numeric.new("queue.max_events", 0),
-      LogStash::Setting::Numeric.new("queue.checkpoint.acks", 1024),
-      LogStash::Setting::Numeric.new("queue.checkpoint.writes", 1024),
-      LogStash::Setting::Numeric.new("queue.checkpoint.interval", 1000),
+      LogStash::Setting::SettingNumeric.new("queue.max_events", 0),
+      LogStash::Setting::SettingNumeric.new("queue.checkpoint.acks", 1024),
+      LogStash::Setting::SettingNumeric.new("queue.checkpoint.writes", 1024),
+      LogStash::Setting::SettingNumeric.new("queue.checkpoint.interval", 1000),
       LogStash::Setting::Boolean.new("queue.checkpoint.retry", false),
       LogStash::Setting::SettingString.new("pipeline.id", pipeline_id),
       LogStash::Setting::PositiveInteger.new("pipeline.batch.size", 125),

logstash-core/spec/logstash/settings/numeric_spec.rb

@@ -18,19 +18,20 @@
 require "spec_helper"
 require "logstash/settings"
 
-describe LogStash::Setting::Numeric do
+# Mirrored in java class org.logstash.settings.SettingNumeric
+describe LogStash::Setting::SettingNumeric do
   subject { described_class.new("a number", nil, false) }
   describe "#set" do
     context "when giving a string which doesn't represent a string" do
       it "should raise an exception" do
-        expect { subject.set("not-a-number") }.to raise_error(ArgumentError)
+        expect { subject.set("not-a-number") }.to raise_error(java.lang.IllegalArgumentException)
       end
     end
     context "when giving a string which represents a " do
       context "float" do
         it "should coerce that string to the number" do
           subject.set("1.1")
-          expect(subject.value).to eq(1.1)
+          expect(subject.value).to be_within(0.01).of(1.1)
         end
       end
       context "int" do

logstash-core/src/main/java/org/logstash/settings/SettingNumeric.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *	http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.logstash.settings;
+
+public class SettingNumeric extends Coercible<Number> {
+
+    public SettingNumeric(String name, Number defaultValue) {
+        super(name, defaultValue, true, noValidator());
+    }
+
+    // constructor used only in tests, but needs to be public to be used in Ruby spec
+    public SettingNumeric(String name, Number defaultValue, boolean strict) {
+        super(name, defaultValue, strict, noValidator());
+    }
+
+    @Override
+    public Number coerce(Object obj) {
+        if (obj == null) {
+            throw new IllegalArgumentException("Failed to coerce value to SettingNumeric. Received null");
+        }
+        if (obj instanceof Number) {
+            return (Number) obj;
+        }
+        try {
+            return Integer.parseInt(obj.toString());
+        } catch (NumberFormatException e) {
+            // ugly flow control
+        }
+        try {
+            return Float.parseFloat(obj.toString());
+        } catch (NumberFormatException e) {
+            // ugly flow control
+        }
+
+        // no integer neither float parsing succeed, invalid coercion
+        throw new IllegalArgumentException(coercionFailureMessage(obj));
+    }
+
+    private String coercionFailureMessage(Object obj) {
+        return String.format("Failed to coerce value to SettingNumeric. Received %s (%s)", obj, obj.getClass());
+    }
+}

logstash-core/src/test/java/org/logstash/settings/SettingNumericTest.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *	http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.logstash.settings;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+// Mirrored from spec/logstash/settings/numeric_spec.rb
+public class SettingNumericTest {
+
+    private SettingNumeric sut;
+
+    @Before
+    public void setUp() {
+        sut = new SettingNumeric("a number", null, false);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void givenValueThatIsNotStringWhenSetIsInvokedThrowsException() {
+        sut.set("not-a-number");
+    }
+
+    @Test
+    public void givenValueStringThatRepresentFloatWhenSetIsInvokedShouldCoerceThatStringToTheNumber() {
+        sut.set("1.1");
+
+        float value = (Float) sut.value();
+        assertEquals(1.1f, value, 0.001);
+    }
+
+    @Test
+    public void givenValueStringThatRepresentIntegerWhenSetIsInvokedShouldCoerceThatStringToTheNumber() {
+        sut.set("1");
+
+        int value = (Integer) sut.value();
+        assertEquals(1, value);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void givenNullValueThenCoerceThrowSpecificError() {
+        sut.set(null);
+    }
+
+}