add Java Collection delete, & and | support. refactored Java interfaces equivalence. specs & more java_integration specs

add Java Collection delete support with specs & more java_integration specs

split a spec

refactored for better specs

intersection specs

Ruby intersection on Java collections, refactored to use Java interfaces

specs for remove_tag from events from json input

refactor usage of subject

added Java Collection union with Ruby array and specs

refactored specs to also test for hash from deserialized json for JrJackson & Java Collections

typo and comments

solves #2261

lib/logstash/java_integration.rb

@@ -6,27 +6,11 @@ require "java"
 # not test for is_a?(Array) or is_a?(Hash) and we do not want to include tests for
 # both classes everywhere. see LogStash::JSon.
 
-class Java::JavaUtil::ArrayList
-  # have ArrayList objects report is_a?(Array) == true
-  def is_a?(clazz)
-    return true if clazz == Array
-    super
-  end
-end
-
-class Java::JavaUtil::LinkedHashMap
-  # have LinkedHashMap objects report is_a?(Array) == true
-  def is_a?(clazz)
-    return true if clazz == Hash
-    super
-  end
-end
-
 class Array
   # enable class equivalence between Array and ArrayList
   # so that ArrayList will work with case o when Array ...
   def self.===(other)
-    return true if other.is_a?(Java::JavaUtil::ArrayList)
+    return true if other.is_a?(Java::JavaUtil::Collection)
     super
   end
 end
@@ -35,7 +19,44 @@ class Hash
   # enable class equivalence between Hash and LinkedHashMap
   # so that LinkedHashMap will work with case o when Hash ...
   def self.===(other)
-    return true if other.is_a?(Java::JavaUtil::LinkedHashMap)
+    return true if other.is_a?(Java::JavaUtil::Map)
     super
   end
 end
+
+module java::util::Map
+  # have Map objects like LinkedHashMap objects report is_a?(Array) == true
+  def is_a?(clazz)
+    return true if clazz == Hash
+    super
+  end
+end
+
+module java::util::Collection
+  # have Collections objects like ArrayList report is_a?(Array) == true
+  def is_a?(clazz)
+    return true if clazz == Array
+    super
+  end
+
+  # support the Ruby Array delete method on a Java Collection
+  def delete(o)
+    self.removeAll([o]) ? o : block_given? ? yield : nil
+  end
+
+  # support the Ruby intersection method on Java Collection
+  def &(other)
+    # transform self into a LinkedHashSet to remove duplicates and preserve order as defined by the Ruby Array intersection contract
+    duped = Java::JavaUtil::LinkedHashSet.new(self)
+    duped.retainAll(other)
+    duped
+  end
+
+  # support the Ruby union method on Java Collection
+  def |(other)
+    # transform self into a LinkedHashSet to remove duplicates and preserve order as defined by the Ruby Array union contract
+    duped = Java::JavaUtil::LinkedHashSet.new(self)
+    duped.addAll(other)
+    duped
+  end
+end

spec/core/event_spec.rb

@@ -2,31 +2,8 @@
 require "spec_helper"
 
 describe LogStash::Event do
-  subject do
-    LogStash::Event.new(
-      "@timestamp" => Time.iso8601("2013-01-01T00:00:00.000Z"),
-      "type" => "sprintf",
-      "message" => "hello world",
-      "tags" => [ "tag1" ],
-      "source" => "/home/foo",
-      "a" => "b",
-      "c" => {
-        "d" => "f",
-        "e" => {"f" => "g"}
-      },
-      "f" => { "g" => { "h" => "i" } },
-      "j" => {
-          "k1" => "v",
-          "k2" => [ "w", "x" ],
-          "k3" => {"4" => "m"},
-          5 => 6,
-          "5" => 7
-      },
-      "nilfield" => nil,
-      "@metadata" => { "fancy" => "pants", "have-to-go" => { "deeper" => "inception" } }
-    )
-  end
 
+  shared_examples "all event tests" do
     context "[]=" do
       it "should raise an exception if you attempt to set @timestamp to a value type other than a Time object" do
         expect{subject["@timestamp"] = "crash!"}.to raise_error(TypeError)
@@ -188,7 +165,10 @@ describe LogStash::Event do
 
       it "should concatenate tags" do
         subject.append(LogStash::Event.new("tags" => [ "tag2" ]))
-      expect(subject["tags"]).to eq([ "tag1", "tag2" ])
+        # added to_a for when array is a Java Collection when produced from json input
+        # TODO: we have to find a better way to handle this in tests. maybe override
+        # rspec eq or == to do an explicit to_a when comparing arrays?
+        expect(subject["tags"].to_a).to eq([ "tag1", "tag2" ])
       end
 
       context "when event field is nil" do
@@ -433,5 +413,44 @@ describe LogStash::Event do
         expect(LogStash::FLUSH).to be_a(LogStash::FlushEvent)
       end
     end
+  end
 
+  let(:event_hash) do
+    {
+      "@timestamp" => "2013-01-01T00:00:00.000Z",
+      "type" => "sprintf",
+      "message" => "hello world",
+      "tags" => [ "tag1" ],
+      "source" => "/home/foo",
+      "a" => "b",
+      "c" => {
+        "d" => "f",
+        "e" => {"f" => "g"}
+      },
+      "f" => { "g" => { "h" => "i" } },
+      "j" => {
+          "k1" => "v",
+          "k2" => [ "w", "x" ],
+          "k3" => {"4" => "m"},
+          5 => 6,
+          "5" => 7
+      },
+      "nilfield" => nil,
+      "@metadata" => { "fancy" => "pants", "have-to-go" => { "deeper" => "inception" } }
+    }
+  end
+
+  describe "using normal hash input" do
+    it_behaves_like "all event tests" do
+      subject{LogStash::Event.new(event_hash)}
+    end
+  end
+
+  describe "using hash input from deserialized json" do
+    # this is to test the case when JrJackson deserialises Json and produces
+    # native Java Collections objects for efficiency
+    it_behaves_like "all event tests" do
+      subject{LogStash::Event.new(LogStash::Json.load(LogStash::Json.dump(event_hash)))}
+    end
+  end
 end

spec/filters/base_spec.rb

@@ -1,5 +1,6 @@
 # encoding: utf-8
 require "spec_helper"
+require "logstash/json"
 
 # use a dummy NOOP filter to test Filters::Base
 class LogStash::Filters::NOOP < LogStash::Filters::Base
@@ -196,9 +197,21 @@ describe LogStash::Filters::NOOP do
       insist { subject["tags"] } == ["t1"]
     end
 
+    # also test from Json deserialized data to test the handling of native Java collections by JrJackson
+    # see https://github.com/elastic/logstash/issues/2261
+    sample(LogStash::Json.load("{\"type\":\"noop\", \"tags\":[\"t1\", \"t2\", \"t3\"]}")) do
+      insist { subject["tags"] } == ["t1"]
+    end
+
     sample("type" => "noop", "tags" => ["t1", "t2"]) do
       insist { subject["tags"] } == ["t1"]
     end
+
+    # also test from Json deserialized data to test the handling of native Java collections by JrJackson
+    # see https://github.com/elastic/logstash/issues/2261
+    sample(LogStash::Json.load("{\"type\":\"noop\", \"tags\":[\"t1\", \"t2\"]}")) do
+      insist { subject["tags"] } == ["t1"]
+    end
   end
 
   describe "remove_tag with dynamic value" do
@@ -215,6 +228,12 @@ describe LogStash::Filters::NOOP do
     sample("type" => "noop", "tags" => ["t1", "goaway", "t3"], "blackhole" => "goaway") do
       insist { subject["tags"] } == ["t1", "t3"]
     end
+
+    # also test from Json deserialized data to test the handling of native Java collections by JrJackson
+    # see https://github.com/elastic/logstash/issues/2261
+    sample(LogStash::Json.load("{\"type\":\"noop\", \"tags\":[\"t1\", \"goaway\", \"t3\"], \"blackhole\":\"goaway\"}")) do
+      insist { subject["tags"] } == ["t1", "t3"]
+    end
   end
 
   describe "remove_field" do

spec/lib/logstash/java_integration_spec.rb

@@ -0,0 +1,169 @@
+# encoding: utf-8
+require "spec_helper"
+require "logstash/java_integration"
+
+describe "Java integration" do
+
+  context "type equivalence" do
+
+    # here we test for both is_a? and case/when usage of the Java types
+    # because these are the specific use-cases in our code and the expected
+    # behaviour.
+
+    context "Java::JavaUtil::ArrayList" do
+
+      it "should report to be a Ruby Array" do
+        expect(Java::JavaUtil::ArrayList.new.is_a?(Array)).to be_true
+      end
+
+      it "should be class equivalent to Ruby Array" do
+        expect do
+          case Java::JavaUtil::ArrayList.new
+          when Array
+            true
+          else
+            raise
+          end
+        end.not_to raise_error
+
+        expect(Array === Java::JavaUtil::ArrayList.new).to be_true
+      end
+    end
+
+    context "Java::JavaUtil::LinkedHashMap" do
+      it "should report to be a Ruby Hash" do
+        expect(Java::JavaUtil::LinkedHashMap.new.is_a?(Hash)).to be_true
+      end
+
+      it "should be class equivalent to Ruby Hash" do
+        expect do
+          case Java::JavaUtil::LinkedHashMap.new
+          when Hash
+            true
+          else
+            raise
+          end
+        end.not_to raise_error
+
+        expect(Hash === Java::JavaUtil::LinkedHashMap.new).to be_true
+      end
+    end
+  end
+
+  context "Java::JavaUtil::Collection" do
+    subject{Java::JavaUtil::ArrayList.new(initial_array)}
+
+    context "when deleting a unique instance" do
+      let(:initial_array) {["foo", "bar"]}
+
+      it "should return the deleted object" do
+        expect(subject.delete("foo")).to eq("foo")
+      end
+
+      it "should remove the object to delete" do
+        expect{subject.delete("foo")}.to change{subject.to_a}.from(initial_array).to(["bar"])
+      end
+    end
+
+    context "when deleting multiple instances" do
+      let(:initial_array) {["foo", "bar", "foo"]}
+
+      it "should return the last deleted object" do
+        expect(subject.delete("foo")).to eq("foo")
+      end
+
+      it "should remove all the objects to delete" do
+        expect{subject.delete("foo")}.to change{subject.to_a}.from(initial_array).to(["bar"])
+      end
+    end
+
+    context "when deleting non existing object" do
+      let(:initial_array) {["foo", "bar", "foo"]}
+
+      it "should return nil" do
+        expect(subject.delete("baz")).to be_nil
+      end
+
+      it "should not change the collection" do
+        expect{subject.delete("baz")}.to_not change{subject.to_a}
+      end
+
+      it "should yield to block when given" do
+        expect(subject.delete("baz"){"foobar"}).to eq("foobar")
+      end
+    end
+
+    context "when deleting on empty collection" do
+      let(:initial_array) {[]}
+
+      it "should return nil" do
+        expect(subject.delete("baz")).to be_nil
+      end
+
+      it "should not change the collection" do
+        expect{subject.delete("baz")}.to_not change{subject.to_a}
+      end
+    end
+
+    context "when intersecting with a Ruby Array" do
+
+      context "using string collection with duplicates and single result" do
+        let(:initial_array) {["foo", "bar", "foo"]}
+
+        it "should not change original collection" do
+          expect{subject & ["foo"]}.to_not change{subject.to_a}
+        end
+
+        it "should return a new array containing elements common to the two arrays, excluding any duplicate" do
+          expect((subject & ["foo"]).to_a).to eq(["foo"])
+        end
+      end
+
+      context "using string collection with duplicates and multiple results" do
+        let(:original) {["foo", "bar", "foo", "baz"]}
+        let(:target) {["baz", "foo"]}
+        let(:result) {["foo", "baz"]}
+
+        it "should return a new array containing elements common to the two arrays, excluding any duplicate and preserve order from the original array" do
+          # this is the Ruby contract
+          expect(original & target).to eq(result)
+
+          # this should work the same
+          expect((Java::JavaUtil::ArrayList.new(original) & target).to_a).to eq(result)
+        end
+      end
+
+      context "Ruby doc examples" do
+        it "should return a new array containing elements common to the two arrays, excluding any duplicate" do
+          expect(Java::JavaUtil::ArrayList.new(([1, 1, 3, 5]) & [1, 2, 3]).to_a).to eq([1, 3])
+          expect(Java::JavaUtil::ArrayList.new((['a', 'b', 'b', 'z']) & ['a', 'b', 'c']).to_a).to eq(['a', 'b'])
+        end
+      end
+    end
+
+    context "when unioning with a Ruby Array" do
+
+      context "using string collection with duplicates" do
+        let(:initial_array) {["foo", "bar", "foo"]}
+
+        it "should not change original collection" do
+          expect{subject | ["bar", "baz"]}.to_not change{subject.to_a}
+        end
+
+        it "should return a new array by joining excluding any duplicates and preserving the order from the original array" do
+          expect((subject | ["bar", "baz"]).to_a).to eq(["foo", "bar", "baz"])
+        end
+
+        it "should remove duplicates when joining empty array" do
+          expect((subject | []).to_a).to eq(["foo", "bar"])
+        end
+      end
+
+      context "Ruby doc examples" do
+        it "should return a new array containing elements common to the two arrays, excluding any duplicate" do
+          expect(Java::JavaUtil::ArrayList.new((["a", "b", "c"]) | ["c", "d", "a"]).to_a).to eq(["a", "b", "c", "d"])
+        end
+      end
+    end
+  end
+end