Updated SettingsPortRange to work on generic Object so that the constructor can coerce on values offered for default value. Fixed failing test trasforming Ruby rage to newly introduced Java Range class

Author: andsel

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

@@ -27,12 +27,12 @@
     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
 
@@ -48,12 +48,12 @@
     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
 
@@ -82,23 +82,23 @@
   end
 
   context "When value is a range" do
-    subject { LogStash::Setting::PortRange.new("mynewtest", 9000..10000) }
+    subject { LogStash::Setting::PortRange.new("mynewtest", ::LogStash.as_java_range(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)
+      subject.set(::LogStash.as_java_range(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::PortRange.new("mynewtest", ::LogStash.as_java_range(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::PortRange.new("mynewtest", ::LogStash.as_java_range(-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::PortRange.new("ruby.actual", ::LogStash.as_java_range(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/Range.java

@@ -1,5 +1,6 @@
 package org.logstash.settings;
 
+import java.util.Objects;
 import java.util.function.BiConsumer;
 
 public class Range<T extends Integer> {
@@ -40,4 +41,25 @@ public void eachWithIndex(BiConsumer<Integer, Integer> consumer) {
     public int count() {
         return last.intValue() - first.intValue() + 1;
     }
+
+    @Override
+    public String toString() {
+        return this.getClass().getName() +
+                "{first=" + first +
+                ", last=" + last +
+                '}';
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Range<?> range = (Range<?>) o;
+        return Objects.equals(first, range.first) && Objects.equals(last, range.last);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(first, last);
+    }
 }

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

@@ -1,19 +1,24 @@
 package org.logstash.settings;
 
-import java.util.function.Predicate;
-
+// 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 SettingPortRange extends Coercible<Range<Integer>> {
+public class SettingPortRange extends Coercible<Object> {
 
     private static final Range<Integer> VALID_PORT_RANGE = new Range<>(1, 65535);
     public static final String PORT_SEPARATOR = "-";
 
-    public SettingPortRange(String name, Range<Integer> defaultValue) {
+    public SettingPortRange(String name, Object defaultValue) {
         super(name, defaultValue, true, SettingPortRange::isValid);
     }
 
-    public static boolean isValid(Range<Integer> range) {
-        return VALID_PORT_RANGE.contains(range);
+    public static boolean isValid(Object range) {
+        if (!(range instanceof Range)) {
+            return false;
+        }
+
+        return VALID_PORT_RANGE.contains((Range<Integer>) range);
     }
 
     // TODO cover with tests
@@ -55,10 +60,10 @@ public Range<Integer> coerce(Object obj) {
     }
 
     @Override
-    public void validate(Range<Integer> value) throws IllegalArgumentException {
+    public void validate(Object value) throws IllegalArgumentException {
         if (!isValid(value)) {
-            final String msg = String.format("Invalid value \"{}: {}}\", valid options are within the range of {}-{}",
-                    getName(), value, VALID_PORT_RANGE, VALID_PORT_RANGE.getFirst(), VALID_PORT_RANGE.getLast());
+            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);
         }