From 7dc76dca528733a237c9778280cdcf41731ebdcb Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Wed, 25 Jul 2018 20:45:47 +1000 Subject: [PATCH] fix: correctly handle an 'each like' inside a 'like' Closes: https://github.com/pact-foundation/pact-support/issues/47 https://github.com/pact-foundation/pact-go/issues/54 https://github.com/pact-foundation/pact-provider-verifier/issues/24 --- lib/pact/matching_rules/merge.rb | 2 +- lib/pact/matching_rules/v3/merge.rb | 52 ++++++++++++------- .../matching_rules_extract_and_merge_spec.rb | 45 ++++++++++++++-- spec/lib/pact/matching_rules/v3/merge_spec.rb | 28 ++++++++-- 4 files changed, 101 insertions(+), 26 deletions(-) diff --git a/lib/pact/matching_rules/merge.rb b/lib/pact/matching_rules/merge.rb index 1480be1..0487b2a 100644 --- a/lib/pact/matching_rules/merge.rb +++ b/lib/pact/matching_rules/merge.rb @@ -92,7 +92,7 @@ def wrap object, path def handle_match_type object, path, rules log_used_rule(path, 'match', 'type') - Pact::SomethingLike.new(object) + Pact::SomethingLike.new(recurse(object, path)) end def handle_regex object, path, rules diff --git a/lib/pact/matching_rules/v3/merge.rb b/lib/pact/matching_rules/v3/merge.rb index eb8edfa..f9d29f3 100644 --- a/lib/pact/matching_rules/v3/merge.rb +++ b/lib/pact/matching_rules/v3/merge.rb @@ -18,15 +18,15 @@ def initialize expected, matching_rules, root_path def call return @expected if @matching_rules.nil? || @matching_rules.empty? - recurse @expected, @root_path + recurse(@expected, @root_path).tap { log_ignored_rules } end private def standardise_paths matching_rules return matching_rules if matching_rules.nil? || matching_rules.empty? - matching_rules.each_with_object({}) do | (path, rule), new_matching_rules | - new_matching_rules[JsonPath.new(path).to_s] = rule + matching_rules.each_with_object({}) do | (path, rules), new_matching_rules | + new_matching_rules[JsonPath.new(path).to_s] = Marshal.load(Marshal.dump(rules)) # simplest way to deep clone end end @@ -47,14 +47,14 @@ def recurse_hash hash, path end def recurse_array array, path + + parent_match_rule = @matching_rules[path] && @matching_rules[path]['matchers'] && @matching_rules[path]['matchers'].first && @matching_rules[path]['matchers'].first.delete('match') array_like_children_path = "#{path}[*]*" - parent_match_rule = @matching_rules[path] && @matching_rules[path]['matchers'] && @matching_rules[path]['matchers'].first && @matching_rules[path]['matchers'].first['match'] - children_match_rule = @matching_rules[array_like_children_path] && @matching_rules[array_like_children_path]['matchers'] && @matching_rules[array_like_children_path]['matchers'].first && @matching_rules[array_like_children_path]['matchers'].first['match'] - min = @matching_rules[path] && @matching_rules[path]['matchers'] && @matching_rules[path]['matchers'].first && @matching_rules[path]['matchers'].first['min'] + children_match_rule = @matching_rules[array_like_children_path] && @matching_rules[array_like_children_path]['matchers'] && @matching_rules[array_like_children_path]['matchers'].first && @matching_rules[array_like_children_path]['matchers'].first.delete('match') + min = @matching_rules[path] && @matching_rules[path]['matchers'] && @matching_rules[path]['matchers'].first && @matching_rules[path]['matchers'].first.delete('min') if min && (children_match_rule == 'type' || (children_match_rule.nil? && parent_match_rule == 'type')) warn_when_not_one_example_item(array, path) - # log_ignored_rules(path, @matching_rules[path], {'min' => min}) Pact::ArrayLike.new(recurse(array.first, "#{path}[*]"), min: min) else new_array = [] @@ -82,30 +82,46 @@ def wrap object, path elsif rules['regex'] handle_regex(object, path, rules) else - log_ignored_rules(path, rules, {}) + #log_ignored_rules(path, rules, {}) object end end def handle_match_type object, path, rules - log_ignored_rules(path, rules, {'match' => 'type'}) - Pact::SomethingLike.new(object) + rules.delete('match') + Pact::SomethingLike.new(recurse(object, path)) end def handle_regex object, path, rules - log_ignored_rules(path, rules, {'match' => 'regex', 'regex' => rules['regex']}) - Pact::Term.new(generate: object, matcher: Regexp.new(rules['regex'])) + rules.delete('match') + regex = rules.delete('regex') + Pact::Term.new(generate: object, matcher: Regexp.new(regex)) end - def log_ignored_rules path, rules, used_rules - dup_rules = rules.dup - used_rules.each_pair do | used_key, used_value | - dup_rules.delete(used_key) if dup_rules[used_key] == used_value + def log_ignored_rules + @matching_rules.each do | jsonpath, rules_hash | + rules_array = rules_hash["matchers"] + ((rules_array.length - 1)..0).each do | index | + rules_array.delete_at(index) if rules_array[index].empty? + end end - if dup_rules.any? - $stderr.puts "WARN: Ignoring unsupported matching rules #{dup_rules} for path #{path}" + + if @matching_rules.any? + @matching_rules.each do | path, rules_hash | + rules_hash.each do | key, value | + $stderr.puts "WARN: Ignoring unsupported #{key} #{value} for path #{path}" if value.any? + end + end end end + + def find_rule(path, key) + @matching_rules[path] && @matching_rules[path][key] + end + + def log_used_rule path, key, value + @used_rules << [path, key, value] + end end end end diff --git a/spec/integration/matching_rules_extract_and_merge_spec.rb b/spec/integration/matching_rules_extract_and_merge_spec.rb index c53acd6..849feb4 100644 --- a/spec/integration/matching_rules_extract_and_merge_spec.rb +++ b/spec/integration/matching_rules_extract_and_merge_spec.rb @@ -1,7 +1,9 @@ require 'pact/term' require 'pact/something_like' require 'pact/matching_rules/extract' +require 'pact/matching_rules/v3/extract' require 'pact/matching_rules/merge' +require 'pact/matching_rules/v3/merge' require 'pact/reification' describe "converting Pact::Term and Pact::SomethingLike to matching rules and back again" do @@ -10,6 +12,9 @@ let(:matching_rules) { Pact::MatchingRules::Extract.(expected) } let(:recreated_expected) { Pact::MatchingRules::Merge.(example, matching_rules)} + let(:recreated_expected_v3) { Pact::MatchingRules::V3::Merge.(example, matching_rules_v3) } + let(:matching_rules_v3) { Pact::MatchingRules::V3::Extract.(expected) } + context "with a Pact::Term" do let(:expected) do { @@ -21,9 +26,29 @@ } end - it "recreates the same object hierarchy" do + it "recreates the same object hierarchy with v2 matching" do + expect(recreated_expected).to eq expected + end + + it "recreates the same object hierarchy with v3 matching" do + expect(recreated_expected_v3).to eq expected + end + end + + context "with a Pact::SomethingLike containing a Pact::ArrayLike" do + let(:expected) do + { + body: Pact::SomethingLike.new(children: Pact::ArrayLike.new("foo", min: 2)) + } + end + + it "recreates the same object hierarchy with v2 matching" do expect(recreated_expected).to eq expected end + + it "recreates the same object hierarchy with v3 matching" do + expect(recreated_expected_v3).to eq expected + end end context "with a Pact::SomethingLike" do @@ -37,9 +62,13 @@ } end - it "recreates the same object hierarchy" do + it "recreates the same object hierarchy with v2 matching" do expect(recreated_expected).to eq expected end + + it "recreates the same object hierarchy with v3 matching" do + expect(recreated_expected_v3).to eq expected + end end context "with a Pact::SomethingLike containing a Hash" do @@ -61,9 +90,13 @@ } end - it "recreates the same object hierarchy" do + it "recreates the same object hierarchy with v2 matching" do expect(recreated_expected).to eq expected end + + it "recreates the same object hierarchy with v3 matching" do + expect(recreated_expected_v3).to eq expected + end end context "with a Pact::SomethingLike containing an Array" do @@ -83,8 +116,12 @@ } end - it "recreates the same object hierarchy" do + it "recreates the same object hierarchy with v2 matching" do expect(recreated_expected).to eq expected end + + it "recreates the same object hierarchy with v3 matching" do + expect(recreated_expected_v3).to eq expected + end end end diff --git a/spec/lib/pact/matching_rules/v3/merge_spec.rb b/spec/lib/pact/matching_rules/v3/merge_spec.rb index 728abae..f1d687a 100644 --- a/spec/lib/pact/matching_rules/v3/merge_spec.rb +++ b/spec/lib/pact/matching_rules/v3/merge_spec.rb @@ -7,10 +7,16 @@ module V3 subject { Merge.(expected, matching_rules) } before do - allow($stderr).to receive(:puts) + allow($stderr).to receive(:puts) do | message | + raise "Was not expecting stderr to receive #{message.inspect} in this spec. This may be because of a missed rule deletion in Merge." + end end describe "no recognised rules" do + before do + allow($stderr).to receive(:puts) + end + let(:expected) do { "_links" => { @@ -65,6 +71,10 @@ module V3 end describe "type based matching" do + before do + allow($stderr).to receive(:puts) + end + let(:expected) do { "name" => "Mary" @@ -88,11 +98,19 @@ module V3 subject end + it "does not alter the passed in rules hash" do + original_matching_rules = JSON.parse(matching_rules.to_json) + subject + expect(matching_rules).to eq original_matching_rules + end end describe "regular expressions" do - describe "in a hash" do + before do + allow($stderr).to receive(:puts) + end + let(:expected) do { "_links" => { @@ -275,6 +293,10 @@ module V3 end describe "with an example array with more than one item" do + before do + allow($stderr).to receive(:puts) + end + let(:expected) do { @@ -292,7 +314,7 @@ module V3 } end - xit "doesn't warn about the min size being ignored" do + it "doesn't warn about the min size being ignored" do expect(Pact.configuration.error_stream).to receive(:puts).once subject end