From 50215f9525b6b5e3bfe703724b9f68177ed8565d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 20 Sep 2010 10:18:44 +0200 Subject: Rely on Rack::Session stores API for more compatibility across the Ruby world. --- Gemfile | 1 + actionpack/CHANGELOG | 6 +- actionpack/lib/action_controller/test_case.rb | 8 +- actionpack/lib/action_dispatch/http/url.rb | 5 - .../middleware/session/abstract_store.rb | 280 ++++----------------- .../middleware/session/cookie_store.rb | 64 ++--- .../middleware/session/mem_cache_store.rb | 53 +--- .../test/dispatch/session/cookie_store_test.rb | 12 - activerecord/lib/active_record/session_store.rb | 7 +- 9 files changed, 90 insertions(+), 346 deletions(-) diff --git a/Gemfile b/Gemfile index 9d29c42e1c..7fe1c3ad22 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ else gem "arel", :git => "git://github.com/rails/arel.git" end +gem "rack", :git => "git://github.com/rack/rack.git" gem "rails", :path => File.dirname(__FILE__) gem "rake", ">= 0.8.7" diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6f8314109b..6352b97a6b 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,12 +1,12 @@ *Rails 3.1.0 (unreleased)* +* Rely on Rack::Session stores API for more compatibility across the Ruby world. This is backwards incompatible since Rack::Session expects #get_session to accept 4 arguments and requires #destroy_session instead of simply #destroy. [José Valim] + * file_field automatically adds :multipart => true to the enclosing form. [Santiago Pastorino] * Renames csrf_meta_tag -> csrf_meta_tags, and aliases csrf_meta_tag for backwards compatibility. [fxn] -* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used - for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply - to the browser only. +* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply to the browser only. [Yehuda Katz, Carl Lerche] *Rails 3.0.0 (August 29, 2010)* diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 1af75fc2d7..d7b54c2abc 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -187,15 +187,17 @@ module ActionController end end - class TestSession < ActionDispatch::Session::AbstractStore::SessionHash #:nodoc: - DEFAULT_OPTIONS = ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS + class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: + DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) replace(session.stringify_keys) @loaded = true end - def exists?; true; end + def exists? + true + end end # Superclass for ActionController functional tests. Functional tests allow you to diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3e5cd6a2f9..cfee95eb4b 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -18,11 +18,6 @@ module ActionDispatch @protocol ||= ssl? ? 'https://' : 'http://' end - # Is this an SSL request? - def ssl? - @ssl ||= @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' - end - # Returns the \host for this request, such as "example.com". def raw_host_with_port if forwarded = env["HTTP_X_FORWARDED_HOST"] diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index db0187c015..679ba7fc8e 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -1,5 +1,6 @@ require 'rack/utils' require 'rack/request' +require 'rack/session/abstract/id' require 'action_dispatch/middleware/cookies' require 'active_support/core_ext/object/blank' @@ -8,252 +9,69 @@ module ActionDispatch class SessionRestoreError < StandardError #:nodoc: end - class AbstractStore - ENV_SESSION_KEY = 'rack.session'.freeze - ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze - - # thin wrapper around Hash that allows us to lazily - # load session id into session_options - class OptionsHash < Hash - def initialize(by, env, default_options) - @by = by - @env = env - @session_id_loaded = false - merge!(default_options) - end - - def [](key) - if key == :id - load_session_id! unless key?(:id) || has_session_id? - end - super - end - - private - - def has_session_id? - @session_id_loaded - end - - def load_session_id! - self[:id] = @by.send(:extract_session_id, @env) - @session_id_loaded = true - end - end - - class SessionHash < Hash - def initialize(by, env) - super() - @by = by - @env = env - @loaded = false - end - - def [](key) - load_for_read! - super(key.to_s) - end - - def has_key?(key) - load_for_read! - super(key.to_s) - end - - def []=(key, value) - load_for_write! - super(key.to_s, value) - end - - def clear - load_for_write! - super - end - - def to_hash - load_for_read! - h = {}.replace(self) - h.delete_if { |k,v| v.nil? } - h - end - - def update(hash) - load_for_write! - super(hash.stringify_keys) - end - - def delete(key) - load_for_write! - super(key.to_s) - end - - def inspect - load_for_read! - super - end - - def exists? - return @exists if instance_variable_defined?(:@exists) - @exists = @by.send(:exists?, @env) - end - - def loaded? - @loaded - end - - def destroy - clear - @by.send(:destroy, @env) if defined?(@by) && @by - @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if defined?(@env) && @env && @env[ENV_SESSION_OPTIONS_KEY] - @loaded = false - end - - private - - def load_for_read! - load! if !loaded? && exists? - end - - def load_for_write! - load! unless loaded? - end - - def load! - id, session = @by.send(:load_session, @env) - @env[ENV_SESSION_OPTIONS_KEY][:id] = id - replace(session.stringify_keys) - @loaded = true - end - + module DestroyableSession + def destroy + clear + options = @env[Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY] if @env + options ||= {} + @by.send(:destroy_session, @env, options[:id], options) if @by + options[:id] = nil + @loaded = false end + end - DEFAULT_OPTIONS = { - :key => '_session_id', - :path => '/', - :domain => nil, - :expire_after => nil, - :secure => false, - :httponly => true, - :cookie_only => true - } + ::Rack::Session::Abstract::SessionHash.send :include, DestroyableSession + module Compatibility def initialize(app, options = {}) - @app = app - @default_options = DEFAULT_OPTIONS.merge(options) - @key = @default_options.delete(:key).freeze - @cookie_only = @default_options.delete(:cookie_only) - ensure_session_key! + options[:key] ||= '_session_id' + super end - def call(env) - prepare!(env) - response = @app.call(env) - - session_data = env[ENV_SESSION_KEY] - options = env[ENV_SESSION_OPTIONS_KEY] - - if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after] - request = ActionDispatch::Request.new(env) - - return response if (options[:secure] && !request.ssl?) - - session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded? - - sid = options[:id] || generate_sid - session_data = session_data.to_hash - - value = set_session(env, sid, session_data) - return response unless value - - cookie = { :value => value } - if options[:expire_after] - cookie[:expires] = Time.now + options.delete(:expire_after) - end - - set_cookie(request, cookie.merge!(options)) - end - - response + def generate_sid + ActiveSupport::SecureRandom.hex(16) end + end - private - - def prepare!(env) - env[ENV_SESSION_KEY] = SessionHash.new(self, env) - env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options) - end - - def generate_sid - ActiveSupport::SecureRandom.hex(16) - end - - def set_cookie(request, options) - if request.cookie_jar[@key] != options[:value] || !options[:expires].nil? - request.cookie_jar[@key] = options - end - end - - def load_session(env) - stale_session_check! do - sid = current_session_id(env) - sid, session = get_session(env, sid) - [sid, session] - end - end - - def extract_session_id(env) - stale_session_check! do - request = ActionDispatch::Request.new(env) - sid = request.cookies[@key] - sid ||= request.params[@key] unless @cookie_only - sid - end - end - - def current_session_id(env) - env[ENV_SESSION_OPTIONS_KEY][:id] - end + module StaleSessionCheck + def load_session(env) + stale_session_check! { super } + end - def ensure_session_key! - if @key.blank? - raise ArgumentError, 'A key is required to write a ' + - 'cookie containing the session data. Use ' + - 'config.session_store SESSION_STORE, { :key => ' + - '"_myapp_session" } in config/application.rb' - end - end + def extract_session_id(env) + stale_session_check! { super } + end - def stale_session_check! - yield - rescue ArgumentError => argument_error - if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} - begin - # Note that the regexp does not allow $1 to end with a ':' - $1.constantize - rescue LoadError, NameError => const_error - raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" - end - retry - else - raise + def stale_session_check! + yield + rescue ArgumentError => argument_error + if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} + begin + # Note that the regexp does not allow $1 to end with a ':' + $1.constantize + rescue LoadError, NameError => const_error + raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" end + retry + else + raise end + end + end - def exists?(env) - current_session_id(env).present? - end - - def get_session(env, sid) - raise '#get_session needs to be implemented.' - end + class AbstractStore < Rack::Session::Abstract::ID + include Compatibility + include StaleSessionCheck - def set_session(env, sid, session_data) - raise '#set_session needs to be implemented and should return ' << - 'the value to be stored in the cookie (usually the sid)' - end + def destroy_session(env, sid, options) + ActiveSupport::Deprecation.warn "Implementing #destroy in session stores is deprecated. " << + "Please implement destroy_session(env, session_id, options) instead." + destroy(env) + end - def destroy(env) - raise '#destroy needs to be implemented.' - end + def destroy(env) + raise '#destroy needs to be implemented.' + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index ca1494425f..9c9ccc62f5 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -1,5 +1,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/blank' +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/cookie' module ActionDispatch module Session @@ -38,58 +40,32 @@ module ActionDispatch # "rake secret" and set the key in config/initializers/secret_token.rb. # # Note that changing digest or secret invalidates all existing sessions! - class CookieStore < AbstractStore - - def initialize(app, options = {}) - super(app, options.merge!(:cookie_only => true)) - freeze - end + class CookieStore < Rack::Session::Cookie + include Compatibility + include StaleSessionCheck private - def load_session(env) - data = unpacked_cookie_data(env) - data = persistent_session_id!(data) - [data["session_id"], data] - end - - def extract_session_id(env) - if data = unpacked_cookie_data(env) - data["session_id"] - else - nil - end - end - - def unpacked_cookie_data(env) - env["action_dispatch.request.unsigned_session_cookie"] ||= begin - stale_session_check! do - request = ActionDispatch::Request.new(env) - if data = request.cookie_jar.signed[@key] - data.stringify_keys! - end - data || {} + def unpacked_cookie_data(env) + env["action_dispatch.request.unsigned_session_cookie"] ||= begin + stale_session_check! do + request = ActionDispatch::Request.new(env) + if data = request.cookie_jar.signed[@key] + data.stringify_keys! end + data || {} end end + end - def set_cookie(request, options) - request.cookie_jar.signed[@key] = options - end - - def set_session(env, sid, session_data) - persistent_session_id!(session_data, sid) - end - - def destroy(env) - # session data is stored on client; nothing to do here - end + def set_session(env, sid, session_data, options) + persistent_session_id!(session_data, sid) + end - def persistent_session_id!(data, sid=nil) - data ||= {} - data["session_id"] ||= sid || generate_sid - data - end + def set_cookie(env, session_id, cookie) + request = ActionDispatch::Request.new(env) + request.cookie_jar.signed[@key] = cookie + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb index 28e3dbd732..4dd9a946c2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb @@ -1,56 +1,17 @@ +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/memcache' + module ActionDispatch module Session - class MemCacheStore < AbstractStore + class MemCacheStore < Rack::Session::Memcache + include Compatibility + include StaleSessionCheck + def initialize(app, options = {}) require 'memcache' - - # Support old :expires option options[:expire_after] ||= options[:expires] - - super - - @default_options = { - :namespace => 'rack:session', - :memcache_server => 'localhost:11211' - }.merge(@default_options) - - @pool = options[:cache] || MemCache.new(@default_options[:memcache_server], @default_options) - unless @pool.servers.any? { |s| s.alive? } - raise "#{self} unable to find server during initialization." - end - @mutex = Mutex.new - super end - - private - def get_session(env, sid) - sid ||= generate_sid - begin - session = @pool.get(sid) || {} - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - session = {} - end - [sid, session] - end - - def set_session(env, sid, session_data) - options = env['rack.session.options'] - expiry = options[:expire_after] || 0 - @pool.set(sid, session_data, expiry) - sid - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - - def destroy(env) - if sid = current_session_id(env) - @pool.delete(sid) - end - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - end end end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 3489f628ed..256d0781c7 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -53,18 +53,6 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def rescue_action(e) raise end end - def test_raises_argument_error_if_missing_session_key - assert_raise(ArgumentError, nil.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => nil, :secret => SessionSecret) - } - - assert_raise(ArgumentError, ''.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => '', :secret => SessionSecret) - } - end - def test_setting_session_value with_test_route_set do get '/set_session_value' diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index 01cc14b8d6..3fc596e02a 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -288,6 +288,7 @@ module ActiveRecord self.session_class = Session SESSION_RECORD_KEY = 'rack.session.record' + ENV_SESSION_OPTIONS_KEY = Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY private def get_session(env, sid) @@ -299,7 +300,7 @@ module ActiveRecord end end - def set_session(env, sid, session_data) + def set_session(env, sid, session_data, options) Base.silence do record = get_session_model(env, sid) record.data = session_data @@ -316,12 +317,14 @@ module ActiveRecord sid end - def destroy(env) + def destroy_session(env, session_id, options) if sid = current_session_id(env) Base.silence do get_session_model(env, sid).destroy end end + + generate_sid unless options[:drop] end def get_session_model(env, sid) -- cgit v1.2.3 From 74dd8a3681c6984ea35c879f88c6a87521b58ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 22 Sep 2010 21:40:14 +0200 Subject: Move ETag and ConditionalGet logic from AD::Response to the middleware stack. --- actionpack/lib/action_dispatch/http/cache.rb | 22 +---- actionpack/lib/action_dispatch/http/response.rb | 2 +- actionpack/test/controller/new_base/etag_test.rb | 46 --------- actionpack/test/controller/render_test.rb | 118 ----------------------- actionpack/test/dispatch/response_test.rb | 11 +-- railties/lib/rails/application.rb | 4 +- railties/test/application/middleware_test.rb | 64 ++++++++---- 7 files changed, 51 insertions(+), 216 deletions(-) delete mode 100644 actionpack/test/controller/new_base/etag_test.rb diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 047fab006e..4061222d11 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -50,8 +50,7 @@ module ActionDispatch if cache_control = self["Cache-Control"] cache_control.split(/,\s*/).each do |segment| first, last = segment.split("=") - last ||= true - @cache_control[first.to_sym] = last + @cache_control[first.to_sym] = last || true end end end @@ -88,28 +87,9 @@ module ActionDispatch def handle_conditional_get! if etag? || last_modified? || !@cache_control.empty? set_conditional_cache_control! - elsif nonempty_ok_response? - self.etag = body - - if request && request.etag_matches?(etag) - self.status = 304 - self.body = [] - end - - set_conditional_cache_control! - else - headers["Cache-Control"] = "no-cache" end end - def nonempty_ok_response? - @status == 200 && string_body? - end - - def string_body? - !@blank && @body.respond_to?(:all?) && @body.all? { |part| part.is_a?(String) } - end - DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate" def set_conditional_cache_control! diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 151c90167b..72871e328a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -132,7 +132,7 @@ module ActionDispatch # :nodoc: # information. attr_accessor :charset, :content_type - CONTENT_TYPE = "Content-Type" + CONTENT_TYPE = "Content-Type" cattr_accessor(:default_charset) { "utf-8" } diff --git a/actionpack/test/controller/new_base/etag_test.rb b/actionpack/test/controller/new_base/etag_test.rb deleted file mode 100644 index 2bca5aec6a..0000000000 --- a/actionpack/test/controller/new_base/etag_test.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'abstract_unit' - -module Etags - class BasicController < ActionController::Base - self.view_paths = [ActionView::FixtureResolver.new( - "etags/basic/base.html.erb" => "Hello from without_layout.html.erb", - "layouts/etags.html.erb" => "teh <%= yield %> tagz" - )] - - def without_layout - render :action => "base" - end - - def with_layout - render :action => "base", :layout => "etags" - end - end - - class EtagTest < Rack::TestCase - describe "Rendering without any special etag options returns an etag that is an MD5 hash of its text" - - test "an action without a layout" do - get "/etags/basic/without_layout" - - body = "Hello from without_layout.html.erb" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - test "an action with a layout" do - get "/etags/basic/with_layout" - - body = "teh Hello from without_layout.html.erb tagz" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - private - - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - end -end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 7ca784c467..fca8de60bc 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -99,11 +99,6 @@ class TestController < ActionController::Base render :template => "test/hello_world" end - def render_hello_world_with_etag_set - response.etag = "hello_world" - render :template => "test/hello_world" - end - # :ported: compatibility def render_hello_world_with_forward_slash render :template => "/test/hello_world" @@ -1386,119 +1381,6 @@ class ExpiresInRenderTest < ActionController::TestCase end end - -class EtagRenderTest < ActionController::TestCase - tests TestController - - def setup - super - @request.host = "www.nextangle.com" - @expected_bang_etag = etag_for(expand_key([:foo, 123])) - end - - def test_render_blank_body_shouldnt_set_etag - get :blank_response - assert !@response.etag? - end - - def test_render_200_should_set_etag - get :render_hello_world_from_variable - assert_equal etag_for("hello david"), @response.headers['ETag'] - assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control'] - end - - def test_render_against_etag_request_should_304_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - assert @response.body.empty? - end - - def test_render_against_etag_request_should_have_no_content_length_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert !@response.headers.has_key?("Content-Length") - end - - def test_render_against_etag_request_should_200_when_no_match - @request.if_none_match = etag_for("hello somewhere else") - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - assert !@response.body.empty? - end - - def test_render_should_not_set_etag_when_last_modified_has_been_specified - get :render_hello_world_with_last_modified_set - assert_equal 200, @response.status.to_i - assert_not_nil @response.last_modified - assert_nil @response.etag - assert_present @response.body - end - - def test_render_with_etag - get :render_hello_world_from_variable - expected_etag = etag_for('hello david') - assert_equal expected_etag, @response.headers['ETag'] - @response = ActionController::TestResponse.new - - @request.if_none_match = expected_etag - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - - @response = ActionController::TestResponse.new - @request.if_none_match = "\"diftag\"" - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - end - - def render_with_404_shouldnt_have_etag - get :render_custom_code - assert_nil @response.headers['ETag'] - end - - def test_etag_should_not_be_changed_when_already_set - get :render_hello_world_with_etag_set - assert_equal etag_for("hello_world"), @response.headers['ETag'] - end - - def test_etag_should_govern_renders_with_layouts_too - get :builder_layout_test - assert_equal "\n\n

Hello

\n

This is grand!

\n\n\n", @response.body - assert_equal etag_for("\n\n

Hello

\n

This is grand!

\n\n\n"), @response.headers['ETag'] - end - - def test_etag_with_bang_should_set_etag - get :conditional_hello_with_bangs - assert_equal @expected_bang_etag, @response.headers["ETag"] - assert_response :success - end - - def test_etag_with_bang_should_obey_if_none_match - @request.if_none_match = @expected_bang_etag - get :conditional_hello_with_bangs - assert_response :not_modified - end - - def test_etag_with_public_true_should_set_header - get :conditional_hello_with_public_header - assert_equal "public", @response.headers['Cache-Control'] - end - - def test_etag_with_public_true_should_set_header_and_retain_other_headers - get :conditional_hello_with_public_header_and_expires_at - assert_equal "max-age=60, public", @response.headers['Cache-Control'] - end - - protected - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - - def expand_key(args) - ActiveSupport::Cache.expand_cache_key(args) - end -end - class LastModifiedRenderTest < ActionController::TestCase tests TestController diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index cd0418c338..be6398fead 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -11,9 +11,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"' + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] @@ -27,9 +25,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"' + "Content-Type" => "text/html; charset=utf-8" }, headers) end @@ -41,8 +37,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "no-cache" + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 4d04184b20..075e3c5692 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -145,8 +145,8 @@ module Rails rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache require "action_dispatch/http/rack_cache" if rack_cache + middleware.use ::Rack::Cache, rack_cache if rack_cache - middleware.use ::Rack::Cache, rack_cache if rack_cache middleware.use ::ActionDispatch::Static, config.static_asset_paths if config.serve_static_assets middleware.use ::Rack::Lock if !config.allow_concurrency middleware.use ::Rack::Runtime @@ -165,6 +165,8 @@ module Rails middleware.use ::ActionDispatch::ParamsParser middleware.use ::Rack::MethodOverride middleware.use ::ActionDispatch::Head + middleware.use ::Rack::ConditionalGet + middleware.use ::Rack::ETag, "no-cache" middleware.use ::ActionDispatch::BestStandardsSupport, config.action_dispatch.best_standards_support if config.action_dispatch.best_standards_support end end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 2372ad85b8..173ac40b12 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -36,6 +36,8 @@ module ApplicationTests "ActionDispatch::ParamsParser", "Rack::MethodOverride", "ActionDispatch::Head", + "Rack::ConditionalGet", + "Rack::ETag", "ActionDispatch::BestStandardsSupport" ], middleware end @@ -45,27 +47,7 @@ module ApplicationTests boot! - assert_equal [ - "Rack::Cache", - "ActionDispatch::Static", - "Rack::Lock", - "ActiveSupport::Cache::Strategy::LocalCache", - "Rack::Runtime", - "Rails::Rack::Logger", - "ActionDispatch::ShowExceptions", - "ActionDispatch::RemoteIp", - "Rack::Sendfile", - "ActionDispatch::Callbacks", - "ActiveRecord::ConnectionAdapters::ConnectionManagement", - "ActiveRecord::QueryCache", - "ActionDispatch::Cookies", - "ActionDispatch::Session::CookieStore", - "ActionDispatch::Flash", - "ActionDispatch::ParamsParser", - "Rack::MethodOverride", - "ActionDispatch::Head", - "ActionDispatch::BestStandardsSupport" - ], middleware + assert_equal "Rack::Cache", middleware.first end test "removing Active Record omits its middleware" do @@ -129,6 +111,46 @@ module ApplicationTests assert_equal "Rack::Config", middleware.first end + # ConditionalGet + Etag + test "conditional get + etag middlewares handle http caching based on body" do + make_basic_app + + class ::OmgController < ActionController::Base + def index + if params[:nothing] + render :text => "" + else + render :text => "OMG" + end + end + end + + etag = "5af83e3196bf99f440f31f2e1a6c9afe".inspect + + get "/" + assert_equal 200, last_response.status + assert_equal "OMG", last_response.body + assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"] + assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"] + assert_equal etag, last_response.headers["Etag"] + + get "/", {}, "HTTP_IF_NONE_MATCH" => etag + assert_equal 304, last_response.status + assert_equal "", last_response.body + assert_equal nil, last_response.headers["Content-Type"] + assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"] + assert_equal etag, last_response.headers["Etag"] + + get "/?nothing=true" + puts last_response.body + assert_equal 200, last_response.status + assert_equal "", last_response.body + assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"] + assert_equal "no-cache", last_response.headers["Cache-Control"] + assert_equal nil, last_response.headers["Etag"] + end + + # Show exceptions middleware test "show exceptions middleware filter backtrace before logging" do my_middleware = Struct.new(:app) do def call(env) -- cgit v1.2.3 From 653acac069e66f53b791caa4838a1e25de905f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 3 Oct 2010 21:45:27 +0200 Subject: Solve some warnings and a failing test. --- actionpack/lib/action_controller/test_case.rb | 1 + actionpack/lib/action_dispatch/http/request.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index d7b54c2abc..6061945622 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -191,6 +191,7 @@ module ActionController DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) + @env, @by = nil, nil replace(session.stringify_keys) @loaded = true end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 09d6ba8223..bbcdefb190 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -199,7 +199,7 @@ module ActionDispatch # TODO This should be broken apart into AD::Request::Session and probably # be included by the session middleware. def reset_session - session.destroy if session + session.destroy if session && session.respond_to?(:destroy) self.session = {} @env['action_dispatch.request.flash_hash'] = nil end -- cgit v1.2.3