Skip to content

Commit

Permalink
Fix action-cached exception responses.
Browse files Browse the repository at this point in the history
Methods raising ActiveRecord::RecordNotFound were returning 404 on first request and 200 OK with blank body on subsequent requests.

[#2533 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information
johndouthat authored and jeremy committed Apr 30, 2009
1 parent 00d1a57 commit e59835b
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
10 changes: 9 additions & 1 deletion actionpack/lib/action_controller/caching/actions.rb
Expand Up @@ -61,7 +61,9 @@ def caches_action(*actions)
filter_options = { :only => actions, :if => options.delete(:if), :unless => options.delete(:unless) }

cache_filter = ActionCacheFilter.new(:layout => options.delete(:layout), :cache_path => options.delete(:cache_path), :store_options => options)
around_filter(cache_filter, filter_options)
around_filter(filter_options) do |controller, action|
cache_filter.filter(controller, action)
end
end
end

Expand All @@ -83,6 +85,12 @@ def initialize(options, &block)
@options = options
end

def filter(controller, action)
should_continue = before(controller)
action.call if should_continue
after(controller)
end

def before(controller)
cache_path = ActionCachePath.new(controller, path_options_for(controller, @options.slice(:cache_path)))
if cache = controller.read_fragment(cache_path.path, @options[:store_options])
Expand Down
35 changes: 35 additions & 0 deletions actionpack/test/controller/caching_test.rb
@@ -1,5 +1,6 @@
require 'fileutils'
require 'abstract_unit'
require 'active_record_unit'

CACHE_DIR = 'test_cache'
# Don't change '/../temp/' cavalierly or you might hose something you don't want hosed
Expand Down Expand Up @@ -153,6 +154,7 @@ class ActionCachingTestController < ActionController::Base
caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" }
caches_action :with_layout
caches_action :layout_false, :layout => false
caches_action :record_not_found, :four_oh_four, :simple_runtime_error

layout 'talk_from_action.erb'

Expand All @@ -175,6 +177,18 @@ def with_layout
render :text => @cache_this, :layout => true
end

def record_not_found
raise ActiveRecord::RecordNotFound, "oops!"
end

def four_oh_four
render :text => "404'd!", :status => 404
end

def simple_runtime_error
raise "oops!"
end

alias_method :show, :index
alias_method :edit, :index
alias_method :destroy, :index
Expand Down Expand Up @@ -458,6 +472,27 @@ def test_file_extensions
assert_response :success
end

def test_record_not_found_returns_404_for_multiple_requests
get :record_not_found
assert_response 404
get :record_not_found
assert_response 404
end

def test_four_oh_four_returns_404_for_multiple_requests
get :four_oh_four
assert_response 404
get :four_oh_four
assert_response 404
end

def test_simple_runtime_error_returns_500_for_multiple_requests
get :simple_runtime_error
assert_response 500
get :simple_runtime_error
assert_response 500
end

private
def content_to_cache
assigns(:cache_this)
Expand Down

0 comments on commit e59835b

Please sign in to comment.