diff --git a/logstash-core/lib/logstash/environment.rb b/logstash-core/lib/logstash/environment.rb index a29faff6ef6..3058536dac3 100644 --- a/logstash-core/lib/logstash/environment.rb +++ b/logstash-core/lib/logstash/environment.rb @@ -46,8 +46,8 @@ module Environment Setting::Boolean.new("metric.collect", true), Setting::SettingString.new("pipeline.id", "main"), Setting::Boolean.new("pipeline.system", false), - Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum), - Setting::PositiveInteger.new("pipeline.batch.size", 125), + Setting::SettingPositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum), + Setting::SettingPositiveInteger.new("pipeline.batch.size", 125), Setting::SettingNumeric.new("pipeline.batch.delay", 50), # in milliseconds Setting::Boolean.new("pipeline.unsafe_shutdown", false), Setting::Boolean.new("pipeline.reloadable", true), diff --git a/logstash-core/lib/logstash/settings.rb b/logstash-core/lib/logstash/settings.rb index e879613e7d0..3c69862f1e7 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/logstash-core/lib/logstash/settings.rb @@ -415,37 +415,42 @@ def coerce(value) java_import org.logstash.settings.Boolean java_import org.logstash.settings.SettingNumeric - class Integer < Coercible - def initialize(name, default = nil, strict = true) - super(name, ::Integer, default, strict) - end - - def coerce(value) - return value unless value.is_a?(::String) - - coerced_value = Integer(value) rescue nil - - if coerced_value.nil? - raise ArgumentError.new("Failed to coerce value to Integer. Received #{value} (#{value.class})") - else - coerced_value - end - end - end - - class PositiveInteger < Integer - def initialize(name, default = nil, strict = true) - super(name, default, strict) do |v| - if v > 0 - true - else - raise ArgumentError.new("Number must be bigger than 0. Received: #{v}") - end - end - end - end + java_import org.logstash.settings.SettingInteger + # Integer = org::logstash::settings::SettingInteger - class Port < Integer + # class Integer < Coercible + # def initialize(name, default = nil, strict = true) + # super(name, ::Integer, default, strict) + # end + # + # def coerce(value) + # return value unless value.is_a?(::String) + # + # coerced_value = Integer(value) rescue nil + # + # if coerced_value.nil? + # raise ArgumentError.new("Failed to coerce value to Integer. Received #{value} (#{value.class})") + # else + # coerced_value + # end + # end + # end + + java_import org.logstash.settings.SettingPositiveInteger + + # class PositiveInteger < Integer + # def initialize(name, default = nil, strict = true) + # super(name, default, strict) do |v| + # if v > 0 + # true + # else + # raise ArgumentError.new("Number must be bigger than 0. Received: #{v}") + # end + # end + # end + # end + + class Port < SettingInteger VALID_PORT_RANGE = 1..65535 def initialize(name, default = nil, strict = true) diff --git a/logstash-core/spec/logstash/pipeline_pq_file_spec.rb b/logstash-core/spec/logstash/pipeline_pq_file_spec.rb index fe0f80fe68e..acfa62f8609 100644 --- a/logstash-core/spec/logstash/pipeline_pq_file_spec.rb +++ b/logstash-core/spec/logstash/pipeline_pq_file_spec.rb @@ -103,6 +103,7 @@ def close let(:collected_metric) { metric_store.get_with_path("stats/pipelines/") } before :each do + puts "DNADBG>> setup test - start" FileUtils.mkdir_p(this_queue_folder) pipeline_settings_obj.set("path.queue", this_queue_folder) @@ -112,14 +113,21 @@ def close allow(LogStash::Plugin).to receive(:lookup).with("filter", "dummyfilter").and_return(LogStash::Filters::DummyFilter) allow(LogStash::Plugin).to receive(:lookup).with("output", "pipelinepqfileoutput").and_return(PipelinePqFileOutput) + puts "DNADBG>> setup test - workers read" pipeline_workers_setting = LogStash::SETTINGS.get_setting("pipeline.workers") allow(pipeline_workers_setting).to receive(:default).and_return(worker_thread_count) - pipeline_settings.each {|k, v| pipeline_settings_obj.set(k, v) } + puts "DNADBG>> setup test - workers read 2" + pipeline_settings.each {|k, v| + puts "DNADBG>> setup test - workers read 2 before set #{k}" + pipeline_settings_obj.set(k, v) + } + puts "DNADBG>> setup test - workers read 3" pipeline_settings_obj.set("queue.page_capacity", page_capacity) pipeline_settings_obj.set("queue.max_bytes", max_bytes) pipeline_settings_obj.set("queue.drain", true) times.push(Time.now.to_f) + puts "DNADBG>> before start" subject.start sleep(0.1) until subject.ready? diff --git a/logstash-core/spec/logstash/queue_factory_spec.rb b/logstash-core/spec/logstash/queue_factory_spec.rb index cae6b1fe7dd..3a5fa016035 100644 --- a/logstash-core/spec/logstash/queue_factory_spec.rb +++ b/logstash-core/spec/logstash/queue_factory_spec.rb @@ -32,8 +32,8 @@ 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::Setting::PositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum) + LogStash::Setting::SettingPositiveInteger.new("pipeline.batch.size", 125), + LogStash::Setting::SettingPositiveInteger.new("pipeline.workers", LogStash::Config::CpuCoreStrategy.maximum) ] end diff --git a/logstash-core/spec/logstash/settings/integer_spec.rb b/logstash-core/spec/logstash/settings/integer_spec.rb index c301576fd87..e1c1e424afc 100644 --- a/logstash-core/spec/logstash/settings/integer_spec.rb +++ b/logstash-core/spec/logstash/settings/integer_spec.rb @@ -18,12 +18,12 @@ require "spec_helper" require "logstash/settings" -describe LogStash::Setting::Integer do +describe LogStash::Setting::SettingInteger do subject { described_class.new("a number", nil, false) } describe "#set" do context "when giving a number which is not an integer" do it "should raise an exception" do - expect { subject.set(1.1) }.to raise_error(ArgumentError) + expect { subject.set(1.1) }.to raise_error(java.lang.IllegalArgumentException) end end context "when giving a number which is an integer" do diff --git a/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java b/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java new file mode 100644 index 00000000000..68351f7d492 --- /dev/null +++ b/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java @@ -0,0 +1,48 @@ +package org.logstash.settings; + +import java.util.function.Predicate; + +public class SettingInteger extends Coercible { + + public SettingInteger(String name, Integer defaultValue) { + super(name, defaultValue, true, noValidator()); + } + + // constructor used only in tests, but needs to be public to be used in Ruby spec + public SettingInteger(String name, Integer defaultValue, boolean strict) { + super(name, defaultValue, strict, noValidator()); + } + + // Exposed to be redefined in subclasses + protected SettingInteger(String name, Integer defaultValue, boolean strict, Predicate validator) { + super(name, defaultValue, strict, validator); + } + + @Override + public Integer coerce(Object obj) { + if (!(obj instanceof String)) { + // it's an Integer and cast + if (obj instanceof Integer) { + return (Integer) obj; + } + // JRuby bridge convert ints to Long + if (obj instanceof Long) { + return ((Long) obj).intValue(); + } + } else { + // try to parse string to int + try { + return Integer.parseInt(obj.toString()); + } catch (NumberFormatException e) { + // ugly flow control + } + } + + // invalid coercion + throw new IllegalArgumentException(coercionFailureMessage(obj)); + } + + private String coercionFailureMessage(Object obj) { + return String.format("Failed to coerce value to SettingInteger. Received %s (%s)", obj, obj.getClass()); + } +} diff --git a/logstash-core/src/main/java/org/logstash/settings/SettingPositiveInteger.java b/logstash-core/src/main/java/org/logstash/settings/SettingPositiveInteger.java new file mode 100644 index 00000000000..fae4def237a --- /dev/null +++ b/logstash-core/src/main/java/org/logstash/settings/SettingPositiveInteger.java @@ -0,0 +1,18 @@ +package org.logstash.settings; + +import java.util.function.Predicate; + +public class SettingPositiveInteger extends SettingInteger { + + public SettingPositiveInteger(String name, Integer defaultValue) { + super(name, defaultValue, true, new Predicate() { + @Override + public boolean test(Integer v) { + if (v <= 0) { + throw new IllegalArgumentException("Number must be bigger than 0. Received: " + v); + } + return true; + } + }); + } +} diff --git a/logstash-core/src/test/java/org/logstash/settings/SettingIntegerTest.java b/logstash-core/src/test/java/org/logstash/settings/SettingIntegerTest.java new file mode 100644 index 00000000000..2da67b8ef79 --- /dev/null +++ b/logstash-core/src/test/java/org/logstash/settings/SettingIntegerTest.java @@ -0,0 +1,28 @@ +package org.logstash.settings; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class SettingIntegerTest { + + private SettingInteger sut; + + @Before + public void setUp() { + sut = new SettingInteger("a number", null, false); + } + + @Test(expected = IllegalArgumentException.class) + public void givenNumberWhichIsNotIntegerWhenSetIsInvokedThrowsException() { + sut.set(1.1); + } + + @Test + public void givenNumberWhichIsIntegerWhenSetIsInvokedThenShouldSetTheNumber() { + sut.set(100); + + Assert.assertEquals(Integer.valueOf(100), sut.value()); + } + +} \ No newline at end of file