From e19f5586e2f65bd9b8941fbec8003936f8a86b66 Mon Sep 17 00:00:00 2001 From: andsel Date: Mon, 31 Mar 2025 16:50:46 +0200 Subject: [PATCH 1/2] Moved Integer Ruby class to Java --- logstash-core/lib/logstash/settings.rb | 35 ++++++++++-------- .../org/logstash/settings/SettingInteger.java | 37 +++++++++++++++++++ .../logstash/settings/SettingIntegerTest.java | 28 ++++++++++++++ 3 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 logstash-core/src/main/java/org/logstash/settings/SettingInteger.java create mode 100644 logstash-core/src/test/java/org/logstash/settings/SettingIntegerTest.java diff --git a/logstash-core/lib/logstash/settings.rb b/logstash-core/lib/logstash/settings.rb index e879613e7d0..d9ff983cdbd 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/logstash-core/lib/logstash/settings.rb @@ -415,23 +415,26 @@ 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 + java_import org.logstash.settings.SettingInteger + # Integer = org::logstash::settings::SettingInteger - if coerced_value.nil? - raise ArgumentError.new("Failed to coerce value to Integer. Received #{value} (#{value.class})") - else - coerced_value - end - end - end + # 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) 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..82cc06c581c --- /dev/null +++ b/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java @@ -0,0 +1,37 @@ +package org.logstash.settings; + +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()); + } + + @Override + public Integer coerce(Object obj) { + if (!(obj instanceof String)) { + // it's an Integer and cast + if (obj instanceof Integer) { + return (Integer) obj; + } + } 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/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 From 500ee82da8197ea0fa24e58598bb5b6e7b24b780 Mon Sep 17 00:00:00 2001 From: andsel Date: Tue, 1 Apr 2025 10:41:47 +0200 Subject: [PATCH 2/2] Moved PositiveInteger Ruby class to Java SettingPositiveInteger --- logstash-core/lib/logstash/environment.rb | 4 +-- logstash-core/lib/logstash/settings.rb | 26 ++++++++++--------- .../spec/logstash/pipeline_pq_file_spec.rb | 10 ++++++- .../spec/logstash/queue_factory_spec.rb | 4 +-- .../spec/logstash/settings/integer_spec.rb | 4 +-- .../org/logstash/settings/SettingInteger.java | 11 ++++++++ .../settings/SettingPositiveInteger.java | 18 +++++++++++++ 7 files changed, 58 insertions(+), 19 deletions(-) create mode 100644 logstash-core/src/main/java/org/logstash/settings/SettingPositiveInteger.java 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 d9ff983cdbd..3c69862f1e7 100644 --- a/logstash-core/lib/logstash/settings.rb +++ b/logstash-core/lib/logstash/settings.rb @@ -436,19 +436,21 @@ def coerce(value) # 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.SettingPositiveInteger - class Port < Integer + # 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 index 82cc06c581c..68351f7d492 100644 --- a/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java +++ b/logstash-core/src/main/java/org/logstash/settings/SettingInteger.java @@ -1,5 +1,7 @@ package org.logstash.settings; +import java.util.function.Predicate; + public class SettingInteger extends Coercible { public SettingInteger(String name, Integer defaultValue) { @@ -11,6 +13,11 @@ 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)) { @@ -18,6 +25,10 @@ public Integer coerce(Object obj) { 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 { 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; + } + }); + } +}