support etags and last modified times for remote_file

4 files changed, 163 insertions, 39 deletions

diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb
index 4d1e696d0e..e979f5a4ba 100644
--- a/lib/chef/provider/remote_file.rb
+++ b/lib/chef/provider/remote_file.rb
@@ -30,6 +30,12 @@ class Chef
 
       def load_current_resource
         @current_resource = Chef::Resource::RemoteFile.new(@new_resource.name)
+        fileinfo = load_fileinfo
+        if fileinfo["src"]
+          @current_resource.etag fileinfo["etag"]
+          @current_resource.last_modified fileinfo["last_modified"]
+          @current_resource.source fileinfo["src"]
+        end
         super
       end
 
@@ -40,10 +46,13 @@ class Chef
           Chef::Log.debug("#{@new_resource} checksum matches target checksum (#{@new_resource.checksum}) - not updating")
         else
           sources = @new_resource.source
-          raw_file, raw_file_source = try_multiple_sources(sources)
-          if matches_current_checksum?(raw_file)
-            Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating"
+          raw_file, raw_file_source, target_matched = try_multiple_sources(sources)
+          if target_matched
+            Chef::Log.info("#{@new_resource} matched #{raw_file_source}, not updating")
+          elsif matches_current_checksum?(raw_file)
+            Chef::Log.info("#{@new_resource} downloaded from #{raw_file_source}, checksums match, not updating")
           else
+            Chef::Log.info("#{@new_resource} downloaded from #{raw_file_source}")
             description = [] 
             description << "copy file downloaded from #{raw_file_source} into #{@new_resource.path}"
             description << diff_current(raw_file.path)
@@ -98,7 +107,7 @@ class Chef
         source = sources.shift
         begin
           uri = URI.parse(source)
-          raw_file = grab_file_from_uri(uri)
+          raw_file, target_matched = grab_file_from_uri(uri)
         rescue ArgumentError => e
           raise e
         rescue => e
@@ -107,9 +116,9 @@ class Chef
           else
             error = e.to_s
           end
-          Chef::Log.debug("#{@new_resource} cannot be downloaded from #{source}: #{error}")
+          Chef::Log.info("#{@new_resource} cannot be downloaded from #{source}: #{error}")
           if source = sources.shift
-            Chef::Log.debug("#{@new_resource} trying to download from another mirror")
+            Chef::Log.info("#{@new_resource} trying to download from another mirror")
             retry
           else
             raise e
@@ -118,29 +127,91 @@ class Chef
         if uri.userinfo
           uri.password = "********"
         end
-        return raw_file, uri.to_s
+        return raw_file, uri.to_s, target_matched
       end
 
       # Given a source uri, return a Tempfile, or a File that acts like a Tempfile (close! method)
       def grab_file_from_uri(uri)
+        if_modified_since = @new_resource.last_modified
+        if_none_match = @new_resource.etag
+        if uri == @current_resource.source[0]
+          if_modified_since ||= @current_resource.last_modified
+          if_none_match ||= @current_resource.etag
+        end
+        target_matched = false
+        raw_file = nil
+        last_modified = nil
+        etag = nil
         if URI::HTTP === uri
           #HTTP or HTTPS
-          raw_file = RestClient::Request.execute(:method => :get, :url => uri.to_s, :raw_response => true).file
+          begin
+            headers = Hash.new
+            if if_none_match
+              headers[:if_none_match] = "\"#{if_none_match}\""
+            elsif if_modified_since
+              headers[:if_modified_since] = if_modified_since.strftime("%a, %d %b %Y %H:%M:%S %Z")
+            end
+            rest = RestClient::Request.execute(:method => :get, :url => uri.to_s, :headers => headers, :raw_response => true)
+            raw_file = rest.file
+            if rest.headers.include?(:last_modified)
+              last_modified = Time.parse(rest.headers[:last_modified])
+            end
+            if rest.headers.include?(:etag)
+              etag = rest.headers[:etag]
+            end
+          rescue RestClient::Exception => e
+            if e.http_code == 304
+              target_matched = true
+            else
+              raise e
+            end
+          end
         elsif URI::FTP === uri
           #FTP
-          raw_file = FTP::fetch(uri, @new_resource.ftp_active_mode)
+          raw_file, last_modified = FTP::fetch_if_modified(uri, @new_resource.ftp_active_mode, if_modified_since)
+          if last_modified && if_modified_since && last_modified <= if_modified_since
+            target_matched = true
+          end
         elsif uri.scheme == "file"
           #local/network file
+          last_modified = ::File.mtime(uri.path)
           raw_file = ::File.new(uri.path, "r")
           def raw_file.close!
             self.close
           end
+          if last_modified && if_modified_since && last_modified.to_i <= if_modified_since.to_i
+            target_matched = true
+          end
         else
           raise ArgumentError, "Invalid uri. Only http(s), ftp, and file are currently supported"
         end
-        raw_file
+        unless target_matched
+          @new_resource.etag etag unless @new_resource.etag
+          @new_resource.last_modified last_modified unless @new_resource.last_modified
+          save_fileinfo(uri)
+        end
+        return raw_file, target_matched
       end
 
+      def load_fileinfo
+        begin
+          Chef::JSONCompat.from_json(Chef::FileCache.load("remote_file/#{new_resource.name}"))
+        rescue Chef::Exceptions::FileNotFound
+          cache = Hash.new
+          cache["etag"] = nil
+          cache["last_modified"] = nil
+          cache["src"] = nil
+          cache
+        end
+      end
+
+      def save_fileinfo(uri)
+        cache = Hash.new
+        cache["etag"] = @new_resource.etag
+        cache["last_modified"] = @new_resource.last_modified
+        cache["src"] = uri
+        Chef::FileCache.store("remote_file/#{new_resource.name}", cache.to_json)
+      end
     end
   end
 end
diff --git a/lib/chef/provider/remote_file/ftp.rb b/lib/chef/provider/remote_file/ftp.rb
index 3c5d3e0a91..b0e28a4de8 100644
--- a/lib/chef/provider/remote_file/ftp.rb
+++ b/lib/chef/provider/remote_file/ftp.rb
@@ -28,7 +28,24 @@ class Chef
 
         # Fetches the file at uri using Net::FTP, returning a Tempfile
         def self.fetch(uri, ftp_active_mode)
-          self.new(uri, ftp_active_mode).fetch()
+          ftp = self.new(uri, ftp_active_mode)
+          ftp.connect
+          tempfile = ftp.fetch
+          ftp.disconnect
+          tempfile
+        end
+
+        def self.fetch_if_modified(uri, ftp_active_mode, last_modified)
+          ftp = self.new(uri, ftp_active_mode)
+          ftp.connect
+          mtime = ftp.mtime
+          if mtime && last_modified && mtime.to_i <= last_modified.to_i
+            tempfile = nil
+          else
+            tempfile = ftp.fetch
+          end
+          ftp.disconnect
+          return tempfile, mtime
         end
 
         # Parse the uri into instance variables
@@ -42,6 +59,7 @@ class Chef
           @ftp_active_mode = ftp_active_mode
           @hostname = uri.hostname
           @port = uri.port
+          @ftp = Net::FTP.new
           if uri.userinfo
             @user = URI.unescape(uri.user)
             @pass = URI.unescape(uri.password)
@@ -51,28 +69,35 @@ class Chef
           end
         end
 
-        # Fetches using Net::FTP, returns a Tempfile with the content
-        def fetch()
-          tempfile = Tempfile.new(@filename)
-
+        def connect
           # The access sequence is defined by RFC 1738
-          ftp = Net::FTP.new
-          ftp.connect(@hostname, @port)
-          ftp.passive = !@ftp_active_mode
-          ftp.login(@user, @pass)
+          @ftp.connect(@hostname, @port)
+          @ftp.passive = !@ftp_active_mode
+          @ftp.login(@user, @pass)
           @directories.each do |cwd|
-            ftp.voidcmd("CWD #{cwd}")
+            @ftp.voidcmd("CWD #{cwd}")
           end
+        end
+
+        def disconnect
+          @ftp.close
+        end
+
+        def mtime
+          @ftp.mtime(@filename)
+        end
+
+        # Fetches using Net::FTP, returns a Tempfile with the content
+        def fetch
+          tempfile = Tempfile.new(@filename)
           if @typecode
-            ftp.voidcmd("TYPE #{@typecode.upcase}")
+            @ftp.voidcmd("TYPE #{@typecode.upcase}")
           end
-          ftp.getbinaryfile(@filename, tempfile.path)
-          ftp.close
-
+          @ftp.getbinaryfile(@filename, tempfile.path)
           tempfile
         end
 
-				private
+        private
 
         def parse_path(path)
           path = path.sub(%r{\A/}, '%2F') # re-encode the beginning slash because uri library decodes it.
diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb
index 524e00186b..df055ab6b4 100644
--- a/lib/chef/resource/remote_file.rb
+++ b/lib/chef/resource/remote_file.rb
@@ -33,6 +33,8 @@ class Chef
         @resource_name = :remote_file
         @action = "create"
         @source = []
+        @etag = nil
+        @last_modified = nil
         @ftp_active_mode = false
         @provider = Chef::Provider::RemoteFile
       end
@@ -55,6 +57,31 @@ class Chef
         )
       end
 
+      def etag(args=nil)
+        # Only store the etag itself, skip the quotes (and leading W/ if it's there)
+        if args.is_a?(String) && args.include?('"')
+          args = args.split('"')[1]
+        end
+        set_or_return(
+          :etag,
+          args,
+          :kind_of => String
+        )
+      end
+
+      def last_modified(args=nil)
+        if args.is_a?(String)
+          args = Time.parse(args).gmtime
+        elsif args.is_a?(Time)
+          args.gmtime
+        end
+        set_or_return(
+          :last_modified,
+          args,
+          :kind_of => Time
+        )
+      end
+
       def ftp_active_mode(args=nil)
         set_or_return(
           :ftp_active_mode,
diff --git a/spec/unit/provider/remote_file_spec.rb b/spec/unit/provider/remote_file_spec.rb
index 149f0462c3..2cd7559c95 100644
--- a/spec/unit/provider/remote_file_spec.rb
+++ b/spec/unit/provider/remote_file_spec.rb
@@ -46,7 +46,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
   describe "when fetching the file from the remote" do
     before(:each) do
       @tempfile = Tempfile.new("chef-rspec-remote_file_spec-line#{__LINE__}--")
-      @rawresp = RestClient::RawResponse.new(@tempfile, nil, nil)
+      @rawresp = RestClient::RawResponse.new(@tempfile, Hash.new, nil)
 
       RestClient::Request.stub!(:execute).and_return(@rawresp)
       @resource.cookbook_name = "monkey"
@@ -79,14 +79,14 @@ describe Chef::Provider::RemoteFile, "action_create" do
 
     shared_examples_for "source specified with multiple URIs" do
       it "should try to download the next URI when the first one fails" do
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :raw_response => true).once.and_raise(SocketError)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :raw_response => true).once.and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :headers => {}, :raw_response => true).once.and_raise(SocketError)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :headers => {}, :raw_response => true).once.and_return(@rawresp)
         @provider.run_action(:create)
       end
 
       it "should raise an exception when all the URIs fail" do
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :raw_response => true).once.and_raise(SocketError)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :raw_response => true).once.and_raise(SocketError)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://foo", :headers => {}, :raw_response => true).once.and_raise(SocketError)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://bar", :headers => {}, :raw_response => true).once.and_raise(SocketError)
         lambda { @provider.run_action(:create) }.should raise_error(SocketError)
       end
 
@@ -121,7 +121,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         end
 
         it "does not download the file" do
-          RestClient::Request.should_not_receive(:execute).with("http://opscode.com/seattle.txt").and_return(@tempfile)
+          RestClient::Request.should_not_receive(:execute)
           @provider.run_action(:create)
         end
 
@@ -138,7 +138,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         end
 
         it "should not download the file if the checksum is a partial match from the beginning" do
-          @rawresp.should_not_receive(:fetch).with("http://opscode.com/seattle.txt").and_return(@tempfile)
+          RestClient::Request.should_not_receive(:execute)
           @provider.run_action(:create)
         end
 
@@ -152,7 +152,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
       describe "and the existing file doesn't match the given checksum" do
         it "downloads the file" do
           @resource.checksum("this hash doesn't match")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
+          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
@@ -160,7 +160,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
         it "does not consider the checksum a match if the matching string is offset" do
           # i.e., the existing file is      "0fd012fdc96e96f8f7cf2046522a54aed0ce470224513e45da6bc1a17a4924aa"
           @resource.checksum("fd012fd")
-          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
+          RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
           @provider.stub!(:update_new_file_state)
           @provider.run_action(:create)
         end
@@ -171,7 +171,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     describe "and the resource doesn't specify a checksum" do
       it "should download the file from the remote URL" do
         @resource.checksum(nil)
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
         @provider.run_action(:create)
       end
     end
@@ -188,7 +188,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     context "and the target file is a tarball" do
       before do
         @resource.path(File.expand_path(File.join(CHEF_SPEC_DATA, "seattle.tar.gz")))
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :raw_response => true).and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://opscode.com/seattle.txt", :headers => {}, :raw_response => true).and_return(@rawresp)
       end
 
       it "disables gzip in the http client" do
@@ -200,7 +200,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
     context "and the source appears to be a tarball" do
       before do
         @resource.source("http://example.com/tarball.tgz")
-        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://example.com/tarball.tgz", :raw_response => true).and_return(@rawresp)
+        RestClient::Request.should_receive(:execute).with(:method => :get, :url => "http://example.com/tarball.tgz", :headers => {}, :raw_response => true).and_return(@rawresp)
       end
 
       it "disables gzip in the http client" do
@@ -214,13 +214,13 @@ describe Chef::Provider::RemoteFile, "action_create" do
       end
 
       it "should fetch with ftp in passive mode" do
-        Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), false).and_return(@tempfile)
+        Chef::Provider::RemoteFile::FTP.should_receive(:fetch_if_modified).with(URI.parse("ftp://opscode.com/seattle.txt"), false, nil).and_return(@tempfile)
         @provider.run_action(:create)
       end
 
       it "should fetch with ftp in active mode" do
         @resource.ftp_active_mode true
-        Chef::Provider::RemoteFile::FTP.should_receive(:fetch).with(URI.parse("ftp://opscode.com/seattle.txt"), true).and_return(@tempfile)
+        Chef::Provider::RemoteFile::FTP.should_receive(:fetch_if_modified).with(URI.parse("ftp://opscode.com/seattle.txt"), true, nil).and_return(@tempfile)
         @provider.run_action(:create)
       end
     end
@@ -231,6 +231,7 @@ describe Chef::Provider::RemoteFile, "action_create" do
       end
 
       it "should load the local file" do
+        File.should_receive(:mtime).with("/nyan_cat.png").and_return(Time.now)
         File.should_receive(:new).with("/nyan_cat.png", "r").and_return(File.open(File.join(CHEF_SPEC_DATA, "remote_file", "nyan_cat.png"), "r"))
         @provider.run_action(:create)
       end