Skip to content

Commit

Permalink
First pass at config options to handle non-successful responses, with…
Browse files Browse the repository at this point in the history
… broken tests
  • Loading branch information
Ron Smith committed Dec 13, 2013
1 parent ce4fec4 commit 1f4735e
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 5 deletions.
24 changes: 22 additions & 2 deletions lib/billy/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,23 @@ def initialize
reset
end

def cacheable?(url, headers)
def cacheable?(url, headers, status)
if Billy.config.cache
url = URI(url)

if !successful_status?(status)
error_level = Billy.config.non_successful_error_level
error_message = "Received response status code #{status} for #{format_url(url)}"
case Billy.config.non_successful_error_level
when :error
raise error_message
else
Billy.log(error_level, error_message)
end
end

# Cache the responses if they aren't whitelisted host[:port]s but always cache blacklisted paths on any hosts
!whitelisted_host?(url.host) && !whitelisted_host?("#{url.host}:#{url.port}") || blacklisted_path?(url.path)
cacheable_status?(status) && (!whitelisted_host?(url.host) && !whitelisted_host?("#{url.host}:#{url.port}") || blacklisted_path?(url.path))
# TODO test headers for cacheability
end
end
Expand All @@ -28,6 +40,14 @@ def blacklisted_path?(path)
Billy.config.path_blacklist.index{|bl| path.include?(bl)}
end

def successful_status?(status)
(200..299).include?(status)
end

def cacheable_status?(status)
Billy.config.non_successful_cache_disabled ? successful_status?(status) : true
end

def cached?(method, url, body)
key = key(method, url, body)
!@cache[key].nil? or persisted?(key)
Expand Down
7 changes: 6 additions & 1 deletion lib/billy/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ module Billy
class Config
DEFAULT_WHITELIST = ['127.0.0.1', 'localhost']

attr_accessor :logger, :cache, :whitelist, :path_blacklist, :ignore_params, :persist_cache, :ignore_cache_port, :cache_path
attr_accessor :logger, :cache, :whitelist, :path_blacklist, :ignore_params, :persist_cache,
:ignore_cache_port, :non_successful_cache_disabled, :non_successful_error_level,
:allow_http_connections_when_no_cache, :cache_path

def initialize
@logger = defined?(Rails) ? Rails.logger : Logger.new(STDOUT)
Expand All @@ -19,6 +21,9 @@ def reset
@ignore_params = []
@persist_cache = false
@ignore_cache_port = true
@non_successful_cache_disabled = false
@non_successful_error_level = :warn
@allow_http_connections_when_no_cache = true
@cache_path = File.join(Dir.tmpdir, 'puffing-billy')
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/billy/proxy_connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def handle_request
respond_from_cache
else
Billy.log(:info, "PROXY #{@parser.http_method} #{@url}")
raise "Connection to #{@url}#{@parser.http_method == 'post' ? " with body '#{@body}'" : ''} not proxied and new http connections are disabled" unless Billy.config.allow_http_connections_when_no_cache
proxy_request
end
end
Expand Down Expand Up @@ -115,7 +116,7 @@ def proxy_request
res_headers = res_headers.merge('Connection' => 'close')
res_headers.delete('Transfer-Encoding')
res_content = req.response.force_encoding('BINARY')
if cache.cacheable?(@url, res_headers)
if cache.cacheable?(@url, res_headers, res_status)
cache.store(@parser.http_method.downcase, @url, @body, res_status, res_headers, res_content)
end

Expand Down
42 changes: 41 additions & 1 deletion spec/lib/proxy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
http.get('/foo').body.should == 'hello, GET!'
end

it 'should stub GET response statuses' do
proxy.stub("#{url}/foo").
and_return(:code => 200)
http.get('/foo').status.should == 200
end

it 'should stub POST requests' do
proxy.stub("#{url}/bar", :method => :post).
and_return(:text => 'hello, POST!')
Expand All @@ -46,7 +52,7 @@
it 'should stub HEAD requests' do
proxy.stub("#{url}/bap", :method => :head).
and_return(:headers => {'HTTP-X-Hello' => 'hello, HEAD!'})
http.head('/bap').headers['http_x_hello'] == 'hello, HEAD!'
http.head('/bap').headers['http-x-hello'].should == 'hello, HEAD!'
end

it 'should stub DELETE requests' do
Expand Down Expand Up @@ -167,6 +173,40 @@
end
end

context 'allow_http_connections_when_no_cache requests' do
before { Billy.config.allow_http_connections_when_no_cache = false }

it 'should raise error when disabled' do
# This causes the test to hang indefinitely. Guessing it has to do with the proxy process.
# expect(http.get('/foo')).to raise RuntimeError
end
end

context 'non_successful_cache_disabled requests' do
before { Billy.config.non_successful_cache_disabled = true }

it 'should not cache non-successful response when enabled' do
# Using this method never creates a file
# proxy.stub("#{url}/foo").and_return(:text => 'GET /foo', :code => 500)
# The test server in spec/support/test_server.rb is hard-coded to return a 200
# Need a good way to test this
# http.get('/foo')
# File.exists?(cached_file).should be_false
end

it 'should cache successful response when enabled' do
assert_cached_url
end
end

context 'non_successful_error_level requests' do
before { Billy.config.non_successful_error_level = :error }

it 'should raise error for non-successful responses when :error' do
# Need a way to simulate a non-successful response for this test
end
end

end

context "disabled" do
Expand Down

0 comments on commit 1f4735e

Please sign in to comment.