From 38b608ecab2441cd0c4e75bc08bdf57fcf85dd71 Mon Sep 17 00:00:00 2001
From: Yehuda Katz + Carl Lerche <ykatz+clerche@engineyard.com>
Date: Mon, 8 Jun 2009 16:14:38 -0700
Subject: Writing comments to AbstractController
---
actionpack/lib/action_controller/abstract/base.rb | 105 +++++++++++++++++----
.../lib/action_controller/abstract/benchmarker.rb | 10 ++
.../lib/action_controller/abstract/callbacks.rb | 60 ++++++++++--
actionpack/lib/action_controller/new_base/base.rb | 2 +-
.../abstract_controller_test.rb | 24 ++---
.../test/abstract_controller/callbacks_test.rb | 30 +++---
actionpack/test/abstract_controller/helper_test.rb | 2 +-
.../test/abstract_controller/layouts_test.rb | 30 +++---
8 files changed, 191 insertions(+), 72 deletions(-)
(limited to 'actionpack')
diff --git a/actionpack/lib/action_controller/abstract/base.rb b/actionpack/lib/action_controller/abstract/base.rb
index 306f3d2ccb..d2cf4b1aad 100644
--- a/actionpack/lib/action_controller/abstract/base.rb
+++ b/actionpack/lib/action_controller/abstract/base.rb
@@ -17,36 +17,58 @@ module AbstractController
class << self
attr_reader :abstract
+ alias_method :abstract?, :abstract
+ # Define a controller as abstract. See internal_methods for more
+ # details.
def abstract!
@abstract = true
end
- alias_method :abstract?, :abstract
-
def inherited(klass)
- ::AbstractController::Base.subclasses << klass.to_s
+ ::AbstractController::Base.descendants << klass.to_s
super
end
- def subclasses
- @subclasses ||= []
+ # A list of all descendents of AbstractController::Base. This is
+ # useful for initializers which need to add behavior to all controllers.
+ def descendants
+ @descendants ||= []
end
+ # A list of all internal methods for a controller. This finds the first
+ # abstract superclass of a controller, and gets a list of all public
+ # instance methods on that abstract class. Public instance methods of
+ # a controller would normally be considered action methods, so we
+ # are removing those methods on classes declared as abstract
+ # (ActionController::Http and ActionController::Base are defined
+ # as abstract)
def internal_methods
controller = self
controller = controller.superclass until controller.abstract?
controller.public_instance_methods(true)
end
- def process(action)
- new.process(action.to_s)
- end
-
+ # The list of hidden actions to an empty Array. Defaults to an
+ # empty Array. This can be modified by other modules or subclasses
+ # to specify particular actions as hidden.
+ #
+ # ==== Returns
+ # Array[String]:: An array of method names that should not be
+ # considered actions.
def hidden_actions
[]
end
+ # A list of method names that should be considered actions. This
+ # includes all public instance methods on a controller, less
+ # any internal methods (see #internal_methods), adding back in
+ # any methods that are internal, but still exist on the class
+ # itself. Finally, #hidden_actions are removed.
+ #
+ # ==== Returns
+ # Array[String]:: A list of all methods that should be considered
+ # actions.
def action_methods
@action_methods ||=
# All public instance methods of this class, including ancestors
@@ -62,6 +84,14 @@ module AbstractController
abstract!
+ # Calls the action going through the entire action dispatch stack.
+ #
+ # The actual method that is called is determined by calling
+ # #method_for_action. If no method can handle the action, then an
+ # ActionNotFound error is raised.
+ #
+ # ==== Returns
+ # self
def process(action)
@_action_name = action_name = action.to_s
@@ -74,33 +104,68 @@ module AbstractController
end
private
+ # See AbstractController::Base.action_methods
def action_methods
self.class.action_methods
end
- def action_method?(action)
- action_methods.include?(action)
+ # Returns true if the name can be considered an action. This can
+ # be overridden in subclasses to modify the semantics of what
+ # can be considered an action.
+ #
+ # ==== Parameters
+ # name<String>:: The name of an action to be tested
+ #
+ # ==== Returns
+ # TrueClass, FalseClass
+ def action_method?(name)
+ action_methods.include?(name)
end
- # It is possible for respond_to?(action_name) to be false and
- # respond_to?(:action_missing) to be false if respond_to_action?
- # is overridden in a subclass. For instance, ActionController::Base
- # overrides it to include the case where a template matching the
- # action_name is found.
+ # Call the action. Override this in a subclass to modify the
+ # behavior around processing an action. This, and not #process,
+ # is the intended way to override action dispatching.
def process_action(method_name)
send_action(method_name)
end
+ # Actually call the method associated with the action. Override
+ # this method if you wish to change how action methods are called,
+ # not to add additional behavior around it. For example, you would
+ # override #send_action if you want to inject arguments into the
+ # method.
alias send_action send
+ # If the action name was not found, but a method called "action_missing"
+ # was found, #method_for_action will return "_handle_action_missing".
+ # This method calls #action_missing with the current action name.
def _handle_action_missing
action_missing(@_action_name)
end
- # Override this to change the conditions that will raise an
- # ActionNotFound error. If you accept a difference case,
- # you must handle it by also overriding process_action and
- # handling the case.
+ # Takes an action name and returns the name of the method that will
+ # handle the action. In normal cases, this method returns the same
+ # name as it receives. By default, if #method_for_action receives
+ # a name that is not an action, it will look for an #action_missing
+ # method and return "_handle_action_missing" if one is found.
+ #
+ # Subclasses may override this method to add additional conditions
+ # that should be considered an action. For instance, an HTTP controller
+ # with a template matching the action name is considered to exist.
+ #
+ # If you override this method to handle additional cases, you may
+ # also provide a method (like _handle_method_missing) to handle
+ # the case.
+ #
+ # If none of these conditions are true, and method_for_action
+ # returns nil, an ActionNotFound exception will be raised.
+ #
+ # ==== Parameters
+ # action_name<String>:: An action name to find a method name for
+ #
+ # ==== Returns
+ # String:: The name of the method that handles the action
+ # nil:: No method name could be found. Raise ActionNotFound.
def method_for_action(action_name)
if action_method?(action_name) then action_name
elsif respond_to?(:action_missing, true) then "_handle_action_missing"
diff --git a/actionpack/lib/action_controller/abstract/benchmarker.rb b/actionpack/lib/action_controller/abstract/benchmarker.rb
index 07294cede3..58e9564c2f 100644
--- a/actionpack/lib/action_controller/abstract/benchmarker.rb
+++ b/actionpack/lib/action_controller/abstract/benchmarker.rb
@@ -5,6 +5,16 @@ module AbstractController
include Logger
module ClassMethods
+ # Execute the passed in block, timing the duration of the block in ms.
+ #
+ # ==== Parameters
+ # title<#to_s>:: The title of block to benchmark
+ # log_level<Integer>:: A valid log level. Defaults to Logger::DEBUG
+ # use_silence<TrueClass, FalseClass>:: Whether or not to silence the
+ # logger for the duration of the block.
+ #
+ # ==== Returns
+ # Object:: The result of the block
def benchmark(title, log_level = ::Logger::DEBUG, use_silence = true)
if logger && logger.level >= log_level
result = nil
diff --git a/actionpack/lib/action_controller/abstract/callbacks.rb b/actionpack/lib/action_controller/abstract/callbacks.rb
index fd7e5ebfda..0d5161c80e 100644
--- a/actionpack/lib/action_controller/abstract/callbacks.rb
+++ b/actionpack/lib/action_controller/abstract/callbacks.rb
@@ -2,12 +2,17 @@ module AbstractController
module Callbacks
extend ActiveSupport::Concern
+ # Uses ActiveSupport::NewCallbacks as the base functionality. For
+ # more details on the whole callback system, read the documentation
+ # for ActiveSupport::NewCallbacks.
include ActiveSupport::NewCallbacks
included do
define_callbacks :process_action, "response_body"
end
+ # Override AbstractController::Base's process_action to run the
+ # process_action callbacks around the normal behavior.
def process_action(method_name)
_run_process_action_callbacks(method_name) do
super
@@ -15,6 +20,17 @@ module AbstractController
end
module ClassMethods
+ # If :only or :accept are used, convert the options into the
+ # primitive form (:per_key) used by ActiveSupport::Callbacks.
+ # The basic idea is that :only => :index gets converted to
+ # :if => proc {|c| c.action_name == "index" }, but that the
+ # proc is only evaluated once per action for the lifetime of
+ # a Rails process.
+ #
+ # ==== Options
+ # :only<#to_s>:: The callback should be run only for this action
+ # :except<#to_s>:: The callback should be run for all actions
+ # except this action
def _normalize_callback_options(options)
if only = options[:only]
only = Array(only).map {|o| "action_name == '#{o}'"}.join(" || ")
@@ -26,41 +42,69 @@ module AbstractController
end
end
+ # Skip before, after, and around filters matching any of the names
+ #
+ # ==== Parameters
+ # *names<Object>:: A list of valid names that could be used for
+ # callbacks. Note that skipping uses Ruby equality, so it's
+ # impossible to skip a callback defined using an anonymous proc
+ # using #skip_filter
def skip_filter(*names, &blk)
- skip_before_filter(*names, &blk)
- skip_after_filter(*names, &blk)
- skip_around_filter(*names, &blk)
+ skip_before_filter(*names)
+ skip_after_filter(*names)
+ skip_around_filter(*names)
end
- def _insert_callbacks(names, block)
- options = names.last.is_a?(Hash) ? names.pop : {}
+ # Take callback names and an optional callback proc, normalize them,
+ # then call the block with each callback. This allows us to abstract
+ # the normalization across several methods that use it.
+ #
+ # ==== Parameters
+ # callbacks<Array[*Object, Hash]>:: A list of callbacks, with an optional
+ # options hash as the last parameter.
+ # block<Proc>:: A proc that should be added to the callbacks.
+ #
+ # ==== Block Parameters
+ # name<Symbol>:: The callback to be added
+ # options<Hash>:: A list of options to be used when adding the callback
+ def _insert_callbacks(callbacks, block)
+ options = callbacks.last.is_a?(Hash) ? callbacks.pop : {}
_normalize_callback_options(options)
- names.push(block) if block
- names.each do |name|
- yield name, options
+ callbacks.push(block) if block
+ callbacks.each do |callback|
+ yield callback, options
end
end
+ # set up before_filter, prepend_before_filter, skip_before_filter, etc.
+ # for each of before, after, and around.
[:before, :after, :around].each do |filter|
class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
+ # Append a before, after or around filter. See _insert_callbacks
+ # for details on the allowed parameters.
def #{filter}_filter(*names, &blk)
_insert_callbacks(names, blk) do |name, options|
set_callback(:process_action, :#{filter}, name, options)
end
end
+ # Prepend a before, after or around filter. See _insert_callbacks
+ # for details on the allowed parameters.
def prepend_#{filter}_filter(*names, &blk)
_insert_callbacks(names, blk) do |name, options|
set_callback(:process_action, :#{filter}, name, options.merge(:prepend => true))
end
end
+ # Skip a before, after or around filter. See _insert_callbacks
+ # for details on the allowed parameters.
def skip_#{filter}_filter(*names, &blk)
_insert_callbacks(names, blk) do |name, options|
skip_callback(:process_action, :#{filter}, name, options)
end
end
+ # *_filter is the same as append_*_filter
alias_method :append_#{filter}_filter, :#{filter}_filter
RUBY_EVAL
end
diff --git a/actionpack/lib/action_controller/new_base/base.rb b/actionpack/lib/action_controller/new_base/base.rb
index d7b65d37fa..e8fc153578 100644
--- a/actionpack/lib/action_controller/new_base/base.rb
+++ b/actionpack/lib/action_controller/new_base/base.rb
@@ -39,7 +39,7 @@ module ActionController
# TODO: Extract into its own module
# This should be moved together with other normalizing behavior
module ImplicitRender
- def send_action(method_name)
+ def send_action(*)
ret = super
default_render unless performed?
ret
diff --git a/actionpack/test/abstract_controller/abstract_controller_test.rb b/actionpack/test/abstract_controller/abstract_controller_test.rb
index 9c028e7d1e..c7eaaeb4ba 100644
--- a/actionpack/test/abstract_controller/abstract_controller_test.rb
+++ b/actionpack/test/abstract_controller/abstract_controller_test.rb
@@ -19,7 +19,7 @@ module AbstractController
class TestBasic < ActiveSupport::TestCase
test "dispatching works" do
- result = Me.process(:index)
+ result = Me.new.process(:index)
assert_equal "Hello world", result.response_body
end
end
@@ -68,27 +68,27 @@ module AbstractController
class TestRenderer < ActiveSupport::TestCase
test "rendering templates works" do
- result = Me2.process(:index)
+ result = Me2.new.process(:index)
assert_equal "Hello from index.erb", result.response_body
end
test "rendering passes ivars to the view" do
- result = Me2.process(:action_with_ivars)
+ result = Me2.new.process(:action_with_ivars)
assert_equal "Hello from index_with_ivars.erb", result.response_body
end
test "rendering with no template name" do
- result = Me2.process(:naked_render)
+ result = Me2.new.process(:naked_render)
assert_equal "Hello from naked_render.erb", result.response_body
end
test "rendering to a rack body" do
- result = Me2.process(:rendering_to_body)
+ result = Me2.new.process(:rendering_to_body)
assert_equal "Hello from naked_render.erb", result.response_body
end
test "rendering to a string" do
- result = Me2.process(:rendering_to_string)
+ result = Me2.new.process(:rendering_to_string)
assert_equal "Hello from naked_render.erb", result.response_body
end
end
@@ -120,12 +120,12 @@ module AbstractController
class TestPrefixedViews < ActiveSupport::TestCase
test "templates are located inside their 'prefix' folder" do
- result = Me3.process(:index)
+ result = Me3.new.process(:index)
assert_equal "Hello from me3/index.erb", result.response_body
end
test "templates included their format" do
- result = Me3.process(:formatted)
+ result = Me3.new.process(:formatted)
assert_equal "Hello from me3/formatted.html.erb", result.response_body
end
end
@@ -173,7 +173,7 @@ module AbstractController
class TestLayouts < ActiveSupport::TestCase
test "layouts are included" do
- result = Me4.process(:index)
+ result = Me4.new.process(:index)
assert_equal "Me4 Enter : Hello from me4/index.erb : Exit", result.response_body
end
end
@@ -210,7 +210,7 @@ module AbstractController
class TestRespondToAction < ActiveSupport::TestCase
def assert_dispatch(klass, body = "success", action = :index)
- response = klass.process(action).response_body
+ response = klass.new.process(action).response_body
assert_equal body, response
end
@@ -219,7 +219,7 @@ module AbstractController
end
test "raises ActionNotFound when method does not exist and action_missing is not defined" do
- assert_raise(ActionNotFound) { DefaultRespondToActionController.process(:fail) }
+ assert_raise(ActionNotFound) { DefaultRespondToActionController.new.process(:fail) }
end
test "dispatches to action_missing when method does not exist and action_missing is defined" do
@@ -231,7 +231,7 @@ module AbstractController
end
test "raises ActionNotFound if method is defined but respond_to_action? returns false" do
- assert_raise(ActionNotFound) { RespondToActionController.process(:fail) }
+ assert_raise(ActionNotFound) { RespondToActionController.new.process(:fail) }
end
end
diff --git a/actionpack/test/abstract_controller/callbacks_test.rb b/actionpack/test/abstract_controller/callbacks_test.rb
index 32450257f0..817f60f7d1 100644
--- a/actionpack/test/abstract_controller/callbacks_test.rb
+++ b/actionpack/test/abstract_controller/callbacks_test.rb
@@ -21,7 +21,7 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "basic callbacks work" do
- result = Callback1.process(:index)
+ result = Callback1.new.process(:index)
assert_equal "Hello world", result.response_body
end
end
@@ -52,17 +52,17 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "before_filter works" do
- result = Callback2.process(:index)
+ result = Callback2.new.process(:index)
assert_equal "Hello world", result.response_body
end
test "after_filter works" do
- result = Callback2.process(:index)
+ result = Callback2.new.process(:index)
assert_equal "Goodbye", result.instance_variable_get("@second")
end
test "around_filter works" do
- result = Callback2.process(:index)
+ result = Callback2.new.process(:index)
assert_equal "FIRSTSECOND", result.instance_variable_get("@aroundz")
end
end
@@ -83,12 +83,12 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "before_filter works with procs" do
- result = Callback3.process(:index)
+ result = Callback3.new.process(:index)
assert_equal "Hello world", result.response_body
end
test "after_filter works with procs" do
- result = Callback3.process(:index)
+ result = Callback3.new.process(:index)
assert_equal "Goodbye", result.instance_variable_get("@second")
end
end
@@ -118,17 +118,17 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "when :only is specified, a before filter is triggered on that action" do
- result = CallbacksWithConditions.process(:index)
+ result = CallbacksWithConditions.new.process(:index)
assert_equal "Hello, World", result.response_body
end
test "when :only is specified, a before filter is not triggered on other actions" do
- result = CallbacksWithConditions.process(:sekrit_data)
+ result = CallbacksWithConditions.new.process(:sekrit_data)
assert_equal "true", result.response_body
end
test "when :except is specified, an after filter is not triggered on that action" do
- result = CallbacksWithConditions.process(:index)
+ result = CallbacksWithConditions.new.process(:index)
assert_nil result.instance_variable_get("@authenticated")
end
end
@@ -158,17 +158,17 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "when :only is specified with an array, a before filter is triggered on that action" do
- result = CallbacksWithArrayConditions.process(:index)
+ result = CallbacksWithArrayConditions.new.process(:index)
assert_equal "Hello, World", result.response_body
end
test "when :only is specified with an array, a before filter is not triggered on other actions" do
- result = CallbacksWithArrayConditions.process(:sekrit_data)
+ result = CallbacksWithArrayConditions.new.process(:sekrit_data)
assert_equal "true", result.response_body
end
test "when :except is specified with an array, an after filter is not triggered on that action" do
- result = CallbacksWithArrayConditions.process(:index)
+ result = CallbacksWithArrayConditions.new.process(:index)
assert_nil result.instance_variable_get("@authenticated")
end
end
@@ -183,12 +183,12 @@ module AbstractController
class TestCallbacks < ActiveSupport::TestCase
test "when a callback is modified in a child with :only, it works for the :only action" do
- result = ChangedConditions.process(:index)
+ result = ChangedConditions.new.process(:index)
assert_equal "Hello world", result.response_body
end
test "when a callback is modified in a child with :only, it does not work for other actions" do
- result = ChangedConditions.process(:not_index)
+ result = ChangedConditions.new.process(:not_index)
assert_equal "", result.response_body
end
end
@@ -207,7 +207,7 @@ module AbstractController
class TestHalting < ActiveSupport::TestCase
test "when a callback sets the response body, the action should not be invoked" do
- result = SetsResponseBody.process(:index)
+ result = SetsResponseBody.new.process(:index)
assert_equal "Success", result.response_body
end
end
diff --git a/actionpack/test/abstract_controller/helper_test.rb b/actionpack/test/abstract_controller/helper_test.rb
index f91aefe606..0a2535f834 100644
--- a/actionpack/test/abstract_controller/helper_test.rb
+++ b/actionpack/test/abstract_controller/helper_test.rb
@@ -34,7 +34,7 @@ module AbstractController
class TestHelpers < ActiveSupport::TestCase
def test_helpers
- result = MyHelpers1.process(:index)
+ result = MyHelpers1.new.process(:index)
assert_equal "Hello World : Included", result.response_body
end
end
diff --git a/actionpack/test/abstract_controller/layouts_test.rb b/actionpack/test/abstract_controller/layouts_test.rb
index d3440c3de0..4b66f063f3 100644
--- a/actionpack/test/abstract_controller/layouts_test.rb
+++ b/actionpack/test/abstract_controller/layouts_test.rb
@@ -142,7 +142,7 @@ module AbstractControllerTests
end
# TODO Move to bootloader
- AbstractController::Base.subclasses.each do |klass|
+ AbstractController::Base.descendants.each do |klass|
klass = klass.constantize
next unless klass < AbstractController::Layouts
klass.class_eval do
@@ -152,70 +152,70 @@ module AbstractControllerTests
class TestBase < ActiveSupport::TestCase
test "when no layout is specified, and no default is available, render without a layout" do
- result = Blank.process(:index)
+ result = Blank.new.process(:index)
assert_equal "Hello blank!", result.response_body
end
test "when layout is specified as a string, render with that layout" do
- result = WithString.process(:index)
+ result = WithString.new.process(:index)
assert_equal "With String Hello string!", result.response_body
end
test "when layout is specified as a string, but the layout is missing, raise an exception" do
- assert_raises(ActionView::MissingTemplate) { WithMissingLayout.process(:index) }
+ assert_raises(ActionView::MissingTemplate) { WithMissingLayout.new.process(:index) }
end
test "when layout is specified as false, do not use a layout" do
- result = WithFalseLayout.process(:index)
+ result = WithFalseLayout.new.process(:index)
assert_equal "Hello false!", result.response_body
end
test "when layout is specified as nil, do not use a layout" do
- result = WithNilLayout.process(:index)
+ result = WithNilLayout.new.process(:index)
assert_equal "Hello nil!", result.response_body
end
test "when layout is specified as a symbol, call the requested method and use the layout returned" do
- result = WithSymbol.process(:index)
+ result = WithSymbol.new.process(:index)
assert_equal "OMGHI2U Hello symbol!", result.response_body
end
test "when layout is specified as a symbol and the method returns nil, don't use a layout" do
- result = WithSymbolReturningNil.process(:index)
+ result = WithSymbolReturningNil.new.process(:index)
assert_equal "Hello nilz!", result.response_body
end
test "when the layout is specified as a symbol and the method doesn't exist, raise an exception" do
- assert_raises(NoMethodError, /:nilz/) { WithSymbolAndNoMethod.process(:index) }
+ assert_raises(NoMethodError, /:nilz/) { WithSymbolAndNoMethod.new.process(:index) }
end
test "when the layout is specified as a symbol and the method returns something besides a string/false/nil, raise an exception" do
- assert_raises(ArgumentError) { WithSymbolReturningObj.process(:index) }
+ assert_raises(ArgumentError) { WithSymbolReturningObj.new.process(:index) }
end
test "when a child controller does not have a layout, use the parent controller layout" do
- result = WithStringChild.process(:index)
+ result = WithStringChild.new.process(:index)
assert_equal "With String Hello string!", result.response_body
end
test "when a child controller has specified a layout, use that layout and not the parent controller layout" do
- result = WithStringOverriddenChild.process(:index)
+ result = WithStringOverriddenChild.new.process(:index)
assert_equal "With Override Hello string!", result.response_body
end
test "when a child controller has an implied layout, use that layout and not the parent controller layout" do
- result = WithStringImpliedChild.process(:index)
+ result = WithStringImpliedChild.new.process(:index)
assert_equal "With Implied Hello string!", result.response_body
end
test "when a child controller specifies layout nil, do not use the parent layout" do
- result = WithNilChild.process(:index)
+ result = WithNilChild.new.process(:index)
assert_equal "Hello string!", result.response_body
end
test "when a grandchild has no layout specified, the child has an implied layout, and the " \
"parent has specified a layout, use the child controller layout" do
- result = WithChildOfImplied.process(:index)
+ result = WithChildOfImplied.new.process(:index)
assert_equal "With Implied Hello string!", result.response_body
end
--
cgit v1.2.3