From 0f46be982d0aef3be16ab77ce322090d1f5087be Mon Sep 17 00:00:00 2001
From: Brice Figureau <brice@daysofwonder.com>
Date: Thu, 14 May 2020 15:39:09 +0200
Subject: [PATCH] (#223) display diff for nested arrays of objects

When using octocatalog-diff with puppet resources with deep
nested datastructures such as nested arrays of objects, octocatalog-
diff would not display diffs when array elements are modified,added or removed.

In fact, it turns out `dig_out_key` doesn't descend into arrays index
that hashdiff can produce, like for instance:
testtype::test::parameters::testparam::another-one::an-array[0]::env

`dig_out_key` would stop at `an-array` because it doesn't know that's
and array index it should try to descend into.

This patch adds the functionality for `dig_out_key` to follow array
index and descend into those nested structure.
---
 lib/octocatalog-diff/catalog-diff/differ.rb   | 11 ++-
 .../tests/catalog-diff/differ_spec.rb         | 77 +++++++++++++++++++
 2 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/lib/octocatalog-diff/catalog-diff/differ.rb b/lib/octocatalog-diff/catalog-diff/differ.rb
index 07e19e89..3ce00ac5 100644
--- a/lib/octocatalog-diff/catalog-diff/differ.rb
+++ b/lib/octocatalog-diff/catalog-diff/differ.rb
@@ -622,9 +622,14 @@ def hashdiff_nested_changes(hashdiff_add_remove, remaining1, remaining2)
       def dig_out_key(hash_in, key_array)
         return hash_in if key_array.empty?
         return hash_in unless hash_in.is_a?(Hash)
-        return nil unless hash_in.key?(key_array[0])
-        next_key = key_array.shift
-        dig_out_key(hash_in[next_key], key_array)
+        key_without_index = key_array[0].sub(/\[\d+\]/, '')
+        return nil unless hash_in.key?(key_without_index)
+        full_key = key_array.shift
+        next_obj = hash_in[key_without_index]
+        # jump into array index if needed
+        md = full_key.match(/\[(\d+)\]/)
+        next_obj = next_obj[md[1].to_i] if md
+        dig_out_key(next_obj, key_array)
       end
 
       # This is a helper for the constructor, verifying that the incoming catalog is an expected
diff --git a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb
index 32bde785..2b29edb7 100644
--- a/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb
+++ b/spec/octocatalog-diff/tests/catalog-diff/differ_spec.rb
@@ -1414,6 +1414,83 @@
       fileref = { 'file' => '/var/tmp/foo', 'line' => 5 }
       expect(result[0]).to eq(['!', "Class\fOpenssl::Package\fparameters\fcommon-array", [1, 2, 3], [1, 5, 25], fileref, fileref])
     end
+
+    it 'should return array with proper results for deeply nested arrays of hashes' do
+      hashdiff_add_remove = [
+        "Class\fOpenssl::Package\fparameters\fobject\farray[0]\fcommon-array"
+      ]
+
+      empty_puppet_catalog_json = File.read(OctocatalogDiff::Spec.fixture_path('catalogs/catalog-empty.json'))
+      empty_puppet_catalog = OctocatalogDiff::Catalog.create(json: empty_puppet_catalog_json)
+      obj = OctocatalogDiff::CatalogDiff::Differ.new({}, empty_puppet_catalog, empty_puppet_catalog)
+
+      arr1 = [
+        {
+          'name' => 'test', 'value' => 'abc'
+        },
+        {
+          'name' => 'test2', 'value' => 'def'
+        }
+      ]
+      cat1 = [
+        {
+          'type' => 'Class',
+          'title' => 'Openssl::Package',
+          'parameters' => {
+            'object' => {
+              'array' => [
+                {
+                  'common-array' => arr1
+                }
+              ]
+            }
+          },
+          'file' => '/var/tmp/foo',
+          'line' => 5
+        }
+      ]
+
+      arr2 = [
+        {
+          'name' => 'test', 'value' => 'abc'
+        },
+        {
+          'name' => 'test2', 'value' => 'def'
+        },
+        {
+          'name' => 'test3', 'value' => 'ghj'
+        }
+      ]
+      cat2 = [
+        {
+          'type' => 'Class',
+          'title' => 'Openssl::Package',
+          'parameters' => {
+            'object' => {
+              'array' => [
+                {
+                  'common-array' => arr2
+                }
+              ]
+            }
+          },
+          'file' => '/var/tmp/foo',
+          'line' => 5
+        }
+      ]
+
+      remaining1 = obj.send(:resources_as_hashes_with_serialized_keys, cat1)
+      remaining2 = obj.send(:resources_as_hashes_with_serialized_keys, cat2)
+
+      result = obj.send(:hashdiff_nested_changes, hashdiff_add_remove, remaining1, remaining2)
+      expect(result).to be_a_kind_of(Array)
+      expect(result.size).to eq(1)
+
+      fileref = { 'file' => '/var/tmp/foo', 'line' => 5 }
+      expect(result[0]).to eq(['!',
+        "Class\fOpenssl::Package\fparameters\fobject\farray[0]\fcommon-array",
+        arr1, arr2, fileref, fileref])
+    end
   end
 
   describe '#regexp_operator_match?' do