From b9deb09d857bcaacb0e8d97c00701e24aa35e2c3 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Sat, 11 Jan 2020 19:14:15 +1100 Subject: [PATCH] fix: use configured credentials when fetching the diff with previous version Closes: https://github.com/pact-foundation/pact-ruby/issues/205 --- lib/pact/hal/http_client.rb | 7 +- lib/pact/hal/link.rb | 24 ++-- lib/pact/hal/non_json_entity.rb | 28 ++++ lib/pact/provider/help/content.rb | 8 +- lib/pact/provider/help/pact_diff.rb | 42 ++---- lib/pact/provider/help/write.rb | 13 +- lib/pact/provider/pact_source.rb | 9 ++ lib/pact/provider/pact_spec_runner.rb | 2 +- spec/integration/publish_verification_spec.rb | 2 +- spec/lib/pact/hal/entity_spec.rb | 2 +- spec/lib/pact/hal/http_client_spec.rb | 7 +- spec/lib/pact/hal/link_spec.rb | 2 +- spec/lib/pact/pact_broker/fetch_pacts_spec.rb | 2 +- spec/lib/pact/provider/help/content_spec.rb | 17 +-- spec/lib/pact/provider/help/pact_diff_spec.rb | 128 ------------------ spec/lib/pact/provider/help/write_spec.rb | 8 +- .../verification_results/publish_spec.rb | 4 +- .../pact_ruby_fetch_pacts_test.rb | 30 ++++ 18 files changed, 127 insertions(+), 208 deletions(-) create mode 100644 lib/pact/hal/non_json_entity.rb delete mode 100644 spec/lib/pact/provider/help/pact_diff_spec.rb diff --git a/lib/pact/hal/http_client.rb b/lib/pact/hal/http_client.rb index d32891d0..3225df3d 100644 --- a/lib/pact/hal/http_client.rb +++ b/lib/pact/hal/http_client.rb @@ -36,12 +36,9 @@ def post href, body = nil, headers = {} def create_request uri, http_method, body = nil, headers = {} request = Net::HTTP.const_get(http_method).new(uri.request_uri) - request['Content-Type'] = "application/json" if ['Post', 'Put', 'Patch'].include?(http_method) - request['Accept'] = "application/hal+json" headers.each do | key, value | request[key] = value end - request.body = body if body request.basic_auth username, password if username request['Authorization'] = "Bearer #{token}" if token @@ -85,6 +82,10 @@ def status def success? __getobj__().code.start_with?("2") end + + def json? + self['content-type'] && self['content-type'] =~ /json/ + end end end end diff --git a/lib/pact/hal/link.rb b/lib/pact/hal/link.rb index b9abd219..d248b432 100644 --- a/lib/pact/hal/link.rb +++ b/lib/pact/hal/link.rb @@ -23,14 +23,14 @@ def initialize(attrs, http_client) end def run(payload = nil) - response = case request_method - when :get - get(payload) - when :put - put(payload) - when :post - post(payload) - end + case request_method + when :get + get(payload) + when :put + put(payload) + when :post + post(payload) + end end def title_or_name @@ -89,8 +89,14 @@ def with_query(query) def wrap_response(href, http_response) require 'pact/hal/entity' # avoid circular reference + require 'pact/hal/non_json_entity' + if http_response.success? - Entity.new(href, http_response.body, @http_client, http_response) + if http_response.json? + Entity.new(href, http_response.body, @http_client, http_response) + else + NonJsonEntity.new(href, http_response.raw_body, @http_client, http_response) + end else ErrorEntity.new(href, http_response.raw_body, @http_client, http_response) end diff --git a/lib/pact/hal/non_json_entity.rb b/lib/pact/hal/non_json_entity.rb new file mode 100644 index 00000000..83c96da7 --- /dev/null +++ b/lib/pact/hal/non_json_entity.rb @@ -0,0 +1,28 @@ +module Pact + module Hal + class NonJsonEntity + def initialize(href, body, http_client, response = nil) + @href = href + @body = body + @client = http_client + @response = response + end + + def success? + true + end + + def response + @response + end + + def body + @body + end + + def assert_success! + self + end + end + end +end diff --git a/lib/pact/provider/help/content.rb b/lib/pact/provider/help/content.rb index 26452ea2..631ef28f 100644 --- a/lib/pact/provider/help/content.rb +++ b/lib/pact/provider/help/content.rb @@ -5,8 +5,8 @@ module Provider module Help class Content - def initialize pact_jsons - @pact_jsons = pact_jsons + def initialize pact_sources + @pact_sources = pact_sources end def text @@ -15,7 +15,7 @@ def text private - attr_reader :pact_jsons + attr_reader :pact_sources def help_text temp_dir = Pact.configuration.tmp_dir @@ -28,7 +28,7 @@ def template_string end def pact_diffs - pact_jsons.collect do | pact_json | + pact_sources.collect do | pact_json | PactDiff.call(pact_json) end.compact.join("\n") end diff --git a/lib/pact/provider/help/pact_diff.rb b/lib/pact/provider/help/pact_diff.rb index 97769d63..f9677890 100644 --- a/lib/pact/provider/help/pact_diff.rb +++ b/lib/pact/provider/help/pact_diff.rb @@ -1,24 +1,24 @@ +require 'pact/hal/entity' + module Pact module Provider module Help class PactDiff class PrintPactDiffError < StandardError; end - attr_reader :pact_json, :output + attr_reader :pact_source, :output - def initialize pact_json - @pact_json = pact_json + def initialize pact_source + @pact_source = pact_source end - def self.call pact_json - new(pact_json).call + def self.call pact_source + new(pact_source).call end def call begin - if diff_rel && diff_url - header + "\n" + get_diff - end + header + "\n" + get_diff rescue PrintPactDiffError => e return e.message end @@ -30,35 +30,13 @@ def header "The following changes have been made since the previous distinct version of this pact, and may be responsible for verification failure:\n" end - def pact_hash - @pact_hash ||= json_load(pact_json) - end - - def links - pact_hash['_links'] || pact_hash['links'] - end - - def diff_rel - return nil unless links - key = links.keys.find { | key | key =~ /diff/ && key =~ /distinct/ && key =~ /previous/} - key ? links[key] : nil - end - - def diff_url - diff_rel['href'] - end - def get_diff begin - URI.open(diff_url) { | file | file.read } + pact_source.hal_entity._link!("pb:diff-previous-distinct").get!(nil, "Accept" => "text/plain").body rescue StandardError => e - raise PrintPactDiffError.new("Tried to retrieve diff with previous pact from #{diff_url}, but received response code #{e}.") + raise PrintPactDiffError.new("Tried to retrieve diff with previous pact, but received error #{e.class} #{e.message}.") end end - - def json_load json - JSON.load(json, nil, { max_nesting: 50 }) - end end end end diff --git a/lib/pact/provider/help/write.rb b/lib/pact/provider/help/write.rb index 29e06a95..e32575ab 100644 --- a/lib/pact/provider/help/write.rb +++ b/lib/pact/provider/help/write.rb @@ -9,12 +9,12 @@ class Write HELP_FILE_NAME = 'help.md' - def self.call pact_jsons, reports_dir = Pact.configuration.reports_dir - new(pact_jsons, reports_dir).call + def self.call pact_sources, reports_dir = Pact.configuration.reports_dir + new(pact_sources, reports_dir).call end - def initialize pact_jsons, reports_dir - @pact_jsons = pact_jsons + def initialize pact_sources, reports_dir + @pact_sources = pact_sources @reports_dir = File.expand_path(reports_dir) end @@ -25,7 +25,7 @@ def call private - attr_reader :reports_dir, :pact_jsons + attr_reader :reports_dir, :pact_sources def clean_reports_dir raise "Cleaning report dir #{reports_dir} would delete project!" if reports_dir_contains_pwd @@ -46,9 +46,8 @@ def help_path end def help_text - Content.new(pact_jsons).text + Content.new(pact_sources).text end - end end end diff --git a/lib/pact/provider/pact_source.rb b/lib/pact/provider/pact_source.rb index 44b3cb33..c1c009f1 100644 --- a/lib/pact/provider/pact_source.rb +++ b/lib/pact/provider/pact_source.rb @@ -1,4 +1,6 @@ require 'pact/consumer_contract/pact_file' +require 'pact/hal/http_client' +require 'pact/hal/entity' module Pact module Provider @@ -17,6 +19,13 @@ def pact_json def pact_hash @pact_hash ||= JSON.load(pact_json, nil, { max_nesting: 50 }) end + + def hal_entity + http_client_keys = [:username, :password, :token] + http_client_options = uri.options.reject{ |k, _| !http_client_keys.include?(k) } + http_client = Pact::Hal::HttpClient.new(http_client_options.merge(verbose: true)) + Pact::Hal::Entity.new(uri, pact_hash, http_client) + end end end end diff --git a/lib/pact/provider/pact_spec_runner.rb b/lib/pact/provider/pact_spec_runner.rb index 5e4c9b68..a9ad378d 100644 --- a/lib/pact/provider/pact_spec_runner.rb +++ b/lib/pact/provider/pact_spec_runner.rb @@ -80,7 +80,7 @@ def configure_rspec executing_with_ruby = executing_with_ruby? config.after(:suite) do | suite | - Pact::Provider::Help::Write.call(jsons) if executing_with_ruby + Pact::Provider::Help::Write.call(Pact.provider_world.pact_sources) if executing_with_ruby end end diff --git a/spec/integration/publish_verification_spec.rb b/spec/integration/publish_verification_spec.rb index 983ffce4..b5a467c3 100644 --- a/spec/integration/publish_verification_spec.rb +++ b/spec/integration/publish_verification_spec.rb @@ -59,7 +59,7 @@ subject { Pact::Provider::VerificationResults::PublishAll.call(pact_sources, test_results_hash) } let!(:request) do - stub_request(:post, 'http://publish').to_return(status: 200, body: created_verification_body) + stub_request(:post, 'http://publish').to_return(status: 200, headers: {'Content-Type' => 'application/hal+json'}, body: created_verification_body) end it "publishes the results" do diff --git a/spec/lib/pact/hal/entity_spec.rb b/spec/lib/pact/hal/entity_spec.rb index c6b809a0..9663565f 100644 --- a/spec/lib/pact/hal/entity_spec.rb +++ b/spec/lib/pact/hal/entity_spec.rb @@ -9,7 +9,7 @@ module Hal end let(:provider_response) do - double('response', body: provider_hash, success?: true) + double('response', body: provider_hash, success?: true, json?: true) end let(:provider_hash) do diff --git a/spec/lib/pact/hal/http_client_spec.rb b/spec/lib/pact/hal/http_client_spec.rb index e8811501..f79f03cb 100644 --- a/spec/lib/pact/hal/http_client_spec.rb +++ b/spec/lib/pact/hal/http_client_spec.rb @@ -13,7 +13,7 @@ module Hal let!(:request) do stub_request(:get, "http://example.org/"). with( headers: { - 'Accept'=>'application/hal+json', + 'Accept'=>'*/*', 'Authorization'=>'Basic Zm9vOmJhcg==' }). to_return(status: 200, body: response_body, headers: {'Content-Type' => 'application/json'}) @@ -86,9 +86,8 @@ module Hal let!(:request) do stub_request(:post, "http://example.org/"). with( headers: { - 'Accept'=>'application/hal+json', - 'Authorization'=>'Basic Zm9vOmJhcg==', - 'Content-Type'=>'application/json' + 'Accept'=>'*/*', + 'Authorization'=>'Basic Zm9vOmJhcg==' }, body: request_body). to_return(status: 200, body: response_body, headers: {'Content-Type' => 'application/json'}) diff --git a/spec/lib/pact/hal/link_spec.rb b/spec/lib/pact/hal/link_spec.rb index 8e83d532..441153f1 100644 --- a/spec/lib/pact/hal/link_spec.rb +++ b/spec/lib/pact/hal/link_spec.rb @@ -10,7 +10,7 @@ module Hal end let(:response) do - instance_double('Pact::Hal::HttpClient::Response', success?: success, body: response_body, raw_body: response_body.to_json) + instance_double('Pact::Hal::HttpClient::Response', success?: success, body: response_body, raw_body: response_body.to_json, json?: true) end let(:success) { true } diff --git a/spec/lib/pact/pact_broker/fetch_pacts_spec.rb b/spec/lib/pact/pact_broker/fetch_pacts_spec.rb index 096960d0..50d8fc31 100644 --- a/spec/lib/pact/pact_broker/fetch_pacts_spec.rb +++ b/spec/lib/pact/pact_broker/fetch_pacts_spec.rb @@ -33,7 +33,7 @@ module PactBroker context "when there is a HAL relation missing" do before do - stub_request(:get, "http://broker.org/").to_return(status: 200, body: {"_links" => {} }.to_json, headers: {}) + stub_request(:get, "http://broker.org/").to_return(status: 200, body: {"_links" => {} }.to_json, headers: {"Content-Type" => "application/hal+json"}) end it "raises a Pact::Error" do diff --git a/spec/lib/pact/provider/help/content_spec.rb b/spec/lib/pact/provider/help/content_spec.rb index 1aaf17af..928623ba 100644 --- a/spec/lib/pact/provider/help/content_spec.rb +++ b/spec/lib/pact/provider/help/content_spec.rb @@ -4,19 +4,17 @@ module Pact module Provider module Help describe Content do - describe "#text" do - - let(:pact_1_json) { { some: 'json'}.to_json } - let(:pact_2_json) { { some: 'other json'}.to_json } - let(:pact_jsons) { [pact_1_json, pact_2_json] } - before do - allow(PactDiff).to receive(:call).with(pact_1_json).and_return('diff 1') - allow(PactDiff).to receive(:call).with(pact_2_json).and_return(nil) + allow(PactDiff).to receive(:call).with(pact_source_1).and_return('diff 1') + allow(PactDiff).to receive(:call).with(pact_source_2).and_return(nil) end - subject { Content.new(pact_jsons) } + let(:pact_source_1) { { some: 'json'}.to_json } + let(:pact_source_2) { { some: 'other json'}.to_json } + let(:pact_sources) { [pact_source_1, pact_source_2] } + + subject { Content.new(pact_sources) } it "displays the log path" do expect(subject.text).to include Pact.configuration.log_path @@ -29,7 +27,6 @@ module Help it "displays the diff" do expect(subject.text).to include 'diff 1' end - end end end diff --git a/spec/lib/pact/provider/help/pact_diff_spec.rb b/spec/lib/pact/provider/help/pact_diff_spec.rb deleted file mode 100644 index e32a8fa2..00000000 --- a/spec/lib/pact/provider/help/pact_diff_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -require 'pact/provider/help/pact_diff' - -module Pact - module Provider - module Help - - describe PactDiff do - - let(:href) { 'http://pact-broker/diff' } - let(:pact_json) do - { - '_links' => { - 'pb:diff-previous-distinct' => { - 'href' => href - } - } - }.to_json - end - - let(:output) { subject } - let(:diff) { {some: 'diff'}.to_json } - let(:diff_stream) { double('diff stream', read: diff) } - - describe ".call" do - - before do - stub_request(:get, "http://pact-broker/diff") - .to_return(status: 200, body: diff, headers: {}) - end - - subject { PactDiff.(pact_json) } - - context "when there are no links" do - let(:pact_json) { {}.to_json } - - it "returns nothing" do - subject - expect(output).to be_nil - end - end - - context "when the diff rel is not found" do - let(:pact_json) do - { - '_links' => {} - }.to_json - end - - it "returns nothing" do - subject - expect(output).to be_nil - end - end - - context "when the diff rel does not have a href" do - let(:pact_json) do - { - '_links' => { - 'pb:diff-previous-distinct' => {} - } - }.to_json - end - - it "returns nothing" do - subject - expect(output).to be_nil - end - end - - context "when the diff rel changes because Beth can't make up her mind" do - let(:pact_json) do - { - '_links' => { - 'distinct-diff-previous' => { - 'href' => href - } - } - }.to_json - end - - it "returns something" do - subject - expect(output).to_not be_nil - end - end - - context "when the diff resource exists" do - - it "returns a message" do - subject - expect(output).to include("The following changes") - end - - it "returns the diff" do - subject - expect(output).to include('some') - expect(output).to include('diff') - end - end - - context "when the diff resource doesn't exist" do - before do - stub_request(:get, "http://pact-broker/diff") - .to_return(status: 404) - end - - it "returns a warning" do - subject - expect(output).to include "Tried to retrieve diff with previous pact from #{href}, but received response code 404" - end - end - - context "when a redirect is received" do - before do - stub_request(:get, "http://pact-broker/diff") - .to_return(status: 301, body: diff, headers: {}) - end - - xit "follows the redirect" do - subject - expect(output).to_not include "301" - end - end - end - end - end - end -end diff --git a/spec/lib/pact/provider/help/write_spec.rb b/spec/lib/pact/provider/help/write_spec.rb index 9148c704..4a11a7b6 100644 --- a/spec/lib/pact/provider/help/write_spec.rb +++ b/spec/lib/pact/provider/help/write_spec.rb @@ -7,7 +7,7 @@ module Help describe "#call" do - let(:pact_jsons) { double('pact jsons') } + let(:pact_sources) { double('pact jsons') } let(:reports_dir) { "./tmp/reports" } let(:text) { "help text" } @@ -16,12 +16,12 @@ module Help allow_any_instance_of(Content).to receive(:text).and_return(text) end - subject { Write.call(pact_jsons, reports_dir) } + subject { Write.call(pact_sources, reports_dir) } let(:actual_contents) { File.read(File.join(reports_dir, Write::HELP_FILE_NAME)) } - it "passes the pact_jsons into the Content" do - expect(Content).to receive(:new).with(pact_jsons).and_return(double(text: '')) + it "passes the pact_sources into the Content" do + expect(Content).to receive(:new).with(pact_sources).and_return(double(text: '')) subject end diff --git a/spec/lib/pact/provider/verification_results/publish_spec.rb b/spec/lib/pact/provider/verification_results/publish_spec.rb index 6ad3b051..72cca405 100644 --- a/spec/lib/pact/provider/verification_results/publish_spec.rb +++ b/spec/lib/pact/provider/verification_results/publish_spec.rb @@ -82,7 +82,7 @@ module VerificationResults allow($stdout).to receive(:puts) allow($stderr).to receive(:puts) allow(Pact.configuration).to receive(:provider).and_return(provider_configuration) - stub_request(:post, stubbed_publish_verification_url).to_return(status: 200, body: created_verification_body) + stub_request(:post, stubbed_publish_verification_url).to_return(status: 200, headers: { 'Content-Type' => 'application/hal+json'}, body: created_verification_body) stub_request(:put, 'http://provider/version/1.2.3/tag/foo').to_return(status: 200, headers: { 'Content-Type' => 'application/hal+json'}, body: tag_body) stub_request(:get, provider_url).to_return(status: 200, headers: { 'Content-Type' => 'application/hal+json'}, body: provider_body) allow(Retry).to receive(:until_true) { |&block| block.call } @@ -186,7 +186,7 @@ module VerificationResults context "with https" do before do - stub_request(:post, publish_verification_url).to_return(status: 200, body: created_verification_body) + stub_request(:post, publish_verification_url).to_return(status: 200, headers: { 'Content-Type' => 'application/json' }, body: created_verification_body) end let(:publish_verification_url) { stubbed_publish_verification_url.gsub('http', 'https') } diff --git a/spec/service_providers/pact_ruby_fetch_pacts_test.rb b/spec/service_providers/pact_ruby_fetch_pacts_test.rb index 65305310..9379eecb 100644 --- a/spec/service_providers/pact_ruby_fetch_pacts_test.rb +++ b/spec/service_providers/pact_ruby_fetch_pacts_test.rb @@ -26,6 +26,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:latest-provider-pacts' => { @@ -65,6 +68,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -105,6 +111,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -128,6 +137,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -171,6 +183,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [] @@ -187,6 +202,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -227,6 +245,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [] @@ -256,6 +277,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -280,6 +304,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [ @@ -323,6 +350,9 @@ ) .will_respond_with( status: 200, + headers: { + 'Content-Type' => Pact.term('application/hal+json', /json/) + }, body: { _links: { 'pb:pacts' => [