From c58ff568d200ecb033cd4f8b29a34339480f9265 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Mon, 14 Jun 2021 11:27:19 -0500 Subject: [PATCH] Add EquivalentArrayNoDatatypes filter --- doc/advanced-filter.md | 23 +++- lib/octocatalog-diff/catalog-diff/filter.rb | 3 +- .../filter/equivalent_array_no_datatypes.rb | 64 +++++++++ .../catalogs/filter-equivalent-array-1.json | 55 ++++++++ .../catalogs/filter-equivalent-array-2.json | 55 ++++++++ ...lter_equivalent_array_no_datatypes_spec.rb | 52 ++++++++ .../equivalent_array_no_datatypes_spec.rb | 121 ++++++++++++++++++ 7 files changed, 370 insertions(+), 3 deletions(-) create mode 100644 lib/octocatalog-diff/catalog-diff/filter/equivalent_array_no_datatypes.rb create mode 100644 spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-1.json create mode 100644 spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-2.json create mode 100644 spec/octocatalog-diff/integration/filter_equivalent_array_no_datatypes_spec.rb create mode 100644 spec/octocatalog-diff/tests/catalog-diff/filter/equivalent_array_no_datatypes_spec.rb diff --git a/doc/advanced-filter.md b/doc/advanced-filter.md index 2efc3ef1..5bfe4410 100644 --- a/doc/advanced-filter.md +++ b/doc/advanced-filter.md @@ -24,7 +24,7 @@ Here is the list of available filters and an explanation of each: #### Description -When the `AbsentFile` filter is enabled, if any file is `ensure => absent` in the *new* catalog, then changes to any other parameters will be suppressed. +When the `AbsentFile` filter is enabled, if any file is `ensure => absent` in the _new_ catalog, then changes to any other parameters will be suppressed. Consider that a file resource is declared as follows in two catalogs: @@ -71,6 +71,25 @@ Wouldn't it be nice if the meaningless information didn't appear, and all you sa + absent ``` +## Equivalent Array (not considering datatypes) + +#### Usage + +``` +--filters EquivalentArrayNoDatatypes +``` + +#### Description + +In an array, ignore changes where the old and new arrays are "equivalent" as described below. This is useful when octocatalog-diff is comparing changes between a catalog with stringified values and a catalog with non-stringified values. + +The following are considered equivalent when this filter is engaged: + +- Stringified integers (`[0, 1]` and `['0', '1']`) +- Stringified floats (`[0.0, 1.0]` and `['0.0', '1.0']`) +- Numerically-equal integers and floats (`[0, 1]` and `[0.0, 1.0]`) +- Symbols and corresponding strings (`[:foo, :bar]` and `[':foo', ':bar']`) + ## JSON #### Usage @@ -105,7 +124,7 @@ New: { "notify": [ "Service[foo]" ] } This filter will suppress differences for the value of a parameter when: - The value in one catalog is an object, AND -- The value in the other catalog is an array containing *only* that same object +- The value in the other catalog is an array containing _only_ that same object ## YAML diff --git a/lib/octocatalog-diff/catalog-diff/filter.rb b/lib/octocatalog-diff/catalog-diff/filter.rb index bdd484d4..e0ccfb1b 100644 --- a/lib/octocatalog-diff/catalog-diff/filter.rb +++ b/lib/octocatalog-diff/catalog-diff/filter.rb @@ -1,6 +1,7 @@ require_relative '../api/v1/diff' require_relative 'filter/absent_file' require_relative 'filter/compilation_dir' +require_relative 'filter/equivalent_array_no_datatypes' require_relative 'filter/json' require_relative 'filter/single_item_array' require_relative 'filter/yaml' @@ -14,7 +15,7 @@ class Filter attr_accessor :logger # List the available filters here (by class name) for use in the validator method. - AVAILABLE_FILTERS = %w(AbsentFile CompilationDir JSON SingleItemArray YAML).freeze + AVAILABLE_FILTERS = %w(AbsentFile CompilationDir EquivalentArrayNoDatatypes JSON SingleItemArray YAML).freeze # Public: Determine whether a particular filter exists. This can be used to validate # a user-submitted filter. diff --git a/lib/octocatalog-diff/catalog-diff/filter/equivalent_array_no_datatypes.rb b/lib/octocatalog-diff/catalog-diff/filter/equivalent_array_no_datatypes.rb new file mode 100644 index 00000000..9c3226a8 --- /dev/null +++ b/lib/octocatalog-diff/catalog-diff/filter/equivalent_array_no_datatypes.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require_relative '../filter' + +module OctocatalogDiff + module CatalogDiff + class Filter + # Filter out changes in parameters where the elements of an array are the + # same values but different data types. For example, this would filter out + # the following diffs: + # Exec[some command] => + # parameters => + # returns => + # - ["0", "1"] + # + [0, 1] + class EquivalentArrayNoDatatypes < OctocatalogDiff::CatalogDiff::Filter + # Public: Implement the filter for arrays that have the same elements + # but possibly different data types. + # + # @param diff [OctocatalogDiff::API::V1::Diff] Difference + # @param _options [Hash] Additional options (there are none for this filter) + # @return [Boolean] true if this should be filtered out, false otherwise + def filtered?(diff, _options = {}) + # Skip additions or removals - focus only on changes + return false unless diff.change? + old_value = diff.old_value + new_value = diff.new_value + + # Skip unless both the old and new values are arrays. + return false unless old_value.is_a?(Array) && new_value.is_a?(Array) + + # Avoid generating comparable values if the arrays are a different + # size, because there's no possible way that they are equivalent. + return false unless old_value.size == new_value.size + + # Generate and then compare the comparable arrays. + old_value.map { |x| comparable_value(x) } == new_value.map { |x| comparable_value(x) } + end + + # Private: Get a more easily comparable value for an array element. + # Integers, floats, and strings that look like integers or floats become + # floats, and symbols are converted to string representation. + # + # @param input [any] Value to convert + # @return [any] "Comparable" value + def comparable_value(input) + # Any string that looks like a number is converted to a float. + if input.is_a?(String) && input =~ /\A-?(([0-9]*\.[0-9]+)|([0-9]+))\z/ + return input.to_f + end + + # Any number is converted to a float + return input.to_f if input.is_a?(Integer) || input.is_a?(Float) + + # Symbols are converted to ":xxx" strings. + return ":#{input}" if input.is_a?(Symbol) + + # Everything else is unconverted. + input + end + end + end + end +end diff --git a/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-1.json b/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-1.json new file mode 100644 index 00000000..1df413f1 --- /dev/null +++ b/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-1.json @@ -0,0 +1,55 @@ +{ + "document_type": "Catalog", + "data": { + "tags": [ + "settings" + ], + "name": "my.rspec.node", + "version": "production", + "environment": "production", + "resources": [ + { + "type": "Exec", + "title": "run-my-command 1", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": "0" + } + }, + { + "type": "Exec", + "title": "run-my-command 2", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": ["0", "1"] + } + }, + { + "type": "Exec", + "title": "run-my-command 3", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": ["0", "1", "2"] + } + } + ], + "classes": [ + "settings" + ] + }, + "metadata": { + "api_version": 1 + } +} diff --git a/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-2.json b/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-2.json new file mode 100644 index 00000000..039644c0 --- /dev/null +++ b/spec/octocatalog-diff/fixtures/catalogs/filter-equivalent-array-2.json @@ -0,0 +1,55 @@ +{ + "document_type": "Catalog", + "data": { + "tags": [ + "settings" + ], + "name": "my.rspec.node", + "version": "production", + "environment": "production", + "resources": [ + { + "type": "Exec", + "title": "run-my-command 1", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": 0 + } + }, + { + "type": "Exec", + "title": "run-my-command 2", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": [0, 1] + } + }, + { + "type": "Exec", + "title": "run-my-command 3", + "file": "/environments/production/modules/foo/manifests/init.pp", + "line": 10, + "exported": false, + "parameters": { + "path": "/usr/bin", + "command": "id", + "returns": [0, 1, 2, 3] + } + } + ], + "classes": [ + "settings" + ] + }, + "metadata": { + "api_version": 1 + } +} diff --git a/spec/octocatalog-diff/integration/filter_equivalent_array_no_datatypes_spec.rb b/spec/octocatalog-diff/integration/filter_equivalent_array_no_datatypes_spec.rb new file mode 100644 index 00000000..873ef9b4 --- /dev/null +++ b/spec/octocatalog-diff/integration/filter_equivalent_array_no_datatypes_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require_relative 'integration_helper' + +describe 'equivalent array no datatypes filter integration' do + context 'with default behavior' do + before(:all) do + @result = OctocatalogDiff::Integration.integration( + spec_catalog_old: 'filter-equivalent-array-1.json', + spec_catalog_new: 'filter-equivalent-array-2.json' + ) + end + + it 'should succeed' do + expect(@result[:exitcode]).not_to eq(-1), "Internal error: #{@result[:exception]}\n#{@result[:logs]}" + expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}" + end + + it 'should not suppress equivalent-but-for-data-type arrays' do + diffs = OctocatalogDiff::Spec.remove_file_and_line(@result[:diffs]) + expect(diffs.size).to eq(3), diffs.inspect + expect(diffs[0][1..3]).to eq(["Exec\frun-my-command 1\fparameters\freturns", '0'.inspect, 0]) + expect(diffs[1][1..3]).to eq(["Exec\frun-my-command 2\fparameters\freturns", %w[0 1], [0, 1]]) + expect(diffs[2][1..3]).to eq(["Exec\frun-my-command 3\fparameters\freturns", %w[0 1 2], [0, 1, 2, 3]]) + end + end + + context 'with equivalent array no datatypes filter engaged' do + before(:all) do + @result = OctocatalogDiff::Integration.integration( + spec_catalog_old: 'filter-equivalent-array-1.json', + spec_catalog_new: 'filter-equivalent-array-2.json', + argv: ['--filters', 'EquivalentArrayNoDatatypes'] + ) + end + + it 'should succeed' do + expect(@result[:exitcode]).not_to eq(-1), "Internal error: #{@result[:exception]}\n#{@result[:logs]}" + expect(@result[:exitcode]).to eq(2), "Runtime error: #{@result[:logs]}" + end + + it 'should suppress equivalent-but-for-data-type arrays' do + diffs = OctocatalogDiff::Spec.remove_file_and_line(@result[:diffs]) + expect(diffs.size).to eq(2), diffs.inspect + # '0' => 0 is not suppressed because it's not an array + expect(diffs[0][1..3]).to eq(["Exec\frun-my-command 1\fparameters\freturns", '0'.inspect, 0]) + # %w[0 1] => [0, 1] is suppressed + # %w[0 1 2] => [0, 1, 2, 3] is not suppressed because it's not equivalent + expect(diffs[1][1..3]).to eq(["Exec\frun-my-command 3\fparameters\freturns", %w[0 1 2], [0, 1, 2, 3]]) + end + end +end diff --git a/spec/octocatalog-diff/tests/catalog-diff/filter/equivalent_array_no_datatypes_spec.rb b/spec/octocatalog-diff/tests/catalog-diff/filter/equivalent_array_no_datatypes_spec.rb new file mode 100644 index 00000000..14f56b42 --- /dev/null +++ b/spec/octocatalog-diff/tests/catalog-diff/filter/equivalent_array_no_datatypes_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require_relative '../../spec_helper' +require OctocatalogDiff::Spec.require_path('/api/v1/diff') +require OctocatalogDiff::Spec.require_path('/catalog-diff/filter/equivalent_array_no_datatypes') + +describe OctocatalogDiff::CatalogDiff::Filter::EquivalentArrayNoDatatypes do + let(:subject) { described_class.new } + + describe '#filtered?' do + it 'should not filter out an added resource' do + diff = ['+', "Exec\fmy-command", { 'parameters' => { 'returns' => [0, 1] } }] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + + it 'should not filter out a removed resource' do + diff = ['-', "Exec\fmy-command", { 'parameters' => { 'returns' => [0, 1] } }] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + + it 'should not filter when old value is not an array' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + '1', + [1] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + + it 'should not filter when new value is not an array' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + ['1'], + '1' + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + + it 'should not filter when arrays have different sizes' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + %w[1 2], + [1, 2, 3] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + + it 'should filter for identical arrays' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + [0, 1], + [0, 1] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(true) + end + + it 'should filter for integers, floats, and strings in arrays' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + ['0', 1, 3.14159, '-4', '0.222', '.333', '-1.444', '5.555'], + [0, '1', '3.14159', -4, 0.222, 0.333, -1.444, 5.555] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(true) + end + + it 'should filter integer-value floats and equivalent integers' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + ['0.000', 1, -3], + [0, 1.0, -3.000] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(true) + end + + it 'should filter symbols' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + [:foo, ':bar'], + [':foo', :bar] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(true) + end + + it 'should not filter data structures' do + diff = [ + '~', + "Exec\fmy-command\fparameters\freturns", + [[], {}], + [{}, []] + ] + diff_obj = OctocatalogDiff::API::V1::Diff.new(diff) + result = subject.filtered?(diff_obj) + expect(result).to eq(false) + end + end +end