Fix bugs in deep_replace_href and deep_select

The fixtures `refs_to_arrays.xml` and `nested_refs_to_arrays.xml` both could not
be parsed correctly even though they are valid XML. Adding these tests to
the router allowed me to trace down the errors to `deep_select`.

`deep_select` didn't support finding elements inside arrays. Unit tests were
added to the `dispatcher_spec` to show these cases and they were fixed.

In order to make it easier to test that `deep_select` was working properly, I
changed its signature so that it doesn't take a block and it always just selects
ids which was they only way it was used in anycase.

Searching through arrays was also not supported by `deep_replace_href`. Unit
tests were adding showing these bugs. And now this function can also replace arrays in
nested hashes.

Author: rmeritz

lib/wash_out/dispatcher.rb

@@ -173,28 +173,42 @@ def self.included(controller)
       controller.send :"skip_before_#{entity}", :verify_authenticity_token
     end
 
-    def self.deep_select(hash, result=[], &blk)
-      result += Hash[hash.select(&blk)].values
-
-      hash.each do |key, value|
-        result = deep_select(value, result, &blk) if value.is_a? Hash
+    def self.deep_select(collection, result = [])
+      is_id = lambda { |v| v.is_a?(Hash) && v.has_key?(:@id) }
+      values = collection.respond_to?(:values) ? collection.values : collection
+      result += values.select(&is_id)
+
+      values.each do |value|
+        if value.is_a?(Hash) || value.is_a?(Array)
+          result = deep_select(value, result)
+        end
       end
 
       result
     end
 
     def self.deep_replace_href(hash, replace)
-      return replace[hash[:@href]] if hash.has_key?(:@href)
+      hash.each do |key, value|
+        if value.respond_to?(:has_key?) && value.has_key?(:@href)
+          hash[key] = replace[value[:@href]]
+          hash = deep_replace_href(hash, replace) if hash.is_a?(Hash)
+        elsif key == :@href
+          hash = replace[value]
+        end
 
-      hash.keys.each do |key, value|
-        hash[key] = deep_replace_href(hash[key], replace) if hash[key].is_a?(Hash)
+        if hash[key].is_a?(Hash)
+          hash[key] = deep_replace_href(hash[key], replace)
+        elsif hash[key].is_a?(Array)
+          hash[key] = hash[key].map do |v|
+            v.is_a?(Hash) ? deep_replace_href(v, replace) : v
+          end
+        end
       end
 
       hash
     end
 
     private
-
     def action_spec
       self.class.soap_actions[soap_action]
     end

lib/wash_out/router.rb

@@ -61,9 +61,8 @@ def soap_body(env)
 
     def parse_soap_parameters(env)
       return env['wash_out.soap_data'] if env['wash_out.soap_data']
-
       env['wash_out.soap_data'] = nori(controller.soap_config.snakecase_input).parse(soap_body env)
-      references = WashOut::Dispatcher.deep_select(env['wash_out.soap_data']){|k,v| v.is_a?(Hash) && v.has_key?(:@id)}
+      references = WashOut::Dispatcher.deep_select(env['wash_out.soap_data'])
 
       unless references.blank?
         replaces = {}; references.each{|r| replaces['#'+r[:@id]] = r}

spec/fixtures/nested_refs_to_arrays.xml

@@ -0,0 +1,19 @@
+<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
+  <s:Body s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    <parent href="#id1" />
+    <q2:child id="id1" xsi:type="q2:child" xmlns:q2="urn:FakeService">
+      <first_list href="#id2" />
+      <second_list href="#id3" />
+    </q2:child>
+    <q3:Array id="id2" q3:arrayType="xsd:int[1]" xmlns:q3="http://schemas.xmlsoap.org/soap/encoding/">
+      <Item>1</Item>
+      <Item>2</Item>
+    </q3:Array>
+    <q4:Array id="id3" q4:arrayType="xsd:int[1]" xmlns:q4="http://schemas.xmlsoap.org/soap/encoding/">
+      <Item>11</Item>
+      <Item>22</Item>
+    </q4:Array>
+  </s:Body>
+</s:Envelope>

spec/fixtures/ref_to_one_array.xml

@@ -0,0 +1,11 @@
+<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
+  <s:Body s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    <list href="#id1" />
+    <q1:Array id="id1" q1:arrayType="xsd:int[1]" xmlns:q1="http://schemas.xmlsoap.org/soap/encoding/">
+      <Item>1</Item>
+      <Item>2</Item>
+    </q1:Array>
+  </s:Body>
+</s:Envelope>

spec/fixtures/refs_to_arrays.xml

@@ -0,0 +1,16 @@
+<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
+  <s:Body s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+    <first_list href="#id1" />
+    <second_list href="#id2" />
+    <q1:Array id="id1" q1:arrayType="xsd:int[1]" xmlns:q1="http://schemas.xmlsoap.org/soap/encoding/">
+      <Item>1</Item>
+      <Item>2</Item>
+    </q1:Array>
+    <q2:Array id="id2" q2:arrayType="xsd:int[1]" xmlns:q2="http://schemas.xmlsoap.org/soap/encoding/">
+      <Item>11</Item>
+      <Item>22</Item>
+    </q2:Array>
+  </s:Body>
+</s:Envelope>

spec/lib/wash_out/dispatcher_spec.rb

@@ -12,14 +12,119 @@ def params
     end
   end
 
-  it "finds nested hashes" do
-    expect(WashOut::Dispatcher.deep_select(:foo => 1){|k,v| k == :foo}).to eq [1]
-    expect(WashOut::Dispatcher.deep_select({:foo => {:foo => 1}}){|k,v| k == :foo}).to eq([{:foo => 1}, 1])
+  describe ".deep_select" do
+    it "find no elements if there aren't any ids" do
+      expect(WashOut::Dispatcher.deep_select({k: {v: :v2}})).to eq []
+    end
+
+    it "finds elements with ids in a hash" do
+      expect(WashOut::Dispatcher.deep_select({k: {:@id => 5, x: :y}})).to eq [{:@id => 5, x: :y}]
+    end
+
+    it "finds elements with ids in a array" do
+      expect(WashOut::Dispatcher.deep_select({k: [{:@id => 5, x: :y}]})).to eq [{:@id => 5, x: :y}]
+    end
+
+    it "finds elements with ids in hashes" do
+      expect(WashOut::Dispatcher.deep_select(
+        {
+          k: {:@id => 5, x: :y},
+          k2: {:@id => 6, n: :m}
+        })).to eq [{:@id => 5, x: :y}, {:@id => 6, n: :m}]
+    end
+
+    it "finds elements in a hash and in a array" do
+      expect(WashOut::Dispatcher.deep_select(
+        {
+          k: [{:@id => 5, x: :y}],
+          k2: {:@id => 6, n: :m}
+        })).to contain_exactly({:@id => 5, x: :y}, {:@id => 6, n: :m})
+    end
+
+    it "finds elements with ids in multiple arrays" do
+      expect(WashOut::Dispatcher.deep_select(
+        {
+          k: [{:@id => 5, x: :y}],
+          k2: [{:@id => 6, n: :m}]
+        })).to eq [{:@id => 5, x: :y}, {:@id => 6, n: :m}]
+    end
   end
 
-  it "replaces nested hashed" do
-    expect(WashOut::Dispatcher.deep_replace_href({:foo => {:@href => 1}}, {1 => 2})).to eq({:foo => 2})
-    expect(WashOut::Dispatcher.deep_replace_href({:bar => {:foo => {:@href => 1}}}, {1 => 2})).to eq({:bar => {:foo => 2}})
+  describe ".deep_replace_href" do
+    it "replaces nested hashed" do
+      expect(WashOut::Dispatcher.deep_replace_href(
+        {:foo => {:@href => 1}},
+        {1 => 2})).to eq(
+          {:foo => 2}
+        )
+    end
+
+    it "replaces deeper nested hashes" do
+      expect(WashOut::Dispatcher.deep_replace_href(
+        {:bar => {:foo => {:@href => 1}}},
+        {1 => 2}
+      )).to eq(
+        {:bar => {:foo => 2}}
+      )
+    end
+
+    it "replace nested refs" do
+      hash = {fizz: {:@href => "#id4"}}
+      replaces = {
+        "#id4" => {:@href => "#id6"},
+        "#id6" => {foo: :bar}
+      }
+      expect(WashOut::Dispatcher.deep_replace_href(hash, replaces)).to eq({
+        fizz: {foo: :bar}})
+    end
+
+    it "replace really nested refs" do
+      hash = {fizz: {:@href => "#id4"}}
+      replaces = {
+        "#id4" => {:@href => "#id6"},
+        "#id6" => {:@href => "#id7"},
+        "#id7" => {foo: :bar}
+      }
+      expect(WashOut::Dispatcher.deep_replace_href(hash, replaces)).to eq({
+        fizz: {foo: :bar}})
+    end
+
+    it "replaces arrays in nested hashes" do
+      hash = {
+        fizz: {:@href => "#id4"},
+        Array: [
+          {Item: [{:@href => "#id6"}, {:@href => "#id7"}]},
+          {Item: {loo: :iioo}}
+        ]
+      }
+      replaces = {
+        "#id4" => {Item: [{:@href => "#id6"}, {:@href => "#id7"}]},
+        "#id6" => {foo: :bar},
+        "#id7" => {baz: :bats}
+      }
+      expect(WashOut::Dispatcher.deep_replace_href(hash, replaces)).to eq({
+        fizz: {Item: [
+          {foo: :bar},
+          {baz: :bats}
+        ]},
+        Array: [
+          {Item: [{foo: :bar}, {baz: :bats}]},
+          {Item: {loo: :iioo}}
+        ]
+      })
+    end
+
+    it "can traverse arrays that do not contain hashes" do
+      hash = {
+        fizz: {:@href => "#id1"},
+      }
+      replaces = {
+        "#id1" => {Item: ["1", "2"]},
+      }
+      expect(WashOut::Dispatcher.deep_replace_href(hash, replaces)).to eq({
+        fizz: {Item: ["1", "2"]},
+      })
+    end
   end
 
   xit "parses typical request" do

spec/lib/wash_out/router_spec.rb

@@ -19,4 +19,35 @@
     msg = result[2][0]
     expect(msg).to eq('OK')
   end
+
+  def parse_soap_params_from_xml(filename)
+    xml = File.read(File.expand_path("../../../fixtures/#{filename}", __FILE__))
+    env = {'rack.input' => StringIO.new(xml)}
+
+    router = WashOut::Router.new('')
+    controller = double("controller", soap_config: WashOut::SoapConfig.new)
+    allow(router).to receive(:controller).and_return(controller)
+
+    router.parse_soap_parameters(env)[:Envelope][:Body]
+  end
+
+  it "returns refs to arrays correctly" do
+    body = parse_soap_params_from_xml('ref_to_one_array.xml')
+
+    expect(body[:list][:Item]).to eq(["1", "2"])
+  end
+
+  it "returns refs to multiple arrays correctly" do
+    body = parse_soap_params_from_xml('refs_to_arrays.xml')
+
+    expect(body[:first_list][:Item]).to eq(["1", "2"])
+    expect(body[:second_list][:Item]).to eq(["11", "22"])
+  end
+
+  it "returns nested refs to multiple arrays correctly" do
+    body = parse_soap_params_from_xml('nested_refs_to_arrays.xml')
+
+    expect(body[:parent][:first_list][:Item]).to eq(["1", "2"])
+    expect(body[:parent][:second_list][:Item]).to eq(["11", "22"])
+  end
 end