From 07d8f46b85b08187cabd477cc3ca37f2c818e9aa Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 13 Mar 2007 02:14:31 +0000 Subject: 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 | 2 + actionpack/lib/action_controller/benchmarking.rb | 60 +++++++++++----------- actionpack/lib/action_controller/filters.rb | 20 ++++---- actionpack/lib/action_controller/flash.rb | 32 ++++++------ actionpack/lib/action_controller/layout.rb | 39 +++++++------- activerecord/CHANGELOG | 2 + activerecord/lib/active_record/callbacks.rb | 4 ++ .../lib/active_record/locking/optimistic.rb | 56 ++++++++++---------- activerecord/lib/active_record/timestamp.rb | 35 ++++++------- 9 files changed, 131 insertions(+), 119 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 669732a148..2ea1b4da27 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *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] diff --git a/actionpack/lib/action_controller/benchmarking.rb b/actionpack/lib/action_controller/benchmarking.rb index 469976e532..8e0d83c2d6 100644 --- a/actionpack/lib/action_controller/benchmarking.rb +++ b/actionpack/lib/action_controller/benchmarking.rb @@ -40,44 +40,44 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 3290defff8..ec5bb759df 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -615,15 +615,6 @@ module ActionController #:nodoc: 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 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/flash.rb b/actionpack/lib/action_controller/flash.rb index 0d12503904..2544db2fda 100644 --- a/actionpack/lib/action_controller/flash.rb +++ b/actionpack/lib/action_controller/flash.rb @@ -136,23 +136,14 @@ module ActionController #:nodoc: 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 flash["notice"] to read a notice you put there or # flash["notice"] = "hello" to put a new one. # Note that if sessions are disabled only flash.now will work. @@ -177,6 +168,17 @@ module ActionController #:nodoc: 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 diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index 115fd7ac18..ca070d0577 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -233,31 +233,32 @@ module ActionController #:nodoc: 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 diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index cbe9b8c4f0..11b965ad6b 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *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] diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index bfe96fd2ed..7a19dcb746 100755 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -227,6 +227,7 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 217bd3def7..7b7f87341e 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -23,7 +23,6 @@ module ActiveRecord # This method uses the same syntax as set_table_name module Optimistic def self.included(base) #:nodoc: - super base.extend ClassMethods base.cattr_accessor :lock_optimistically, :instance_writer => false @@ -41,41 +40,42 @@ module ActiveRecord 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' diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index 9c7e798183..77bcbd4c88 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -18,8 +18,6 @@ module ActiveRecord # ActiveRecord::Base.default_timezone = :utc module Timestamp def self.included(base) #:nodoc: - super - base.alias_method_chain :create, :timestamps base.alias_method_chain :update, :timestamps @@ -27,25 +25,26 @@ module ActiveRecord 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 -- cgit v1.2.3