Skip to content

Convert Ruby Integer and PositiveInteger settings classes to Java #17460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
65 changes: 35 additions & 30 deletions logstash-core/lib/logstash/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion logstash-core/spec/logstash/pipeline_pq_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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?

Expand Down
4 changes: 2 additions & 2 deletions logstash-core/spec/logstash/queue_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions logstash-core/spec/logstash/settings/integer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.logstash.settings;

import java.util.function.Predicate;

public class SettingInteger extends Coercible<Integer> {

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<Integer> 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());
}
}
Original file line number Diff line number Diff line change
@@ -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<Integer>() {
@Override
public boolean test(Integer v) {
if (v <= 0) {
throw new IllegalArgumentException("Number must be bigger than 0. Received: " + v);
}
return true;
}
});
}
}
Original file line number Diff line number Diff line change
@@ -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());
}

}