diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 8298a199d8a0f..6b2373d0a2987 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,21 @@ +* Expand `ActionController::ConditionalGet#fresh_when` and `stale?` to also + accept a collection of records as the first argument, so that the + following code can be written in a shorter form. + + # Before + def index + @article = Article.all + fresh_when(etag: @articles, last_modified: @articles.maximum(:created_at)) + end + + # After + def index + @article = Article.all + fresh_when(@articles) + end + + *claudiob* + * Explicitly ignored wildcard verbs when searching for HEAD routes before fallback Fixes an issue where a mounted rack app at root would intercept the HEAD diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 3a5929adef9e1..28f0b6e34967c 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -57,15 +57,25 @@ def etag(&etagger) # This will render the show template if the request isn't sending a matching ETag or # If-Modified-Since header and just a 304 Not Modified response if there's a match. # - # You can also just pass a record where +last_modified+ will be set by calling - # +updated_at+ and the +etag+ by passing the object itself. + # You can also just pass a record. In this case +last_modified+ will be set + # by calling +updated_at+ and +etag+ by passing the object itself. # # def show # @article = Article.find(params[:id]) # fresh_when(@article) # end # - # When passing a record, you can still set whether the public header: + # You can also pass an object that responds to +maximum+, such as a + # collection of active records. In this case +last_modified+ will be set by + # calling +maximum(:updated_at)+ on the collection (the timestamp of the + # most recently updated record) and the +etag+ by passing the object itself. + # + # def index + # @articles = Article.all + # fresh_when(@articles) + # end + # + # When passing a record or a collection, you can still set the public header: # # def show # @article = Article.find(params[:id]) @@ -77,8 +87,8 @@ def etag(&etagger) # # before_action { fresh_when @article, template: 'widgets/show' } # - def fresh_when(record = nil, etag: record, last_modified: nil, public: false, template: nil) - last_modified ||= record.try(:updated_at) + def fresh_when(object = nil, etag: object, last_modified: nil, public: false, template: nil) + last_modified ||= object.try(:updated_at) || object.try(:maximum, :updated_at) if etag || template response.etag = combine_etags(etag: etag, last_modified: last_modified, @@ -121,8 +131,8 @@ def fresh_when(record = nil, etag: record, last_modified: nil, public: false, te # end # end # - # You can also just pass a record where +last_modified+ will be set by calling - # +updated_at+ and the +etag+ by passing the object itself. + # You can also just pass a record. In this case +last_modified+ will be set + # by calling +updated_at+ and +etag+ by passing the object itself. # # def show # @article = Article.find(params[:id]) @@ -135,7 +145,23 @@ def fresh_when(record = nil, etag: record, last_modified: nil, public: false, te # end # end # - # When passing a record, you can still set whether the public header: + # You can also pass an object that responds to +maximum+, such as a + # collection of active records. In this case +last_modified+ will be set by + # calling +maximum(:updated_at)+ on the collection (the timestamp of the + # most recently updated record) and the +etag+ by passing the object itself. + # + # def index + # @articles = Article.all + # + # if stale?(@articles) + # @statistics = @articles.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + # + # When passing a record or a collection, you can still set the public header: # # def show # @article = Article.find(params[:id]) @@ -155,8 +181,8 @@ def fresh_when(record = nil, etag: record, last_modified: nil, public: false, te # super if stale? @article, template: 'widgets/show' # end # - def stale?(record = nil, etag: record, last_modified: nil, public: nil, template: nil) - fresh_when(record, etag: etag, last_modified: last_modified, public: public, template: template) + def stale?(object = nil, etag: object, last_modified: nil, public: nil, template: nil) + fresh_when(object, etag: etag, last_modified: last_modified, public: public, template: template) !request.fresh?(response) end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index a5dd96ae9130a..8d6b62f2bf8a8 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -58,6 +58,27 @@ def conditional_hello_with_record end end + class Collection + def initialize(records) + @records = records + end + + def maximum(attribute) + @records.max_by(&attribute).public_send(attribute) + end + end + + def conditional_hello_with_collection_of_records + ts = Time.now.utc.beginning_of_day + + record = Struct.new(:updated_at, :cache_key).new(ts, "foo/123") + old_record = Struct.new(:updated_at, :cache_key).new(ts - 1.day, "bar/123") + + if stale?(Collection.new([record, old_record])) + render action: 'hello_world' + end + end + def conditional_hello_with_expires_in expires_in 60.1.seconds render :action => 'hello_world' @@ -288,7 +309,6 @@ def test_request_modified assert_equal @last_modified, @response.headers['Last-Modified'] end - def test_responds_with_last_modified_with_record get :conditional_hello_with_record assert_equal @last_modified, @response.headers['Last-Modified'] @@ -318,6 +338,34 @@ def test_request_modified_with_record assert_equal @last_modified, @response.headers['Last-Modified'] end + def test_responds_with_last_modified_with_collection_of_records + get :conditional_hello_with_collection_of_records + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_with_collection_of_records + @request.if_modified_since = @last_modified + get :conditional_hello_with_collection_of_records + assert_equal 304, @response.status.to_i + assert @response.body.blank? + assert_equal @last_modified, @response.headers['Last-Modified'] + end + + def test_request_not_modified_but_etag_differs_with_collection_of_records + @request.if_modified_since = @last_modified + @request.if_none_match = "234" + get :conditional_hello_with_collection_of_records + assert_response :success + end + + def test_request_modified_with_collection_of_records + @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT' + get :conditional_hello_with_collection_of_records + assert_equal 200, @response.status.to_i + assert @response.body.present? + assert_equal @last_modified, @response.headers['Last-Modified'] + end + def test_request_with_bang_gets_last_modified get :conditional_hello_with_bangs assert_equal @last_modified, @response.headers['Last-Modified']