From 8def6881798bbb68a75a125d7c6c407ef38a1fa1 Mon Sep 17 00:00:00 2001 From: Simon Jesenko Date: Fri, 18 Jul 2014 09:46:49 +0200 Subject: [PATCH 1/2] Deprecate usage of sweepers as controller filters and execute ActiveRecord observers only when in the context of controller/action specified by cache_sweeper method --- .../action_controller/caching/sweeper.rb | 19 +++++++++++-- test/sweeper_test.rb | 27 ++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/rails/observers/action_controller/caching/sweeper.rb b/lib/rails/observers/action_controller/caching/sweeper.rb index f6bf1ba..2fcb758 100644 --- a/lib/rails/observers/action_controller/caching/sweeper.rb +++ b/lib/rails/observers/action_controller/caching/sweeper.rb @@ -27,6 +27,14 @@ def around(controller) clean_up end + # Execute standard observer logic only if controller was initiated, i.e. + # it is being triggered from the controller/action as specified in + # controller's cache_sweeper class method. + def update(observed_method, object, *extra_args, &block) #:nodoc: + return unless controller + super + end + protected # gets the action cache path for the given options. def action_path_for(options) @@ -48,8 +56,15 @@ def callback(timing) controller_callback_method_name = "#{timing}_#{controller.controller_name.underscore}" action_callback_method_name = "#{controller_callback_method_name}_#{controller.action_name}" - __send__(controller_callback_method_name) if respond_to?(controller_callback_method_name, true) - __send__(action_callback_method_name) if respond_to?(action_callback_method_name, true) + if respond_to?(controller_callback_method_name, true) + warn "[DEPRECATION] Using Sweeper for controller callback is deprecated. Sweeper is just an observer of ActiveRecord object that gets triggered when specified controller action is executed" + __send__(controller_callback_method_name) + end + + if respond_to?(action_callback_method_name, true) + warn "[DEPRECATION] Using Sweeper for controller callback is deprecated. Sweeper is just an observer of ActiveRecord object that gets triggered when specified controller action is executed" + __send__(action_callback_method_name) + end end def method_missing(method, *arguments, &block) diff --git a/test/sweeper_test.rb b/test/sweeper_test.rb index ed9ad61..fef7614 100644 --- a/test/sweeper_test.rb +++ b/test/sweeper_test.rb @@ -6,7 +6,13 @@ SharedTestRoutes = ActionDispatch::Routing::RouteSet.new -class AppSweeper < ActionController::Caching::Sweeper; end +class AppSweeper < ActionController::Caching::Sweeper + + def around_save(record) + yield :in_around_save + end + +end class SweeperTestController < ActionController::Base include SharedTestRoutes.url_helpers @@ -40,6 +46,25 @@ def test_sweeper_should_not_ignore_no_method_error end end + def test_sweeper_should_not_respond_to_update_if_controller_is_not_set + sweeper = AppSweeper.send(:new) + yielded_value = nil + sweeper.update(:around_save, Object.new) do |val| + yielded_value = val + end + assert_nil yielded_value + end + + def test_sweeper_should_respond_to_update_if_controller_is_set + sweeper = AppSweeper.send(:new) + sweeper.controller = SweeperTestController.new + yielded_value = nil + sweeper.update(:around_save, Object.new) do |val| + yielded_value = val + end + assert_equal :in_around_save, yielded_value + end + def test_sweeper_should_not_block_rendering response = test_process(SweeperTestController) assert_equal 'hello world', response.body From d342b617cccb0d58fc104c6fd1a4f7fdb2ced620 Mon Sep 17 00:00:00 2001 From: Simon Jesenko Date: Fri, 18 Jul 2014 10:03:25 +0200 Subject: [PATCH 2/2] Update documentation for CacheSweeper --- lib/rails/observers/action_controller/caching/sweeping.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/rails/observers/action_controller/caching/sweeping.rb b/lib/rails/observers/action_controller/caching/sweeping.rb index 5d5e31c..326c427 100644 --- a/lib/rails/observers/action_controller/caching/sweeping.rb +++ b/lib/rails/observers/action_controller/caching/sweeping.rb @@ -1,7 +1,9 @@ module ActionController #:nodoc: module Caching # Sweepers are the terminators of the caching world and responsible for expiring caches when model objects change. - # They do this by being half-observers, half-filters and implementing callbacks for both roles. A Sweeper example: + # They do this by being observers of model that are executed only when specified controller action is being executed. + # Sweeper can access the corresponding controller via controller accessor. Additionally, missing method calls are automatically routed to the corresponding controller. + # A Sweeper example: # # class ListSweeper < ActionController::Caching::Sweeper # observe List, Item @@ -21,7 +23,7 @@ module Caching # cache_sweeper :list_sweeper, :only => [ :edit, :destroy, :share ] # end # - # In the example above, four actions are cached and three actions are responsible for expiring those caches. + # In the example above, four actions are cached and three actions are responsible for expiring those caches, i.e. ListSweeper.after_save will only be executed in the context of those actions. # # You can also name an explicit class in the declaration of a sweeper, which is needed if the sweeper is in a module: #