Consistent public/protected/private visibility for chained methods. Closes #7813.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6396 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

 *SVN*
 
+* Consistent public/protected/private visibility for chained methods.  #7813 [dcmanges]
+
 * Prefer MIME constants to strings.  #7707 [Dan Kubb]
 
 * Allow array and hash query parameters. Array route parameters are converted/to/a/path as before.  #6765, #7047, #7462 [bgipsy, Jeremy McAnally, Dan Kubb, brendan]

actionpack/lib/action_controller/benchmarking.rb

@@ -40,44 +40,44 @@ def silence
       end
     end
 
-    def render_with_benchmark(options = nil, deprecated_status = nil, &block)
-      unless logger
-        render_without_benchmark(options, deprecated_status, &block)
-      else
-        db_runtime = ActiveRecord::Base.connection.reset_runtime if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
+    protected
+      def render_with_benchmark(options = nil, deprecated_status = nil, &block)
+        unless logger
+          render_without_benchmark(options, deprecated_status, &block)
+        else
+          db_runtime = ActiveRecord::Base.connection.reset_runtime if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
 
-        render_output = nil
-        @rendering_runtime = Benchmark::measure{ render_output = render_without_benchmark(options, deprecated_status, &block) }.real
+          render_output = nil
+          @rendering_runtime = Benchmark::measure{ render_output = render_without_benchmark(options, deprecated_status, &block) }.real
 
-        if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
-          @db_rt_before_render = db_runtime
-          @db_rt_after_render = ActiveRecord::Base.connection.reset_runtime
-          @rendering_runtime -= @db_rt_after_render
-        end
+          if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
+            @db_rt_before_render = db_runtime
+            @db_rt_after_render = ActiveRecord::Base.connection.reset_runtime
+            @rendering_runtime -= @db_rt_after_render
+          end
 
-        render_output
-      end
-    end    
+          render_output
+        end
+      end    
 
-    def perform_action_with_benchmark
-      unless logger
-        perform_action_without_benchmark
-      else
-        runtime = [ Benchmark::measure{ perform_action_without_benchmark }.real, 0.0001 ].max
+    private
+      def perform_action_with_benchmark
+        unless logger
+          perform_action_without_benchmark
+        else
+          runtime = [ Benchmark::measure{ perform_action_without_benchmark }.real, 0.0001 ].max
 
-        log_message  = "Completed in #{sprintf("%.5f", runtime)} (#{(1 / runtime).floor} reqs/sec)"
-        log_message << rendering_runtime(runtime) if defined?(@rendering_runtime)
-        log_message << active_record_runtime(runtime) if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
-        log_message << " | #{headers["Status"]}"
-        log_message << " [#{complete_request_uri rescue "unknown"}]"
+          log_message  = "Completed in #{sprintf("%.5f", runtime)} (#{(1 / runtime).floor} reqs/sec)"
+          log_message << rendering_runtime(runtime) if defined?(@rendering_runtime)
+          log_message << active_record_runtime(runtime) if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected?
+          log_message << " | #{headers["Status"]}"
+          log_message << " [#{complete_request_uri rescue "unknown"}]"
 
-        logger.info(log_message)
-        response.headers["X-Runtime"] = sprintf("%.5f", runtime)
+          logger.info(log_message)
+          response.headers["X-Runtime"] = sprintf("%.5f", runtime)
+        end
       end
-    end
 
-
-    private
       def rendering_runtime(runtime)
         " | Rendering: #{sprintf("%.5f", @rendering_runtime)} (#{sprintf("%d", (@rendering_runtime * 100) / runtime)}%)"
       end

actionpack/lib/action_controller/filters.rb

@@ -615,15 +615,6 @@ def self.included(base)
         end
       end
 
-      def perform_action_with_filters
-        call_filter(self.class.filter_chain, 0)
-      end
-
-      def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
-        @before_filter_chain_aborted = false
-        process_without_filters(request, response, method, *arguments)
-      end
-
       def filter_chain
         self.class.filter_chain
       end
@@ -654,7 +645,18 @@ def halt_filter_chain(filter, reason)
         return false
       end
 
+      protected
+
+        def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
+          @before_filter_chain_aborted = false
+          process_without_filters(request, response, method, *arguments)
+        end
+
       private
+        def perform_action_with_filters
+          call_filter(self.class.filter_chain, 0)
+        end
+
         def process_cleanup_with_filters
           if @before_filter_chain_aborted
             close_session

actionpack/lib/action_controller/flash.rb

@@ -136,23 +136,14 @@ def use(k=nil, v=true)
     end
 
     module InstanceMethods #:nodoc:
-      def assign_shortcuts_with_flash(request, response) #:nodoc:
-        assign_shortcuts_without_flash(request, response)
-        flash(:refresh)
-      end
-      
-      def process_cleanup_with_flash
-        flash.sweep if @_session
-        process_cleanup_without_flash
-      end
 
-      def reset_session_with_flash
-        reset_session_without_flash
-        remove_instance_variable(:@_flash)
-        flash(:refresh)
-      end
+      protected
+        def reset_session_with_flash
+          reset_session_without_flash
+          remove_instance_variable(:@_flash)
+          flash(:refresh)
+        end
       
-      protected 
         # Access the contents of the flash. Use <tt>flash["notice"]</tt> to read a notice you put there or 
         # <tt>flash["notice"] = "hello"</tt> to put a new one.
         # Note that if sessions are disabled only flash.now will work.
@@ -177,6 +168,17 @@ def keep_flash #:doc:
           ActiveSupport::Deprecation.warn 'keep_flash is deprecated; use flash.keep instead.', caller
           flash.keep
         end
+        
+      private
+        def assign_shortcuts_with_flash(request, response) #:nodoc:
+          assign_shortcuts_without_flash(request, response)
+          flash(:refresh)
+        end
+    
+        def process_cleanup_with_flash
+          flash.sweep if @_session
+          process_cleanup_without_flash
+        end
     end
   end
 end

actionpack/lib/action_controller/layout.rb

@@ -233,31 +233,32 @@ def active_layout(passed_layout = nil)
       end
     end
 
-    def render_with_a_layout(options = nil, deprecated_status = nil, deprecated_layout = nil, &block) #:nodoc:
-      template_with_options = options.is_a?(Hash)
+    protected
+      def render_with_a_layout(options = nil, deprecated_status = nil, deprecated_layout = nil, &block) #:nodoc:
+        template_with_options = options.is_a?(Hash)
 
-      if apply_layout?(template_with_options, options) && (layout = pick_layout(template_with_options, options, deprecated_layout))
-        assert_existence_of_template_file(layout)
+        if apply_layout?(template_with_options, options) && (layout = pick_layout(template_with_options, options, deprecated_layout))
+          assert_existence_of_template_file(layout)
 
-        options = options.merge :layout => false if template_with_options
-        logger.info("Rendering template within #{layout}") if logger
+          options = options.merge :layout => false if template_with_options
+          logger.info("Rendering template within #{layout}") if logger
 
-        if template_with_options
-          content_for_layout = render_with_no_layout(options, &block)
-          deprecated_status = options[:status] || deprecated_status
+          if template_with_options
+            content_for_layout = render_with_no_layout(options, &block)
+            deprecated_status = options[:status] || deprecated_status
+          else
+            content_for_layout = render_with_no_layout(options, deprecated_status, &block)
+          end
+
+          erase_render_results
+          add_variables_to_assigns
+          @template.instance_variable_set("@content_for_layout", content_for_layout)
+          response.layout = layout
+          render_text(@template.render_file(layout, true), deprecated_status)
         else
-          content_for_layout = render_with_no_layout(options, deprecated_status, &block)
+          render_with_no_layout(options, deprecated_status, &block)
         end
-
-        erase_render_results
-        add_variables_to_assigns
-        @template.instance_variable_set("@content_for_layout", content_for_layout)
-        response.layout = layout
-        render_text(@template.render_file(layout, true), deprecated_status)
-      else
-        render_with_no_layout(options, deprecated_status, &block)
       end
-    end
 
 
     private

activerecord/CHANGELOG

 *SVN*
 
+* Consistent public/protected/private visibility for chained methods.  #7813 [dcmanges]
+
 * Oracle: fix quoted primary keys and datetime overflow.  #7798 [Michael Schoen]
 
 * Consistently quote primary key column names.  #7763 [toolmantim]

activerecord/lib/active_record/callbacks.rb

@@ -227,6 +227,7 @@ def initialize_with_callbacks(attributes = nil) #:nodoc:
       callback(:after_initialize) if respond_to_without_attributes?(:after_initialize)
       result
     end
+    private :initialize_with_callbacks
 
     # Is called _before_ Base.save (regardless of whether it's a create or update save).
     def before_save() end
@@ -243,6 +244,7 @@ def create_or_update_with_callbacks #:nodoc:
       callback(:after_save)
       result
     end
+    private :create_or_update_with_callbacks
 
     # Is called _before_ Base.save on new objects that haven't been saved yet (no record exists).
     def before_create() end
@@ -255,6 +257,7 @@ def create_with_callbacks #:nodoc:
       callback(:after_create)
       result
     end
+    private :create_with_callbacks
 
     # Is called _before_ Base.save on existing objects that have a record.
     def before_update() end
@@ -268,6 +271,7 @@ def update_with_callbacks #:nodoc:
       callback(:after_update)
       result
     end
+    private :update_with_callbacks
 
     # Is called _before_ Validations.validate (which is part of the Base.save call).
     def before_validation() end

activerecord/lib/active_record/locking/optimistic.rb

@@ -23,7 +23,6 @@ module Locking
     # This method uses the same syntax as <tt>set_table_name</tt>
     module Optimistic
       def self.included(base) #:nodoc:
-        super
         base.extend ClassMethods
 
         base.cattr_accessor :lock_optimistically, :instance_writer => false
@@ -41,41 +40,42 @@ def locking_enabled? #:nodoc:
         self.class.locking_enabled?
       end
 
-      def attributes_from_column_definition_with_lock
-        result = attributes_from_column_definition_without_lock
+      private
+        def attributes_from_column_definition_with_lock
+          result = attributes_from_column_definition_without_lock
         
-        # If the locking column has no default value set,
-        # start the lock version at zero.  Note we can't use
-        # locking_enabled? at this point as @attributes may
-        # not have been initialized yet
+          # If the locking column has no default value set,
+          # start the lock version at zero.  Note we can't use
+          # locking_enabled? at this point as @attributes may
+          # not have been initialized yet
         
-        if lock_optimistically && result.include?(self.class.locking_column)
-          result[self.class.locking_column] ||= 0
-        end
+          if lock_optimistically && result.include?(self.class.locking_column)
+            result[self.class.locking_column] ||= 0
+          end
         
-        return result
-      end
+          return result
+        end
 
-      def update_with_lock #:nodoc:
-        return update_without_lock unless locking_enabled?
+        def update_with_lock #:nodoc:
+          return update_without_lock unless locking_enabled?
 
-        lock_col = self.class.locking_column
-        previous_value = send(lock_col)
-        send(lock_col + '=', previous_value + 1)
+          lock_col = self.class.locking_column
+          previous_value = send(lock_col)
+          send(lock_col + '=', previous_value + 1)
 
-        affected_rows = connection.update(<<-end_sql, "#{self.class.name} Update with optimistic locking")
-          UPDATE #{self.class.table_name}
-          SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))}
-          WHERE #{self.class.primary_key} = #{quote_value(id)}
-          AND #{self.class.quoted_locking_column} = #{quote_value(previous_value)}
-        end_sql
+          affected_rows = connection.update(<<-end_sql, "#{self.class.name} Update with optimistic locking")
+            UPDATE #{self.class.table_name}
+            SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))}
+            WHERE #{self.class.primary_key} = #{quote_value(id)}
+            AND #{self.class.quoted_locking_column} = #{quote_value(previous_value)}
+          end_sql
 
-        unless affected_rows == 1
-          raise ActiveRecord::StaleObjectError, "Attempted to update a stale object"
-        end
+          unless affected_rows == 1
+            raise ActiveRecord::StaleObjectError, "Attempted to update a stale object"
+          end
 
-        return true
-      end
+          return true
+        end
 
       module ClassMethods
         DEFAULT_LOCKING_COLUMN = 'lock_version'

activerecord/lib/active_record/timestamp.rb

@@ -18,8 +18,6 @@ module ActiveRecord
   #   <tt>ActiveRecord::Base.default_timezone = :utc</tt>
   module Timestamp
     def self.included(base) #:nodoc:
-      super
-
       base.alias_method_chain :create, :timestamps
       base.alias_method_chain :update, :timestamps
 
@@ -27,25 +25,26 @@ def self.included(base) #:nodoc:
       base.record_timestamps = true
     end
 
-    def create_with_timestamps #:nodoc:
-      if record_timestamps
-        t = self.class.default_timezone == :utc ? Time.now.utc : Time.now
-        write_attribute('created_at', t) if respond_to?(:created_at) && created_at.nil?
-        write_attribute('created_on', t) if respond_to?(:created_on) && created_on.nil?
+    private
+      def create_with_timestamps #:nodoc:
+        if record_timestamps
+          t = self.class.default_timezone == :utc ? Time.now.utc : Time.now
+          write_attribute('created_at', t) if respond_to?(:created_at) && created_at.nil?
+          write_attribute('created_on', t) if respond_to?(:created_on) && created_on.nil?
 
-        write_attribute('updated_at', t) if respond_to?(:updated_at)
-        write_attribute('updated_on', t) if respond_to?(:updated_on)
+          write_attribute('updated_at', t) if respond_to?(:updated_at)
+          write_attribute('updated_on', t) if respond_to?(:updated_on)
+        end
+        create_without_timestamps
       end
-      create_without_timestamps
-    end
 
-    def update_with_timestamps #:nodoc:
-      if record_timestamps
-        t = self.class.default_timezone == :utc ? Time.now.utc : Time.now
-        write_attribute('updated_at', t) if respond_to?(:updated_at)
-        write_attribute('updated_on', t) if respond_to?(:updated_on)
+      def update_with_timestamps #:nodoc:
+        if record_timestamps
+          t = self.class.default_timezone == :utc ? Time.now.utc : Time.now
+          write_attribute('updated_at', t) if respond_to?(:updated_at)
+          write_attribute('updated_on', t) if respond_to?(:updated_on)
+        end
+        update_without_timestamps
       end
-      update_without_timestamps
-    end
   end
 end