Merge pull request #12682 from chef/lcg/modernize-http-resource

2 files changed, 71 insertions, 81 deletions

diff --git a/lib/chef/provider/http_request.rb b/lib/chef/provider/http_request.rb
index 8e7a7f1fc9..0b25c728cf 100644
--- a/lib/chef/provider/http_request.rb
+++ b/lib/chef/provider/http_request.rb
@@ -25,18 +25,20 @@ class Chef
 
       provides :http_request
 
-      attr_accessor :http
+      attr_writer :http
 
-      def load_current_resource
-        @http = Chef::HTTP::Simple.new(new_resource.url)
+      def http
+        @http ||= Chef::HTTP::Simple.new(new_resource.url)
       end
 
+      def load_current_resource; end
+
       # Send a HEAD request to new_resource.url
       action :head do
         message = check_message(new_resource.message)
         # CHEF-4762: we expect a nil return value from Chef::HTTP for a "200 Success" response
         # and false for a "304 Not Modified" response
-        modified = @http.head(
+        modified = http.head(
           (new_resource.url).to_s,
           new_resource.headers
         )
@@ -53,7 +55,7 @@ class Chef
         converge_by("#{new_resource} GET to #{new_resource.url}") do
 
           message = check_message(new_resource.message)
-          body = @http.get(
+          body = http.get(
             (new_resource.url).to_s,
             new_resource.headers
           )
@@ -66,7 +68,7 @@ class Chef
       action :patch do
         converge_by("#{new_resource} PATCH to #{new_resource.url}") do
           message = check_message(new_resource.message)
-          body = @http.patch(
+          body = http.patch(
             (new_resource.url).to_s,
             message,
             new_resource.headers
@@ -80,7 +82,7 @@ class Chef
       action :put do
         converge_by("#{new_resource} PUT to #{new_resource.url}") do
           message = check_message(new_resource.message)
-          body = @http.put(
+          body = http.put(
             (new_resource.url).to_s,
             message,
             new_resource.headers
@@ -94,7 +96,7 @@ class Chef
       action :post do
         converge_by("#{new_resource} POST to #{new_resource.url}") do
           message = check_message(new_resource.message)
-          body = @http.post(
+          body = http.post(
             (new_resource.url).to_s,
             message,
             new_resource.headers
@@ -107,7 +109,7 @@ class Chef
       # Send a DELETE request to new_resource.url
       action :delete do
         converge_by("#{new_resource} DELETE to #{new_resource.url}") do
-          body = @http.delete(
+          body = http.delete(
             (new_resource.url).to_s,
             new_resource.headers
           )
diff --git a/spec/unit/provider/http_request_spec.rb b/spec/unit/provider/http_request_spec.rb
index b30ed2dd30..595b1f4295 100644
--- a/spec/unit/provider/http_request_spec.rb
+++ b/spec/unit/provider/http_request_spec.rb
@@ -19,87 +19,75 @@
 require "spec_helper"
 
 describe Chef::Provider::HttpRequest do
-  before(:each) do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-
-    @new_resource = Chef::Resource::HttpRequest.new("adam")
-    @new_resource.name "adam"
-    @new_resource.url "http://www.opscode.com/"
-    @new_resource.message "is cool"
-
-    @provider = Chef::Provider::HttpRequest.new(@new_resource, @run_context)
-  end
-
-  describe "load_current_resource" do
-
-    it "should set up a Chef::ServerAPI client, with no authentication" do
-      expect(Chef::HTTP::Simple).to receive(:new).with(@new_resource.url)
-      @provider.load_current_resource
+  let(:node) { Chef::Node.new }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:new_resource) do
+    Chef::Resource::HttpRequest.new("adam").tap do |new_resource|
+      new_resource.name "adam"
+      new_resource.url "http://www.opscode.com/"
+      new_resource.message "is cool"
     end
   end
 
+  let(:provider) { Chef::Provider::HttpRequest.new(new_resource, run_context) }
+
   describe "when making REST calls" do
-    before(:each) do
-      # run_action(x) forces load_current_resource to run;
-      # that would overwrite our supplied mock Chef::Rest # object
-      allow(@provider).to receive(:load_current_resource).and_return(true)
-      @http = double("Chef::ServerAPI")
-      @provider.http = @http
+    let(:http) do
+      provider.http = double("Chef::ServerAPI")
     end
 
     describe "action_get" do
 
       it "should inflate a message block at runtime" do
-        @new_resource.message { "return" }
-        expect(@http).to receive(:get).with("http://www.opscode.com/", {})
-        @provider.run_action(:get)
-        expect(@new_resource).to be_updated
+        new_resource.message { "return" }
+        expect(http).to receive(:get).with("http://www.opscode.com/", {})
+        provider.run_action(:get)
+        expect(new_resource).to be_updated
       end
 
       it "should run a GET request" do
-        expect(@http).to receive(:get).with("http://www.opscode.com/", {})
-        @provider.run_action(:get)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:get).with("http://www.opscode.com/", {})
+        provider.run_action(:get)
+        expect(new_resource).to be_updated
       end
     end
 
     describe "action_put" do
       it "should run a PUT request with the message as the payload" do
-        expect(@http).to receive(:put).with("http://www.opscode.com/", @new_resource.message, {})
-        @provider.run_action(:put)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:put).with("http://www.opscode.com/", new_resource.message, {})
+        provider.run_action(:put)
+        expect(new_resource).to be_updated
       end
 
       it "should inflate a message block at runtime" do
-        @new_resource.message(lambda { "return" })
-        expect(@http).to receive(:put).with("http://www.opscode.com/", "return", {})
-        @provider.run_action(:put)
-        expect(@new_resource).to be_updated
+        new_resource.message(lambda { "return" })
+        expect(http).to receive(:put).with("http://www.opscode.com/", "return", {})
+        provider.run_action(:put)
+        expect(new_resource).to be_updated
       end
     end
 
     describe "action_post" do
       it "should run a PUT request with the message as the payload" do
-        expect(@http).to receive(:post).with("http://www.opscode.com/", @new_resource.message, {})
-        @provider.run_action(:post)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:post).with("http://www.opscode.com/", new_resource.message, {})
+        provider.run_action(:post)
+        expect(new_resource).to be_updated
       end
 
       it "should inflate a message block at runtime" do
-        @new_resource.message { "return" }
-        expect(@http).to receive(:post).with("http://www.opscode.com/", "return", {})
-        @provider.run_action(:post)
-        expect(@new_resource).to be_updated
+        new_resource.message { "return" }
+        expect(http).to receive(:post).with("http://www.opscode.com/", "return", {})
+        provider.run_action(:post)
+        expect(new_resource).to be_updated
       end
     end
 
     describe "action_delete" do
       it "should run a DELETE request" do
-        expect(@http).to receive(:delete).with("http://www.opscode.com/", {})
-        @provider.run_action(:delete)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:delete).with("http://www.opscode.com/", {})
+        provider.run_action(:delete)
+        expect(new_resource).to be_updated
       end
     end
 
@@ -107,52 +95,52 @@ describe Chef::Provider::HttpRequest do
     # and false for a "304 Not Modified" response
     describe "action_head" do
       before do
-        @provider.http = @http
+        provider.http = http
       end
 
       it "should inflate a message block at runtime" do
-        @new_resource.message { "return" }
-        expect(@http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
-        @provider.run_action(:head)
-        expect(@new_resource).to be_updated
+        new_resource.message { "return" }
+        expect(http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
+        provider.run_action(:head)
+        expect(new_resource).to be_updated
       end
 
       it "should run a HEAD request" do
-        expect(@http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
-        @provider.run_action(:head)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
+        provider.run_action(:head)
+        expect(new_resource).to be_updated
       end
 
       it "should update a HEAD request with empty string response body (CHEF-4762)" do
-        expect(@http).to receive(:head).with("http://www.opscode.com/", {}).and_return("")
-        @provider.run_action(:head)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:head).with("http://www.opscode.com/", {}).and_return("")
+        provider.run_action(:head)
+        expect(new_resource).to be_updated
       end
 
       it "should update a HEAD request with nil response body (CHEF-4762)" do
-        expect(@http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
-        @provider.run_action(:head)
-        expect(@new_resource).to be_updated
+        expect(http).to receive(:head).with("http://www.opscode.com/", {}).and_return(nil)
+        provider.run_action(:head)
+        expect(new_resource).to be_updated
       end
 
       it "should not update a HEAD request if a not modified response (CHEF-4762)" do
         if_modified_since = File.mtime(__FILE__).httpdate
-        @new_resource.headers "If-Modified-Since" => if_modified_since
-        expect(@http).to receive(:head).with("http://www.opscode.com/", { "If-Modified-Since" => if_modified_since }).and_return(false)
-        @provider.run_action(:head)
-        expect(@new_resource).not_to be_updated
+        new_resource.headers "If-Modified-Since" => if_modified_since
+        expect(http).to receive(:head).with("http://www.opscode.com/", { "If-Modified-Since" => if_modified_since }).and_return(false)
+        provider.run_action(:head)
+        expect(new_resource).not_to be_updated
       end
 
       it "should run a HEAD request with If-Modified-Since header" do
-        @new_resource.headers "If-Modified-Since" => File.mtime(__FILE__).httpdate
-        expect(@http).to receive(:head).with("http://www.opscode.com/", @new_resource.headers)
-        @provider.run_action(:head)
+        new_resource.headers "If-Modified-Since" => File.mtime(__FILE__).httpdate
+        expect(http).to receive(:head).with("http://www.opscode.com/", new_resource.headers)
+        provider.run_action(:head)
       end
 
       it "doesn't call converge_by if HEAD does not return modified" do
-        expect(@http).to receive(:head).and_return(false)
-        expect(@provider).not_to receive(:converge_by)
-        @provider.run_action(:head)
+        expect(http).to receive(:head).and_return(false)
+        expect(provider).not_to receive(:converge_by)
+        provider.run_action(:head)
       end
     end
   end