Skip to content

Commit d4e10e8

Browse files
author
Glyn Normington
committed
Use files downloaded during internet detection
This avoids the first file downloaded to determine internet availability from being downloaded again unnecessarily. [#57792554]
1 parent 78b6445 commit d4e10e8

2 files changed

Lines changed: 56 additions & 19 deletions

File tree

‎lib/java_buildpack/util/download_cache.rb‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,19 @@ def initialize(cache_root = Dir.tmpdir)
6060
# deleted while it is being used, the cached item can only be accessed as part of a block.
6161
# @return [void]
6262
def get(uri)
63-
internet_up = DownloadCache.internet_available? uri, @logger
64-
6563
filenames = filenames(uri)
64+
6665
File.open(filenames[:lock], File::CREAT) do |lock_file|
6766
lock_file.flock(File::LOCK_EX)
6867

69-
if internet_up && should_update(filenames)
70-
update(filenames, uri)
71-
elsif should_download(filenames)
72-
download(filenames, uri, internet_up)
68+
internet_up, file_downloaded = DownloadCache.internet_available?(filenames, uri, @logger)
69+
70+
unless file_downloaded
71+
if internet_up && should_update(filenames)
72+
update(filenames, uri)
73+
elsif should_download(filenames)
74+
download(filenames, uri, internet_up)
75+
end
7376
end
7477

7578
lock_file.flock(File::LOCK_SH)
@@ -128,13 +131,13 @@ def self.get_configuration
128131

129132
TIMEOUT_SECONDS = 10
130133

131-
def self.internet_available?(uri, logger)
134+
def self.internet_available?(filenames, uri, logger)
132135
@@monitor.synchronize do
133-
return @@internet_up if @@internet_checked
136+
return @@internet_up, false if @@internet_checked # rubocop:disable RedundantReturn
134137
end
135138
cache_configuration = get_configuration
136139
if cache_configuration['remote_downloads'] == 'disabled'
137-
store_internet_availability false
140+
return store_internet_availability(false), false # rubocop:disable RedundantReturn
138141
elsif cache_configuration['remote_downloads'] == 'enabled'
139142
begin
140143
rich_uri = URI(uri)
@@ -144,12 +147,13 @@ def self.internet_available?(uri, logger)
144147
request = Net::HTTP::Get.new(uri)
145148
http.request request do |response|
146149
internet_up = response.code == HTTP_OK
147-
store_internet_availability internet_up
150+
write_response(filenames, response) if internet_up
151+
return store_internet_availability(internet_up), internet_up # rubocop:disable RedundantReturn
148152
end
149153
end
150154
rescue *HTTP_ERRORS => ex
151155
logger.debug { "Internet detection failed with #{ex}" }
152-
store_internet_availability false
156+
return store_internet_availability(false), false # rubocop:disable RedundantReturn
153157
end
154158
else
155159
fail "Invalid remote_downloads property in cache configuration: #{cache_configuration}"
@@ -179,10 +183,10 @@ def download(filenames, uri, internet_up)
179183
begin
180184
rich_uri = URI(uri)
181185

182-
Net::HTTP.start(rich_uri.host, rich_uri.port, use_ssl: use_ssl?(rich_uri)) do |http|
186+
Net::HTTP.start(rich_uri.host, rich_uri.port, use_ssl: DownloadCache.use_ssl?(rich_uri)) do |http|
183187
request = Net::HTTP::Get.new(uri)
184188
http.request request do |response|
185-
write_response(filenames, response)
189+
DownloadCache.write_response(filenames, response)
186190
end
187191
end
188192

@@ -224,7 +228,7 @@ def look_aside(filenames, uri)
224228
end
225229
end
226230

227-
def persist_header(response, header, filename)
231+
def self.persist_header(response, header, filename)
228232
unless response[header].nil?
229233
File.open(filename, File::CREAT | File::WRONLY) do |file|
230234
file.write(response[header])
@@ -252,25 +256,25 @@ def should_update(filenames)
252256
def update(filenames, uri)
253257
rich_uri = URI(uri)
254258

255-
Net::HTTP.start(rich_uri.host, rich_uri.port, use_ssl: use_ssl?(rich_uri)) do |http|
259+
Net::HTTP.start(rich_uri.host, rich_uri.port, use_ssl: DownloadCache.use_ssl?(rich_uri)) do |http|
256260
request = Net::HTTP::Get.new(uri)
257261
set_header request, 'If-None-Match', filenames[:etag]
258262
set_header request, 'If-Modified-Since', filenames[:last_modified]
259263

260264
http.request request do |response|
261-
write_response(filenames, response) unless response.code == '304'
265+
DownloadCache.write_response(filenames, response) unless response.code == '304'
262266
end
263267
end
264268

265269
rescue *HTTP_ERRORS => ex
266270
@logger.warn "Unable to update from #{uri} due to #{ex}. Using cached version."
267271
end
268272

269-
def use_ssl?(uri)
273+
def self.use_ssl?(uri)
270274
uri.scheme == 'https'
271275
end
272276

273-
def write_response(filenames, response)
277+
def self.write_response(filenames, response)
274278
persist_header response, 'Etag', filenames[:etag]
275279
persist_header response, 'Last-Modified', filenames[:last_modified]
276280

‎spec/java_buildpack/util/download_cache_spec.rb‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ module JavaBuildpack::Util
2222

2323
describe DownloadCache do
2424

25+
def suppress_internet_availability_check
26+
DownloadCache.send :store_internet_availability, true
27+
end
28+
2529
before do
2630
JavaBuildpack::Diagnostics::LoggerFactory.send :close
2731
$stderr = StringIO.new
@@ -42,7 +46,28 @@ module JavaBuildpack::Util
4246
DownloadCache.class_variable_set :@@internet_checked, false
4347
end
4448

45-
it 'should download from a uri if the cached file does not exist' do
49+
it 'should download (during internet availability checking) from a uri if the cached file does not exist' do
50+
stub_request(:get, 'http://foo-uri/').to_return(
51+
status: 200,
52+
body: 'foo-cached',
53+
headers: {
54+
Etag: 'foo-etag',
55+
'Last-Modified' => 'foo-last-modified'
56+
}
57+
)
58+
59+
Dir.mktmpdir do |root|
60+
DownloadCache.new(root).get('http://foo-uri/') { }
61+
62+
expect_file_content root, 'cached', 'foo-cached'
63+
expect_file_content root, 'etag', 'foo-etag'
64+
expect_file_content root, 'last_modified', 'foo-last-modified'
65+
end
66+
end
67+
68+
it 'should download (after internet availability checking) from a uri if the cached file does not exist' do
69+
suppress_internet_availability_check
70+
4671
stub_request(:get, 'http://foo-uri/').to_return(
4772
status: 200,
4873
body: 'foo-cached',
@@ -70,6 +95,8 @@ module JavaBuildpack::Util
7095
end
7196

7297
it 'should download from a uri if the cached file exists and etag exists' do
98+
suppress_internet_availability_check
99+
73100
stub_request(:get, 'http://foo-uri/').with(
74101
headers: {
75102
'If-None-Match' => 'foo-etag'
@@ -107,6 +134,8 @@ module JavaBuildpack::Util
107134
end
108135

109136
it 'should download from a uri if the cached file exists and last modified exists' do
137+
suppress_internet_availability_check
138+
110139
stub_request(:get, 'http://foo-uri/').with(
111140
headers: {
112141
'If-Modified-Since' => 'foo-last-modified'
@@ -221,6 +250,8 @@ module JavaBuildpack::Util
221250
end
222251

223252
it 'should overwrite existing information if 304 is not received' do
253+
suppress_internet_availability_check
254+
224255
stub_request(:get, 'http://foo-uri/').with(
225256
headers: {
226257
'If-None-Match' => 'foo-etag',
@@ -249,6 +280,8 @@ module JavaBuildpack::Util
249280
end
250281

251282
it 'should not overwrite existing information if the update request fails' do
283+
suppress_internet_availability_check
284+
252285
stub_request(:get, 'http://foo-uri/').with(
253286
headers: {
254287
'If-None-Match' => 'foo-etag',

0 commit comments

Comments
 (0)