From a909fe71842b874305a3ca5aa2767a4986bcd4c1 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Tue, 19 Dec 2017 15:17:06 -0800 Subject: allow injecting tempfiles into Chef::HTTP use this to inject the tempfile from Chef::FileContentManagement::Tempfile closes #2401 Signed-off-by: Lamont Granquist --- lib/chef/http.rb | 20 +++++++------- lib/chef/provider/remote_file/http.rb | 5 ++-- spec/unit/http_spec.rb | 9 +++++++ spec/unit/provider/remote_file/http_spec.rb | 42 ++++++++++++++++------------- 4 files changed, 45 insertions(+), 31 deletions(-) diff --git a/lib/chef/http.rb b/lib/chef/http.rb index 14dd8b93a5..c9df4e1235 100644 --- a/lib/chef/http.rb +++ b/lib/chef/http.rb @@ -170,11 +170,10 @@ class Chef raise end - def streaming_request_with_progress(path, headers = {}, &progress_block) + def streaming_request_with_progress(path, headers = {}, tempfile = nil, &progress_block) http_attempts ||= 0 url = create_url(path) response, rest_request, return_value = nil, nil, nil - tempfile = nil data = nil method = :GET @@ -182,7 +181,7 @@ class Chef response, rest_request, return_value = send_http_request(method, url, processed_headers, data) do |http_response| if http_response.kind_of?(Net::HTTPSuccess) - tempfile = stream_to_tempfile(url, http_response, &progress_block) + tempfile = stream_to_tempfile(url, http_response, tempfile, &progress_block) end apply_stream_complete_middleware(http_response, rest_request, return_value) end @@ -217,11 +216,10 @@ class Chef # # @yield [tempfile] block to process the tempfile # @yieldparams [tempfile] tempfile - def streaming_request(path, headers = {}) + def streaming_request(path, headers = {}, tempfile = nil) http_attempts ||= 0 url = create_url(path) response, rest_request, return_value = nil, nil, nil - tempfile = nil data = nil method = :GET @@ -229,7 +227,7 @@ class Chef response, rest_request, return_value = send_http_request(method, url, processed_headers, data) do |http_response| if http_response.kind_of?(Net::HTTPSuccess) - tempfile = stream_to_tempfile(url, http_response) + tempfile = stream_to_tempfile(url, http_response, tempfile) end apply_stream_complete_middleware(http_response, rest_request, return_value) end @@ -500,11 +498,13 @@ class Chef end # @api private - def stream_to_tempfile(url, response, &progress_block) + def stream_to_tempfile(url, response, tf = nil, &progress_block) content_length = response["Content-Length"] - tf = Tempfile.open("chef-rest") - if Chef::Platform.windows? - tf.binmode # required for binary files on Windows platforms + if tf.nil? + tf = Tempfile.open("chef-rest") + if Chef::Platform.windows? + tf.binmode # required for binary files on Windows platforms + end end Chef::Log.debug("Streaming download from #{url} to tempfile #{tf.path}") # Stolen from http://www.ruby-forum.com/topic/166423 diff --git a/lib/chef/provider/remote_file/http.rb b/lib/chef/provider/remote_file/http.rb index ad044f9e3c..4732253e5b 100644 --- a/lib/chef/provider/remote_file/http.rb +++ b/lib/chef/provider/remote_file/http.rb @@ -61,12 +61,13 @@ class Chef def fetch http = Chef::HTTP::Simple.new(uri, http_client_opts) + tempfile = Chef::FileContentManagement::Tempfile.new(@new_resource).tempfile if want_progress? - tempfile = http.streaming_request_with_progress(uri, headers) do |size, total| + tempfile = http.streaming_request_with_progress(uri, headers, tempfile) do |size, total| events.resource_update_progress(new_resource, size, total, progress_interval) end else - tempfile = http.streaming_request(uri, headers) + tempfile = http.streaming_request(uri, headers, tempfile) end if tempfile update_cache_control_data(tempfile, http.last_response) diff --git a/spec/unit/http_spec.rb b/spec/unit/http_spec.rb index d58f07c417..bf873b8535 100644 --- a/spec/unit/http_spec.rb +++ b/spec/unit/http_spec.rb @@ -93,6 +93,15 @@ describe Chef::HTTP do expect { http.send(:stream_to_tempfile, uri, resp) }.to raise_error("TestError") end + it "accepts a tempfile" do + resp = Net::HTTPOK.new("1.1", 200, "OK") + http = Chef::HTTP.new(uri) + tempfile = Tempfile.open("tempy-mctempfile") + expect(Tempfile).not_to receive(:open) + expect(resp).to receive(:read_body).and_yield("conty-mccontent") + http.send(:stream_to_tempfile, uri, resp, tempfile) + expect(IO.read(tempfile.path)).to eql("conty-mccontent") + end end describe "head" do diff --git a/spec/unit/provider/remote_file/http_spec.rb b/spec/unit/provider/remote_file/http_spec.rb index f58a3d3c14..ec9c0cf2e3 100644 --- a/spec/unit/provider/remote_file/http_spec.rb +++ b/spec/unit/provider/remote_file/http_spec.rb @@ -157,9 +157,9 @@ describe Chef::Provider::RemoteFile::HTTP do let(:expected_http_opts) { {} } let(:expected_http_args) { [uri, expected_http_opts] } - let(:tempfile_path) { "/tmp/chef-mock-tempfile-abc123" } + let(:tempfile_path) { tempfile.path } - let(:tempfile) { double(Tempfile, :path => tempfile_path, :close => nil) } + let(:tempfile) { Tempfile.open("muhtempfile") } let(:last_response) { {} } @@ -171,7 +171,7 @@ describe Chef::Provider::RemoteFile::HTTP do let(:rest) do rest = double(Chef::HTTP::Simple) - allow(rest).to receive(:streaming_request).and_return(tempfile) + allow_any_instance_of(Chef::FileContentManagement::Tempfile).to receive(:tempfile).and_return(tempfile) allow(rest).to receive(:last_response).and_return(last_response) rest end @@ -189,17 +189,35 @@ describe Chef::Provider::RemoteFile::HTTP do it "should return a nil tempfile for a 304 HTTPNotModifed" do # Streaming request returns nil for 304 errors - allow(rest).to receive(:streaming_request).and_return(nil) + expect(rest).to receive(:streaming_request).with(uri, {}, tempfile).and_return(nil) expect(fetcher.fetch).to be_nil end end - describe "and the request returns new content" do + context "with progress reports" do + let(:fetched_content_checksum) { "e2a8938cc31754f6c067b35aab1d0d4864272e9bf8504536ef3e79ebf8432305" } + + before do + expect(cache_control_data).to receive(:save) + expect(Chef::Digester).to receive(:checksum_for_file).with(tempfile_path).and_return(fetched_content_checksum) + Chef::Config[:show_download_progress] = true + end + + it "should yield its progress" do + expect(rest).to receive(:streaming_request_with_progress).with(uri, {}, tempfile).and_yield(50, 100).and_yield(70, 100).and_return(tempfile) + expect(event_dispatcher).to receive(:formatter?).and_return(true) + expect(event_dispatcher).to receive(:resource_update_progress).with(new_resource, 50, 100, 10).ordered + expect(event_dispatcher).to receive(:resource_update_progress).with(new_resource, 70, 100, 10).ordered + fetcher.fetch + end + end + describe "and the request returns new content" do let(:fetched_content_checksum) { "e2a8938cc31754f6c067b35aab1d0d4864272e9bf8504536ef3e79ebf8432305" } before do + expect(rest).to receive(:streaming_request).with(uri, {}, tempfile).and_return(tempfile) expect(cache_control_data).to receive(:save) expect(Chef::Digester).to receive(:checksum_for_file).with(tempfile_path).and_return(fetched_content_checksum) end @@ -212,20 +230,6 @@ describe Chef::Provider::RemoteFile::HTTP do expect(cache_control_data.checksum).to eq(fetched_content_checksum) end - context "with progress reports" do - before do - Chef::Config[:show_download_progress] = true - end - - it "should yield its progress" do - allow(rest).to receive(:streaming_request_with_progress).and_yield(50, 100).and_yield(70, 100).and_return(tempfile) - expect(event_dispatcher).to receive(:formatter?).and_return(true) - expect(event_dispatcher).to receive(:resource_update_progress).with(new_resource, 50, 100, 10).ordered - expect(event_dispatcher).to receive(:resource_update_progress).with(new_resource, 70, 100, 10).ordered - fetcher.fetch - end - end - context "and the response does not contain an etag" do let(:last_response) { { "etag" => nil } } it "does not include an etag in the result" do -- cgit v1.2.1