From bdf783b5a8b83b1d565027130de4743fda336523 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Sun, 10 Aug 2008 21:25:24 +0200 Subject: update i18n usage for pluralization hashes (api change) --- activerecord/lib/active_record/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index e7a9676394..0de430567c 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -662,7 +662,7 @@ module ActiveRecord finder_class.with_exclusive_scope do if finder_class.exists?([condition_sql, *condition_params]) - message = record.errors.generate_message(attr_name, :taken, :default => configuration[:message]) + message = record.errors.generate_message(attr_name, :taken, :default => configuration[:message], :value => value) record.errors.add(attr_name, message) end end -- cgit v1.2.3 From f26380b7757666fa793c150538e8444a640d29aa Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Wed, 13 Aug 2008 09:53:25 +0200 Subject: switch to using I18n.load_translations instead of requiring plain ruby files --- activerecord/lib/active_record.rb | 2 +- activerecord/lib/active_record/locale/en-US.rb | 47 +++++++++++++------------- 2 files changed, 25 insertions(+), 24 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 17a7949959..08b6b19f7f 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -82,6 +82,6 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_record/schema_dumper' I18n.backend.populate do - require 'active_record/locale/en-US.rb' + I18n.load_translations File.dirname(__FILE__) + '/active_record/locale/en-US.rb' end diff --git a/activerecord/lib/active_record/locale/en-US.rb b/activerecord/lib/active_record/locale/en-US.rb index b31e13ed3a..4057a467e3 100644 --- a/activerecord/lib/active_record/locale/en-US.rb +++ b/activerecord/lib/active_record/locale/en-US.rb @@ -1,25 +1,26 @@ -I18n.backend.store_translations :'en-US', { - :active_record => { - :error_messages => { - :inclusion => "is not included in the list", - :exclusion => "is reserved", - :invalid => "is invalid", - :confirmation => "doesn't match confirmation", - :accepted => "must be accepted", - :empty => "can't be empty", - :blank => "can't be blank", - :too_long => "is too long (maximum is {{count}} characters)", - :too_short => "is too short (minimum is {{count}} characters)", - :wrong_length => "is the wrong length (should be {{count}} characters)", - :taken => "has already been taken", - :not_a_number => "is not a number", - :greater_than => "must be greater than {{count}}", - :greater_than_or_equal_to => "must be greater than or equal to {{count}}", - :equal_to => "must be equal to {{count}}", - :less_than => "must be less than {{count}}", - :less_than_or_equal_to => "must be less than or equal to {{count}}", - :odd => "must be odd", - :even => "must be even" - } +{ :'en-US' => { + :active_record => { + :error_messages => { + :inclusion => "is not included in the list", + :exclusion => "is reserved", + :invalid => "is invalid", + :confirmation => "doesn't match confirmation", + :accepted => "must be accepted", + :empty => "can't be empty", + :blank => "can't be blank", + :too_long => "is too long (maximum is {{count}} characters)", + :too_short => "is too short (minimum is {{count}} characters)", + :wrong_length => "is the wrong length (should be {{count}} characters)", + :taken => "has already been taken", + :not_a_number => "is not a number", + :greater_than => "must be greater than {{count}}", + :greater_than_or_equal_to => "must be greater than or equal to {{count}}", + :equal_to => "must be equal to {{count}}", + :less_than => "must be less than {{count}}", + :less_than_or_equal_to => "must be less than or equal to {{count}}", + :odd => "must be odd", + :even => "must be even" + } + } } } \ No newline at end of file -- cgit v1.2.3 From e3ecc3375fef4faa807638ff9e9cf309094272bd Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Wed, 13 Aug 2008 13:52:07 +0200 Subject: provide more useful feedback on missing translations for validation error messages --- activerecord/lib/active_record/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 0de430567c..85b8e6232b 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -70,7 +70,7 @@ module ActiveRecord msgs << options[:default] if options[:default] msgs << key - I18n.t nil, options.merge(:default => msgs, :scope => [:active_record, :error_messages]) + I18n.t msgs.shift, options.merge(:default => msgs, :scope => [:active_record, :error_messages]) end # Returns true if the specified +attribute+ has errors associated with it. -- cgit v1.2.3 From ffeab4e0c171aecced4ddbe29b82aed064be9bdb Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Thu, 14 Aug 2008 01:28:31 +0200 Subject: Cleaned up ActiveRecord i18n scoping --- activerecord/lib/active_record/base.rb | 30 ++++++- activerecord/lib/active_record/locale/en-US.rb | 48 +++++----- activerecord/lib/active_record/validations.rb | 120 +++++++++++++------------ 3 files changed, 116 insertions(+), 82 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 29c2995334..02a0ca7e44 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1220,13 +1220,35 @@ module ActiveRecord #:nodoc: subclasses.each { |klass| klass.reset_inheritable_attributes; klass.reset_column_information } end + def self_and_descendents_from_active_record#nodoc: + klass = self + classes = [klass] + while klass != klass.base_class + classes << klass = klass.superclass + end + classes + rescue + # OPTIMIZE this rescue is to fix this test: ./test/cases/reflection_test.rb:56:in `test_human_name_for_column' + # Appearantly the method base_class causes some trouble. + # It now works for sure. + [self] + end + # Transforms attribute key names into a more humane format, such as "First name" instead of "first_name". Example: # Person.human_attribute_name("first_name") # => "First name" - # Deprecated in favor of just calling "first_name".humanize - def human_attribute_name(attribute_key_name) #:nodoc: - attribute_key_name.humanize + # This used to be depricated in favor of humanize, but is now preferred, because it automatically uses the I18n + # module now. + # Specify +options+ with additional translating options. + def human_attribute_name(attribute_key_name, options = {}) + defaults = self_and_descendents_from_active_record.map do |klass| + :"#{klass.name.underscore}.#{attribute_key_name}" + end + defaults << options[:default] if options[:default] + defaults.flatten! + defaults << attribute_key_name.humanize + options[:count] ||= 1 + I18n.translate(defaults.shift, options.merge(:default => defaults, :scope => [:activerecord, :attributes])) end - # True if this isn't a concrete subclass needing a STI type condition. def descends_from_active_record? if superclass.abstract_class? diff --git a/activerecord/lib/active_record/locale/en-US.rb b/activerecord/lib/active_record/locale/en-US.rb index 4057a467e3..89a5baba06 100644 --- a/activerecord/lib/active_record/locale/en-US.rb +++ b/activerecord/lib/active_record/locale/en-US.rb @@ -1,26 +1,28 @@ { :'en-US' => { - :active_record => { - :error_messages => { - :inclusion => "is not included in the list", - :exclusion => "is reserved", - :invalid => "is invalid", - :confirmation => "doesn't match confirmation", - :accepted => "must be accepted", - :empty => "can't be empty", - :blank => "can't be blank", - :too_long => "is too long (maximum is {{count}} characters)", - :too_short => "is too short (minimum is {{count}} characters)", - :wrong_length => "is the wrong length (should be {{count}} characters)", - :taken => "has already been taken", - :not_a_number => "is not a number", - :greater_than => "must be greater than {{count}}", - :greater_than_or_equal_to => "must be greater than or equal to {{count}}", - :equal_to => "must be equal to {{count}}", - :less_than => "must be less than {{count}}", - :less_than_or_equal_to => "must be less than or equal to {{count}}", - :odd => "must be odd", - :even => "must be even" - } + :activerecord => { + :errors => { + :messages => { + :inclusion => "is not included in the list", + :exclusion => "is reserved", + :invalid => "is invalid", + :confirmation => "doesn't match confirmation", + :accepted => "must be accepted", + :empty => "can't be empty", + :blank => "can't be blank", + :too_long => "is too long (maximum is {{count}} characters)", + :too_short => "is too short (minimum is {{count}} characters)", + :wrong_length => "is the wrong length (should be {{count}} characters)", + :taken => "has already been taken", + :not_a_number => "is not a number", + :greater_than => "must be greater than {{count}}", + :greater_than_or_equal_to => "must be greater than or equal to {{count}}", + :equal_to => "must be equal to {{count}}", + :less_than => "must be less than {{count}}", + :less_than_or_equal_to => "must be less than or equal to {{count}}", + :odd => "must be odd", + :even => "must be even" + } + } } } -} \ No newline at end of file +} diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 85b8e6232b..92b3e9a597 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -21,8 +21,8 @@ module ActiveRecord class << self def default_error_messages - ActiveSupport::Deprecation.warn("ActiveRecord::Errors.default_error_messages has been deprecated. Please use I18n.translate('active_record.error_messages').") - I18n.translate 'active_record.error_messages' + ActiveSupport::Deprecation.warn("ActiveRecord::Errors.default_error_messages has been deprecated. Please use I18n.translate('activerecord.errors.messages').") + I18n.translate 'activerecord.errors.messages' end end @@ -38,22 +38,24 @@ module ActiveRecord add(:base, msg) end - # Adds an error message (+msg+) to the +attribute+, which will be returned on a call to on(attribute) + # Adds an error message (+messsage+) to the +attribute+, which will be returned on a call to on(attribute) # for the same attribute and ensure that this error object returns false when asked if empty?. More than one # error can be added to the same +attribute+ in which case an array will be returned on a call to on(attribute). - # If no +msg+ is supplied, "invalid" is assumed. - def add(attribute, message = nil) - message ||= I18n.translate :"active_record.error_messages.invalid" + # If no +messsage+ is supplied, :invalid is assumed. + # If +message+ is a Symbol, it will be translated, using the appropriate scope (see translate_error). + def add(attribute, message = nil, options = {}) + message ||= :invalid + message = generate_message(attribute, message, options) if message.is_a?(Symbol) @errors[attribute.to_s] ||= [] @errors[attribute.to_s] << message - end + end # Will add an error message to each of the attributes in +attributes+ that is empty. def add_on_empty(attributes, custom_message = nil) for attr in [attributes].flatten value = @base.respond_to?(attr.to_s) ? @base.send(attr.to_s) : @base[attr.to_s] is_empty = value.respond_to?("empty?") ? value.empty? : false - add(attr, generate_message(attr, :empty, :default => custom_message)) unless !value.nil? && !is_empty + add(attr, :empty, :default => custom_message) unless !value.nil? && !is_empty end end @@ -61,16 +63,48 @@ module ActiveRecord def add_on_blank(attributes, custom_message = nil) for attr in [attributes].flatten value = @base.respond_to?(attr.to_s) ? @base.send(attr.to_s) : @base[attr.to_s] - add(attr, generate_message(attr, :blank, :default => custom_message)) if value.blank? + add(attr, :blank, :default => custom_message) if value.blank? end end - def generate_message(attr, key, options = {}) - msgs = base_classes(@base.class).map{|klass| :"custom.#{klass.name.underscore}.#{attr}.#{key}"} - msgs << options[:default] if options[:default] - msgs << key + # Translates an error message in it's default scope (activerecord.errrors.messages). + # Error messages are first looked up in custom.MODEL.ATTRIBUTE.MESSAGE, if it's not there, it's looked up + # in custom.MODEL.ATTRIBUTE and if that is not there it returns the translation of the default message + # (e.g. activerecord.errors.messages.MESSAGE). Both the model name and the attribute name are available for + # interpolation. + # + # When using inheritence in your models, it will check all the inherited models too, but only if the model itself + # hasn't been found. Say you have class Admin < User; end and you wanted the translation for the :blank + # error +message+ for the title +attribute+, it looks for these translations: + # + #
    + #
  1. activerecord.errors.messages.custom.admin.title.blank
  2. + #
  3. activerecord.errors.messages.custom.admin.blank
  4. + #
  5. activerecord.errors.messages.custom.user.title.blank
  6. + #
  7. activerecord.errors.messages.custom.user.blank
  8. + #
  9. activerecord.errors.messages.blank
  10. + #
  11. any default you provided through the +options+ hash
  12. + #
+ def generate_message(attribute, message = :invalid, options = {}) + + defaults = @base.class.self_and_descendents_from_active_record.map do |klass| + [ :"custom.#{klass.name.underscore}.#{attribute}.#{message}", + :"custom.#{klass.name.underscore}.#{message}" ] + end + + defaults << options[:default] if options[:default] + defaults.flatten! << message + + model_name = @base.class.name + key = defaults.shift - I18n.t msgs.shift, options.merge(:default => msgs, :scope => [:active_record, :error_messages]) + options.merge!({ + :default => defaults, + :model => I18n.translate(model_name.underscore, :default => model_name.humanize, :scope => [:activerecord, :models], :count => 1), + :attribute => @base.class.human_attribute_name(attribute.to_s), + :scope => [:activerecord, :errors, :messages] }) + + I18n.translate(key, options) end # Returns true if the specified +attribute+ has errors associated with it. @@ -166,9 +200,9 @@ module ActiveRecord if attr == "base" full_messages << message else - key = :"active_record.human_attribute_names.#{@base.class.name.underscore.to_sym}.#{attr}" - attr_name = I18n.translate(key, :locale => options[:locale], :default => @base.class.human_attribute_name(attr)) - full_messages << attr_name + " " + message + #key = :"activerecord.att.#{@base.class.name.underscore.to_sym}.#{attr}" + attr_name = @base.class.human_attribute_name(attr) + full_messages << attr_name + ' ' + message end end end @@ -219,16 +253,6 @@ module ActiveRecord end end - protected - - # TODO maybe this should be on ActiveRecord::Base, maybe #self_and_descendents_from_active_record - def base_classes(klass) - classes = [klass] - while klass != klass.base_class - classes << klass = klass.superclass - end - classes - end end @@ -398,8 +422,7 @@ module ActiveRecord validates_each(attr_names, configuration) do |record, attr_name, value| unless record.send("#{attr_name}_confirmation").nil? or value == record.send("#{attr_name}_confirmation") - message = record.errors.generate_message(attr_name, :confirmation, :default => configuration[:message]) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :confirmation, :default => configuration[:message]) end end end @@ -441,8 +464,7 @@ module ActiveRecord validates_each(attr_names,configuration) do |record, attr_name, value| unless value == configuration[:accept] - message = record.errors.generate_message(attr_name, :accepted, :default => configuration[:message]) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :accepted, :default => configuration[:message]) end end end @@ -544,11 +566,9 @@ module ActiveRecord validates_each(attrs, options) do |record, attr, value| value = options[:tokenizer].call(value) if value.kind_of?(String) if value.nil? or value.size < option_value.begin - message = record.errors.generate_message(attr, :too_short, :default => options[:too_short], :count => option_value.begin) - record.errors.add(attr, message) + record.errors.add(attr, :too_short, :default => options[:too_short], :count => option_value.begin) elsif value.size > option_value.end - message = record.errors.generate_message(attr, :too_long, :default => options[:too_long], :count => option_value.end) - record.errors.add(attr, message) + record.errors.add(attr, :too_long, :default => options[:too_long], :count => option_value.end) end end when :is, :minimum, :maximum @@ -563,8 +583,7 @@ module ActiveRecord unless !value.nil? and value.size.method(validity_checks[option])[option_value] key = message_options[option] custom_message = options[:message] || options[key] - message = record.errors.generate_message(attr, key, :default => custom_message, :count => option_value) - record.errors.add(attr, message) + record.errors.add(attr, key, :default => custom_message, :count => option_value) end end end @@ -662,8 +681,7 @@ module ActiveRecord finder_class.with_exclusive_scope do if finder_class.exists?([condition_sql, *condition_params]) - message = record.errors.generate_message(attr_name, :taken, :default => configuration[:message], :value => value) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :taken, :default => configuration[:message], :value => value) end end end @@ -701,8 +719,7 @@ module ActiveRecord validates_each(attr_names, configuration) do |record, attr_name, value| unless value.to_s =~ configuration[:with] - message = record.errors.generate_message(attr_name, :invalid, :default => configuration[:message], :value => value) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value) end end end @@ -736,8 +753,7 @@ module ActiveRecord validates_each(attr_names, configuration) do |record, attr_name, value| unless enum.include?(value) - message = record.errors.generate_message(attr_name, :inclusion, :default => configuration[:message], :value => value) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :inclusion, :default => configuration[:message], :value => value) end end end @@ -771,8 +787,7 @@ module ActiveRecord validates_each(attr_names, configuration) do |record, attr_name, value| if enum.include?(value) - message = record.errors.generate_message(attr_name, :exclusion, :default => configuration[:message], :value => value) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :exclusion, :default => configuration[:message], :value => value) end end end @@ -814,8 +829,7 @@ module ActiveRecord validates_each(attr_names, configuration) do |record, attr_name, value| unless (value.is_a?(Array) ? value : [value]).inject(true) { |v, r| (r.nil? || r.valid?) && v } - message = record.errors.generate_message(attr_name, :invalid, :default => configuration[:message], :value => value) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value) end end end @@ -864,8 +878,7 @@ module ActiveRecord if configuration[:only_integer] unless raw_value.to_s =~ /\A[+-]?\d+\Z/ - message = record.errors.generate_message(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) next end raw_value = raw_value.to_i @@ -873,8 +886,7 @@ module ActiveRecord begin raw_value = Kernel.Float(raw_value) rescue ArgumentError, TypeError - message = record.errors.generate_message(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) - record.errors.add(attr_name, message) + record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) next end end @@ -883,12 +895,10 @@ module ActiveRecord case option when :odd, :even unless raw_value.to_i.method(ALL_NUMERICALITY_CHECKS[option])[] - message = record.errors.generate_message(attr_name, option, :value => raw_value, :default => configuration[:message]) - record.errors.add(attr_name, message) + record.errors.add(attr_name, option, :value => raw_value, :default => configuration[:message]) end else - message = record.errors.generate_message(attr_name, option, :default => configuration[:message], :value => raw_value, :count => configuration[option]) - record.errors.add(attr_name, message) unless raw_value.method(ALL_NUMERICALITY_CHECKS[option])[configuration[option]] + record.errors.add(attr_name, option, :default => configuration[:message], :value => raw_value, :count => configuration[option]) unless raw_value.method(ALL_NUMERICALITY_CHECKS[option])[configuration[option]] end end end -- cgit v1.2.3 From e3523f1d33c3cf53f1a65e520be5e937e9c68c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Fri, 15 Aug 2008 18:39:11 +0300 Subject: Fixed validates_uniqueness_of with decimal columns Only use special case-sensitive comparison operators for text columns in validates_uniqueness_of as mysql can fail at decimal comparisons with the BINARY operator. --- activerecord/lib/active_record/validations.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index e7a9676394..b7e6394748 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -629,12 +629,11 @@ module ActiveRecord if value.nil? comparison_operator = "IS ?" - else + elsif is_text_column comparison_operator = "#{connection.case_sensitive_equality_operator} ?" - - if is_text_column - value = value.to_s - end + value = value.to_s + else + comparison_operator = "= ?" end sql_attribute = "#{record.class.quoted_table_name}.#{connection.quote_column_name(attr_name)}" -- cgit v1.2.3 From b3c9d53b348f586ed223ec5de9f525faee6f564d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Mon, 2 Jun 2008 16:48:06 +0300 Subject: Use type_condition method for hmt STI condition --- .../lib/active_record/associations/has_many_through_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index e1bfff5923..24b02efc35 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -237,7 +237,7 @@ module ActiveRecord end def build_sti_condition - "#{@reflection.through_reflection.quoted_table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.sti_name)}" + @reflection.through_reflection.klass.send(:type_condition) end alias_method :sql_conditions, :conditions -- cgit v1.2.3 From 8f4d3957a6986fe450cfd9058bb92ae1d6e5e745 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Fri, 15 Aug 2008 13:51:57 -0700 Subject: Don't raise exception when comparing ActiveRecord::Reflection. [#842 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 3f74c03714..935b1939d8 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -109,7 +109,7 @@ module ActiveRecord # Returns +true+ if +self+ and +other_aggregation+ have the same +name+ attribute, +active_record+ attribute, # and +other_aggregation+ has an options hash assigned to it. def ==(other_aggregation) - name == other_aggregation.name && other_aggregation.options && active_record == other_aggregation.active_record + other_aggregation.kind_of?(self.class) && name == other_aggregation.name && other_aggregation.options && active_record == other_aggregation.active_record end def sanitized_conditions #:nodoc: -- cgit v1.2.3 From 2b69a636c431d62a85b2896d87b69cb13e2b8af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Sat, 16 Aug 2008 02:24:29 +0300 Subject: Fixed STI type condition for eager loading of associations Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 6 ++---- activerecord/lib/active_record/base.rb | 7 ++++--- 2 files changed, 6 insertions(+), 7 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4e33dfe69f..b72fdb305f 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2099,10 +2099,8 @@ module ActiveRecord else "" end || '' - join << %(AND %s.%s = %s ) % [ - connection.quote_table_name(aliased_table_name), - connection.quote_column_name(klass.inheritance_column), - klass.quote_value(klass.sti_name)] unless klass.descends_from_active_record? + join << %(AND %s) % [ + klass.send(:type_condition, aliased_table_name)] unless klass.descends_from_active_record? [through_reflection, reflection].each do |ref| join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} " if ref && ref.options[:conditions] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2c4ead081d..6eb4d42d51 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1609,10 +1609,11 @@ module ActiveRecord #:nodoc: sql << "WHERE #{merged_conditions} " unless merged_conditions.blank? end - def type_condition + def type_condition(table_alias=nil) + quoted_table_alias = self.connection.quote_table_name(table_alias || table_name) quoted_inheritance_column = connection.quote_column_name(inheritance_column) - type_condition = subclasses.inject("#{quoted_table_name}.#{quoted_inheritance_column} = '#{sti_name}' ") do |condition, subclass| - condition << "OR #{quoted_table_name}.#{quoted_inheritance_column} = '#{subclass.sti_name}' " + type_condition = subclasses.inject("#{quoted_table_alias}.#{quoted_inheritance_column} = '#{sti_name}' ") do |condition, subclass| + condition << "OR #{quoted_table_alias}.#{quoted_inheritance_column} = '#{subclass.sti_name}' " end " (#{type_condition}) " -- cgit v1.2.3 From 8cfdcdb35db6e2f6fd5a72a38f4352beab148af1 Mon Sep 17 00:00:00 2001 From: Nathan Witmer Date: Sat, 16 Aug 2008 13:38:01 -0600 Subject: Updated has_and_belongs_to_many association to fix :finder_sql interpolation. [#848 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations/association_proxy.rb | 4 ---- .../active_record/associations/has_and_belongs_to_many_association.rb | 4 +--- 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 77fc827e11..981be3b1a9 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -131,10 +131,6 @@ module ActiveRecord records.map { |record| record.quoted_id }.join(',') end - def interpolate_sql_options!(options, *keys) - keys.each { |key| options[key] &&= interpolate_sql(options[key]) } - end - def interpolate_sql(sql, record = nil) @owner.send(:interpolate_sql, sql, record) end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index d516d54151..e7e433b6b6 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -70,10 +70,8 @@ module ActiveRecord end def construct_sql - interpolate_sql_options!(@reflection.options, :finder_sql) - if @reflection.options[:finder_sql] - @finder_sql = @reflection.options[:finder_sql] + @finder_sql = interpolate_sql(@reflection.options[:finder_sql]) else @finder_sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} " @finder_sql << " AND (#{conditions})" if conditions -- cgit v1.2.3 From cd8e653d5b18e6d3c3acc9930832f8e23945e392 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 11 Aug 2008 10:25:57 -0700 Subject: Performance: freeze cached rows instead of duping --- activerecord/lib/active_record/base.rb | 2 +- .../connection_adapters/abstract/query_cache.rb | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 6eb4d42d51..5357255bad 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -612,7 +612,7 @@ module ActiveRecord #:nodoc: # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # > [#"The Cheap Man Buys Twice"}>, ...] def find_by_sql(sql) - connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } + connection.select_all(sanitize_sql(sql), "#{name} Load").map { |record| instantiate(record) } end # Checks whether a record exists in the database that matches conditions given. These conditions diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 2afd6064ad..81a2e56b34 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -72,21 +72,12 @@ module ActiveRecord private def cache_sql(sql) - result = - if @query_cache.has_key?(sql) - log_info(sql, "CACHE", 0.0) - @query_cache[sql] - else - @query_cache[sql] = yield - end - - if Array === result - result.collect { |row| row.dup } + if @query_cache.has_key?(sql) + log_info(sql, "CACHE", 0.0) + @query_cache[sql] else - result.duplicable? ? result.dup : result + @query_cache[sql] = yield.freeze end - rescue TypeError - result end end end -- cgit v1.2.3 From ae8a35d8f6d5db6ae9a1877918d45c15d21e24fe Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Sat, 16 Aug 2008 20:01:42 +0200 Subject: Added Base.human_name method --- activerecord/lib/active_record/base.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 02a0ca7e44..edab017d26 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1249,6 +1249,19 @@ module ActiveRecord #:nodoc: options[:count] ||= 1 I18n.translate(defaults.shift, options.merge(:default => defaults, :scope => [:activerecord, :attributes])) end + + # Transform the modelname into a more humane format, using I18n. + # Defaults to the basic humanize method. + # Default scope of the translation is activerecord.models + # Specify +options+ with additional translating options. + def human_name(options = {}) + defaults = self_and_descendents_from_active_record.map do |klass| + :"#{klass.name.underscore}" + end + defaults << self.name.humanize + I18n.translate(defaults.shift, {:scope => [:activerecord, :models], :count => 1, :default => defaults}.merge(options)) + end + # True if this isn't a concrete subclass needing a STI type condition. def descends_from_active_record? if superclass.abstract_class? -- cgit v1.2.3 From c531248938302477c5e52138d59a6c3d1527d963 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Sat, 16 Aug 2008 20:22:40 +0200 Subject: Introduced AR::Base.human_name to validations --- activerecord/lib/active_record/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 92b3e9a597..040681a09c 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -100,7 +100,7 @@ module ActiveRecord options.merge!({ :default => defaults, - :model => I18n.translate(model_name.underscore, :default => model_name.humanize, :scope => [:activerecord, :models], :count => 1), + :model => @base.class.human_name, :attribute => @base.class.human_attribute_name(attribute.to_s), :scope => [:activerecord, :errors, :messages] }) -- cgit v1.2.3 From e2b191681e1c8c8b4344f1fc755e48fccdd1d603 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Sat, 16 Aug 2008 21:45:23 +0200 Subject: Added :value as interpolation variable available to error messages --- activerecord/lib/active_record/validations.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 040681a09c..a442d008a9 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -92,17 +92,19 @@ module ActiveRecord :"custom.#{klass.name.underscore}.#{message}" ] end - defaults << options[:default] if options[:default] - defaults.flatten! << message + defaults << options.delete(:default) + defaults = defaults.compact.flatten << message model_name = @base.class.name key = defaults.shift - - options.merge!({ - :default => defaults, - :model => @base.class.human_name, - :attribute => @base.class.human_attribute_name(attribute.to_s), - :scope => [:activerecord, :errors, :messages] }) + value = @base.send(attribute) if @base.respond_to?(attribute) + + options = { :default => defaults, + :model => @base.class.human_name, + :attribute => @base.class.human_attribute_name(attribute.to_s), + :value => value, + :scope => [:activerecord, :errors, :messages] + }.merge(options) I18n.translate(key, options) end -- cgit v1.2.3 From febe2ea9775c863cb9744c6343291e550e4628b8 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Tue, 19 Aug 2008 23:19:57 +0200 Subject: Locale file changed to yaml --- activerecord/lib/active_record.rb | 2 +- activerecord/lib/active_record/locale/en-US.yml | 33 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 activerecord/lib/active_record/locale/en-US.yml (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 08b6b19f7f..bc27d17ccd 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -82,6 +82,6 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_record/schema_dumper' I18n.backend.populate do - I18n.load_translations File.dirname(__FILE__) + '/active_record/locale/en-US.rb' + I18n.load_translations File.dirname(__FILE__) + '/active_record/locale/en-US.yml' end diff --git a/activerecord/lib/active_record/locale/en-US.yml b/activerecord/lib/active_record/locale/en-US.yml new file mode 100644 index 0000000000..8148f31a81 --- /dev/null +++ b/activerecord/lib/active_record/locale/en-US.yml @@ -0,0 +1,33 @@ +en-US: + activerecord: + errors: + # The values :model, :attribute and :value are always available for interpolation + # The value :count is available when applicable. Can be used for pluralization. + messages: + inclusion: "is not included in the list" + exclusion: "is reserved" + invalid: "is invalid" + confirmation: "doesn't match confirmation" + accepted: "must be accepted" + empty: "can't be empty" + blank: "can't be blank" + too_long: "is too long (maximum is {{count}} characters)" + too_short: "is too short (minimum is {{count}} characters)" + wrong_length: "is the wrong length (should be {{count}} characters)" + taken: "has already been taken" + not_a_number: "is not a number" + greater_than: "must be greater than {{count}}" + greater_than_or_equal_to: "must be greater than or equal to {{count}}" + equal_to: "must be equal to {{count}}" + less_than: "must be less than {{count}}" + less_than_or_equal_to: "must be less than or equal to {{count}}" + odd: "must be odd" + even: "must be even" + # Append your own errors here or at the model/attributes scope. + + models: + # Overrides default messages + + attributes: + # Overrides model and default messages. + -- cgit v1.2.3 From 72366398b28cec2f8b9809f463e0b1271e3d5ba0 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Tue, 19 Aug 2008 23:22:59 +0200 Subject: Removed en-US ruby locale in favor of yaml --- activerecord/lib/active_record/locale/en-US.rb | 28 -------------------------- 1 file changed, 28 deletions(-) delete mode 100644 activerecord/lib/active_record/locale/en-US.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/locale/en-US.rb b/activerecord/lib/active_record/locale/en-US.rb deleted file mode 100644 index 89a5baba06..0000000000 --- a/activerecord/lib/active_record/locale/en-US.rb +++ /dev/null @@ -1,28 +0,0 @@ -{ :'en-US' => { - :activerecord => { - :errors => { - :messages => { - :inclusion => "is not included in the list", - :exclusion => "is reserved", - :invalid => "is invalid", - :confirmation => "doesn't match confirmation", - :accepted => "must be accepted", - :empty => "can't be empty", - :blank => "can't be blank", - :too_long => "is too long (maximum is {{count}} characters)", - :too_short => "is too short (minimum is {{count}} characters)", - :wrong_length => "is the wrong length (should be {{count}} characters)", - :taken => "has already been taken", - :not_a_number => "is not a number", - :greater_than => "must be greater than {{count}}", - :greater_than_or_equal_to => "must be greater than or equal to {{count}}", - :equal_to => "must be equal to {{count}}", - :less_than => "must be less than {{count}}", - :less_than_or_equal_to => "must be less than or equal to {{count}}", - :odd => "must be odd", - :even => "must be even" - } - } - } - } -} -- cgit v1.2.3 From 2415652660242d6b0da97119c562ecff82928575 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Thu, 21 Aug 2008 12:37:19 +0100 Subject: Support find_all on named scopes. [#730 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 0902018155..26701548c2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -103,7 +103,7 @@ module ActiveRecord attr_reader :proxy_scope, :proxy_options [].methods.each do |m| - unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|find|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ + unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ delegate m, :to => :proxy_found end end -- cgit v1.2.3 From ea40f71431a821b2ddb37be6ea3ee7d8dac63b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ku=C5=BAma?= Date: Thu, 21 Aug 2008 12:55:35 +0200 Subject: Fix that has_one natural assignment to already associated record. [#854 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations/has_one_association.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index fdc0fa52c9..18733255d2 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -21,8 +21,8 @@ module ActiveRecord def replace(obj, dont_save = false) load_target - unless @target.nil? - if dependent? && !dont_save && @target != obj + unless @target.nil? || @target == obj + if dependent? && !dont_save @target.destroy unless @target.new_record? @owner.clear_association_cache else -- cgit v1.2.3 From a970f916fb1e05376733e2d42d9bcc2b873af355 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 21 Aug 2008 15:45:06 +0100 Subject: Fix has_many#count_records. [#865 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/associations/has_many_association.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index e6fa15c173..ce62127505 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -35,8 +35,11 @@ module ActiveRecord else @reflection.klass.count(:conditions => @counter_sql, :include => @reflection.options[:include]) end - - @target = [] and loaded if count == 0 + + # If there's nothing in the database and @target has no new records + # we are certain the current target is an empty array. This is a + # documented side-effect of the method that may avoid an extra SELECT. + @target ||= [] and loaded if count == 0 if @reflection.options[:limit] count = [ @reflection.options[:limit], count ].min -- cgit v1.2.3 From 49c0e1e594c95d7e8446ebabecc9147afa62de7d Mon Sep 17 00:00:00 2001 From: Philip Hallstrom Date: Thu, 21 Aug 2008 16:08:42 +0100 Subject: Fix generated WHERE IN query for named scopes. [#583 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5357255bad..15c6bc1b4a 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1750,7 +1750,7 @@ module ActiveRecord #:nodoc: def attribute_condition(argument) case argument when nil then "IS ?" - when Array, ActiveRecord::Associations::AssociationCollection then "IN (?)" + when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope then "IN (?)" when Range then "BETWEEN ? AND ?" else "= ?" end -- cgit v1.2.3 From 0d74e72e6de7b96e158950a449ea1ccce6f5b8d7 Mon Sep 17 00:00:00 2001 From: Miles Georgi Date: Sun, 17 Aug 2008 23:45:25 -0700 Subject: Fix postgres bug when change_column is called with invalid parameters. [#861 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tarmo Tänav Signed-off-by: Pratik Naik --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 856435517a..74da0d9c85 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -761,7 +761,8 @@ module ActiveRecord begin execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => e + raise e if postgresql_version > 80000 # This is PostgreSQL 7.x, so we have to use a more arcane way of doing it. begin begin_db_transaction -- cgit v1.2.3 From 3724dafe71f4afb2ca9f4d7d2526b228aa6c05a3 Mon Sep 17 00:00:00 2001 From: Tom Lea Date: Thu, 21 Aug 2008 16:38:27 +0100 Subject: Fix incorrect signature for NamedScope#respond_to? [#852 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 26701548c2..c99c4beca9 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -140,8 +140,8 @@ module ActiveRecord @found ? @found.empty? : count.zero? end - def respond_to?(method) - super || @proxy_scope.respond_to?(method) + def respond_to?(method, include_private = false) + super || @proxy_scope.respond_to?(method, include_private) end def any? -- cgit v1.2.3 From 8622787f8748434b4ceb2b925a35b17a38e1f2d6 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Wed, 2 Jul 2008 21:27:42 -0400 Subject: Don't interpret decimals as table names in ActiveRecord::Associations::ClassMethods#references_eager_loaded_tables? [#532 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) mode change 100644 => 100755 activerecord/lib/active_record/associations.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb old mode 100644 new mode 100755 index b72fdb305f..b9039ce996 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1679,19 +1679,19 @@ module ActiveRecord else all << cond end end - conditions.join(' ').scan(/([\.\w]+).?\./).flatten + conditions.join(' ').scan(/([\.a-zA-Z_]+).?\./).flatten end def order_tables(options) order = [options[:order], scope(:find, :order) ].join(", ") return [] unless order && order.is_a?(String) - order.scan(/([\.\w]+).?\./).flatten + order.scan(/([\.a-zA-Z_]+).?\./).flatten end def selects_tables(options) select = options[:select] return [] unless select && select.is_a?(String) - select.scan(/"?([\.\w]+)"?.?\./).flatten + select.scan(/"?([\.a-zA-Z_]+)"?.?\./).flatten end # Checks if the conditions reference a table other than the current model table -- cgit v1.2.3 From cf6840773b4f5046151f4d28c286069aabaafa10 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Wed, 20 Aug 2008 23:43:46 +0200 Subject: Custom error messages scope improved --- activerecord/lib/active_record/validations.rb | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index a442d008a9..1ea520f3b6 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -68,42 +68,42 @@ module ActiveRecord end # Translates an error message in it's default scope (activerecord.errrors.messages). - # Error messages are first looked up in custom.MODEL.ATTRIBUTE.MESSAGE, if it's not there, it's looked up - # in custom.MODEL.ATTRIBUTE and if that is not there it returns the translation of the default message - # (e.g. activerecord.errors.messages.MESSAGE). Both the model name and the attribute name are available for - # interpolation. + # Error messages are first looked up in models.MODEL.attributes.ATTRIBUTE.MESSAGE, if it's not there, + # it's looked up in models.MODEL.MESSAGE and if that is not there it returns the translation of the + # default message (e.g. activerecord.errors.messages.MESSAGE). The translated model name, + # translated attribute name and the value are available for interpolation. # # When using inheritence in your models, it will check all the inherited models too, but only if the model itself # hasn't been found. Say you have class Admin < User; end and you wanted the translation for the :blank # error +message+ for the title +attribute+, it looks for these translations: # #
    - #
  1. activerecord.errors.messages.custom.admin.title.blank
  2. - #
  3. activerecord.errors.messages.custom.admin.blank
  4. - #
  5. activerecord.errors.messages.custom.user.title.blank
  6. - #
  7. activerecord.errors.messages.custom.user.blank
  8. + #
  9. activerecord.errors.models.admin.attributes.title.blank
  10. + #
  11. activerecord.errors.models.admin.blank
  12. + #
  13. activerecord.errors.models.user.attributes.title.blank
  14. + #
  15. activerecord.errors.models.user.blank
  16. #
  17. activerecord.errors.messages.blank
  18. - #
  19. any default you provided through the +options+ hash
  20. + #
  21. any default you provided through the +options+ hash (in the activerecord.errors scope)
  22. #
def generate_message(attribute, message = :invalid, options = {}) defaults = @base.class.self_and_descendents_from_active_record.map do |klass| - [ :"custom.#{klass.name.underscore}.#{attribute}.#{message}", - :"custom.#{klass.name.underscore}.#{message}" ] + [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}", + :"models.#{klass.name.underscore}.#{message}" ] end defaults << options.delete(:default) - defaults = defaults.compact.flatten << message + defaults = defaults.compact.flatten << :"messages.#{message}" model_name = @base.class.name key = defaults.shift - value = @base.send(attribute) if @base.respond_to?(attribute) + value = @base.respond_to?(attribute) ? @base.send(attribute) : nil options = { :default => defaults, :model => @base.class.human_name, :attribute => @base.class.human_attribute_name(attribute.to_s), :value => value, - :scope => [:activerecord, :errors, :messages] + :scope => [:activerecord, :errors] }.merge(options) I18n.translate(key, options) -- cgit v1.2.3 From aee14630d4dc0856e597794cc731fac68c2d2e34 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 18 Aug 2008 15:56:37 -0700 Subject: coerce blank strings to nil values for boolean and integer fields Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/base.rb | 13 ++++++++----- .../connection_adapters/abstract/schema_definitions.rb | 6 +++++- 2 files changed, 13 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 15c6bc1b4a..f4f07aa740 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2572,11 +2572,14 @@ module ActiveRecord #:nodoc: end def convert_number_column_value(value) - case value - when FalseClass; 0 - when TrueClass; 1 - when ''; nil - else value + if value == false + 0 + elsif value == true + 1 + elsif value.is_a?(String) && value.blank? + nil + else + value end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 31d6c7942c..08b2c79389 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -138,7 +138,11 @@ module ActiveRecord # convert something to a boolean def value_to_boolean(value) - TRUE_VALUES.include?(value) + if value.blank? + nil + else + TRUE_VALUES.include?(value) + end end # convert something to a BigDecimal -- cgit v1.2.3 From a5eb297424f68583636b762686726bc0c84703c0 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 21 Aug 2008 21:34:17 -0700 Subject: Revert "coerce blank strings to nil values for boolean and integer fields" This reverts commit aee14630d4dc0856e597794cc731fac68c2d2e34. [#860 state:incomplete] --- activerecord/lib/active_record/base.rb | 13 +++++-------- .../connection_adapters/abstract/schema_definitions.rb | 6 +----- 2 files changed, 6 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f4f07aa740..15c6bc1b4a 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2572,14 +2572,11 @@ module ActiveRecord #:nodoc: end def convert_number_column_value(value) - if value == false - 0 - elsif value == true - 1 - elsif value.is_a?(String) && value.blank? - nil - else - value + case value + when FalseClass; 0 + when TrueClass; 1 + when ''; nil + else value end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 08b2c79389..31d6c7942c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -138,11 +138,7 @@ module ActiveRecord # convert something to a boolean def value_to_boolean(value) - if value.blank? - nil - else - TRUE_VALUES.include?(value) - end + TRUE_VALUES.include?(value) end # convert something to a BigDecimal -- cgit v1.2.3 From 6e3d2a7996f52bd0d7a5157f73a471307ba8aabd Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 21 Aug 2008 21:40:49 -0700 Subject: Revert "Performance: freeze cached rows instead of duping" This reverts commit cd8e653d5b18e6d3c3acc9930832f8e23945e392. --- activerecord/lib/active_record/base.rb | 2 +- .../connection_adapters/abstract/query_cache.rb | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 15c6bc1b4a..0cce1e0157 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -612,7 +612,7 @@ module ActiveRecord #:nodoc: # Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date] # > [#"The Cheap Man Buys Twice"}>, ...] def find_by_sql(sql) - connection.select_all(sanitize_sql(sql), "#{name} Load").map { |record| instantiate(record) } + connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } end # Checks whether a record exists in the database that matches conditions given. These conditions diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 81a2e56b34..2afd6064ad 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -72,12 +72,21 @@ module ActiveRecord private def cache_sql(sql) - if @query_cache.has_key?(sql) - log_info(sql, "CACHE", 0.0) - @query_cache[sql] + result = + if @query_cache.has_key?(sql) + log_info(sql, "CACHE", 0.0) + @query_cache[sql] + else + @query_cache[sql] = yield + end + + if Array === result + result.collect { |row| row.dup } else - @query_cache[sql] = yield.freeze + result.duplicable? ? result.dup : result end + rescue TypeError + result end end end -- cgit v1.2.3 From d3b894563a114912113b816f07ed16511363fb65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Fri, 22 Aug 2008 09:44:38 +0300 Subject: Properly quote CREATE DATABASE parameters in postgresql [#771 state:resolved] --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 74da0d9c85..723e1a42dd 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -534,13 +534,13 @@ module ActiveRecord option_string = options.symbolize_keys.sum do |key, value| case key when :owner - " OWNER = '#{value}'" + " OWNER = \"#{value}\"" when :template - " TEMPLATE = #{value}" + " TEMPLATE = \"#{value}\"" when :encoding " ENCODING = '#{value}'" when :tablespace - " TABLESPACE = #{value}" + " TABLESPACE = \"#{value}\"" when :connection_limit " CONNECTION LIMIT = #{value}" else -- cgit v1.2.3 From 683ff235e6b81d28962f5a71ff53730a1c118fc8 Mon Sep 17 00:00:00 2001 From: Patrick Reagan Date: Fri, 22 Aug 2008 12:48:00 +0100 Subject: Ensure t.timestamps respects options. [#828 state:resolved] Signed-off-by: Pratik Naik --- .../connection_adapters/abstract/schema_definitions.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 31d6c7942c..98016ddab5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -443,9 +443,10 @@ module ActiveRecord # Appends :datetime columns :created_at and # :updated_at to the table. - def timestamps - column(:created_at, :datetime) - column(:updated_at, :datetime) + def timestamps(*args) + options = args.extract_options! + column(:created_at, :datetime, options) + column(:updated_at, :datetime, options) end def references(*args) -- cgit v1.2.3 From 707ee0e2695e85186d59aa407f09691ebfcc3125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Fri, 22 Aug 2008 23:53:31 +0300 Subject: Made migrations transactional for PostgreSQL [#834 state:resolved] Patch originally from http://dev.rubyonrails.org/ticket/5470 --- .../connection_adapters/abstract_adapter.rb | 7 ++++++ .../connection_adapters/postgresql_adapter.rb | 4 ++++ activerecord/lib/active_record/migration.rb | 25 ++++++++++++++++++---- 3 files changed, 32 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 47dbf5a5f3..6924bb7e6f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -51,6 +51,13 @@ module ActiveRecord true end + # Does this adapter support DDL rollbacks in transactions? That is, would + # CREATE TABLE or ALTER TABLE get rolled back by a transaction? PostgreSQL, + # SQL Server, and others support this. MySQL and others do not. + def supports_ddl_transactions? + false + end + # Should primary key values be selected from their corresponding # sequence before the insert statement? If true, next_sequence_value # is called before each insert to set the record's primary key. diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 723e1a42dd..bc6fd4e722 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -335,6 +335,10 @@ module ActiveRecord postgresql_version >= 80200 end + def supports_ddl_transactions? + true + end + # Returns the configured supported identifier length supported by PostgreSQL, # or report the default of 63 on PostgreSQL 7.x. def table_alias_length diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index fd77f27b77..c7bc76264d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -461,14 +461,22 @@ module ActiveRecord Base.logger.info "Migrating to #{migration.name} (#{migration.version})" # On our way up, we skip migrating the ones we've already migrated - # On our way down, we skip reverting the ones we've never migrated next if up? && migrated.include?(migration.version.to_i) + # On our way down, we skip reverting the ones we've never migrated if down? && !migrated.include?(migration.version.to_i) migration.announce 'never migrated, skipping'; migration.write - else - migration.migrate(@direction) - record_version_state_after_migrating(migration.version) + next + end + + begin + ddl_transaction do + migration.migrate(@direction) + record_version_state_after_migrating(migration.version) + end + rescue => e + canceled_msg = Base.connection.supports_ddl_transactions? ? "this and " : "" + raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace end end end @@ -531,5 +539,14 @@ module ActiveRecord def down? @direction == :down end + + # Wrap the migration in a transaction only if supported by the adapter. + def ddl_transaction(&block) + if Base.connection.supports_ddl_transactions? + Base.transaction { block.call } + else + block.call + end + end end end -- cgit v1.2.3 From e48e77e0222292176cd9f68658dd54524f582d9b Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 18 Aug 2008 15:56:37 -0700 Subject: coerce blank strings to nil values for boolean and integer fields [#860 state:resolved] --- activerecord/lib/active_record/base.rb | 13 ++++++++----- .../connection_adapters/abstract/schema_definitions.rb | 6 +++++- 2 files changed, 13 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 45d9372842..2e139c5cc0 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2607,11 +2607,14 @@ module ActiveRecord #:nodoc: end def convert_number_column_value(value) - case value - when FalseClass; 0 - when TrueClass; 1 - when ''; nil - else value + if value == false + 0 + elsif value == true + 1 + elsif value.is_a?(String) && value.blank? + nil + else + value end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 98016ddab5..75032efe57 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -138,7 +138,11 @@ module ActiveRecord # convert something to a boolean def value_to_boolean(value) - TRUE_VALUES.include?(value) + if value.is_a?(String) && value.blank? + nil + else + TRUE_VALUES.include?(value) + end end # convert something to a BigDecimal -- cgit v1.2.3 From cf28109158054fbab91de2d6d86efe1b40e68d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Sat, 23 Aug 2008 18:05:52 +0300 Subject: Always require activesupport, even if its constant already exists This is needed because the existance of the ActiveSupport constant by itself does not guarantee that the whole library has been loaded. Also load the StringInquirer in the Rails#env method as the it might be called inside the initializer block before activesupport itself has been loaded. --- activerecord/lib/active_record.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index bc27d17ccd..7612015ca5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -24,16 +24,14 @@ $:.unshift(File.dirname(__FILE__)) unless $:.include?(File.dirname(__FILE__)) || $:.include?(File.expand_path(File.dirname(__FILE__))) -unless defined? ActiveSupport - active_support_path = File.dirname(__FILE__) + "/../../activesupport/lib" - if File.exist?(active_support_path) - $:.unshift active_support_path - require 'active_support' - else - require 'rubygems' - gem 'activesupport' - require 'active_support' - end +active_support_path = File.dirname(__FILE__) + "/../../activesupport/lib" +if File.exist?(active_support_path) + $:.unshift active_support_path + require 'active_support' +else + require 'rubygems' + gem 'activesupport' + require 'active_support' end require 'active_record/base' -- cgit v1.2.3 From 74c3c701f73407a5bb1a11be2b5b221fe39895d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Sat, 23 Aug 2008 19:51:09 +0300 Subject: Don't set "NULL" as a constraint on nullable columns [#398 state:resolved] This is already the default and adding it breaks SQL standards compatibility. --- .../connection_adapters/abstract/schema_statements.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 0f60a91ef1..bececf82a0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -384,12 +384,8 @@ module ActiveRecord def add_column_options!(sql, options) #:nodoc: sql << " DEFAULT #{quote(options[:default], options[:column])}" if options_include_default?(options) # must explicitly check for :null to allow change_column to work on migrations - if options.has_key? :null - if options[:null] == false - sql << " NOT NULL" - else - sql << " NULL" - end + if options[:null] == false + sql << " NOT NULL" end end -- cgit v1.2.3 From c471f13db63844fe290615ed6e1ddca32b26570d Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 23 Aug 2008 21:26:14 -0700 Subject: Ruby 1.9 compat: Hash is now flattenable, so explicitly exclude it --- activerecord/lib/active_record/associations/association_proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 981be3b1a9..99b8748a48 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -213,7 +213,7 @@ module ActiveRecord # Array#flatten has problems with recursive arrays. Going one level deeper solves the majority of the problems. def flatten_deeper(array) - array.collect { |element| element.respond_to?(:flatten) ? element.flatten : element }.flatten + array.collect { |element| (element.respond_to?(:flatten) && !element.is_a?(Hash)) ? element.flatten : element }.flatten end def owner_quoted_id -- cgit v1.2.3 From e02f0dcc24f871d8429229db4219ee1e93636496 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sun, 24 Aug 2008 02:51:45 +0200 Subject: Rollback the transaction when a before_* callback returns false. Previously this would have committed the transaction but not carried out save or destroy operation. [#891 state:committed] Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/callbacks.rb | 12 ++++++++++++ activerecord/lib/active_record/transactions.rb | 16 ++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index be2621fdb6..eec531c514 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -169,6 +169,18 @@ module ActiveRecord # If a before_* callback returns +false+, all the later callbacks and the associated action are cancelled. If an after_* callback returns # +false+, all the later callbacks are cancelled. Callbacks are generally run in the order they are defined, with the exception of callbacks # defined as methods on the model, which are called last. + # + # == Transactions + # + # The entire callback chain of a +save+, save!, or +destroy+ call runs + # within a transaction. That includes after_* hooks. If everything + # goes fine a COMMIT is executed once the chain has been completed. + # + # If a before_* callback cancels the action a ROLLBACK is issued. You + # can also trigger a ROLLBACK raising an exception in any of the callbacks, + # including after_* hooks. Note, however, that in that case the client + # needs to be aware of it because an ordinary +save+ will raise such exception + # instead of quietly returning +false+. module Callbacks CALLBACKS = %w( after_find after_initialize before_save after_save before_create after_create before_update after_update before_validation diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 0531afbb52..81462a2917 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -91,11 +91,11 @@ module ActiveRecord end def destroy_with_transactions #:nodoc: - transaction { destroy_without_transactions } + with_transaction_returning_status(:destroy_without_transactions) end def save_with_transactions(perform_validation = true) #:nodoc: - rollback_active_record_state! { transaction { save_without_transactions(perform_validation) } } + rollback_active_record_state! { with_transaction_returning_status(:save_without_transactions, perform_validation) } end def save_with_transactions! #:nodoc: @@ -118,5 +118,17 @@ module ActiveRecord end raise end + + # Executes +method+ within a transaction and captures its return value as a + # status flag. If the status is true the transaction is committed, otherwise + # a ROLLBACK is issued. In any case the status flag is returned. + def with_transaction_returning_status(method, *args) + status = nil + transaction do + status = send(method, *args) + raise ActiveRecord::Rollback unless status + end + status + end end end -- cgit v1.2.3 From b7a37b742c0abd1df8ea48cc82f76385cc0c41ea Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Mon, 25 Aug 2008 22:36:19 +0100 Subject: Fix preloading of has_one through associations [#903 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/association_preload.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index c7594809b7..61fa34ac39 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -51,9 +51,7 @@ module ActiveRecord def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| - association_proxy = parent_record.send(reflection_name) - association_proxy.loaded - association_proxy.target = associated_record + parent_record.send("set_#{reflection_name}_target", associated_record) end end @@ -112,8 +110,8 @@ module ActiveRecord def preload_has_one_association(records, reflection, preload_options={}) id_to_record_map, ids = construct_id_map(records) options = reflection.options + records.each {|record| record.send("set_#{reflection.name}_target", nil)} if options[:through] - records.each {|record| record.send(reflection.name) && record.send(reflection.name).loaded} through_records = preload_through_records(records, reflection, options[:through]) through_reflection = reflections[options[:through]] through_primary_key = through_reflection.primary_key_name @@ -126,8 +124,6 @@ module ActiveRecord end end else - records.each {|record| record.send("set_#{reflection.name}_target", nil)} - set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.primary_key_name) end end -- cgit v1.2.3 From 172606e21f54fea39af68ede5f55a43deaf3ac68 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 25 Aug 2008 21:22:34 -0700 Subject: Harmonize framework require strategy. Don't add self to load path since Rails initializer and RubyGems handle it. --- activerecord/lib/active_record.rb | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 7612015ca5..e9767c2d5e 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -21,17 +21,14 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -$:.unshift(File.dirname(__FILE__)) unless - $:.include?(File.dirname(__FILE__)) || $:.include?(File.expand_path(File.dirname(__FILE__))) - -active_support_path = File.dirname(__FILE__) + "/../../activesupport/lib" -if File.exist?(active_support_path) - $:.unshift active_support_path - require 'active_support' -else - require 'rubygems' - gem 'activesupport' +begin require 'active_support' +rescue LoadError + activesupport_path = "#{File.dirname(__FILE__)}/../../activesupport/lib" + if File.directory?(activesupport_path) + $:.unshift activesupport_path + require 'active_support' + end end require 'active_record/base' -- cgit v1.2.3 From 2dbda11945507a0541d61d13b8c564121c1cd362 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Mon, 25 Aug 2008 23:20:10 +0100 Subject: Implement old-skool eagerloading for has_one :through Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b9039ce996..46b79c5a0d 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1974,7 +1974,7 @@ module ActiveRecord @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join") end - if reflection.macro == :has_many && reflection.options[:through] + if [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through] @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join") end end @@ -1998,7 +1998,7 @@ module ActiveRecord ] when :has_many, :has_one case - when reflection.macro == :has_many && reflection.options[:through] + when reflection.options[:through] through_conditions = through_reflection.options[:conditions] ? "AND #{interpolate_sql(sanitize_sql(through_reflection.options[:conditions]))}" : '' jt_foreign_key = jt_as_extra = jt_source_extra = jt_sti_extra = nil -- cgit v1.2.3 From a445cdd8840c4e99c40c6d5b15ab380d39a56be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 03:01:24 +0300 Subject: Load the first and not the last has_one result when doing join-based eager loading This matters when the has_one is defined with an order in which case there is an expectation that the first one will be loaded. [#904 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 46b79c5a0d..f915daafba 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1893,6 +1893,7 @@ module ActiveRecord collection.target.push(association) when :has_one return if record.id.to_s != join.parent.record_id(row).to_s + return if record.instance_variable_defined?("@#{join.reflection.name}") association = join.instantiate(row) unless row[join.aliased_primary_key].nil? record.send("set_#{join.reflection.name}_target", association) when :belongs_to -- cgit v1.2.3 From 3d2ac918b987ef0cff34f6a7fdd20926b7a9e5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 04:55:57 +0300 Subject: Cache migrated versions list in Migrator and use it to fetch the latest migrated version name [#845 state:resolved] Also optimized Migrator#current_version class method to fetch only the latest version number and not all of them. With this change no matter how many migrations there are the schema_migrations table is only SELECTed from once. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/migration.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index c7bc76264d..7a1fd7cfbc 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -407,10 +407,9 @@ module ActiveRecord end def current_version - version = Base.connection.select_values( - "SELECT version FROM #{schema_migrations_table_name}" - ).map(&:to_i).max rescue nil - version || 0 + Base.connection.select_value( + "SELECT MAX(CAST(version AS integer)) FROM #{schema_migrations_table_name}" + ).to_i rescue 0 end def proper_table_name(name) @@ -426,7 +425,7 @@ module ActiveRecord end def current_version - self.class.current_version + migrated.last || 0 end def current_migration @@ -518,16 +517,19 @@ module ActiveRecord def migrated sm_table = self.class.schema_migrations_table_name - Base.connection.select_values("SELECT version FROM #{sm_table}").map(&:to_i).sort + @migrated_versions ||= Base.connection.select_values("SELECT version FROM #{sm_table}").map(&:to_i).sort end private def record_version_state_after_migrating(version) sm_table = self.class.schema_migrations_table_name + @migrated_versions ||= [] if down? + @migrated_versions.delete(version.to_i) Base.connection.update("DELETE FROM #{sm_table} WHERE version = '#{version}'") else + @migrated_versions.push(version.to_i).sort! Base.connection.insert("INSERT INTO #{sm_table} (version) VALUES ('#{version}')") end end -- cgit v1.2.3 From 77b003fb615a1a0b197af9fbb9066622bf489b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 09:14:12 +0300 Subject: Use DECIMAL instead of INTEGER when casting as mysql doesn't work with just "INTEGER" and other databases don't like "UNSIGNED" which mysql requires And don't mask exceptions. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/migration.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 7a1fd7cfbc..6d101e9db5 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -407,9 +407,14 @@ module ActiveRecord end def current_version - Base.connection.select_value( - "SELECT MAX(CAST(version AS integer)) FROM #{schema_migrations_table_name}" - ).to_i rescue 0 + sm_table = schema_migrations_table_name + if Base.connection.table_exists?(sm_table) + Base.connection.select_value( + "SELECT MAX(CAST(version AS DECIMAL)) FROM #{sm_table}" + ).to_i + else + 0 + end end def proper_table_name(name) -- cgit v1.2.3 From 143f5fbb21b6dfcaab63d67b44afd922dab9dcf5 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 25 Aug 2008 19:32:19 -0700 Subject: refactor dynamic finder name matching into its own class Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/base.rb | 121 +++++++++------------ .../lib/active_record/dynamic_finder_match.rb | 33 ++++++ 3 files changed, 83 insertions(+), 72 deletions(-) create mode 100644 activerecord/lib/active_record/dynamic_finder_match.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index e9767c2d5e..c47ca486c8 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -51,6 +51,7 @@ require 'active_record/calculations' require 'active_record/serialization' require 'active_record/attribute_methods' require 'active_record/dirty' +require 'active_record/dynamic_finder_match' ActiveRecord::Base.class_eval do extend ActiveRecord::QueryCache diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2e139c5cc0..523b619e62 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1354,8 +1354,8 @@ module ActiveRecord #:nodoc: end def respond_to?(method_id, include_private = false) - if match = matches_dynamic_finder?(method_id) || matches_dynamic_finder_with_initialize_or_create?(method_id) - return true if all_attributes_exists?(extract_attribute_names_from_match(match)) + if match = DynamicFinderMatch.match(method_id) + return true if all_attributes_exists?(match.attribute_names) end super end @@ -1674,88 +1674,65 @@ module ActiveRecord #:nodoc: # Each dynamic finder or initializer/creator is also defined in the class after it is first invoked, so that future # attempts to use it do not run through method_missing. def method_missing(method_id, *arguments) - if match = matches_dynamic_finder?(method_id) - finder = determine_finder(match) - - attribute_names = extract_attribute_names_from_match(match) + if match = DynamicFinderMatch.match(method_id) + attribute_names = match.attribute_names super unless all_attributes_exists?(attribute_names) - - self.class_eval %{ - def self.#{method_id}(*args) - options = args.extract_options! - attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) - finder_options = { :conditions => attributes } - validate_find_options(options) - set_readonly_option!(options) - - if options[:conditions] - with_scope(:find => finder_options) do - ActiveSupport::Deprecation.silence { send(:#{finder}, options) } + if match.finder? + finder = match.finder + self.class_eval %{ + def self.#{method_id}(*args) + options = args.extract_options! + attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) + finder_options = { :conditions => attributes } + validate_find_options(options) + set_readonly_option!(options) + + if options[:conditions] + with_scope(:find => finder_options) do + ActiveSupport::Deprecation.silence { send(:#{finder}, options) } + end + else + ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } end - else - ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } - end - end - }, __FILE__, __LINE__ - send(method_id, *arguments) - elsif match = matches_dynamic_finder_with_initialize_or_create?(method_id) - instantiator = determine_instantiator(match) - attribute_names = extract_attribute_names_from_match(match) - super unless all_attributes_exists?(attribute_names) - - self.class_eval %{ - def self.#{method_id}(*args) - guard_protected_attributes = false - - if args[0].is_a?(Hash) - guard_protected_attributes = true - attributes = args[0].with_indifferent_access - find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}]) - else - find_attributes = attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) end + }, __FILE__, __LINE__ + send(method_id, *arguments) + elsif match.instantiator? + instantiator = match.instantiator + self.class_eval %{ + def self.#{method_id}(*args) + guard_protected_attributes = false + + if args[0].is_a?(Hash) + guard_protected_attributes = true + attributes = args[0].with_indifferent_access + find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}]) + else + find_attributes = attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) + end - options = { :conditions => find_attributes } - set_readonly_option!(options) + options = { :conditions => find_attributes } + set_readonly_option!(options) - record = find_initial(options) + record = find_initial(options) - if record.nil? - record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } - #{'yield(record) if block_given?'} - #{'record.save' if instantiator == :create} - record - else - record + if record.nil? + record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } + #{'yield(record) if block_given?'} + #{'record.save' if instantiator == :create} + record + else + record + end end - end - }, __FILE__, __LINE__ - send(method_id, *arguments) + }, __FILE__, __LINE__ + send(method_id, *arguments) + end else super end end - def matches_dynamic_finder?(method_id) - /^find_(all_by|by)_([_a-zA-Z]\w*)$/.match(method_id.to_s) - end - - def matches_dynamic_finder_with_initialize_or_create?(method_id) - /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/.match(method_id.to_s) - end - - def determine_finder(match) - match.captures.first == 'all_by' ? :find_every : :find_initial - end - - def determine_instantiator(match) - match.captures.first == 'initialize' ? :new : :create - end - - def extract_attribute_names_from_match(match) - match.captures.last.split('_and_') - end - def construct_attributes_from_arguments(attribute_names, arguments) attributes = {} attribute_names.each_with_index { |name, idx| attributes[name] = arguments[idx] } diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb new file mode 100644 index 0000000000..4618e777a4 --- /dev/null +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -0,0 +1,33 @@ +module ActiveRecord + class DynamicFinderMatch + def self.match(method) + df_match = self.new(method) + df_match.finder ? df_match : nil + end + + def initialize(method) + @finder = :find_initial + case method.to_s + when /^find_(all_by|by)_([_a-zA-Z]\w*)$/ + @finder = :find_every if $1 == 'all_by' + names = $2 + when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ + @instantiator = $1 == 'initialize' ? :new : :create + names = $2 + else + @finder = nil + end + @attribute_names = names && names.split('_and_') + end + + attr_reader :finder, :attribute_names, :instantiator + + def finder? + !@finder.nil? && @instantiator.nil? + end + + def instantiator? + @finder == :find_initial && !@instantiator.nil? + end + end +end -- cgit v1.2.3 From 1092c181b5568d06e84f6a3253aaca81c02a2b2c Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 25 Aug 2008 21:28:53 -0700 Subject: add dynamic finder bang version to raise RecordNotFound [#905 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/base.rb | 4 +++- activerecord/lib/active_record/dynamic_finder_match.rb | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 523b619e62..5c30f80555 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1679,6 +1679,7 @@ module ActiveRecord #:nodoc: super unless all_attributes_exists?(attribute_names) if match.finder? finder = match.finder + bang = match.bang? self.class_eval %{ def self.#{method_id}(*args) options = args.extract_options! @@ -1687,13 +1688,14 @@ module ActiveRecord #:nodoc: validate_find_options(options) set_readonly_option!(options) - if options[:conditions] + #{'result = ' if bang}if options[:conditions] with_scope(:find => finder_options) do ActiveSupport::Deprecation.silence { send(:#{finder}, options) } end else ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } end + #{'result || raise(RecordNotFound)' if bang} end }, __FILE__, __LINE__ send(method_id, *arguments) diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 4618e777a4..b105b919f5 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -11,6 +11,9 @@ module ActiveRecord when /^find_(all_by|by)_([_a-zA-Z]\w*)$/ @finder = :find_every if $1 == 'all_by' names = $2 + when /^find_by_([_a-zA-Z]\w*)\!$/ + @bang = true + names = $1 when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ @instantiator = $1 == 'initialize' ? :new : :create names = $2 @@ -29,5 +32,9 @@ module ActiveRecord def instantiator? @finder == :find_initial && !@instantiator.nil? end + + def bang? + @bang + end end end -- cgit v1.2.3 From b319e69ecb06692995664dba263813d99bc2145f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 26 Aug 2008 01:48:41 -0700 Subject: PostgreSQL: pg driver expects nil instead of empty string for missing user/pass --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index bc6fd4e722..55c7da5b4f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -23,8 +23,8 @@ module ActiveRecord config = config.symbolize_keys host = config[:host] port = config[:port] || 5432 - username = config[:username].to_s - password = config[:password].to_s + username = config[:username].to_s if config[:username] + password = config[:password].to_s if config[:password] if config.has_key?(:database) database = config[:database] -- cgit v1.2.3 From 00d2165f748bc3d7c235daa959c54266a0a283db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 12:57:33 +0300 Subject: Back to fetching all versions in ruby instead of letting SQL do it as it's difficult to get all databases to convert the text value to a number with the same SQL Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/migration.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 6d101e9db5..1d843fff28 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -406,12 +406,14 @@ module ActiveRecord Base.table_name_prefix + 'schema_migrations' + Base.table_name_suffix end + def get_all_versions + Base.connection.select_values("SELECT version FROM #{schema_migrations_table_name}").map(&:to_i).sort + end + def current_version sm_table = schema_migrations_table_name if Base.connection.table_exists?(sm_table) - Base.connection.select_value( - "SELECT MAX(CAST(version AS DECIMAL)) FROM #{sm_table}" - ).to_i + get_all_versions.max || 0 else 0 end @@ -521,8 +523,7 @@ module ActiveRecord end def migrated - sm_table = self.class.schema_migrations_table_name - @migrated_versions ||= Base.connection.select_values("SELECT version FROM #{sm_table}").map(&:to_i).sort + @migrated_versions ||= self.class.get_all_versions end private -- cgit v1.2.3 From 3dfecfe77347421fbb54be70a5a84eb47c30acb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 13:41:41 +0300 Subject: Print the queries that were executed if assert_queries fails --- activerecord/lib/active_record/test_case.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index ca5591ae35..ffaa41282f 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -37,7 +37,7 @@ module ActiveRecord $queries_executed = [] yield ensure - assert_equal num, $queries_executed.size, "#{$queries_executed.size} instead of #{num} queries were executed." + assert_equal num, $queries_executed.size, "#{$queries_executed.size} instead of #{num} queries were executed.#{$queries_executed.size == 0 ? '' : "\nQueries:\n#{$queries_executed.join("\n")}"}" end def assert_no_queries(&block) -- cgit v1.2.3 From 8756dd75b257b17ddda92674d4cc0db307d2153b Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 26 Aug 2008 14:24:52 -0500 Subject: Performance: reduce garbage created by ActiveRecord::Calculations#column_alias_for --- activerecord/lib/active_record/calculations.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 246f87b7a9..77cc6da251 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -260,7 +260,14 @@ module ActiveRecord # column_alias_for("count(*)") # => "count_all" # column_alias_for("count", "id") # => "count_id" def column_alias_for(*keys) - connection.table_alias_for(keys.join(' ').downcase.gsub(/\*/, 'all').gsub(/\W+/, ' ').strip.gsub(/ +/, '_')) + table_name = keys.join(' ') + table_name.downcase! + table_name.gsub!(/\*/, 'all') + table_name.gsub!(/\W+/, ' ') + table_name.strip! + table_name.gsub!(/ +/, '_') + + connection.table_alias_for(table_name) end def column_for(field) -- cgit v1.2.3 From 0fcd5b5466461e44b6f3b007fa2a2fdf43f55681 Mon Sep 17 00:00:00 2001 From: Marko Seppae Date: Wed, 27 Aug 2008 10:36:00 +0200 Subject: I18n: removed call to #populate from main library files --- activerecord/lib/active_record.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index c47ca486c8..a6bbd6fc82 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -77,7 +77,5 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_record/schema_dumper' -I18n.backend.populate do - I18n.load_translations File.dirname(__FILE__) + '/active_record/locale/en-US.yml' -end +I18n.load_translations File.dirname(__FILE__) + '/active_record/locale/en-US.yml' -- cgit v1.2.3 From 9dbde4f5cbd0617ee6cce3e41d41335f9c9ce3fd Mon Sep 17 00:00:00 2001 From: pivotal Date: Tue, 26 Aug 2008 09:20:24 -0700 Subject: Fix two has_one :through errors * Set the association target on assignment; * Reset target to nil on reset, rather than empty array. Signed-off-by: Michael Koziarski [#895 state:committed] --- activerecord/lib/active_record/associations.rb | 5 ++--- .../lib/active_record/associations/has_one_through_association.rb | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index f915daafba..4d935612ca 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1270,10 +1270,9 @@ module ActiveRecord association.create_through_record(new_value) self.send(reflection.name, new_value) else - association.replace(new_value) + association.replace(new_value) + instance_variable_set(ivar, new_value.nil? ? nil : association) end - - instance_variable_set(ivar, new_value.nil? ? nil : association) end define_method("set_#{reflection.name}_target") do |target| diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index c846956e1f..b78bd5d931 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -22,6 +22,10 @@ module ActiveRecord def find_target super.first + end + + def reset_target! + @target = nil end end end -- cgit v1.2.3 From a444c782125e10ead6227f7cc57b2f5c739111f2 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 27 Aug 2008 21:32:51 -0700 Subject: respond_to? passes along splat args to avoid introducing the second arg if it was omitted --- activerecord/lib/active_record/associations/association_proxy.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 99b8748a48..5427681f3c 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -69,8 +69,8 @@ module ActiveRecord @target end - def respond_to?(symbol, include_priv = false) - proxy_respond_to?(symbol, include_priv) || (load_target && @target.respond_to?(symbol, include_priv)) + def respond_to?(*args) + proxy_respond_to?(*args) || (load_target && @target.respond_to?(*args)) end # Explicitly proxy === because the instance method removal above -- cgit v1.2.3 From c2068d14d29ec767c681798b3814f0a8e22fb0ff Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 27 Aug 2008 22:49:50 -0700 Subject: PostgreSQL: fix quote_string for certain old pg drivers. [#94 state:resolved] --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 55c7da5b4f..0c2532f21d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -380,7 +380,7 @@ module ActiveRecord # There are some incorrectly compiled postgres drivers out there # that don't define PGconn.escape. self.class.instance_eval do - undef_method(:quote_string) + remove_method(:quote_string) end end quote_string(s) -- cgit v1.2.3 From ad562c58eabfb8b44cb8ac9e87b87a7f998325fd Mon Sep 17 00:00:00 2001 From: Tom Lea Date: Mon, 11 Aug 2008 14:12:53 +0100 Subject: Dirty: treat two changes resulting in the original value as being unchanged. [#798 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/dirty.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb index 63bf8c8f5b..7e246e62ca 100644 --- a/activerecord/lib/active_record/dirty.rb +++ b/activerecord/lib/active_record/dirty.rb @@ -123,7 +123,10 @@ module ActiveRecord attr = attr.to_s # The attribute already has an unsaved change. - unless changed_attributes.include?(attr) + if changed_attributes.include?(attr) + old = changed_attributes[attr] + changed_attributes.delete(attr) unless field_changed?(attr, old, value) + else old = clone_attribute_value(:read_attribute, attr) changed_attributes[attr] = old if field_changed?(attr, old, value) end -- cgit v1.2.3 From 13671cc565aad2327f81a29789154b829ceeda04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 19:00:47 +0300 Subject: Alias included associations if needed when doing a count [#302 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/calculations.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 77cc6da251..08306f361a 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -186,11 +186,17 @@ module ActiveRecord sql << " FROM (SELECT #{distinct}#{column_name}" if use_workaround sql << " FROM #{connection.quote_table_name(table_name)} " end + + joins = "" + add_joins!(joins, options, scope) + if merged_includes.any? - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, options[:joins]) + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins) sql << join_dependency.join_associations.collect{|join| join.association_join }.join end - add_joins!(sql, options, scope) + + sql << joins unless joins.blank? + add_conditions!(sql, options[:conditions], scope) add_limited_ids_condition!(sql, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) -- cgit v1.2.3 From 96c6fe084228d570dad80e3100830edb2bc0448d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarmo=20T=C3=A4nav?= Date: Tue, 26 Aug 2008 19:29:16 +0300 Subject: Implement count limit/offset support for has_many associations [#348 state:resolved] Signed-off-by: Jeremy Kemper --- .../lib/active_record/associations/has_many_association.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index ce62127505..1535995410 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -14,7 +14,16 @@ module ActiveRecord @finder_sql + " AND (#{sanitize_sql(options[:conditions])})" options[:include] ||= @reflection.options[:include] - @reflection.klass.count(column_name, options) + value = @reflection.klass.count(column_name, options) + + limit = @reflection.options[:limit] + offset = @reflection.options[:offset] + + if limit || offset + [ [value - offset.to_i, 0].max, limit.to_i ].min + else + value + end end end -- cgit v1.2.3 From 44af2efa2c7391681968c827ca47201a0a02e974 Mon Sep 17 00:00:00 2001 From: Ernie Miller Date: Thu, 28 Aug 2008 14:01:42 -0400 Subject: Refactored AssociationCollection#count for uniformity and Ruby 1.8.7 support. [#831 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 3 +++ .../associations/association_collection.rb | 29 ++++++++++++++++++++++ .../has_and_belongs_to_many_association.rb | 10 ++++++++ .../associations/has_many_association.rb | 26 ------------------- .../associations/has_many_through_association.rb | 10 -------- 5 files changed, 42 insertions(+), 36 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4d935612ca..98710dee09 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1164,6 +1164,9 @@ module ActiveRecord # If true, duplicate associated objects will be ignored by accessors and query methods. # [:finder_sql] # Overwrite the default generated SQL statement used to fetch the association with a manual statement + # [:counter_sql] + # Specify a complete SQL statement to fetch the size of the association. If :finder_sql is + # specified but not :counter_sql, :counter_sql will be generated by replacing SELECT ... FROM with SELECT COUNT(*) FROM. # [:delete_sql] # Overwrite the default generated SQL statement used to remove links between the associated # classes with a manual statement. diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 9061037b39..5092ccc1dc 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -128,6 +128,35 @@ module ActiveRecord end end + # Count all records using SQL. If the +:counter_sql+ option is set for the association, it will + # be used for the query. If no +:counter_sql+ was supplied, but +:finder_sql+ was set, the + # descendant's +construct_sql+ method will have set :counter_sql automatically. + # Otherwise, construct options and pass them with scope to the target class's +count+. + def count(*args) + if @reflection.options[:counter_sql] + @reflection.klass.count_by_sql(@counter_sql) + else + column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args) + if @reflection.options[:uniq] + # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. + column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" if column_name == :all + options.merge!(:distinct => true) + end + + value = @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) } + + limit = @reflection.options[:limit] + offset = @reflection.options[:offset] + + if limit || offset + [ [value - offset.to_i, 0].max, limit.to_i ].min + else + value + end + end + end + + # Remove +records+ from this association. Does not destroy +records+. def delete(*records) records = flatten_deeper(records) diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index e7e433b6b6..3d689098b5 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -78,6 +78,16 @@ module ActiveRecord end @join_sql = "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}" + + if @reflection.options[:counter_sql] + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + elsif @reflection.options[:finder_sql] + # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */ + @reflection.options[:counter_sql] = @reflection.options[:finder_sql].sub(/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" } + @counter_sql = interpolate_sql(@reflection.options[:counter_sql]) + else + @counter_sql = @finder_sql + end end def construct_scope diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 1535995410..1838021d40 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -1,32 +1,6 @@ module ActiveRecord module Associations class HasManyAssociation < AssociationCollection #:nodoc: - # Count the number of associated records. All arguments are optional. - def count(*args) - if @reflection.options[:counter_sql] - @reflection.klass.count_by_sql(@counter_sql) - elsif @reflection.options[:finder_sql] - @reflection.klass.count_by_sql(@finder_sql) - else - column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args) - options[:conditions] = options[:conditions].blank? ? - @finder_sql : - @finder_sql + " AND (#{sanitize_sql(options[:conditions])})" - options[:include] ||= @reflection.options[:include] - - value = @reflection.klass.count(column_name, options) - - limit = @reflection.options[:limit] - offset = @reflection.options[:offset] - - if limit || offset - [ [value - offset.to_i, 0].max, limit.to_i ].min - else - value - end - end - end - protected def owner_quoted_id if @reflection.options[:primary_key] diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 24b02efc35..84fa900f46 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -31,16 +31,6 @@ module ActiveRecord return count end - def count(*args) - column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args) - if @reflection.options[:uniq] - # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL statement. - column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" if column_name == :all - options.merge!(:distinct => true) - end - @reflection.klass.send(:with_scope, construct_scope) { @reflection.klass.count(column_name, options) } - end - protected def construct_find_options!(options) options[:select] = construct_select(options[:select]) -- cgit v1.2.3 From db22c89543f45d7f27847003af949afa21cb6fa1 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Thu, 28 Aug 2008 17:00:18 +0100 Subject: Merge scoped :joins together instead of overwriting them. May expose scoping bugs in your code! [#501 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 4 +-- activerecord/lib/active_record/base.rb | 37 +++++++++++++++++++------- activerecord/lib/active_record/calculations.rb | 2 +- 3 files changed, 30 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 98710dee09..3ca93db10f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1599,7 +1599,7 @@ module ActiveRecord sql = "SELECT #{column_aliases(join_dependency)} FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " sql << join_dependency.join_associations.collect{|join| join.association_join }.join - add_joins!(sql, options, scope) + add_joins!(sql, options[:joins], scope) add_conditions!(sql, options[:conditions], scope) add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) @@ -1655,7 +1655,7 @@ module ActiveRecord if is_distinct sql << distinct_join_associations.collect { |assoc| assoc.association_join }.join - add_joins!(sql, options, scope) + add_joins!(sql, options[:joins], scope) end add_conditions!(sql, options[:conditions], scope) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5c30f80555..b5ffc471bc 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1549,7 +1549,7 @@ module ActiveRecord #:nodoc: sql = "SELECT #{options[:select] || (scope && scope[:select]) || ((options[:joins] || (scope && scope[:joins])) && quoted_table_name + '.*') || '*'} " sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " - add_joins!(sql, options, scope) + add_joins!(sql, options[:joins], scope) add_conditions!(sql, options[:conditions], scope) add_group!(sql, options[:group], scope) @@ -1565,6 +1565,22 @@ module ActiveRecord #:nodoc: (safe_to_array(first) + safe_to_array(second)).uniq end + def merge_joins(first, second) + if first.is_a?(String) && second.is_a?(String) + "#{first} #{second}" + elsif first.is_a?(String) || second.is_a?(String) + if first.is_a?(String) + join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, second, nil) + "#{first} #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join}" + else + join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, first, nil) + "#{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} #{second}" + end + else + (safe_to_array(first) + safe_to_array(second)).uniq + end + end + # Object#to_a is deprecated, though it does have the desired behavior def safe_to_array(o) case o @@ -1620,16 +1636,15 @@ module ActiveRecord #:nodoc: end # The optional scope argument is for the current :find scope. - def add_joins!(sql, options, scope = :auto) + def add_joins!(sql, joins, scope = :auto) scope = scope(:find) if :auto == scope - [(scope && scope[:joins]), options[:joins]].each do |join| - case join - when Symbol, Hash, Array - join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil) - sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} " - else - sql << " #{join} " - end + merged_joins = scope && scope[:joins] && joins ? merge_joins(scope[:joins], joins) : (joins || scope && scope[:joins]) + case merged_joins + when Symbol, Hash, Array + join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil) + sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} " + when String + sql << " #{merged_joins} " end end @@ -1879,6 +1894,8 @@ module ActiveRecord #:nodoc: hash[method][key] = merge_conditions(params[key], hash[method][key]) elsif key == :include && merge hash[method][key] = merge_includes(hash[method][key], params[key]).uniq + elsif key == :joins && merge + hash[method][key] = merge_joins(params[key], hash[method][key]) else hash[method][key] = hash[method][key] || params[key] end diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 08306f361a..0325a8f8ca 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -188,7 +188,7 @@ module ActiveRecord end joins = "" - add_joins!(joins, options, scope) + add_joins!(joins, options[:joins], scope) if merged_includes.any? join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, joins) -- cgit v1.2.3 From db116a2ed688d36570f412a42e9fc33dcbca56c7 Mon Sep 17 00:00:00 2001 From: Jan De Poorter Date: Mon, 25 Aug 2008 12:37:34 +0200 Subject: Fix NamedScope regex so methods containing "an" get delegated to proxy_found. [#901 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index c99c4beca9..b9b7eb5f00 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -103,7 +103,7 @@ module ActiveRecord attr_reader :proxy_scope, :proxy_options [].methods.each do |m| - unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty?|any?|respond_to?)/ + unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty\?|any\?|respond_to\?)/ delegate m, :to => :proxy_found end end -- cgit v1.2.3 From a9086b3daa0a5174ba2118a2131eb5121f32d41b Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 29 Aug 2008 10:19:26 -0500 Subject: Make query-cache thread-local Signed-off-by: Joshua Peek --- .../connection_adapters/abstract/query_cache.rb | 43 +++++++++++++++------- 1 file changed, 29 insertions(+), 14 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index 2afd6064ad..2fc50b9bfa 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -4,7 +4,6 @@ module ActiveRecord class << self def included(base) base.class_eval do - attr_accessor :query_cache_enabled alias_method_chain :columns, :query_cache alias_method_chain :select_all, :query_cache end @@ -16,7 +15,7 @@ module ActiveRecord method_names.each do |method_name| base.class_eval <<-end_code, __FILE__, __LINE__ def #{method_name}_with_query_dirty(*args) - clear_query_cache if @query_cache_enabled + clear_query_cache if query_cache_enabled #{method_name}_without_query_dirty(*args) end @@ -26,22 +25,38 @@ module ActiveRecord end end + def query_cache_enabled + Thread.current['query_cache_enabled'] + end + + def query_cache_enabled=(flag) + Thread.current['query_cache_enabled'] = flag + end + + def query_cache + Thread.current['query_cache'] + end + + def query_cache=(cache) + Thread.current['query_cache'] = cache + end + # Enable the query cache within the block. def cache - old, @query_cache_enabled = @query_cache_enabled, true - @query_cache ||= {} + old, self.query_cache_enabled = query_cache_enabled, true + self.query_cache ||= {} yield ensure clear_query_cache - @query_cache_enabled = old + self.query_cache_enabled = old end # Disable the query cache within the block. def uncached - old, @query_cache_enabled = @query_cache_enabled, false + old, self.query_cache_enabled = query_cache_enabled, false yield ensure - @query_cache_enabled = old + self.query_cache_enabled = old end # Clears the query cache. @@ -51,11 +66,11 @@ module ActiveRecord # the same SQL query and repeatedly return the same result each time, silently # undermining the randomness you were expecting. def clear_query_cache - @query_cache.clear if @query_cache + query_cache.clear if query_cache end def select_all_with_query_cache(*args) - if @query_cache_enabled + if query_cache_enabled cache_sql(args.first) { select_all_without_query_cache(*args) } else select_all_without_query_cache(*args) @@ -63,8 +78,8 @@ module ActiveRecord end def columns_with_query_cache(*args) - if @query_cache_enabled - @query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args) + if query_cache_enabled + query_cache["SHOW FIELDS FROM #{args.first}"] ||= columns_without_query_cache(*args) else columns_without_query_cache(*args) end @@ -73,11 +88,11 @@ module ActiveRecord private def cache_sql(sql) result = - if @query_cache.has_key?(sql) + if query_cache.has_key?(sql) log_info(sql, "CACHE", 0.0) - @query_cache[sql] + query_cache[sql] else - @query_cache[sql] = yield + query_cache[sql] = yield end if Array === result -- cgit v1.2.3 From 743f0e7114b071bf7786a80227c12dcc7ccee6c1 Mon Sep 17 00:00:00 2001 From: Eugene Pimenov Date: Fri, 29 Aug 2008 14:36:00 +0400 Subject: Make case insensitive validates_uniqueness_of use unicode aware downcase method. Signed-off-by: Michael Koziarski --- activerecord/lib/active_record/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 8fe4336bbc..9ee80e6655 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -664,7 +664,7 @@ module ActiveRecord condition_params = [value] else condition_sql = "LOWER(#{sql_attribute}) #{comparison_operator}" - condition_params = [value.downcase] + condition_params = [value.chars.downcase] end if scope = configuration[:scope] -- cgit v1.2.3 From 6edaa267174dfedaf5b152b9eba25b4eb5e34c99 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 19 Apr 2008 00:24:01 -0500 Subject: Initial conversion to connection pool So far so good, tests still run clean. Next steps: synchronize connection pool access and modification, and change allow_concurrency to simply switch a real lock for a null lock. --- .../abstract/connection_pool.rb | 128 +++++++++++++ .../abstract/connection_specification.rb | 198 +++------------------ .../connection_adapters/abstract_adapter.rb | 1 + 3 files changed, 156 insertions(+), 171 deletions(-) create mode 100644 activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb mode change 100644 => 100755 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb new file mode 100644 index 0000000000..8685988e80 --- /dev/null +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -0,0 +1,128 @@ +module ActiveRecord + module ConnectionAdapters + class ConnectionPool + # Check for activity after at least +verification_timeout+ seconds. + # Defaults to 0 (always check.) + attr_accessor :verification_timeout + attr_reader :active_connections, :spec + + def initialize(spec) + @verification_timeout = 0 + + # The thread id -> adapter cache. + @active_connections = {} + + # The ConnectionSpecification for this pool + @spec = spec + end + + def active_connection_name #:nodoc: + Thread.current.object_id + end + + def active_connection + active_connections[active_connection_name] + end + + # Returns the connection currently associated with the class. This can + # also be used to "borrow" the connection to do database work unrelated + # to any of the specific Active Records. + def connection + if conn = active_connections[active_connection_name] + conn + else + # retrieve_connection sets the cache key. + conn = retrieve_connection + active_connections[active_connection_name] = conn + end + end + + # Clears the cache which maps classes to connections. + def clear_active_connections! + clear_entries!(@active_connections, [active_connection_name]) do |name, conn| + conn.disconnect! + end + end + + # Clears the cache which maps classes + def clear_reloadable_connections! + @active_connections.each do |name, conn| + if conn.requires_reloading? + conn.disconnect! + @active_connections.delete(name) + end + end + end + + # Verify active connections. + def verify_active_connections! #:nodoc: + remove_stale_cached_threads!(@active_connections) do |name, conn| + conn.disconnect! + end + active_connections.each_value do |connection| + connection.verify!(@verification_timeout) + end + end + + def retrieve_connection #:nodoc: + # Name is nil if establish_connection hasn't been called for + # some class along the inheritance chain up to AR::Base yet. + name = active_connection_name + if conn = active_connections[name] + # Verify the connection. + conn.verify!(@verification_timeout) + else + self.connection = spec + conn = active_connections[name] + end + + conn or raise ConnectionNotEstablished + end + + # Returns true if a connection that's accessible to this class has already been opened. + def connected? + active_connections[active_connection_name] ? true : false + end + + def disconnect! + clear_cache!(@active_connections) do |name, conn| + conn.disconnect! + end + end + + # Set the connection for the class. + def connection=(spec) #:nodoc: + if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) + active_connections[active_connection_name] = spec + elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification) + self.connection = ActiveRecord::Base.send(spec.adapter_method, spec.config) + else + raise ConnectionNotEstablished + end + end + + private + def clear_cache!(cache, &block) + cache.each(&block) if block_given? + cache.clear + end + + # Remove stale threads from the cache. + def remove_stale_cached_threads!(cache, &block) + stale = Set.new(cache.keys) + + Thread.list.each do |thread| + stale.delete(thread.object_id) if thread.alive? + end + clear_entries!(cache, stale, &block) + end + + def clear_entries!(cache, keys, &block) + keys.each do |key| + block.call(key, cache[key]) + cache.delete(key) + end + end + end + end +end \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 2a8807fb78..6eee1ac8c1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -14,156 +14,45 @@ module ActiveRecord cattr_accessor :verification_timeout, :instance_writer => false @@verification_timeout = 0 - # The class -> [adapter_method, config] map + # The class -> connection pool map @@defined_connections = {} - # The class -> thread id -> adapter cache. (class -> adapter if not allow_concurrency) - @@active_connections = {} - class << self - # Retrieve the connection cache. - def thread_safe_active_connections #:nodoc: - @@active_connections[Thread.current.object_id] ||= {} - end - - def single_threaded_active_connections #:nodoc: - @@active_connections - end - - # pick up the right active_connection method from @@allow_concurrency - if @@allow_concurrency - alias_method :active_connections, :thread_safe_active_connections - else - alias_method :active_connections, :single_threaded_active_connections - end - - # set concurrency support flag (not thread safe, like most of the methods in this file) - def allow_concurrency=(threaded) #:nodoc: - logger.debug "allow_concurrency=#{threaded}" if logger - return if @@allow_concurrency == threaded - clear_all_cached_connections! - @@allow_concurrency = threaded - method_prefix = threaded ? "thread_safe" : "single_threaded" - sing = (class << self; self; end) - [:active_connections, :scoped_methods].each do |method| - sing.send(:alias_method, method, "#{method_prefix}_#{method}") - end - log_connections if logger - end - - def active_connection_name #:nodoc: - @active_connection_name ||= - if active_connections[name] || @@defined_connections[name] - name - elsif self == ActiveRecord::Base - nil - else - superclass.active_connection_name - end - end - - def clear_active_connection_name #:nodoc: - @active_connection_name = nil - subclasses.each { |klass| klass.clear_active_connection_name } + # for internal use only + def active_connections + @@defined_connections.inject([]) {|arr,kv| arr << kv.last.active_connection}.compact.uniq end # Returns the connection currently associated with the class. This can # also be used to "borrow" the connection to do database work unrelated # to any of the specific Active Records. def connection - if defined?(@active_connection_name) && (conn = active_connections[@active_connection_name]) - conn - else - # retrieve_connection sets the cache key. - conn = retrieve_connection - active_connections[@active_connection_name] = conn - end + retrieve_connection end # Clears the cache which maps classes to connections. def clear_active_connections! - clear_cache!(@@active_connections) do |name, conn| - conn.disconnect! + clear_cache!(@@defined_connections) do |name, pool| + pool.disconnect! end end # Clears the cache which maps classes def clear_reloadable_connections! - if @@allow_concurrency - # With concurrent connections @@active_connections is - # a hash keyed by thread id. - @@active_connections.each do |thread_id, conns| - conns.each do |name, conn| - if conn.requires_reloading? - conn.disconnect! - @@active_connections[thread_id].delete(name) - end - end - end - else - @@active_connections.each do |name, conn| - if conn.requires_reloading? - conn.disconnect! - @@active_connections.delete(name) - end - end + clear_cache!(@@defined_connections) do |name, pool| + pool.clear_reloadable_connections! end end # Verify active connections. def verify_active_connections! #:nodoc: - if @@allow_concurrency - remove_stale_cached_threads!(@@active_connections) do |name, conn| - conn.disconnect! - end - end - - active_connections.each_value do |connection| - connection.verify!(@@verification_timeout) - end + @@defined_connections.each_value {|pool| pool.verify_active_connections!} end private - def clear_cache!(cache, thread_id = nil, &block) - if cache - if @@allow_concurrency - thread_id ||= Thread.current.object_id - thread_cache, cache = cache, cache[thread_id] - return unless cache - end - - cache.each(&block) if block_given? - cache.clear - end - ensure - if thread_cache && @@allow_concurrency - thread_cache.delete(thread_id) - end - end - - # Remove stale threads from the cache. - def remove_stale_cached_threads!(cache, &block) - stale = Set.new(cache.keys) - - Thread.list.each do |thread| - stale.delete(thread.object_id) if thread.alive? - end - - stale.each do |thread_id| - clear_cache!(cache, thread_id, &block) - end - end - - def clear_all_cached_connections! - if @@allow_concurrency - @@active_connections.each_value do |connection_hash_for_thread| - connection_hash_for_thread.each_value {|conn| conn.disconnect! } - connection_hash_for_thread.clear - end - else - @@active_connections.each_value {|conn| conn.disconnect! } - end - @@active_connections.clear + def clear_cache!(cache, &block) + cache.each(&block) if block_given? + cache.clear end end @@ -208,9 +97,7 @@ module ActiveRecord raise AdapterNotSpecified unless defined? RAILS_ENV establish_connection(RAILS_ENV) when ConnectionSpecification - clear_active_connection_name - @active_connection_name = name - @@defined_connections[name] = spec + @@defined_connections[name] = ConnectionAdapters::ConnectionPool.new(spec) when Symbol, String if configuration = configurations[spec.to_s] establish_connection(configuration) @@ -248,26 +135,20 @@ module ActiveRecord # opened and set as the active connection for the class it was defined # for (not necessarily the current class). def self.retrieve_connection #:nodoc: - # Name is nil if establish_connection hasn't been called for - # some class along the inheritance chain up to AR::Base yet. - if name = active_connection_name - if conn = active_connections[name] - # Verify the connection. - conn.verify!(@@verification_timeout) - elsif spec = @@defined_connections[name] - # Activate this connection specification. - klass = name.constantize - klass.connection = spec - conn = active_connections[name] - end - end + pool = retrieve_connection_pool + (pool && pool.connection) or raise ConnectionNotEstablished + end - conn or raise ConnectionNotEstablished + def self.retrieve_connection_pool + pool = @@defined_connections[name] + return pool if pool + return nil if ActiveRecord::Base == self + superclass.retrieve_connection_pool end # Returns true if a connection that's accessible to this class has already been opened. def self.connected? - active_connections[active_connection_name] ? true : false + retrieve_connection_pool.connected? end # Remove the connection for this class. This will close the active @@ -275,35 +156,10 @@ module ActiveRecord # can be used as an argument for establish_connection, for easily # re-establishing the connection. def self.remove_connection(klass=self) - spec = @@defined_connections[klass.name] - konn = active_connections[klass.name] - @@defined_connections.delete_if { |key, value| value == spec } - active_connections.delete_if { |key, value| value == konn } - konn.disconnect! if konn - spec.config if spec - end - - # Set the connection for the class. - def self.connection=(spec) #:nodoc: - if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) - active_connections[name] = spec - elsif spec.kind_of?(ConnectionSpecification) - config = spec.config.reverse_merge(:allow_concurrency => @@allow_concurrency) - self.connection = self.send(spec.adapter_method, config) - elsif spec.nil? - raise ConnectionNotEstablished - else - establish_connection spec - end - end - - # connection state logging - def self.log_connections #:nodoc: - if logger - logger.info "Defined connections: #{@@defined_connections.inspect}" - logger.info "Active connections: #{active_connections.inspect}" - logger.info "Active connection name: #{@active_connection_name}" - end + pool = @@defined_connections[klass.name] + @@defined_connections.delete_if { |key, value| value == pool } + pool.disconnect! if pool + pool.spec.config if pool end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb old mode 100644 new mode 100755 index 6924bb7e6f..af8cfbee7a --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -8,6 +8,7 @@ require 'active_record/connection_adapters/abstract/schema_statements' require 'active_record/connection_adapters/abstract/database_statements' require 'active_record/connection_adapters/abstract/quoting' require 'active_record/connection_adapters/abstract/connection_specification' +require 'active_record/connection_adapters/abstract/connection_pool' require 'active_record/connection_adapters/abstract/query_cache' module ActiveRecord -- cgit v1.2.3 From 5879b15f23f080badedbbab7835eda8c2c748a52 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 19 Apr 2008 00:34:22 -0500 Subject: Rename defined_connections to connection_pools - Distinguis meaning of "active_connections" to always mean connections associated with the current thread --- .../abstract/connection_specification.rb | 30 ++++++++++++++-------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 6eee1ac8c1..b2771c60e7 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -15,12 +15,16 @@ module ActiveRecord @@verification_timeout = 0 # The class -> connection pool map - @@defined_connections = {} + @@connection_pools = {} class << self # for internal use only def active_connections - @@defined_connections.inject([]) {|arr,kv| arr << kv.last.active_connection}.compact.uniq + @@connection_pools.inject({}) do |hash,kv| + hash[kv.first] = kv.last.active_connection + hash.delete(kv.first) unless hash[kv.first] + hash + end end # Returns the connection currently associated with the class. This can @@ -32,21 +36,27 @@ module ActiveRecord # Clears the cache which maps classes to connections. def clear_active_connections! - clear_cache!(@@defined_connections) do |name, pool| - pool.disconnect! + clear_cache!(@@connection_pools) do |name, pool| + pool.clear_active_connections! end end # Clears the cache which maps classes def clear_reloadable_connections! - clear_cache!(@@defined_connections) do |name, pool| + clear_cache!(@@connection_pools) do |name, pool| pool.clear_reloadable_connections! end end + def clear_all_connections! + clear_cache!(@@connection_pools) do |name, pool| + pool.disconnect! + end + end + # Verify active connections. def verify_active_connections! #:nodoc: - @@defined_connections.each_value {|pool| pool.verify_active_connections!} + @@connection_pools.each_value {|pool| pool.verify_active_connections!} end private @@ -97,7 +107,7 @@ module ActiveRecord raise AdapterNotSpecified unless defined? RAILS_ENV establish_connection(RAILS_ENV) when ConnectionSpecification - @@defined_connections[name] = ConnectionAdapters::ConnectionPool.new(spec) + @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) when Symbol, String if configuration = configurations[spec.to_s] establish_connection(configuration) @@ -140,7 +150,7 @@ module ActiveRecord end def self.retrieve_connection_pool - pool = @@defined_connections[name] + pool = @@connection_pools[name] return pool if pool return nil if ActiveRecord::Base == self superclass.retrieve_connection_pool @@ -156,8 +166,8 @@ module ActiveRecord # can be used as an argument for establish_connection, for easily # re-establishing the connection. def self.remove_connection(klass=self) - pool = @@defined_connections[klass.name] - @@defined_connections.delete_if { |key, value| value == pool } + pool = @@connection_pools[klass.name] + @@connection_pools.delete_if { |key, value| value == pool } pool.disconnect! if pool pool.spec.config if pool end -- cgit v1.2.3 From 50cd4bdc99ebaf3ac879e4e7fea43c5b55ca5f68 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 19 Apr 2008 12:42:43 -0500 Subject: Introduce synchronization around connection pool access - use new active support Module#synchronize - allow_concurrency now switches between a null monitor and a regular monitor (defaulting to null monitor to avoid overhead) --- .../abstract/connection_pool.rb | 15 ++++--- .../abstract/connection_specification.rb | 49 ++++++++++++++++------ 2 files changed, 43 insertions(+), 21 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 8685988e80..8f241a39ca 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1,14 +1,12 @@ +require 'set' + module ActiveRecord module ConnectionAdapters class ConnectionPool - # Check for activity after at least +verification_timeout+ seconds. - # Defaults to 0 (always check.) - attr_accessor :verification_timeout + delegate :verification_timeout, :to => "::ActiveRecord::Base" attr_reader :active_connections, :spec def initialize(spec) - @verification_timeout = 0 - # The thread id -> adapter cache. @active_connections = {} @@ -44,7 +42,7 @@ module ActiveRecord end end - # Clears the cache which maps classes + # Clears the cache which maps classes def clear_reloadable_connections! @active_connections.each do |name, conn| if conn.requires_reloading? @@ -60,7 +58,7 @@ module ActiveRecord conn.disconnect! end active_connections.each_value do |connection| - connection.verify!(@verification_timeout) + connection.verify!(verification_timeout) end end @@ -70,7 +68,7 @@ module ActiveRecord name = active_connection_name if conn = active_connections[name] # Verify the connection. - conn.verify!(@verification_timeout) + conn.verify!(verification_timeout) else self.connection = spec conn = active_connections[name] @@ -119,6 +117,7 @@ module ActiveRecord def clear_entries!(cache, keys, &block) keys.each do |key| + next unless cache.has_key?(key) block.call(key, cache[key]) cache.delete(key) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index b2771c60e7..e262b2bac5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -1,4 +1,4 @@ -require 'set' +require 'monitor' module ActiveRecord class Base @@ -9,6 +9,15 @@ module ActiveRecord end end + class NullMonitor + def synchronize + yield + end + end + + cattr_accessor :connection_pools_lock, :instance_writer => false + @@connection_pools_lock = NullMonitor.new + # Check for activity after at least +verification_timeout+ seconds. # Defaults to 0 (always check.) cattr_accessor :verification_timeout, :instance_writer => false @@ -18,8 +27,18 @@ module ActiveRecord @@connection_pools = {} class << self + def allow_concurrency=(flag) + if @@allow_concurrency != flag + if flag + self.connection_pools_lock = Monitor.new + else + self.connection_pools_lock = NullMonitor.new + end + end + end + # for internal use only - def active_connections + def active_connections #:nodoc: @@connection_pools.inject({}) do |hash,kv| hash[kv.first] = kv.last.active_connection hash.delete(kv.first) unless hash[kv.first] @@ -36,22 +55,16 @@ module ActiveRecord # Clears the cache which maps classes to connections. def clear_active_connections! - clear_cache!(@@connection_pools) do |name, pool| - pool.clear_active_connections! - end + @@connection_pools.each_value {|pool| pool.clear_active_connections! } end - # Clears the cache which maps classes + # Clears the cache which maps classes def clear_reloadable_connections! - clear_cache!(@@connection_pools) do |name, pool| - pool.clear_reloadable_connections! - end + @@connection_pools.each_value {|pool| pool.clear_reloadable_connections! } end def clear_all_connections! - clear_cache!(@@connection_pools) do |name, pool| - pool.disconnect! - end + clear_cache!(@@connection_pools) {|name, pool| pool.disconnect! } end # Verify active connections. @@ -59,6 +72,9 @@ module ActiveRecord @@connection_pools.each_value {|pool| pool.verify_active_connections!} end + synchronize :active_connections, :clear_active_connections!, :clear_reloadable_connections!, + :clear_all_connections!, :verify_active_connections!, :with => :connection_pools_lock + private def clear_cache!(cache, &block) cache.each(&block) if block_given? @@ -107,7 +123,9 @@ module ActiveRecord raise AdapterNotSpecified unless defined? RAILS_ENV establish_connection(RAILS_ENV) when ConnectionSpecification - @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) + connection_pools_lock.synchronize do + @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) + end when Symbol, String if configuration = configurations[spec.to_s] establish_connection(configuration) @@ -171,5 +189,10 @@ module ActiveRecord pool.disconnect! if pool pool.spec.config if pool end + + class << self + synchronize :retrieve_connection, :retrieve_connection_pool, :connected?, + :remove_connection, :with => :connection_pools_lock + end end end -- cgit v1.2.3 From cab76ce6ac2983f59451e2d53b23746a2873aea0 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 19 Apr 2008 14:42:56 -0500 Subject: Add synchronization to connection pool also --- .../connection_adapters/abstract/connection_pool.rb | 14 +++++++++++--- .../abstract/connection_specification.rb | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 8f241a39ca..2d13d02fad 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -12,6 +12,9 @@ module ActiveRecord # The ConnectionSpecification for this pool @spec = spec + + # The mutex used to synchronize pool access + @connection_mutex = Monitor.new end def active_connection_name #:nodoc: @@ -70,7 +73,7 @@ module ActiveRecord # Verify the connection. conn.verify!(verification_timeout) else - self.connection = spec + self.set_connection spec conn = active_connections[name] end @@ -82,6 +85,7 @@ module ActiveRecord active_connections[active_connection_name] ? true : false end + # Disconnect all connections in the pool. def disconnect! clear_cache!(@active_connections) do |name, conn| conn.disconnect! @@ -89,16 +93,20 @@ module ActiveRecord end # Set the connection for the class. - def connection=(spec) #:nodoc: + def set_connection(spec) #:nodoc: if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) active_connections[active_connection_name] = spec elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification) - self.connection = ActiveRecord::Base.send(spec.adapter_method, spec.config) + self.set_connection ActiveRecord::Base.send(spec.adapter_method, spec.config) else raise ConnectionNotEstablished end end + synchronize :active_connection, :connection, :clear_active_connections!, + :clear_reloadable_connections!, :verify_active_connections!, :retrieve_connection, + :connected?, :disconnect!, :set_connection, :with => :@connection_mutex + private def clear_cache!(cache, &block) cache.each(&block) if block_given? diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index e262b2bac5..ed9d074506 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -27,6 +27,8 @@ module ActiveRecord @@connection_pools = {} class << self + # Turning on allow_concurrency basically switches a null mutex for a real one, so that + # multi-threaded access of the connection pools hash is synchronized. def allow_concurrency=(flag) if @@allow_concurrency != flag if flag @@ -37,7 +39,7 @@ module ActiveRecord end end - # for internal use only + # for internal use only and for testing def active_connections #:nodoc: @@connection_pools.inject({}) do |hash,kv| hash[kv.first] = kv.last.active_connection -- cgit v1.2.3 From 37b0b36918f14519b28326057bba38ca6fcfbd3b Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 19 Apr 2008 23:11:28 -0500 Subject: Fix failure to retain value of allow_concurrency - Also carry allow_concurrency value through to connection adapter (for postgresql) --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 3 ++- .../connection_adapters/abstract/connection_specification.rb | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 2d13d02fad..08fc61daaa 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -97,7 +97,8 @@ module ActiveRecord if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) active_connections[active_connection_name] = spec elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification) - self.set_connection ActiveRecord::Base.send(spec.adapter_method, spec.config) + config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) + self.set_connection ActiveRecord::Base.send(spec.adapter_method, config) else raise ConnectionNotEstablished end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index ed9d074506..ddca97c3bf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -31,6 +31,7 @@ module ActiveRecord # multi-threaded access of the connection pools hash is synchronized. def allow_concurrency=(flag) if @@allow_concurrency != flag + @@allow_concurrency = flag if flag self.connection_pools_lock = Monitor.new else -- cgit v1.2.3 From ff97e9d029d6164fa2e921a5d0acab13f39058b0 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Sat, 7 Jun 2008 00:30:15 -0500 Subject: Connection handling methods extracted out into separate ConnectionHandler class - delegating methods left behind --- .../abstract/connection_pool.rb | 89 ++++++++++++++ .../abstract/connection_specification.rb | 131 +++++---------------- .../connection_adapters/abstract_adapter.rb | 2 +- 3 files changed, 117 insertions(+), 105 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 08fc61daaa..01a75365d3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1,3 +1,4 @@ +require 'monitor' require 'set' module ActiveRecord @@ -132,5 +133,93 @@ module ActiveRecord end end end + + class ConnectionHandler + attr_reader :connection_pools_lock + + def initialize + @connection_pools = {} + @connection_pools_lock = Monitor.new + end + + def establish_connection(name, spec) + connection_pools_lock.synchronize do + @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) + end + end + + # for internal use only and for testing + def active_connections #:nodoc: + @connection_pools.inject({}) do |hash,kv| + hash[kv.first] = kv.last.active_connection + hash.delete(kv.first) unless hash[kv.first] + hash + end + end + + # Clears the cache which maps classes to connections. + def clear_active_connections! + @connection_pools.each_value {|pool| pool.clear_active_connections! } + end + + # Clears the cache which maps classes + def clear_reloadable_connections! + @connection_pools.each_value {|pool| pool.clear_reloadable_connections! } + end + + def clear_all_connections! + clear_cache!(@connection_pools) {|name, pool| pool.disconnect! } + end + + # Verify active connections. + def verify_active_connections! #:nodoc: + @connection_pools.each_value {|pool| pool.verify_active_connections!} + end + + # Locate the connection of the nearest super class. This can be an + # active or defined connection: if it is the latter, it will be + # opened and set as the active connection for the class it was defined + # for (not necessarily the current class). + def retrieve_connection(klass) #:nodoc: + pool = retrieve_connection_pool(klass) + (pool && pool.connection) or raise ConnectionNotEstablished + end + + def retrieve_connection_pool(klass) + loop do + pool = @connection_pools[klass.name] + return pool if pool + return nil if ActiveRecord::Base == klass + klass = klass.superclass + end + end + + # Returns true if a connection that's accessible to this class has already been opened. + def connected?(klass) + retrieve_connection_pool(klass).connected? + end + + # Remove the connection for this class. This will close the active + # connection and the defined connection (if they exist). The result + # can be used as an argument for establish_connection, for easily + # re-establishing the connection. + def remove_connection(klass) + pool = @connection_pools[klass.name] + @connection_pools.delete_if { |key, value| value == pool } + pool.disconnect! if pool + pool.spec.config if pool + end + + synchronize :retrieve_connection, :retrieve_connection_pool, :connected?, + :remove_connection, :active_connections, :clear_active_connections!, + :clear_reloadable_connections!, :clear_all_connections!, + :verify_active_connections!, :with => :connection_pools_lock + + private + def clear_cache!(cache, &block) + cache.each(&block) if block_given? + cache.clear + end + end end end \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index ddca97c3bf..2dc3201208 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -1,5 +1,3 @@ -require 'monitor' - module ActiveRecord class Base class ConnectionSpecification #:nodoc: @@ -9,80 +7,19 @@ module ActiveRecord end end - class NullMonitor - def synchronize - yield - end - end - - cattr_accessor :connection_pools_lock, :instance_writer => false - @@connection_pools_lock = NullMonitor.new - # Check for activity after at least +verification_timeout+ seconds. # Defaults to 0 (always check.) cattr_accessor :verification_timeout, :instance_writer => false @@verification_timeout = 0 - # The class -> connection pool map - @@connection_pools = {} - - class << self - # Turning on allow_concurrency basically switches a null mutex for a real one, so that - # multi-threaded access of the connection pools hash is synchronized. - def allow_concurrency=(flag) - if @@allow_concurrency != flag - @@allow_concurrency = flag - if flag - self.connection_pools_lock = Monitor.new - else - self.connection_pools_lock = NullMonitor.new - end - end - end - - # for internal use only and for testing - def active_connections #:nodoc: - @@connection_pools.inject({}) do |hash,kv| - hash[kv.first] = kv.last.active_connection - hash.delete(kv.first) unless hash[kv.first] - hash - end - end - - # Returns the connection currently associated with the class. This can - # also be used to "borrow" the connection to do database work unrelated - # to any of the specific Active Records. - def connection - retrieve_connection - end - - # Clears the cache which maps classes to connections. - def clear_active_connections! - @@connection_pools.each_value {|pool| pool.clear_active_connections! } - end - - # Clears the cache which maps classes - def clear_reloadable_connections! - @@connection_pools.each_value {|pool| pool.clear_reloadable_connections! } - end + # The connection handler + cattr_accessor :connection_handler, :instance_writer => false + @@connection_handler = ConnectionAdapters::ConnectionHandler.new - def clear_all_connections! - clear_cache!(@@connection_pools) {|name, pool| pool.disconnect! } - end - - # Verify active connections. - def verify_active_connections! #:nodoc: - @@connection_pools.each_value {|pool| pool.verify_active_connections!} - end - - synchronize :active_connections, :clear_active_connections!, :clear_reloadable_connections!, - :clear_all_connections!, :verify_active_connections!, :with => :connection_pools_lock - - private - def clear_cache!(cache, &block) - cache.each(&block) if block_given? - cache.clear - end + # Turning on allow_concurrency basically switches a null mutex for a real one, so that + # multi-threaded access of the connection pools hash is synchronized. + def self.allow_concurrency=(flag) + @@allow_concurrency = flag end # Returns the connection currently associated with the class. This can @@ -126,9 +63,7 @@ module ActiveRecord raise AdapterNotSpecified unless defined? RAILS_ENV establish_connection(RAILS_ENV) when ConnectionSpecification - connection_pools_lock.synchronize do - @@connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) - end + @@connection_handler.establish_connection(name, spec) when Symbol, String if configuration = configurations[spec.to_s] establish_connection(configuration) @@ -161,41 +96,29 @@ module ActiveRecord end end - # Locate the connection of the nearest super class. This can be an - # active or defined connection: if it is the latter, it will be - # opened and set as the active connection for the class it was defined - # for (not necessarily the current class). - def self.retrieve_connection #:nodoc: - pool = retrieve_connection_pool - (pool && pool.connection) or raise ConnectionNotEstablished - end + class << self + # Returns the connection currently associated with the class. This can + # also be used to "borrow" the connection to do database work unrelated + # to any of the specific Active Records. + def connection + retrieve_connection + end - def self.retrieve_connection_pool - pool = @@connection_pools[name] - return pool if pool - return nil if ActiveRecord::Base == self - superclass.retrieve_connection_pool - end + def retrieve_connection + connection_handler.retrieve_connection(self) + end - # Returns true if a connection that's accessible to this class has already been opened. - def self.connected? - retrieve_connection_pool.connected? - end + def connected? + connection_handler.connected?(self) + end - # Remove the connection for this class. This will close the active - # connection and the defined connection (if they exist). The result - # can be used as an argument for establish_connection, for easily - # re-establishing the connection. - def self.remove_connection(klass=self) - pool = @@connection_pools[klass.name] - @@connection_pools.delete_if { |key, value| value == pool } - pool.disconnect! if pool - pool.spec.config if pool - end + def remove_connection(klass=self) + connection_handler.remove_connection(klass) + end - class << self - synchronize :retrieve_connection, :retrieve_connection_pool, :connected?, - :remove_connection, :with => :connection_pools_lock + delegate :active_connections, :clear_active_connections!, + :clear_reloadable_connections!, :clear_all_connections!, + :verify_active_connections!, :to => :connection_handler end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index af8cfbee7a..4b78d9f2e9 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -7,8 +7,8 @@ require 'active_record/connection_adapters/abstract/schema_definitions' require 'active_record/connection_adapters/abstract/schema_statements' require 'active_record/connection_adapters/abstract/database_statements' require 'active_record/connection_adapters/abstract/quoting' -require 'active_record/connection_adapters/abstract/connection_specification' require 'active_record/connection_adapters/abstract/connection_pool' +require 'active_record/connection_adapters/abstract/connection_specification' require 'active_record/connection_adapters/abstract/query_cache' module ActiveRecord -- cgit v1.2.3 From 72d959d9b5255a449a554a1f011386d3c7a568cf Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Sat, 7 Jun 2008 16:30:56 -0500 Subject: Split connection handler into single- and multiple-thread versions. --- .../abstract/connection_pool.rb | 66 +++++++++++++--------- .../abstract/connection_specification.rb | 22 ++++++-- 2 files changed, 57 insertions(+), 31 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 01a75365d3..ca9f0a5d9b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -134,18 +134,17 @@ module ActiveRecord end end - class ConnectionHandler - attr_reader :connection_pools_lock + module ConnectionHandlerMethods + def initialize(pools = {}) + @connection_pools = pools + end - def initialize - @connection_pools = {} - @connection_pools_lock = Monitor.new + def connection_pools + @connection_pools ||= {} end def establish_connection(name, spec) - connection_pools_lock.synchronize do - @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) - end + @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) end # for internal use only and for testing @@ -168,7 +167,7 @@ module ActiveRecord end def clear_all_connections! - clear_cache!(@connection_pools) {|name, pool| pool.disconnect! } + @connection_pools.each_value {|pool| pool.disconnect! } end # Verify active connections. @@ -185,15 +184,6 @@ module ActiveRecord (pool && pool.connection) or raise ConnectionNotEstablished end - def retrieve_connection_pool(klass) - loop do - pool = @connection_pools[klass.name] - return pool if pool - return nil if ActiveRecord::Base == klass - klass = klass.superclass - end - end - # Returns true if a connection that's accessible to this class has already been opened. def connected?(klass) retrieve_connection_pool(klass).connected? @@ -210,16 +200,40 @@ module ActiveRecord pool.spec.config if pool end - synchronize :retrieve_connection, :retrieve_connection_pool, :connected?, - :remove_connection, :active_connections, :clear_active_connections!, - :clear_reloadable_connections!, :clear_all_connections!, - :verify_active_connections!, :with => :connection_pools_lock - private - def clear_cache!(cache, &block) - cache.each(&block) if block_given? - cache.clear + def retrieve_connection_pool(klass) + loop do + pool = @connection_pools[klass.name] + return pool if pool + return nil if ActiveRecord::Base == klass + klass = klass.superclass + end end end + + # This connection handler is not thread-safe, as it does not protect access + # to the underlying connection pools. + class SingleThreadConnectionHandler + include ConnectionHandlerMethods + end + + # This connection handler is thread-safe. Each access or modification of a thread + # pool is synchronized by an internal monitor. + class MultipleThreadConnectionHandler + attr_reader :connection_pools_lock + include ConnectionHandlerMethods + + def initialize(pools = {}) + super + @connection_pools_lock = Monitor.new + end + + # Apply monitor to all public methods that access the pool. + synchronize :establish_connection, :retrieve_connection, + :connected?, :remove_connection, :active_connections, + :clear_active_connections!, :clear_reloadable_connections!, + :clear_all_connections!, :verify_active_connections!, + :with => :connection_pools_lock + end end end \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 2dc3201208..0ebb191381 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -14,12 +14,24 @@ module ActiveRecord # The connection handler cattr_accessor :connection_handler, :instance_writer => false - @@connection_handler = ConnectionAdapters::ConnectionHandler.new + @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new - # Turning on allow_concurrency basically switches a null mutex for a real one, so that - # multi-threaded access of the connection pools hash is synchronized. + # Turning on allow_concurrency changes the single threaded connection handler + # for a multiple threaded one, so that multi-threaded access of the + # connection pools is synchronized. def self.allow_concurrency=(flag) - @@allow_concurrency = flag + if @@allow_concurrency != flag + @@allow_concurrency = flag + # When switching connection handlers, preserve the existing pools so that + # #establish_connection doesn't need to be called again. + if @@allow_concurrency + self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new( + self.connection_handler.connection_pools) + else + self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new( + self.connection_handler.connection_pools) + end + end end # Returns the connection currently associated with the class. This can @@ -112,7 +124,7 @@ module ActiveRecord connection_handler.connected?(self) end - def remove_connection(klass=self) + def remove_connection(klass = self) connection_handler.remove_connection(klass) end -- cgit v1.2.3 From 029952edf464b94184d9b48f3bdff49d2746d721 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Mon, 4 Aug 2008 22:43:32 -0700 Subject: Extract a base class for connection pools, start to flesh out reserve/release API --- .../abstract/connection_pool.rb | 189 ++++++++++++--------- 1 file changed, 105 insertions(+), 84 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index ca9f0a5d9b..cebff0262d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -3,135 +3,156 @@ require 'set' module ActiveRecord module ConnectionAdapters + # Connection pool API for ActiveRecord database connections. class ConnectionPool - delegate :verification_timeout, :to => "::ActiveRecord::Base" - attr_reader :active_connections, :spec + # Factory method for connection pools. + # Determines pool type to use based on contents of connection specification. + # FIXME: specification configuration TBD. + def self.create(spec) + ConnectionPerThread.new(spec) + end + delegate :verification_timeout, :to => "::ActiveRecord::Base" + attr_reader :spec def initialize(spec) - # The thread id -> adapter cache. - @active_connections = {} - - # The ConnectionSpecification for this pool @spec = spec - + # The cache of reserved connections mapped to threads + @reserved_connections = {} # The mutex used to synchronize pool access @connection_mutex = Monitor.new end - def active_connection_name #:nodoc: - Thread.current.object_id + # Retrieve the connection reserved for the current thread, or call #reserve to obtain one + # if necessary. + def open_connection + if conn = @reserved_connections[active_connection_name] + conn.verify!(verification_timeout) + conn + else + @reserved_connections[active_connection_name] = reserve + end end + alias connection open_connection - def active_connection - active_connections[active_connection_name] + def close_connection + conn = @reserved_connections.delete(active_connection_name) + release conn if conn end - # Returns the connection currently associated with the class. This can - # also be used to "borrow" the connection to do database work unrelated - # to any of the specific Active Records. - def connection - if conn = active_connections[active_connection_name] - conn - else - # retrieve_connection sets the cache key. - conn = retrieve_connection - active_connections[active_connection_name] = conn - end + # Returns true if a connection has already been opened. + def connected? + !connections.empty? end - # Clears the cache which maps classes to connections. - def clear_active_connections! - clear_entries!(@active_connections, [active_connection_name]) do |name, conn| + # Reserve (check-out) a database connection for the current thread. + def reserve + raise NotImplementedError, "reserve is an abstract method" + end + alias checkout reserve + + # Release (check-in) a database connection for the current thread. + def release(connection) + raise NotImplementedError, "release is an abstract method" + end + alias checkin release + + # Disconnect all connections in the pool. + def disconnect! + @reserved_connections.each do |name,conn| + release(conn) + end + connections.each do |conn| conn.disconnect! end + @reserved_connections = {} end # Clears the cache which maps classes def clear_reloadable_connections! - @active_connections.each do |name, conn| + @reserved_connections.each do |name, conn| + release(conn) + end + @reserved_connections = {} + connections.each do |conn| if conn.requires_reloading? conn.disconnect! - @active_connections.delete(name) + remove_connection conn end end end # Verify active connections. def verify_active_connections! #:nodoc: - remove_stale_cached_threads!(@active_connections) do |name, conn| - conn.disconnect! + remove_stale_cached_threads!(@reserved_connections) do |name, conn| + release(conn) end - active_connections.each_value do |connection| + connections.each do |connection| connection.verify!(verification_timeout) end end - def retrieve_connection #:nodoc: - # Name is nil if establish_connection hasn't been called for - # some class along the inheritance chain up to AR::Base yet. - name = active_connection_name - if conn = active_connections[name] - # Verify the connection. - conn.verify!(verification_timeout) - else - self.set_connection spec - conn = active_connections[name] - end + synchronize :open_connection, :close_connection, :reserve, :release, + :clear_reloadable_connections!, :verify_active_connections!, + :connected?, :disconnect!, :with => :@connection_mutex - conn or raise ConnectionNotEstablished + private + def active_connection_name #:nodoc: + Thread.current.object_id end - # Returns true if a connection that's accessible to this class has already been opened. - def connected? - active_connections[active_connection_name] ? true : false + def remove_connection(conn) + raise NotImplementedError, "remove_connection is an abstract method" end - # Disconnect all connections in the pool. - def disconnect! - clear_cache!(@active_connections) do |name, conn| - conn.disconnect! - end + # Array containing all connections (reserved or available) in the pool. + def connections + raise NotImplementedError, "connections is an abstract method" end - # Set the connection for the class. - def set_connection(spec) #:nodoc: - if spec.kind_of?(ActiveRecord::ConnectionAdapters::AbstractAdapter) - active_connections[active_connection_name] = spec - elsif spec.kind_of?(ActiveRecord::Base::ConnectionSpecification) - config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) - self.set_connection ActiveRecord::Base.send(spec.adapter_method, config) - else - raise ConnectionNotEstablished + # Remove stale threads from the cache. + def remove_stale_cached_threads!(cache, &block) + keys = Set.new(cache.keys) + + Thread.list.each do |thread| + keys.delete(thread.object_id) if thread.alive? + end + keys.each do |key| + next unless cache.has_key?(key) + block.call(key, cache[key]) + cache.delete(key) end end + end + + class ConnectionPerThread < ConnectionPool + def active_connection + @reserved_connections[active_connection_name] + end - synchronize :active_connection, :connection, :clear_active_connections!, - :clear_reloadable_connections!, :verify_active_connections!, :retrieve_connection, - :connected?, :disconnect!, :set_connection, :with => :@connection_mutex + def active_connections; @reserved_connections; end - private - def clear_cache!(cache, &block) - cache.each(&block) if block_given? - cache.clear - end + def reserve + new_connection + end - # Remove stale threads from the cache. - def remove_stale_cached_threads!(cache, &block) - stale = Set.new(cache.keys) + def release(conn) + conn.disconnect! + end - Thread.list.each do |thread| - stale.delete(thread.object_id) if thread.alive? - end - clear_entries!(cache, stale, &block) - end + private + # Set the connection for the class. + def new_connection + config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) + ActiveRecord::Base.send(spec.adapter_method, config) + end - def clear_entries!(cache, keys, &block) - keys.each do |key| - next unless cache.has_key?(key) - block.call(key, cache[key]) - cache.delete(key) - end - end + def connections + @reserved_connections.values + end + + def remove_connection(conn) + @reserved_connections.delete_if {|k,v| v == conn} + end end module ConnectionHandlerMethods @@ -144,7 +165,7 @@ module ActiveRecord end def establish_connection(name, spec) - @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) + @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) end # for internal use only and for testing @@ -158,7 +179,7 @@ module ActiveRecord # Clears the cache which maps classes to connections. def clear_active_connections! - @connection_pools.each_value {|pool| pool.clear_active_connections! } + @connection_pools.each_value {|pool| pool.close_connection } end # Clears the cache which maps classes -- cgit v1.2.3 From 82fcd9d85fe245e8041f8d375175dde13688fce4 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Wed, 6 Aug 2008 22:54:06 -0700 Subject: Clean up the code, get rid of reserve/release, add some more docs --- .../abstract/connection_pool.rb | 85 +++++++++++++--------- 1 file changed, 51 insertions(+), 34 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index cebff0262d..28446fd64d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -3,7 +3,7 @@ require 'set' module ActiveRecord module ConnectionAdapters - # Connection pool API for ActiveRecord database connections. + # Connection pool base class and ActiveRecord database connections. class ConnectionPool # Factory method for connection pools. # Determines pool type to use based on contents of connection specification. @@ -14,6 +14,7 @@ module ActiveRecord delegate :verification_timeout, :to => "::ActiveRecord::Base" attr_reader :spec + def initialize(spec) @spec = spec # The cache of reserved connections mapped to threads @@ -22,21 +23,35 @@ module ActiveRecord @connection_mutex = Monitor.new end - # Retrieve the connection reserved for the current thread, or call #reserve to obtain one - # if necessary. - def open_connection + # Retrieve the connection associated with the current thread, or call + # #checkout to obtain one if necessary. + # + # #connection can be called any number of times; the connection is + # held in a hash keyed by the thread id. + def connection if conn = @reserved_connections[active_connection_name] conn.verify!(verification_timeout) conn else - @reserved_connections[active_connection_name] = reserve + @reserved_connections[active_connection_name] = checkout end end - alias connection open_connection - def close_connection + # Signal that the thread is finished with the current connection. + # #release_thread_connection releases the connection-thread association + # and returns the connection to the pool. + def release_thread_connection conn = @reserved_connections.delete(active_connection_name) - release conn if conn + checkin conn if conn + end + + # Reserve a connection, and yield it to a block. Ensure the connection is + # checked back in when finished. + def with_connection + conn = checkout + yield conn + ensure + checkin conn end # Returns true if a connection has already been opened. @@ -44,22 +59,10 @@ module ActiveRecord !connections.empty? end - # Reserve (check-out) a database connection for the current thread. - def reserve - raise NotImplementedError, "reserve is an abstract method" - end - alias checkout reserve - - # Release (check-in) a database connection for the current thread. - def release(connection) - raise NotImplementedError, "release is an abstract method" - end - alias checkin release - # Disconnect all connections in the pool. def disconnect! @reserved_connections.each do |name,conn| - release(conn) + checkin conn end connections.each do |conn| conn.disconnect! @@ -70,7 +73,7 @@ module ActiveRecord # Clears the cache which maps classes def clear_reloadable_connections! @reserved_connections.each do |name, conn| - release(conn) + checkin conn end @reserved_connections = {} connections.each do |conn| @@ -84,30 +87,42 @@ module ActiveRecord # Verify active connections. def verify_active_connections! #:nodoc: remove_stale_cached_threads!(@reserved_connections) do |name, conn| - release(conn) + checkin conn end connections.each do |connection| connection.verify!(verification_timeout) end end - synchronize :open_connection, :close_connection, :reserve, :release, - :clear_reloadable_connections!, :verify_active_connections!, - :connected?, :disconnect!, :with => :@connection_mutex + # Check-out a database connection from the pool. + def checkout + raise NotImplementedError, "checkout is an abstract method" + end - private - def active_connection_name #:nodoc: - Thread.current.object_id + # Check-in a database connection back into the pool. + def checkin(connection) + raise NotImplementedError, "checkin is an abstract method" end def remove_connection(conn) raise NotImplementedError, "remove_connection is an abstract method" end + private :remove_connection # Array containing all connections (reserved or available) in the pool. def connections raise NotImplementedError, "connections is an abstract method" end + private :connections + + synchronize :connection, :release_thread_connection, :checkout, :checkin, + :clear_reloadable_connections!, :verify_active_connections!, + :connected?, :disconnect!, :with => :@connection_mutex + + private + def active_connection_name #:nodoc: + Thread.current.object_id + end # Remove stale threads from the cache. def remove_stale_cached_threads!(cache, &block) @@ -131,11 +146,11 @@ module ActiveRecord def active_connections; @reserved_connections; end - def reserve + def checkout new_connection end - def release(conn) + def checkin(conn) conn.disconnect! end @@ -168,7 +183,8 @@ module ActiveRecord @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) end - # for internal use only and for testing + # for internal use only and for testing; + # only works with ConnectionPerThread pool class def active_connections #:nodoc: @connection_pools.inject({}) do |hash,kv| hash[kv.first] = kv.last.active_connection @@ -179,7 +195,7 @@ module ActiveRecord # Clears the cache which maps classes to connections. def clear_active_connections! - @connection_pools.each_value {|pool| pool.close_connection } + @connection_pools.each_value {|pool| pool.release_thread_connection } end # Clears the cache which maps classes @@ -205,7 +221,8 @@ module ActiveRecord (pool && pool.connection) or raise ConnectionNotEstablished end - # Returns true if a connection that's accessible to this class has already been opened. + # Returns true if a connection that's accessible to this class has + # already been opened. def connected?(klass) retrieve_connection_pool(klass).connected? end -- cgit v1.2.3 From fe575dd4a9f0fa0e71a89fae9f4a951a9fb36058 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 7 Aug 2008 01:27:40 -0700 Subject: Nearing the finish line. Initial fixed-size connection pool implemented, more docs --- .../abstract/connection_pool.rb | 138 +++++++++++++++++---- .../abstract/connection_specification.rb | 4 + 2 files changed, 120 insertions(+), 22 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 28446fd64d..97c6e3e886 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -2,14 +2,43 @@ require 'monitor' require 'set' module ActiveRecord + # Raised when a connection could not be obtained within the connection + # acquisition timeout period. + class ConnectionTimeoutError < ConnectionNotEstablished + end + module ConnectionAdapters - # Connection pool base class and ActiveRecord database connections. - class ConnectionPool + # Connection pool base class for managing ActiveRecord database + # connections. + # + # Connections can be obtained and used from a connection pool in several + # ways: + # + # 1. Simply use ActiveRecord::Base.connection as in pre-connection-pooled + # ActiveRecord. Eventually, when you're done with the connection and + # wish it to be returned to the pool, you call + # ActiveRecord::Base.connection_pool.release_thread_connection. + # 2. Manually check out a connection from the pool with + # ActiveRecord::Base.connection_pool.checkout. You are responsible for + # returning this connection to the pool when finished by calling + # ActiveRecord::Base.connection_pool.checkin(connection). + # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which + # obtains a connection, yields it as the sole argument to the block, + # and returns it to the pool after the block completes. + class AbstractConnectionPool # Factory method for connection pools. - # Determines pool type to use based on contents of connection specification. - # FIXME: specification configuration TBD. + # Determines pool type to use based on contents of connection + # specification. Additional options for connection specification: + # + # * +pool+: number indicating size of fixed connection pool to use + # * +wait_timeout+ (optional): number of seconds to block and wait + # for a connection before giving up and raising a timeout error. def self.create(spec) - ConnectionPerThread.new(spec) + if spec.config[:pool] && spec.config[:pool].to_i > 0 + ConnectionPool.new(spec) + else + ConnectionPerThread.new(spec) + end end delegate :verification_timeout, :to => "::ActiveRecord::Base" @@ -115,11 +144,16 @@ module ActiveRecord end private :connections - synchronize :connection, :release_thread_connection, :checkout, :checkin, + synchronize :connection, :release_thread_connection, :clear_reloadable_connections!, :verify_active_connections!, :connected?, :disconnect!, :with => :@connection_mutex private + def new_connection + config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) + ActiveRecord::Base.send(spec.adapter_method, config) + end + def active_connection_name #:nodoc: Thread.current.object_id end @@ -139,7 +173,10 @@ module ActiveRecord end end - class ConnectionPerThread < ConnectionPool + # ConnectionPerThread is a simple implementation: always create/disconnect + # on checkout/checkin, and use the base class reserved connections hash to + # manage the per-thread connections. + class ConnectionPerThread < AbstractConnectionPool def active_connection @reserved_connections[active_connection_name] end @@ -155,12 +192,6 @@ module ActiveRecord end private - # Set the connection for the class. - def new_connection - config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) - ActiveRecord::Base.send(spec.adapter_method, config) - end - def connections @reserved_connections.values end @@ -170,6 +201,70 @@ module ActiveRecord end end + # ConnectionPool provides a full, fixed-size connection pool with timed + # waits when the pool is exhausted. + class ConnectionPool < AbstractConnectionPool + def initialize(spec) + super + # default 5 second timeout + @timeout = spec.config[:wait_timeout] || 5 + @size = spec.config[:pool].to_i + @queue = @connection_mutex.new_cond + @connections = [] + @checked_out = [] + end + + def checkout + # Checkout an available connection + conn = @connection_mutex.synchronize do + if @connections.length < @size + checkout_new_connection + elsif @checked_out.size < @connections.size + checkout_existing_connection + end + end + return conn if conn + + # No connections available; wait for one + @connection_mutex.synchronize do + if @queue.wait(@timeout) + checkout_existing_connection + else + raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion" + end + end + end + + def checkin(conn) + @connection_mutex.synchronize do + @checked_out -= conn + @queue.signal + end + end + + private + def checkout_new_connection + c = new_connection + @connections << c + @checked_out << c + c + end + + def checkout_existing_connection + c = [@connections - @checked_out].first + @checked_out << c + c + end + + def connections + @connections + end + + def remove_connection(conn) + @connections.delete conn + end + end + module ConnectionHandlerMethods def initialize(pools = {}) @connection_pools = pools @@ -180,7 +275,7 @@ module ActiveRecord end def establish_connection(name, spec) - @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) + @connection_pools[name] = ConnectionAdapters::AbstractConnectionPool.create(spec) end # for internal use only and for testing; @@ -238,15 +333,14 @@ module ActiveRecord pool.spec.config if pool end - private - def retrieve_connection_pool(klass) - loop do - pool = @connection_pools[klass.name] - return pool if pool - return nil if ActiveRecord::Base == klass - klass = klass.superclass - end + def retrieve_connection_pool(klass) + loop do + pool = @connection_pools[klass.name] + return pool if pool + return nil if ActiveRecord::Base == klass + klass = klass.superclass end + end end # This connection handler is not thread-safe, as it does not protect access diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 0ebb191381..0910db1951 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -116,6 +116,10 @@ module ActiveRecord retrieve_connection end + def connection_pool + connection_handler.retrieve_connection_pool(self) + end + def retrieve_connection connection_handler.retrieve_connection(self) end -- cgit v1.2.3 From 3ce64d4f1608330072e1959a10f9b84205baebfa Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 7 Aug 2008 08:33:42 -0700 Subject: Fix checkin method, add a couple more tests --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 97c6e3e886..3d727bd0e6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -237,7 +237,7 @@ module ActiveRecord def checkin(conn) @connection_mutex.synchronize do - @checked_out -= conn + @checked_out.delete conn @queue.signal end end -- cgit v1.2.3 From 817a07b45105f7043846973525a9edc44028c0d4 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 7 Aug 2008 22:24:42 -0700 Subject: More doco and class/method renames. Now have a strategy for integration with ActionPack. --- .../abstract/connection_pool.rb | 63 +++++++++++++--------- 1 file changed, 39 insertions(+), 24 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 3d727bd0e6..50b2badbe6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -14,10 +14,12 @@ module ActiveRecord # Connections can be obtained and used from a connection pool in several # ways: # - # 1. Simply use ActiveRecord::Base.connection as in pre-connection-pooled - # ActiveRecord. Eventually, when you're done with the connection and - # wish it to be returned to the pool, you call - # ActiveRecord::Base.connection_pool.release_thread_connection. + # 1. Simply use ActiveRecord::Base.connection as with ActiveRecord 2.1 and + # earlier (pre-connection-pooling). Eventually, when you're done with + # the connection(s) and wish it to be returned to the pool, you call + # ActiveRecord::Base.clear_active_connections!. This will be the + # default behavior for ActiveRecord when used in conjunction with + # ActionPack's request handling cycle. # 2. Manually check out a connection from the pool with # ActiveRecord::Base.connection_pool.checkout. You are responsible for # returning this connection to the pool when finished by calling @@ -25,7 +27,7 @@ module ActiveRecord # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which # obtains a connection, yields it as the sole argument to the block, # and returns it to the pool after the block completes. - class AbstractConnectionPool + class ConnectionPool # Factory method for connection pools. # Determines pool type to use based on contents of connection # specification. Additional options for connection specification: @@ -35,9 +37,11 @@ module ActiveRecord # for a connection before giving up and raising a timeout error. def self.create(spec) if spec.config[:pool] && spec.config[:pool].to_i > 0 - ConnectionPool.new(spec) + FixedSizeConnectionPool.new(spec) + elsif spec.config[:jndi] # JRuby appserver datasource pool; passthrough + NewConnectionEveryTime.new(spec) else - ConnectionPerThread.new(spec) + CachedConnectionPerThread.new(spec) end end @@ -67,9 +71,9 @@ module ActiveRecord end # Signal that the thread is finished with the current connection. - # #release_thread_connection releases the connection-thread association + # #release_connection releases the connection-thread association # and returns the connection to the pool. - def release_thread_connection + def release_connection conn = @reserved_connections.delete(active_connection_name) checkin conn if conn end @@ -113,7 +117,8 @@ module ActiveRecord end end - # Verify active connections. + # Verify active connections and remove and disconnect connections + # associated with stale threads. def verify_active_connections! #:nodoc: remove_stale_cached_threads!(@reserved_connections) do |name, conn| checkin conn @@ -133,18 +138,17 @@ module ActiveRecord raise NotImplementedError, "checkin is an abstract method" end - def remove_connection(conn) + def remove_connection(conn) #:nodoc: raise NotImplementedError, "remove_connection is an abstract method" end private :remove_connection - # Array containing all connections (reserved or available) in the pool. - def connections + def connections #:nodoc: raise NotImplementedError, "connections is an abstract method" end private :connections - synchronize :connection, :release_thread_connection, + synchronize :connection, :release_connection, :clear_reloadable_connections!, :verify_active_connections!, :connected?, :disconnect!, :with => :@connection_mutex @@ -173,10 +177,9 @@ module ActiveRecord end end - # ConnectionPerThread is a simple implementation: always create/disconnect - # on checkout/checkin, and use the base class reserved connections hash to - # manage the per-thread connections. - class ConnectionPerThread < AbstractConnectionPool + # NewConnectionEveryTime is a simple implementation: always + # create/disconnect on checkout/checkin. + class NewConnectionEveryTime < ConnectionPool def active_connection @reserved_connections[active_connection_name] end @@ -201,9 +204,21 @@ module ActiveRecord end end - # ConnectionPool provides a full, fixed-size connection pool with timed - # waits when the pool is exhausted. - class ConnectionPool < AbstractConnectionPool + # CachedConnectionPerThread is a compatible pseudo-connection pool that + # caches connections per-thread. In order to hold onto threads in the same + # manner as ActiveRecord 2.1 and earlier, it only disconnects the + # connection when the connection is checked in, or when calling + # ActiveRecord::Base.clear_all_connections!, and not during + # #release_connection. + class CachedConnectionPerThread < NewConnectionEveryTime + def release_connection + # no-op; keep the connection + end + end + + # FixedSizeConnectionPool provides a full, fixed-size connection pool with + # timed waits when the pool is exhausted. + class FixedSizeConnectionPool < ConnectionPool def initialize(spec) super # default 5 second timeout @@ -275,7 +290,7 @@ module ActiveRecord end def establish_connection(name, spec) - @connection_pools[name] = ConnectionAdapters::AbstractConnectionPool.create(spec) + @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) end # for internal use only and for testing; @@ -290,7 +305,7 @@ module ActiveRecord # Clears the cache which maps classes to connections. def clear_active_connections! - @connection_pools.each_value {|pool| pool.release_thread_connection } + @connection_pools.each_value {|pool| pool.release_connection } end # Clears the cache which maps classes @@ -304,7 +319,7 @@ module ActiveRecord # Verify active connections. def verify_active_connections! #:nodoc: - @connection_pools.each_value {|pool| pool.verify_active_connections!} + @connection_pools.each_value {|pool| pool.verify_active_connections! } end # Locate the connection of the nearest super class. This can be an -- cgit v1.2.3 From 1712e37c90d0ac74b21589c0ee7b0365cb2b7beb Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 7 Aug 2008 23:13:21 -0700 Subject: Favor existing connections over new ones if available --- .../active_record/connection_adapters/abstract/connection_pool.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 50b2badbe6..1d7efc89e1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -232,10 +232,10 @@ module ActiveRecord def checkout # Checkout an available connection conn = @connection_mutex.synchronize do - if @connections.length < @size - checkout_new_connection - elsif @checked_out.size < @connections.size + if @checked_out.size < @connections.size checkout_existing_connection + elsif @connections.size < @size + checkout_new_connection end end return conn if conn -- cgit v1.2.3 From d7d2d73d88e465cb0c618283862179accd8932e1 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 8 Aug 2008 00:45:16 -0700 Subject: Fix typo: was using brackets instead of parens. Must need more sleep. --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 1d7efc89e1..988f85e909 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -266,7 +266,7 @@ module ActiveRecord end def checkout_existing_connection - c = [@connections - @checked_out].first + c = (@connections - @checked_out).first @checked_out << c c end -- cgit v1.2.3 From a96b7d4c33757364a19ed1fc34f0a89801b8b2d7 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 11:43:20 -0500 Subject: Add connection reset and verification upon each connection checkout --- .../connection_adapters/abstract/connection_pool.rb | 9 +++++++-- .../active_record/connection_adapters/abstract_adapter.rb | 13 +++++++++++-- .../lib/active_record/connection_adapters/mysql_adapter.rb | 9 +++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 988f85e909..365c80fe1d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -261,12 +261,17 @@ module ActiveRecord def checkout_new_connection c = new_connection @connections << c - @checked_out << c - c + checkout_and_verify(c) end def checkout_existing_connection c = (@connections - @checked_out).first + checkout_and_verify(c) + end + + def checkout_and_verify(c) + c.reset! + c.verify!(verification_timeout) @checked_out << c c end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 4b78d9f2e9..c37ebf1410 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -103,14 +103,23 @@ module ActiveRecord @active = false end + # Reset the state of this connection, directing the DBMS to clear + # transactions and other connection-related server-side state. Usually a + # database-dependent operation; the default method simply executes a + # ROLLBACK and swallows any exceptions which is probably not enough to + # ensure the connection is clean. + def reset! + execute "ROLLBACK" rescue nil + end + # Returns true if its safe to reload the connection between requests for development mode. # This is not the case for Ruby/MySQL and it's not necessary for any adapters except SQLite. def requires_reloading? false end - # Lazily verify this connection, calling active? only if it hasn't - # been called for +timeout+ seconds. + # Lazily verify this connection, calling active? only if it + # hasn't been called for +timeout+ seconds. def verify!(timeout) now = Time.now.to_i if (now - @last_verification) > timeout diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 204ebaa2e2..0a935a1b7a 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -280,6 +280,15 @@ module ActiveRecord @connection.close rescue nil end + def reset! + if @connection.respond_to?(:change_user) + # See http://bugs.mysql.com/bug.php?id=33540 -- the workaround way to + # reset the connection is to change the user to the same user. + @connection.change_user(@config[:username], @config[:password], @config[:database]) + else + super + end + end # DATABASE STATEMENTS ====================================== -- cgit v1.2.3 From ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 12:19:29 -0500 Subject: Deprecate allow_concurrency and make it have no effect --- activerecord/lib/active_record/base.rb | 20 +-------------- .../abstract/connection_pool.rb | 25 ++++-------------- .../abstract/connection_specification.rb | 30 ++++++++-------------- .../connection_adapters/abstract_adapter.rb | 4 ++- 4 files changed, 20 insertions(+), 59 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b5ffc471bc..bc6d61301f 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -452,13 +452,6 @@ module ActiveRecord #:nodoc: cattr_accessor :default_timezone, :instance_writer => false @@default_timezone = :local - # Determines whether to use a connection for each thread, or a single shared connection for all threads. - # Defaults to false. If you're writing a threaded application, set to true - # and periodically call verify_active_connections! to clear out connections - # assigned to stale threads. - cattr_accessor :allow_concurrency, :instance_writer => false - @@allow_concurrency = false - # Specifies the format to use when dumping the database schema with Rails' # Rakefile. If :sql, the schema is dumped as (potentially database- # specific) SQL statements. If :ruby, the schema is dumped as an @@ -1943,22 +1936,11 @@ module ActiveRecord #:nodoc: end end - def thread_safe_scoped_methods #:nodoc: + def scoped_methods #:nodoc: scoped_methods = (Thread.current[:scoped_methods] ||= {}) scoped_methods[self] ||= [] end - def single_threaded_scoped_methods #:nodoc: - @scoped_methods ||= [] - end - - # pick up the correct scoped_methods version from @@allow_concurrency - if @@allow_concurrency - alias_method :scoped_methods, :thread_safe_scoped_methods - else - alias_method :scoped_methods, :single_threaded_scoped_methods - end - def current_scoped_methods #:nodoc: scoped_methods.last end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 365c80fe1d..04c8361c64 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -154,7 +154,7 @@ module ActiveRecord private def new_connection - config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) + config = spec.config.reverse_merge(:allow_concurrency => true) ActiveRecord::Base.send(spec.adapter_method, config) end @@ -285,9 +285,12 @@ module ActiveRecord end end - module ConnectionHandlerMethods + class ConnectionHandler + attr_reader :connection_pools_lock + def initialize(pools = {}) @connection_pools = pools + @connection_pools_lock = Monitor.new end def connection_pools @@ -361,24 +364,6 @@ module ActiveRecord klass = klass.superclass end end - end - - # This connection handler is not thread-safe, as it does not protect access - # to the underlying connection pools. - class SingleThreadConnectionHandler - include ConnectionHandlerMethods - end - - # This connection handler is thread-safe. Each access or modification of a thread - # pool is synchronized by an internal monitor. - class MultipleThreadConnectionHandler - attr_reader :connection_pools_lock - include ConnectionHandlerMethods - - def initialize(pools = {}) - super - @connection_pools_lock = Monitor.new - end # Apply monitor to all public methods that access the pool. synchronize :establish_connection, :retrieve_connection, diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 0910db1951..47fc11a620 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -14,25 +14,7 @@ module ActiveRecord # The connection handler cattr_accessor :connection_handler, :instance_writer => false - @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new - - # Turning on allow_concurrency changes the single threaded connection handler - # for a multiple threaded one, so that multi-threaded access of the - # connection pools is synchronized. - def self.allow_concurrency=(flag) - if @@allow_concurrency != flag - @@allow_concurrency = flag - # When switching connection handlers, preserve the existing pools so that - # #establish_connection doesn't need to be called again. - if @@allow_concurrency - self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new( - self.connection_handler.connection_pools) - else - self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new( - self.connection_handler.connection_pools) - end - end - end + @@connection_handler = ConnectionAdapters::ConnectionHandler.new # Returns the connection currently associated with the class. This can # also be used to "borrow" the connection to do database work that isn't @@ -109,6 +91,16 @@ module ActiveRecord end class << self + # Deprecated and no longer has any effect. + def allow_concurrency + ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency has been deprecated and no longer has any effect. Please remove all references to allow_concurrency.") + end + + # Deprecated and no longer has any effect. + def allow_concurrency=(flag) + ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency= has been deprecated and no longer has any effect. Please remove all references to allow_concurrency=.") + end + # Returns the connection currently associated with the class. This can # also be used to "borrow" the connection to do database work unrelated # to any of the specific Active Records. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index c37ebf1410..7ef8834547 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -109,7 +109,9 @@ module ActiveRecord # ROLLBACK and swallows any exceptions which is probably not enough to # ensure the connection is clean. def reset! - execute "ROLLBACK" rescue nil + silence_stderr do # postgres prints on stderr when you do this w/o a txn + execute "ROLLBACK" rescue nil + end end # Returns true if its safe to reload the connection between requests for development mode. -- cgit v1.2.3 From 212134dce158db0ecb4169c28fd9ef80ea1a55b2 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 13:45:10 -0500 Subject: Remove CachedConnectionPerThread per-thread pooling mechanism in favor of a fixed pool with default maximum of 5 connections --- .../connection_adapters/abstract/connection_pool.rb | 21 ++++----------------- .../connection_adapters/mysql_adapter.rb | 5 +++++ activerecord/lib/active_record/fixtures.rb | 2 +- 3 files changed, 10 insertions(+), 18 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 04c8361c64..1b908c3113 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -36,12 +36,10 @@ module ActiveRecord # * +wait_timeout+ (optional): number of seconds to block and wait # for a connection before giving up and raising a timeout error. def self.create(spec) - if spec.config[:pool] && spec.config[:pool].to_i > 0 - FixedSizeConnectionPool.new(spec) - elsif spec.config[:jndi] # JRuby appserver datasource pool; passthrough + if spec.config[:jndi] # JRuby appserver datasource pool; passthrough NewConnectionEveryTime.new(spec) else - CachedConnectionPerThread.new(spec) + FixedSizeConnectionPool.new(spec) end end @@ -204,18 +202,6 @@ module ActiveRecord end end - # CachedConnectionPerThread is a compatible pseudo-connection pool that - # caches connections per-thread. In order to hold onto threads in the same - # manner as ActiveRecord 2.1 and earlier, it only disconnects the - # connection when the connection is checked in, or when calling - # ActiveRecord::Base.clear_all_connections!, and not during - # #release_connection. - class CachedConnectionPerThread < NewConnectionEveryTime - def release_connection - # no-op; keep the connection - end - end - # FixedSizeConnectionPool provides a full, fixed-size connection pool with # timed waits when the pool is exhausted. class FixedSizeConnectionPool < ConnectionPool @@ -223,7 +209,8 @@ module ActiveRecord super # default 5 second timeout @timeout = spec.config[:wait_timeout] || 5 - @size = spec.config[:pool].to_i + # default max pool size to 5 + @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 @queue = @connection_mutex.new_cond @connections = [] @checked_out = [] diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 0a935a1b7a..14c76ac455 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -285,6 +285,7 @@ module ActiveRecord # See http://bugs.mysql.com/bug.php?id=33540 -- the workaround way to # reset the connection is to change the user to the same user. @connection.change_user(@config[:username], @config[:password], @config[:database]) + configure_connection else super end @@ -538,7 +539,11 @@ module ActiveRecord end @connection.real_connect(*@connection_options) + configure_connection + end + def configure_connection + encoding = @config[:encoding] execute("SET NAMES '#{encoding}'") if encoding # By default, MySQL 'where id is null' selects the last inserted id. diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 622cfc3c3f..114141a646 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -955,7 +955,7 @@ module Test #:nodoc: ActiveRecord::Base.connection.rollback_db_transaction ActiveRecord::Base.connection.decrement_open_transactions end - ActiveRecord::Base.verify_active_connections! + ActiveRecord::Base.clear_active_connections! end private -- cgit v1.2.3 From d07a6b1a4a234908959650197f596329ca08b4f0 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 14:11:44 -0500 Subject: Make clear_active_connections! also return stale connections back to the pool - also clean up some cruft remaining from per-thread connection cache --- .../abstract/connection_pool.rb | 53 ++++++++++------------ .../abstract/connection_specification.rb | 5 +- 2 files changed, 25 insertions(+), 33 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 1b908c3113..472cf64c54 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -60,11 +60,11 @@ module ActiveRecord # #connection can be called any number of times; the connection is # held in a hash keyed by the thread id. def connection - if conn = @reserved_connections[active_connection_name] + if conn = @reserved_connections[current_connection_id] conn.verify!(verification_timeout) conn else - @reserved_connections[active_connection_name] = checkout + @reserved_connections[current_connection_id] = checkout end end @@ -72,7 +72,7 @@ module ActiveRecord # #release_connection releases the connection-thread association # and returns the connection to the pool. def release_connection - conn = @reserved_connections.delete(active_connection_name) + conn = @reserved_connections.delete(current_connection_id) checkin conn if conn end @@ -118,14 +118,20 @@ module ActiveRecord # Verify active connections and remove and disconnect connections # associated with stale threads. def verify_active_connections! #:nodoc: - remove_stale_cached_threads!(@reserved_connections) do |name, conn| - checkin conn - end + clear_stale_cached_connections! connections.each do |connection| connection.verify!(verification_timeout) end end + # Return any checked-out connections back to the pool by threads that + # are no longer alive. + def clear_stale_cached_connections! + remove_stale_cached_threads!(@reserved_connections) do |name, conn| + checkin conn + end + end + # Check-out a database connection from the pool. def checkout raise NotImplementedError, "checkout is an abstract method" @@ -156,7 +162,7 @@ module ActiveRecord ActiveRecord::Base.send(spec.adapter_method, config) end - def active_connection_name #:nodoc: + def current_connection_id #:nodoc: Thread.current.object_id end @@ -178,12 +184,6 @@ module ActiveRecord # NewConnectionEveryTime is a simple implementation: always # create/disconnect on checkout/checkin. class NewConnectionEveryTime < ConnectionPool - def active_connection - @reserved_connections[active_connection_name] - end - - def active_connections; @reserved_connections; end - def checkout new_connection end @@ -288,19 +288,14 @@ module ActiveRecord @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) end - # for internal use only and for testing; - # only works with ConnectionPerThread pool class - def active_connections #:nodoc: - @connection_pools.inject({}) do |hash,kv| - hash[kv.first] = kv.last.active_connection - hash.delete(kv.first) unless hash[kv.first] - hash - end - end - - # Clears the cache which maps classes to connections. + # Returns any connections in use by the current thread back to the pool, + # and also returns connections to the pool cached by threads that are no + # longer alive. def clear_active_connections! - @connection_pools.each_value {|pool| pool.release_connection } + @connection_pools.each_value do |pool| + pool.release_connection + pool.clear_stale_cached_connections! + end end # Clears the cache which maps classes @@ -353,11 +348,9 @@ module ActiveRecord end # Apply monitor to all public methods that access the pool. - synchronize :establish_connection, :retrieve_connection, - :connected?, :remove_connection, :active_connections, - :clear_active_connections!, :clear_reloadable_connections!, - :clear_all_connections!, :verify_active_connections!, - :with => :connection_pools_lock + synchronize :establish_connection, :retrieve_connection, :connected?, :remove_connection, + :clear_active_connections!, :clear_reloadable_connections!, :clear_all_connections!, + :verify_active_connections!, :with => :connection_pools_lock end end end \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 47fc11a620..417a333aab 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -124,9 +124,8 @@ module ActiveRecord connection_handler.remove_connection(klass) end - delegate :active_connections, :clear_active_connections!, - :clear_reloadable_connections!, :clear_all_connections!, - :verify_active_connections!, :to => :connection_handler + delegate :clear_active_connections!, :clear_reloadable_connections!, + :clear_all_connections!,:verify_active_connections!, :to => :connection_handler end end end -- cgit v1.2.3 From 8e5e02bdad5f5aaed8ea72e9da13f8d6aa22ab34 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 14:40:06 -0500 Subject: Collapse connection pool class hierarchy; YAGNI. - Add connection checkin and checkout callbacks to adapter to allow adapter-specific customization of behavior (e.g., JRuby w/ JNDI) --- .../abstract/connection_pool.rb | 160 +++++++-------------- .../connection_adapters/abstract_adapter.rb | 3 + 2 files changed, 51 insertions(+), 112 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 472cf64c54..85ecd9d6c6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -27,22 +27,14 @@ module ActiveRecord # 3. Use ActiveRecord::Base.connection_pool.with_connection(&block), which # obtains a connection, yields it as the sole argument to the block, # and returns it to the pool after the block completes. + # + # There are two connection-pooling-related options that you can add to + # your database connection configuration: + # + # * +pool+: number indicating size of connection pool (default 5) + # * +wait_timeout+: number of seconds to block and wait for a connection + # before giving up and raising a timeout error (default 5 seconds). class ConnectionPool - # Factory method for connection pools. - # Determines pool type to use based on contents of connection - # specification. Additional options for connection specification: - # - # * +pool+: number indicating size of fixed connection pool to use - # * +wait_timeout+ (optional): number of seconds to block and wait - # for a connection before giving up and raising a timeout error. - def self.create(spec) - if spec.config[:jndi] # JRuby appserver datasource pool; passthrough - NewConnectionEveryTime.new(spec) - else - FixedSizeConnectionPool.new(spec) - end - end - delegate :verification_timeout, :to => "::ActiveRecord::Base" attr_reader :spec @@ -52,6 +44,13 @@ module ActiveRecord @reserved_connections = {} # The mutex used to synchronize pool access @connection_mutex = Monitor.new + @queue = @connection_mutex.new_cond + # default 5 second timeout + @timeout = spec.config[:wait_timeout] || 5 + # default max pool size to 5 + @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 + @connections = [] + @checked_out = [] end # Retrieve the connection associated with the current thread, or call @@ -87,7 +86,7 @@ module ActiveRecord # Returns true if a connection has already been opened. def connected? - !connections.empty? + !@connections.empty? end # Disconnect all connections in the pool. @@ -95,10 +94,11 @@ module ActiveRecord @reserved_connections.each do |name,conn| checkin conn end - connections.each do |conn| + @reserved_connections = {} + @connections.each do |conn| conn.disconnect! end - @reserved_connections = {} + @connections = [] end # Clears the cache which maps classes @@ -107,19 +107,17 @@ module ActiveRecord checkin conn end @reserved_connections = {} - connections.each do |conn| - if conn.requires_reloading? - conn.disconnect! - remove_connection conn - end + @connections.each do |conn| + conn.disconnect! if conn.requires_reloading? end + @connections = [] end # Verify active connections and remove and disconnect connections # associated with stale threads. def verify_active_connections! #:nodoc: clear_stale_cached_connections! - connections.each do |connection| + @connections.each do |connection| connection.verify!(verification_timeout) end end @@ -134,23 +132,34 @@ module ActiveRecord # Check-out a database connection from the pool. def checkout - raise NotImplementedError, "checkout is an abstract method" - end - - # Check-in a database connection back into the pool. - def checkin(connection) - raise NotImplementedError, "checkin is an abstract method" - end + # Checkout an available connection + conn = @connection_mutex.synchronize do + if @checked_out.size < @connections.size + checkout_existing_connection + elsif @connections.size < @size + checkout_new_connection + end + end + return conn if conn - def remove_connection(conn) #:nodoc: - raise NotImplementedError, "remove_connection is an abstract method" + # No connections available; wait for one + @connection_mutex.synchronize do + if @queue.wait(@timeout) + checkout_existing_connection + else + raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion" + end + end end - private :remove_connection - def connections #:nodoc: - raise NotImplementedError, "connections is an abstract method" + # Check-in a database connection back into the pool. + def checkin(conn) + @connection_mutex.synchronize do + conn.run_callbacks :checkin + @checked_out.delete conn + @queue.signal + end end - private :connections synchronize :connection, :release_connection, :clear_reloadable_connections!, :verify_active_connections!, @@ -179,72 +188,7 @@ module ActiveRecord cache.delete(key) end end - end - - # NewConnectionEveryTime is a simple implementation: always - # create/disconnect on checkout/checkin. - class NewConnectionEveryTime < ConnectionPool - def checkout - new_connection - end - - def checkin(conn) - conn.disconnect! - end - - private - def connections - @reserved_connections.values - end - - def remove_connection(conn) - @reserved_connections.delete_if {|k,v| v == conn} - end - end - - # FixedSizeConnectionPool provides a full, fixed-size connection pool with - # timed waits when the pool is exhausted. - class FixedSizeConnectionPool < ConnectionPool - def initialize(spec) - super - # default 5 second timeout - @timeout = spec.config[:wait_timeout] || 5 - # default max pool size to 5 - @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 - @queue = @connection_mutex.new_cond - @connections = [] - @checked_out = [] - end - - def checkout - # Checkout an available connection - conn = @connection_mutex.synchronize do - if @checked_out.size < @connections.size - checkout_existing_connection - elsif @connections.size < @size - checkout_new_connection - end - end - return conn if conn - - # No connections available; wait for one - @connection_mutex.synchronize do - if @queue.wait(@timeout) - checkout_existing_connection - else - raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion" - end - end - end - def checkin(conn) - @connection_mutex.synchronize do - @checked_out.delete conn - @queue.signal - end - end - - private def checkout_new_connection c = new_connection @connections << c @@ -257,19 +201,11 @@ module ActiveRecord end def checkout_and_verify(c) - c.reset! + c.run_callbacks :checkout c.verify!(verification_timeout) @checked_out << c c end - - def connections - @connections - end - - def remove_connection(conn) - @connections.delete conn - end end class ConnectionHandler @@ -285,7 +221,7 @@ module ActiveRecord end def establish_connection(name, spec) - @connection_pools[name] = ConnectionAdapters::ConnectionPool.create(spec) + @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) end # Returns any connections in use by the current thread back to the pool, diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 7ef8834547..005be9d72f 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -25,6 +25,9 @@ module ActiveRecord class AbstractAdapter include Quoting, DatabaseStatements, SchemaStatements include QueryCache + include ActiveSupport::Callbacks + define_callbacks :checkout, :checkin + checkout :reset! @@row_even = true def initialize(connection, logger = nil) #:nodoc: -- cgit v1.2.3 From 113cc4e1c41b8246b8f6327b58bd315be72469e7 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Sat, 23 Aug 2008 01:24:34 -0500 Subject: Remove some synchronization that's probably overkill, assuming one doesn't establish connections frequently --- .../connection_adapters/abstract/connection_pool.rb | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 85ecd9d6c6..fe6ba47d69 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -161,8 +161,7 @@ module ActiveRecord end end - synchronize :connection, :release_connection, - :clear_reloadable_connections!, :verify_active_connections!, + synchronize :clear_reloadable_connections!, :verify_active_connections!, :connected?, :disconnect!, :with => :@connection_mutex private @@ -209,11 +208,8 @@ module ActiveRecord end class ConnectionHandler - attr_reader :connection_pools_lock - def initialize(pools = {}) @connection_pools = pools - @connection_pools_lock = Monitor.new end def connection_pools @@ -282,11 +278,6 @@ module ActiveRecord klass = klass.superclass end end - - # Apply monitor to all public methods that access the pool. - synchronize :establish_connection, :retrieve_connection, :connected?, :remove_connection, - :clear_active_connections!, :clear_reloadable_connections!, :clear_all_connections!, - :verify_active_connections!, :with => :connection_pools_lock end end end \ No newline at end of file -- cgit v1.2.3 From 300754509b6990b387b056c122e90f50a79eeb81 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Tue, 26 Aug 2008 11:08:06 -0500 Subject: Minor tweak to retrieve_connection_pool -- recurse instead of loop --- .../connection_adapters/abstract/connection_pool.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index fe6ba47d69..838b0434b0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -271,12 +271,10 @@ module ActiveRecord end def retrieve_connection_pool(klass) - loop do - pool = @connection_pools[klass.name] - return pool if pool - return nil if ActiveRecord::Base == klass - klass = klass.superclass - end + pool = @connection_pools[klass.name] + return pool if pool + return nil if ActiveRecord::Base == klass + retrieve_connection_pool klass.superclass end end end -- cgit v1.2.3 From 99492bad885aa0ee44c770e2c61ad36c3058c697 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 29 Aug 2008 21:12:37 +0200 Subject: Use a set for the named scope methods not a big regexp. --- activerecord/lib/active_record/named_scope.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index b9b7eb5f00..570416d4e0 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -101,9 +101,9 @@ module ActiveRecord class Scope attr_reader :proxy_scope, :proxy_options - + NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set [].methods.each do |m| - unless m =~ /(^__|^nil\?|^send|^object_id$|class|extend|^find$|count|sum|average|maximum|minimum|paginate|first|last|empty\?|any\?|respond_to\?)/ + unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m) delegate m, :to => :proxy_found end end -- cgit v1.2.3 From c0361344d95162cd8573592d60e52986eaca0377 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 29 Aug 2008 15:43:07 -0500 Subject: 1.9: methods need to be coerced into strings --- activerecord/lib/active_record/named_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 570416d4e0..7397d70279 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -103,7 +103,7 @@ module ActiveRecord attr_reader :proxy_scope, :proxy_options NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set [].methods.each do |m| - unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m) + unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m.to_s) delegate m, :to => :proxy_found end end -- cgit v1.2.3 From 7f179f8540ab92dbd9d3e650b465de5b694d93d4 Mon Sep 17 00:00:00 2001 From: Tom Stuart Date: Fri, 29 Aug 2008 15:11:25 +0100 Subject: Make NamedScope#size behave identically to AssociationCollection#size. [#933 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 7397d70279..d7e152ed1d 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -101,7 +101,7 @@ module ActiveRecord class Scope attr_reader :proxy_scope, :proxy_options - NON_DELEGATE_METHODS = %w(nil? send object_id class extend find count sum average maximum minimum paginate first last empty? any? respond_to?).to_set + NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set [].methods.each do |m| unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m.to_s) delegate m, :to => :proxy_found @@ -136,6 +136,10 @@ module ActiveRecord end end + def size + @found ? @found.length : count + end + def empty? @found ? @found.empty? : count.zero? end -- cgit v1.2.3 From b163d83b8bab9103dc0e73b86212c2629cb45ca2 Mon Sep 17 00:00:00 2001 From: miloops Date: Sat, 30 Aug 2008 19:17:29 -0300 Subject: Performance: Better query for ASSOCIATION_ids. Select only ids if the association hasn't been loaded. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3ca93db10f..62cc414555 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1304,7 +1304,11 @@ module ActiveRecord end define_method("#{reflection.name.to_s.singularize}_ids") do - send(reflection.name).map { |record| record.id } + if send(reflection.name).loaded? + send(reflection.name).map(&:id) + else + send(reflection.name).all(:select => "#{reflection.quoted_table_name}.id").map(&:id) + end end end -- cgit v1.2.3 From 6183e55f714b436335dc843528be7525c342d922 Mon Sep 17 00:00:00 2001 From: miloops Date: Sat, 30 Aug 2008 21:13:53 -0300 Subject: Use reflection primary_key instead of id for when selecting association ids. [#906 state:resolved] Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 62cc414555..2470eb44b4 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1307,7 +1307,7 @@ module ActiveRecord if send(reflection.name).loaded? send(reflection.name).map(&:id) else - send(reflection.name).all(:select => "#{reflection.quoted_table_name}.id").map(&:id) + send(reflection.name).all(:select => "#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").map(&:id) end end end -- cgit v1.2.3 From 76797b443929005f43512a147e97f02f3145ed81 Mon Sep 17 00:00:00 2001 From: Iain Hecker Date: Sun, 31 Aug 2008 12:14:24 +0200 Subject: translates when a message symbol has been set on builtin validations Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/validations.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 9ee80e6655..dae4ae8122 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -87,6 +87,8 @@ module ActiveRecord # def generate_message(attribute, message = :invalid, options = {}) + message, options[:default] = options[:default], message if options[:default].is_a?(Symbol) + defaults = @base.class.self_and_descendents_from_active_record.map do |klass| [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}", :"models.#{klass.name.underscore}.#{message}" ] @@ -95,7 +97,6 @@ module ActiveRecord defaults << options.delete(:default) defaults = defaults.compact.flatten << :"messages.#{message}" - model_name = @base.class.name key = defaults.shift value = @base.respond_to?(attribute) ? @base.send(attribute) : nil -- cgit v1.2.3 From 1646e8c36495680756304b23b7301dbda9cad07a Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Tue, 2 Sep 2008 10:04:53 +0200 Subject: More symbols for send and respond_to?. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/attribute_methods.rb | 4 ++-- activerecord/lib/active_record/validations.rb | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index fab16a4446..ace335ed87 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -330,8 +330,8 @@ module ActiveRecord end end - # A Person object with a name attribute can ask person.respond_to?("name"), - # person.respond_to?("name="), and person.respond_to?("name?") + # A Person object with a name attribute can ask person.respond_to?(:name), + # person.respond_to?(:name=), and person.respond_to?(:name?) # which will all return +true+. alias :respond_to_without_attributes? :respond_to? def respond_to?(method, include_priv = false) diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index dae4ae8122..9941b752cb 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -54,7 +54,7 @@ module ActiveRecord def add_on_empty(attributes, custom_message = nil) for attr in [attributes].flatten value = @base.respond_to?(attr.to_s) ? @base.send(attr.to_s) : @base[attr.to_s] - is_empty = value.respond_to?("empty?") ? value.empty? : false + is_empty = value.respond_to?(:empty?) ? value.empty? : false add(attr, :empty, :default => custom_message) unless !value.nil? && !is_empty end end @@ -751,7 +751,7 @@ module ActiveRecord enum = configuration[:in] || configuration[:within] - raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?("include?") + raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?) validates_each(attr_names, configuration) do |record, attr_name, value| unless enum.include?(value) @@ -785,7 +785,7 @@ module ActiveRecord enum = configuration[:in] || configuration[:within] - raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?("include?") + raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?) validates_each(attr_names, configuration) do |record, attr_name, value| if enum.include?(value) -- cgit v1.2.3 From ba3ecf53b4902a9a5943f4dcf2073fe413de4778 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Tue, 2 Sep 2008 10:31:49 +0200 Subject: Some performance goodness for AR associations. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/associations.rb | 64 +++++++++++++------------- 1 file changed, 32 insertions(+), 32 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2470eb44b4..dccd9abf31 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -878,10 +878,10 @@ module ActiveRecord method_name = "has_one_after_save_for_#{reflection.name}".to_sym define_method(method_name) do - association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") + association = instance_variable_get(ivar) if instance_variable_defined?(ivar) - if !association.nil? && (new_record? || association.new_record? || association["#{reflection.primary_key_name}"] != id) - association["#{reflection.primary_key_name}"] = id + if !association.nil? && (new_record? || association.new_record? || association[reflection.primary_key_name] != id) + association[reflection.primary_key_name] = id association.save(true) end end @@ -994,7 +994,7 @@ module ActiveRecord method_name = "polymorphic_belongs_to_before_save_for_#{reflection.name}".to_sym define_method(method_name) do - association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") + association = instance_variable_get(ivar) if instance_variable_defined?(ivar) if association && association.target if association.new_record? @@ -1002,8 +1002,8 @@ module ActiveRecord end if association.updated? - self["#{reflection.primary_key_name}"] = association.id - self["#{reflection.options[:foreign_type]}"] = association.class.base_class.name.to_s + self[reflection.primary_key_name] = association.id + self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s end end end @@ -1015,7 +1015,7 @@ module ActiveRecord method_name = "belongs_to_before_save_for_#{reflection.name}".to_sym define_method(method_name) do - association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") + association = instance_variable_get(ivar) if instance_variable_defined?(ivar) if !association.nil? if association.new_record? @@ -1023,7 +1023,7 @@ module ActiveRecord end if association.updated? - self["#{reflection.primary_key_name}"] = association.id + self[reflection.primary_key_name] = association.id end end end @@ -1038,15 +1038,15 @@ module ActiveRecord method_name = "belongs_to_counter_cache_after_create_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") - association.class.increment_counter("#{cache_column}", send("#{reflection.primary_key_name}")) unless association.nil? + association = send(reflection.name) + association.class.increment_counter(cache_column, send(reflection.primary_key_name)) unless association.nil? end after_create method_name method_name = "belongs_to_counter_cache_before_destroy_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") - association.class.decrement_counter("#{cache_column}", send("#{reflection.primary_key_name}")) unless association.nil? + association = send(reflection.name) + association.class.decrement_counter(cache_column, send(reflection.primary_key_name)) unless association.nil? end before_destroy method_name @@ -1329,19 +1329,19 @@ module ActiveRecord end end end - + def add_single_associated_validation_callbacks(association_name) method_name = "validate_associated_records_for_#{association_name}".to_sym define_method(method_name) do association = instance_variable_get("@#{association_name}") if !association.nil? - errors.add "#{association_name}" unless association.target.nil? || association.valid? + errors.add association_name unless association.target.nil? || association.valid? end end - + validate method_name end - + def add_multiple_associated_validation_callbacks(association_name) method_name = "validate_associated_records_for_#{association_name}".to_sym ivar = "@#{association_name}" @@ -1357,7 +1357,7 @@ module ActiveRecord else association.target.select { |record| record.new_record? } end.each do |record| - errors.add "#{association_name}" unless record.valid? + errors.add association_name unless record.valid? end end end @@ -1377,7 +1377,7 @@ module ActiveRecord method_name = "after_create_or_update_associated_records_for_#{association_name}".to_sym define_method(method_name) do - association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") + association = instance_variable_get(ivar) if instance_variable_defined?(ivar) records_to_save = if @new_record_before_save association @@ -1444,7 +1444,7 @@ module ActiveRecord when :destroy method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym define_method(method_name) do - send("#{reflection.name}").each { |o| o.destroy } + send(reflection.name).each { |o| o.destroy } end before_destroy method_name when :delete_all @@ -1463,22 +1463,22 @@ module ActiveRecord when :destroy method_name = "has_one_dependent_destroy_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") + association = send(reflection.name) association.destroy unless association.nil? end before_destroy method_name when :delete method_name = "has_one_dependent_delete_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") + association = send(reflection.name) association.class.delete(association.id) unless association.nil? end before_destroy method_name when :nullify method_name = "has_one_dependent_nullify_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") - association.update_attribute("#{reflection.primary_key_name}", nil) unless association.nil? + association = send(reflection.name) + association.update_attribute(reflection.primary_key_name, nil) unless association.nil? end before_destroy method_name else @@ -1493,14 +1493,14 @@ module ActiveRecord when :destroy method_name = "belongs_to_dependent_destroy_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") + association = send(reflection.name) association.destroy unless association.nil? end before_destroy method_name when :delete method_name = "belongs_to_dependent_delete_for_#{reflection.name}".to_sym define_method(method_name) do - association = send("#{reflection.name}") + association = send(reflection.name) association.class.delete(association.id) unless association.nil? end before_destroy method_name @@ -1535,7 +1535,7 @@ module ActiveRecord create_reflection(:has_one, association_id, options, self) end - + def create_has_one_through_reflection(association_id, options) options.assert_valid_keys( :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :through, :source, :source_type, :validate @@ -1927,7 +1927,7 @@ module ActiveRecord end def aliased_primary_key - "#{ aliased_prefix }_r0" + "#{aliased_prefix}_r0" end def aliased_table_name @@ -1939,7 +1939,7 @@ module ActiveRecord @column_names_with_alias = [] ([primary_key] + (column_names - [primary_key])).each_with_index do |column_name, i| - @column_names_with_alias << [column_name, "#{ aliased_prefix }_r#{ i }"] + @column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"] end end @@ -1976,11 +1976,11 @@ module ActiveRecord @aliased_prefix = "t#{ join_dependency.joins.size }" @parent_table_name = parent.active_record.table_name @aliased_table_name = aliased_table_name_for(table_name) - + if reflection.macro == :has_and_belongs_to_many @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join") end - + if [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through] @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join") end @@ -2117,7 +2117,7 @@ module ActiveRecord end protected - + def aliased_table_name_for(name, suffix = nil) if !parent.table_joins.blank? && parent.table_joins.to_s.downcase =~ %r{join(\s+\w+)?\s+#{name.downcase}\son} @join_dependency.table_aliases[name] += 1 @@ -2135,7 +2135,7 @@ module ActiveRecord name end - + def pluralize(table_name) ActiveRecord::Base.pluralize_table_names ? table_name.to_s.pluralize : table_name end -- cgit v1.2.3 From 4d092ba2089de185cc8f5a8d16432b348e102046 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Tue, 2 Sep 2008 10:59:20 +0200 Subject: Some performance goodness for AR. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/attribute_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index ace335ed87..0a1baff87d 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -214,7 +214,7 @@ module ActiveRecord if logger logger.warn "Exception occurred during reader method compilation." logger.warn "Maybe #{attr_name} is not a valid Ruby identifier?" - logger.warn "#{err.message}" + logger.warn err.message end end end -- cgit v1.2.3 From 288e947ae1737645985fde76f5382baaff700505 Mon Sep 17 00:00:00 2001 From: Clemens Kofler Date: Tue, 2 Sep 2008 15:33:49 +0200 Subject: Some performance goodness for inheritable attributes. Signed-off-by: Jeremy Kemper --- activerecord/lib/active_record/base.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index bc6d61301f..2367277c03 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -920,12 +920,12 @@ module ActiveRecord #:nodoc: # To start from an all-closed default and enable attributes as needed, # have a look at +attr_accessible+. def attr_protected(*attributes) - write_inheritable_attribute("attr_protected", Set.new(attributes.map(&:to_s)) + (protected_attributes || [])) + write_inheritable_attribute(:attr_protected, Set.new(attributes.map(&:to_s)) + (protected_attributes || [])) end # Returns an array of all the attributes that have been protected from mass-assignment. def protected_attributes # :nodoc: - read_inheritable_attribute("attr_protected") + read_inheritable_attribute(:attr_protected) end # Specifies a white list of model attributes that can be set via @@ -953,22 +953,22 @@ module ActiveRecord #:nodoc: # customer.credit_rating = "Average" # customer.credit_rating # => "Average" def attr_accessible(*attributes) - write_inheritable_attribute("attr_accessible", Set.new(attributes.map(&:to_s)) + (accessible_attributes || [])) + write_inheritable_attribute(:attr_accessible, Set.new(attributes.map(&:to_s)) + (accessible_attributes || [])) end # Returns an array of all the attributes that have been made accessible to mass-assignment. def accessible_attributes # :nodoc: - read_inheritable_attribute("attr_accessible") + read_inheritable_attribute(:attr_accessible) end # Attributes listed as readonly can be set for a new record, but will be ignored in database updates afterwards. def attr_readonly(*attributes) - write_inheritable_attribute("attr_readonly", Set.new(attributes.map(&:to_s)) + (readonly_attributes || [])) + write_inheritable_attribute(:attr_readonly, Set.new(attributes.map(&:to_s)) + (readonly_attributes || [])) end # Returns an array of all the attributes that have been specified as readonly. def readonly_attributes - read_inheritable_attribute("attr_readonly") + read_inheritable_attribute(:attr_readonly) end # If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object, @@ -992,7 +992,7 @@ module ActiveRecord #:nodoc: # Returns a hash of all the attributes that have been specified for serialization as keys and their class restriction as values. def serialized_attributes - read_inheritable_attribute("attr_serialized") or write_inheritable_attribute("attr_serialized", {}) + read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {}) end -- cgit v1.2.3