From 7f749a97ec2765140d0a54ba11be5f0366effd9b Mon Sep 17 00:00:00 2001 From: Aleksey V Zapparov Date: Tue, 18 Mar 2014 19:17:57 +0100 Subject: [PATCH 01/39] Remove obsolete BodyDelegator --- lib/http/response.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/response.rb b/lib/http/response.rb index 7fa4e794..44195f07 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -102,7 +102,7 @@ def parse(as = nil) # Inspect a response def inspect - "#<#{self.class}/#{@version} #{code} #{reason} #{headers.to_h.inspect}>" + "#<#{self.class}/#{@version} #{status} #{reason} headers=#{headers.inspect}>" end end end From 0900813520d9595f39b0f6be95ec8418cbd34d26 Mon Sep 17 00:00:00 2001 From: Aleksey V Zapparov Date: Tue, 18 Mar 2014 19:18:59 +0100 Subject: [PATCH 02/39] Bump up YARDStick threshold --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 33fdc6f4..ef88879c 100644 --- a/Rakefile +++ b/Rakefile @@ -23,7 +23,7 @@ end require "yardstick/rake/verify" Yardstick::Rake::Verify.new do |verify| verify.require_exact_threshold = false - verify.threshold = 58 + verify.threshold = 58.1 end task :default => [:spec, :rubocop, :verify_measurements] From 0b4c444580403ba6db9e81f9848073467ef7beb1 Mon Sep 17 00:00:00 2001 From: Hendrik Beskow Date: Tue, 18 Mar 2014 09:41:14 +0100 Subject: [PATCH 03/39] HTTP Caching --- lib/http/cache.rb | 206 ++++++++++++++++++++++++++++++ lib/http/cache/collection.rb | 54 ++++++++ lib/http/cache/in_memory_cache.rb | 33 +++++ lib/http/cache/redis_cache.rb | 52 ++++++++ lib/http/chainable.rb | 4 + lib/http/client.rb | 12 ++ lib/http/errors.rb | 3 + lib/http/options.rb | 25 +++- lib/http/request.rb | 8 ++ lib/http/response.rb | 5 +- 10 files changed, 400 insertions(+), 2 deletions(-) create mode 100644 lib/http/cache.rb create mode 100644 lib/http/cache/collection.rb create mode 100644 lib/http/cache/in_memory_cache.rb create mode 100644 lib/http/cache/redis_cache.rb diff --git a/lib/http/cache.rb b/lib/http/cache.rb new file mode 100644 index 00000000..41874961 --- /dev/null +++ b/lib/http/cache.rb @@ -0,0 +1,206 @@ +require 'time' + +module HTTP + class Cache + CACHEABLE_METHODS = [:get, :head].freeze + INVALIDATING_METHODS = [:post, :put, :delete].freeze + CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410].freeze + ALLOWED_CACHE_MODES = [:public, :private].freeze + + class CacheModeError < CacheError; end + + attr_reader :request, :response + + def initialize(options) + @options = options + unless ALLOWED_CACHE_MODES.include?(@options.cache[:mode]) + fail CacheModeError, "Invalid cache_mode #{options.cache[:mode]} supplied" + end + @cache_mode = @options.cache[:mode] + @cache_adapter = @options.cache[:adapter] + end + + def perform_request(request) + @request = request + + if @response = @cached_response = cache_lookup + if forces_cache_deletion?(request) + invalidate_cache + elsif needs_revalidation? + set_validation_headers! + else + @cached_response + end + else + nil + end + end + + def cache_lookup + @cache_adapter.lookup(request) unless skip_cache? + end + + def forces_cache_deletion?(re) + re.headers['Cache-Control'] && re.headers['Cache-Control'].include?('no-store') + end + + def needs_revalidation? + return true if forces_revalidation? + return true if stale? + return true if max_age && current_age > max_age + return true if must_be_revalidated? + false + end + + def forces_revalidation? + max_age == 0 || skip_cache? + end + + def max_age + if request.headers['Cache-Control'] && request.headers['Cache-Control'].include?('max-age') + request.headers['Cache-Control'].split(',').grep(/max-age/).first.split('=').last.to_i + end + end + + def skip_cache? + return true unless CACHEABLE_METHODS.include?(request.verb) + return false unless request.headers['Cache-Control'] + request.headers['Cache-Control'].include?('no-cache') + end + + # Algo from https://tools.ietf.org/html/rfc2616#section-13.2.3 + def current_age + now = Time.now + age_value = response.headers['Age'].to_i + date_value = Time.httpdate(response.headers['Date']) + + apparent_age = [0, response.response_time - date_value].max + corrected_received_age = [apparent_age, age_value].max + response_delay = response.response_time - response.request_time + corrected_initial_age = corrected_received_age + response_delay + resident_time = now - response.response_time + corrected_initial_age + resident_time + end + + def set_validation_headers! + if response.headers['Etag'] + request.headers['If-None-Match'] = response.headers['Etag'] + end + if response.headers['Last-Modified'] + request.headers['If-Modified-Since'] = response.headers['Last-Modified'] + end + request.headers['Cache-Control'] = 'max-age=0' if must_be_revalidated? + nil + end + + def perform_response(response) + @response = response + response.request_time = request.request_time + response.authoritative = true + # RFC2618 - 14.18 : A received message that does not have a Date header + # field MUST be assigned one by the recipient if the message will be cached + # by that recipient. + response.headers['Date'] ||= response.response_time.httpdate + + if @cached_response + if forces_cache_deletion?(response) + invalidate_cache + elsif response.reason == 'Not Modified' + revalidate_response! + end + end + + if request_cacheable? && response_cacheable? + store_in_cache + elsif invalidates_cache? + invalidate_cache + end + end + + def revalidate_response! + @cached_response.headers.merge!(response.headers) + @cached_response.request_time = response.request_time + @cached_response.response_time = response.response_time + @cached_response.authoritative = true + @response = @cached_response + end + + def request_cacheable? + return false unless response.status.between?(200, 299) + return false unless CACHEABLE_METHODS.include?(request.verb) + return false if request.headers['Cache-Control'] && request.headers['Cache-Control'].include?('no-store') + true + end + + def response_cacheable? + return @cacheable if @cacheable + + if CACHEABLE_RESPONSE_CODES.include?(response.code) + @cacheable = true + + if response.headers['Cache-Control'] + @cacheable = :public if response.headers['Cache-Control'].include?('public') + @cacheable = :private if response.headers['Cache-Control'].include?('private') + @cacheable = false if response.headers['Cache-Control'].include?('no-cache') + @cacheable = false if response.headers['Cache-Control'].include?('no-store') + end + + # A Vary header field-value of "*" always fails to match + # and subsequent requests on that resource can only be properly interpreted by the origin server. + @cacheable = false if response.headers['Vary'] && response.headers['Vary'].include?('*') + else + @cacheable = false + end + + unless @cacheable == true + if @cacheable == @cache_mode + @cacheable = true + else + @cacheable = false + end + end + + @cacheable + end + + def store_in_cache + @cache_adapter.store(request, response) + end + + def invalidates_cache? + INVALIDATING_METHODS.include?(request.verb) + end + + def invalidate_cache + @cache_adapter.invalidate(request.uri) + nil + end + + def expired? + if response.headers['Cache-Control'] && m_age_str = response.headers['Cache-Control'].match(/max-age=(\d+)/) + current_age > m_age_str[1].to_i + elsif response.headers['Expires'] + begin + Time.httpdate(response.headers['Expires']) < Time.now + rescue ArgumentError + # Some servers only send a "Expire: -1" header which must be treated as expired + true + end + else + false + end + end + + def stale? + return true if expired? + return false unless response.headers['Cache-Control'] + return true if response.headers['Cache-Control'].match(/must-revalidate|no-cache/) + + false + end + + def must_be_revalidated? + response.headers['Cache-Control'] && response.headers['Cache-Control'].include?('must-revalidate') + end + end +end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb new file mode 100644 index 00000000..02968968 --- /dev/null +++ b/lib/http/cache/collection.rb @@ -0,0 +1,54 @@ +require 'http/headers' + +module HTTP + class Cache + class CacheEntryCollection + include Enumerable + + def initialize + @entries = [] + end + + def each(&block) + @entries.each(&block) + end + + def [](request) + entry = find { |entry| entry.valid_for?(request) } + entry.response if entry + end + + def []=(request, response) + @entries.delete_if { |entry| entry.valid_for?(request) } + @entries << CacheEntry.new(request, response) + response + end + end + + class CacheEntry + attr_reader :request, :response + + def initialize(request, response) + @request, @response = request, response + end + + def valid_for?(request) + request.uri == @request.uri and + select_request_headers.all? do |key, value| + request.headers[key] == value + end + end + + def select_request_headers + headers = HTTP::Headers.new + + @response.headers['Vary'].split(',').each do |name| + name.strip! + headers[name] = @request.headers[name] if @request.headers[name] + end if @response.headers['Vary'] + + headers.to_h + end + end + end +end diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb new file mode 100644 index 00000000..aec6f98b --- /dev/null +++ b/lib/http/cache/in_memory_cache.rb @@ -0,0 +1,33 @@ +require 'thread' +require 'http/cache/collection' + +module HTTP + class Cache + class InMemoryCache + def initialize + @mutex = Mutex.new + @collection = Hash.new{ |h,k| h[k] = CacheEntryCollection.new } + end + + def lookup(request) + @mutex.synchronize do + response = @collection[request.uri.to_s][request] + response.authoritative = false if response + response + end + end + + def store(request, response) + @mutex.synchronize do + @collection[request.uri.to_s][request] = response + end + end + + def invalidate(uri) + @mutex.synchronize do + @collection.delete(uri.to_s) + end + end + end + end +end diff --git a/lib/http/cache/redis_cache.rb b/lib/http/cache/redis_cache.rb new file mode 100644 index 00000000..69856c3f --- /dev/null +++ b/lib/http/cache/redis_cache.rb @@ -0,0 +1,52 @@ +# Currently broken +# require 'connection_pool' +# require 'redis' +# require 'http/cache/collection' +# require 'digest/sha2' + +# module HTTP +# class Cache +# class RedisCache +# def initialize(options = {}) +# @redis = ConnectionPool.new(size: (options[:pool_size] ||5), +# timeout: (options[:pool_timeout] ||5)) { Redis.new(options) } +# end + +# def lookup(request) +# response = cache_entries_for(request)[request] +# response.authoritative = false if response +# response +# end + +# def store(request, response) +# entries = cache_entries_for(request) + +# @redis.with do |redis| +# entries[request] = response +# redis.set(uri_hash(request.uri), Marshal.dump(entries)) +# end +# end + +# def invalidate(uri) +# @redis.with do |redis| +# redis.del(uri_hash(uri)) +# end +# end + +# private +# def cache_entries_for(request) +# @redis.with do |redis| +# if entries = redis.get(uri_hash(request.uri)) +# Marshal.load(entries) +# else +# CacheEntryCollection.new +# end +# end +# end + +# def uri_hash(uri) +# Digest::SHA2.hexdigest(uri.to_s) +# end +# end +# end +# end diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb index 835065be..84b58d4d 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -107,6 +107,10 @@ def follow(opts = true) # @see #follow alias_method :with_follow, :follow + def with_cache(cache) + branch default_options.with_cache(cache) + end + # Make a request with the given headers # @param headers def with_headers(headers) diff --git a/lib/http/client.rb b/lib/http/client.rb index f1899224..def65ee3 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -3,6 +3,7 @@ require "http/form_data" require "http/options" require "http/redirector" +require "http/cache" module HTTP # Clients make requests and receive responses @@ -42,6 +43,13 @@ def request(verb, uri, opts = {}) # Perform a single (no follow) HTTP request def perform(req, options) + if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) + cache = Cache.new(options) + if res = cache.perform_request(req) + return res + end + end + # finish previous response if client was re-used # TODO: this is pretty wrong, as socket shoud be part of response # connection, so that re-use of client will not break multiple @@ -63,6 +71,10 @@ def perform(req, options) finish_response if :head == req.verb + if Cache::ALLOWED_CACHE_MODES.include?(options.cache_mode) + cache.perform_response(res) + end + res end diff --git a/lib/http/errors.rb b/lib/http/errors.rb index af08b313..6a288489 100644 --- a/lib/http/errors.rb +++ b/lib/http/errors.rb @@ -11,6 +11,9 @@ class ResponseError < Error; end # Requested to do something when we're in the wrong state class StateError < ResponseError; end + # Generic Cache error + class CacheError < Error; end + # Header name is invalid class InvalidHeaderNameError < Error; end end diff --git a/lib/http/options.rb b/lib/http/options.rb index 33d6655f..e7592934 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -1,6 +1,8 @@ require "http/headers" require "openssl" require "socket" +require 'http/cache' +require 'http/cache/in_memory_cache' module HTTP class Options @@ -34,13 +36,19 @@ class Options # Follow redirects attr_accessor :follow + # Cache, needs a hash. See @default_cache. Valid modes are: false, :public, :private + attr_accessor :cache + protected :response=, :headers=, :proxy=, :params=, :form=, :json=, :follow= @default_socket_class = TCPSocket @default_ssl_socket_class = OpenSSL::SSL::SSLSocket + @default_cache = {:mode => false, :adapter => HTTP::Cache::InMemoryCache.new} + class << self attr_accessor :default_socket_class, :default_ssl_socket_class + attr_accessor :default_cache def new(options = {}) return options if options.is_a?(self) @@ -62,6 +70,8 @@ def initialize(options = {}) @socket_class = options[:socket_class] || self.class.default_socket_class @ssl_socket_class = options[:ssl_socket_class] || self.class.default_ssl_socket_class @ssl_context = options[:ssl_context] + + @cache = options[:cache] || self.class.default_cache end def with_headers(headers) @@ -78,6 +88,18 @@ def with_#{method_name}(value) RUBY end + def with_cache(cache) + my_cache = cache.dup + + unless my_cache.is_a?(Hash) + my_cache = self.class.default_cache.merge({:mode => my_cache}) + end + + dup do |opts| + opts.cache = my_cache + end + end + def [](option) send(option) rescue nil end @@ -111,7 +133,8 @@ def to_hash :follow => follow, :socket_class => socket_class, :ssl_socket_class => ssl_socket_class, - :ssl_context => ssl_context + :ssl_context => ssl_context, + :cache => cache } end diff --git a/lib/http/request.rb b/lib/http/request.rb index 74e0c275..f0b233ac 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -4,6 +4,7 @@ require "http/version" require "base64" require "uri" +require "time" module HTTP class Request @@ -65,6 +66,10 @@ def __method__(*args) attr_reader :uri attr_reader :proxy, :body, :version + # "request_time" as per RFC 2616 + # https://tools.ietf.org/html/rfc2616#section-13.2.3 + attr_accessor :request_time + # :nodoc: def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") # rubocop:disable ParameterLists @verb = verb.to_s.downcase.to_sym @@ -80,6 +85,9 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") @headers["Host"] ||= default_host @headers["User-Agent"] ||= USER_AGENT + now = Time.now + @request_time = now + @headers['Date'] = now.httpdate end # Returns new Request with updated uri diff --git a/lib/http/response.rb b/lib/http/response.rb index 44195f07..e6f433e7 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -4,6 +4,7 @@ require "http/content_type" require "http/mime_type" require "http/response/status" +require "time" module HTTP class Response @@ -26,12 +27,14 @@ class Response # @return [URI, nil] attr_reader :uri + attr_reader :response_time + attr_accessor :authoritative, :request_time def initialize(status, version, headers, body, uri = nil) # rubocop:disable ParameterLists @version, @body, @uri = version, body, uri - @status = HTTP::Response::Status.new status @headers = HTTP::Headers.coerce(headers || {}) + @response_time = Time.now end # @!method reason From 4310b9ea62683c0548d8ece5c77c7d0ffd1d35b7 Mon Sep 17 00:00:00 2001 From: Hendrik Beskow Date: Wed, 19 Mar 2014 11:39:06 +0100 Subject: [PATCH 04/39] Typo --- lib/http/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/client.rb b/lib/http/client.rb index def65ee3..43db7e60 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -71,7 +71,7 @@ def perform(req, options) finish_response if :head == req.verb - if Cache::ALLOWED_CACHE_MODES.include?(options.cache_mode) + if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) cache.perform_response(res) end From 2533e015b991acfe37e280d4652b0daa2725993f Mon Sep 17 00:00:00 2001 From: Hendrik Beskow Date: Wed, 19 Mar 2014 14:48:15 +0100 Subject: [PATCH 05/39] Small fixups --- lib/http/cache.rb | 58 ++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 41874961..763ca595 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -12,12 +12,11 @@ class CacheModeError < CacheError; end attr_reader :request, :response def initialize(options) - @options = options - unless ALLOWED_CACHE_MODES.include?(@options.cache[:mode]) + unless ALLOWED_CACHE_MODES.include?(options.cache[:mode]) fail CacheModeError, "Invalid cache_mode #{options.cache[:mode]} supplied" end - @cache_mode = @options.cache[:mode] - @cache_adapter = @options.cache[:adapter] + @cache_mode = options.cache[:mode] + @cache_adapter = options.cache[:adapter] end def perform_request(request) @@ -36,6 +35,32 @@ def perform_request(request) end end + def perform_response(response) + @response = response + response.request_time = request.request_time + response.authoritative = true + # RFC2618 - 14.18 : A received message that does not have a Date header + # field MUST be assigned one by the recipient if the message will be cached + # by that recipient. + response.headers['Date'] ||= response.response_time.httpdate + + if @cached_response + if forces_cache_deletion?(response) + invalidate_cache + elsif response.reason == 'Not Modified' + revalidate_response! + end + end + + if request_cacheable? && response_cacheable? + store_in_cache + elsif invalidates_cache? + invalidate_cache + end + end + + private + def cache_lookup @cache_adapter.lookup(request) unless skip_cache? end @@ -93,30 +118,6 @@ def set_validation_headers! nil end - def perform_response(response) - @response = response - response.request_time = request.request_time - response.authoritative = true - # RFC2618 - 14.18 : A received message that does not have a Date header - # field MUST be assigned one by the recipient if the message will be cached - # by that recipient. - response.headers['Date'] ||= response.response_time.httpdate - - if @cached_response - if forces_cache_deletion?(response) - invalidate_cache - elsif response.reason == 'Not Modified' - revalidate_response! - end - end - - if request_cacheable? && response_cacheable? - store_in_cache - elsif invalidates_cache? - invalidate_cache - end - end - def revalidate_response! @cached_response.headers.merge!(response.headers) @cached_response.request_time = response.request_time @@ -165,6 +166,7 @@ def response_cacheable? def store_in_cache @cache_adapter.store(request, response) + nil end def invalidates_cache? From e7f753b68ab1a02d18d0eb4423574efe611504a1 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 2 Feb 2015 08:05:05 -0700 Subject: [PATCH 06/39] Fix a couple of specs and remove some rubocop warnings --- lib/http/cache.rb | 56 ++++++++++++++--------------- lib/http/cache/collection.rb | 6 ++-- lib/http/cache/in_memory_cache.rb | 4 +-- lib/http/cache/redis_cache.rb | 8 ++--- lib/http/request.rb | 2 +- lib/http/response.rb | 2 +- spec/lib/http/options/merge_spec.rb | 4 ++- 7 files changed, 42 insertions(+), 40 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 763ca595..e53997de 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,4 +1,4 @@ -require 'time' +require "time" module HTTP class Cache @@ -42,12 +42,12 @@ def perform_response(response) # RFC2618 - 14.18 : A received message that does not have a Date header # field MUST be assigned one by the recipient if the message will be cached # by that recipient. - response.headers['Date'] ||= response.response_time.httpdate + response.headers["Date"] ||= response.response_time.httpdate if @cached_response if forces_cache_deletion?(response) invalidate_cache - elsif response.reason == 'Not Modified' + elsif response.reason == "Not Modified" revalidate_response! end end @@ -66,7 +66,7 @@ def cache_lookup end def forces_cache_deletion?(re) - re.headers['Cache-Control'] && re.headers['Cache-Control'].include?('no-store') + re.headers["Cache-Control"] && re.headers["Cache-Control"].include?("no-store") end def needs_revalidation? @@ -82,22 +82,22 @@ def forces_revalidation? end def max_age - if request.headers['Cache-Control'] && request.headers['Cache-Control'].include?('max-age') - request.headers['Cache-Control'].split(',').grep(/max-age/).first.split('=').last.to_i + if request.headers["Cache-Control"] && request.headers["Cache-Control"].include?("max-age") + request.headers["Cache-Control"].split(",").grep(/max-age/).first.split("=").last.to_i end end def skip_cache? return true unless CACHEABLE_METHODS.include?(request.verb) - return false unless request.headers['Cache-Control'] - request.headers['Cache-Control'].include?('no-cache') + return false unless request.headers["Cache-Control"] + request.headers["Cache-Control"].include?("no-cache") end # Algo from https://tools.ietf.org/html/rfc2616#section-13.2.3 def current_age now = Time.now - age_value = response.headers['Age'].to_i - date_value = Time.httpdate(response.headers['Date']) + age_value = response.headers["Age"].to_i + date_value = Time.httpdate(response.headers["Date"]) apparent_age = [0, response.response_time - date_value].max corrected_received_age = [apparent_age, age_value].max @@ -108,13 +108,13 @@ def current_age end def set_validation_headers! - if response.headers['Etag'] - request.headers['If-None-Match'] = response.headers['Etag'] + if response.headers["Etag"] + request.headers["If-None-Match"] = response.headers["Etag"] end - if response.headers['Last-Modified'] - request.headers['If-Modified-Since'] = response.headers['Last-Modified'] + if response.headers["Last-Modified"] + request.headers["If-Modified-Since"] = response.headers["Last-Modified"] end - request.headers['Cache-Control'] = 'max-age=0' if must_be_revalidated? + request.headers["Cache-Control"] = "max-age=0" if must_be_revalidated? nil end @@ -129,7 +129,7 @@ def revalidate_response! def request_cacheable? return false unless response.status.between?(200, 299) return false unless CACHEABLE_METHODS.include?(request.verb) - return false if request.headers['Cache-Control'] && request.headers['Cache-Control'].include?('no-store') + return false if request.headers["Cache-Control"] && request.headers["Cache-Control"].include?("no-store") true end @@ -139,16 +139,16 @@ def response_cacheable? if CACHEABLE_RESPONSE_CODES.include?(response.code) @cacheable = true - if response.headers['Cache-Control'] - @cacheable = :public if response.headers['Cache-Control'].include?('public') - @cacheable = :private if response.headers['Cache-Control'].include?('private') - @cacheable = false if response.headers['Cache-Control'].include?('no-cache') - @cacheable = false if response.headers['Cache-Control'].include?('no-store') + if response.headers["Cache-Control"] + @cacheable = :public if response.headers["Cache-Control"].include?("public") + @cacheable = :private if response.headers["Cache-Control"].include?("private") + @cacheable = false if response.headers["Cache-Control"].include?("no-cache") + @cacheable = false if response.headers["Cache-Control"].include?("no-store") end # A Vary header field-value of "*" always fails to match # and subsequent requests on that resource can only be properly interpreted by the origin server. - @cacheable = false if response.headers['Vary'] && response.headers['Vary'].include?('*') + @cacheable = false if response.headers["Vary"] && response.headers["Vary"].include?("*") else @cacheable = false end @@ -179,11 +179,11 @@ def invalidate_cache end def expired? - if response.headers['Cache-Control'] && m_age_str = response.headers['Cache-Control'].match(/max-age=(\d+)/) + if response.headers["Cache-Control"] && m_age_str = response.headers["Cache-Control"].match(/max-age=(\d+)/) current_age > m_age_str[1].to_i - elsif response.headers['Expires'] + elsif response.headers["Expires"] begin - Time.httpdate(response.headers['Expires']) < Time.now + Time.httpdate(response.headers["Expires"]) < Time.now rescue ArgumentError # Some servers only send a "Expire: -1" header which must be treated as expired true @@ -195,14 +195,14 @@ def expired? def stale? return true if expired? - return false unless response.headers['Cache-Control'] - return true if response.headers['Cache-Control'].match(/must-revalidate|no-cache/) + return false unless response.headers["Cache-Control"] + return true if response.headers["Cache-Control"].match(/must-revalidate|no-cache/) false end def must_be_revalidated? - response.headers['Cache-Control'] && response.headers['Cache-Control'].include?('must-revalidate') + response.headers["Cache-Control"] && response.headers["Cache-Control"].include?("must-revalidate") end end end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index 02968968..c427a957 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -1,4 +1,4 @@ -require 'http/headers' +require "http/headers" module HTTP class Cache @@ -42,10 +42,10 @@ def valid_for?(request) def select_request_headers headers = HTTP::Headers.new - @response.headers['Vary'].split(',').each do |name| + @response.headers["Vary"].split(",").each do |name| name.strip! headers[name] = @request.headers[name] if @request.headers[name] - end if @response.headers['Vary'] + end if @response.headers["Vary"] headers.to_h end diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb index aec6f98b..71036e00 100644 --- a/lib/http/cache/in_memory_cache.rb +++ b/lib/http/cache/in_memory_cache.rb @@ -1,5 +1,5 @@ -require 'thread' -require 'http/cache/collection' +require "thread" +require "http/cache/collection" module HTTP class Cache diff --git a/lib/http/cache/redis_cache.rb b/lib/http/cache/redis_cache.rb index 69856c3f..2428df60 100644 --- a/lib/http/cache/redis_cache.rb +++ b/lib/http/cache/redis_cache.rb @@ -1,8 +1,8 @@ # Currently broken -# require 'connection_pool' -# require 'redis' -# require 'http/cache/collection' -# require 'digest/sha2' +# require "connection_pool" +# require "redis" +# require "http/cache/collection" +# require "digest/sha2" # module HTTP # class Cache diff --git a/lib/http/request.rb b/lib/http/request.rb index f0b233ac..5ed27cc9 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -87,7 +87,7 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") @headers["User-Agent"] ||= USER_AGENT now = Time.now @request_time = now - @headers['Date'] = now.httpdate + @headers["Date"] = now.httpdate end # Returns new Request with updated uri diff --git a/lib/http/response.rb b/lib/http/response.rb index e6f433e7..ba26335f 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -105,7 +105,7 @@ def parse(as = nil) # Inspect a response def inspect - "#<#{self.class}/#{@version} #{status} #{reason} headers=#{headers.inspect}>" + "#<#{self.class}/#{@version} #{code} #{reason} #{headers.to_h.inspect}>" end end end diff --git a/spec/lib/http/options/merge_spec.rb b/spec/lib/http/options/merge_spec.rb index 820006bd..ddf91b7d 100644 --- a/spec/lib/http/options/merge_spec.rb +++ b/spec/lib/http/options/merge_spec.rb @@ -1,3 +1,4 @@ + RSpec.describe HTTP::Options, "merge" do let(:opts) { HTTP::Options.new } @@ -44,6 +45,7 @@ :follow => nil, :socket_class => described_class.default_socket_class, :ssl_socket_class => described_class.default_ssl_socket_class, - :ssl_context => nil) + :ssl_context => nil, + :cache => described_class.default_cache) end end From c4f1e456f41d1a1d100e93fbd4358066ea8329ca Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 2 Feb 2015 17:26:37 -0700 Subject: [PATCH 07/39] basic specs for caching --- lib/http/cache.rb | 33 ++++-- lib/http/client.rb | 8 +- lib/http/response.rb | 3 +- spec/lib/http/cache_spec.rb | 210 +++++++++++++++++++++++++++++++++++ spec/lib/http/client_spec.rb | 14 ++- spec/lib/http_spec.rb | 14 +++ 6 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 spec/lib/http/cache_spec.rb diff --git a/lib/http/cache.rb b/lib/http/cache.rb index e53997de..75d4282a 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -17,25 +17,40 @@ def initialize(options) end @cache_mode = options.cache[:mode] @cache_adapter = options.cache[:adapter] + @cacheable = nil end - def perform_request(request) + # @return [Response] a cached response that is valid for the request or + # the result of executing the provided block. + def perform(request, options, &request_performer) @request = request - if @response = @cached_response = cache_lookup - if forces_cache_deletion?(request) - invalidate_cache - elsif needs_revalidation? + if forces_cache_deletion?(request) + invalidate_cache + + elsif @response = @cached_response = cache_lookup + if needs_revalidation? set_validation_headers! else - @cached_response + return @cached_response end + end + # cache miss! + + response = yield request, options + + register(response) + + if response.status == 304 + @cached_response else - nil + response end end - def perform_response(response) + protected + + def register(response) @response = response response.request_time = request.request_time response.authoritative = true @@ -59,8 +74,6 @@ def perform_response(response) end end - private - def cache_lookup @cache_adapter.lookup(request) unless skip_cache? end diff --git a/lib/http/client.rb b/lib/http/client.rb index 43db7e60..797885c4 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -45,11 +45,15 @@ def request(verb, uri, opts = {}) def perform(req, options) if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) cache = Cache.new(options) - if res = cache.perform_request(req) - return res + cache.perform(req, options) do |req, options| + make_request(req, options) end + else + make_request(req, options) end + end + def make_request(req, options) # finish previous response if client was re-used # TODO: this is pretty wrong, as socket shoud be part of response # connection, so that re-use of client will not break multiple diff --git a/lib/http/response.rb b/lib/http/response.rb index ba26335f..e049c8d1 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -27,8 +27,7 @@ class Response # @return [URI, nil] attr_reader :uri - attr_reader :response_time - attr_accessor :authoritative, :request_time + attr_accessor :authoritative, :request_time, :response_time def initialize(status, version, headers, body, uri = nil) # rubocop:disable ParameterLists @version, @body, @uri = version, body, uri diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb new file mode 100644 index 00000000..569cac63 --- /dev/null +++ b/spec/lib/http/cache_spec.rb @@ -0,0 +1,210 @@ +require "support/dummy_server" + +RSpec.describe HTTP::Cache do + describe "creation" do + subject { described_class } + + it "allows private mode" do + expect(subject.new(options(mode: :private, adapter: cache_adapter))) + .to be_kind_of HTTP::Cache + end + + it "allows public mode" do + expect(subject.new(options(mode: :public, adapter: cache_adapter))) + .to be_kind_of HTTP::Cache + end + end + + let(:opts) { options(mode: :private, adapter: cache_adapter) } + subject { described_class.new(opts) } + + describe "#perform" do + it "calls request_performer block when cache miss" do + expect { |b| subject.perform(request, opts) {|*args| + b.to_proc.call(*args) + origin_response } + }.to yield_with_args(request, opts) + end + + it "does not call request_performer block when cache it" do + allow(cache_adapter).to receive(:lookup).and_return(origin_response) + + expect { |b| subject.perform(request, opts, &b) }.not_to yield_control + end + end + + context "empty cache, cacheable request, cacheable response" do + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "tries to lookup request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "stores response in cache" do + expect(cache_adapter).to have_received(:store).with(request, origin_response) + end + end + + context "cache by-passing request, cacheable response" do + let(:request) { HTTP::Request.new(:get, "http://example.com/", + {"Cache-Control" => "no-cache"}) } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "doesn't lookup request" do + expect(cache_adapter).not_to have_received(:lookup) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "stores response in cache" do + expect(cache_adapter).to have_received(:store).with(request, origin_response) + end + end + + context "empty cache, cacheable request, 'no-store' response" do + let(:origin_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "no-cache"}, + "") } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "tries to lookup request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "doesn't store response in cache" do + expect(cache_adapter).not_to have_received(:store) + end + + end + + context "empty cache, cacheable request, 'no-cache' response" do + let(:origin_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "no-store"}, + "") } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "tries to lookup request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "doesn't store response in cache" do + expect(cache_adapter).not_to have_received(:store) + end + end + + context "empty cache, cacheable request, 'no-store' response" do + let(:origin_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "no-store"}, + "") } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "tries to lookup request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "doesn't store response in cache" do + expect(cache_adapter).not_to have_received(:store) + end + end + + context "warm cache, cacheable request, cacheable response" do + let(:cached_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "private"}, + "") } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "lookups request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns cached response" do + expect(response).to eq cached_response + end + end + + context "stale cache, cacheable request, cacheable response" do + let(:cached_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "private, max-age=1", + "Date" => (Time.now - 2).httpdate}, + "") + .tap{|x| x.request_time = (Time.now - 2)} } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "lookups request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "returns origin servers response" do + expect(response).to eq origin_response + end + + it "stores fresh response in cache" do + expect(cache_adapter).to have_received(:store).with(request, origin_response) + end + end + + context "stale cache, cacheable request, not modified response" do + let(:cached_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "private, max-age=1", + "Etag" => "foo", + "Date" => (Time.now - 2).httpdate}, + "") + .tap{|x| x.request_time = (Time.now - 2)} } + let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } + let!(:response) { subject.perform(request, opts) { origin_response } } + + it "lookups request" do + expect(cache_adapter).to have_received(:lookup).with(request) + end + + it "modifies request with conditional request headers" do + subject.perform(request, opts) { response } + expect(request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] + expect(request.headers["If-Modified-Since"]) + .to eq cached_response.headers["Last-Modified"] + end + + it "returns cached servers response" do + expect(response).to eq cached_response + end + + it "updates the stored response in cache" do + expect(cache_adapter).to have_received(:store).with(request, cached_response) + end + end + + + # Background + + let(:cache_adapter) { double("cache_adapter", lookup: cached_response, store: nil) } + + let(:request) { HTTP::Request.new(:get, "http://example.com/") } + + let(:origin_response) { HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "private"}, + "") } + + let(:cached_response) { nil } # cold cache by default + + def options(cache_opts) + HTTP::Options.new(cache: cache_opts) + end +end diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 6c5586fe..5290cb86 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -4,7 +4,7 @@ run_server(:dummy) { DummyServer.new } StubbedClient = Class.new(HTTP::Client) do - def perform(request, options) + def make_request(request, options) stubs.fetch(request.uri.to_s) { super(request, options) } end @@ -71,6 +71,18 @@ def simple_response(body, status = 200) end end + describe "caching" do + it "returns cached responses if they exist" do + cached_response = simple_response("OK") + cache = double("cache", lookup: cached_response) + client = StubbedClient.new(:cache => {mode: :private, adapter: cache}).stub( + "http://example.com/" => simple_response("OK") + ) + + expect(client.get("http://example.com/")).to eq cached_response + end + end + describe "parsing params" do let(:client) { HTTP::Client.new } before { allow(client).to receive :perform } diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 860c9fa9..9099871f 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -155,4 +155,18 @@ .to match(/^Basic [A-Za-z0-9+\/]+=*$/) end end + + describe ".with_cache" do + it "sets cache options for valid mode" do + client = HTTP.with_cache "private" + expect(client.default_options[:cache]).to include mode: "private", adapter: HTTP::Cache::InMemoryCache + end + + it "sets cache options for valid mode and adapter" do + adapter = double("an_adapter") + client = HTTP.with_cache mode: :private, adapter: adapter + expect(client.default_options[:cache]).to include mode: :private, adapter: adapter + end + end + end From 75d8068e0b9c9604f2e4a1307096410881bcd193 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 15:35:46 -0700 Subject: [PATCH 08/39] extract some caching related helper objects and write specs --- lib/http/cache.rb | 205 +++--------------- lib/http/cache/cache_control.rb | 69 ++++++ lib/http/cache/collection.rb | 2 +- lib/http/cache/request_with_cache_behavior.rb | 74 +++++++ .../cache/response_with_cache_behavior.rb | 112 ++++++++++ lib/http/client.rb | 4 +- spec/lib/http/cache/cache_control_spec.rb | 126 +++++++++++ .../cache/request_with_cache_behavior_spec.rb | 141 ++++++++++++ .../response_with_cache_behavior_spec.rb | 188 ++++++++++++++++ spec/lib/http/cache_spec.rb | 58 +++-- spec/lib/http/client_spec.rb | 2 +- 11 files changed, 784 insertions(+), 197 deletions(-) create mode 100644 lib/http/cache/cache_control.rb create mode 100644 lib/http/cache/request_with_cache_behavior.rb create mode 100644 lib/http/cache/response_with_cache_behavior.rb create mode 100644 spec/lib/http/cache/cache_control_spec.rb create mode 100644 spec/lib/http/cache/request_with_cache_behavior_spec.rb create mode 100644 spec/lib/http/cache/response_with_cache_behavior_spec.rb diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 75d4282a..ee077e57 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,10 +1,10 @@ require "time" +require "http/cache/cache_control" +require "http/cache/response_with_cache_behavior" +require "http/cache/request_with_cache_behavior" module HTTP class Cache - CACHEABLE_METHODS = [:get, :head].freeze - INVALIDATING_METHODS = [:post, :put, :delete].freeze - CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410].freeze ALLOWED_CACHE_MODES = [:public, :private].freeze class CacheModeError < CacheError; end @@ -17,205 +17,64 @@ def initialize(options) end @cache_mode = options.cache[:mode] @cache_adapter = options.cache[:adapter] - @cacheable = nil end # @return [Response] a cached response that is valid for the request or # the result of executing the provided block. def perform(request, options, &request_performer) - @request = request + req = RequestWithCacheBehavior.coerce(request) - if forces_cache_deletion?(request) - invalidate_cache + if req.invalidates_cache? + invalidate_cache(req) - elsif @response = @cached_response = cache_lookup - if needs_revalidation? - set_validation_headers! - else - return @cached_response - end - end - # cache miss! - - response = yield request, options - - register(response) + elsif cached_resp = cache_lookup(req) + return cached_resp unless cached_resp.stale? - if response.status == 304 - @cached_response - else - response + req.set_validation_headers!(cached_resp) end - end - - protected - def register(response) - @response = response - response.request_time = request.request_time - response.authoritative = true - # RFC2618 - 14.18 : A received message that does not have a Date header - # field MUST be assigned one by the recipient if the message will be cached - # by that recipient. - response.headers["Date"] ||= response.response_time.httpdate - - if @cached_response - if forces_cache_deletion?(response) - invalidate_cache - elsif response.reason == "Not Modified" - revalidate_response! - end - end + # cache miss! Do this the hard way... + req.sent_at = Time.now + act_resp = ResponseWithCacheBehavior.coerce(yield(req, options)) - if request_cacheable? && response_cacheable? - store_in_cache - elsif invalidates_cache? - invalidate_cache - end - end + act_resp.received_at = Time.now + act_resp.requested_at = req.sent_at - def cache_lookup - @cache_adapter.lookup(request) unless skip_cache? - end + if act_resp.status.not_modified? && cached_resp + cached_resp.validated!(act_resp) + store_in_cache(req, cached_resp) + return cached_resp - def forces_cache_deletion?(re) - re.headers["Cache-Control"] && re.headers["Cache-Control"].include?("no-store") - end - - def needs_revalidation? - return true if forces_revalidation? - return true if stale? - return true if max_age && current_age > max_age - return true if must_be_revalidated? - false - end - - def forces_revalidation? - max_age == 0 || skip_cache? - end - - def max_age - if request.headers["Cache-Control"] && request.headers["Cache-Control"].include?("max-age") - request.headers["Cache-Control"].split(",").grep(/max-age/).first.split("=").last.to_i - end - end - - def skip_cache? - return true unless CACHEABLE_METHODS.include?(request.verb) - return false unless request.headers["Cache-Control"] - request.headers["Cache-Control"].include?("no-cache") - end - - # Algo from https://tools.ietf.org/html/rfc2616#section-13.2.3 - def current_age - now = Time.now - age_value = response.headers["Age"].to_i - date_value = Time.httpdate(response.headers["Date"]) - - apparent_age = [0, response.response_time - date_value].max - corrected_received_age = [apparent_age, age_value].max - response_delay = response.response_time - response.request_time - corrected_initial_age = corrected_received_age + response_delay - resident_time = now - response.response_time - corrected_initial_age + resident_time - end + elsif req.cacheable? && act_resp.cacheable? + store_in_cache(req, act_resp) + return act_resp - def set_validation_headers! - if response.headers["Etag"] - request.headers["If-None-Match"] = response.headers["Etag"] - end - if response.headers["Last-Modified"] - request.headers["If-Modified-Since"] = response.headers["Last-Modified"] + else + return act_resp end - request.headers["Cache-Control"] = "max-age=0" if must_be_revalidated? - nil - end - - def revalidate_response! - @cached_response.headers.merge!(response.headers) - @cached_response.request_time = response.request_time - @cached_response.response_time = response.response_time - @cached_response.authoritative = true - @response = @cached_response - end - - def request_cacheable? - return false unless response.status.between?(200, 299) - return false unless CACHEABLE_METHODS.include?(request.verb) - return false if request.headers["Cache-Control"] && request.headers["Cache-Control"].include?("no-store") - true end - def response_cacheable? - return @cacheable if @cacheable - - if CACHEABLE_RESPONSE_CODES.include?(response.code) - @cacheable = true + protected - if response.headers["Cache-Control"] - @cacheable = :public if response.headers["Cache-Control"].include?("public") - @cacheable = :private if response.headers["Cache-Control"].include?("private") - @cacheable = false if response.headers["Cache-Control"].include?("no-cache") - @cacheable = false if response.headers["Cache-Control"].include?("no-store") - end - # A Vary header field-value of "*" always fails to match - # and subsequent requests on that resource can only be properly interpreted by the origin server. - @cacheable = false if response.headers["Vary"] && response.headers["Vary"].include?("*") + def cache_lookup(request) + return nil if request.skips_cache? + c = @cache_adapter.lookup(request) + if c + ResponseWithCacheBehavior.coerce(c) else - @cacheable = false + nil end - - unless @cacheable == true - if @cacheable == @cache_mode - @cacheable = true - else - @cacheable = false - end - end - - @cacheable end - def store_in_cache + def store_in_cache(request, response) @cache_adapter.store(request, response) nil end - def invalidates_cache? - INVALIDATING_METHODS.include?(request.verb) - end - - def invalidate_cache + def invalidate_cache(request) @cache_adapter.invalidate(request.uri) - nil end - def expired? - if response.headers["Cache-Control"] && m_age_str = response.headers["Cache-Control"].match(/max-age=(\d+)/) - current_age > m_age_str[1].to_i - elsif response.headers["Expires"] - begin - Time.httpdate(response.headers["Expires"]) < Time.now - rescue ArgumentError - # Some servers only send a "Expire: -1" header which must be treated as expired - true - end - else - false - end - end - - def stale? - return true if expired? - return false unless response.headers["Cache-Control"] - return true if response.headers["Cache-Control"].match(/must-revalidate|no-cache/) - - false - end - - def must_be_revalidated? - response.headers["Cache-Control"] && response.headers["Cache-Control"].include?("must-revalidate") - end end end diff --git a/lib/http/cache/cache_control.rb b/lib/http/cache/cache_control.rb new file mode 100644 index 00000000..36cc53c8 --- /dev/null +++ b/lib/http/cache/cache_control.rb @@ -0,0 +1,69 @@ +module HTTP + class Cache + # Convenience methods around cache control headers. + class CacheControl + def initialize(message) + @headers = message.headers + end + + def forces_revalidation? + must_revalidate? || max_age == 0 + end + + def must_revalidate? + matches?(/\bmust-revalidate\b/i) + end + + def no_cache? + matches?(/\bno-cache\b/i) + end + + def no_store? + matches?(/\bno-store\b/i) + end + + def public? + matches?(/\bpublic\b/i) + end + + def private? + matches?(/\bprivate\b/i) + end + + def max_age + explicit_max_age || seconds_til_expires || Float::INFINITY + end + + def vary_star? + headers.get("Vary").any?{|v| "*" == v.strip } + end + + protected + + attr_reader :headers + + def matches?(pattern) + headers.get("Cache-Control").any?{|v| v =~ pattern } + end + + # --- + # Some servers send a "Expire: -1" header which must be treated as expired + def seconds_til_expires + headers.get("Expires") + .map{|e| Time.httpdate(e) rescue Time.at(0) } + .compact + .map{|e| e - Time.now} + .map{|a| a < 0 ? 0 : a} # age is 0 if it is expired + .max + end + + def explicit_max_age + headers.get("Cache-Control") + .map{|v| (/max-age=(\d+)/i).match(v) } + .compact + .map{|m| m[1].to_i } + .max + end + end + end +end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index c427a957..e718b82b 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -14,7 +14,7 @@ def each(&block) end def [](request) - entry = find { |entry| entry.valid_for?(request) } + entry = find { |e| e.valid_for?(request) } entry.response if entry end diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb new file mode 100644 index 00000000..1ecc6b2f --- /dev/null +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -0,0 +1,74 @@ +require "http/request" + +module HTTP + class Cache + # Decorator class for requests to provide convenience methods + # related to caching. Instantiate using the `.coerce` method. + class RequestWithCacheBehavior < DelegateClass(HTTP::Request) + INVALIDATING_METHODS = [:post, :put, :delete, :patch].freeze + CACHEABLE_METHODS = [:get, :head].freeze + + class << self + protected :new + + # Returns a instance of self by wrapping `another` a new + # instance of self or by just returning it + def coerce(another) + if another.respond_to? :cacheable? + another + else + self.new(another) + end + end + end + + # When was this request sent to the server. + attr_accessor :sent_at + + # Returns true iff request demands the resources cache entry be invalidated. + def invalidates_cache? + INVALIDATING_METHODS.include?(verb) || + cache_control.no_store? + end + + # Returns true iff request is cacheable + def cacheable? + CACHEABLE_METHODS.include?(verb) && + !cache_control.no_store? + end + + # Returns true iff the cache control info of this request + # demands that the response be revalidated by the origin server. + def skips_cache? + 0 == cache_control.max_age || + cache_control.must_revalidate? || + cache_control.no_cache? + end + + # Update headers such that this request becomes a condition + # request revalidating `response` + def set_validation_headers!(response) + response.headers.get("Etag") + .each{|etag| headers.add("If-None-Match", etag) } + + response.headers.get("Last-Modified") + .each{|last_mod| headers.add("If-Modified-Since", last_mod) } + + headers.add("Cache-Control", "max-age=0") if cache_control.forces_revalidation? + end + + # Returns cache control helper for this request. + def cache_control + @cache_control ||= CacheControl.new(self) + end + + protected + + def initialize(obj) + super + @requested_at = nil + @received_at = nil + end + end + end +end diff --git a/lib/http/cache/response_with_cache_behavior.rb b/lib/http/cache/response_with_cache_behavior.rb new file mode 100644 index 00000000..97f51583 --- /dev/null +++ b/lib/http/cache/response_with_cache_behavior.rb @@ -0,0 +1,112 @@ +require "http/response" + +module HTTP + class Cache + # Decorator class for responses to provide convenience methods + # related to caching. Instantiate using the `.coerce` method. + class ResponseWithCacheBehavior < DelegateClass(HTTP::Response) + CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410].freeze + + class << self + protected :new + + # Returns a instance of self by wrapping `another` a new + # instance of self or by just returning it + def coerce(another) + if another.respond_to? :cacheable? + another + else + self.new(another) + end + end + end + + # Returns true iff this response is stale; otherwise false + def stale? + expired? || cache_control.must_revalidate? + end + + # Returns true iff this response has expired; otherwise false + def expired? + current_age > cache_control.max_age + end + + # Return true iff this response is cacheable; otherwise false + # + # --- + # A Vary header field-value of "*" always fails to match and + # subsequent requests on that resource can only be properly + # interpreted by the + def cacheable? + @cacheable ||= + begin + CACHEABLE_RESPONSE_CODES.include?(code) && + (cache_control.public? || cache_control.private?) && + !cache_control.vary_star? && + !cache_control.no_store? && + !cache_control.no_cache? + end + end + + # Returns the current age (in seconds) of this response + # + # --- + # Algo from https://tools.ietf.org/html/rfc2616#section-13.2.3 + def current_age + now = Time.now + age_value = headers.get("Age").map(&:to_i).max || 0 + + apparent_age = [0, received_at - server_response_time].max + corrected_received_age = [apparent_age, age_value].max + response_delay = received_at - requested_at + corrected_initial_age = corrected_received_age + response_delay + resident_time = now - received_at + corrected_initial_age + resident_time + end + + # Returns the time at which this response was requested + def requested_at + @requested_at ||= Time.now + end + attr_writer :requested_at + + # Returns the time at which this response was received + def received_at + @received_at || Time.now + end + attr_writer :received_at + + # Update self based on this response being revalidated by the + # server. + def validated!(validating_response) + headers.merge!(validating_response.headers) + self.request_time = validating_response.request_time + self.response_time = validating_response.response_time + self.authoritative = true + end + + # Returns cache control helper object. + def cache_control + @cache_control ||= CacheControl.new(self) + end + + protected + + # Returns the time at which the server generated this response. + def server_response_time + headers.get("Date").map{|s| Time.httpdate(s) rescue Time.at(0) }.max || + begin + headers["Date"] = received_at.httpdate + received_at + end + end + + def initialize(obj) + super + @requested_at = nil + @received_at = nil + end + + end + end +end diff --git a/lib/http/client.rb b/lib/http/client.rb index 797885c4..65049529 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -45,8 +45,8 @@ def request(verb, uri, opts = {}) def perform(req, options) if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) cache = Cache.new(options) - cache.perform(req, options) do |req, options| - make_request(req, options) + cache.perform(req, options) do |r, opts| + make_request(r, opts) end else make_request(req, options) diff --git a/spec/lib/http/cache/cache_control_spec.rb b/spec/lib/http/cache/cache_control_spec.rb new file mode 100644 index 00000000..897e579f --- /dev/null +++ b/spec/lib/http/cache/cache_control_spec.rb @@ -0,0 +1,126 @@ +RSpec.describe HTTP::Cache::CacheControl do + describe ".new" do + it "accepts a request" do + expect { described_class.new request }.not_to raise_error + end + + it "accepts a request" do + expect { described_class.new response }.not_to raise_error + end + + it "it rejects any object that does not respond to #headers" do + expect { described_class.new double }.to raise_error + end + end + + subject { described_class.new response } + + context "cache-control: private" do + let(:cache_control) { "private" } + + it "know it is private" do + expect(subject.private?).to be_truthy + end + end + + context "cache-control: public" do + let(:cache_control) { "public" } + + it "know it is public" do + expect(subject.public?).to be_truthy + end + end + + context "cache-control: no-cache" do + let(:cache_control) { "no-cache" } + + it "know it is no-cache" do + expect(subject.no_cache?).to be_truthy + end + end + + context "cache-control: no-store" do + let(:cache_control) { "no-store" } + + it "know it is no-store" do + expect(subject.no_store?).to be_truthy + end + end + + context "cache-control: max-age=100" do + let(:cache_control) { "max-age=100" } + + it "knows max age" do + expect(subject.max_age).to eq 100 + end + end + + context "expires: {100 seconds from now}" do + let(:headers) { {"Expires" => (Time.now+100).httpdate } } + + it "knows max age" do + expect(subject.max_age).to be_within(1).of(100) + end + end + + context "expires: {100 seconds before now}" do + let(:headers) { {"Expires" => (Time.now-100).httpdate } } + + it "knows max age" do + expect(subject.max_age).to eq 0 + end + end + + context "expires: -1" do + let(:headers) { {"Expires" => "-1" } } + + it "knows max age" do + expect(subject.max_age).to eq 0 + end + end + + context "vary: *" do + let(:headers) { {"Vary" => "*" } } + + it "knows it is vary *" do + expect(subject.vary_star?).to be_truthy + end + end + + context "no headers" do + let(:headers) { {} } + + it "knows max age" do + expect(subject.max_age).to eq Float::INFINITY + end + + it "know it is private" do + expect(subject.private?).to be_falsy + end + + it "know it is public" do + expect(subject.public?).to be_falsy + end + + it "know it is no-cache" do + expect(subject.no_cache?).to be_falsy + end + + it "know it is no-store" do + expect(subject.no_store?).to be_falsy + end + + it "knows it is not vary *" do + expect(subject.vary_star?).to be_falsy + end + + end + + # Background + let(:cache_control) { "private" } + let(:headers) { {"Cache-Control" => cache_control} } + let(:request) { HTTP::Request.new(:get, "http://example.com/") } + + let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } + +end diff --git a/spec/lib/http/cache/request_with_cache_behavior_spec.rb b/spec/lib/http/cache/request_with_cache_behavior_spec.rb new file mode 100644 index 00000000..5584121d --- /dev/null +++ b/spec/lib/http/cache/request_with_cache_behavior_spec.rb @@ -0,0 +1,141 @@ +RSpec.describe HTTP::Cache::RequestWithCacheBehavior do + describe ".coerce" do + it "should accept a base request" do + expect(described_class.coerce(request)).to be_kind_of described_class + end + + it "should accept an already decorated request" do + decorated_req = described_class.coerce(request) + expect(decorated_req).to be_kind_of described_class + end + end + + subject { described_class.coerce(request) } + + it "provides access to it's cache control object" do + expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl + end + + context "basic GET request" do + it "is cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_falsy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_falsy + end + + it "can turn itself into a condition request based on a cached response" do + mod_date = Time.now.httpdate + cached_resp = HTTP::Response.new(200, "http/1.1", + {"Etag" => "foo", + "Last-Modified" => mod_date}, + "") + subject.set_validation_headers!(cached_resp) + expect(subject.headers["If-None-Match"]).to eq "foo" + expect(subject.headers["If-Modified-Since"]).to eq mod_date + end + end + + context "GET request w/ must-revalidate" do + let(:request) { HTTP::Request.new(:get, "http://example.com/", + {"cache-control" => "must-revalidate"}) } + + it "is cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_falsy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_truthy + end + + it "can turn itself into a condition request based on a cached response" do + mod_date = Time.now.httpdate + cached_resp = HTTP::Response.new(200, "http/1.1", + {"Etag" => "foo", + "Last-Modified" => mod_date}, + "") + subject.set_validation_headers!(cached_resp) + expect(subject.headers["If-None-Match"]).to eq "foo" + expect(subject.headers["If-Modified-Since"]).to eq mod_date + expect(subject.cache_control.max_age).to eq 0 + end + end + + context "basic POST request" do + let(:request) { HTTP::Request.new(:post, "http://example.com/") } + + it "is cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_truthy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_falsy + end + end + + context "basic PUT request" do + let(:request) { HTTP::Request.new(:put, "http://example.com/") } + + it "is cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_truthy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_falsy + end + end + + context "basic delete request" do + let(:request) { HTTP::Request.new(:delete, "http://example.com/") } + + it "is cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_truthy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_falsy + end + end + + context "basic patch request" do + let(:request) { HTTP::Request.new(:patch, "http://example.com/") } + + it "is cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "does not invalidate cache" do + expect(subject.invalidates_cache?).to be_truthy + end + + it "does not skip cache" do + expect(subject.skips_cache?).to be_falsy + end + end + + + # Background + let(:request) { HTTP::Request.new(:get, "http://example.com/") } + +end diff --git a/spec/lib/http/cache/response_with_cache_behavior_spec.rb b/spec/lib/http/cache/response_with_cache_behavior_spec.rb new file mode 100644 index 00000000..7f006489 --- /dev/null +++ b/spec/lib/http/cache/response_with_cache_behavior_spec.rb @@ -0,0 +1,188 @@ +RSpec.describe HTTP::Cache::ResponseWithCacheBehavior do + describe ".coerce" do + it "should accept a base response" do + expect(described_class.coerce(response)).to be_kind_of described_class + end + + it "should accept an already decorated response" do + decorated_req = described_class.coerce(response) + expect(decorated_req).to be_kind_of described_class + end + end + + subject { described_class.coerce(response) } + + it "provides access to it's cache control object" do + expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl + end + + it "allows requested_at to be set" do + subject.requested_at = Time.now + expect(subject.requested_at).to be_within(1).of(Time.now) + end + + it "allows received_at to be set" do + subject.received_at = Time.now + expect(subject.received_at).to be_within(1).of(Time.now) + end + + describe "basic 200 response w/ private cache control" do + let(:cache_control) { "private" } + + it "is cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + + describe "basic 200 response w/ public cache control" do + let(:cache_control) { "public" } + + it "is cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + + describe "basic 200 response w/ no-cache" do + let(:cache_control) { "no-cache" } + + it "is not cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + + describe "basic 200 response w/ no-store" do + let(:cache_control) { "no-store" } + + it "is not cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + + describe "basic 200 response w/ public & max age" do + let(:cache_control) { "public, max-age=100" } + + it "is not cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + + context "with age of max-age + 1 seconds" do + let(:headers) { {"cache-control" => cache_control, + "age" => "101"} } + + it "is stale" do + expect(subject.stale?).to be_truthy + end + + it "is expired" do + expect(subject.expired?).to be_truthy + end + + it "is expected to be max-age + 1 seconds old" do + expect(subject.current_age).to be_within(1).of(101) + end + end + + context "after max-age + 1 seconds" do + before do subject.received_at = subject.requested_at = (Time.now - 101) end + + it "is stale" do + expect(subject.stale?).to be_truthy + end + + it "is expired" do + expect(subject.expired?).to be_truthy + end + + it "is expected to be max-age + 1 seconds old" do + expect(subject.current_age).to be_within(1).of(101) + end + end + + end + + describe "basic 400 response " do + let(:response) { HTTP::Response.new(400, "http/1.1", {}, "") } + + it "is not cacheable" do + expect(subject.cacheable?).to be_falsy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + + # Background + let(:cache_control) { "" } + let(:headers) { {"cache-control" => cache_control} } + let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } + +end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 569cac63..ba870607 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -19,17 +19,23 @@ subject { described_class.new(opts) } describe "#perform" do - it "calls request_performer block when cache miss" do + it "calls request_performer blocck when cache miss" do expect { |b| subject.perform(request, opts) {|*args| b.to_proc.call(*args) origin_response } }.to yield_with_args(request, opts) end - it "does not call request_performer block when cache it" do - allow(cache_adapter).to receive(:lookup).and_return(origin_response) + context "cache hit" do + let(:cached_response) { HTTP::Cache::ResponseWithCacheBehavior.coerce( + HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "private", "test"=> "foo"}, + "") + ).tap{|r| r.requested_at = r.received_at = Time.now } } - expect { |b| subject.perform(request, opts, &b) }.not_to yield_control + it "does not call request_performer block" do + expect { |b| subject.perform(request, opts, &b) }.not_to yield_control + end end end @@ -67,7 +73,7 @@ end end - context "empty cache, cacheable request, 'no-store' response" do + context "empty cache, cacheable request, 'nreceiver' response" do let(:origin_response) { HTTP::Response.new(200, "http/1.1", {"Cache-Control" => "no-cache"}, "") } @@ -126,9 +132,9 @@ end context "warm cache, cacheable request, cacheable response" do - let(:cached_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "private"}, - "") } + let(:cached_response) { build_cached_response(200, "http/1.1", + {"Cache-Control" => "private"}, + "") } let!(:response) { subject.perform(request, opts) { origin_response } } it "lookups request" do @@ -141,11 +147,13 @@ end context "stale cache, cacheable request, cacheable response" do - let(:cached_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "private, max-age=1", - "Date" => (Time.now - 2).httpdate}, - "") - .tap{|x| x.request_time = (Time.now - 2)} } + let(:cached_response) { + build_cached_response(200, "http/1.1", + {"Cache-Control" => "private, max-age=1", + "Date" => (Time.now - 2).httpdate}, + "") do |t| + t.request_time = (Time.now - 2) + end } let!(:response) { subject.perform(request, opts) { origin_response } } it "lookups request" do @@ -162,12 +170,14 @@ end context "stale cache, cacheable request, not modified response" do - let(:cached_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "private, max-age=1", - "Etag" => "foo", - "Date" => (Time.now - 2).httpdate}, - "") - .tap{|x| x.request_time = (Time.now - 2)} } + let(:cached_response) { + build_cached_response(200, "http/1.1", + {"Cache-Control" => "private, max-age=1", + "Etag" => "foo", + "Date" => (Time.now - 2).httpdate}, + "") do |x| + x.request_time = (Time.now - 2) + end } let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } let!(:response) { subject.perform(request, opts) { origin_response } } @@ -176,7 +186,6 @@ end it "modifies request with conditional request headers" do - subject.perform(request, opts) { response } expect(request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] expect(request.headers["If-Modified-Since"]) .to eq cached_response.headers["Last-Modified"] @@ -204,6 +213,15 @@ let(:cached_response) { nil } # cold cache by default + def build_cached_response(*args) + r = HTTP::Cache::ResponseWithCacheBehavior.coerce(HTTP::Response.new(*args)) + r.requested_at = r.received_at = Time.now + + yield r if block_given? + + r + end + def options(cache_opts) HTTP::Options.new(cache: cache_opts) end diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 5290cb86..aa7e1137 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -73,7 +73,7 @@ def simple_response(body, status = 200) describe "caching" do it "returns cached responses if they exist" do - cached_response = simple_response("OK") + cached_response = HTTP::Cache::ResponseWithCacheBehavior.coerce(simple_response("OK")) cache = double("cache", lookup: cached_response) client = StubbedClient.new(:cache => {mode: :private, adapter: cache}).stub( "http://example.com/" => simple_response("OK") From b3499bdaf8b6267b1c8d1e8e349a8fde76555a3c Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 16:48:40 -0700 Subject: [PATCH 09/39] improve cache options --- lib/http/cache.rb | 18 ++++++++--------- lib/http/cache/collection.rb | 4 ++-- .../cache/response_with_cache_behavior.rb | 7 +++---- lib/http/client.rb | 13 ++---------- lib/http/options.rb | 16 +++++++-------- .../response_with_cache_behavior_spec.rb | 20 +++++++++++++++++++ spec/lib/http/cache_spec.rb | 12 +++++------ spec/lib/http/client_spec.rb | 4 ++-- spec/lib/http_spec.rb | 17 ++++++++-------- 9 files changed, 61 insertions(+), 50 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index ee077e57..9f9025ea 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -5,23 +5,23 @@ module HTTP class Cache - ALLOWED_CACHE_MODES = [:public, :private].freeze - - class CacheModeError < CacheError; end - attr_reader :request, :response - def initialize(options) - unless ALLOWED_CACHE_MODES.include?(options.cache[:mode]) - fail CacheModeError, "Invalid cache_mode #{options.cache[:mode]} supplied" + # NoOp cache. Always makes the request. + class NullCache + def perform(request, options, &request_performer) + yield(request, options) end - @cache_mode = options.cache[:mode] - @cache_adapter = options.cache[:adapter] + end + + def initialize(adapter=HTTP::Cache::InMemoryCache.new) + @cache_adapter = adapter end # @return [Response] a cached response that is valid for the request or # the result of executing the provided block. def perform(request, options, &request_performer) + puts "cache is handling request" req = RequestWithCacheBehavior.coerce(request) if req.invalidates_cache? diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index e718b82b..e7c66b76 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -42,10 +42,10 @@ def valid_for?(request) def select_request_headers headers = HTTP::Headers.new - @response.headers["Vary"].split(",").each do |name| + @response.headers.get("Vary").flat_map{|v| v.split(",")}.uniq.each do |name| name.strip! headers[name] = @request.headers[name] if @request.headers[name] - end if @response.headers["Vary"] + end headers.to_h end diff --git a/lib/http/cache/response_with_cache_behavior.rb b/lib/http/cache/response_with_cache_behavior.rb index 97f51583..8d7ed28a 100644 --- a/lib/http/cache/response_with_cache_behavior.rb +++ b/lib/http/cache/response_with_cache_behavior.rb @@ -41,10 +41,9 @@ def cacheable? @cacheable ||= begin CACHEABLE_RESPONSE_CODES.include?(code) && - (cache_control.public? || cache_control.private?) && - !cache_control.vary_star? && - !cache_control.no_store? && - !cache_control.no_cache? + !(cache_control.vary_star? || + cache_control.no_store? || + cache_control.no_cache? ) end end diff --git a/lib/http/client.rb b/lib/http/client.rb index 65049529..b758de64 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -43,13 +43,8 @@ def request(verb, uri, opts = {}) # Perform a single (no follow) HTTP request def perform(req, options) - if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) - cache = Cache.new(options) - cache.perform(req, options) do |r, opts| - make_request(r, opts) - end - else - make_request(req, options) + options.cache.perform(req, options) do |r, opts| + make_request(r, opts) end end @@ -75,10 +70,6 @@ def make_request(req, options) finish_response if :head == req.verb - if Cache::ALLOWED_CACHE_MODES.include?(options.cache[:mode]) - cache.perform_response(res) - end - res end diff --git a/lib/http/options.rb b/lib/http/options.rb index e7592934..7b8b63c6 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -44,7 +44,7 @@ class Options @default_socket_class = TCPSocket @default_ssl_socket_class = OpenSSL::SSL::SSLSocket - @default_cache = {:mode => false, :adapter => HTTP::Cache::InMemoryCache.new} + @default_cache = Http::Cache::NullCache.new class << self attr_accessor :default_socket_class, :default_ssl_socket_class @@ -88,15 +88,15 @@ def with_#{method_name}(value) RUBY end - def with_cache(cache) - my_cache = cache.dup - - unless my_cache.is_a?(Hash) - my_cache = self.class.default_cache.merge({:mode => my_cache}) - end + def with_cache(persistence_adapter_or_cache) + cache = if persistence_adapter_or_cache.respond_to? :perform + persistence_adapter_or_cache + else + HTTP::Cache.new(persistence_adapter_or_cache) + end dup do |opts| - opts.cache = my_cache + opts.cache = cache end end diff --git a/spec/lib/http/cache/response_with_cache_behavior_spec.rb b/spec/lib/http/cache/response_with_cache_behavior_spec.rb index 7f006489..1295ac0b 100644 --- a/spec/lib/http/cache/response_with_cache_behavior_spec.rb +++ b/spec/lib/http/cache/response_with_cache_behavior_spec.rb @@ -106,6 +106,26 @@ end end + describe "basic 200 response w/ max age" do + let(:cache_control) { "max-age=100" } + + it "is not cacheable" do + expect(subject.cacheable?).to be_truthy + end + + it "is not stale" do + expect(subject.stale?).to be_falsy + end + + it "is not expired" do + expect(subject.expired?).to be_falsy + end + + it "is expected to be 0 seconds old" do + expect(subject.current_age).to be_within(1).of(0) + end + end + describe "basic 200 response w/ public & max age" do let(:cache_control) { "public, max-age=100" } diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index ba870607..460c37b0 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -5,18 +5,18 @@ subject { described_class } it "allows private mode" do - expect(subject.new(options(mode: :private, adapter: cache_adapter))) + expect(subject.new(cache_adapter)) .to be_kind_of HTTP::Cache end it "allows public mode" do - expect(subject.new(options(mode: :public, adapter: cache_adapter))) + expect(subject.new(cache_adapter)) .to be_kind_of HTTP::Cache end end - let(:opts) { options(mode: :private, adapter: cache_adapter) } - subject { described_class.new(opts) } + let(:opts) { options } + subject { described_class.new(cache_adapter) } describe "#perform" do it "calls request_performer blocck when cache miss" do @@ -222,7 +222,7 @@ def build_cached_response(*args) r end - def options(cache_opts) - HTTP::Options.new(cache: cache_opts) + def options() + HTTP::Options.new() end end diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index aa7e1137..226f845b 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -74,8 +74,8 @@ def simple_response(body, status = 200) describe "caching" do it "returns cached responses if they exist" do cached_response = HTTP::Cache::ResponseWithCacheBehavior.coerce(simple_response("OK")) - cache = double("cache", lookup: cached_response) - client = StubbedClient.new(:cache => {mode: :private, adapter: cache}).stub( + adapter = double("persistance_adapter", lookup: cached_response) + client = StubbedClient.new(:cache => HTTP::Cache.new(adapter)).stub( "http://example.com/" => simple_response("OK") ) diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 9099871f..eda5ab19 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -157,16 +157,17 @@ end describe ".with_cache" do - it "sets cache options for valid mode" do - client = HTTP.with_cache "private" - expect(client.default_options[:cache]).to include mode: "private", adapter: HTTP::Cache::InMemoryCache + it "sets cache option" do + cache = double(:cache, :perform => nil) + client = HTTP.with_cache cache + expect(client.default_options[:cache]).to eq cache end - it "sets cache options for valid mode and adapter" do - adapter = double("an_adapter") - client = HTTP.with_cache mode: :private, adapter: adapter - expect(client.default_options[:cache]).to include mode: :private, adapter: adapter + it "sets cache option" do + persistence_adapter = double(:cache, :lookup => nil) + client = HTTP.with_cache persistence_adapter + expect(client.default_options[:cache]).not_to eq persistence_adapter end - end + end end From 153fe3b250c7593c6ec71928ced1bdb3c5de1c8f Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 16:56:25 -0700 Subject: [PATCH 10/39] remove dead code --- lib/http/cache/redis_cache.rb | 52 ----------------------------------- 1 file changed, 52 deletions(-) delete mode 100644 lib/http/cache/redis_cache.rb diff --git a/lib/http/cache/redis_cache.rb b/lib/http/cache/redis_cache.rb deleted file mode 100644 index 2428df60..00000000 --- a/lib/http/cache/redis_cache.rb +++ /dev/null @@ -1,52 +0,0 @@ -# Currently broken -# require "connection_pool" -# require "redis" -# require "http/cache/collection" -# require "digest/sha2" - -# module HTTP -# class Cache -# class RedisCache -# def initialize(options = {}) -# @redis = ConnectionPool.new(size: (options[:pool_size] ||5), -# timeout: (options[:pool_timeout] ||5)) { Redis.new(options) } -# end - -# def lookup(request) -# response = cache_entries_for(request)[request] -# response.authoritative = false if response -# response -# end - -# def store(request, response) -# entries = cache_entries_for(request) - -# @redis.with do |redis| -# entries[request] = response -# redis.set(uri_hash(request.uri), Marshal.dump(entries)) -# end -# end - -# def invalidate(uri) -# @redis.with do |redis| -# redis.del(uri_hash(uri)) -# end -# end - -# private -# def cache_entries_for(request) -# @redis.with do |redis| -# if entries = redis.get(uri_hash(request.uri)) -# Marshal.load(entries) -# else -# CacheEntryCollection.new -# end -# end -# end - -# def uri_hash(uri) -# Digest::SHA2.hexdigest(uri.to_s) -# end -# end -# end -# end From 69a343d9e6ae80008e61a08f466a7574d195d1de Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 17:02:25 -0700 Subject: [PATCH 11/39] remove unused argument to make rubocop happy --- lib/http/cache.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 9f9025ea..aa3f1d98 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -9,7 +9,9 @@ class Cache # NoOp cache. Always makes the request. class NullCache - def perform(request, options, &request_performer) + # Yields request and options to block so that it can make + # request. + def perform(request, options) yield(request, options) end end @@ -20,7 +22,10 @@ def initialize(adapter=HTTP::Cache::InMemoryCache.new) # @return [Response] a cached response that is valid for the request or # the result of executing the provided block. - def perform(request, options, &request_performer) + # + # Yields request and options to block if when there is a cache + # miss so that the request can be make for real. + def perform(request, options) puts "cache is handling request" req = RequestWithCacheBehavior.coerce(request) From 40fb8b6d448605e3d6174c000b17848fef989f7a Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 17:06:24 -0700 Subject: [PATCH 12/39] remove debug puts --- lib/http/cache.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index aa3f1d98..37a8d6bd 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -26,7 +26,6 @@ def initialize(adapter=HTTP::Cache::InMemoryCache.new) # Yields request and options to block if when there is a cache # miss so that the request can be make for real. def perform(request, options) - puts "cache is handling request" req = RequestWithCacheBehavior.coerce(request) if req.invalidates_cache? From bbfd8c31cf66812c470c79cca2684dfffe822868 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 20:04:30 -0700 Subject: [PATCH 13/39] remove many rubocop issues --- lib/http/cache.rb | 17 ++-- lib/http/cache/cache_control.rb | 20 ++-- lib/http/cache/collection.rb | 4 +- lib/http/cache/in_memory_cache.rb | 2 +- lib/http/cache/request_with_cache_behavior.rb | 8 +- .../cache/response_with_cache_behavior.rb | 25 +++-- lib/http/options.rb | 4 +- lib/http/request.rb | 5 - spec/lib/http/cache/cache_control_spec.rb | 10 +- .../cache/request_with_cache_behavior_spec.rb | 9 +- .../response_with_cache_behavior_spec.rb | 11 +-- spec/lib/http/cache_spec.rb | 98 ++++++++++++------- spec/lib/http/client_spec.rb | 2 +- spec/lib/http_spec.rb | 1 - 14 files changed, 123 insertions(+), 93 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 37a8d6bd..90134b1a 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -16,7 +16,7 @@ def perform(request, options) end end - def initialize(adapter=HTTP::Cache::InMemoryCache.new) + def initialize(adapter = HTTP::Cache::InMemoryCache.new) @cache_adapter = adapter end @@ -25,19 +25,24 @@ def initialize(adapter=HTTP::Cache::InMemoryCache.new) # # Yields request and options to block if when there is a cache # miss so that the request can be make for real. - def perform(request, options) + def perform(request, options, &request_performer) req = RequestWithCacheBehavior.coerce(request) if req.invalidates_cache? invalidate_cache(req) - elsif cached_resp = cache_lookup(req) + elsif cached_resp = cache_lookup(req) # rubocop:disable all return cached_resp unless cached_resp.stale? req.set_validation_headers!(cached_resp) end - # cache miss! Do this the hard way... + handle_miss(req, cached_resp, options, &request_performer) + end + + protected + + def handle_miss(req, cached_resp, options) req.sent_at = Time.now act_resp = ResponseWithCacheBehavior.coerce(yield(req, options)) @@ -58,9 +63,6 @@ def perform(request, options) end end - protected - - def cache_lookup(request) return nil if request.skips_cache? c = @cache_adapter.lookup(request) @@ -79,6 +81,5 @@ def store_in_cache(request, response) def invalidate_cache(request) @cache_adapter.invalidate(request.uri) end - end end diff --git a/lib/http/cache/cache_control.rb b/lib/http/cache/cache_control.rb index 36cc53c8..392a8fbe 100644 --- a/lib/http/cache/cache_control.rb +++ b/lib/http/cache/cache_control.rb @@ -35,7 +35,7 @@ def max_age end def vary_star? - headers.get("Vary").any?{|v| "*" == v.strip } + headers.get("Vary").any? { |v| "*" == v.strip } end protected @@ -43,25 +43,31 @@ def vary_star? attr_reader :headers def matches?(pattern) - headers.get("Cache-Control").any?{|v| v =~ pattern } + headers.get("Cache-Control").any? { |v| v =~ pattern } end # --- # Some servers send a "Expire: -1" header which must be treated as expired def seconds_til_expires headers.get("Expires") - .map{|e| Time.httpdate(e) rescue Time.at(0) } + .map(&method(:to_time_or_epoch)) .compact - .map{|e| e - Time.now} - .map{|a| a < 0 ? 0 : a} # age is 0 if it is expired + .map { |e| e - Time.now } + .map { |a| a < 0 ? 0 : a } # age is 0 if it is expired .max end + def to_time_or_epoch(t_str) + Time.httpdate(t_str) + rescue ArgumentError + Time.at(0) + end + def explicit_max_age headers.get("Cache-Control") - .map{|v| (/max-age=(\d+)/i).match(v) } + .map { |v| (/max-age=(\d+)/i).match(v) } .compact - .map{|m| m[1].to_i } + .map { |m| m[1].to_i } .max end end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index e7c66b76..e8cc179b 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -33,7 +33,7 @@ def initialize(request, response) end def valid_for?(request) - request.uri == @request.uri and + request.uri == @request.uri && select_request_headers.all? do |key, value| request.headers[key] == value end @@ -42,7 +42,7 @@ def valid_for?(request) def select_request_headers headers = HTTP::Headers.new - @response.headers.get("Vary").flat_map{|v| v.split(",")}.uniq.each do |name| + @response.headers.get("Vary").flat_map { |v| v.split(",") }.uniq.each do |name| name.strip! headers[name] = @request.headers[name] if @request.headers[name] end diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb index 71036e00..747f2078 100644 --- a/lib/http/cache/in_memory_cache.rb +++ b/lib/http/cache/in_memory_cache.rb @@ -6,7 +6,7 @@ class Cache class InMemoryCache def initialize @mutex = Mutex.new - @collection = Hash.new{ |h,k| h[k] = CacheEntryCollection.new } + @collection = Hash.new { |h, k| h[k] = CacheEntryCollection.new } end def lookup(request) diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb index 1ecc6b2f..fe5df538 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -17,7 +17,7 @@ def coerce(another) if another.respond_to? :cacheable? another else - self.new(another) + new(another) end end end @@ -40,7 +40,7 @@ def cacheable? # Returns true iff the cache control info of this request # demands that the response be revalidated by the origin server. def skips_cache? - 0 == cache_control.max_age || + 0 == cache_control.max_age || cache_control.must_revalidate? || cache_control.no_cache? end @@ -49,10 +49,10 @@ def skips_cache? # request revalidating `response` def set_validation_headers!(response) response.headers.get("Etag") - .each{|etag| headers.add("If-None-Match", etag) } + .each { |etag| headers.add("If-None-Match", etag) } response.headers.get("Last-Modified") - .each{|last_mod| headers.add("If-Modified-Since", last_mod) } + .each { |last_mod| headers.add("If-Modified-Since", last_mod) } headers.add("Cache-Control", "max-age=0") if cache_control.forces_revalidation? end diff --git a/lib/http/cache/response_with_cache_behavior.rb b/lib/http/cache/response_with_cache_behavior.rb index 8d7ed28a..3eb5455c 100644 --- a/lib/http/cache/response_with_cache_behavior.rb +++ b/lib/http/cache/response_with_cache_behavior.rb @@ -16,7 +16,7 @@ def coerce(another) if another.respond_to? :cacheable? another else - self.new(another) + new(another) end end end @@ -42,8 +42,8 @@ def cacheable? begin CACHEABLE_RESPONSE_CODES.include?(code) && !(cache_control.vary_star? || - cache_control.no_store? || - cache_control.no_cache? ) + cache_control.no_store? || + cache_control.no_cache?) end end @@ -93,11 +93,19 @@ def cache_control # Returns the time at which the server generated this response. def server_response_time - headers.get("Date").map{|s| Time.httpdate(s) rescue Time.at(0) }.max || - begin - headers["Date"] = received_at.httpdate - received_at - end + headers.get("Date") + .map(&method(:to_time_or_epoch)) + .max || begin + # set it if it is not already set + headers["Date"] = received_at.httpdate + received_at + end + end + + def to_time_or_epoch(t_str) + Time.httpdate(t_str) + rescue ArgumentError + Time.at(0) end def initialize(obj) @@ -105,7 +113,6 @@ def initialize(obj) @requested_at = nil @received_at = nil end - end end end diff --git a/lib/http/options.rb b/lib/http/options.rb index 7b8b63c6..d17b0f81 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -1,8 +1,8 @@ require "http/headers" require "openssl" require "socket" -require 'http/cache' -require 'http/cache/in_memory_cache' +require "http/cache" +require "http/cache/in_memory_cache" module HTTP class Options diff --git a/lib/http/request.rb b/lib/http/request.rb index 5ed27cc9..f052c96b 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -66,10 +66,6 @@ def __method__(*args) attr_reader :uri attr_reader :proxy, :body, :version - # "request_time" as per RFC 2616 - # https://tools.ietf.org/html/rfc2616#section-13.2.3 - attr_accessor :request_time - # :nodoc: def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") # rubocop:disable ParameterLists @verb = verb.to_s.downcase.to_sym @@ -86,7 +82,6 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") @headers["Host"] ||= default_host @headers["User-Agent"] ||= USER_AGENT now = Time.now - @request_time = now @headers["Date"] = now.httpdate end diff --git a/spec/lib/http/cache/cache_control_spec.rb b/spec/lib/http/cache/cache_control_spec.rb index 897e579f..b9bd37f7 100644 --- a/spec/lib/http/cache/cache_control_spec.rb +++ b/spec/lib/http/cache/cache_control_spec.rb @@ -56,7 +56,7 @@ end context "expires: {100 seconds from now}" do - let(:headers) { {"Expires" => (Time.now+100).httpdate } } + let(:headers) { {"Expires" => (Time.now + 100).httpdate} } it "knows max age" do expect(subject.max_age).to be_within(1).of(100) @@ -64,7 +64,7 @@ end context "expires: {100 seconds before now}" do - let(:headers) { {"Expires" => (Time.now-100).httpdate } } + let(:headers) { {"Expires" => (Time.now - 100).httpdate} } it "knows max age" do expect(subject.max_age).to eq 0 @@ -72,7 +72,7 @@ end context "expires: -1" do - let(:headers) { {"Expires" => "-1" } } + let(:headers) { {"Expires" => "-1"} } it "knows max age" do expect(subject.max_age).to eq 0 @@ -80,7 +80,7 @@ end context "vary: *" do - let(:headers) { {"Vary" => "*" } } + let(:headers) { {"Vary" => "*"} } it "knows it is vary *" do expect(subject.vary_star?).to be_truthy @@ -113,7 +113,6 @@ it "knows it is not vary *" do expect(subject.vary_star?).to be_falsy end - end # Background @@ -122,5 +121,4 @@ let(:request) { HTTP::Request.new(:get, "http://example.com/") } let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } - end diff --git a/spec/lib/http/cache/request_with_cache_behavior_spec.rb b/spec/lib/http/cache/request_with_cache_behavior_spec.rb index 5584121d..e5ed2feb 100644 --- a/spec/lib/http/cache/request_with_cache_behavior_spec.rb +++ b/spec/lib/http/cache/request_with_cache_behavior_spec.rb @@ -42,8 +42,11 @@ end context "GET request w/ must-revalidate" do - let(:request) { HTTP::Request.new(:get, "http://example.com/", - {"cache-control" => "must-revalidate"}) } + let(:request) do + HTTP::Request.new(:get, + "http://example.com/", + "cache-control" => "must-revalidate") + end it "is cacheable" do expect(subject.cacheable?).to be_truthy @@ -134,8 +137,6 @@ end end - # Background let(:request) { HTTP::Request.new(:get, "http://example.com/") } - end diff --git a/spec/lib/http/cache/response_with_cache_behavior_spec.rb b/spec/lib/http/cache/response_with_cache_behavior_spec.rb index 1295ac0b..4be8f515 100644 --- a/spec/lib/http/cache/response_with_cache_behavior_spec.rb +++ b/spec/lib/http/cache/response_with_cache_behavior_spec.rb @@ -125,7 +125,7 @@ expect(subject.current_age).to be_within(1).of(0) end end - + describe "basic 200 response w/ public & max age" do let(:cache_control) { "public, max-age=100" } @@ -146,8 +146,7 @@ end context "with age of max-age + 1 seconds" do - let(:headers) { {"cache-control" => cache_control, - "age" => "101"} } + let(:headers) { {"cache-control" => cache_control, "age" => "101"} } it "is stale" do expect(subject.stale?).to be_truthy @@ -163,7 +162,9 @@ end context "after max-age + 1 seconds" do - before do subject.received_at = subject.requested_at = (Time.now - 101) end + before do + subject.received_at = subject.requested_at = (Time.now - 101) + end it "is stale" do expect(subject.stale?).to be_truthy @@ -177,7 +178,6 @@ expect(subject.current_age).to be_within(1).of(101) end end - end describe "basic 400 response " do @@ -204,5 +204,4 @@ let(:cache_control) { "" } let(:headers) { {"cache-control" => cache_control} } let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } - end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 460c37b0..c4646c12 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -20,18 +20,23 @@ describe "#perform" do it "calls request_performer blocck when cache miss" do - expect { |b| subject.perform(request, opts) {|*args| - b.to_proc.call(*args) - origin_response } - }.to yield_with_args(request, opts) + expect do |b| + subject.perform(request, opts) do |*args| + b.to_proc.call(*args) + origin_response + end + end.to yield_with_args(request, opts) end context "cache hit" do - let(:cached_response) { HTTP::Cache::ResponseWithCacheBehavior.coerce( - HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "private", "test"=> "foo"}, - "") - ).tap{|r| r.requested_at = r.received_at = Time.now } } + let(:cached_response) do + HTTP::Cache::ResponseWithCacheBehavior.coerce( + HTTP::Response.new(200, + "http/1.1", + {"Cache-Control" => "private", "test" => "foo"}, + "") + ).tap { |r| r.requested_at = r.received_at = Time.now } + end it "does not call request_performer block" do expect { |b| subject.perform(request, opts, &b) }.not_to yield_control @@ -56,8 +61,11 @@ end context "cache by-passing request, cacheable response" do - let(:request) { HTTP::Request.new(:get, "http://example.com/", - {"Cache-Control" => "no-cache"}) } + let(:request) do + HTTP::Request.new(:get, + "http://example.com/", + "Cache-Control" => "no-cache") + end let!(:response) { subject.perform(request, opts) { origin_response } } it "doesn't lookup request" do @@ -74,9 +82,11 @@ end context "empty cache, cacheable request, 'nreceiver' response" do - let(:origin_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "no-cache"}, - "") } + let(:origin_response) do + HTTP::Response.new(200, "http/1.1", + {"Cache-Control" => "no-cache"}, + "") + end let!(:response) { subject.perform(request, opts) { origin_response } } it "tries to lookup request" do @@ -90,13 +100,15 @@ it "doesn't store response in cache" do expect(cache_adapter).not_to have_received(:store) end - end context "empty cache, cacheable request, 'no-cache' response" do - let(:origin_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "no-store"}, - "") } + let(:origin_response) do + HTTP::Response.new(200, + "http/1.1", + {"Cache-Control" => "no-store"}, + "") + end let!(:response) { subject.perform(request, opts) { origin_response } } it "tries to lookup request" do @@ -113,9 +125,12 @@ end context "empty cache, cacheable request, 'no-store' response" do - let(:origin_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "no-store"}, - "") } + let(:origin_response) do + HTTP::Response.new(200, + "http/1.1", + {"Cache-Control" => "no-store"}, + "") + end let!(:response) { subject.perform(request, opts) { origin_response } } it "tries to lookup request" do @@ -132,9 +147,12 @@ end context "warm cache, cacheable request, cacheable response" do - let(:cached_response) { build_cached_response(200, "http/1.1", - {"Cache-Control" => "private"}, - "") } + let(:cached_response) do + build_cached_response(200, + "http/1.1", + {"Cache-Control" => "private"}, + "") + end let!(:response) { subject.perform(request, opts) { origin_response } } it "lookups request" do @@ -147,13 +165,15 @@ end context "stale cache, cacheable request, cacheable response" do - let(:cached_response) { - build_cached_response(200, "http/1.1", + let(:cached_response) do + build_cached_response(200, + "http/1.1", {"Cache-Control" => "private, max-age=1", "Date" => (Time.now - 2).httpdate}, "") do |t| t.request_time = (Time.now - 2) - end } + end + end let!(:response) { subject.perform(request, opts) { origin_response } } it "lookups request" do @@ -170,14 +190,16 @@ end context "stale cache, cacheable request, not modified response" do - let(:cached_response) { - build_cached_response(200, "http/1.1", + let(:cached_response) do + build_cached_response(200, + "http/1.1", {"Cache-Control" => "private, max-age=1", "Etag" => "foo", "Date" => (Time.now - 2).httpdate}, "") do |x| x.request_time = (Time.now - 2) - end } + end + end let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } let!(:response) { subject.perform(request, opts) { origin_response } } @@ -200,16 +222,18 @@ end end - # Background - let(:cache_adapter) { double("cache_adapter", lookup: cached_response, store: nil) } + let(:cache_adapter) { double("cache_adapter", :lookup => cached_response, :store => nil) } let(:request) { HTTP::Request.new(:get, "http://example.com/") } - let(:origin_response) { HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "private"}, - "") } + let(:origin_response) do + HTTP::Response.new(200, + "http/1.1", + {"Cache-Control" => "private"}, + "") + end let(:cached_response) { nil } # cold cache by default @@ -222,7 +246,7 @@ def build_cached_response(*args) r end - def options() - HTTP::Options.new() + def options + HTTP::Options.new end end diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 226f845b..935e13c7 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -74,7 +74,7 @@ def simple_response(body, status = 200) describe "caching" do it "returns cached responses if they exist" do cached_response = HTTP::Cache::ResponseWithCacheBehavior.coerce(simple_response("OK")) - adapter = double("persistance_adapter", lookup: cached_response) + adapter = double("persistance_adapter", :lookup => cached_response) client = StubbedClient.new(:cache => HTTP::Cache.new(adapter)).stub( "http://example.com/" => simple_response("OK") ) diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index eda5ab19..e74096c6 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -168,6 +168,5 @@ client = HTTP.with_cache persistence_adapter expect(client.default_options[:cache]).not_to eq persistence_adapter end - end end From 2a57ce3019485d327fc600019201027409dafd42 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 20:16:50 -0700 Subject: [PATCH 14/39] swap `find` for `detect` to appease rubocop --- lib/http/cache/collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index e8cc179b..19c0e2b8 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -14,7 +14,7 @@ def each(&block) end def [](request) - entry = find { |e| e.valid_for?(request) } + entry = detect { |e| e.valid_for?(request) } entry.response if entry end From 31a60f6846f885c1e0dd8be820a7a1b5f80597e4 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 21:03:47 -0700 Subject: [PATCH 15/39] improve factoring of cache perform method --- lib/http/cache.rb | 35 +++++++++++-------- lib/http/cache/request_with_cache_behavior.rb | 30 +++++++++++----- .../cache/request_with_cache_behavior_spec.rb | 19 +++++----- spec/lib/http/cache_spec.rb | 9 +++-- 4 files changed, 58 insertions(+), 35 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 90134b1a..268a4bec 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -28,26 +28,25 @@ def initialize(adapter = HTTP::Cache::InMemoryCache.new) def perform(request, options, &request_performer) req = RequestWithCacheBehavior.coerce(request) - if req.invalidates_cache? - invalidate_cache(req) + invalidate_cache(req) if req.invalidates_cache? - elsif cached_resp = cache_lookup(req) # rubocop:disable all - return cached_resp unless cached_resp.stale? - - req.set_validation_headers!(cached_resp) - end - - handle_miss(req, cached_resp, options, &request_performer) + get_response(req, options, request_performer) end protected - def handle_miss(req, cached_resp, options) - req.sent_at = Time.now - act_resp = ResponseWithCacheBehavior.coerce(yield(req, options)) + def get_response(req, options, request_performer) + cached_resp = cache_lookup(req) + return cached_resp if cached_resp && !cached_resp.stale? + + # cache miss - act_resp.received_at = Time.now - act_resp.requested_at = req.sent_at + act_req = if cached_resp + req.conditional_on_changes_to(cached_resp) + else + req + end + act_resp = make_request(act_req, options, request_performer) if act_resp.status.not_modified? && cached_resp cached_resp.validated!(act_resp) @@ -63,6 +62,14 @@ def handle_miss(req, cached_resp, options) end end + def make_request(req, options, request_performer) + req.sent_at = Time.now + ResponseWithCacheBehavior.coerce(request_performer.call(req, options)).tap do |resp| + resp.received_at = Time.now + resp.requested_at = req.sent_at + end + end + def cache_lookup(request) return nil if request.skips_cache? c = @cache_adapter.lookup(request) diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb index fe5df538..6defb806 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -45,16 +45,14 @@ def skips_cache? cache_control.no_cache? end - # Update headers such that this request becomes a condition - # request revalidating `response` - def set_validation_headers!(response) - response.headers.get("Etag") - .each { |etag| headers.add("If-None-Match", etag) } - - response.headers.get("Last-Modified") - .each { |last_mod| headers.add("If-Modified-Since", last_mod) } + # Returns new request based on this one but conditional on the + # resource having changed since `cached_response` + def conditional_on_changes_to(cached_response) + raw_cond_req = HTTP::Request.new(verb, uri, + headers.merge(conditional_headers_for(cached_response)), + proxy, body, version) - headers.add("Cache-Control", "max-age=0") if cache_control.forces_revalidation? + self.class.coerce(raw_cond_req) end # Returns cache control helper for this request. @@ -64,6 +62,20 @@ def cache_control protected + def conditional_headers_for(cached_response) + headers = HTTP::Headers.new + + cached_response.headers.get("Etag") + .each { |etag| headers.add("If-None-Match", etag) } + + cached_response.headers.get("Last-Modified") + .each { |last_mod| headers.add("If-Modified-Since", last_mod) } + + headers.add("Cache-Control", "max-age=0") if cache_control.forces_revalidation? + + headers + end + def initialize(obj) super @requested_at = nil diff --git a/spec/lib/http/cache/request_with_cache_behavior_spec.rb b/spec/lib/http/cache/request_with_cache_behavior_spec.rb index e5ed2feb..de3dabbb 100644 --- a/spec/lib/http/cache/request_with_cache_behavior_spec.rb +++ b/spec/lib/http/cache/request_with_cache_behavior_spec.rb @@ -29,15 +29,16 @@ expect(subject.skips_cache?).to be_falsy end - it "can turn itself into a condition request based on a cached response" do + it "can construct a new conditional version of itself based on a cached response" do mod_date = Time.now.httpdate cached_resp = HTTP::Response.new(200, "http/1.1", {"Etag" => "foo", "Last-Modified" => mod_date}, "") - subject.set_validation_headers!(cached_resp) - expect(subject.headers["If-None-Match"]).to eq "foo" - expect(subject.headers["If-Modified-Since"]).to eq mod_date + cond_req = subject.conditional_on_changes_to(cached_resp) + + expect(cond_req.headers["If-None-Match"]).to eq "foo" + expect(cond_req.headers["If-Modified-Since"]).to eq mod_date end end @@ -60,16 +61,16 @@ expect(subject.skips_cache?).to be_truthy end - it "can turn itself into a condition request based on a cached response" do + it "can construct a condition version of itself based on a cached response" do mod_date = Time.now.httpdate cached_resp = HTTP::Response.new(200, "http/1.1", {"Etag" => "foo", "Last-Modified" => mod_date}, "") - subject.set_validation_headers!(cached_resp) - expect(subject.headers["If-None-Match"]).to eq "foo" - expect(subject.headers["If-Modified-Since"]).to eq mod_date - expect(subject.cache_control.max_age).to eq 0 + cond_req = subject.conditional_on_changes_to(cached_resp) + expect(cond_req.headers["If-None-Match"]).to eq "foo" + expect(cond_req.headers["If-Modified-Since"]).to eq mod_date + expect(cond_req.cache_control.max_age).to eq 0 end end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index c4646c12..7bdda26b 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -207,9 +207,12 @@ expect(cache_adapter).to have_received(:lookup).with(request) end - it "modifies request with conditional request headers" do - expect(request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] - expect(request.headers["If-Modified-Since"]) + it "makes request with conditional request headers" do + actual_request = nil + subject.perform(request, opts) { |r, _| actual_request = r; origin_response } + + expect(actual_request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] + expect(actual_request.headers["If-Modified-Since"]) .to eq cached_response.headers["Last-Modified"] end From 08df7669b6d747212f9ed2355e6755dc5475e628 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 21:22:13 -0700 Subject: [PATCH 16/39] further factoring improvements to cache perform --- .rubocop.yml | 3 ++ lib/http/cache.rb | 30 +++++++++++-------- lib/http/cache/request_with_cache_behavior.rb | 4 +-- spec/lib/http/cache_spec.rb | 5 +++- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d603c46b..f471e2d7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -59,3 +59,6 @@ Style/StringLiterals: Style/TrivialAccessors: Enabled: false + +Style/AlignParameters: + Enabled: false \ No newline at end of file diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 268a4bec..a0c743fa 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -41,24 +41,28 @@ def get_response(req, options, request_performer) # cache miss - act_req = if cached_resp - req.conditional_on_changes_to(cached_resp) - else - req - end - act_resp = make_request(act_req, options, request_performer) - - if act_resp.status.not_modified? && cached_resp - cached_resp.validated!(act_resp) + actual_req = if cached_resp + req.conditional_on_changes_to(cached_resp) + else + req + end + actual_resp = make_request(actual_req, options, request_performer) + + handle_response(cached_resp, actual_resp, req) + end + + def handle_response(cached_resp, actual_resp, req) + if actual_resp.status.not_modified? && cached_resp + cached_resp.validated!(actual_resp) store_in_cache(req, cached_resp) return cached_resp - elsif req.cacheable? && act_resp.cacheable? - store_in_cache(req, act_resp) - return act_resp + elsif req.cacheable? && actual_resp.cacheable? + store_in_cache(req, actual_resp) + return actual_resp else - return act_resp + return actual_resp end end diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb index 6defb806..ade96e11 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -49,8 +49,8 @@ def skips_cache? # resource having changed since `cached_response` def conditional_on_changes_to(cached_response) raw_cond_req = HTTP::Request.new(verb, uri, - headers.merge(conditional_headers_for(cached_response)), - proxy, body, version) + headers.merge(conditional_headers_for(cached_response)), + proxy, body, version) self.class.coerce(raw_cond_req) end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 7bdda26b..92e98fa2 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -209,7 +209,10 @@ it "makes request with conditional request headers" do actual_request = nil - subject.perform(request, opts) { |r, _| actual_request = r; origin_response } + subject.perform(request, opts) do |r, _| + actual_request = r + origin_response + end expect(actual_request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] expect(actual_request.headers["If-Modified-Since"]) From f2346bd2c14e589ef6b9cb74e3d213c7631e4a47 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 22:17:50 -0700 Subject: [PATCH 17/39] Convert docs to yard --- lib/http/cache.rb | 35 +++++++++++----- lib/http/cache/cache_control.rb | 24 +++++++++-- lib/http/cache/collection.rb | 31 ++++++++++---- lib/http/cache/in_memory_cache.rb | 16 ++++++-- lib/http/cache/request_with_cache_behavior.rb | 40 ++++++++++++++----- .../cache/response_with_cache_behavior.rb | 18 ++++----- 6 files changed, 118 insertions(+), 46 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index a0c743fa..1af772fe 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -5,26 +5,20 @@ module HTTP class Cache - attr_reader :request, :response - # NoOp cache. Always makes the request. class NullCache - # Yields request and options to block so that it can make - # request. + # @return [Response] the result of the provided block + # @yield [request, options] so that the request can actually be made def perform(request, options) yield(request, options) end end - def initialize(adapter = HTTP::Cache::InMemoryCache.new) - @cache_adapter = adapter - end - # @return [Response] a cached response that is valid for the request or - # the result of executing the provided block. + # the result of executing the provided block # - # Yields request and options to block if when there is a cache - # miss so that the request can be make for real. + # @yield [request, options] on cache miss so that an actual + # request can be made def perform(request, options, &request_performer) req = RequestWithCacheBehavior.coerce(request) @@ -35,6 +29,8 @@ def perform(request, options, &request_performer) protected + # @return [Response] the response to the request, either from the + # cache or by actually making the request def get_response(req, options, request_performer) cached_resp = cache_lookup(req) return cached_resp if cached_resp && !cached_resp.stale? @@ -51,6 +47,8 @@ def get_response(req, options, request_performer) handle_response(cached_resp, actual_resp, req) end + # @returns [Response] the most useful of the responses after + # updating the cache as appropriate def handle_response(cached_resp, actual_resp, req) if actual_resp.status.not_modified? && cached_resp cached_resp.validated!(actual_resp) @@ -66,6 +64,8 @@ def handle_response(cached_resp, actual_resp, req) end end + # @return [ResponseWithCacheBehavior] the actual response returned + # by request_performer def make_request(req, options, request_performer) req.sent_at = Time.now ResponseWithCacheBehavior.coerce(request_performer.call(req, options)).tap do |resp| @@ -74,6 +74,8 @@ def make_request(req, options, request_performer) end end + # @return [ResponseWithCacheBehavior] the cached response for the + # request or nil if there isn't one def cache_lookup(request) return nil if request.skips_cache? c = @cache_adapter.lookup(request) @@ -84,13 +86,24 @@ def cache_lookup(request) end end + # Store response in cache + # + # @return [nil] def store_in_cache(request, response) @cache_adapter.store(request, response) nil end + # Invalidate all response from the requested resource + # + # @return [nil] def invalidate_cache(request) @cache_adapter.invalidate(request.uri) end + + # Inits a new instance + def initialize(adapter = HTTP::Cache::InMemoryCache.new) + @cache_adapter = adapter + end end end diff --git a/lib/http/cache/cache_control.rb b/lib/http/cache/cache_control.rb index 392a8fbe..60370192 100644 --- a/lib/http/cache/cache_control.rb +++ b/lib/http/cache/cache_control.rb @@ -2,38 +2,43 @@ module HTTP class Cache # Convenience methods around cache control headers. class CacheControl - def initialize(message) - @headers = message.headers - end - + # @return [Boolean] does this message force revalidation def forces_revalidation? must_revalidate? || max_age == 0 end + # @return [Boolean] does the cache control include 'must-revalidate' def must_revalidate? matches?(/\bmust-revalidate\b/i) end + # @return [Boolean] does the cache control include 'no-cache' def no_cache? matches?(/\bno-cache\b/i) end + # @return [Boolean] does the cache control include 'no-stor' def no_store? matches?(/\bno-store\b/i) end + # @return [Boolean] does the cache control include 'public' def public? matches?(/\bpublic\b/i) end + # @return [Boolean] does the cache control include 'private' def private? matches?(/\bprivate\b/i) end + # @return [Numeric] the max number of seconds this message is + # considered fresh. def max_age explicit_max_age || seconds_til_expires || Float::INFINITY end + # @return [Boolean] is the vary header set to '*' def vary_star? headers.get("Vary").any? { |v| "*" == v.strip } end @@ -42,10 +47,14 @@ def vary_star? attr_reader :headers + # @return [Boolean] true when cache-control header matches the pattern def matches?(pattern) headers.get("Cache-Control").any? { |v| v =~ pattern } end + # @return [Numeric] number of seconds until the time in the + # expires header is reached. + # # --- # Some servers send a "Expire: -1" header which must be treated as expired def seconds_til_expires @@ -57,12 +66,14 @@ def seconds_til_expires .max end + # @return [Time] parses t_str at a time; if that fails returns epoch time def to_time_or_epoch(t_str) Time.httpdate(t_str) rescue ArgumentError Time.at(0) end + # @return [Numeric] the value of the max-age component of cache control def explicit_max_age headers.get("Cache-Control") .map { |v| (/max-age=(\d+)/i).match(v) } @@ -70,6 +81,11 @@ def explicit_max_age .map { |m| m[1].to_i } .max end + + # Inits a new instance + def initialize(message) + @headers = message.headers + end end end end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb index 19c0e2b8..8119fee2 100644 --- a/lib/http/cache/collection.rb +++ b/lib/http/cache/collection.rb @@ -2,36 +2,43 @@ module HTTP class Cache + # Collection of all entries in the cache. class CacheEntryCollection include Enumerable - def initialize - @entries = [] - end - + # @yield [CacheEntry] each successive entry in the cache. def each(&block) @entries.each(&block) end + # @return [Response] the response for the request or nil if + # there isn't one. def [](request) entry = detect { |e| e.valid_for?(request) } entry.response if entry end + # @return [Response] the specified response after inserting it + # into the cache. def []=(request, response) @entries.delete_if { |entry| entry.valid_for?(request) } @entries << CacheEntry.new(request, response) response end + + protected + + def initialize + @entries = [] + end end + # An entry for a single response in the cache class CacheEntry attr_reader :request, :response - def initialize(request, response) - @request, @response = request, response - end - + # @return [Boolean] true iff this entry is valid for the + # request. def valid_for?(request) request.uri == @request.uri && select_request_headers.all? do |key, value| @@ -39,6 +46,10 @@ def valid_for?(request) end end + protected + + # @return [Hash] the headers that matter of matching requests to + # this response. def select_request_headers headers = HTTP::Headers.new @@ -49,6 +60,10 @@ def select_request_headers headers.to_h end + + def initialize(request, response) + @request, @response = request, response + end end end end diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb index 747f2078..636abd3f 100644 --- a/lib/http/cache/in_memory_cache.rb +++ b/lib/http/cache/in_memory_cache.rb @@ -4,11 +4,9 @@ module HTTP class Cache class InMemoryCache - def initialize - @mutex = Mutex.new - @collection = Hash.new { |h, k| h[k] = CacheEntryCollection.new } - end + # @return [Response] the response for the request or nil if one + # isn't found def lookup(request) @mutex.synchronize do response = @collection[request.uri.to_s][request] @@ -17,17 +15,27 @@ def lookup(request) end end + # Stores response to be looked up later. def store(request, response) @mutex.synchronize do @collection[request.uri.to_s][request] = response end end + # Invalidates the all responses from the specified resource. def invalidate(uri) @mutex.synchronize do @collection.delete(uri.to_s) end end + + protected + + def initialize + @mutex = Mutex.new + @collection = Hash.new { |h, k| h[k] = CacheEntryCollection.new } + end + end end end diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb index ade96e11..a4e31123 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -11,8 +11,11 @@ class RequestWithCacheBehavior < DelegateClass(HTTP::Request) class << self protected :new - # Returns a instance of self by wrapping `another` a new - # instance of self or by just returning it + # @return [RequestWithCacheBehavior]] a instance of self by + # wrapping `another` a new instance of self or by just + # returning it + # + # @api public def coerce(another) if another.respond_to? :cacheable? another @@ -22,31 +25,43 @@ def coerce(another) end end - # When was this request sent to the server. + # When was this request sent to the server + # + # @api public attr_accessor :sent_at - # Returns true iff request demands the resources cache entry be invalidated. + # @return [Boolean] true iff request demands the resources cache entry be invalidated + # + # @api public def invalidates_cache? INVALIDATING_METHODS.include?(verb) || cache_control.no_store? end - # Returns true iff request is cacheable + # @return [Boolean] true iff request is cacheable + # + # @api public def cacheable? CACHEABLE_METHODS.include?(verb) && !cache_control.no_store? end - # Returns true iff the cache control info of this request - # demands that the response be revalidated by the origin server. + # @return [Boolean] true iff the cache control info of this + # request demands that the response be revalidated by the origin + # server. + # + # @api public def skips_cache? 0 == cache_control.max_age || cache_control.must_revalidate? || cache_control.no_cache? end - # Returns new request based on this one but conditional on the - # resource having changed since `cached_response` + # @return [RequestWithCacheBehavior] new request based on this + # one but conditional on the resource having changed since + # `cached_response` + # + # @api public def conditional_on_changes_to(cached_response) raw_cond_req = HTTP::Request.new(verb, uri, headers.merge(conditional_headers_for(cached_response)), @@ -55,13 +70,16 @@ def conditional_on_changes_to(cached_response) self.class.coerce(raw_cond_req) end - # Returns cache control helper for this request. + # @return [CacheControl] cache control helper for this request + # @api public def cache_control @cache_control ||= CacheControl.new(self) end protected + # @return [Headers] conditional request headers + # @api private def conditional_headers_for(cached_response) headers = HTTP::Headers.new @@ -76,6 +94,8 @@ def conditional_headers_for(cached_response) headers end + # Inits a new instance + # @api private def initialize(obj) super @requested_at = nil diff --git a/lib/http/cache/response_with_cache_behavior.rb b/lib/http/cache/response_with_cache_behavior.rb index 3eb5455c..d67dc27d 100644 --- a/lib/http/cache/response_with_cache_behavior.rb +++ b/lib/http/cache/response_with_cache_behavior.rb @@ -10,7 +10,7 @@ class ResponseWithCacheBehavior < DelegateClass(HTTP::Response) class << self protected :new - # Returns a instance of self by wrapping `another` a new + # @return [Boolean] a instance of self by wrapping `another` a new # instance of self or by just returning it def coerce(another) if another.respond_to? :cacheable? @@ -21,17 +21,17 @@ def coerce(another) end end - # Returns true iff this response is stale; otherwise false + # @return [Boolean] true iff this response is stale def stale? expired? || cache_control.must_revalidate? end - # Returns true iff this response has expired; otherwise false + # @returns [Boolean] true iff this response has expired def expired? current_age > cache_control.max_age end - # Return true iff this response is cacheable; otherwise false + # @return [Boolean] true iff this response is cacheable # # --- # A Vary header field-value of "*" always fails to match and @@ -47,7 +47,7 @@ def cacheable? end end - # Returns the current age (in seconds) of this response + # @return [Numeric] the current age (in seconds) of this response # # --- # Algo from https://tools.ietf.org/html/rfc2616#section-13.2.3 @@ -63,13 +63,13 @@ def current_age corrected_initial_age + resident_time end - # Returns the time at which this response was requested + # @return [Time] the time at which this response was requested def requested_at @requested_at ||= Time.now end attr_writer :requested_at - # Returns the time at which this response was received + # @return [Time] the time at which this response was received def received_at @received_at || Time.now end @@ -84,14 +84,14 @@ def validated!(validating_response) self.authoritative = true end - # Returns cache control helper object. + # @return [CacheControl] cache control helper object. def cache_control @cache_control ||= CacheControl.new(self) end protected - # Returns the time at which the server generated this response. + # @return [Time] the time at which the server generated this response. def server_response_time headers.get("Date") .map(&method(:to_time_or_epoch)) From 4c10ea802ea72fa4a886a86571f4b9bc1acf1486 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Tue, 3 Feb 2015 22:21:22 -0700 Subject: [PATCH 18/39] blank lines break builds, apparently --- lib/http/cache/in_memory_cache.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb index 636abd3f..b8d33d51 100644 --- a/lib/http/cache/in_memory_cache.rb +++ b/lib/http/cache/in_memory_cache.rb @@ -4,7 +4,6 @@ module HTTP class Cache class InMemoryCache - # @return [Response] the response for the request or nil if one # isn't found def lookup(request) @@ -35,7 +34,6 @@ def initialize @mutex = Mutex.new @collection = Hash.new { |h, k| h[k] = CacheEntryCollection.new } end - end end end From bcf947e9fa56c20e3af6598f7e26a7880d67c4fd Mon Sep 17 00:00:00 2001 From: Alexey V Zapparov Date: Wed, 4 Feb 2015 14:48:03 +0100 Subject: [PATCH 19/39] Enable align parameters cop back again --- .rubocop.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f471e2d7..d603c46b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -59,6 +59,3 @@ Style/StringLiterals: Style/TrivialAccessors: Enabled: false - -Style/AlignParameters: - Enabled: false \ No newline at end of file From 69dfa0ade6164c0bb626009c0011f538d63464a3 Mon Sep 17 00:00:00 2001 From: Alexey V Zapparov Date: Wed, 4 Feb 2015 15:33:46 +0100 Subject: [PATCH 20/39] Rename *WithCacheBehavior to *::Cached --- lib/http/cache.rb | 22 +++---- lib/http/request.rb | 6 ++ .../cached.rb} | 64 ++++++++----------- lib/http/response.rb | 6 ++ .../cached.rb} | 45 ++++++------- spec/lib/http/cache_spec.rb | 12 ++-- spec/lib/http/client_spec.rb | 2 +- .../cached_spec.rb} | 26 +++----- spec/lib/http/request_spec.rb | 5 ++ .../cached_spec.rb} | 20 ++---- spec/lib/http/response_spec.rb | 8 +++ 11 files changed, 99 insertions(+), 117 deletions(-) rename lib/http/{cache/request_with_cache_behavior.rb => request/cached.rb} (64%) rename lib/http/{cache/response_with_cache_behavior.rb => response/cached.rb} (77%) rename spec/lib/http/{cache/request_with_cache_behavior_spec.rb => request/cached_spec.rb} (86%) rename spec/lib/http/{cache/response_with_cache_behavior_spec.rb => response/cached_spec.rb} (91%) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 1af772fe..dd29ab07 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,7 +1,5 @@ require "time" require "http/cache/cache_control" -require "http/cache/response_with_cache_behavior" -require "http/cache/request_with_cache_behavior" module HTTP class Cache @@ -20,7 +18,7 @@ def perform(request, options) # @yield [request, options] on cache miss so that an actual # request can be made def perform(request, options, &request_performer) - req = RequestWithCacheBehavior.coerce(request) + req = request.cached invalidate_cache(req) if req.invalidates_cache? @@ -64,26 +62,22 @@ def handle_response(cached_resp, actual_resp, req) end end - # @return [ResponseWithCacheBehavior] the actual response returned + # @return [HTTP::Response::Cached] the actual response returned # by request_performer def make_request(req, options, request_performer) req.sent_at = Time.now - ResponseWithCacheBehavior.coerce(request_performer.call(req, options)).tap do |resp| - resp.received_at = Time.now - resp.requested_at = req.sent_at + + request_performer.call(req, options).cached.tap do |res| + res.received_at = Time.now + res.requested_at = req.sent_at end end - # @return [ResponseWithCacheBehavior] the cached response for the - # request or nil if there isn't one + # @return [HTTP::Response::Cached, nil] the cached response for the request def cache_lookup(request) return nil if request.skips_cache? c = @cache_adapter.lookup(request) - if c - ResponseWithCacheBehavior.coerce(c) - else - nil - end + c && c.cached end # Store response in cache diff --git a/lib/http/request.rb b/lib/http/request.rb index f052c96b..599c48f7 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -1,5 +1,6 @@ require "http/errors" require "http/headers" +require "http/request/cached" require "http/request/writer" require "http/version" require "base64" @@ -130,6 +131,11 @@ def socket_port using_proxy? ? proxy[:proxy_port] : uri.port end + # @return [HTTP::Request::Cached] + def cached + Cached.new self + end + private def path_for_request_header diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/request/cached.rb similarity index 64% rename from lib/http/cache/request_with_cache_behavior.rb rename to lib/http/request/cached.rb index a4e31123..9c6e3f8c 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/request/cached.rb @@ -1,35 +1,31 @@ -require "http/request" +require "http/cache/cache_control" module HTTP - class Cache + class Request # Decorator class for requests to provide convenience methods - # related to caching. Instantiate using the `.coerce` method. - class RequestWithCacheBehavior < DelegateClass(HTTP::Request) + # related to caching. + class Cached < DelegateClass(HTTP::Request) INVALIDATING_METHODS = [:post, :put, :delete, :patch].freeze CACHEABLE_METHODS = [:get, :head].freeze - class << self - protected :new - - # @return [RequestWithCacheBehavior]] a instance of self by - # wrapping `another` a new instance of self or by just - # returning it - # - # @api public - def coerce(another) - if another.respond_to? :cacheable? - another - else - new(another) - end - end - end - # When was this request sent to the server # # @api public attr_accessor :sent_at + # Inits a new instance + # @api private + def initialize(obj) + super + @requested_at = nil + @received_at = nil + end + + # @return [HTTP::Request::Cached] + def cached + self + end + # @return [Boolean] true iff request demands the resources cache entry be invalidated # # @api public @@ -38,7 +34,7 @@ def invalidates_cache? cache_control.no_store? end - # @return [Boolean] true iff request is cacheable + # @return [Boolean] true if request is cacheable # # @api public def cacheable? @@ -57,26 +53,24 @@ def skips_cache? cache_control.no_cache? end - # @return [RequestWithCacheBehavior] new request based on this + # @return [HTTP::Request::Cached] new request based on this # one but conditional on the resource having changed since # `cached_response` # # @api public def conditional_on_changes_to(cached_response) - raw_cond_req = HTTP::Request.new(verb, uri, - headers.merge(conditional_headers_for(cached_response)), - proxy, body, version) - - self.class.coerce(raw_cond_req) + self.class.new HTTP::Request.new( + verb, uri, headers.merge(conditional_headers_for(cached_response)), + proxy, body, version) end - # @return [CacheControl] cache control helper for this request + # @return [HTTP::Cache::CacheControl] cache control helper for this request # @api public def cache_control - @cache_control ||= CacheControl.new(self) + @cache_control ||= HTTP::Cache::CacheControl.new(self) end - protected + private # @return [Headers] conditional request headers # @api private @@ -93,14 +87,6 @@ def conditional_headers_for(cached_response) headers end - - # Inits a new instance - # @api private - def initialize(obj) - super - @requested_at = nil - @received_at = nil - end end end end diff --git a/lib/http/response.rb b/lib/http/response.rb index e049c8d1..b7d11747 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -3,6 +3,7 @@ require "http/headers" require "http/content_type" require "http/mime_type" +require "http/response/cached" require "http/response/status" require "time" @@ -106,5 +107,10 @@ def parse(as = nil) def inspect "#<#{self.class}/#{@version} #{code} #{reason} #{headers.to_h.inspect}>" end + + # @return [HTTP::Response::Cached] + def cached + Cached.new self + end end end diff --git a/lib/http/cache/response_with_cache_behavior.rb b/lib/http/response/cached.rb similarity index 77% rename from lib/http/cache/response_with_cache_behavior.rb rename to lib/http/response/cached.rb index d67dc27d..9a5810ba 100644 --- a/lib/http/cache/response_with_cache_behavior.rb +++ b/lib/http/response/cached.rb @@ -1,24 +1,21 @@ -require "http/response" +require "http/cache/cache_control" module HTTP - class Cache + class Response # Decorator class for responses to provide convenience methods - # related to caching. Instantiate using the `.coerce` method. - class ResponseWithCacheBehavior < DelegateClass(HTTP::Response) + # related to caching. + class Cached < DelegateClass(HTTP::Response) CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410].freeze - class << self - protected :new + def initialize(obj) + super + @requested_at = nil + @received_at = nil + end - # @return [Boolean] a instance of self by wrapping `another` a new - # instance of self or by just returning it - def coerce(another) - if another.respond_to? :cacheable? - another - else - new(another) - end - end + # @return [HTTP::Response::Cached] + def cached + self end # @return [Boolean] true iff this response is stale @@ -40,10 +37,10 @@ def expired? def cacheable? @cacheable ||= begin - CACHEABLE_RESPONSE_CODES.include?(code) && - !(cache_control.vary_star? || - cache_control.no_store? || - cache_control.no_cache?) + CACHEABLE_RESPONSE_CODES.include?(code) \ + && !(cache_control.vary_star? || + cache_control.no_store? || + cache_control.no_cache?) end end @@ -84,9 +81,9 @@ def validated!(validating_response) self.authoritative = true end - # @return [CacheControl] cache control helper object. + # @return [HTTP::Cache::CacheControl] cache control helper object. def cache_control - @cache_control ||= CacheControl.new(self) + @cache_control ||= HTTP::Cache::CacheControl.new(self) end protected @@ -107,12 +104,6 @@ def to_time_or_epoch(t_str) rescue ArgumentError Time.at(0) end - - def initialize(obj) - super - @requested_at = nil - @received_at = nil - end end end end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 92e98fa2..0e811247 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -30,12 +30,10 @@ context "cache hit" do let(:cached_response) do - HTTP::Cache::ResponseWithCacheBehavior.coerce( - HTTP::Response.new(200, - "http/1.1", - {"Cache-Control" => "private", "test" => "foo"}, - "") - ).tap { |r| r.requested_at = r.received_at = Time.now } + headers = {"Cache-Control" => "private", "test" => "foo"} + HTTP::Response.new(200, "http/1.1", headers, "").cached.tap do |r| + r.requested_at = r.received_at = Time.now + end end it "does not call request_performer block" do @@ -244,7 +242,7 @@ let(:cached_response) { nil } # cold cache by default def build_cached_response(*args) - r = HTTP::Cache::ResponseWithCacheBehavior.coerce(HTTP::Response.new(*args)) + r = HTTP::Response.new(*args).cached r.requested_at = r.received_at = Time.now yield r if block_given? diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 935e13c7..37401be3 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -73,7 +73,7 @@ def simple_response(body, status = 200) describe "caching" do it "returns cached responses if they exist" do - cached_response = HTTP::Cache::ResponseWithCacheBehavior.coerce(simple_response("OK")) + cached_response = simple_response("OK").cached adapter = double("persistance_adapter", :lookup => cached_response) client = StubbedClient.new(:cache => HTTP::Cache.new(adapter)).stub( "http://example.com/" => simple_response("OK") diff --git a/spec/lib/http/cache/request_with_cache_behavior_spec.rb b/spec/lib/http/request/cached_spec.rb similarity index 86% rename from spec/lib/http/cache/request_with_cache_behavior_spec.rb rename to spec/lib/http/request/cached_spec.rb index de3dabbb..c8560c36 100644 --- a/spec/lib/http/cache/request_with_cache_behavior_spec.rb +++ b/spec/lib/http/request/cached_spec.rb @@ -1,16 +1,5 @@ -RSpec.describe HTTP::Cache::RequestWithCacheBehavior do - describe ".coerce" do - it "should accept a base request" do - expect(described_class.coerce(request)).to be_kind_of described_class - end - - it "should accept an already decorated request" do - decorated_req = described_class.coerce(request) - expect(decorated_req).to be_kind_of described_class - end - end - - subject { described_class.coerce(request) } +RSpec.describe HTTP::Request::Cached do + subject { described_class.new request } it "provides access to it's cache control object" do expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl @@ -30,11 +19,11 @@ end it "can construct a new conditional version of itself based on a cached response" do - mod_date = Time.now.httpdate + mod_date = Time.now.httpdate cached_resp = HTTP::Response.new(200, "http/1.1", {"Etag" => "foo", "Last-Modified" => mod_date}, - "") + "") cond_req = subject.conditional_on_changes_to(cached_resp) expect(cond_req.headers["If-None-Match"]).to eq "foo" @@ -66,7 +55,7 @@ cached_resp = HTTP::Response.new(200, "http/1.1", {"Etag" => "foo", "Last-Modified" => mod_date}, - "") + "") cond_req = subject.conditional_on_changes_to(cached_resp) expect(cond_req.headers["If-None-Match"]).to eq "foo" expect(cond_req.headers["If-Modified-Since"]).to eq mod_date @@ -140,4 +129,9 @@ # Background let(:request) { HTTP::Request.new(:get, "http://example.com/") } + + describe "#cached" do + subject(:cached_request) { request.cached } + it { is_expected.to be cached_request } + end end diff --git a/spec/lib/http/request_spec.rb b/spec/lib/http/request_spec.rb index c6091c75..3ccb0bf9 100644 --- a/spec/lib/http/request_spec.rb +++ b/spec/lib/http/request_spec.rb @@ -138,4 +138,9 @@ its(:verb) { is_expected.to be :get } end end + + describe "#cached" do + subject { request.cached } + it { is_expected.to be_a HTTP::Request::Cached } + end end diff --git a/spec/lib/http/cache/response_with_cache_behavior_spec.rb b/spec/lib/http/response/cached_spec.rb similarity index 91% rename from spec/lib/http/cache/response_with_cache_behavior_spec.rb rename to spec/lib/http/response/cached_spec.rb index 4be8f515..b55408b8 100644 --- a/spec/lib/http/cache/response_with_cache_behavior_spec.rb +++ b/spec/lib/http/response/cached_spec.rb @@ -1,16 +1,5 @@ -RSpec.describe HTTP::Cache::ResponseWithCacheBehavior do - describe ".coerce" do - it "should accept a base response" do - expect(described_class.coerce(response)).to be_kind_of described_class - end - - it "should accept an already decorated response" do - decorated_req = described_class.coerce(response) - expect(decorated_req).to be_kind_of described_class - end - end - - subject { described_class.coerce(response) } +RSpec.describe HTTP::Response::Cached do + subject { described_class.new response } it "provides access to it's cache control object" do expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl @@ -204,4 +193,9 @@ let(:cache_control) { "" } let(:headers) { {"cache-control" => cache_control} } let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } + + describe "#cached" do + subject(:cached_response) { response.cached } + it { is_expected.to be cached_response } + end end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index e7cd373d..42a0eba4 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -1,4 +1,7 @@ RSpec.describe HTTP::Response do + let(:body) { "Hello world!" } + subject(:response) { HTTP::Response.new 200, "1.1", {}, body } + it "includes HTTP::Headers::Mixin" do expect(described_class).to include HTTP::Headers::Mixin end @@ -106,4 +109,9 @@ .to eq '#"text/plain"}>' end end + + describe "#cached" do + subject { response.cached } + it { is_expected.to be_a HTTP::Response::Cached } + end end From 156ce9dfd71cd71cd052e20ffbce59c408ebd19f Mon Sep 17 00:00:00 2001 From: Alexey V Zapparov Date: Wed, 4 Feb 2015 16:15:58 +0100 Subject: [PATCH 21/39] Refacor CacheControl helper. Renamed it to Cache::Headers --- expected_results | 55 ++++++++ lib/http/cache.rb | 2 +- .../cache/{cache_control.rb => headers.rb} | 32 +++-- lib/http/request/cached.rb | 20 +-- lib/http/response/cached.rb | 18 +-- spec/lib/http/cache/cache_control_spec.rb | 124 ------------------ spec/lib/http/cache/headers_spec.rb | 77 +++++++++++ spec/lib/http/cache_spec.rb | 5 +- spec/lib/http/request/cached_spec.rb | 31 ++--- spec/lib/http/response/cached_spec.rb | 7 +- 10 files changed, 191 insertions(+), 180 deletions(-) create mode 100644 expected_results rename lib/http/cache/{cache_control.rb => headers.rb} (83%) delete mode 100644 spec/lib/http/cache/cache_control_spec.rb create mode 100644 spec/lib/http/cache/headers_spec.rb diff --git a/expected_results b/expected_results new file mode 100644 index 00000000..dda049d3 --- /dev/null +++ b/expected_results @@ -0,0 +1,55 @@ + +POST /1416179329 HTTP/1.1 +Host: localhost:8080 +User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0 Iceweasel/32.0 +Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 +Accept-Language: en-US,en;q=0.5 +Accept-Encoding: gzip, deflate +DNT: 1 +Referer: http://localhost:8080/1416179301 +Cookie: __utma=111872281.776418867.1390272758.1415735077.1415742340.6; __utmz=111872281.1390272758.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); _jsuid=2367279670; sid=1bad10a0201c253cfaf0bfebe50405fef82b2267; _vidiq.sess=BAh7CUkiDHVzZXJfaWQGOgZFVCIdNTQ1YThlZjg5N2YwMTQwNzk1MDAwMDAxSSIPY3JlYXRlZF9hdAY7AFRsKwcjj1pUSSIPc2Vzc2lvbl9pZAY7AFRJIiU3YTE4NGQ0YTc1ZTI0YzEwOGFhNjZmYzNkOTQ3NjkzNAY7AFRJIhFsYXN0X3JlcXVlc3QGOwBUbCsHspNaVA%3D%3D--2d06b2c91f7eddd469d2781ab14a57759dc44543; locale=en-US; _vidiq.auth=1; utag_main=v_id:0149448f5106001d3fd3082bf59d0405300160090086e$_sn:1$_ss:0$_pn:5%3Bexp-session$_st:1414196750542$ses_id:1414194483462%3Bexp-session; s_fid=7486C318933E2879-25F3BFA3ED8EC585; s_nr=1414194950805-New +Connection: keep-alive +Content-Type: multipart/form-data; boundary=---------------------------38016663816079156891839870716 +Content-Length: 1360 + +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="t1" + +test_1 +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="t2" + +test_2 +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="t2" + +test_3 +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="f1"; filename="index.slim" +Content-Type: application/octet-stream + +doctype html +html + body + form action="http://localhost" method="POST" + + input type="text" name="i1" + input type="text" name="i2" + input type="file" name="f1" + textarea name="t1" + + input type="submit" + +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="f2"; filename="index.html" +Content-Type: text/html + +
+
+ +-----------------------------38016663816079156891839870716 +Content-Disposition: form-data; name="f2"; filename="" +Content-Type: application/octet-stream + + +-----------------------------38016663816079156891839870716-- diff --git a/lib/http/cache.rb b/lib/http/cache.rb index dd29ab07..6e09437b 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,5 +1,5 @@ require "time" -require "http/cache/cache_control" +require "http/cache/in_memory_cache" module HTTP class Cache diff --git a/lib/http/cache/cache_control.rb b/lib/http/cache/headers.rb similarity index 83% rename from lib/http/cache/cache_control.rb rename to lib/http/cache/headers.rb index 60370192..02f1df18 100644 --- a/lib/http/cache/cache_control.rb +++ b/lib/http/cache/headers.rb @@ -1,7 +1,20 @@ +require "delegate" + +require "http/errors" +require "http/headers" + module HTTP class Cache # Convenience methods around cache control headers. - class CacheControl + class Headers < ::SimpleDelegator + def initialize(headers) + if headers.is_a? HTTP::Headers + super headers + else + super HTTP::Headers.coerce headers + end + end + # @return [Boolean] does this message force revalidation def forces_revalidation? must_revalidate? || max_age == 0 @@ -40,16 +53,14 @@ def max_age # @return [Boolean] is the vary header set to '*' def vary_star? - headers.get("Vary").any? { |v| "*" == v.strip } + get("Vary").any? { |v| "*" == v.strip } end - protected - - attr_reader :headers + private # @return [Boolean] true when cache-control header matches the pattern def matches?(pattern) - headers.get("Cache-Control").any? { |v| v =~ pattern } + get("Cache-Control").any? { |v| v =~ pattern } end # @return [Numeric] number of seconds until the time in the @@ -58,7 +69,7 @@ def matches?(pattern) # --- # Some servers send a "Expire: -1" header which must be treated as expired def seconds_til_expires - headers.get("Expires") + get("Expires") .map(&method(:to_time_or_epoch)) .compact .map { |e| e - Time.now } @@ -75,17 +86,12 @@ def to_time_or_epoch(t_str) # @return [Numeric] the value of the max-age component of cache control def explicit_max_age - headers.get("Cache-Control") + get("Cache-Control") .map { |v| (/max-age=(\d+)/i).match(v) } .compact .map { |m| m[1].to_i } .max end - - # Inits a new instance - def initialize(message) - @headers = message.headers - end end end end diff --git a/lib/http/request/cached.rb b/lib/http/request/cached.rb index 9c6e3f8c..6c007699 100644 --- a/lib/http/request/cached.rb +++ b/lib/http/request/cached.rb @@ -1,4 +1,4 @@ -require "http/cache/cache_control" +require "http/cache/headers" module HTTP class Request @@ -31,7 +31,7 @@ def cached # @api public def invalidates_cache? INVALIDATING_METHODS.include?(verb) || - cache_control.no_store? + cache_headers.no_store? end # @return [Boolean] true if request is cacheable @@ -39,7 +39,7 @@ def invalidates_cache? # @api public def cacheable? CACHEABLE_METHODS.include?(verb) && - !cache_control.no_store? + !cache_headers.no_store? end # @return [Boolean] true iff the cache control info of this @@ -48,9 +48,9 @@ def cacheable? # # @api public def skips_cache? - 0 == cache_control.max_age || - cache_control.must_revalidate? || - cache_control.no_cache? + 0 == cache_headers.max_age || + cache_headers.must_revalidate? || + cache_headers.no_cache? end # @return [HTTP::Request::Cached] new request based on this @@ -64,10 +64,10 @@ def conditional_on_changes_to(cached_response) proxy, body, version) end - # @return [HTTP::Cache::CacheControl] cache control helper for this request + # @return [HTTP::Cache::Headers] cache control helper for this request # @api public - def cache_control - @cache_control ||= HTTP::Cache::CacheControl.new(self) + def cache_headers + @cache_headers ||= HTTP::Cache::Headers.new headers end private @@ -83,7 +83,7 @@ def conditional_headers_for(cached_response) cached_response.headers.get("Last-Modified") .each { |last_mod| headers.add("If-Modified-Since", last_mod) } - headers.add("Cache-Control", "max-age=0") if cache_control.forces_revalidation? + headers.add("Cache-Control", "max-age=0") if cache_headers.forces_revalidation? headers end diff --git a/lib/http/response/cached.rb b/lib/http/response/cached.rb index 9a5810ba..4be1a06e 100644 --- a/lib/http/response/cached.rb +++ b/lib/http/response/cached.rb @@ -1,4 +1,4 @@ -require "http/cache/cache_control" +require "http/cache/headers" module HTTP class Response @@ -20,12 +20,12 @@ def cached # @return [Boolean] true iff this response is stale def stale? - expired? || cache_control.must_revalidate? + expired? || cache_headers.must_revalidate? end # @returns [Boolean] true iff this response has expired def expired? - current_age > cache_control.max_age + current_age > cache_headers.max_age end # @return [Boolean] true iff this response is cacheable @@ -38,9 +38,9 @@ def cacheable? @cacheable ||= begin CACHEABLE_RESPONSE_CODES.include?(code) \ - && !(cache_control.vary_star? || - cache_control.no_store? || - cache_control.no_cache?) + && !(cache_headers.vary_star? || + cache_headers.no_store? || + cache_headers.no_cache?) end end @@ -81,9 +81,9 @@ def validated!(validating_response) self.authoritative = true end - # @return [HTTP::Cache::CacheControl] cache control helper object. - def cache_control - @cache_control ||= HTTP::Cache::CacheControl.new(self) + # @return [HTTP::Cache::Headers] cache control headers helper object. + def cache_headers + @cache_headers ||= HTTP::Cache::Headers.new headers end protected diff --git a/spec/lib/http/cache/cache_control_spec.rb b/spec/lib/http/cache/cache_control_spec.rb deleted file mode 100644 index b9bd37f7..00000000 --- a/spec/lib/http/cache/cache_control_spec.rb +++ /dev/null @@ -1,124 +0,0 @@ -RSpec.describe HTTP::Cache::CacheControl do - describe ".new" do - it "accepts a request" do - expect { described_class.new request }.not_to raise_error - end - - it "accepts a request" do - expect { described_class.new response }.not_to raise_error - end - - it "it rejects any object that does not respond to #headers" do - expect { described_class.new double }.to raise_error - end - end - - subject { described_class.new response } - - context "cache-control: private" do - let(:cache_control) { "private" } - - it "know it is private" do - expect(subject.private?).to be_truthy - end - end - - context "cache-control: public" do - let(:cache_control) { "public" } - - it "know it is public" do - expect(subject.public?).to be_truthy - end - end - - context "cache-control: no-cache" do - let(:cache_control) { "no-cache" } - - it "know it is no-cache" do - expect(subject.no_cache?).to be_truthy - end - end - - context "cache-control: no-store" do - let(:cache_control) { "no-store" } - - it "know it is no-store" do - expect(subject.no_store?).to be_truthy - end - end - - context "cache-control: max-age=100" do - let(:cache_control) { "max-age=100" } - - it "knows max age" do - expect(subject.max_age).to eq 100 - end - end - - context "expires: {100 seconds from now}" do - let(:headers) { {"Expires" => (Time.now + 100).httpdate} } - - it "knows max age" do - expect(subject.max_age).to be_within(1).of(100) - end - end - - context "expires: {100 seconds before now}" do - let(:headers) { {"Expires" => (Time.now - 100).httpdate} } - - it "knows max age" do - expect(subject.max_age).to eq 0 - end - end - - context "expires: -1" do - let(:headers) { {"Expires" => "-1"} } - - it "knows max age" do - expect(subject.max_age).to eq 0 - end - end - - context "vary: *" do - let(:headers) { {"Vary" => "*"} } - - it "knows it is vary *" do - expect(subject.vary_star?).to be_truthy - end - end - - context "no headers" do - let(:headers) { {} } - - it "knows max age" do - expect(subject.max_age).to eq Float::INFINITY - end - - it "know it is private" do - expect(subject.private?).to be_falsy - end - - it "know it is public" do - expect(subject.public?).to be_falsy - end - - it "know it is no-cache" do - expect(subject.no_cache?).to be_falsy - end - - it "know it is no-store" do - expect(subject.no_store?).to be_falsy - end - - it "knows it is not vary *" do - expect(subject.vary_star?).to be_falsy - end - end - - # Background - let(:cache_control) { "private" } - let(:headers) { {"Cache-Control" => cache_control} } - let(:request) { HTTP::Request.new(:get, "http://example.com/") } - - let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } -end diff --git a/spec/lib/http/cache/headers_spec.rb b/spec/lib/http/cache/headers_spec.rb new file mode 100644 index 00000000..f2d7dc9e --- /dev/null +++ b/spec/lib/http/cache/headers_spec.rb @@ -0,0 +1,77 @@ +RSpec.describe HTTP::Cache::Headers do + subject(:cache_headers) { described_class.new headers } + + describe ".new" do + it "accepts instance of HTTP::Headers" do + expect { described_class.new HTTP::Headers.new }.not_to raise_error + end + + it "it rejects any object that does not respond to #headers" do + expect { described_class.new double }.to raise_error HTTP::Error + end + end + + context "with " do + let(:headers) { {"Cache-Control" => "private"} } + it { is_expected.to be_private } + end + + context "with " do + let(:headers) { {"Cache-Control" => "public"} } + it { is_expected.to be_public } + end + + context "with " do + let(:headers) { {"Cache-Control" => "no-cache"} } + it { is_expected.to be_no_cache } + end + + context "with " do + let(:headers) { {"Cache-Control" => "no-store"} } + it { is_expected.to be_no_store } + end + + describe "#max_age" do + subject { cache_headers.max_age } + + context "with " do + let(:headers) { {"Cache-Control" => "max-age=100"} } + it { is_expected.to eq 100 } + end + + context "with " do + let(:headers) { {"Expires" => (Time.now + 100).httpdate} } + it { is_expected.to be_within(1).of(100) } + end + + context "with " do + let(:headers) { {"Expires" => (Time.now - 100).httpdate} } + it { is_expected.to eq 0 } + end + + context "with " do + let(:headers) { {"Expires" => "-1"} } + it { is_expected.to eq 0 } + end + end + + context "with " do + let(:headers) { {"Vary" => "*"} } + it { is_expected.to be_vary_star } + end + + context "with no cache related headers" do + let(:headers) { {} } + + it { is_expected.not_to be_private } + it { is_expected.not_to be_public } + it { is_expected.not_to be_no_cache } + it { is_expected.not_to be_no_store } + it { is_expected.not_to be_vary_star } + + describe "#max_age" do + subject { cache_headers.max_age } + it { is_expected.to eq Float::INFINITY } + end + end +end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 0e811247..f565a767 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -60,9 +60,8 @@ context "cache by-passing request, cacheable response" do let(:request) do - HTTP::Request.new(:get, - "http://example.com/", - "Cache-Control" => "no-cache") + headers = {"Cache-Control" => "no-cache"} + HTTP::Request.new(:get, "http://example.com/", headers) end let!(:response) { subject.perform(request, opts) { origin_response } } diff --git a/spec/lib/http/request/cached_spec.rb b/spec/lib/http/request/cached_spec.rb index c8560c36..61695e8b 100644 --- a/spec/lib/http/request/cached_spec.rb +++ b/spec/lib/http/request/cached_spec.rb @@ -1,8 +1,9 @@ RSpec.describe HTTP::Request::Cached do - subject { described_class.new request } + subject(:cached_request) { described_class.new request } - it "provides access to it's cache control object" do - expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl + describe "#cache_headers" do + subject { cached_request.cache_headers } + it { is_expected.to be_a HTTP::Cache::Headers } end context "basic GET request" do @@ -20,10 +21,8 @@ it "can construct a new conditional version of itself based on a cached response" do mod_date = Time.now.httpdate - cached_resp = HTTP::Response.new(200, "http/1.1", - {"Etag" => "foo", - "Last-Modified" => mod_date}, - "") + headers = {"Etag" => "foo", "Last-Modified" => mod_date} + cached_resp = HTTP::Response.new(200, "http/1.1", headers, "") cond_req = subject.conditional_on_changes_to(cached_resp) expect(cond_req.headers["If-None-Match"]).to eq "foo" @@ -33,9 +32,8 @@ context "GET request w/ must-revalidate" do let(:request) do - HTTP::Request.new(:get, - "http://example.com/", - "cache-control" => "must-revalidate") + headers = {"cache-control" => "must-revalidate"} + HTTP::Request.new(:get, "http://example.com/", headers) end it "is cacheable" do @@ -51,15 +49,14 @@ end it "can construct a condition version of itself based on a cached response" do - mod_date = Time.now.httpdate - cached_resp = HTTP::Response.new(200, "http/1.1", - {"Etag" => "foo", - "Last-Modified" => mod_date}, - "") - cond_req = subject.conditional_on_changes_to(cached_resp) + mod_date = Time.now.httpdate + headers = {"Etag" => "foo", "Last-Modified" => mod_date} + cached_resp = HTTP::Response.new(200, "http/1.1", headers, "") + cond_req = subject.conditional_on_changes_to(cached_resp) + expect(cond_req.headers["If-None-Match"]).to eq "foo" expect(cond_req.headers["If-Modified-Since"]).to eq mod_date - expect(cond_req.cache_control.max_age).to eq 0 + expect(cond_req.cache_headers.max_age).to eq 0 end end diff --git a/spec/lib/http/response/cached_spec.rb b/spec/lib/http/response/cached_spec.rb index b55408b8..b897084c 100644 --- a/spec/lib/http/response/cached_spec.rb +++ b/spec/lib/http/response/cached_spec.rb @@ -1,8 +1,9 @@ RSpec.describe HTTP::Response::Cached do - subject { described_class.new response } + subject(:cached_response) { described_class.new response } - it "provides access to it's cache control object" do - expect(subject.cache_control).to be_kind_of HTTP::Cache::CacheControl + describe "#cache_headers" do + subject { cached_response.cache_headers } + it { is_expected.to be_a HTTP::Cache::Headers } end it "allows requested_at to be set" do From 0adc2a109a7d6bd1f35f6dc69846e9d7f8b4f3ce Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Feb 2015 08:19:34 -0700 Subject: [PATCH 22/39] reenabled parameter alignment cop --- .rubocop.yml | 3 --- lib/http/cache/request_with_cache_behavior.rb | 7 +++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f471e2d7..d603c46b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -59,6 +59,3 @@ Style/StringLiterals: Style/TrivialAccessors: Enabled: false - -Style/AlignParameters: - Enabled: false \ No newline at end of file diff --git a/lib/http/cache/request_with_cache_behavior.rb b/lib/http/cache/request_with_cache_behavior.rb index a4e31123..8030f49d 100644 --- a/lib/http/cache/request_with_cache_behavior.rb +++ b/lib/http/cache/request_with_cache_behavior.rb @@ -63,9 +63,12 @@ def skips_cache? # # @api public def conditional_on_changes_to(cached_response) - raw_cond_req = HTTP::Request.new(verb, uri, + raw_cond_req = HTTP::Request.new(verb, + uri, headers.merge(conditional_headers_for(cached_response)), - proxy, body, version) + proxy, + body, + version) self.class.coerce(raw_cond_req) end From d22eaa5e0c3028f5a9b97705be538e2bc9f7fbf0 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Feb 2015 09:28:25 -0700 Subject: [PATCH 23/39] remove a trailing line comment that made yard fail on ruby 2.2.0 --- lib/http/cache/headers.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/http/cache/headers.rb b/lib/http/cache/headers.rb index 02f1df18..503286f4 100644 --- a/lib/http/cache/headers.rb +++ b/lib/http/cache/headers.rb @@ -70,13 +70,16 @@ def matches?(pattern) # Some servers send a "Expire: -1" header which must be treated as expired def seconds_til_expires get("Expires") - .map(&method(:to_time_or_epoch)) - .compact - .map { |e| e - Time.now } - .map { |a| a < 0 ? 0 : a } # age is 0 if it is expired + .map { |e| http_date_to_ttl(e) } .max end + def http_date_to_ttl(t_str) + ttl = to_time_or_epoch(t_str) - Time.now + + ttl < 0 ? 0 : ttl + end + # @return [Time] parses t_str at a time; if that fails returns epoch time def to_time_or_epoch(t_str) Time.httpdate(t_str) From a33190f8971926d756cb1b7d763093658bcc6fa1 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Feb 2015 10:12:49 -0700 Subject: [PATCH 24/39] rename HTTP::{Request,Response}::Cached to Caching --- lib/http/cache.rb | 10 +++++----- lib/http/request.rb | 8 ++++---- lib/http/request/{cached.rb => caching.rb} | 13 ++++++------- lib/http/response.rb | 8 ++++---- lib/http/response/{cached.rb => caching.rb} | 6 +++--- spec/lib/http/cache_spec.rb | 4 ++-- spec/lib/http/client_spec.rb | 2 +- .../request/{cached_spec.rb => caching_spec.rb} | 16 ++++++++-------- spec/lib/http/request_spec.rb | 6 +++--- .../response/{cached_spec.rb => caching_spec.rb} | 12 ++++++------ spec/lib/http/response_spec.rb | 6 +++--- 11 files changed, 45 insertions(+), 46 deletions(-) rename lib/http/request/{cached.rb => caching.rb} (88%) rename lib/http/response/{cached.rb => caching.rb} (96%) rename spec/lib/http/request/{cached_spec.rb => caching_spec.rb} (91%) rename spec/lib/http/response/{cached_spec.rb => caching_spec.rb} (94%) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 6e09437b..b5b99d93 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -18,7 +18,7 @@ def perform(request, options) # @yield [request, options] on cache miss so that an actual # request can be made def perform(request, options, &request_performer) - req = request.cached + req = request.caching invalidate_cache(req) if req.invalidates_cache? @@ -62,22 +62,22 @@ def handle_response(cached_resp, actual_resp, req) end end - # @return [HTTP::Response::Cached] the actual response returned + # @return [HTTP::Response::Caching] the actual response returned # by request_performer def make_request(req, options, request_performer) req.sent_at = Time.now - request_performer.call(req, options).cached.tap do |res| + request_performer.call(req, options).caching.tap do |res| res.received_at = Time.now res.requested_at = req.sent_at end end - # @return [HTTP::Response::Cached, nil] the cached response for the request + # @return [HTTP::Response::Caching, nil] the cached response for the request def cache_lookup(request) return nil if request.skips_cache? c = @cache_adapter.lookup(request) - c && c.cached + c && c.caching end # Store response in cache diff --git a/lib/http/request.rb b/lib/http/request.rb index 599c48f7..226bf620 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -1,6 +1,6 @@ require "http/errors" require "http/headers" -require "http/request/cached" +require "http/request/caching" require "http/request/writer" require "http/version" require "base64" @@ -131,9 +131,9 @@ def socket_port using_proxy? ? proxy[:proxy_port] : uri.port end - # @return [HTTP::Request::Cached] - def cached - Cached.new self + # @return [HTTP::Request::Caching] + def caching + Caching.new self end private diff --git a/lib/http/request/cached.rb b/lib/http/request/caching.rb similarity index 88% rename from lib/http/request/cached.rb rename to lib/http/request/caching.rb index e49db9eb..2f0e5ef7 100644 --- a/lib/http/request/cached.rb +++ b/lib/http/request/caching.rb @@ -2,9 +2,8 @@ module HTTP class Request - # Decorator class for requests to provide convenience methods - # related to caching. - class Cached < DelegateClass(HTTP::Request) + # Decorator for requests to provide convenience methods related to caching. + class Caching < DelegateClass(HTTP::Request) INVALIDATING_METHODS = [:post, :put, :delete, :patch].freeze CACHEABLE_METHODS = [:get, :head].freeze @@ -21,8 +20,8 @@ def initialize(obj) @received_at = nil end - # @return [HTTP::Request::Cached] - def cached + # @return [HTTP::Request::Caching] + def caching self end @@ -53,7 +52,7 @@ def skips_cache? cache_headers.no_cache? end - # @return [HTTP::Request::Cached] new request based on this + # @return [HTTP::Request::Caching] new request based on this # one but conditional on the resource having changed since # `cached_response` # @@ -61,7 +60,7 @@ def skips_cache? def conditional_on_changes_to(cached_response) self.class.new HTTP::Request.new( verb, uri, headers.merge(conditional_headers_for(cached_response)), - proxy, body, version).cached + proxy, body, version).caching end # @return [HTTP::Cache::Headers] cache control helper for this request diff --git a/lib/http/response.rb b/lib/http/response.rb index b7d11747..dc44df00 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -3,7 +3,7 @@ require "http/headers" require "http/content_type" require "http/mime_type" -require "http/response/cached" +require "http/response/caching" require "http/response/status" require "time" @@ -108,9 +108,9 @@ def inspect "#<#{self.class}/#{@version} #{code} #{reason} #{headers.to_h.inspect}>" end - # @return [HTTP::Response::Cached] - def cached - Cached.new self + # @return [HTTP::Response::Caching] + def caching + Caching.new self end end end diff --git a/lib/http/response/cached.rb b/lib/http/response/caching.rb similarity index 96% rename from lib/http/response/cached.rb rename to lib/http/response/caching.rb index 4be1a06e..42e31d96 100644 --- a/lib/http/response/cached.rb +++ b/lib/http/response/caching.rb @@ -4,7 +4,7 @@ module HTTP class Response # Decorator class for responses to provide convenience methods # related to caching. - class Cached < DelegateClass(HTTP::Response) + class Caching < DelegateClass(HTTP::Response) CACHEABLE_RESPONSE_CODES = [200, 203, 300, 301, 410].freeze def initialize(obj) @@ -13,8 +13,8 @@ def initialize(obj) @received_at = nil end - # @return [HTTP::Response::Cached] - def cached + # @return [HTTP::Response::Caching] + def caching self end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index f565a767..736288d6 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -31,7 +31,7 @@ context "cache hit" do let(:cached_response) do headers = {"Cache-Control" => "private", "test" => "foo"} - HTTP::Response.new(200, "http/1.1", headers, "").cached.tap do |r| + HTTP::Response.new(200, "http/1.1", headers, "").caching.tap do |r| r.requested_at = r.received_at = Time.now end end @@ -241,7 +241,7 @@ let(:cached_response) { nil } # cold cache by default def build_cached_response(*args) - r = HTTP::Response.new(*args).cached + r = HTTP::Response.new(*args).caching r.requested_at = r.received_at = Time.now yield r if block_given? diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 37401be3..7d29cbcb 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -73,7 +73,7 @@ def simple_response(body, status = 200) describe "caching" do it "returns cached responses if they exist" do - cached_response = simple_response("OK").cached + cached_response = simple_response("OK").caching adapter = double("persistance_adapter", :lookup => cached_response) client = StubbedClient.new(:cache => HTTP::Cache.new(adapter)).stub( "http://example.com/" => simple_response("OK") diff --git a/spec/lib/http/request/cached_spec.rb b/spec/lib/http/request/caching_spec.rb similarity index 91% rename from spec/lib/http/request/cached_spec.rb rename to spec/lib/http/request/caching_spec.rb index 61695e8b..824bdbd0 100644 --- a/spec/lib/http/request/cached_spec.rb +++ b/spec/lib/http/request/caching_spec.rb @@ -1,8 +1,8 @@ -RSpec.describe HTTP::Request::Cached do - subject(:cached_request) { described_class.new request } +RSpec.describe HTTP::Request::Caching do + subject(:caching_request) { described_class.new request } describe "#cache_headers" do - subject { cached_request.cache_headers } + subject { caching_request.cache_headers } it { is_expected.to be_a HTTP::Cache::Headers } end @@ -19,7 +19,7 @@ expect(subject.skips_cache?).to be_falsy end - it "can construct a new conditional version of itself based on a cached response" do + it "can construct a new conditional version of itself based on a caching response" do mod_date = Time.now.httpdate headers = {"Etag" => "foo", "Last-Modified" => mod_date} cached_resp = HTTP::Response.new(200, "http/1.1", headers, "") @@ -48,7 +48,7 @@ expect(subject.skips_cache?).to be_truthy end - it "can construct a condition version of itself based on a cached response" do + it "can construct a condition version of itself based on a caching response" do mod_date = Time.now.httpdate headers = {"Etag" => "foo", "Last-Modified" => mod_date} cached_resp = HTTP::Response.new(200, "http/1.1", headers, "") @@ -127,8 +127,8 @@ # Background let(:request) { HTTP::Request.new(:get, "http://example.com/") } - describe "#cached" do - subject(:cached_request) { request.cached } - it { is_expected.to be cached_request } + describe "#caching" do + subject(:caching_request) { request.caching } + it { is_expected.to be caching_request } end end diff --git a/spec/lib/http/request_spec.rb b/spec/lib/http/request_spec.rb index 3ccb0bf9..e8a35f41 100644 --- a/spec/lib/http/request_spec.rb +++ b/spec/lib/http/request_spec.rb @@ -139,8 +139,8 @@ end end - describe "#cached" do - subject { request.cached } - it { is_expected.to be_a HTTP::Request::Cached } + describe "#caching" do + subject { request.caching } + it { is_expected.to be_a HTTP::Request::Caching } end end diff --git a/spec/lib/http/response/cached_spec.rb b/spec/lib/http/response/caching_spec.rb similarity index 94% rename from spec/lib/http/response/cached_spec.rb rename to spec/lib/http/response/caching_spec.rb index b897084c..ca3978d1 100644 --- a/spec/lib/http/response/cached_spec.rb +++ b/spec/lib/http/response/caching_spec.rb @@ -1,8 +1,8 @@ -RSpec.describe HTTP::Response::Cached do - subject(:cached_response) { described_class.new response } +RSpec.describe HTTP::Response::Caching do + subject(:caching_response) { described_class.new response } describe "#cache_headers" do - subject { cached_response.cache_headers } + subject { caching_response.cache_headers } it { is_expected.to be_a HTTP::Cache::Headers } end @@ -195,8 +195,8 @@ let(:headers) { {"cache-control" => cache_control} } let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } - describe "#cached" do - subject(:cached_response) { response.cached } - it { is_expected.to be cached_response } + describe "#caching" do + subject(:caching_response) { response.caching } + it { is_expected.to be caching_response } end end diff --git a/spec/lib/http/response_spec.rb b/spec/lib/http/response_spec.rb index 42a0eba4..4ec44423 100644 --- a/spec/lib/http/response_spec.rb +++ b/spec/lib/http/response_spec.rb @@ -110,8 +110,8 @@ end end - describe "#cached" do - subject { response.cached } - it { is_expected.to be_a HTTP::Response::Cached } + describe "#caching" do + subject { response.caching } + it { is_expected.to be_a HTTP::Response::Caching } end end From 71bc8845f601114084c3b3361f23f0ba80d44743 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Feb 2015 10:36:36 -0700 Subject: [PATCH 25/39] remove some unused attributes from Response --- lib/http/cache/in_memory_cache.rb | 1 - lib/http/response.rb | 2 -- lib/http/response/caching.rb | 5 ++--- spec/lib/http/cache_spec.rb | 4 ++-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb index b8d33d51..29cdc63c 100644 --- a/lib/http/cache/in_memory_cache.rb +++ b/lib/http/cache/in_memory_cache.rb @@ -9,7 +9,6 @@ class InMemoryCache def lookup(request) @mutex.synchronize do response = @collection[request.uri.to_s][request] - response.authoritative = false if response response end end diff --git a/lib/http/response.rb b/lib/http/response.rb index dc44df00..9104fc11 100644 --- a/lib/http/response.rb +++ b/lib/http/response.rb @@ -28,13 +28,11 @@ class Response # @return [URI, nil] attr_reader :uri - attr_accessor :authoritative, :request_time, :response_time def initialize(status, version, headers, body, uri = nil) # rubocop:disable ParameterLists @version, @body, @uri = version, body, uri @status = HTTP::Response::Status.new status @headers = HTTP::Headers.coerce(headers || {}) - @response_time = Time.now end # @!method reason diff --git a/lib/http/response/caching.rb b/lib/http/response/caching.rb index 42e31d96..ace1a1f7 100644 --- a/lib/http/response/caching.rb +++ b/lib/http/response/caching.rb @@ -76,9 +76,8 @@ def received_at # server. def validated!(validating_response) headers.merge!(validating_response.headers) - self.request_time = validating_response.request_time - self.response_time = validating_response.response_time - self.authoritative = true + self.requested_at = validating_response.requested_at + self.received_at = validating_response.received_at end # @return [HTTP::Cache::Headers] cache control headers helper object. diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 736288d6..c76ef83e 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -168,7 +168,7 @@ {"Cache-Control" => "private, max-age=1", "Date" => (Time.now - 2).httpdate}, "") do |t| - t.request_time = (Time.now - 2) + t.requested_at = (Time.now - 2) end end let!(:response) { subject.perform(request, opts) { origin_response } } @@ -194,7 +194,7 @@ "Etag" => "foo", "Date" => (Time.now - 2).httpdate}, "") do |x| - x.request_time = (Time.now - 2) + x.requested_at = (Time.now - 2) end end let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } From 5814dbbd6a4b772654146941c835fc996d3d9226 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Feb 2015 16:29:54 -0700 Subject: [PATCH 26/39] Use rack-cache's storage implementations rather than rolling our own --- http.gemspec | 1 + lib/http.rb | 1 + lib/http/cache.rb | 6 +- lib/http/cache/collection.rb | 69 ------------------- lib/http/cache/in_memory_cache.rb | 38 ---------- lib/http/cache/rack_cache_stores_adapter.rb | 50 ++++++++++++++ lib/http/headers.rb | 1 + lib/http/options.rb | 1 - lib/http/request/caching.rb | 4 ++ lib/http/response/caching.rb | 39 +++++++++-- lib/http/response/string_body.rb | 53 ++++++++++++++ .../cache/rack_cache_stores_adapter_spec.rb | 43 ++++++++++++ spec/lib/http/response/string_body_spec.rb | 36 ++++++++++ 13 files changed, 226 insertions(+), 116 deletions(-) delete mode 100644 lib/http/cache/collection.rb delete mode 100644 lib/http/cache/in_memory_cache.rb create mode 100644 lib/http/cache/rack_cache_stores_adapter.rb create mode 100644 lib/http/response/string_body.rb create mode 100644 spec/lib/http/cache/rack_cache_stores_adapter_spec.rb create mode 100644 spec/lib/http/response/string_body_spec.rb diff --git a/http.gemspec b/http.gemspec index 749bf2c2..8bedd2cf 100644 --- a/http.gemspec +++ b/http.gemspec @@ -24,6 +24,7 @@ Gem::Specification.new do |gem| gem.add_runtime_dependency "http_parser.rb", "~> 0.6.0" gem.add_runtime_dependency "http-form_data", "~> 1.0.0" + gem.add_runtime_dependency "rack-cache", "~> 1.2" gem.add_development_dependency "bundler", "~> 1.0" end diff --git a/lib/http.rb b/lib/http.rb index 32f61286..9b5d269e 100644 --- a/lib/http.rb +++ b/lib/http.rb @@ -9,6 +9,7 @@ require "http/response" require "http/response/body" require "http/response/parser" +require "http/response/string_body" # HTTP should be easy module HTTP diff --git a/lib/http/cache.rb b/lib/http/cache.rb index b5b99d93..3c305314 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,5 +1,5 @@ require "time" -require "http/cache/in_memory_cache" +require "http/cache/rack_cache_stores_adapter" module HTTP class Cache @@ -92,11 +92,11 @@ def store_in_cache(request, response) # # @return [nil] def invalidate_cache(request) - @cache_adapter.invalidate(request.uri) + @cache_adapter.invalidate(request) end # Inits a new instance - def initialize(adapter = HTTP::Cache::InMemoryCache.new) + def initialize(adapter = HTTP::Cache::RackCacheStoresAdapter.new(:metastore => "heap:/", :entitystore => "heap:/")) @cache_adapter = adapter end end diff --git a/lib/http/cache/collection.rb b/lib/http/cache/collection.rb deleted file mode 100644 index 8119fee2..00000000 --- a/lib/http/cache/collection.rb +++ /dev/null @@ -1,69 +0,0 @@ -require "http/headers" - -module HTTP - class Cache - # Collection of all entries in the cache. - class CacheEntryCollection - include Enumerable - - # @yield [CacheEntry] each successive entry in the cache. - def each(&block) - @entries.each(&block) - end - - # @return [Response] the response for the request or nil if - # there isn't one. - def [](request) - entry = detect { |e| e.valid_for?(request) } - entry.response if entry - end - - # @return [Response] the specified response after inserting it - # into the cache. - def []=(request, response) - @entries.delete_if { |entry| entry.valid_for?(request) } - @entries << CacheEntry.new(request, response) - response - end - - protected - - def initialize - @entries = [] - end - end - - # An entry for a single response in the cache - class CacheEntry - attr_reader :request, :response - - # @return [Boolean] true iff this entry is valid for the - # request. - def valid_for?(request) - request.uri == @request.uri && - select_request_headers.all? do |key, value| - request.headers[key] == value - end - end - - protected - - # @return [Hash] the headers that matter of matching requests to - # this response. - def select_request_headers - headers = HTTP::Headers.new - - @response.headers.get("Vary").flat_map { |v| v.split(",") }.uniq.each do |name| - name.strip! - headers[name] = @request.headers[name] if @request.headers[name] - end - - headers.to_h - end - - def initialize(request, response) - @request, @response = request, response - end - end - end -end diff --git a/lib/http/cache/in_memory_cache.rb b/lib/http/cache/in_memory_cache.rb deleted file mode 100644 index 29cdc63c..00000000 --- a/lib/http/cache/in_memory_cache.rb +++ /dev/null @@ -1,38 +0,0 @@ -require "thread" -require "http/cache/collection" - -module HTTP - class Cache - class InMemoryCache - # @return [Response] the response for the request or nil if one - # isn't found - def lookup(request) - @mutex.synchronize do - response = @collection[request.uri.to_s][request] - response - end - end - - # Stores response to be looked up later. - def store(request, response) - @mutex.synchronize do - @collection[request.uri.to_s][request] = response - end - end - - # Invalidates the all responses from the specified resource. - def invalidate(uri) - @mutex.synchronize do - @collection.delete(uri.to_s) - end - end - - protected - - def initialize - @mutex = Mutex.new - @collection = Hash.new { |h, k| h[k] = CacheEntryCollection.new } - end - end - end -end diff --git a/lib/http/cache/rack_cache_stores_adapter.rb b/lib/http/cache/rack_cache_stores_adapter.rb new file mode 100644 index 00000000..c97d2956 --- /dev/null +++ b/lib/http/cache/rack_cache_stores_adapter.rb @@ -0,0 +1,50 @@ +require "rack-cache" + +module HTTP + class Cache + # Cache persistence based on rack-cache's storage + # implementations. See for + # more information. + class RackCacheStoresAdapter + # Stores response in cache + # + # @return [nil] + def store(request, response) + metastore.store(request, response, entitystore) + end + + # @return [HTTP::Request::Caching, nil] A cached response to + # request, or nil if one wasn't found + def lookup(request) + rack_resp = metastore.lookup(request, entitystore) + return if rack_resp.nil? + + HTTP::Response.new( + rack_resp.status, "1.1", rack_resp.headers, rack_resp.body.join("") + ).caching + end + + # Invalidate any cached responses for the request. + # + # @return [nil] + def invalidate(request) + metastore.invalidate(request, entitystore) + end + + protected + + # @option opts [String] :metastore The location of the metastore + # @option opts [String] :entitystore The location of the entity store + def initialize(opts) + @metastore = storage.resolve_metastore_uri(opts[:metastore]) + @entitystore = storage.resolve_entitystore_uri(opts[:entitystore]) + end + + attr_reader :metastore, :entitystore + + def storage + @@storage ||= Rack::Cache::Storage.new # rubocop:disable Style/ClassVars + end + end + end +end diff --git a/lib/http/headers.rb b/lib/http/headers.rb index 90b85b4f..d9652835 100644 --- a/lib/http/headers.rb +++ b/lib/http/headers.rb @@ -82,6 +82,7 @@ def [](name) def to_h Hash[keys.map { |k| [k, self[k]] }] end + alias_method :to_hash, :to_h # Returns headers key/value pairs. # diff --git a/lib/http/options.rb b/lib/http/options.rb index d17b0f81..493b6c3c 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -2,7 +2,6 @@ require "openssl" require "socket" require "http/cache" -require "http/cache/in_memory_cache" module HTTP class Options diff --git a/lib/http/request/caching.rb b/lib/http/request/caching.rb index 2f0e5ef7..ce59f441 100644 --- a/lib/http/request/caching.rb +++ b/lib/http/request/caching.rb @@ -69,6 +69,10 @@ def cache_headers @cache_headers ||= HTTP::Cache::Headers.new headers end + def env + {"rack-cache.cache_key" => ->(r) { r.uri.to_s }} + end + private # @return [Headers] conditional request headers diff --git a/lib/http/response/caching.rb b/lib/http/response/caching.rb index ace1a1f7..b1d8971c 100644 --- a/lib/http/response/caching.rb +++ b/lib/http/response/caching.rb @@ -25,7 +25,7 @@ def stale? # @returns [Boolean] true iff this response has expired def expired? - current_age > cache_headers.max_age + current_age >= cache_headers.max_age end # @return [Boolean] true iff this response is cacheable @@ -54,21 +54,22 @@ def current_age apparent_age = [0, received_at - server_response_time].max corrected_received_age = [apparent_age, age_value].max - response_delay = received_at - requested_at + response_delay = [0, received_at - requested_at].max corrected_initial_age = corrected_received_age + response_delay - resident_time = now - received_at + resident_time = [0, now - received_at].max + corrected_initial_age + resident_time end # @return [Time] the time at which this response was requested def requested_at - @requested_at ||= Time.now + @requested_at ||= received_at end attr_writer :requested_at # @return [Time] the time at which this response was received def received_at - @received_at || Time.now + @received_at ||= Time.now end attr_writer :received_at @@ -85,6 +86,34 @@ def cache_headers @cache_headers ||= HTTP::Cache::Headers.new headers end + def body + @body ||= if __getobj__.body.respond_to? :each + __getobj__.body + else + StringBody.new(__getobj__.body.to_s) + end + end + + def body=(new_body) + @body = if new_body.respond_to? :readpartial + # Normal body, just use it + new_body + + elsif new_body.respond_to? :join + # probably a rack enumerable body, join the parts + # into a single string. + StringBody.new(new_body.join("")) + + else + # backstop, just to_s it + StringBody.new(new_body.to_s) + end + end + + def vary + headers.get("Vary").first + end + protected # @return [Time] the time at which the server generated this response. diff --git a/lib/http/response/string_body.rb b/lib/http/response/string_body.rb new file mode 100644 index 00000000..98b1f59d --- /dev/null +++ b/lib/http/response/string_body.rb @@ -0,0 +1,53 @@ +module HTTP + class Response + # A Body class that wraps a String, rather than a the client + # object. + class StringBody + include Enumerable + extend Forwardable + + # @return [String,nil] the next `size` octets part of the + # body, or nil if whole body has already been read. + def readpartial(size = @contents.length) + stream! + return nil if @streaming_offset >= @contents.length + + @contents[@streaming_offset, size].tap do |part| + @streaming_offset += (part.length + 1) + end + end + + # Iterate over the body, allowing it to be enumerable + def each + yield @contents + end + + # @return [String] eagerly consume the entire body as a string + def to_s + @contents + end + alias_method :to_str, :to_s + + def_delegator :@contents, :empty? + + # Assert that the body is actively being streamed + def stream! + fail StateError, "body has already been consumed" if @streaming == false + @streaming = true + end + + # Easier to interpret string inspect + def inspect + "#<#{self.class}:#{object_id.to_s(16)}>" + end + + protected + + def initialize(contents) + @contents = contents + @streaming = nil + @streaming_offset = 0 + end + end + end +end diff --git a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb new file mode 100644 index 00000000..b13e97ea --- /dev/null +++ b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb @@ -0,0 +1,43 @@ +RSpec.describe HTTP::Cache::RackCacheStoresAdapter do + describe ".new" do + it "accepts opts :metastore and :entitystore" do + expect(described_class.new(:metastore => "heap:/", :entitystore => "heap:/")) + .to be_kind_of described_class + end + end + + subject { described_class.new(:metastore => "heap:/", :entitystore => "heap:/") } + + describe "store and retrieve" do + it "returns the correct response" do + subject.store(request, response) + expect(subject.lookup(request)).to be_equivalent_to response + end + end + + describe "store, invalidate and retrieve" do + it "returns the correct response" do + subject.store(request, response) + subject.invalidate(request) + + expect(subject.lookup(request)).to be_stale + end + end + + # Background + let(:request) { HTTP::Request.new(:get, "http://example.com").caching } + let(:response) do + HTTP::Response.new( + 200, "HTTP/1.1", + {"X-test" => "#{rand}", "Date" => Time.now.httpdate, "Cache-Control" => "max-age=100"}, + "testing 1, 2, #{rand}").caching + end + + matcher :be_equivalent_to do |expected| + match do |actual| + actual.body.to_s == expected.body.to_s && + actual.headers == expected.headers && + actual.status == expected.status + end + end +end diff --git a/spec/lib/http/response/string_body_spec.rb b/spec/lib/http/response/string_body_spec.rb new file mode 100644 index 00000000..eb173d61 --- /dev/null +++ b/spec/lib/http/response/string_body_spec.rb @@ -0,0 +1,36 @@ +RSpec.describe HTTP::Response::StringBody do + subject(:body) { described_class.new "Hello, World!" } + + it "has the content" do + expect(subject.to_s).to eq "Hello, World!" + end + + context "when body empty" do + subject(:body) { described_class.new "" } + + it "returns responds to empty? with true" do + expect(subject).to be_empty + end + end + + describe "#readpartial" do + context "with size given" do + it "returns only that amount" do + expect(body.readpartial(4)).to eq "Hell" + end + end + + context "without size given" do + it "returns parts of the full content" do + expect(body.readpartial).to eq "Hello, World!" + end + end + end + + describe "#each" do + it "yields contents" do + expect { |b| body.each(&b) }.to yield_with_args "Hello, World!" + end + end + +end From 429a5a011b582bcc2619a407e4cc2f834aa04c93 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Feb 2015 09:01:15 -0700 Subject: [PATCH 27/39] handle the rack-cache's result when using file stores --- Gemfile | 1 + lib/http/cache/rack_cache_stores_adapter.rb | 2 +- .../cache/rack_cache_stores_adapter_spec.rb | 29 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index fad92193..2035477e 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,7 @@ group :test do gem "rubocop", "~> 0.25.0" gem "simplecov", ">= 0.9" gem "yardstick" + gem "fakefs", :require => "fakefs/safe" end group :doc do diff --git a/lib/http/cache/rack_cache_stores_adapter.rb b/lib/http/cache/rack_cache_stores_adapter.rb index c97d2956..05a6dc6d 100644 --- a/lib/http/cache/rack_cache_stores_adapter.rb +++ b/lib/http/cache/rack_cache_stores_adapter.rb @@ -20,7 +20,7 @@ def lookup(request) return if rack_resp.nil? HTTP::Response.new( - rack_resp.status, "1.1", rack_resp.headers, rack_resp.body.join("") + rack_resp.status, "1.1", rack_resp.headers, rack_resp.body.reduce(""){|b, part| b << part } ).caching end diff --git a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb index b13e97ea..a631afbd 100644 --- a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb +++ b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb @@ -1,3 +1,5 @@ +require 'fakefs/safe' + RSpec.describe HTTP::Cache::RackCacheStoresAdapter do describe ".new" do it "accepts opts :metastore and :entitystore" do @@ -24,6 +26,27 @@ end end + context "file storage" do + before do + FakeFS.activate! + end + + subject { described_class.new(:metastore => "file:/var/cache/http.rb-test/meta", + :entitystore => "file:/var/cache/http.rb-test/entity") } + + describe "store and retrieve" do + it "returns the correct response" do + subject.store(request, response) + + expect(subject.lookup(request)).to be_equivalent_to response + end + end + + after do + FakeFS.deactivate! + end + end + # Background let(:request) { HTTP::Request.new(:get, "http://example.com").caching } let(:response) do @@ -35,9 +58,13 @@ matcher :be_equivalent_to do |expected| match do |actual| - actual.body.to_s == expected.body.to_s && + stringify(actual.body) == stringify(expected.body) && actual.headers == expected.headers && actual.status == expected.status end + + def stringify(body) + body.reduce(""){|b,part| b << part} + end end end From 79cdbb74b92a3ceb868f8be378e4aa822a540971 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Feb 2015 11:43:47 -0700 Subject: [PATCH 28/39] verify that cache's file store works --- lib/http/cache/rack_cache_stores_adapter.rb | 16 ++++- lib/http/response/caching.rb | 4 +- lib/http/response/io_body.rb | 63 +++++++++++++++++++ .../cache/rack_cache_stores_adapter_spec.rb | 47 ++++++++++---- spec/lib/http/response/io_body_spec.rb | 36 +++++++++++ 5 files changed, 151 insertions(+), 15 deletions(-) create mode 100644 lib/http/response/io_body.rb create mode 100644 spec/lib/http/response/io_body_spec.rb diff --git a/lib/http/cache/rack_cache_stores_adapter.rb b/lib/http/cache/rack_cache_stores_adapter.rb index 05a6dc6d..cbba79e0 100644 --- a/lib/http/cache/rack_cache_stores_adapter.rb +++ b/lib/http/cache/rack_cache_stores_adapter.rb @@ -20,7 +20,7 @@ def lookup(request) return if rack_resp.nil? HTTP::Response.new( - rack_resp.status, "1.1", rack_resp.headers, rack_resp.body.reduce(""){|b, part| b << part } + rack_resp.status, "1.1", rack_resp.headers, stringify(rack_resp.body) ).caching end @@ -36,7 +36,7 @@ def invalidate(request) # @option opts [String] :metastore The location of the metastore # @option opts [String] :entitystore The location of the entity store def initialize(opts) - @metastore = storage.resolve_metastore_uri(opts[:metastore]) + @metastore = storage.resolve_metastore_uri(opts[:metastore]) @entitystore = storage.resolve_entitystore_uri(opts[:entitystore]) end @@ -45,6 +45,18 @@ def initialize(opts) def storage @@storage ||= Rack::Cache::Storage.new # rubocop:disable Style/ClassVars end + + def stringify(body) + if body.respond_to?(:each) + String.new.tap do |buf| + body.each do |part| + buf << part + end + end + else + body.to_s + end + end end end end diff --git a/lib/http/response/caching.rb b/lib/http/response/caching.rb index b1d8971c..25e83f71 100644 --- a/lib/http/response/caching.rb +++ b/lib/http/response/caching.rb @@ -1,4 +1,6 @@ require "http/cache/headers" +require "http/response/string_body" +require "http/response/io_body" module HTTP class Response @@ -97,7 +99,7 @@ def body def body=(new_body) @body = if new_body.respond_to? :readpartial # Normal body, just use it - new_body + IoBody.new(new_body) elsif new_body.respond_to? :join # probably a rack enumerable body, join the parts diff --git a/lib/http/response/io_body.rb b/lib/http/response/io_body.rb new file mode 100644 index 00000000..0d502357 --- /dev/null +++ b/lib/http/response/io_body.rb @@ -0,0 +1,63 @@ +module HTTP + class Response + # A Body class that wraps an IO, rather than a the client + # object. + class IoBody + include Enumerable + extend Forwardable + + # @return [String,nil] the next `size` octets part of the + # body, or nil if whole body has already been read. + def readpartial(size = HTTP::Client::BUFFER_SIZE) + stream! + return nil if stream.eof? + + stream.readpartial(size) + end + + # Iterate over the body, allowing it to be enumerable + def each + while part = readpartial # rubocop:disable Lint/AssignmentInCondition + yield part + end + end + + # @return [String] eagerly consume the entire body as a string + def to_s + @contents ||= readall + end + alias_method :to_str, :to_s + + def_delegator :to_s, :empty? + + # Assert that the body is actively being streamed + def stream! + fail StateError, "body has already been consumed" if @streaming == false + @streaming = true + end + + # Easier to interpret string inspect + def inspect + "#<#{self.class}:#{object_id.to_s(16)} @streaming=#{!!@streaming}>" + end + + protected + + def initialize(an_io) + @streaming = nil + @stream = an_io + end + + attr_reader :contents, :stream + + def readall + fail StateError, "body is being streamed" unless @streaming.nil? + + @streaming = false + String.new.tap do |buf| + buf << stream.read until stream.eof? + end + end + end + end +end diff --git a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb index a631afbd..a4b1ad7b 100644 --- a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb +++ b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb @@ -1,5 +1,3 @@ -require 'fakefs/safe' - RSpec.describe HTTP::Cache::RackCacheStoresAdapter do describe ".new" do it "accepts opts :metastore and :entitystore" do @@ -15,6 +13,11 @@ subject.store(request, response) expect(subject.lookup(request)).to be_equivalent_to response end + + it "original response is still usable after being stored" do + subject.store(request, response) + expect(response.body.to_s).to eq body_content + end end describe "store, invalidate and retrieve" do @@ -27,13 +30,12 @@ end context "file storage" do - before do - FakeFS.activate! + subject do + described_class.new( + :metastore => "file:tmp/cache/meta", :entitystore => "file:tmp/cache/entity" + ) end - subject { described_class.new(:metastore => "file:/var/cache/http.rb-test/meta", - :entitystore => "file:/var/cache/http.rb-test/entity") } - describe "store and retrieve" do it "returns the correct response" do subject.store(request, response) @@ -42,18 +44,39 @@ end end - after do - FakeFS.deactivate! + it "original response is still usable after being stored" do + subject.store(request, response) + expect(response.body.to_s).to eq body_content end end # Background let(:request) { HTTP::Request.new(:get, "http://example.com").caching } + let(:body_content) { "testing 1, 2, #{rand}" } let(:response) do HTTP::Response.new( - 200, "HTTP/1.1", + 200, + "HTTP/1.1", {"X-test" => "#{rand}", "Date" => Time.now.httpdate, "Cache-Control" => "max-age=100"}, - "testing 1, 2, #{rand}").caching + HTTP::Response::Body.new(client) + ).caching + end + let(:client) do + StringIO.new(body_content).tap do |s| + class << s + def readpartial(*args) + if eof? + nil + else + super + end + end + + def to_s + string + end + end + end end matcher :be_equivalent_to do |expected| @@ -64,7 +87,7 @@ end def stringify(body) - body.reduce(""){|b,part| b << part} + body.inject("") { |a, e| a + e } end end end diff --git a/spec/lib/http/response/io_body_spec.rb b/spec/lib/http/response/io_body_spec.rb new file mode 100644 index 00000000..940f2f62 --- /dev/null +++ b/spec/lib/http/response/io_body_spec.rb @@ -0,0 +1,36 @@ +RSpec.describe HTTP::Response::IoBody do + subject(:body) { described_class.new StringIO.new("Hello, World!") } + + it "has the content" do + expect(subject.to_s).to eq "Hello, World!" + end + + context "when body empty" do + subject(:body) { described_class.new StringIO.new("") } + + it "returns responds to empty? with true" do + expect(subject).to be_empty + end + end + + describe "#readpartial" do + context "with size given" do + it "returns only that amount" do + expect(body.readpartial(4)).to eq "Hell" + end + end + + context "without size given" do + it "returns parts of the content" do + expect(body.readpartial).to eq "Hello, World!" + end + end + end + + describe "#each" do + it "yields successive parts of the content" do + expect { |b| body.each(&b) }.to yield_with_args "Hello, World!" + end + end + +end From 7153878184170c31b30e500ea2cde5e6f39bcece Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Feb 2015 12:31:03 -0700 Subject: [PATCH 29/39] allow both original requester and cache store to read response body --- lib/http/cache.rb | 7 +++++++ lib/http/response/io_body.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 3c305314..80c861eb 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -83,7 +83,14 @@ def cache_lookup(request) # Store response in cache # # @return [nil] + # + # --- + # + # We have to convert the response body in to a string body so + # that the cache store reading the body will not prevent the + # original requester from doing so. def store_in_cache(request, response) + response.body = response.body.to_s @cache_adapter.store(request, response) nil end diff --git a/lib/http/response/io_body.rb b/lib/http/response/io_body.rb index 0d502357..12afbe5f 100644 --- a/lib/http/response/io_body.rb +++ b/lib/http/response/io_body.rb @@ -17,7 +17,7 @@ def readpartial(size = HTTP::Client::BUFFER_SIZE) # Iterate over the body, allowing it to be enumerable def each - while part = readpartial # rubocop:disable Lint/AssignmentInCondition + while (part = readpartial) yield part end end From cac374da76e638f6b2443e65b748160b4afbc1a8 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Feb 2015 13:36:03 -0700 Subject: [PATCH 30/39] remove a layer of indirection (and code) --- lib/http/cache.rb | 61 +++++++- lib/http/cache/rack_cache_stores_adapter.rb | 62 -------- lib/http/options.rb | 8 +- .../cache/rack_cache_stores_adapter_spec.rb | 93 ------------ spec/lib/http/cache_spec.rb | 142 +++++------------- spec/lib/http/client_spec.rb | 19 ++- spec/lib/http_spec.rb | 6 - 7 files changed, 105 insertions(+), 286 deletions(-) delete mode 100644 lib/http/cache/rack_cache_stores_adapter.rb delete mode 100644 spec/lib/http/cache/rack_cache_stores_adapter_spec.rb diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 80c861eb..871903c2 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -1,5 +1,5 @@ require "time" -require "http/cache/rack_cache_stores_adapter" +require "rack-cache" module HTTP class Cache @@ -12,6 +12,21 @@ def perform(request, options) end end + # NoOp logger. + class NullLogger + def error(_msg = nil) + end + + def debug(_msg = nil) + end + + def info(_msg = nil) + end + + def warn(_msg = nil) + end + end + # @return [Response] a cached response that is valid for the request or # the result of executing the provided block # @@ -34,7 +49,7 @@ def get_response(req, options, request_performer) return cached_resp if cached_resp && !cached_resp.stale? # cache miss - + logger.debug { "Cache miss for <#{req.uri}>, making request" } actual_req = if cached_resp req.conditional_on_changes_to(cached_resp) else @@ -49,6 +64,7 @@ def get_response(req, options, request_performer) # updating the cache as appropriate def handle_response(cached_resp, actual_resp, req) if actual_resp.status.not_modified? && cached_resp + logger.debug { "<#{req.uri}> not modified, using cached version." } cached_resp.validated!(actual_resp) store_in_cache(req, cached_resp) return cached_resp @@ -76,8 +92,13 @@ def make_request(req, options, request_performer) # @return [HTTP::Response::Caching, nil] the cached response for the request def cache_lookup(request) return nil if request.skips_cache? - c = @cache_adapter.lookup(request) - c && c.caching + + rack_resp = metastore.lookup(request, entitystore) + return if rack_resp.nil? + + HTTP::Response.new( + rack_resp.status, "1.1", rack_resp.headers, stringify(rack_resp.body) + ).caching end # Store response in cache @@ -91,7 +112,7 @@ def cache_lookup(request) # original requester from doing so. def store_in_cache(request, response) response.body = response.body.to_s - @cache_adapter.store(request, response) + metastore.store(request, response, entitystore) nil end @@ -99,12 +120,36 @@ def store_in_cache(request, response) # # @return [nil] def invalidate_cache(request) - @cache_adapter.invalidate(request) + metastore.invalidate(request, entitystore) end # Inits a new instance - def initialize(adapter = HTTP::Cache::RackCacheStoresAdapter.new(:metastore => "heap:/", :entitystore => "heap:/")) - @cache_adapter = adapter + # + # @option opts [String] :metastore URL to the metastore location + # @option opts [String] :entitystore URL to the entitystore location + # @option opts [Logger] :logger logger to use + def initialize(opts) + @metastore = storage.resolve_metastore_uri(opts.fetch(:metastore)) + @entitystore = storage.resolve_entitystore_uri(opts.fetch(:entitystore)) + @logger = opts.fetch(:logger) { NullLogger.new } + end + + attr_reader :metastore, :entitystore, :logger + + def storage + @@storage ||= Rack::Cache::Storage.new # rubocop:disable Style/ClassVars + end + + def stringify(body) + if body.respond_to?(:each) + String.new.tap do |buf| + body.each do |part| + buf << part + end + end + else + body.to_s + end end end end diff --git a/lib/http/cache/rack_cache_stores_adapter.rb b/lib/http/cache/rack_cache_stores_adapter.rb deleted file mode 100644 index cbba79e0..00000000 --- a/lib/http/cache/rack_cache_stores_adapter.rb +++ /dev/null @@ -1,62 +0,0 @@ -require "rack-cache" - -module HTTP - class Cache - # Cache persistence based on rack-cache's storage - # implementations. See for - # more information. - class RackCacheStoresAdapter - # Stores response in cache - # - # @return [nil] - def store(request, response) - metastore.store(request, response, entitystore) - end - - # @return [HTTP::Request::Caching, nil] A cached response to - # request, or nil if one wasn't found - def lookup(request) - rack_resp = metastore.lookup(request, entitystore) - return if rack_resp.nil? - - HTTP::Response.new( - rack_resp.status, "1.1", rack_resp.headers, stringify(rack_resp.body) - ).caching - end - - # Invalidate any cached responses for the request. - # - # @return [nil] - def invalidate(request) - metastore.invalidate(request, entitystore) - end - - protected - - # @option opts [String] :metastore The location of the metastore - # @option opts [String] :entitystore The location of the entity store - def initialize(opts) - @metastore = storage.resolve_metastore_uri(opts[:metastore]) - @entitystore = storage.resolve_entitystore_uri(opts[:entitystore]) - end - - attr_reader :metastore, :entitystore - - def storage - @@storage ||= Rack::Cache::Storage.new # rubocop:disable Style/ClassVars - end - - def stringify(body) - if body.respond_to?(:each) - String.new.tap do |buf| - body.each do |part| - buf << part - end - end - else - body.to_s - end - end - end - end -end diff --git a/lib/http/options.rb b/lib/http/options.rb index 493b6c3c..e898ea40 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -87,11 +87,11 @@ def with_#{method_name}(value) RUBY end - def with_cache(persistence_adapter_or_cache) - cache = if persistence_adapter_or_cache.respond_to? :perform - persistence_adapter_or_cache + def with_cache(cache_or_cache_options) + cache = if cache_or_cache_options.respond_to? :perform + cache_or_cache_options else - HTTP::Cache.new(persistence_adapter_or_cache) + HTTP::Cache.new(cache_or_cache_options) end dup do |opts| diff --git a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb b/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb deleted file mode 100644 index a4b1ad7b..00000000 --- a/spec/lib/http/cache/rack_cache_stores_adapter_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -RSpec.describe HTTP::Cache::RackCacheStoresAdapter do - describe ".new" do - it "accepts opts :metastore and :entitystore" do - expect(described_class.new(:metastore => "heap:/", :entitystore => "heap:/")) - .to be_kind_of described_class - end - end - - subject { described_class.new(:metastore => "heap:/", :entitystore => "heap:/") } - - describe "store and retrieve" do - it "returns the correct response" do - subject.store(request, response) - expect(subject.lookup(request)).to be_equivalent_to response - end - - it "original response is still usable after being stored" do - subject.store(request, response) - expect(response.body.to_s).to eq body_content - end - end - - describe "store, invalidate and retrieve" do - it "returns the correct response" do - subject.store(request, response) - subject.invalidate(request) - - expect(subject.lookup(request)).to be_stale - end - end - - context "file storage" do - subject do - described_class.new( - :metastore => "file:tmp/cache/meta", :entitystore => "file:tmp/cache/entity" - ) - end - - describe "store and retrieve" do - it "returns the correct response" do - subject.store(request, response) - - expect(subject.lookup(request)).to be_equivalent_to response - end - end - - it "original response is still usable after being stored" do - subject.store(request, response) - expect(response.body.to_s).to eq body_content - end - end - - # Background - let(:request) { HTTP::Request.new(:get, "http://example.com").caching } - let(:body_content) { "testing 1, 2, #{rand}" } - let(:response) do - HTTP::Response.new( - 200, - "HTTP/1.1", - {"X-test" => "#{rand}", "Date" => Time.now.httpdate, "Cache-Control" => "max-age=100"}, - HTTP::Response::Body.new(client) - ).caching - end - let(:client) do - StringIO.new(body_content).tap do |s| - class << s - def readpartial(*args) - if eof? - nil - else - super - end - end - - def to_s - string - end - end - end - end - - matcher :be_equivalent_to do |expected| - match do |actual| - stringify(actual.body) == stringify(expected.body) && - actual.headers == expected.headers && - actual.status == expected.status - end - - def stringify(body) - body.inject("") { |a, e| a + e } - end - end -end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index c76ef83e..cec56b69 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -4,19 +4,14 @@ describe "creation" do subject { described_class } - it "allows private mode" do - expect(subject.new(cache_adapter)) - .to be_kind_of HTTP::Cache - end - - it "allows public mode" do - expect(subject.new(cache_adapter)) + it "allows metastore and entitystore" do + expect(subject.new(:metastore => "heap:/", :entitystore => "heap:/")) .to be_kind_of HTTP::Cache end end let(:opts) { options } - subject { described_class.new(cache_adapter) } + subject { described_class.new(:metastore => "heap:/", :entitystore => "heap:/") } describe "#perform" do it "calls request_performer blocck when cache miss" do @@ -29,14 +24,11 @@ end context "cache hit" do - let(:cached_response) do - headers = {"Cache-Control" => "private", "test" => "foo"} - HTTP::Response.new(200, "http/1.1", headers, "").caching.tap do |r| - r.requested_at = r.received_at = Time.now + it "does not call request_performer block" do + subject.perform(request, opts) do |*_t| + origin_response end - end - it "does not call request_performer block" do expect { |b| subject.perform(request, opts, &b) }.not_to yield_control end end @@ -45,17 +37,9 @@ context "empty cache, cacheable request, cacheable response" do let!(:response) { subject.perform(request, opts) { origin_response } } - it "tries to lookup request" do - expect(cache_adapter).to have_received(:lookup).with(request) - end - it "returns origin servers response" do expect(response).to eq origin_response end - - it "stores response in cache" do - expect(cache_adapter).to have_received(:store).with(request, origin_response) - end end context "cache by-passing request, cacheable response" do @@ -65,38 +49,9 @@ end let!(:response) { subject.perform(request, opts) { origin_response } } - it "doesn't lookup request" do - expect(cache_adapter).not_to have_received(:lookup) - end - it "returns origin servers response" do expect(response).to eq origin_response end - - it "stores response in cache" do - expect(cache_adapter).to have_received(:store).with(request, origin_response) - end - end - - context "empty cache, cacheable request, 'nreceiver' response" do - let(:origin_response) do - HTTP::Response.new(200, "http/1.1", - {"Cache-Control" => "no-cache"}, - "") - end - let!(:response) { subject.perform(request, opts) { origin_response } } - - it "tries to lookup request" do - expect(cache_adapter).to have_received(:lookup).with(request) - end - - it "returns origin servers response" do - expect(response).to eq origin_response - end - - it "doesn't store response in cache" do - expect(cache_adapter).not_to have_received(:store) - end end context "empty cache, cacheable request, 'no-cache' response" do @@ -108,17 +63,9 @@ end let!(:response) { subject.perform(request, opts) { origin_response } } - it "tries to lookup request" do - expect(cache_adapter).to have_received(:lookup).with(request) - end - it "returns origin servers response" do expect(response).to eq origin_response end - - it "doesn't store response in cache" do - expect(cache_adapter).not_to have_received(:store) - end end context "empty cache, cacheable request, 'no-store' response" do @@ -130,59 +77,46 @@ end let!(:response) { subject.perform(request, opts) { origin_response } } - it "tries to lookup request" do - expect(cache_adapter).to have_received(:lookup).with(request) - end - it "returns origin servers response" do expect(response).to eq origin_response end - - it "doesn't store response in cache" do - expect(cache_adapter).not_to have_received(:store) - end end context "warm cache, cacheable request, cacheable response" do let(:cached_response) do build_cached_response(200, - "http/1.1", - {"Cache-Control" => "private"}, - "") + "1.1", + {"Cache-Control" => "max-age=100"}, + "cached") end - let!(:response) { subject.perform(request, opts) { origin_response } } - - it "lookups request" do - expect(cache_adapter).to have_received(:lookup).with(request) + before do + subject.perform(request, opts) { cached_response } end + let(:response) { subject.perform(request, opts) { origin_response } } + it "returns cached response" do - expect(response).to eq cached_response + expect(response.body.to_s).to eq cached_response.body.to_s end end context "stale cache, cacheable request, cacheable response" do let(:cached_response) do build_cached_response(200, - "http/1.1", + "1.1", {"Cache-Control" => "private, max-age=1", "Date" => (Time.now - 2).httpdate}, - "") do |t| + "cached") do |t| t.requested_at = (Time.now - 2) end end - let!(:response) { subject.perform(request, opts) { origin_response } } - - it "lookups request" do - expect(cache_adapter).to have_received(:lookup).with(request) + before do + subject.perform(request, opts) { cached_response } end + let(:response) { subject.perform(request, opts) { origin_response } } it "returns origin servers response" do - expect(response).to eq origin_response - end - - it "stores fresh response in cache" do - expect(cache_adapter).to have_received(:store).with(request, origin_response) + expect(response.body.to_s).to eq origin_response.body.to_s end end @@ -197,45 +131,39 @@ x.requested_at = (Time.now - 2) end end - let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } - let!(:response) { subject.perform(request, opts) { origin_response } } - - it "lookups request" do - expect(cache_adapter).to have_received(:lookup).with(request) + before do + subject.perform(request, opts) { cached_response } end + let(:origin_response) { HTTP::Response.new(304, "http/1.1", {}, "") } + let(:response) { subject.perform(request, opts) { origin_response } } + it "makes request with conditional request headers" do - actual_request = nil - subject.perform(request, opts) do |r, _| - actual_request = r + subject.perform(request, opts) do |actual_request, _| + expect(actual_request.headers["If-None-Match"]) + .to eq cached_response.headers["Etag"] + expect(actual_request.headers["If-Modified-Since"]) + .to eq cached_response.headers["Last-Modified"] + origin_response end - - expect(actual_request.headers["If-None-Match"]).to eq cached_response.headers["Etag"] - expect(actual_request.headers["If-Modified-Since"]) - .to eq cached_response.headers["Last-Modified"] end it "returns cached servers response" do - expect(response).to eq cached_response - end - - it "updates the stored response in cache" do - expect(cache_adapter).to have_received(:store).with(request, cached_response) + expect(response.body.to_s).to eq cached_response.body.to_s end end # Background - let(:cache_adapter) { double("cache_adapter", :lookup => cached_response, :store => nil) } - - let(:request) { HTTP::Request.new(:get, "http://example.com/") } + let(:sn) { SecureRandom.urlsafe_base64(3) } + let(:request) { HTTP::Request.new(:get, "http://example.com/#{sn}") } let(:origin_response) do HTTP::Response.new(200, "http/1.1", {"Cache-Control" => "private"}, - "") + "origin") end let(:cached_response) { nil } # cold cache by default diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 7d29cbcb..2c269a2a 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -72,14 +72,21 @@ def simple_response(body, status = 200) end describe "caching" do + let(:sn) { SecureRandom.urlsafe_base64(3) } + it "returns cached responses if they exist" do - cached_response = simple_response("OK").caching - adapter = double("persistance_adapter", :lookup => cached_response) - client = StubbedClient.new(:cache => HTTP::Cache.new(adapter)).stub( - "http://example.com/" => simple_response("OK") - ) + cached_response = simple_response("cached").caching + StubbedClient.new(:cache => HTTP::Cache.new(:metastore => "heap:/", :entitystore => "heap:/")) + .stub("http://example.com/#{sn}" => cached_response) + .get("http://example.com/#{sn}") + + # cache is now warm + + client = StubbedClient.new(:cache => HTTP::Cache.new(:metastore => "heap:/", :entitystore => "heap:/")) + .stub("http://example.com/#{sn}" => simple_response("OK")) - expect(client.get("http://example.com/")).to eq cached_response + expect(client.get("http://example.com/#{sn}").body.to_s) + .to eq cached_response.body.to_s end end diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index e74096c6..bc684479 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -162,11 +162,5 @@ client = HTTP.with_cache cache expect(client.default_options[:cache]).to eq cache end - - it "sets cache option" do - persistence_adapter = double(:cache, :lookup => nil) - client = HTTP.with_cache persistence_adapter - expect(client.default_options[:cache]).not_to eq persistence_adapter - end end end From 042d9044667b70cd8c1798a05d0972f8452c713b Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Feb 2015 13:45:40 -0700 Subject: [PATCH 31/39] fix a few minor stylistic thing for rubocop --- lib/http/cache.rb | 2 +- lib/http/response/caching.rb | 11 +++++++---- lib/http/response/io_body.rb | 2 +- spec/lib/http/response/io_body_spec.rb | 1 - spec/lib/http/response/string_body_spec.rb | 1 - 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 871903c2..7a6ba241 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -142,7 +142,7 @@ def storage def stringify(body) if body.respond_to?(:each) - String.new.tap do |buf| + "".tap do |buf| body.each do |part| buf << part end diff --git a/lib/http/response/caching.rb b/lib/http/response/caching.rb index 25e83f71..dd28e9b0 100644 --- a/lib/http/response/caching.rb +++ b/lib/http/response/caching.rb @@ -97,15 +97,18 @@ def body end def body=(new_body) - @body = if new_body.respond_to? :readpartial - # Normal body, just use it + @body = if new_body.respond_to?(:readpartial) && new_body.respond_to?(:read) + # IO-ish, probably a rack cache response body IoBody.new(new_body) elsif new_body.respond_to? :join - # probably a rack enumerable body, join the parts - # into a single string. + # probably an array of body parts (rack cache does this sometimes) StringBody.new(new_body.join("")) + elsif new_body.respond_to? :readpartial + # normal body, just use it. + new_body + else # backstop, just to_s it StringBody.new(new_body.to_s) diff --git a/lib/http/response/io_body.rb b/lib/http/response/io_body.rb index 12afbe5f..7e4c8abb 100644 --- a/lib/http/response/io_body.rb +++ b/lib/http/response/io_body.rb @@ -54,7 +54,7 @@ def readall fail StateError, "body is being streamed" unless @streaming.nil? @streaming = false - String.new.tap do |buf| + "".tap do |buf| buf << stream.read until stream.eof? end end diff --git a/spec/lib/http/response/io_body_spec.rb b/spec/lib/http/response/io_body_spec.rb index 940f2f62..66fc52ab 100644 --- a/spec/lib/http/response/io_body_spec.rb +++ b/spec/lib/http/response/io_body_spec.rb @@ -32,5 +32,4 @@ expect { |b| body.each(&b) }.to yield_with_args "Hello, World!" end end - end diff --git a/spec/lib/http/response/string_body_spec.rb b/spec/lib/http/response/string_body_spec.rb index eb173d61..26415eba 100644 --- a/spec/lib/http/response/string_body_spec.rb +++ b/spec/lib/http/response/string_body_spec.rb @@ -32,5 +32,4 @@ expect { |b| body.each(&b) }.to yield_with_args "Hello, World!" end end - end From 182d5dbf8343c9c7d1f633d706f33ab89fb09e6b Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Mar 2015 10:16:42 -0700 Subject: [PATCH 32/39] delay loading rack-cache until caching is requested --- README.md | 21 +++++++++++++++++++++ lib/http.rb | 1 - lib/http/cache.rb | 9 --------- lib/http/cache/null_cache.rb | 13 +++++++++++++ lib/http/client.rb | 1 - lib/http/options.rb | 6 +++--- spec/lib/http/cache_spec.rb | 1 + spec/lib/http/client_spec.rb | 5 ++++- 8 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 lib/http/cache/null_cache.rb diff --git a/README.md b/README.md index 4f6a3b82..403982c9 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,27 @@ There's a little more to it, but that's the core idea! * [Full parallel HTTP fetcher example](https://github.com/httprb/http.rb/wiki/Parallel-requests-with-Celluloid%3A%3AIO) * See also: [Celluloid::IO](https://github.com/celluloid/celluloid-io) +### Caching + +http.rb provides caching of HTTP request (per +[RFC 7234](https://tools.ietf.org/html/rfc7234)) when configured to do +so. + +```ruby +require 'http' + +http = HTTP.with_cache(metastore: "file:/var/cache/my-app-http/meta", + entitystore: "file:/var/cache/my-app-http/entity") + +http.get("http://example.com/") # makes request +http.get("http://example.com/") # skips making request and returns + # previously cached response +``` + +http.rb's caching is backed by +[rack-cache's excellent storage subsystem](http://rtomayko.github.io/rack-cache/storage.html). Any +storage URL supported by rack-cache is supported by http.rb's cache. + Supported Ruby Versions ----------------------- diff --git a/lib/http.rb b/lib/http.rb index 9b5d269e..32f61286 100644 --- a/lib/http.rb +++ b/lib/http.rb @@ -9,7 +9,6 @@ require "http/response" require "http/response/body" require "http/response/parser" -require "http/response/string_body" # HTTP should be easy module HTTP diff --git a/lib/http/cache.rb b/lib/http/cache.rb index 7a6ba241..40210b00 100644 --- a/lib/http/cache.rb +++ b/lib/http/cache.rb @@ -3,15 +3,6 @@ module HTTP class Cache - # NoOp cache. Always makes the request. - class NullCache - # @return [Response] the result of the provided block - # @yield [request, options] so that the request can actually be made - def perform(request, options) - yield(request, options) - end - end - # NoOp logger. class NullLogger def error(_msg = nil) diff --git a/lib/http/cache/null_cache.rb b/lib/http/cache/null_cache.rb new file mode 100644 index 00000000..31ef7ddb --- /dev/null +++ b/lib/http/cache/null_cache.rb @@ -0,0 +1,13 @@ +module HTTP + class Cache + # NoOp cache. Always makes the request. Allows avoiding + # conditionals in the request flow. + class NullCache + # @return [Response] the result of the provided block + # @yield [request, options] so that the request can actually be made + def perform(request, options) + yield(request, options) + end + end + end +end diff --git a/lib/http/client.rb b/lib/http/client.rb index b758de64..f75c2dda 100644 --- a/lib/http/client.rb +++ b/lib/http/client.rb @@ -3,7 +3,6 @@ require "http/form_data" require "http/options" require "http/redirector" -require "http/cache" module HTTP # Clients make requests and receive responses diff --git a/lib/http/options.rb b/lib/http/options.rb index e898ea40..3209a9dc 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -1,7 +1,7 @@ require "http/headers" require "openssl" require "socket" -require "http/cache" +require "http/cache/null_cache" module HTTP class Options @@ -35,14 +35,13 @@ class Options # Follow redirects attr_accessor :follow - # Cache, needs a hash. See @default_cache. Valid modes are: false, :public, :private + # Cache to use when making requests attr_accessor :cache protected :response=, :headers=, :proxy=, :params=, :form=, :json=, :follow= @default_socket_class = TCPSocket @default_ssl_socket_class = OpenSSL::SSL::SSLSocket - @default_cache = Http::Cache::NullCache.new class << self @@ -91,6 +90,7 @@ def with_cache(cache_or_cache_options) cache = if cache_or_cache_options.respond_to? :perform cache_or_cache_options else + require "http/cache" HTTP::Cache.new(cache_or_cache_options) end diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index cec56b69..584c7990 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -1,4 +1,5 @@ require "support/dummy_server" +require "http/cache" RSpec.describe HTTP::Cache do describe "creation" do diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 2c269a2a..5361c4b2 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -72,11 +72,14 @@ def simple_response(body, status = 200) end describe "caching" do + before { require "http/cache" } + let(:sn) { SecureRandom.urlsafe_base64(3) } it "returns cached responses if they exist" do cached_response = simple_response("cached").caching - StubbedClient.new(:cache => HTTP::Cache.new(:metastore => "heap:/", :entitystore => "heap:/")) + StubbedClient.new(:cache => + HTTP::Cache.new(:metastore => "heap:/", :entitystore => "heap:/")) .stub("http://example.com/#{sn}" => cached_response) .get("http://example.com/#{sn}") From 2d1f8475f9004722c930df45fbbfe9fe99be60c3 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Wed, 4 Mar 2015 11:43:33 -0700 Subject: [PATCH 33/39] reduce length of options to appease rubocop --- lib/http/options.rb | 133 ++++++++++------------------ spec/lib/http/options/merge_spec.rb | 2 +- 2 files changed, 50 insertions(+), 85 deletions(-) diff --git a/lib/http/options.rb b/lib/http/options.rb index 3209a9dc..ce766b10 100644 --- a/lib/http/options.rb +++ b/lib/http/options.rb @@ -5,43 +5,9 @@ module HTTP class Options - # How to format the response [:object, :body, :parse_body] - attr_accessor :response - - # HTTP headers to include in the request - attr_accessor :headers - - # Query string params to add to the url - attr_accessor :params - - # Form data to embed in the request - attr_accessor :form - - # JSON data to embed in the request - attr_accessor :json - - # Explicit request body of the request - attr_accessor :body - - # HTTP proxy to route request - attr_accessor :proxy - - # Socket classes - attr_accessor :socket_class, :ssl_socket_class - - # SSL context - attr_accessor :ssl_context - - # Follow redirects - attr_accessor :follow - - # Cache to use when making requests - attr_accessor :cache - - protected :response=, :headers=, :proxy=, :params=, :form=, :json=, :follow= - @default_socket_class = TCPSocket @default_ssl_socket_class = OpenSSL::SSL::SSLSocket + @default_cache = Http::Cache::NullCache.new class << self @@ -52,50 +18,56 @@ def new(options = {}) return options if options.is_a?(self) super end - end - def initialize(options = {}) - @response = options[:response] || :auto - @proxy = options[:proxy] || {} - @body = options[:body] - @params = options[:params] - @form = options[:form] - @json = options[:json] - @follow = options[:follow] + def defined_options + @defined_options ||= [] + end - @headers = HTTP::Headers.coerce(options[:headers] || {}) + protected - @socket_class = options[:socket_class] || self.class.default_socket_class - @ssl_socket_class = options[:ssl_socket_class] || self.class.default_ssl_socket_class - @ssl_context = options[:ssl_context] + def def_option(name, &interpreter) + defined_options << name.to_sym + interpreter ||= ->(v) { v } - @cache = options[:cache] || self.class.default_cache + attr_accessor name + protected :"#{name}=" + + define_method(:"with_#{name}") do |value| + dup { |opts| opts.send(:"#{name}=", instance_exec(value, &interpreter)) } + end + end end - def with_headers(headers) - dup do |opts| - opts.headers = self.headers.merge(headers) + def initialize(options = {}) + defaults = {:response => :auto, + :proxy => {}, + :socket_class => self.class.default_socket_class, + :ssl_socket_class => self.class.default_ssl_socket_class, + :cache => self.class.default_cache, + :headers => {}} + + opts_w_defaults = defaults.merge(options) + opts_w_defaults[:headers] = HTTP::Headers.coerce(opts_w_defaults[:headers]) + + opts_w_defaults.each do |(opt_name, opt_val)| + self[opt_name] = opt_val end end - %w(proxy params form json body follow).each do |method_name| - class_eval <<-RUBY, __FILE__, __LINE__ - def with_#{method_name}(value) - dup { |opts| opts.#{method_name} = value } - end - RUBY + def_option :headers do |headers| + self.headers.merge(headers) end - def with_cache(cache_or_cache_options) - cache = if cache_or_cache_options.respond_to? :perform - cache_or_cache_options - else - require "http/cache" - HTTP::Cache.new(cache_or_cache_options) - end + %w(proxy params form json body follow response socket_class ssl_socket_class ssl_context).each do |method_name| + def_option method_name + end - dup do |opts| - opts.cache = cache + def_option :cache do |cache_or_cache_options| + if cache_or_cache_options.respond_to? :perform + cache_or_cache_options + else + require "http/cache" + HTTP::Cache.new(cache_or_cache_options) end end @@ -118,23 +90,10 @@ def merge(other) end def to_hash - # FIXME: hardcoding these fields blows! We should have a declarative - # way of specifying all the options fields, and ensure they *all* - # get serialized here, rather than manually having to add them each time - { - :response => response, - :headers => headers.to_h, - :proxy => proxy, - :params => params, - :form => form, - :json => json, - :body => body, - :follow => follow, - :socket_class => socket_class, - :ssl_socket_class => ssl_socket_class, - :ssl_context => ssl_context, - :cache => cache - } + hash_pairs = self.class + .defined_options + .flat_map { |opt_name| [opt_name, self[opt_name]] } + Hash[*hash_pairs] end def dup @@ -143,6 +102,12 @@ def dup dupped end + protected + + def []=(option, val) + send(:"#{option}=", val) + end + private def argument_error!(message) diff --git a/spec/lib/http/options/merge_spec.rb b/spec/lib/http/options/merge_spec.rb index ddf91b7d..32820a18 100644 --- a/spec/lib/http/options/merge_spec.rb +++ b/spec/lib/http/options/merge_spec.rb @@ -40,7 +40,7 @@ :form => {:bar => "bar"}, :body => "body-bar", :json => {:bar => "bar"}, - :headers => {"Accept" => "xml", "Foo" => "foo", "Bar" => "bar"}, + :headers => HTTP::Headers.coerce("Foo" => "foo", "Accept" => "xml", "Bar" => "bar"), :proxy => {:proxy_address => "127.0.0.1", :proxy_port => 8080}, :follow => nil, :socket_class => described_class.default_socket_class, From 833deae533202ccc9b967f8215e5b48ec0a46a5d Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 08:33:58 -0600 Subject: [PATCH 34/39] removed unused dependency --- Gemfile | 1 - 1 file changed, 1 deletion(-) diff --git a/Gemfile b/Gemfile index 9c909581..91ccafc5 100644 --- a/Gemfile +++ b/Gemfile @@ -26,7 +26,6 @@ group :test do gem "rspec-its" gem "rubocop" gem "yardstick" - gem "fakefs", :require => "fakefs/safe" end group :doc do From 88dc18173720374a343e066cc938f71ed86b2675 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 08:37:50 -0600 Subject: [PATCH 35/39] move require to top of file --- spec/lib/http/client_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 5361c4b2..52b51968 100644 --- a/spec/lib/http/client_spec.rb +++ b/spec/lib/http/client_spec.rb @@ -1,4 +1,5 @@ require "support/dummy_server" +require "http/cache" RSpec.describe HTTP::Client do run_server(:dummy) { DummyServer.new } @@ -72,8 +73,6 @@ def simple_response(body, status = 200) end describe "caching" do - before { require "http/cache" } - let(:sn) { SecureRandom.urlsafe_base64(3) } it "returns cached responses if they exist" do From 48f3e5c49a617ece771aea3e104a4f506e03a6e1 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 08:42:11 -0600 Subject: [PATCH 36/39] simplify spec --- spec/lib/http/options/merge_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/http/options/merge_spec.rb b/spec/lib/http/options/merge_spec.rb index 32820a18..e305608b 100644 --- a/spec/lib/http/options/merge_spec.rb +++ b/spec/lib/http/options/merge_spec.rb @@ -40,7 +40,7 @@ :form => {:bar => "bar"}, :body => "body-bar", :json => {:bar => "bar"}, - :headers => HTTP::Headers.coerce("Foo" => "foo", "Accept" => "xml", "Bar" => "bar"), + :headers => {"Foo" => "foo", "Accept" => "xml", "Bar" => "bar"}, :proxy => {:proxy_address => "127.0.0.1", :proxy_port => 8080}, :follow => nil, :socket_class => described_class.default_socket_class, From eeeb2c84da07fafec10050c305c2ae69b1efe983 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 09:03:51 -0600 Subject: [PATCH 37/39] improved caching response spec --- spec/lib/http/response/caching_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/http/response/caching_spec.rb b/spec/lib/http/response/caching_spec.rb index ca3978d1..62f362eb 100644 --- a/spec/lib/http/response/caching_spec.rb +++ b/spec/lib/http/response/caching_spec.rb @@ -197,6 +197,6 @@ describe "#caching" do subject(:caching_response) { response.caching } - it { is_expected.to be caching_response } + it { is_expected.to be_kind_of described_class } end end From 86741e2f03b7c64336915fa0ebcc5b6b92c4dff4 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 09:28:00 -0600 Subject: [PATCH 38/39] bury the lede in some specs --- spec/lib/http/cache_spec.rb | 22 ++++++++++------------ spec/lib/http/request/caching_spec.rb | 5 ++--- spec/lib/http/response/caching_spec.rb | 9 ++++----- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/spec/lib/http/cache_spec.rb b/spec/lib/http/cache_spec.rb index 584c7990..bcee1875 100644 --- a/spec/lib/http/cache_spec.rb +++ b/spec/lib/http/cache_spec.rb @@ -12,6 +12,16 @@ end let(:opts) { options } + let(:sn) { SecureRandom.urlsafe_base64(3) } + let(:request) { HTTP::Request.new(:get, "http://example.com/#{sn}") } + + let(:origin_response) do + HTTP::Response.new(200, + "http/1.1", + {"Cache-Control" => "private"}, + "origin") + end + subject { described_class.new(:metastore => "heap:/", :entitystore => "heap:/") } describe "#perform" do @@ -155,18 +165,6 @@ end end - # Background - - let(:sn) { SecureRandom.urlsafe_base64(3) } - let(:request) { HTTP::Request.new(:get, "http://example.com/#{sn}") } - - let(:origin_response) do - HTTP::Response.new(200, - "http/1.1", - {"Cache-Control" => "private"}, - "origin") - end - let(:cached_response) { nil } # cold cache by default def build_cached_response(*args) diff --git a/spec/lib/http/request/caching_spec.rb b/spec/lib/http/request/caching_spec.rb index 824bdbd0..2eeaa098 100644 --- a/spec/lib/http/request/caching_spec.rb +++ b/spec/lib/http/request/caching_spec.rb @@ -1,4 +1,6 @@ RSpec.describe HTTP::Request::Caching do + let(:request) { HTTP::Request.new(:get, "http://example.com/") } + subject(:caching_request) { described_class.new request } describe "#cache_headers" do @@ -124,9 +126,6 @@ end end - # Background - let(:request) { HTTP::Request.new(:get, "http://example.com/") } - describe "#caching" do subject(:caching_request) { request.caching } it { is_expected.to be caching_request } diff --git a/spec/lib/http/response/caching_spec.rb b/spec/lib/http/response/caching_spec.rb index 62f362eb..1081e247 100644 --- a/spec/lib/http/response/caching_spec.rb +++ b/spec/lib/http/response/caching_spec.rb @@ -1,4 +1,8 @@ RSpec.describe HTTP::Response::Caching do + let(:cache_control) { "" } + let(:headers) { {"cache-control" => cache_control} } + let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } + subject(:caching_response) { described_class.new response } describe "#cache_headers" do @@ -190,11 +194,6 @@ end end - # Background - let(:cache_control) { "" } - let(:headers) { {"cache-control" => cache_control} } - let(:response) { HTTP::Response.new(200, "http/1.1", headers, "") } - describe "#caching" do subject(:caching_response) { response.caching } it { is_expected.to be_kind_of described_class } From c234d498f60e955ad9c16c2850d0d5cd4262ecb2 Mon Sep 17 00:00:00 2001 From: Peter Williams Date: Mon, 9 Mar 2015 16:54:45 -0600 Subject: [PATCH 39/39] remove unnecessary/incorrect date addition code in request --- lib/http/request.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/http/request.rb b/lib/http/request.rb index 226bf620..2c359735 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -82,8 +82,6 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") @headers["Host"] ||= default_host @headers["User-Agent"] ||= USER_AGENT - now = Time.now - @headers["Date"] = now.httpdate end # Returns new Request with updated uri