Merge 56854324-no-internet-configuration to master

Glyn Normington · Glyn Normington · commit 8c2a0c3530e0 · 2013-09-19T14:50:26.000+01:00
[Completes #56854324]
diff --git a/ Downloading b/ Downloading
diff --git a/README.md b/README.md
@@ -36,8 +36,8 @@ The buildpack supports configuration and extension through the use of Git reposi
 	* [JREs](docs/extending-jres.md)
 	* [Frameworks](docs/extending-frameworks.md)
 * Utilities
-	* [Caches](docs/util-caches.md)
-	* [Logging](docs/logging.md)
+	* [Caches](docs/util-caches.md) ([Configuration](docs/util-caches.md#configuration))
+	* [Logging](docs/logging.md) ([Configuration](docs/logging.md#configuration)) 
 	* [Repositories](docs/util-repositories.md)
 	* [Repository Builder](docs/util-repository-builder.md)
 	* [Test Applications](docs/util-test-applications.md)
diff --git a/config/cache.yml b/config/cache.yml
@@ -0,0 +1,18 @@
+# Cloud Foundry Java Buildpack
+# Copyright (c) 2013 the original author or authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Download cache configuration
+---
+remote_downloads: enabled
diff --git a/docs/util-caches.md b/docs/util-caches.md
@@ -48,6 +48,18 @@ The [`DownloadCache`][] is the most generic of the three caches.  It allows you
 def initialize(cache_root = Dir.tmpdir)
 ```
 
+### Configuration
+For general information on configuring the buildpack, refer to [Configuration and Extension][].
+
+Caching can be configured by modifying the [`config/cache.yml`][] file.
+
+| Name | Description
+| ---- | -----------
+| `remote_downloads` | This property can take the value `enabled` or `disabled`. <br><br>The default value of `enabled` means that the buildpack will check the internet connection and remember the result for the remainder of the buildpack invocation. If the internet is available, it will then be used to download files. If the internet is not available, cache will be consulted instead. <br><br>Alternatively, the property may be set to `disabled` which avoids the check for an internet connection, does not attempt downloads, and consults the cache instead.
+
+[Configuration and Extension]: ../README.md#Configuration-and-Extension
+[`config/downloadcache.yml`]: ../config/downloadcache.yml
+
 ## `JavaBuildpack::Util::ApplicationCache`
 The [`ApplicationCache`][] is a cache that persists files into the application cache passed to the `compile` script.  It examines `ARGV[1]` for the cache location and configures itself accordingly.
 
diff --git a/lib/java_buildpack/util/download_cache.rb b/lib/java_buildpack/util/download_cache.rb
@@ -18,9 +18,11 @@
 require 'java_buildpack/diagnostics'
 require 'java_buildpack/diagnostics/logger_factory'
 require 'java_buildpack/util'
+require 'monitor'
 require 'net/http'
 require 'tmpdir'
 require 'uri'
+require 'yaml'
 
 module JavaBuildpack::Util
 
@@ -58,14 +60,16 @@ def initialize(cache_root = Dir.tmpdir)
     #                    deleted while it is being used, the cached item can only be accessed as part of a block.
     # @return [void]
     def get(uri)
+      internet_up = DownloadCache.internet_available? uri
+
       filenames = filenames(uri)
       File.open(filenames[:lock], File::CREAT) do |lock_file|
         lock_file.flock(File::LOCK_EX)
 
-        if should_update(filenames)
+        if internet_up && should_update(filenames)
           update(filenames, uri)
         elsif should_download(filenames)
-          download(filenames, uri)
+          download(filenames, uri, internet_up)
         end
 
         lock_file.flock(File::LOCK_SH)
@@ -94,6 +98,8 @@ def evict(uri)
 
     private
 
+    CACHE_CONFIG = '../../../config/cache.yml'.freeze
+
     HTTP_ERRORS = [
         EOFError,
         Errno::ECONNREFUSED,
@@ -107,39 +113,62 @@ def evict(uri)
         Net::ProtocolError,
         SocketError,
         Timeout::Error
-    ]
+    ].freeze
 
-    TEST_URI = 'http://download.pivotal.io.s3.amazonaws.com/openjdk/lucid/x86_64/index.yml'.freeze
+    HTTP_OK = '200'.freeze
 
-    HTTP_OK = '200'
+    @@monitor = Monitor.new
+    @@internet_checked = false
+    @@internet_up = true
 
-    def self.internet_available?
-      uri = TEST_URI
-      rich_uri = URI(uri)
+    def self.get_configuration
+      expanded_path = File.expand_path(CACHE_CONFIG, File.dirname(__FILE__))
+      YAML.load_file(expanded_path)
+    end
 
-      # Beware known problems with timeouts: https://www.ruby-forum.com/topic/143840
-      Net::HTTP.start(rich_uri.host, rich_uri.port, read_timeout: 10, connect_timeout: 10, open_timeout: 10) do |http|
-        request = Net::HTTP::Get.new(uri)
-        http.request request do |response|
-          return response.code == HTTP_OK
+    TIMEOUT_SECONDS = 10
+
+    def self.internet_available?(uri)
+      @@monitor.synchronize do
+        return @@internet_up if @@internet_checked
+      end
+      cache_configuration = get_configuration
+      if cache_configuration['remote_downloads'] == 'disabled'
+        store_internet_availability false
+      elsif cache_configuration['remote_downloads'] == 'enabled'
+        begin
+          rich_uri = URI(uri)
+
+          # Beware known problems with timeouts: https://www.ruby-forum.com/topic/143840
+          Net::HTTP.start(rich_uri.host, rich_uri.port, read_timeout: TIMEOUT_SECONDS, connect_timeout: TIMEOUT_SECONDS, open_timeout: TIMEOUT_SECONDS) do |http|
+            request = Net::HTTP::Get.new(uri)
+            http.request request do |response|
+              internet_up = response.code == HTTP_OK
+              store_internet_availability internet_up
+            end
+          end
+        rescue *HTTP_ERRORS
+          store_internet_availability false
         end
+      else
+        raise "Invalid remote_downloads property in cache configuration: #{cache_configuration}"
       end
-    rescue *HTTP_ERRORS
-      false
     end
 
-    @@internet_up = DownloadCache.internet_available?
+    def self.store_internet_availability(internet_up)
+      @@monitor.synchronize do
+        @@internet_up = internet_up
+        @@internet_checked = true
+      end
+      internet_up
+    end
 
     def delete_file(filename)
       File.delete filename if File.exists? filename
     end
 
-    def self.internet_up
-      @@internet_up
-    end
-
-    def download(filenames, uri)
-      if DownloadCache.internet_up
+    def download(filenames, uri, internet_up)
+      if internet_up
         begin
           rich_uri = URI(uri)
 
@@ -181,8 +210,10 @@ def look_aside(filenames, uri)
         FileUtils.cp(stashed, filenames[:cached])
         @logger.debug "Using copy of #{uri} from buildpack cache."
       else
-        @logger.warn "Buildpack cache does not contain #{uri}. Failing the download."
+        message = "Buildpack cache does not contain #{uri}. Failing the download."
+        @logger.error message
         @logger.debug { "Buildpack cache contents:\n#{`ls -lR #{File.join(ENV['BUILDPACK_CACHE'], 'java-buildpack')}`}" }
+        raise message
       end
     end
 
diff --git a/spec/java_buildpack/repository/repository_index_spec.rb b/spec/java_buildpack/repository/repository_index_spec.rb
@@ -37,7 +37,6 @@ module JavaBuildpack::Repository
 
     it 'should use the read-only buildpack cache when index.yaml cannot be downloaded because the internet is not available' do
       stub_request(:get, 'http://foo.com/index.yml').to_raise(SocketError)
-      JavaBuildpack::Util::DownloadCache.stub(:internet_up).and_return(false)
 
       Dir.mktmpdir do |buildpack_cache|
         java_buildpack_cache = File.join(buildpack_cache, 'java-buildpack')
diff --git a/spec/java_buildpack/util/application_cache_spec.rb b/spec/java_buildpack/util/application_cache_spec.rb
@@ -26,10 +26,17 @@ module JavaBuildpack::Util
       @previous_value = ARGV[1]
       ARGV[1] = nil
       $stdout = StringIO.new
+
+      stub_request(:get, 'http://foo-uri/')
+      .with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' })
+      .to_return(status: 200, body: '', headers: {})
+
+      DownloadCache.class_variable_set :@@internet_checked, false
     end
 
     after do
       ARGV[1] = @previous_value
+      DownloadCache.class_variable_set :@@internet_checked, false
     end
 
     TEST_URI = 'http://foo-uri/'
diff --git a/spec/java_buildpack/util/download_cache_spec.rb b/spec/java_buildpack/util/download_cache_spec.rb
@@ -30,6 +30,16 @@ module JavaBuildpack::Util
       FileUtils.rm_rf diagnostics_directory
       JavaBuildpack::Diagnostics::LoggerFactory.create_logger tmpdir
       $stdout = StringIO.new
+
+      stub_request(:get, 'http://foo-uri/')
+      .with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' })
+      .to_return(status: 200, body: '', headers: {})
+
+      DownloadCache.class_variable_set :@@internet_checked, false
+    end
+
+    after do
+      DownloadCache.class_variable_set :@@internet_checked, false
     end
 
     it 'should download from a uri if the cached file does not exist' do
@@ -238,6 +248,29 @@ module JavaBuildpack::Util
       end
     end
 
+    it 'should not overwrite existing information if the update request fails' do
+      stub_request(:get, 'http://foo-uri/').with(
+          headers: {
+              'If-None-Match' => 'foo-etag',
+              'If-Modified-Since' => 'foo-last-modified'
+          }
+      ).to_raise(SocketError)
+
+      Dir.mktmpdir do |root|
+        touch root, 'cached', 'foo-cached'
+        touch root, 'etag', 'foo-etag'
+        touch root, 'last_modified', 'foo-last-modified'
+
+        DownloadCache.new(root).get('http://foo-uri/') {}
+
+        expect_file_content root, 'cached', 'foo-cached'
+        expect_file_content root, 'etag', 'foo-etag'
+        expect_file_content root, 'last_modified', 'foo-last-modified'
+
+        expect($stderr.string).to match('Unable to update from http://foo-uri/ due to Exception from WebMock. Using cached version.')
+      end
+    end
+
     it 'should pass read-only file to block' do
       stub_request(:get, 'http://foo-uri/').to_return(
           status: 200,
@@ -281,7 +314,6 @@ module JavaBuildpack::Util
           FileUtils.mkdir_p java_buildpack_cache
           touch java_buildpack_cache, 'cached', 'foo-stashed'
           with_buildpack_cache(buildpack_cache) do
-            DownloadCache.stub(:internet_up).and_return(false)
             DownloadCache.new(root).get('http://foo-uri/') do |file|
               expect(file.read).to eq('foo-stashed')
             end
@@ -290,6 +322,38 @@ module JavaBuildpack::Util
       end
     end
 
+    it 'should use the buildpack cache if the cache configuration disables remote downloads' do
+      YAML.stub(:load_file).with(File.expand_path('config/cache.yml')).and_return(
+          'remote_downloads' => 'disabled')
+      Dir.mktmpdir do |root|
+        Dir.mktmpdir do |buildpack_cache|
+          java_buildpack_cache = File.join(buildpack_cache, 'java-buildpack')
+          FileUtils.mkdir_p java_buildpack_cache
+          touch java_buildpack_cache, 'cached', 'foo-stashed'
+          with_buildpack_cache(buildpack_cache) do
+            DownloadCache.new(root).get('http://foo-uri/') do |file|
+              expect(file.read).to eq('foo-stashed')
+            end
+          end
+        end
+      end
+    end
+
+    it 'should raise an error if the cache configuration remote downloads setting is invalid' do
+      YAML.stub(:load_file).with(File.expand_path('config/cache.yml')).and_return(
+          'remote_downloads' => 'junk')
+      Dir.mktmpdir do |root|
+        Dir.mktmpdir do |buildpack_cache|
+          java_buildpack_cache = File.join(buildpack_cache, 'java-buildpack')
+          FileUtils.mkdir_p java_buildpack_cache
+          touch java_buildpack_cache, 'cached', 'foo-stashed'
+          with_buildpack_cache(buildpack_cache) do
+            expect { DownloadCache.new(root).get('http://foo-uri/') }.to raise_error(/Invalid remote_downloads property in cache configuration:/)
+          end
+        end
+      end
+    end
+
     it 'should raise error if download cannot be completed and buildpack cache does not contain the file' do
       stub_request(:get, 'http://foo-uri/').to_raise(SocketError)
 
@@ -304,6 +368,24 @@ module JavaBuildpack::Util
       end
     end
 
+    it 'should fail if a download attempt fails' do
+      stub_request(:get, 'http://foo-uri/').to_return(
+          status: 200,
+          body: 'foo-cached',
+          headers: {
+              Etag: 'foo-etag',
+              'Last-Modified' => 'foo-last-modified'
+          }
+      )
+
+      stub_request(:get, 'http://bar-uri/').to_raise(SocketError)
+
+      Dir.mktmpdir do |root|
+        DownloadCache.new(root).get('http://foo-uri/') {}
+        expect { DownloadCache.new(root).get('http://bar-uri/') {} }.to raise_error(%r(Unable to download from http://bar-uri/))
+      end
+    end
+
     def touch(root, extension, content = '')
       file = File.join(root, "http:%2F%2Ffoo-uri%2F.#{extension}")
       File.open(file, 'w') { |f| f.write(content) }
diff --git a/spec/java_buildpack/util/global_cache_spec.rb b/spec/java_buildpack/util/global_cache_spec.rb
@@ -23,10 +23,17 @@ module JavaBuildpack::Util
 
     before do
       @previous_value = ENV.delete 'BUILDPACK_CACHE'
+
+      stub_request(:get, 'http://foo-uri/')
+      .with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' })
+      .to_return(status: 200, body: '', headers: {})
+
+      DownloadCache.class_variable_set :@@internet_checked, false
     end
 
     after do
       ENV['BUILDPACK_CACHE'] = @previous_value
+      DownloadCache.class_variable_set :@@internet_checked, false
     end
 
     it 'should raise an error if BUILDPACK_CACHE is not defined' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -29,11 +29,6 @@
 require 'java_buildpack/diagnostics/common'
 require 'java_buildpack/diagnostics/logger_factory'
 
-# Following required for class variable initialisation in DownloadCache.
-WebMock::API.stub_request(:get, 'http://download.pivotal.io.s3.amazonaws.com/openjdk/lucid/x86_64/index.yml')
-.with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' })
-.to_return(status: 200, body: '', headers: {})
-
 RSpec.configure do |config|
   config.treat_symbols_as_metadata_keys_with_true_values = true
   config.run_all_when_everything_filtered = true
