diff --git a/logstash-core/lib/logstash/environment.rb b/logstash-core/lib/logstash/environment.rb index e3a04b27cff..a29faff6ef6 100644 --- a/logstash-core/lib/logstash/environment.rb +++ b/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"), diff --git a/logstash-core/lib/logstash/settings.rb b/logstash-core/lib/logstash/settings.rb index 3696886cd1c..e879613e7d0 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/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) diff --git a/logstash-core/spec/logstash/queue_factory_spec.rb b/logstash-core/spec/logstash/queue_factory_spec.rb index 72b01579a32..cae6b1fe7dd 100644 --- a/logstash-core/spec/logstash/queue_factory_spec.rb +++ b/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), diff --git a/logstash-core/spec/logstash/settings/numeric_spec.rb b/logstash-core/spec/logstash/settings/numeric_spec.rb index 29bbe2f782f..78e16ffa0f0 100644 --- a/logstash-core/spec/logstash/settings/numeric_spec.rb +++ b/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 diff --git a/logstash-core/src/main/java/org/logstash/settings/SettingNumeric.java b/logstash-core/src/main/java/org/logstash/settings/SettingNumeric.java new file mode 100644 index 00000000000..f2e71be8963 --- /dev/null +++ b/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 { + + 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()); + } +} diff --git a/logstash-core/src/test/java/org/logstash/settings/SettingNumericTest.java b/logstash-core/src/test/java/org/logstash/settings/SettingNumericTest.java new file mode 100644 index 00000000000..a7ee796d6c4 --- /dev/null +++ b/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); + } + +} \ No newline at end of file