Skip to content

Commit

Permalink
Reimplement LogStash::Numeric setting in Java (#17127)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andsel authored Mar 6, 2025
1 parent a736178 commit 07a3c8e
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 34 deletions.
14 changes: 7 additions & 7 deletions logstash-core/lib/logstash/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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]),
Expand All @@ -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"),
Expand Down
21 changes: 1 addition & 20 deletions logstash-core/lib/logstash/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions logstash-core/spec/logstash/queue_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
7 changes: 4 additions & 3 deletions logstash-core/spec/logstash/settings/numeric_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
@@ -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);
}

}

0 comments on commit 07a3c8e

Please sign in to comment.