Skip to content

Commit 6606dd9

Browse files
committed
Immutable CachedFiles
Previously, the CachedFile always behaved as if it was mutable, even in situations where it was working against an immutable cache. This resulted in situations where a combination of intermittent network connections and read-only file systems would cause an attempted direction creation that would fail. This change enhances the CacheFile such that it can behave in both a mutable and immutable fashion. [#71928434][#54]
1 parent 26bc064 commit 6606dd9

File tree

3 files changed

+34
-8
lines changed

3 files changed

+34
-8
lines changed

lib/java_buildpack/util/cache/cached_file.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,15 @@ class CachedFile
3131
#
3232
# @param [Pathname] cache_root the filesystem root for the file created and expected by this class
3333
# @param [String] uri a uri which uniquely identifies the file in the cache
34-
def initialize(cache_root, uri)
35-
FileUtils.mkdir_p cache_root
36-
34+
# @param [Boolean] mutable whether the cached file should be mutable
35+
def initialize(cache_root, uri, mutable)
3736
key = URI.escape(uri, ':/')
3837
@cached = cache_root + "#{key}.cached"
3938
@etag = cache_root + "#{key}.etag"
4039
@last_modified = cache_root + "#{key}.last_modified"
40+
@mutable = mutable
41+
42+
FileUtils.mkdir_p cache_root if mutable
4143
end
4244

4345
# Opens the cached file
@@ -60,7 +62,7 @@ def cached?
6062

6163
# Destroys the cached file
6264
def destroy
63-
[@cached, @etag, @last_modified].each { |f| f.delete if f.exist? }
65+
[@cached, @etag, @last_modified].each { |f| f.delete if f.exist? } if @mutable
6466
end
6567

6668
# Opens the etag file

lib/java_buildpack/util/cache/download_cache.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def get(uri, &block)
7171
# @param [String] uri the URI of the item
7272
# @return [Void]
7373
def evict(uri)
74-
CachedFile.new(@mutable_cache_root, uri).destroy
74+
CachedFile.new(@mutable_cache_root, uri, true).destroy
7575
end
7676

7777
private
@@ -175,7 +175,7 @@ def cache_last_modified(response, cached_file)
175175
end
176176

177177
def from_mutable_cache(uri)
178-
cached_file = CachedFile.new(@mutable_cache_root, uri)
178+
cached_file = CachedFile.new @mutable_cache_root, uri, true
179179
cached = update URI(uri), cached_file
180180
[cached_file, cached]
181181
rescue => e
@@ -185,7 +185,7 @@ def from_mutable_cache(uri)
185185

186186
def from_immutable_caches(uri)
187187
@immutable_cache_roots.each do |cache_root|
188-
candidate = CachedFile.new cache_root, uri
188+
candidate = CachedFile.new cache_root, uri, false
189189

190190
next unless candidate.cached?
191191

spec/java_buildpack/util/cache/cached_file_spec.rb

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,30 @@
2222
describe JavaBuildpack::Util::Cache::CachedFile do
2323
include_context 'application_helper'
2424

25-
let(:file_cache) { described_class.new(app_dir, 'http://foo-uri/') }
25+
let(:cache_root) { app_dir + 'cache/root' }
26+
27+
let(:file_cache) { described_class.new(app_dir, 'http://foo-uri/', true) }
2628

2729
it 'should not create any files on initialization' do
2830
%w(cached etag last_modified).each { |extension| expect(cache_file(extension)).not_to exist }
2931
end
3032

33+
it 'should create cache_root if mutable' do
34+
expect(cache_root).not_to exist
35+
36+
described_class.new(cache_root, 'http://foo-uri/', true)
37+
38+
expect(cache_root).to exist
39+
end
40+
41+
it 'should not create cache_root if immutable' do
42+
expect(cache_root).not_to exist
43+
44+
described_class.new(cache_root, 'http://foo-uri/', false)
45+
46+
expect(cache_root).not_to exist
47+
end
48+
3149
it 'should not detect cached file' do
3250
expect(file_cache.cached?).not_to be
3351
end
@@ -62,6 +80,12 @@
6280
%w(cached etag last_modified).each { |extension| expect(cache_file(extension)).not_to exist }
6381
end
6482

83+
it 'should not destroy all files if immutable' do
84+
described_class.new(app_dir, 'http://foo-uri/', false).destroy
85+
86+
%w(cached etag last_modified).each { |extension| expect(cache_file(extension)).to exist }
87+
end
88+
6589
it 'should call the block with the content of the etag file' do
6690
expect { |b| file_cache.etag(File::RDONLY, 'test-arg', &b) }.to yield_file_with_content(/foo-etag/)
6791
end

0 commit comments

Comments
 (0)