Skip to content

Commit 961cc7f

Browse files
author
Kevin Paulisse
committed
All filters expect they are getting a diff object not the array representation
1 parent 5811a8d commit 961cc7f

8 files changed

Lines changed: 80 additions & 48 deletions

File tree

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
require_relative '../api/v1/diff'
12
require_relative 'filter/absent_file'
23
require_relative 'filter/compilation_dir'
34
require_relative 'filter/yaml'
@@ -49,8 +50,14 @@ def self.apply_filters(result, filter_names, options = {})
4950
def self.filter(result, filter_class_name, options = {})
5051
assert_that_filter_exists(filter_class_name)
5152
filter_class_name = [name.to_s, filter_class_name].join('::')
52-
obj = Kernel.const_get(filter_class_name).new(result, options[:logger])
53-
result.reject! { |item| obj.filtered?(item, options) }
53+
54+
# Need to convert each of the results array to the OctocatalogDiff::API::V1::Diff object, if
55+
# it isn't already. The comparison is done on that array which is then applied back to the
56+
# original array.
57+
result_hash = {}
58+
result.each { |x| result_hash[x] = OctocatalogDiff::API::V1::Diff.construct(x) }
59+
obj = Kernel.const_get(filter_class_name).new(result_hash.values, options[:logger])
60+
result.reject! { |item| obj.filtered?(result_hash[item], options) }
5461
end
5562

5663
# Inherited: Constructor. Some filters require working on the entire data set and

‎lib/octocatalog-diff/catalog-diff/filter/absent_file.rb‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# frozen_string_literal: true
22

3-
require_relative '../../api/v1/diff'
43
require_relative '../filter'
54

65
require 'set'
@@ -14,8 +13,10 @@ class AbsentFile < OctocatalogDiff::CatalogDiff::Filter
1413

1514
# Constructor: Since this filter requires knowledge of the entire array of diffs,
1615
# override the inherited method to store those diffs in an instance variable.
16+
# @param diffs [Array<OctocatalogDiff::API::V1::Diff>] Difference array
17+
# @param _logger [?] Ignored
1718
def initialize(diffs, _logger = nil)
18-
@diffs = diffs.map { |x| OctocatalogDiff::API::V1::Diff.new(x) }
19+
@diffs = diffs
1920
@results = nil
2021
end
2122

@@ -46,7 +47,7 @@ def build_results
4647
end
4748

4849
# Based on that, which diffs can we ignore?
49-
@results = Set.new @diffs.reject { |diff| keep_diff?(diff) }.map(&:raw)
50+
@results = Set.new @diffs.reject { |diff| keep_diff?(diff) }
5051
end
5152

5253
# Private: Determine whether to keep a particular diff.

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# frozen_string_literal: true
22

3-
require_relative '../../api/v1/diff'
43
require_relative '../filter'
54

65
module OctocatalogDiff
@@ -13,19 +12,18 @@ class CompilationDir < OctocatalogDiff::CatalogDiff::Filter
1312
# (supplied via options) and checking the differences. If the only thing different
1413
# is the compilation directory, filter it out with a warning.
1514
#
16-
# @param diff_in [Array (Internal Diff Format)] Difference
15+
# @param diff [OctocatalogDiff::API::V1::Diff] Difference
1716
# @param options [Hash] Additional options:
1817
# :from_compilation_dir [String] Compilation directory for the "from" catalog
1918
# :to_compilation_dir [String] Compilation directory for the "to" catalog
2019
# @return [Boolean] true if this difference is a YAML file with identical objects, false otherwise
21-
def filtered?(diff_in, options = {})
20+
def filtered?(diff, options = {})
2221
return false unless options[:from_compilation_dir] && options[:to_compilation_dir]
2322
dir1 = options[:to_compilation_dir]
2423
dir1_rexp = Regexp.escape(dir1)
2524
dir2 = options[:from_compilation_dir]
2625
dir2_rexp = Regexp.escape(dir2)
2726
dir = Regexp.new("(?:#{dir1_rexp}|#{dir2_rexp})")
28-
diff = OctocatalogDiff::API::V1::Diff.new(diff_in)
2927

3028
# Check for added/removed resources where the title of the resource includes the compilation directory
3129
if (diff.addition? || diff.removal?) && diff.title.match(dir)

‎lib/octocatalog-diff/catalog-diff/filter/yaml.rb‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# frozen_string_literal: true
22

3-
require_relative '../../api/v1/diff'
43
require_relative '../filter'
54

65
require 'yaml'
@@ -14,13 +13,10 @@ class YAML < OctocatalogDiff::CatalogDiff::Filter
1413
# Return true if the YAML objects are known to be equivalent. Return false if they
1514
# are not equivalent, or if equivalence cannot be determined.
1615
#
17-
# @param diff_in [Array] Difference
16+
# @param diff_in [OctocatalogDiff::API::V1::Diff] Difference
1817
# @param _options [Hash] Additional options (there are none for this filter)
1918
# @return [Boolean] true if this difference is a YAML file with identical objects, false otherwise
20-
def filtered?(diff_in, _options = {})
21-
# Create API object for diff to simplify code
22-
diff = OctocatalogDiff::API::V1::Diff.new(diff_in)
23-
19+
def filtered?(diff, _options = {})
2420
# Skip additions or removals - focus only on changes
2521
return false unless diff.change?
2622

‎spec/octocatalog-diff/tests/catalog-diff/filter/absent_file_spec.rb‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require_relative '../../spec_helper'
4+
require OctocatalogDiff::Spec.require_path('/api/v1/diff')
45
require OctocatalogDiff::Spec.require_path('/catalog')
56
require OctocatalogDiff::Spec.require_path('/catalog-diff/filter/absent_file')
67

@@ -16,8 +17,9 @@
1617
['~', "File\f/tmp/bar\fparameters\ftarget", nil, '/tmp/foo'],
1718
['~', "Exec\f/tmp/bar\fparameters\fcommand", nil, '/tmp/foo']
1819
]
19-
testobj = described_class.new(orig)
20-
result = orig.reject { |x| testobj.filtered?(x) }
21-
expect(result).to eq(orig.values_at(0, 2, 3, 4, 5, 6, 7))
20+
obj = orig.map { |x| OctocatalogDiff::API::V1::Diff.construct(x) }
21+
testobj = described_class.new(obj)
22+
result = obj.reject { |x| testobj.filtered?(x) }
23+
expect(result).to eq(obj.values_at(0, 2, 3, 4, 5, 6, 7))
2224
end
2325
end

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require_relative '../../spec_helper'
4+
require OctocatalogDiff::Spec.require_path('/api/v1/diff')
45
require OctocatalogDiff::Spec.require_path('/catalog-diff/filter/compilation_dir')
56

67
describe OctocatalogDiff::CatalogDiff::Filter::CompilationDir do
@@ -25,7 +26,8 @@
2526
},
2627
{ 'file' => nil, 'line' => nil }
2728
]
28-
expect(subject.filtered?(diff, opts)).to eq(true)
29+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
30+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
2931
end
3032

3133
it 'should remove due to compilation dirs in from-catalog' do
@@ -40,7 +42,8 @@
4042
},
4143
{ 'file' => nil, 'line' => nil }
4244
]
43-
expect(subject.filtered?(diff, opts)).to eq(true)
45+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
46+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
4447
end
4548

4649
it 'should not remove a non-matching directory' do
@@ -55,7 +58,8 @@
5558
},
5659
{ 'file' => nil, 'line' => nil }
5760
]
58-
expect(subject.filtered?(diff, opts)).to eq(false)
61+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
62+
expect(subject.filtered?(diff_obj, opts)).to eq(false)
5963
end
6064
end
6165

@@ -72,7 +76,8 @@
7276
},
7377
{ 'file' => nil, 'line' => nil }
7478
]
75-
expect(subject.filtered?(diff, opts)).to eq(true)
79+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
80+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
7681
end
7782

7883
it 'should remove due to compilation dirs in from-catalog' do
@@ -87,7 +92,8 @@
8792
},
8893
{ 'file' => nil, 'line' => nil }
8994
]
90-
expect(subject.filtered?(diff, opts)).to eq(true)
95+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
96+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
9197
end
9298

9399
it 'should not remove a non-matching directory' do
@@ -102,7 +108,8 @@
102108
},
103109
{ 'file' => nil, 'line' => nil }
104110
]
105-
expect(subject.filtered?(diff, opts)).to eq(false)
111+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
112+
expect(subject.filtered?(diff_obj, opts)).to eq(false)
106113
end
107114
end
108115

@@ -116,7 +123,8 @@
116123
{ 'file' => nil, 'line' => nil },
117124
{ 'file' => nil, 'line' => nil }
118125
]
119-
expect(subject.filtered?(diff, opts)).to eq(true)
126+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
127+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
120128
end
121129

122130
it 'should remove a change where directories are a full match' do
@@ -128,7 +136,8 @@
128136
{ 'file' => nil, 'line' => nil },
129137
{ 'file' => nil, 'line' => nil }
130138
]
131-
expect(subject.filtered?(diff, opts)).to eq(true)
139+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
140+
expect(subject.filtered?(diff_obj, opts)).to eq(true)
132141
end
133142

134143
it 'should not remove a change where directories are inverted' do
@@ -140,7 +149,8 @@
140149
{ 'file' => nil, 'line' => nil },
141150
{ 'file' => nil, 'line' => nil }
142151
]
143-
expect(subject.filtered?(diff, opts)).to eq(false)
152+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
153+
expect(subject.filtered?(diff_obj, opts)).to eq(false)
144154
end
145155

146156
it 'should not remove a change where directories do not match' do
@@ -152,7 +162,8 @@
152162
{ 'file' => nil, 'line' => nil },
153163
{ 'file' => nil, 'line' => nil }
154164
]
155-
expect(subject.filtered?(diff, opts)).to eq(false)
165+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
166+
expect(subject.filtered?(diff_obj, opts)).to eq(false)
156167
end
157168
end
158169

@@ -169,11 +180,13 @@
169180
end
170181

171182
it 'should not remove changes that do not match fully' do
172-
expect(subject.filtered?(diff, opts)).to eq(false)
183+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
184+
expect(subject.filtered?(diff_obj, opts)).to eq(false)
173185
end
174186

175187
it 'should log warning message' do
176-
subject.filtered?(diff, opts)
188+
diff_obj = OctocatalogDiff::API::V1::Diff.construct(diff)
189+
subject.filtered?(diff_obj, opts)
177190
expect(@logger_str.string).to match(/WARN.*Varies_Due_To_Compilation_Dir_3\[Common Title\] parameters => dir.+differences/)
178191
end
179192
end

‎spec/octocatalog-diff/tests/catalog-diff/filter/yaml_spec.rb‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require_relative '../../spec_helper'
4+
require OctocatalogDiff::Spec.require_path('/api/v1/diff')
45
require OctocatalogDiff::Spec.require_path('/catalog-diff/filter/yaml')
56

67
describe OctocatalogDiff::CatalogDiff::Filter::YAML do
@@ -13,63 +14,73 @@
1314

1415
it 'should not filter out an added resource' do
1516
diff = ['+', "File\ffoobar.yaml", { 'parameters' => { 'content' => str1a } }]
16-
result = subject.filtered?(diff)
17+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
18+
result = subject.filtered?(diff_obj)
1719
expect(result).to eq(false)
1820
end
1921

2022
it 'should not filter out a removed resource' do
2123
diff = ['-', "File\ffoobar.yaml", { 'parameters' => { 'content' => str1a } }]
22-
result = subject.filtered?(diff)
24+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
25+
result = subject.filtered?(diff_obj)
2326
expect(result).to eq(false)
2427
end
2528

2629
it 'should not filter out a non-file resource' do
2730
diff = ['~', "Exec\ffoobar.yaml\fparameters\fcontent", str1a, str1b]
28-
result = subject.filtered?(diff)
31+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
32+
result = subject.filtered?(diff_obj)
2933
expect(result).to eq(false)
3034
end
3135

3236
it 'should not filter out a file whose extension is not .yaml / .yml' do
3337
diff = ['~', "File\ffoobar.json\fparameters\fcontent", str1a, str1b]
34-
result = subject.filtered?(diff)
38+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
39+
result = subject.filtered?(diff_obj)
3540
expect(result).to eq(false)
3641
end
3742

3843
it 'should not filter out a change with no content change' do
3944
diff = ['~', "File\ffoobar.json\fparameters\fowner", 'root', 'nobody']
40-
result = subject.filtered?(diff)
45+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
46+
result = subject.filtered?(diff_obj)
4147
expect(result).to eq(false)
4248
end
4349

4450
it 'should not filter out a change where YAML objects are dissimilar' do
4551
diff = ['~', "File\ffoobar.yaml\fparameters\fcontent", str1a, str2]
46-
result = subject.filtered?(diff)
52+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
53+
result = subject.filtered?(diff_obj)
4754
expect(result).to eq(false)
4855
end
4956

5057
it 'should not filter out a change where YAML is invalid' do
5158
x_str = '---{ "blah": "foo" }'
5259
diff = ['~', "File\ffoobar.yaml\fparameters\fcontent", x_str, x_str]
53-
result = subject.filtered?(diff)
60+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
61+
result = subject.filtered?(diff_obj)
5462
expect(result).to eq(false)
5563
end
5664

5765
it 'should not filter out a change where YAML is unparseable' do
5866
x_str = "--- !ruby/object:This::Does::Not::Exist\n foo: bar"
5967
diff = ['~', "File\ffoobar.yaml\fparameters\fcontent", x_str, x_str]
60-
result = subject.filtered?(diff)
68+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
69+
result = subject.filtered?(diff_obj)
6170
expect(result).to eq(false)
6271
end
6372

6473
it 'should filter out a whitespace-only change to a .yaml file' do
6574
diff = ['~', "File\ffoobar.yaml\fparameters\fcontent", str1a, str1b]
66-
result = subject.filtered?(diff)
75+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
76+
result = subject.filtered?(diff_obj)
6777
expect(result).to eq(true)
6878
end
6979

7080
it 'should filter out a whitespace-only change to a .yml file' do
7181
diff = ['~', "File\ffoobar.yml\fparameters\fcontent", str1a, str1b]
72-
result = subject.filtered?(diff)
82+
diff_obj = OctocatalogDiff::API::V1::Diff.new(diff)
83+
result = subject.filtered?(diff_obj)
7384
expect(result).to eq(true)
7485
end
7586
end

‎spec/octocatalog-diff/tests/catalog-diff/filter_spec.rb‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require_relative '../spec_helper'
4+
require OctocatalogDiff::Spec.require_path('/api/v1/diff')
45
require OctocatalogDiff::Spec.require_path('/catalog-diff/filter')
56

67
# rubocop:disable Style/ClassAndModuleChildren
@@ -43,27 +44,30 @@ class OctocatalogDiff::CatalogDiff::Filter::Fake2 < OctocatalogDiff::CatalogDiff
4344

4445
describe '#apply_filters' do
4546
it 'should call self.filter() with appropriate options for each class' do
46-
result = [false]
47+
diff1 = OctocatalogDiff::API::V1::Diff.new(['-', "File\f/tmp/foo"])
48+
result = [diff1]
4749
options = { foo: 'bar' }
4850
classes = %w(Fake1 Fake2)
4951
allow(described_class).to receive(:"filter?").with('Fake1').and_return(true)
5052
allow(described_class).to receive(:"filter?").with('Fake2').and_return(true)
51-
expect_any_instance_of(@class_1).to receive(:'filtered?').with(false, foo: 'bar').and_return(false)
52-
expect_any_instance_of(@class_2).to receive(:'filtered?').with(false, foo: 'bar').and_return(false)
53+
expect_any_instance_of(@class_1).to receive(:'filtered?').with(diff1, foo: 'bar').and_return(false)
54+
expect_any_instance_of(@class_2).to receive(:'filtered?').with(diff1, foo: 'bar').and_return(false)
5355
expect { described_class.apply_filters(result, classes, options) }.not_to raise_error
54-
expect(result).to eq([false])
56+
expect(result).to eq([diff1])
5557
end
5658
end
5759

5860
describe '#filter' do
5961
it 'should call .filtered?() in a class and remove matching items' do
60-
result = [false, true]
62+
diff1 = OctocatalogDiff::API::V1::Diff.new(['-', "File\f/tmp/foo"])
63+
diff2 = OctocatalogDiff::API::V1::Diff.new(['+', "File\f/tmp/foo"])
64+
result = [diff1, diff2]
6165
allow(described_class).to receive(:"filter?").with('Fake1').and_return(true)
6266
allow(described_class).to receive(:"filter?").with('Fake2').and_return(true)
63-
expect_any_instance_of(@class_1).to receive(:'filtered?').with(false, {}).and_return(false)
64-
expect_any_instance_of(@class_1).to receive(:'filtered?').with(true, {}).and_return(true)
67+
expect_any_instance_of(@class_1).to receive(:'filtered?').with(diff1, {}).and_return(false)
68+
expect_any_instance_of(@class_1).to receive(:'filtered?').with(diff2, {}).and_return(true)
6569
expect { described_class.filter(result, 'Fake1') }.not_to raise_error
66-
expect(result).to eq([false])
70+
expect(result).to eq([diff1])
6771
end
6872
end
6973

0 commit comments

Comments
 (0)