Writing comments to AbstractController

4 files changed, 148 insertions, 29 deletions

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