Fix #187,#188 CompilationDir not supporting complex or multiple values

There were two cases where CompilationDir wasn't filtering out changes:
* if the parameter value is an arbitrary data structure (ie hash, array
or mix of both)
* if the parameter value is a string containing more than one occurence
of the compilation dir

It turns out that both can be fixed by just replacing the compilation
dirs in both the new and old values with empty strings and comparing
what's left.

It's probably much slower than the original string-only implementation
but covers much more cases as demonstrated by #187 and #188.

Author: Brice Figureau

lib/octocatalog-diff/catalog-diff/filter/compilation_dir.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative '../filter'
+require_relative '../../util/util'
 
 module OctocatalogDiff
   module CatalogDiff
@@ -35,43 +36,46 @@ def filtered?(diff, options = {})
 
           # Check for a change where the difference in a parameter exactly corresponds to the difference in the
           # compilation directory.
-          if diff.change? && (diff.old_value.is_a?(String) || diff.new_value.is_a?(String))
-            from_before = nil
-            from_after = nil
-            from_match = false
-            to_before = nil
-            to_after = nil
-            to_match = false
+          if diff.change?
+            o = remove_compilation_dir(diff.old_value, dir2)
+            n = remove_compilation_dir(diff.new_value, dir1)
 
-            if diff.old_value =~ /^(.*)#{dir2}(.*)$/m
-              from_before = Regexp.last_match(1) || ''
-              from_after = Regexp.last_match(2) || ''
-              from_match = true
-            end
-
-            if diff.new_value =~ /^(.*)#{dir1}(.*)$/m
-              to_before = Regexp.last_match(1) || ''
-              to_after = Regexp.last_match(2) || ''
-              to_match = true
-            end
-
-            if from_match && to_match && to_before == from_before && to_after == from_after
+            if o != diff.old_value || n != diff.new_value
               message = "Resource key #{diff.type}[#{diff.title}] #{diff.structure.join(' => ')}"
-              message += ' appears to depend on catalog compilation directory. Suppressed from results.'
+              message += ' may depend on catalog compilation directory, but there may be differences.'
+              message += ' This is included in results for now, but please verify.'
               @logger.warn message
-              return true
             end
 
-            if from_match || to_match
+            if o == n
               message = "Resource key #{diff.type}[#{diff.title}] #{diff.structure.join(' => ')}"
-              message += ' may depend on catalog compilation directory, but there may be differences.'
-              message += ' This is included in results for now, but please verify.'
+              message += ' appears to depend on catalog compilation directory. Suppressed from results.'
               @logger.warn message
+              return true
             end
           end
 
           false
         end
+
+        def remove_compilation_dir(v, dir)
+          value = OctocatalogDiff::Util::Util.deep_dup(v)
+          traverse(value) do |e|
+            e.gsub!(dir, '') if e.respond_to?(:gsub!)
+          end
+          value
+        end
+
+        def traverse(a)
+          case a
+          when Array
+            a.map { |v| traverse(v, &Proc.new) }
+          when Hash
+            traverse(a.values, &Proc.new)
+          else
+            yield a
+          end
+        end
       end
     end
   end

script/git-pre-commit

@@ -4,8 +4,10 @@
 # base directory is up two levels, not just one.
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd ../.. && pwd )"
 
+cd "$DIR"
+
 # Make sure we can use git correctly
-cd "$DIR" && git rev-parse --verify HEAD >/dev/null 2>&1
+git rev-parse --verify HEAD >/dev/null 2>&1
 if [ $? -ne 0 ]; then
   echo "Unable to determine revision of this git repo"
   exit 1

spec/octocatalog-diff/fixtures/catalogs/compilation-dir-1.json

@@ -53,6 +53,23 @@
         "parameters": {
           "dir": "/path/to/catalog1/onetime"
         }
+      },
+      {
+        "type": "Varies_Due_To_Compilation_Dir_5",
+        "title": "Common Title",
+        "tags": [
+          "ignoreme"
+        ],
+        "exported": false,
+        "parameters": {
+          "dir": {
+            "component": [
+              "path",
+              "/path/to/catalog1/twotimes",
+              "otherpath"
+            ]
+          }
+        }
       }
     ],
     "classes": [

spec/octocatalog-diff/fixtures/catalogs/compilation-dir-2.json

@@ -53,6 +53,19 @@
         "parameters": {
           "dir": "/path/to/catalog2/twotimes"
         }
+      },
+      {
+        "type": "Varies_Due_To_Compilation_Dir_5",
+        "title": "Common Title",
+        "tags": [
+          "ignoreme"
+        ],
+        "exported": false,
+        "parameters": {
+          "dir": {
+            "component": [ "path", "/path/to/catalog2/twotimes", "otherpath" ]
+          }
+        }
       }
     ],
     "classes": [

spec/octocatalog-diff/tests/catalog-diff/filter/compilation_dir_spec.rb

@@ -165,6 +165,19 @@
       diff_obj = OctocatalogDiff::API::V1::Diff.factory(diff)
       expect(subject.filtered?(diff_obj, opts)).to eq(false)
     end
+
+    it 'should remove a change where directories appear more than one time' do
+      diff = [
+        '~',
+        "Varies_Due_To_Compilation_Dir_3\fCommon Title\fparameters\fdir",
+        'command -a /path/to/catalog1/something -b common-stuff -a /path/to/catalog1/otherthing',
+        'command -a /path/to/catalog2/something -b common-stuff -a /path/to/catalog2/otherthing',
+        { 'file' => nil, 'line' => nil },
+        { 'file' => nil, 'line' => nil }
+      ]
+      diff_obj = OctocatalogDiff::API::V1::Diff.factory(diff)
+      expect(subject.filtered?(diff_obj, opts)).to eq(true)
+    end
   end
 
   context '~ partial indeterminate matches' do
@@ -190,4 +203,40 @@
       expect(@logger_str.string).to match(/WARN.*Varies_Due_To_Compilation_Dir_3\[Common Title\] parameters => dir.+differences/)
     end
   end
+
+  context '~ array value changes' do
+    let(:diff) do
+      [
+        '~',
+        "Varies_Due_To_Compilation_Dir_3\fCommon Title\fparameters\fdir",
+        ['something that doesn\'t change', '/path/to/catalog1', 'something else'],
+        ['something that doesn\'t change', '/path/to/catalog2', 'something else'],
+        { 'file' => nil, 'line' => nil },
+        { 'file' => nil, 'line' => nil }
+      ]
+    end
+
+    it 'should remove a change where directories are a full match' do
+      diff_obj = OctocatalogDiff::API::V1::Diff.factory(diff)
+      expect(subject.filtered?(diff_obj, opts)).to eq(true)
+    end
+  end
+
+  context '~ nested hash changes' do
+    let(:diff) do
+      [
+        '~',
+        "Varies_Due_To_Compilation_Dir_3\fCommon Title\fparameters\fdir",
+        { 'value' => { 'path' => ['thing', '/path/to/catalog1/file', 'otherthing'] } },
+        { 'value' => { 'path' => ['thing', '/path/to/catalog2/file', 'otherthing'] } },
+        { 'file' => nil, 'line' => nil },
+        { 'file' => nil, 'line' => nil }
+      ]
+    end
+
+    it 'should remove a change where directories are a full match' do
+      diff_obj = OctocatalogDiff::API::V1::Diff.factory(diff)
+      expect(subject.filtered?(diff_obj, opts)).to eq(true)
+    end
+  end
 end