Move Port and PortRange Ruby settings to Java (#17964)

Move the existing Ruby code to Port and PortRange classes to Java, leveraging the JRuby bridge and avoiding to use the JRuby extension mechanism.

Co-authored-by: João Duarte <[email protected]>

Author: andsel

logstash-core/lib/logstash/environment.rb

@@ -33,6 +33,10 @@ module Environment
     end
   end
 
+  def self.as_java_range(r)
+    org::logstash::settings::Range.new(r.first.to_java(java.lang.Integer), r.last.to_java(java.lang.Integer))
+  end
+
   [
            Setting::Boolean.new("allow_superuser", false),
             Setting::SettingString.new("node.name", Socket.gethostname),
@@ -67,7 +71,7 @@ module Environment
            Setting::Boolean.new("log.format.json.fix_duplicate_message_fields", true),
            Setting::Boolean.new("api.enabled", true),
             Setting::SettingString.new("api.http.host", "127.0.0.1"),
-         Setting::PortRange.new("api.http.port", 9600..9700),
+         Setting::PortRangeSetting.new("api.http.port", 9600..9700),
             Setting::SettingString.new("api.environment", "production"),
             Setting::SettingString.new("api.auth.type", "none", true, %w(none basic)),
             Setting::SettingString.new("api.auth.basic.username", nil, false).nullable,

logstash-core/lib/logstash/settings.rb

@@ -418,55 +418,10 @@ def coerce(value)
     java_import org.logstash.settings.SettingInteger
 
     java_import org.logstash.settings.SettingPositiveInteger
-    
-    class Port < SettingInteger
-      VALID_PORT_RANGE = 1..65535
-
-      def initialize(name, default = nil, strict = true)
-        super(name, default, strict) { |value| valid?(value) }
-      end
-
-      def valid?(port)
-        VALID_PORT_RANGE.cover?(port)
-      end
-    end
-
-    class PortRange < Coercible
-      PORT_SEPARATOR = "-"
-
-      def initialize(name, default = nil, strict = true)
-        super(name, ::Range, default, strict = true) { |value| valid?(value) }
-      end
-
-      def valid?(range)
-        Port::VALID_PORT_RANGE.first <= range.first && Port::VALID_PORT_RANGE.last >= range.last
-      end
 
-      def coerce(value)
-        case value
-        when ::Range
-          value
-        when ::Integer
-          value..value
-        when ::String
-          first, last = value.split(PORT_SEPARATOR)
-          last = first if last.nil?
-          begin
-            (Integer(first))..(Integer(last))
-          rescue ArgumentError # Trap and reraise a more human error
-            raise ArgumentError.new("Could not coerce #{value} into a port range")
-          end
-        else
-          raise ArgumentError.new("Could not coerce #{value} into a port range")
-        end
-      end
+    java_import org.logstash.settings.PortSetting # seems unused
 
-      def validate(value)
-        unless valid?(value)
-          raise ArgumentError.new("Invalid value \"#{name}: #{value}\", valid options are within the range of #{Port::VALID_PORT_RANGE.first}-#{Port::VALID_PORT_RANGE.last}")
-        end
-      end
-    end
+    java_import org.logstash.settings.PortRangeSetting
 
     class Validator < Setting
       def initialize(name, default = nil, strict = true, validator_class = nil)

logstash-core/spec/logstash/runner_spec.rb

@@ -321,7 +321,7 @@
         it "creates an Agent whose `api.http.port` uses the default value" do
           expect(LogStash::Agent).to receive(:new) do |settings|
             expect(settings.set?("api.http.port")).to be_falsey
-            expect(settings.get("api.http.port")).to eq(9600..9700)
+            expect(settings.get("api.http.port")).to eq(::LogStash.as_java_range(9600..9700))
           end
 
           subject.run("bin/logstash", args)
@@ -335,7 +335,7 @@
           it "creates an Agent whose `api.http.port` is an appropriate single-element range" do
             expect(LogStash::Agent).to receive(:new) do |settings|
               expect(settings.set?("api.http.port")).to be(true)
-              expect(settings.get("api.http.port")).to eq(10000..10000)
+              expect(settings.get("api.http.port")).to eq(::LogStash.as_java_range(10000..10000))
             end
 
             subject.run("bin/logstash", args)
@@ -346,7 +346,7 @@
           it "creates an Agent whose `api.http.port` uses the appropriate inclusive-end range" do
             expect(LogStash::Agent).to receive(:new) do |settings|
               expect(settings.set?("api.http.port")).to be(true)
-              expect(settings.get("api.http.port")).to eq(10000..20000)
+              expect(settings.get("api.http.port")).to eq(::LogStash.as_java_range(10000..20000))
             end
 
             subject.run("bin/logstash", args)

logstash-core/spec/logstash/settings/port_range_spec.rb

@@ -18,87 +18,87 @@
 require "logstash/settings"
 require "spec_helper"
 
-describe LogStash::Setting::PortRange do
+describe LogStash::Setting::PortRangeSetting do
   context "When the value is an Integer" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", 9000) }
+    subject { LogStash::Setting::PortRangeSetting.new("mynewtest", 9000) }
 
     it "coerces the value in a range" do
       expect { subject }.not_to raise_error
     end
 
     it "returns a range" do
-      expect(subject.value).to eq(9000..9000)
+      expect(subject.value).to eq(::LogStash.as_java_range(9000..9000))
     end
 
     it "can update the range" do
       subject.set(10000)
-      expect(subject.value).to eq(10000..10000)
+      expect(subject.value).to eq(::LogStash.as_java_range(10000..10000))
     end
   end
 
   context "When the value is a string" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", "9000-10000") }
+    subject { LogStash::Setting::PortRangeSetting.new("mynewtest", "9000-10000") }
 
     it "coerces a string range with the format (9000-10000)" do
       expect { subject }.not_to raise_error
     end
 
     it "refuses when then upper port is out of range" do
-      expect { LogStash::Setting::PortRange.new("mynewtest", "1000-95000") }.to raise_error
+      expect { LogStash::Setting::PortRangeSetting.new("mynewtest", "1000-95000") }.to raise_error
     end
 
     it "returns a range" do
-      expect(subject.value).to eq(9000..10000)
+      expect(subject.value).to eq(::LogStash.as_java_range(9000..10000))
     end
 
     it "can update the range" do
       subject.set("500-1000")
-      expect(subject.value).to eq(500..1000)
+      expect(subject.value).to eq(::LogStash.as_java_range(500..1000))
     end
   end
 
   context "when the value is a garbage string" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", "fsdfnsdkjnfjs") }
+    subject { LogStash::Setting::PortRangeSetting.new("mynewtest", "fsdfnsdkjnfjs") }
 
     it "raises an argument error" do
       expect { subject }.to raise_error
     end
 
     it "raises an exception on update" do
-      expect { LogStash::Setting::PortRange.new("mynewtest", 10000).set("dsfnsdknfksdnfjksdnfjns") }.to raise_error
+      expect { LogStash::Setting::PortRangeSetting.new("mynewtest", 10000).set("dsfnsdknfksdnfjksdnfjns") }.to raise_error
     end
   end
 
   context "when the value is an unknown type" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", 0.1) }
+    subject { LogStash::Setting::PortRangeSetting.new("mynewtest", 0.1) }
 
     it "raises an argument error" do
       expect { subject }.to raise_error
     end
 
     it "raises an exception on update" do
-      expect { LogStash::Setting::PortRange.new("mynewtest", 10000).set(0.1) }.to raise_error
+      expect { LogStash::Setting::PortRangeSetting.new("mynewtest", 10000).set(0.1) }.to raise_error
     end
   end
 
   context "When value is a range" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", 9000..10000) }
+    subject { LogStash::Setting::PortRangeSetting.new("mynewtest", 9000..10000) }
 
     it "accepts a ruby range as the default value" do
       expect { subject }.not_to raise_error
     end
 
     it "can update the range" do
       subject.set(500..1000)
-      expect(subject.value).to eq(500..1000)
+      expect(subject.value).to eq(::LogStash.as_java_range(500..1000))
     end
 
     it "refuses when then upper port is out of range" do
-      expect { LogStash::Setting::PortRange.new("mynewtest", 9000..1000000) }.to raise_error
+      expect { LogStash::Setting::PortRangeSetting.new("mynewtest", 9000..1000000) }.to raise_error
     end
 
     it "raise an exception on when port are out of range" do
-      expect { LogStash::Setting::PortRange.new("mynewtest", -1000..1000) }.to raise_error
+      expect { LogStash::Setting::PortRangeSetting.new("mynewtest", -1000..1000) }.to raise_error
     end
   end
 end

logstash-core/spec/logstash/settings_spec.rb

@@ -64,7 +64,7 @@
 
   describe "#to_hash" do
     let(:java_deprecated_alias) { LogStash::Setting::Boolean.new("java.actual", true).with_deprecated_alias("java.deprecated") }
-    let(:ruby_deprecated_alias) { LogStash::Setting::PortRange.new("ruby.actual", 9600..9700).with_deprecated_alias("ruby.deprecated") }
+    let(:ruby_deprecated_alias) { LogStash::Setting::PortRangeSetting.new("ruby.actual", 9600..9700).with_deprecated_alias("ruby.deprecated") }
     let(:non_deprecated) { LogStash::Setting::Boolean.new("plain_setting", false) }
 
     before :each do

logstash-core/src/main/java/org/logstash/settings/PortRangeSetting.java

@@ -0,0 +1,100 @@
+/*
+ * 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.jruby.RubyInteger;
+import org.jruby.RubyRange;
+import org.logstash.RubyUtil;
+
+// Ideally would be a Coercible<Range<Integer>>, but given the fact that
+// values can be effectively coerced into the constructor, it needs instances
+// of Objects to represent Integer, String, Long to be later coerced into Range<Integer>.
+@SuppressWarnings({"rawtypes", "unchecked"})
+public class PortRangeSetting extends Coercible<Object> {
+
+    private static final Range<Integer> VALID_PORT_RANGE = new Range<>(1, 65535);
+    public static final String PORT_SEPARATOR = "-";
+
+    public PortRangeSetting(String name, Object defaultValue) {
+        super(name, defaultValue, true, PortRangeSetting::isValid);
+    }
+
+    public static boolean isValid(Object range) {
+        if (!(range instanceof Range)) {
+            return false;
+        }
+
+        return VALID_PORT_RANGE.contains((Range<Integer>) range);
+    }
+
+    @Override
+    public Range<Integer> coerce(Object obj) {
+        if (obj instanceof Range) {
+            return (Range) obj;
+        }
+
+        if (obj instanceof Integer) {
+            Integer val = (Integer) obj;
+            return new Range<>(val, val);
+        }
+
+        if (obj instanceof Long) {
+            Long val = (Long) obj;
+            return new Range<>(val.intValue(), val.intValue());
+        }
+
+        if (obj instanceof String) {
+            String val = ((String) obj).trim();
+            String[] parts = val.split(PORT_SEPARATOR);
+            String firstStr = parts[0];
+            String lastStr;
+            if (parts.length == 1) {
+                lastStr = firstStr;
+            } else {
+                lastStr = parts[1];
+            }
+            try {
+                int first = Integer.parseInt(firstStr);
+                int last = Integer.parseInt(lastStr);
+                return new Range<>(first, last);
+            } catch(NumberFormatException e) {
+                throw new IllegalArgumentException("Could not coerce [" + obj + "](type: " + obj.getClass() + ") into a port range");
+            }
+        }
+
+        if (obj instanceof RubyRange) {
+            RubyRange rubyRange = (RubyRange) obj;
+            RubyInteger begin = rubyRange.begin(RubyUtil.RUBY.getCurrentContext()).convertToInteger();
+            RubyInteger end = rubyRange.end(RubyUtil.RUBY.getCurrentContext()).convertToInteger();
+            return new Range<>(begin.getIntValue(), end.getIntValue());
+        }
+        throw new IllegalArgumentException("Could not coerce [" + obj + "](type: " + obj.getClass() + ") into a port range");
+    }
+
+    @Override
+    public void validate(Object value) throws IllegalArgumentException {
+        if (!isValid(value)) {
+            final String msg = String.format("Invalid value \"%s: %s\", valid options are within the range of %d-%d",
+                    getName(), value, VALID_PORT_RANGE.getFirst(), VALID_PORT_RANGE.getLast());
+
+            throw new IllegalArgumentException(msg);
+        }
+    }
+}

logstash-core/src/main/java/org/logstash/settings/PortSetting.java

@@ -0,0 +1,49 @@
+/*
+ * 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 java.util.function.Predicate;
+
+public final class PortSetting extends SettingInteger {
+
+    public static final Predicate<Integer> VALID_PORT_RANGE = new Predicate<>() {
+        @Override
+        public boolean test(Integer integer) {
+            return isValid(integer);
+        }
+    };
+
+    public PortSetting(String name, Integer defaultValue) {
+        super(name, defaultValue);
+    }
+
+    public PortSetting(String name, Integer defaultValue, boolean strict) {
+        this(name, defaultValue, strict, VALID_PORT_RANGE);
+    }
+
+    protected PortSetting(String name, Integer defaultValue, boolean strict, Predicate<Integer> validator) {
+        super(name, defaultValue, strict, validator);
+    }
+
+    public static boolean isValid(int port) {
+        return 1 <= port && port <= 65535;
+    }
+
+}