From 8f7a70822762b0f382791b5d447abbf35793967a Mon Sep 17 00:00:00 2001 From: Paul Sadauskas Date: Sat, 31 Jan 2009 18:15:22 -0700 Subject: [PATCH] Lots more caching specs --- lib/resourceful/cache_manager.rb | 4 +- lib/resourceful/exceptions.rb | 4 + lib/resourceful/request.rb | 14 +-- lib/resourceful/response.rb | 18 ++-- spec/acceptance/authorization_spec.rb | 16 +++ spec/acceptance/caching_spec.rb | 145 ++++++++++++++++++++++++++ spec/acceptance/header_spec.rb | 12 +++ spec/acceptance/redirecting_spec.rb | 12 +++ spec/acceptance/resource_spec.rb | 1 - spec/acceptance_spec.rb | 40 ------- spec/simple_sinatra_server.rb | 54 ++++------ spec/spec_helper.rb | 29 +++++- 12 files changed, 260 insertions(+), 89 deletions(-) create mode 100644 spec/acceptance/authorization_spec.rb create mode 100644 spec/acceptance/caching_spec.rb create mode 100644 spec/acceptance/header_spec.rb create mode 100644 spec/acceptance/redirecting_spec.rb diff --git a/lib/resourceful/cache_manager.rb b/lib/resourceful/cache_manager.rb index 2133781..4fd8f00 100644 --- a/lib/resourceful/cache_manager.rb +++ b/lib/resourceful/cache_manager.rb @@ -78,7 +78,7 @@ def lookup(request) end def store(request, response) - return unless response.cachable? + return unless response.cacheable? @collection[request.uri.to_s][request] = response end @@ -110,7 +110,7 @@ def lookup(request) end def store(request, response) - return unless response.cachable? + return unless response.cacheable? entries = cache_entries_for(request) entries[request] = response diff --git a/lib/resourceful/exceptions.rb b/lib/resourceful/exceptions.rb index fbf56f3..3d9aef0 100644 --- a/lib/resourceful/exceptions.rb +++ b/lib/resourceful/exceptions.rb @@ -15,6 +15,10 @@ def initialize(http_request, http_response) end end + class MalformedServerResponse < UnsuccessfulHttpRequestError + end + + # Exception indicating that the server used a content coding scheme # that Resourceful is unable to handle. class UnsupportedContentCoding < Exception diff --git a/lib/resourceful/request.rb b/lib/resourceful/request.rb index 4de4297..bc84b60 100644 --- a/lib/resourceful/request.rb +++ b/lib/resourceful/request.rb @@ -48,7 +48,7 @@ def add_credentials! def fetch_response if cached_response if needs_revalidation?(cached_response) - logger.info(" Cache entry is stale") + logger.info(" Cache needs revalidation") set_validation_headers!(cached_response) else # We're done! @@ -58,7 +58,7 @@ def fetch_response response = perform! - response = revalidate_cached_response(response) if response.not_modified? + response = revalidate_cached_response(response) if cached_response && response.not_modified? response = follow_redirect(response) if should_be_redirected?(response) response = retry_with_auth(response) if needs_authorization?(response) @@ -100,6 +100,7 @@ def revalidate_cached_response(not_modified_response) # Follow a redirect response def follow_redirect(response) + raise MalformedServerResponse.new(self, response) unless response.header.location if response.moved_permanently? new_uri = response.header.location.first logger.info(" Permanently redirected to #{new_uri} - Storing new location.") @@ -155,7 +156,7 @@ def invalidates_cache? # Perform the request, with no magic handling of anything. def perform! @request_time = Time.now - logger.debug(" DEBUG: Request Header: #{@header.inspect}") + logger.debug("DEBUG: Request Header: #{@header.inspect}") http_resp = NetHttpAdapter.make_request(@method, @resource.uri, @body, @header) @response = Resourceful::Response.new(uri, *http_resp) @@ -199,9 +200,10 @@ def uri # Does this request force us to revalidate the cache? def forces_revalidation? - if cc = header['Cache-Control'] - cc.include?('no-cache') || max_age == 0 - else + if max_age == 0 || header.cache_control && cc.include?('no-cache') + logger.info(" Client forced revalidation") + true + else false end end diff --git a/lib/resourceful/response.rb b/lib/resourceful/response.rb index 3c29155..375b72b 100644 --- a/lib/resourceful/response.rb +++ b/lib/resourceful/response.rb @@ -8,6 +8,7 @@ module Resourceful class Response REDIRECT_RESPONSE_CODES = [301,302,303,307] + NORMALLY_CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410] attr_reader :uri, :code, :header, :body, :response_time alias headers header @@ -50,12 +51,15 @@ def stale? # Is this response cachable? # # @return true|false - def cachable? - return false unless [200, 203, 300, 301, 410].include?(code.to_i) - return false if header['Vary'] and header['Vary'].include?('*') - return false if header['Cache-Control'] and header['Cache-Control'].include?('no-store') - - true + def cacheable? + @cacheable ||= begin + @cacheable = true if NORMALLY_CACHEABLE_RESPONSE_CODES.include?(code.to_i) + @cacheable = false if header.vary && header.vary.include?('*') + @cacheable = false if header.cache_control && header.cache_control.include?('no-cache') + @cacheable = true if header.cache_control && header.cache_control.include?('public') + @cacheable = true if header.cache_control && header.cache_control.include?('private') + @cacheable || false + end end # Does this response force revalidation? @@ -144,6 +148,8 @@ def body 505 => "HTTP Version Not Supported".freeze, }.freeze + CODES = CODE_NAMES.keys + CODE_NAMES.each do |code, msg| method_name = msg.downcase.gsub(/[- ]/, "_") diff --git a/spec/acceptance/authorization_spec.rb b/spec/acceptance/authorization_spec.rb new file mode 100644 index 0000000..37e72a8 --- /dev/null +++ b/spec/acceptance/authorization_spec.rb @@ -0,0 +1,16 @@ + +require File.dirname(__FILE__) + '/../spec_helper' +require 'resourceful' + +describe Resourceful do + + describe "basic auth" do + + end + + describe "digest auth" do + + end + +end + diff --git a/spec/acceptance/caching_spec.rb b/spec/acceptance/caching_spec.rb new file mode 100644 index 0000000..5cf00ae --- /dev/null +++ b/spec/acceptance/caching_spec.rb @@ -0,0 +1,145 @@ + +require File.dirname(__FILE__) + '/../spec_helper' +require 'resourceful' + +describe Resourceful do + + describe "caching" do + + before do + @http = Resourceful::HttpAccessor.new(:cache_manager => Resourceful::InMemoryCacheManager.new) + if ENV['SPEC_LOGGING'] + @http.logger = Resourceful::StdOutLogger.new + end + end + + def get_with_errors(resource) + begin + resp = resource.get + rescue Resourceful::UnsuccessfulHttpRequestError => e + resp = e.http_response + end + resp + end + + def uri_plus_params(uri, params = {}) + uri = uri.is_a?(Addressable::URI) ? uri : Addressable::URI.parse(uri) + uri.query_values = params + uri + end + + def uri_for_code(code, params = {}) + uri = Addressable::URI.expand_template("http://localhost:3000/code/{code}", + "code" => code.to_s) + + uri_plus_params(uri, params) + end + + describe "response cacheability" do + Resourceful::Response::NORMALLY_CACHEABLE_RESPONSE_CODES.each do |code| + describe "response code #{code}" do + it "should normally be cached" do + resource = @http.resource(uri_for_code(code)) + + resp = get_with_errors(resource) + resp.should be_cacheable + end + + it "should not be cached if Vary: *" do + resource = @http.resource(uri_for_code(200, "Vary" => "*")) + + resp = get_with_errors(resource) + resp.should_not be_cacheable + end + + it "should not be cached if Cache-Control: no-cache'" do + resource = @http.resource(uri_for_code(200, "Cache-Control" => "no-cache")) + + resp = get_with_errors(resource) + resp.should_not be_cacheable + end + end + end + + # I would prefer to do all other codes, but some of them do some magic stuff (100), + # so I'll just spot check. + [201, 206, 302, 307, 404, 500].each do |code| + describe "response code #{code}" do + it "should not normally be cached" do + resource = @http.resource(uri_for_code(code)) + + resp = get_with_errors(resource) + resp.should_not be_cacheable + end + + it "should be cached if Cache-Control: public" do + resource = @http.resource(uri_for_code(code, "Cache-Control" => "public")) + + resp = get_with_errors(resource) + resp.should be_cacheable + end + + it "should be cached if Cache-Control: private" do + resource = @http.resource(uri_for_code(code, "Cache-Control" => "private")) + + resp = get_with_errors(resource) + resp.should be_cacheable + end + end + end + + end + + describe "expiration" do + it 'should use the cached response if Expire: is in the future' do + in_the_future = (Time.now + 60).httpdate + resource = @http.resource(uri_for_code(200, "Expire" => in_the_future)) + + resp = resource.get + resp.should_not be_expired + + resp = resource.get + resp.should be_ok + resp.should_not be_authoritative + end + + it 'should revalidate the cached response if the response is expired' do + in_the_past = (Time.now - 60).httpdate + resource = @http.resource(uri_for_code(200, "Expire" => in_the_past)) + + resp = resource.get + resp.should be_expired + + resp = resource.get + resp.should be_ok + resp.should be_authoritative + end + end + + describe "Not Modified responses" do + it "should replace the 304 with whats in the cache" do + now = Time.now.httpdate + + resource = @http.resource(uri_plus_params('http://localhost:3000/cached', + "modified" => now)) + + resp = resource.get + resp.should be_authoritative + body = resp.body + + resp = resource.get("Cache-Control" => "max-age=0") + resp.should be_authoritative + resp.body.should == body + end + + it "should merge the headers" + end + + describe "cache invalidation" do + + end + + end + +end + diff --git a/spec/acceptance/header_spec.rb b/spec/acceptance/header_spec.rb new file mode 100644 index 0000000..8e9c84e --- /dev/null +++ b/spec/acceptance/header_spec.rb @@ -0,0 +1,12 @@ + +require File.dirname(__FILE__) + '/../spec_helper' +require 'resourceful' + +describe Resourceful do + + describe "manipulating headers" do + + end + +end + diff --git a/spec/acceptance/redirecting_spec.rb b/spec/acceptance/redirecting_spec.rb new file mode 100644 index 0000000..a168ed8 --- /dev/null +++ b/spec/acceptance/redirecting_spec.rb @@ -0,0 +1,12 @@ + +require File.dirname(__FILE__) + '/../spec_helper' +require 'resourceful' + +describe Resourceful do + + describe "redirects" do + + end + +end + diff --git a/spec/acceptance/resource_spec.rb b/spec/acceptance/resource_spec.rb index ba12ce6..edabc9a 100644 --- a/spec/acceptance/resource_spec.rb +++ b/spec/acceptance/resource_spec.rb @@ -18,7 +18,6 @@ it 'should set the user agent string on the default header' do @resource.default_header.should have_key('User-Agent') @resource.default_header['User-Agent'].should == Resourceful::RESOURCEFUL_USER_AGENT_TOKEN - end describe "GET" do diff --git a/spec/acceptance_spec.rb b/spec/acceptance_spec.rb index db42f1e..912a8f3 100644 --- a/spec/acceptance_spec.rb +++ b/spec/acceptance_spec.rb @@ -12,16 +12,6 @@ @accessor = Resourceful::HttpAccessor.new end - it 'should #get a resource, and return a response object' do - resource = @accessor.resource('http://localhost:3000/get') - resp = resource.get - resp.should be_instance_of(Resourceful::Response) - resp.code.should == 200 - resp.body.should == 'Hello, world!' - resp.header.should be_instance_of(Resourceful::Header) - resp.header['Content-Type'].should == ['text/plain'] - end - it 'should set additional headers on the #get' do resource = @accessor.resource('http://localhost:3000/echo_header') resp = resource.get(:foo => :bar) @@ -30,36 +20,6 @@ resp.body.should =~ /"HTTP_FOO"=>"bar"/ end - it 'should #post a resource, and return the response' do - resource = @accessor.resource('http://localhost:3000/post') - resp = resource.post('Hello world from POST', :content_type => 'text/plain') - resp.should be_instance_of(Resourceful::Response) - resp.code.should == 201 - resp.body.should == 'Hello world from POST' - resp.header.should be_instance_of(Resourceful::Header) - resp.header['Content-Type'].should == ['text/plain'] - end - - it 'should #put a resource, and return the response' do - resource = @accessor.resource('http://localhost:3000/put') - resp = resource.put('Hello world from PUT', :content_type => 'text/plain') - resp.should be_instance_of(Resourceful::Response) - resp.code.should == 200 - resp.body.should == 'Hello world from PUT' - resp.header.should be_instance_of(Resourceful::Header) - resp.header['Content-Type'].should == ['text/plain'] - end - - it 'should #delete a resource, and return a response' do - resource = @accessor.resource('http://localhost:3000/delete') - resp = resource.delete - resp.should be_instance_of(Resourceful::Response) - resp.code.should == 200 - resp.body.should == 'KABOOM!' - resp.header.should be_instance_of(Resourceful::Header) - resp.header['Content-Type'].should == ['text/plain'] - end - it 'should take an optional default header for reads' do resource = @accessor.resource('http://localhost:3000/echo_header', :foo => :bar) resp = resource.get diff --git a/spec/simple_sinatra_server.rb b/spec/simple_sinatra_server.rb index dddac38..168c5d8 100644 --- a/spec/simple_sinatra_server.rb +++ b/spec/simple_sinatra_server.rb @@ -1,19 +1,21 @@ require 'sinatra' -Sinatra::Default.set( - :environment => :test, - :run => false, - :raise_errors => true, - :logging => false -) - def any(path, opts={}, &blk) %w[head get post put delete].each do |verb| send verb, path, opts, &blk end end +def set_request_params_as_response_header! + params.each { |k,v| response[k] = v } +end + +def set_request_header_in_body! + response['Content-Type'] ||= "application/yaml" + request.env.to_yaml +end + get '/' do "Hello, world!" end @@ -38,24 +40,29 @@ def any(path, opts={}, &blk) # Responds with the response code in the url any '/code/:code' do status params[:code] - params[:code] + set_request_params_as_response_header! + set_request_header_in_body! end # Sets the response header from the query string, and # dumps the request header into the body as yaml for inspection any '/header' do - params.each { |k,v| response[k] = v } - response['Content-Type'] ||= "application/yaml" - request.env.to_yaml + set_request_params_as_response_header! + set_request_header_in_body! end # Takes a modified=httpdate as a query param, and a If-Modified-Since header, # and responds 304 if they're the same get '/cached' do - modtime = params[:modified] - imstime = request.env['If-Modified-Since'] + set_request_params_as_response_header! + set_request_header_in_body! + + response['Last-Modified'] = params[:modified] - if modtime == imstime + modtime = params[:modified] + imstime = request.env['HTTP_IF_MODIFIED_SINCE'] + + if modtime && imstime && modtime == imstime status 304 end end @@ -64,23 +71,4 @@ def any(path, opts={}, &blk) -require 'thin' -# Only do this shit once, no matter how many times its #require'd -unless @server - # Spawn the server in another process - @server = fork do - Thin::Logging.silent = true - #Thin::Logging.debug = true - - Thin::Server.start(Sinatra::Application) - end - - # Kill the server process when rspec finishes - at_exit { Process.kill("TERM", @server) } - - # Give the app a change to initialize - $stderr.puts "Waiting for thin to initialize..." - sleep 0.2 -end - diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b42b176..3caafba 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,4 +7,31 @@ $LOAD_PATH << Pathname(__FILE__).dirname + "../lib" require 'resourceful' -require Pathname(__FILE__).dirname + 'simple_sinatra_server' +# Only do this shit once, no matter how many times its #require'd +unless @server + require 'thin' + + # Spawn the server in another process + @server = fork do + Thin::Logging.silent = true + #Thin::Logging.debug = true + + require Pathname(__FILE__).dirname + 'simple_sinatra_server' + Sinatra::Default.set( + :environment => :test, + :run => false, + :raise_errors => true, + :logging => ENV["SPEC_LOGGING"] || false + ) + + Thin::Server.start(Sinatra::Application) + end + + # Kill the server process when rspec finishes + at_exit { Process.kill("TERM", @server) } + + # Give the app a change to initialize + $stderr.puts "Waiting for thin to initialize..." + sleep 0.2 +end +