Skip to content

Commit 0415bcd

Browse files
author
Glyn Normington
committed
Merge 57792554-optimise_initial_download to master
[Completes #57792554]
2 parents 78b6445 + d4e10e8 commit 0415bcd

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)