From 2fcafee250ee24224b8fb8c1d884a48770fe08b3 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 19:24:56 +0200 Subject: Fix that #pluck wasn't rescuing ThrowResult, meaning it would blow up when failing to construct_limited_ids_condition. --- activerecord/lib/active_record/relation/calculations.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 7239270c4d..308db8227f 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -189,6 +189,8 @@ module ActiveRecord end columns.one? ? result.map!(&:first) : result end + rescue ThrowResult + [] end # Pluck all the ID's for the relation using the table's primary key -- cgit v1.2.3 From 86cc141ed5fd51ec752c956febfa436d1c816532 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:35:59 +0200 Subject: No point in memoizing a simple literal string. --- activerecord/lib/active_record/null_relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 711fc8b883..5d801fa705 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -39,7 +39,7 @@ module ActiveRecord end def to_sql - @to_sql ||= "" + "" end def where_values_hash -- cgit v1.2.3 From a2e607e1055dcede27970ccd7f5b89a1ddee8c32 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:45:09 +0200 Subject: Make NullRelation a bit more like a real relation by returning 0 for #calculate(:count) --- activerecord/lib/active_record/null_relation.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 5d801fa705..d166f0dd66 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -55,7 +55,11 @@ module ActiveRecord end def calculate(_operation, _column_name, _options = {}) - nil + if _operation == :count + 0 + else + nil + end end def exists?(_id = false) -- cgit v1.2.3 From 118147af53bebf71bf2a1d3ded4fc6491a708697 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:57:12 +0200 Subject: Rather than raising ThrowResult when construct_limited_ids_conditions comes up empty, set the relation to NullRelation and rely on its results. This will help avoid errors like 2fcafee250ee2, because in most cases NullRelation will do the right thing. Minor bonus is avoiding the use of exceptions for flow control. --- activerecord/lib/active_record/errors.rb | 4 ---- .../lib/active_record/relation/calculations.rb | 4 ---- .../lib/active_record/relation/finder_methods.rb | 28 ++++++++++++---------- 3 files changed, 15 insertions(+), 21 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index cd31147414..34bfad2d62 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -69,10 +69,6 @@ module ActiveRecord end end - # Raised when SQL statement is invalid and the application gets a blank result. - class ThrowResult < ActiveRecordError - end - # Defunct wrapper class kept for compatibility. # +StatementInvalid+ wraps the original exception now. class WrappedDatabaseException < StatementInvalid diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 308db8227f..7371fe370e 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -114,8 +114,6 @@ module ActiveRecord else relation.calculate(operation, column_name, options) end - rescue ThrowResult - 0 end # Use pluck as a shortcut to select one or more attributes without @@ -189,8 +187,6 @@ module ActiveRecord end columns.one? ? result.map!(&:first) : result end - rescue ThrowResult - [] end # Pluck all the ID's for the relation using the table's primary key diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ba222aac93..f0edef58bf 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -160,8 +160,9 @@ module ActiveRecord conditions = conditions.id if Base === conditions return false if !conditions - join_dependency = construct_join_dependency - relation = construct_relation_for_association_find(join_dependency) + relation = construct_relation_for_association_find(construct_join_dependency) + return false if ActiveRecord::NullRelation === relation + relation = relation.except(:select, :order).select("1 AS one").limit(1) case conditions @@ -172,8 +173,6 @@ module ActiveRecord end connection.select_value(relation, "#{name} Exists", relation.bind_values) - rescue ThrowResult - false end # This method is called whenever no records are found with either a single @@ -203,10 +202,12 @@ module ActiveRecord def find_with_associations join_dependency = construct_join_dependency relation = construct_relation_for_association_find(join_dependency) - rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) - join_dependency.instantiate(rows) - rescue ThrowResult - [] + if ActiveRecord::NullRelation === relation + [] + else + rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) + join_dependency.instantiate(rows) + end end def construct_join_dependency(joins = []) @@ -230,21 +231,22 @@ module ActiveRecord if using_limitable_reflections?(join_dependency.reflections) relation else - relation.where!(construct_limited_ids_condition(relation)) if relation.limit_value + if relation.limit_value + limited_ids = limited_ids_for(relation) + limited_ids.empty? ? relation.none! : relation.where!(table[primary_key].in(limited_ids)) + end relation.except(:limit, :offset) end end - def construct_limited_ids_condition(relation) + def limited_ids_for(relation) values = @klass.connection.columns_for_distinct( "#{quoted_table_name}.#{quoted_primary_key}", relation.order_values) relation = relation.except(:select).select(values).distinct! id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) - ids_array = id_rows.map {|row| row[primary_key]} - - ids_array.empty? ? raise(ThrowResult) : table[primary_key].in(ids_array) + id_rows.map {|row| row[primary_key]} end def find_with_ids(*ids) -- cgit v1.2.3 From 928a58b2b86c4de966654743ea7d693efb8cd98f Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Tue, 21 May 2013 00:32:34 -0600 Subject: update_counters accepts a hash, not an array of hashes --- activerecord/lib/active_record/counter_cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 81cca37939..e1faadf1ab 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -50,7 +50,7 @@ module ActiveRecord # ==== Parameters # # * +id+ - The id of the object you wish to update a counter on or an Array of ids. - # * +counters+ - An Array of Hashes containing the names of the fields + # * +counters+ - A Hash containing the names of the fields # to update as keys and the amount to update the field by as values. # # ==== Examples -- cgit v1.2.3 From 14f70dfae144f917f099266082cc7a7c9dd065e4 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 23 May 2013 10:57:31 -0400 Subject: enhanced comments for foreign_key_present? method --- activerecord/lib/active_record/associations/association.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 608a6af16c..ee62298793 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -200,13 +200,14 @@ module ActiveRecord creation_attributes.each { |key, value| record[key] = value } end - # Should be true if there is a foreign key present on the owner which + # Returns true if there is a foreign key present on the owner which # references the target. This is used to determine whether we can load # the target if the owner is currently a new record (and therefore - # without a key). + # without a key). If the owner is a new record then foreign_key must + # be present in order to load target. # # Currently implemented by belongs_to (vanilla and polymorphic) and - # has_one/has_many :through associations which go through a belongs_to + # has_one/has_many :through associations which go through a belongs_to. def foreign_key_present? false end -- cgit v1.2.3 From 04f00c62178ac94deab3d5541c0a53fa60dd35ea Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Mon, 27 May 2013 14:51:06 -0400 Subject: minor comments cleanup --- activerecord/lib/active_record/inheritance.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 40976bc29e..e826762def 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -5,7 +5,7 @@ module ActiveRecord extend ActiveSupport::Concern included do - # Determine whether to store the full constant name including namespace when using STI + # Determines whether to store the full constant name including namespace when using STI. class_attribute :store_full_sti_class, instance_writer: false self.store_full_sti_class = true end @@ -13,7 +13,7 @@ module ActiveRecord module ClassMethods # Determines if one of the attributes passed in is the inheritance column, # and if the inheritance column is attr accessible, it initializes an - # instance of the given subclass instead of the base class + # instance of the given subclass instead of the base class. def new(*args, &block) if abstract_class? || self == Base raise NotImplementedError, "#{self} is an abstract class and can not be instantiated." @@ -27,7 +27,8 @@ module ActiveRecord super end - # True if this isn't a concrete subclass needing a STI type condition. + # Returns +true+ if this does not need STI type condition. Returns + # +false+ if STI type condition needs to be applied. def descends_from_active_record? if self == Base false -- cgit v1.2.3 From 348b9824ed2882f400a5e8f966fceba7caa689d2 Mon Sep 17 00:00:00 2001 From: Thiago Pinto Date: Fri, 7 Jun 2013 00:08:36 -0400 Subject: using Model.all.find_each in rails 3 raises an error and should not be recommended --- activerecord/lib/active_record/relation/batches.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index b921f2eddb..2f21b65ee4 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -11,7 +11,7 @@ module ActiveRecord # The #find_each method uses #find_in_batches with a batch size of 1000 (or as # specified by the +:batch_size+ option). # - # Person.all.find_each do |person| + # Person.find_each do |person| # person.do_awesome_stuff # end # @@ -50,7 +50,7 @@ module ActiveRecord # end # # # Let's process the next 2000 records - # Person.all.find_in_batches(start: 2000, batch_size: 2000) do |group| + # Person.find_in_batches(start: 2000, batch_size: 2000) do |group| # group.each { |person| person.party_all_night! } # end def find_in_batches(options = {}) -- cgit v1.2.3 From 3f18fa008c352a2f3dbfcda6d5044225050b891c Mon Sep 17 00:00:00 2001 From: Thiago Pinto Date: Fri, 7 Jun 2013 00:37:13 -0400 Subject: lists the options for find_each and find_in_batches --- activerecord/lib/active_record/relation/batches.rb | 54 +++++++++++++++------- 1 file changed, 37 insertions(+), 17 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 2f21b65ee4..41291844fc 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -19,8 +19,26 @@ module ActiveRecord # person.party_all_night! # end # - # You can also pass the +:start+ option to specify - # an offset to control the starting point. + # ==== Options + # * :batch_size - Specifies the size of the batch. Default to 1000. + # * :start - Specifies the starting point for the batch processing. + # This is especially useful if you want multiple workers dealing with + # the same processing queue. You can make worker 1 handle all the records + # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond + # (by setting the +:start+ option on that worker). + # + # # Let's process for a batch of 2000 records, skiping the first 2000 rows + # Person.find_each(start: 2000, batch_size: 2000) do |person| + # person.party_all_night! + # end + # + # NOTE: It's not possible to set the order. That is automatically set to + # ascending on the primary key ("id ASC") to make the batch ordering + # work. This also means that this method only works with integer-based + # primary keys. + # + # NOTE: You can't set the limit either, that's used to control + # the batch sizes. def find_each(options = {}) find_in_batches(options) do |records| records.each { |record| yield record } @@ -28,31 +46,33 @@ module ActiveRecord end # Yields each batch of records that was found by the find +options+ as - # an array. The size of each batch is set by the +:batch_size+ - # option; the default is 1000. - # - # You can control the starting point for the batch processing by - # supplying the +:start+ option. This is especially useful if you - # want multiple workers dealing with the same processing queue. You can - # make worker 1 handle all the records between id 0 and 10,000 and - # worker 2 handle from 10,000 and beyond (by setting the +:start+ - # option on that worker). - # - # It's not possible to set the order. That is automatically set to - # ascending on the primary key ("id ASC") to make the batch ordering - # work. This also means that this method only works with integer-based - # primary keys. You can't set the limit either, that's used to control - # the batch sizes. + # an array. # # Person.where("age > 21").find_in_batches do |group| # sleep(50) # Make sure it doesn't get too crowded in there! # group.each { |person| person.party_all_night! } # end # + # ==== Options + # * :batch_size - Specifies the size of the batch. Default to 1000. + # * :start - Specifies the starting point for the batch processing. + # This is especially useful if you want multiple workers dealing with + # the same processing queue. You can make worker 1 handle all the records + # between id 0 and 10,000 and worker 2 handle from 10,000 and beyond + # (by setting the +:start+ option on that worker). + # # # Let's process the next 2000 records # Person.find_in_batches(start: 2000, batch_size: 2000) do |group| # group.each { |person| person.party_all_night! } # end + # + # NOTE: It's not possible to set the order. That is automatically set to + # ascending on the primary key ("id ASC") to make the batch ordering + # work. This also means that this method only works with integer-based + # primary keys. + # + # NOTE: You can't set the limit either, that's used to control + # the batch sizes. def find_in_batches(options = {}) options.assert_valid_keys(:start, :batch_size) -- cgit v1.2.3 From 4e2bec383239f9ab3493208ef3bad1f2cbea3c93 Mon Sep 17 00:00:00 2001 From: David Celis Date: Thu, 9 May 2013 20:31:31 +0530 Subject: ActiveRecord::Relation#blank? should `LIMIT 1` This is an SQL improvement to ActiveRecord::Relation#blank?. Currently, it calls `to_a` on the Relation, which loads all records in the association, and calls `blank?` on the loaded Array. There are other ways, however, to check the emptiness of an association that are far more performant. `#empty?`, `#exists?` and `#any?` all attach a `LIMIT 1` to the SQL query before firing it off, which is a nice query improvement. `#blank?` should do the same! Bonus performance improvements will also happen for `#present?`, which merely calls the negation of `#blank?` Signed-off-by: David Celis --- activerecord/lib/active_record/relation.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d54479edbb..5e525a49c0 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -21,6 +21,7 @@ module ActiveRecord alias :model :klass alias :loaded? :loaded alias :default_scoped? :default_scoped + alias :blank? :empty? def initialize(klass, table, values = {}) @klass = klass @@ -575,11 +576,6 @@ module ActiveRecord end end - # Returns true if relation is blank. - def blank? - to_a.blank? - end - def values Hash[@values] end -- cgit v1.2.3 From 2b763131eacaae5bff9ffb5015fbf367d594dc64 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 7 Jun 2013 18:41:11 +0100 Subject: Revert "Merge pull request #10539 from davidcelis/ar-sql-improvements" This reverts commit 257fa6897d9c85da16b7c9fcb4ae3008198d320e, reversing changes made to 94725b81f5588e4b0f43222c4f142c3135941b4b. The build failed https://travis-ci.org/rails/rails/builds/7883546 --- activerecord/lib/active_record/relation.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 5e525a49c0..d54479edbb 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -21,7 +21,6 @@ module ActiveRecord alias :model :klass alias :loaded? :loaded alias :default_scoped? :default_scoped - alias :blank? :empty? def initialize(klass, table, values = {}) @klass = klass @@ -576,6 +575,11 @@ module ActiveRecord end end + # Returns true if relation is blank. + def blank? + to_a.blank? + end + def values Hash[@values] end -- cgit v1.2.3 From d6b03a376787ec9c9e934e5688a38c576f2e39b7 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Fri, 7 Jun 2013 20:59:27 -0700 Subject: Getting rid of the +automatic_inverse_of: false+ option in associations in favor of using +inverse_of: false+ option. Changing the documentation and adding a CHANGELOG entry for the automatic inverse detection feature. --- activerecord/lib/active_record/associations.rb | 11 ++++++----- .../lib/active_record/associations/builder/has_many.rb | 2 +- .../associations/builder/singular_association.rb | 2 +- activerecord/lib/active_record/nested_attributes.rb | 4 ++++ activerecord/lib/active_record/reflection.rb | 14 +++++++------- 5 files changed, 19 insertions(+), 14 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 3490057298..6fd4f3042c 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -586,9 +586,10 @@ module ActiveRecord # belongs_to :tag, inverse_of: :taggings # end # - # If you do not set the +:inverse_of+ record, the association will do its - # best to match itself up with the correct inverse. Automatic +:inverse_of+ - # detection only works on +has_many+, +has_one+, and +belongs_to+ associations. + # If you do not set the :inverse_of record, the association will + # do its best to match itself up with the correct inverse. Automatic + # inverse detection only works on has_many, has_one, and + # belongs_to associations. # # Extra options on the associations, as defined in the # AssociationReflection::INVALID_AUTOMATIC_INVERSE_OPTIONS constant, will @@ -599,10 +600,10 @@ module ActiveRecord # especially the ones with non-standard names. # # You can turn off the automatic detection of inverse associations by setting - # the +:automatic_inverse_of+ option to +false+ like so: + # the :inverse_of option to false like so: # # class Taggable < ActiveRecord::Base - # belongs_to :tag, automatic_inverse_of: false + # belongs_to :tag, inverse_of: false # end # # == Nested \Associations diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 429def5455..0d1bdd21ee 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -5,7 +5,7 @@ module ActiveRecord::Associations::Builder end def valid_options - super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :automatic_inverse_of, :counter_cache] + super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache] end def valid_dependent_options diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 96ccbeb8a3..76e48e66e5 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -3,7 +3,7 @@ module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: def valid_options - super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of, :automatic_inverse_of] + super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of] end def constructable? diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 8bdaeef924..0e8822d63f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -230,6 +230,10 @@ module ActiveRecord # validates_presence_of :member # end # + # Note that if you do not specify the inverse_of option, then + # Active Record will try to automatically guess the inverse association + # based on heuristics. + # # For one-to-one nested associations, if you build the new (in-memory) # child object yourself before assignment, then this module will not # overwrite it, e.g.: diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 27aa20b6c0..2ba89b13b7 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -313,8 +313,7 @@ module ActiveRecord # and prevents this object from finding the inverse association # automatically in the future. def remove_automatic_inverse_of! - @automatic_inverse_of = nil - options[:automatic_inverse_of] = false + @automatic_inverse_of = false end def polymorphic_inverse_of(associated_class) @@ -449,15 +448,16 @@ module ActiveRecord # Checks to see if the reflection doesn't have any options that prevent # us from being able to guess the inverse automatically. First, the - # +automatic_inverse_of+ option cannot be set to false. Second, we must - # have +has_many+, +has_one+, +belongs_to+ associations. Third, we must - # not have options such as +:polymorphic+ or +:foreign_key+ which prevent us - # from correctly guessing the inverse association. + # inverse_of option cannot be set to false. Second, we must + # have has_many, has_one, belongs_to associations. + # Third, we must not have options such as :polymorphic or + # :foreign_key which prevent us from correctly guessing the + # inverse association. # # Anything with a scope can additionally ruin our attempt at finding an # inverse, so we exclude reflections with scopes. def can_find_inverse_of_automatically?(reflection) - reflection.options[:automatic_inverse_of] != false && + reflection.options[:inverse_of] != false && VALID_AUTOMATIC_INVERSE_MACROS.include?(reflection.macro) && !INVALID_AUTOMATIC_INVERSE_OPTIONS.any? { |opt| reflection.options[opt] } && !reflection.scope -- cgit v1.2.3 From b8640d47907876d43335628fea40da03df0e84cd Mon Sep 17 00:00:00 2001 From: Thiago Pinto Date: Sat, 8 Jun 2013 13:35:47 -0400 Subject: explaining ActiveRecord#first in rails 3 and 4 --- activerecord/lib/active_record/relation/finder_methods.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ba222aac93..86976077d6 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -79,6 +79,19 @@ module ActiveRecord # Person.where(["user_name = :u", { u: user_name }]).first # Person.order("created_on DESC").offset(5).first # Person.first(3) # returns the first three objects fetched by SELECT * FROM people LIMIT 3 + # + # ==== Rails 3 + # + # Person.first # SELECT "users".* FROM "users" LIMIT 1 + # + # NOTE: Rails 3 may not +order+ this query by be the primary key. + # The order will depend on the database implementation. + # In order to ensure that behavior use User.order(:id).first instead. + # + # ==== Rails 4 + # + # Person.first # SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1 + # def first(limit = nil) if limit if order_values.empty? && primary_key -- cgit v1.2.3 From 667569ab04159b144d799f5fc97c3d862b2c30aa Mon Sep 17 00:00:00 2001 From: Thiago Pinto Date: Sat, 8 Jun 2013 15:01:15 -0400 Subject: instructions for variations and alternatives for ActiveRecord#find --- .../lib/active_record/relation/finder_methods.rb | 36 +++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 86976077d6..e716ce9675 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -11,7 +11,9 @@ module ActiveRecord # Person.find([1]) # returns an array for the object with ID = 1 # Person.where("administrator = 1").order("created_on DESC").find(1) # - # Note that returned records may not be in the same order as the ids you + # NOTE: An RecordNotFound will be raised if one or more ids are not returned. + # + # NOTE: that returned records may not be in the same order as the ids you # provide since database rows are unordered. Give an explicit order # to ensure the results are sorted. # @@ -28,6 +30,38 @@ module ActiveRecord # person.visits += 1 # person.save! # end + # + # ==== Variations of +find+ + # + # Person.where(name: 'Spartacus', rating: 4) + # # returns a chainable list (which can be empty) + # + # Person.find_by(name: 'Spartacus', rating: 4) + # # returns the first item or nil + # + # Person.where(name: 'Spartacus', rating: 4).first_or_initialize + # # returns the first item or returns a new instance (requires you call .save to persist against the database) + # + # Person.where(name: 'Spartacus', rating: 4).first_or_create + # # returns the first item or creates it and returns it, available since rails 3.2.1 + # + # + # ==== Alternatives for +find+ + # + # Person.where(name: 'Spartacus', rating: 4).exists?(conditions = :none) + # # returns true or false + # + # Person.where(name: 'Spartacus', rating: 4).select("field1, field2, field3") + # # returns a chainable list of instances with only the mentioned fields + # + # Person.where(name: 'Spartacus', rating: 4).ids + # # returns an Array of ids, available since rails 3.2.1 + # + # Person.where(name: 'Spartacus', rating: 4).pluck(:field1, :field2) + # # returns an Array of the required fields, available since rails 3.1 + # + # Person.arel_table + # # returns an instance of Arel::Table, which allows a comprehensive variety of filters def find(*args) if block_given? to_a.find { |*block_args| yield(*block_args) } -- cgit v1.2.3 From 763e5a866283c83453de0c728e1b973afd41f934 Mon Sep 17 00:00:00 2001 From: Thiago Pinto Date: Sat, 8 Jun 2013 15:02:30 -0400 Subject: doc: renaming table name to follow the file's standards --- activerecord/lib/active_record/relation/finder_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index e716ce9675..f373714007 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -116,7 +116,7 @@ module ActiveRecord # # ==== Rails 3 # - # Person.first # SELECT "users".* FROM "users" LIMIT 1 + # Person.first # SELECT "people".* FROM "people" LIMIT 1 # # NOTE: Rails 3 may not +order+ this query by be the primary key. # The order will depend on the database implementation. @@ -124,7 +124,7 @@ module ActiveRecord # # ==== Rails 4 # - # Person.first # SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT 1 + # Person.first # SELECT "people".* FROM "people" ORDER BY "people"."id" ASC LIMIT 1 # def first(limit = nil) if limit -- cgit v1.2.3 From db519c0c9a8d3d4f5afa2029408de5a860e037c1 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 13 May 2013 17:13:55 +0100 Subject: cleanup whitespace in relation.rb --- activerecord/lib/active_record/relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d54479edbb..d37471e9ad 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -247,7 +247,7 @@ module ActiveRecord def empty? return @records.empty? if loaded? - c = count + c = count(:all) c.respond_to?(:zero?) ? c.zero? : c.empty? end -- cgit v1.2.3 From da9b5d4a8435b744fcf278fffd6d7f1e36d4a4f2 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 13 May 2013 18:31:00 +0100 Subject: Remove fall back and column restrictions for `count`. --- activerecord/lib/active_record/relation/calculations.rb | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index ccb48247b7..4becf3980d 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -207,15 +207,18 @@ module ActiveRecord end if operation == "count" - column_name ||= (select_for_count || :all) + if select_values.present? + column_name ||= select_values.join(", ") + else + column_name ||= :all + end unless arel.ast.grep(Arel::Nodes::OuterJoin).empty? distinct = true end column_name = primary_key if column_name == :all && distinct - - distinct = nil if column_name =~ /\s*DISTINCT\s+/i + distinct = nil if column_name =~ /\s*DISTINCT[\s(]+/i end if group_values.any? @@ -376,13 +379,6 @@ module ActiveRecord column ? column.type_cast(value) : value end - def select_for_count - if select_values.present? - select = select_values.join(", ") - select if select !~ /[,*]/ - end - end - def build_count_subquery(relation, column_name, distinct) column_alias = Arel.sql('count_column') subquery_alias = Arel.sql('subquery_for_count') -- cgit v1.2.3 From d5450a6659a33ef08a170afcaae084023bcd275b Mon Sep 17 00:00:00 2001 From: Dmitry Polushkin Date: Sun, 9 Jun 2013 19:36:39 +0200 Subject: Refactored ActiveRecord `first` method to get rid of duplication. --- .../lib/active_record/relation/finder_methods.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 7ddaea1bb0..cf71df8fba 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -81,11 +81,7 @@ module ActiveRecord # Person.first(3) # returns the first three objects fetched by SELECT * FROM people LIMIT 3 def first(limit = nil) if limit - if order_values.empty? && primary_key - order(arel_table[primary_key].asc).limit(limit).to_a - else - limit(limit).to_a - end + find_first_records(order_values, limit) else find_first end @@ -307,12 +303,15 @@ module ActiveRecord if loaded? @records.first else - @first ||= - if with_default_scope.order_values.empty? && primary_key - order(arel_table[primary_key].asc).limit(1).to_a.first - else - limit(1).to_a.first - end + @first ||= find_first_records(with_default_scope.order_values, 1).first + end + end + + def find_first_records(order_values, limit) + if order_values.empty? && primary_key + order(arel_table[primary_key].asc).limit(limit).to_a + else + limit(limit).to_a end end -- cgit v1.2.3 From e720b50fb1ff841970b2f1198996144c35b48461 Mon Sep 17 00:00:00 2001 From: Dmitry Polushkin Date: Mon, 10 Jun 2013 14:38:32 +0200 Subject: rename method `find_first_records` to `find_first_with_limit` --- activerecord/lib/active_record/relation/finder_methods.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index cf71df8fba..f240d0aaa9 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -81,7 +81,7 @@ module ActiveRecord # Person.first(3) # returns the first three objects fetched by SELECT * FROM people LIMIT 3 def first(limit = nil) if limit - find_first_records(order_values, limit) + find_first_with_limit(order_values, limit) else find_first end @@ -303,11 +303,11 @@ module ActiveRecord if loaded? @records.first else - @first ||= find_first_records(with_default_scope.order_values, 1).first + @first ||= find_first_with_limit(with_default_scope.order_values, 1).first end end - def find_first_records(order_values, limit) + def find_first_with_limit(order_values, limit) if order_values.empty? && primary_key order(arel_table[primary_key].asc).limit(limit).to_a else -- cgit v1.2.3 From 8777ae1f4ac42dd47c3455cf40c9cf59daad7ca9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 10:33:16 -0700 Subject: reduce evals in depdendent associations --- activerecord/lib/active_record/associations/builder/association.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 6a3658f328..5804cb7fed 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -117,7 +117,8 @@ module ActiveRecord::Associations::Builder end CODE - model.before_destroy "#{macro}_dependent_for_#{name}" + method = "#{macro}_dependent_for_#{name}" + model.before_destroy lambda { |o| o.public_send method } end def valid_dependent_options -- cgit v1.2.3 From 0d22947e00820b4e011775cfb4ede109650db070 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 10:38:57 -0700 Subject: remove evals from the association --- .../lib/active_record/associations/builder/association.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 5804cb7fed..3254da4677 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -111,14 +111,8 @@ module ActiveRecord::Associations::Builder ) end - mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def #{macro}_dependent_for_#{name} - association(:#{name}).handle_dependency - end - CODE - - method = "#{macro}_dependent_for_#{name}" - model.before_destroy lambda { |o| o.public_send method } + n = name + model.before_destroy lambda { |o| o.association(n).handle_dependency } end def valid_dependent_options -- cgit v1.2.3 From 0d3589ea12d4b564f61659ad47d78bcfbc623a91 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 11:34:53 -0700 Subject: adding callbacks should be private --- .../lib/active_record/associations/builder/belongs_to.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 63e9526436..f2b6a8dfb5 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -19,6 +19,12 @@ module ActiveRecord::Associations::Builder reflection end + def valid_dependent_options + [:destroy, :delete] + end + + private + def add_counter_cache_callbacks(reflection) cache_column = reflection.counter_cache_column foreign_key = reflection.foreign_key @@ -92,9 +98,5 @@ module ActiveRecord::Associations::Builder model.after_touch "belongs_to_touch_after_save_or_destroy_for_#{name}" model.after_destroy "belongs_to_touch_after_save_or_destroy_for_#{name}" end - - def valid_dependent_options - [:destroy, :delete] - end end end -- cgit v1.2.3 From ebd3aedbb477f7e5f3416e781fc82ead7e27f872 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 11:43:04 -0700 Subject: remove evaled belongs_to counter cache method --- .../associations/builder/belongs_to.rb | 30 ++++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index f2b6a8dfb5..2483ea3db3 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -25,18 +25,27 @@ module ActiveRecord::Associations::Builder private + def add_counter_cache_methods(mixin) + return if mixin.method_defined? :belongs_to_counter_cache_after_create + + mixin.class_eval do + def belongs_to_counter_cache_after_create(association, reflection) + if record = send(association.name) + cache_column = reflection.counter_cache_column + record.class.increment_counter(cache_column, record.id) + @_after_create_counter_called = true + end + end + end + end + def add_counter_cache_callbacks(reflection) cache_column = reflection.counter_cache_column foreign_key = reflection.foreign_key - mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def belongs_to_counter_cache_after_create_for_#{name} - if record = #{name} - record.class.increment_counter(:#{cache_column}, record.id) - @_after_create_counter_called = true - end - end + add_counter_cache_methods mixin + mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 def belongs_to_counter_cache_before_destroy_for_#{name} unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == #{foreign_key.to_sym.inspect} record = #{name} @@ -64,7 +73,12 @@ module ActiveRecord::Associations::Builder end CODE - model.after_create "belongs_to_counter_cache_after_create_for_#{name}" + association = self + + model.after_create lambda { |o| + o.belongs_to_counter_cache_after_create(association, reflection) + } + model.before_destroy "belongs_to_counter_cache_before_destroy_for_#{name}" model.after_update "belongs_to_counter_cache_after_update_for_#{name}" -- cgit v1.2.3 From c9d6c1b4afcaf39bf0e83fcf32b13093c94c1247 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 11:47:24 -0700 Subject: push before_destroy counter cache method to a single method --- .../associations/builder/belongs_to.rb | 29 +++++++++++++--------- 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 2483ea3db3..51d24c1db8 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -36,6 +36,17 @@ module ActiveRecord::Associations::Builder @_after_create_counter_called = true end end + + def belongs_to_counter_cache_before_destroy(association, reflection) + foreign_key = reflection.foreign_key.to_sym + unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == foreign_key + record = send association.name + if record && !self.destroyed? + cache_column = reflection.counter_cache_column + record.class.decrement_counter(cache_column, record.id) + end + end + end end end @@ -46,15 +57,6 @@ module ActiveRecord::Associations::Builder add_counter_cache_methods mixin mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def belongs_to_counter_cache_before_destroy_for_#{name} - unless destroyed_by_association && destroyed_by_association.foreign_key.to_sym == #{foreign_key.to_sym.inspect} - record = #{name} - if record && !self.destroyed? - record.class.decrement_counter(:#{cache_column}, record.id) - end - end - end - def belongs_to_counter_cache_after_update_for_#{name} if (@_after_create_counter_called ||= false) @_after_create_counter_called = false @@ -75,11 +77,14 @@ module ActiveRecord::Associations::Builder association = self - model.after_create lambda { |o| - o.belongs_to_counter_cache_after_create(association, reflection) + model.after_create lambda { |record| + record.belongs_to_counter_cache_after_create(association, reflection) + } + + model.before_destroy lambda { |record| + record.belongs_to_counter_cache_before_destroy(association, reflection) } - model.before_destroy "belongs_to_counter_cache_before_destroy_for_#{name}" model.after_update "belongs_to_counter_cache_after_update_for_#{name}" klass = reflection.class_name.safe_constantize -- cgit v1.2.3 From 9ca9ff3fbe704310552d5705138dd8ac31d7ab3c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 11:53:30 -0700 Subject: push belongs_to counter cache method to a single method --- .../associations/builder/belongs_to.rb | 35 +++++++++++----------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 51d24c1db8..e075665c30 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -47,34 +47,33 @@ module ActiveRecord::Associations::Builder end end end - end - end - def add_counter_cache_callbacks(reflection) - cache_column = reflection.counter_cache_column - foreign_key = reflection.foreign_key + def belongs_to_counter_cache_after_update(association, reflection) + foreign_key = reflection.foreign_key + name = association.name + cache_column = reflection.counter_cache_column - add_counter_cache_methods mixin - - mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def belongs_to_counter_cache_after_update_for_#{name} if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif self.#{foreign_key}_changed? && !new_record? && defined?(#{name.to_s.camelize}) - model = #{name.to_s.camelize} - foreign_key_was = self.#{foreign_key}_was - foreign_key = self.#{foreign_key} + elsif self.send("#{foreign_key}_changed?") && !new_record? && Object.const_defined?(name.to_s.camelize) + model = name.to_s.camelize.constantize + foreign_key_was = self.send("#{foreign_key}_was") + foreign_key = self.send foreign_key if foreign_key && model.respond_to?(:increment_counter) - model.increment_counter(:#{cache_column}, foreign_key) + model.increment_counter(cache_column, foreign_key) end if foreign_key_was && model.respond_to?(:decrement_counter) - model.decrement_counter(:#{cache_column}, foreign_key_was) + model.decrement_counter(cache_column, foreign_key_was) end end end - CODE + end + end + def add_counter_cache_callbacks(reflection) + cache_column = reflection.counter_cache_column + add_counter_cache_methods mixin association = self model.after_create lambda { |record| @@ -85,7 +84,9 @@ module ActiveRecord::Associations::Builder record.belongs_to_counter_cache_before_destroy(association, reflection) } - model.after_update "belongs_to_counter_cache_after_update_for_#{name}" + model.after_update lambda { |record| + record.belongs_to_counter_cache_after_update(association, reflection) + } klass = reflection.class_name.safe_constantize klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) -- cgit v1.2.3 From e79fae55847506d138a3eca616b0c6e3ee560e26 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 11:59:50 -0700 Subject: use attribute methods for finding key values rather than generating method names --- activerecord/lib/active_record/associations/builder/belongs_to.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index e075665c30..aaef86d5b9 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -55,10 +55,10 @@ module ActiveRecord::Associations::Builder if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif self.send("#{foreign_key}_changed?") && !new_record? && Object.const_defined?(name.to_s.camelize) + elsif attribute_changed?(foreign_key) && !new_record? && Object.const_defined?(name.to_s.camelize) model = name.to_s.camelize.constantize - foreign_key_was = self.send("#{foreign_key}_was") - foreign_key = self.send foreign_key + foreign_key_was = attribute_was foreign_key + foreign_key = attribute foreign_key if foreign_key && model.respond_to?(:increment_counter) model.increment_counter(cache_column, foreign_key) -- cgit v1.2.3 From bf28422cbda9068afb900020fa6911afd01963e1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 12:01:29 -0700 Subject: check whether the association is constructible rather than checking constants --- activerecord/lib/active_record/associations/builder/belongs_to.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index aaef86d5b9..783f4e857a 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -55,10 +55,10 @@ module ActiveRecord::Associations::Builder if (@_after_create_counter_called ||= false) @_after_create_counter_called = false - elsif attribute_changed?(foreign_key) && !new_record? && Object.const_defined?(name.to_s.camelize) - model = name.to_s.camelize.constantize + elsif attribute_changed?(foreign_key) && !new_record? && association.constructable? + model = reflection.klass foreign_key_was = attribute_was foreign_key - foreign_key = attribute foreign_key + foreign_key = attribute foreign_key if foreign_key && model.respond_to?(:increment_counter) model.increment_counter(cache_column, foreign_key) -- cgit v1.2.3 From 3680074b4de40e52a3d304268b83bb9abb76fb1e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 12:03:01 -0700 Subject: remove unused variable --- activerecord/lib/active_record/associations/builder/belongs_to.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 783f4e857a..6b364cdb7f 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -50,7 +50,6 @@ module ActiveRecord::Associations::Builder def belongs_to_counter_cache_after_update(association, reflection) foreign_key = reflection.foreign_key - name = association.name cache_column = reflection.counter_cache_column if (@_after_create_counter_called ||= false) -- cgit v1.2.3 From 4b824b052046f3ec0cea6cad2adc3ce0bcd5c9fe Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 13:29:59 -0700 Subject: push the touch method outside the eval --- .../associations/builder/belongs_to.rb | 47 ++++++++++++++++------ 1 file changed, 34 insertions(+), 13 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 6b364cdb7f..f2b64fb6f4 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -91,31 +91,52 @@ module ActiveRecord::Associations::Builder klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) end - def add_touch_callbacks(reflection) - mixin.class_eval <<-CODE, __FILE__, __LINE__ + 1 - def belongs_to_touch_after_save_or_destroy_for_#{name} - foreign_key_field = #{reflection.foreign_key.inspect} - old_foreign_id = attribute_was(foreign_key_field) + def add_touch_methods(mixin) + return if mixin.method_defined? :belongs_to_touch_after_save_or_destroy + + mixin.class_eval do + def belongs_to_touch_after_save_or_destroy(foreign_key, name, touch) + old_foreign_id = attribute_was(foreign_key) if old_foreign_id - klass = association(#{name.inspect}).klass + klass = association(name).klass old_record = klass.find_by(klass.primary_key => old_foreign_id) if old_record - old_record.touch #{options[:touch].inspect if options[:touch] != true} + if touch != true + old_record.touch touch + else + old_record.touch + end end end - record = #{name} + record = send name unless record.nil? || record.new_record? - record.touch #{options[:touch].inspect if options[:touch] != true} + if touch != true + record.touch touch + else + record.touch + end end end - CODE + end + end + + def add_touch_callbacks(reflection) + add_touch_methods(mixin) + + foreign_key = reflection.foreign_key + n = name + touch = options[:touch] + + callback = lambda { |record| + record.belongs_to_touch_after_save_or_destroy foreign_key, n, touch + } - model.after_save "belongs_to_touch_after_save_or_destroy_for_#{name}" - model.after_touch "belongs_to_touch_after_save_or_destroy_for_#{name}" - model.after_destroy "belongs_to_touch_after_save_or_destroy_for_#{name}" + model.after_save callback + model.after_touch callback + model.after_destroy callback end end end -- cgit v1.2.3 From 701e48e4acef827522f5c53e1602edb1485ea21b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 13:40:39 -0700 Subject: stop adding a new method for touch callbacks --- .../associations/builder/belongs_to.rb | 50 +++++++++------------- 1 file changed, 21 insertions(+), 29 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index f2b64fb6f4..d4e1a0dda1 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -91,47 +91,39 @@ module ActiveRecord::Associations::Builder klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly) end - def add_touch_methods(mixin) - return if mixin.method_defined? :belongs_to_touch_after_save_or_destroy - - mixin.class_eval do - def belongs_to_touch_after_save_or_destroy(foreign_key, name, touch) - old_foreign_id = attribute_was(foreign_key) - - if old_foreign_id - klass = association(name).klass - old_record = klass.find_by(klass.primary_key => old_foreign_id) - - if old_record - if touch != true - old_record.touch touch - else - old_record.touch - end - end + def self.touch_record(o, foreign_key, name, touch) # :nodoc: + old_foreign_id = o.attribute_was(foreign_key) + + if old_foreign_id + klass = o.association(name).klass + old_record = klass.find_by(klass.primary_key => old_foreign_id) + + if old_record + if touch != true + old_record.touch touch + else + old_record.touch end + end + end - record = send name - unless record.nil? || record.new_record? - if touch != true - record.touch touch - else - record.touch - end - end + record = o.send name + unless record.nil? || record.new_record? + if touch != true + record.touch touch + else + record.touch end end end def add_touch_callbacks(reflection) - add_touch_methods(mixin) - foreign_key = reflection.foreign_key n = name touch = options[:touch] callback = lambda { |record| - record.belongs_to_touch_after_save_or_destroy foreign_key, n, touch + BelongsTo.touch_record(record, foreign_key, n, touch) } model.after_save callback -- cgit v1.2.3 From de9e9bbbada37626ae32aadc1ae6055b61452817 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 11 Jun 2013 15:13:04 -0700 Subject: bind values should not be merged between scopes --- activerecord/lib/active_record/associations/association_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index aa5551fe0c..f1bec5787a 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -96,7 +96,7 @@ module ActiveRecord item = eval_scope(klass, scope_chain_item) if scope_chain_item == self.reflection.scope - scope.merge! item.except(:where, :includes) + scope.merge! item.except(:where, :includes, :bind) end scope.includes! item.includes_values -- cgit v1.2.3 From d7e23109136733995bfadbe461ae18680a7426b5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Jun 2013 11:47:02 -0700 Subject: we should apply the default scope before querying --- activerecord/lib/active_record/relation/finder_methods.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index f0edef58bf..cbb2803593 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -172,7 +172,8 @@ module ActiveRecord relation = relation.where(table[primary_key].eq(conditions)) if conditions != :none end - connection.select_value(relation, "#{name} Exists", relation.bind_values) + relation = relation.with_default_scope + connection.select_value(relation.arel, "#{name} Exists", relation.bind_values) end # This method is called whenever no records are found with either a single -- cgit v1.2.3 From 0ee351b428e038394d495c20a868d40ad3032e83 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Jun 2013 16:36:36 -0700 Subject: remove unnecessary is_a check --- activerecord/lib/active_record/reflection.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2ba89b13b7..5ea103f6e9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -66,8 +66,7 @@ module ActiveRecord # Invoice.reflect_on_association(:line_items).macro # returns :has_many # def reflect_on_association(association) - reflection = reflections[association] - reflection if reflection.is_a?(AssociationReflection) + reflections[association] end # Returns an array of AssociationReflection objects for all associations which have :autosave enabled. -- cgit v1.2.3 From 9d79333140f290eba7c91c48302469e5f73e604d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Jun 2013 16:58:22 -0700 Subject: split aggregates from association reflections to avoid is_a checks later --- activerecord/lib/active_record/reflection.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 5ea103f6e9..1e021765ae 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -5,7 +5,9 @@ module ActiveRecord included do class_attribute :reflections + class_attribute :aggregate_reflections self.reflections = {} + self.aggregate_reflections = {} end # \Reflection enables to interrogate Active Record classes and objects @@ -27,13 +29,18 @@ module ActiveRecord reflection = klass.new(macro, name, scope, options, active_record) - self.reflections = self.reflections.merge(name => reflection) + if klass == AggregateReflection + self.aggregate_reflections = self.aggregate_reflections.merge(name => reflection) + else + self.reflections = self.reflections.merge(name => reflection) + end + reflection end # Returns an array of AggregateReflection objects for all the aggregations in the class. def reflect_on_all_aggregations - reflections.values.grep(AggregateReflection) + aggregate_reflections.values end # Returns the AggregateReflection object for the named +aggregation+ (use the symbol). @@ -41,8 +48,7 @@ module ActiveRecord # Account.reflect_on_aggregation(:balance) # => the balance AggregateReflection # def reflect_on_aggregation(aggregation) - reflection = reflections[aggregation] - reflection if reflection.is_a?(AggregateReflection) + aggregate_reflections[aggregation] end # Returns an array of AssociationReflection objects for all the @@ -56,7 +62,7 @@ module ActiveRecord # Account.reflect_on_all_associations(:has_many) # returns an array of all has_many associations # def reflect_on_all_associations(macro = nil) - association_reflections = reflections.values.grep(AssociationReflection) + association_reflections = reflections.values macro ? association_reflections.select { |reflection| reflection.macro == macro } : association_reflections end -- cgit v1.2.3 From 9e7040d8a01819391c77a34fd185595d845f3aae Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Jun 2013 17:07:35 -0700 Subject: no need to cache hash lookups --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 1e021765ae..f5ea0edb72 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -533,7 +533,7 @@ module ActiveRecord # # => # def through_reflection - @through_reflection ||= active_record.reflect_on_association(options[:through]) + active_record.reflect_on_association(options[:through]) end # Returns an array of reflections which are involved in this association. Each item in the -- cgit v1.2.3 From b3bc3aa5cbe7540de21ae4eb566886e3b8efe6e3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 12 Jun 2013 18:56:23 -0700 Subject: sometimes singularize does not work, so we get a list of two strings. just uniq them --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index f5ea0edb72..2aec73bd5b 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -635,7 +635,7 @@ module ActiveRecord # # => [:tag, :tags] # def source_reflection_names - @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym } + @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }.uniq end def source_options -- cgit v1.2.3 From ea72430b8406c708033b39bbf152762e5ffaa7f8 Mon Sep 17 00:00:00 2001 From: jeran Date: Thu, 4 Apr 2013 19:12:15 -0400 Subject: Moving add_column_options! up to SchemaCreation removed two instances of add_column_options! from abstract_mysql_adapter reworked rename_column_sql to remove add_column_options from schema_statements changed to use new hash syntax. --- .../abstract/schema_definitions.rb | 3 ++ .../abstract/schema_statements.rb | 11 ------ .../connection_adapters/abstract_adapter.rb | 31 ++++++++++++----- .../connection_adapters/abstract_mysql_adapter.rb | 40 ++++++++++++---------- 4 files changed, 47 insertions(+), 38 deletions(-) (limited to 'activerecord/lib/active_record') 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 eb974e4a6e..553bce983a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -25,6 +25,9 @@ module ActiveRecord end end + class ChangeColumnDefinition < Struct.new(:column, :type, :options) #:nodoc: + end + # Represents the schema of an SQL table in an abstract way. This class # provides methods for manipulating the schema representation. # 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 aebcc2d874..742d198096 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -694,17 +694,6 @@ module ActiveRecord end end - 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[:null] == false - sql << " NOT NULL" - end - if options[:auto_increment] == true - sql << " AUTO_INCREMENT" - end - end - # SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause. # Both PostgreSQL and Oracle overrides this for custom DISTINCT syntax. # diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 1915c444ef..0b24c042ee 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -112,6 +112,12 @@ module ActiveRecord send m, o end + def visit_AddColumn(o) + sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale) + sql = "ADD #{quote_column_name(o.name)} #{sql_type}" + add_column_options!(sql, column_options(o)) + end + private def visit_AlterTable(o) @@ -119,12 +125,6 @@ module ActiveRecord sql << o.adds.map { |col| visit_AddColumn col }.join(' ') end - def visit_AddColumn(o) - sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale) - sql = "ADD #{quote_column_name(o.name)} #{sql_type}" - add_column_options!(sql, column_options(o)) - end - def visit_ColumnDefinition(o) sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale) column_sql = "#{quote_column_name(o.name)} #{sql_type}" @@ -145,6 +145,8 @@ module ActiveRecord column_options[:null] = o.null unless o.null.nil? column_options[:default] = o.default unless o.default.nil? column_options[:column] = o + column_options[:first] = o.first + column_options[:after] = o.after column_options end @@ -160,9 +162,20 @@ module ActiveRecord @conn.type_to_sql type.to_sym, limit, precision, scale end - def add_column_options!(column_sql, column_options) - @conn.add_column_options! column_sql, column_options - column_sql + def add_column_options!(sql, options) + sql << " DEFAULT #{@conn.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[:null] == false + sql << " NOT NULL" + end + if options[:auto_increment] == true + sql << " AUTO_INCREMENT" + end + sql + end + + def options_include_default?(options) + options.include?(:default) && !(options[:null] == false && options[:default].nil?) end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index b0b160f9b4..84bf3acf0e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -4,17 +4,26 @@ module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter class SchemaCreation < AbstractAdapter::SchemaCreation - private def visit_AddColumn(o) - add_column_position!(super, o) + add_column_position!(super, column_options(o)) + end + + private + def visit_ChangeColumnDefinition(o) + column = o.column + options = o.options + sql_type = type_to_sql(o.type, options[:limit], options[:precision], options[:scale]) + change_column_sql = "CHANGE #{quote_column_name(column.name)} #{quote_column_name(options[:name])} #{sql_type}" + add_column_options!(change_column_sql, options) + add_column_position!(change_column_sql, options) end - def add_column_position!(sql, column) - if column.first + def add_column_position!(sql, options) + if options[:first] sql << " FIRST" - elsif column.after - sql << " AFTER #{quote_column_name(column.after)}" + elsif options[:after] + sql << " AFTER #{quote_column_name(options[:after])}" end sql end @@ -659,10 +668,9 @@ module ActiveRecord end def add_column_sql(table_name, column_name, type, options = {}) - add_column_sql = "ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - add_column_options!(add_column_sql, options) - add_column_position!(add_column_sql, options) - add_column_sql + td = create_table_definition table_name, options[:temporary], options[:options] + cd = td.new_column_definition(column_name, type, options) + schema_creation.visit_AddColumn cd end def change_column_sql(table_name, column_name, type, options = {}) @@ -676,14 +684,12 @@ module ActiveRecord options[:null] = column.null end - change_column_sql = "CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" - add_column_options!(change_column_sql, options) - add_column_position!(change_column_sql, options) - change_column_sql + options[:name] = column.name + schema_creation.accept ChangeColumnDefinition.new column, type, options end def rename_column_sql(table_name, column_name, new_column_name) - options = {} + options = { name: new_column_name } if column = columns(table_name).find { |c| c.name == column_name.to_s } options[:default] = column.default @@ -694,9 +700,7 @@ module ActiveRecord end current_type = select_one("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE '#{column_name}'", 'SCHEMA')["Type"] - rename_column_sql = "CHANGE #{quote_column_name(column_name)} #{quote_column_name(new_column_name)} #{current_type}" - add_column_options!(rename_column_sql, options) - rename_column_sql + schema_creation.accept ChangeColumnDefinition.new column, current_type, options end def remove_column_sql(table_name, column_name, type = nil, options = {}) -- cgit v1.2.3 From b483a0d2a75bbec2f5eee363c88238cb612f07d6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 09:46:30 -0700 Subject: Ambiguous reflections are on :through relationships are no longer supported. For example, you need to change this: class Author < ActiveRecord::Base has_many :posts has_many :taggings, :through => :posts end class Post < ActiveRecord::Base has_one :tagging has_many :taggings end class Tagging < ActiveRecord::Base end To this: class Author < ActiveRecord::Base has_many :posts has_many :taggings, :through => :posts, :source => :tagging end class Post < ActiveRecord::Base has_one :tagging has_many :taggings end class Tagging < ActiveRecord::Base end --- activerecord/lib/active_record/reflection.rb | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2aec73bd5b..dd437bd066 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -499,6 +499,11 @@ module ActiveRecord delegate :foreign_key, :foreign_type, :association_foreign_key, :active_record_primary_key, :type, :to => :source_reflection + def initialize(macro, name, scope, options, active_record) + super + @source_reflection = nil + end + # Returns the source of the through reflection. It checks both a singularized # and pluralized form for :belongs_to or :has_many. # @@ -517,7 +522,28 @@ module ActiveRecord # # => # def source_reflection - @source_reflection ||= source_reflection_names.collect { |name| through_reflection.klass.reflect_on_association(name) }.compact.first + return @source_reflection if @source_reflection + + reflections = source_reflection_names.collect { |name| + through_reflection.klass.reflect_on_association(name) + }.compact + + if reflections.length > 1 + example_options = options.dup + example_options[:source] = source_reflection_names.first + ActiveSupport::Deprecation.warn <<-eowarn +Ambiguous source reflection for through association. Please specify a :source +directive on your declaration like: + + class #{active_record.name} < ActiveRecord::Base + #{macro} :#{name}, #{example_options} + end + + eowarn + @source_reflection = reflections.first + else + @source_reflection = reflections.first + end end # Returns the AssociationReflection object specified in the :through option -- cgit v1.2.3 From db1b92f4a35d220f3417ba90b58acf11d5595283 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 09:55:21 -0700 Subject: clean up ivar assignment --- activerecord/lib/active_record/reflection.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index dd437bd066..8b7f15882a 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -540,10 +540,9 @@ directive on your declaration like: end eowarn - @source_reflection = reflections.first - else - @source_reflection = reflections.first end + + @source_reflection = reflections.first end # Returns the AssociationReflection object specified in the :through option -- cgit v1.2.3 From 16b70fddd4dc7e7fb7be108add88bae6e3c2509b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 10:09:50 -0700 Subject: push ambiguous reflection warning down to reflection name calculation --- activerecord/lib/active_record/reflection.rb | 51 +++++++++++++++------------- 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 8b7f15882a..98c469f2f3 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -501,7 +501,7 @@ module ActiveRecord def initialize(macro, name, scope, options, active_record) super - @source_reflection = nil + @source_reflection_name = options[:source] end # Returns the source of the through reflection. It checks both a singularized @@ -522,27 +522,7 @@ module ActiveRecord # # => # def source_reflection - return @source_reflection if @source_reflection - - reflections = source_reflection_names.collect { |name| - through_reflection.klass.reflect_on_association(name) - }.compact - - if reflections.length > 1 - example_options = options.dup - example_options[:source] = source_reflection_names.first - ActiveSupport::Deprecation.warn <<-eowarn -Ambiguous source reflection for through association. Please specify a :source -directive on your declaration like: - - class #{active_record.name} < ActiveRecord::Base - #{macro} :#{name}, #{example_options} - end - - eowarn - end - - @source_reflection = reflections.first + through_reflection.klass.reflect_on_association(source_reflection_name) end # Returns the AssociationReflection object specified in the :through option @@ -660,7 +640,32 @@ directive on your declaration like: # # => [:tag, :tags] # def source_reflection_names - @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }.uniq + (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym }.uniq + end + + def source_reflection_name # :nodoc: + return @source_reflection_name if @source_reflection_name + + names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq + names = names.find_all { |n| + through_reflection.klass.reflect_on_association(n) + } + + if names.length > 1 + example_options = options.dup + example_options[:source] = source_reflection_names.first + ActiveSupport::Deprecation.warn <<-eowarn +Ambiguous source reflection for through association. Please specify a :source +directive on your declaration like: + + class #{active_record.name} < ActiveRecord::Base + #{macro} :#{name}, #{example_options} + end + + eowarn + end + + @source_reflection_name = names.first end def source_options -- cgit v1.2.3 From 5d46c570de8c61a934fded247c787e1542504055 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 10:23:15 -0700 Subject: active_record should always be set. Do or do not, there is no try --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 98c469f2f3..6eecc8252b 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -446,7 +446,7 @@ module ActiveRecord # from calling +klass+, +reflection+ will already be set to false. def valid_inverse_reflection?(reflection) reflection && - klass.name == reflection.active_record.try(:name) && + klass.name == reflection.active_record.name && klass.primary_key == reflection.active_record_primary_key && can_find_inverse_of_automatically?(reflection) end -- cgit v1.2.3 From f379185a893503e26d160a4f326e610f05e3d6cc Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 10:49:29 -0700 Subject: reduce automatic_inverse_of caching logic --- activerecord/lib/active_record/reflection.rb | 46 ++++++++++------------------ 1 file changed, 16 insertions(+), 30 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 6eecc8252b..76eeae3160 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -296,15 +296,13 @@ module ActiveRecord alias :source_macro :macro def has_inverse? - @options[:inverse_of] || find_inverse_of_automatically + inverse_name end def inverse_of - @inverse_of ||= if options[:inverse_of] - klass.reflect_on_association(options[:inverse_of]) - else - find_inverse_of_automatically - end + return unless inverse_name + + @inverse_of ||= klass.reflect_on_association inverse_name end # Clears the cached value of +@inverse_of+ on this object. This will @@ -393,26 +391,21 @@ module ActiveRecord INVALID_AUTOMATIC_INVERSE_OPTIONS = [:conditions, :through, :polymorphic, :foreign_key] private - # Attempts to find the inverse association automatically. - # If it cannot find a suitable inverse association, it returns + # Attempts to find the inverse association name automatically. + # If it cannot find a suitable inverse association name, it returns # nil. - def find_inverse_of_automatically - if @automatic_inverse_of == false - nil - elsif @automatic_inverse_of.nil? - set_automatic_inverse_of - else - klass.reflect_on_association(@automatic_inverse_of) + def inverse_name + options.fetch(:inverse_of) do + if @automatic_inverse_of == false + nil + else + @automatic_inverse_of = automatic_inverse_of + end end end - # Sets the +@automatic_inverse_of+ instance variable, and returns - # either nil or the inverse association that it finds. - # - # This method caches the inverse association that is found so that - # future calls to +find_inverse_of_automatically+ have much less - # overhead. - def set_automatic_inverse_of + # returns either nil or the inverse association name that it finds. + def automatic_inverse_of if can_find_inverse_of_automatically?(self) inverse_name = active_record.name.downcase.to_sym @@ -425,15 +418,8 @@ module ActiveRecord end if valid_inverse_reflection?(reflection) - @automatic_inverse_of = inverse_name - reflection - else - @automatic_inverse_of = false - nil + inverse_name end - else - @automatic_inverse_of = false - nil end end -- cgit v1.2.3 From 634fd040dbabf3e268a6a4bf4a9136468222022f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 10:59:11 -0700 Subject: let the object stay in charge of internal cache invalidation --- activerecord/lib/active_record/nested_attributes.rb | 7 +------ activerecord/lib/active_record/reflection.rb | 12 +++++------- 2 files changed, 6 insertions(+), 13 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 0e8822d63f..e53e8553ad 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -306,14 +306,9 @@ module ActiveRecord attr_names.each do |association_name| if reflection = reflect_on_association(association_name) - reflection.options[:autosave] = true + reflection.autosave = true add_autosave_association_callbacks(reflection) - # Clear cached values of any inverse associations found in the - # reflection and prevent the reflection from finding inverses - # automatically in the future. - reflection.remove_automatic_inverse_of! - nested_attributes_options = self.nested_attributes_options.dup nested_attributes_options[association_name.to_sym] = options self.nested_attributes_options = nested_attributes_options diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 76eeae3160..72a05f12aa 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -123,6 +123,11 @@ module ActiveRecord name.to_s.pluralize : name.to_s end + def autosave=(autosave) + @automatic_inverse_of = false + @options[:autosave] = autosave + end + # Returns the class for the macro. # # composed_of :balance, class_name: 'Money' returns the Money class @@ -312,13 +317,6 @@ module ActiveRecord @inverse_of = nil end - # Removes the cached inverse association that was found automatically - # and prevents this object from finding the inverse association - # automatically in the future. - def remove_automatic_inverse_of! - @automatic_inverse_of = false - end - def polymorphic_inverse_of(associated_class) if has_inverse? if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of]) -- cgit v1.2.3 From 27be568639ec14c9696f79d554301fa69cb8edae Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 11:00:02 -0700 Subject: fix caching of automatic inverse of. :bomb: --- activerecord/lib/active_record/reflection.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 72a05f12aa..496237a2f2 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -397,7 +397,7 @@ module ActiveRecord if @automatic_inverse_of == false nil else - @automatic_inverse_of = automatic_inverse_of + @automatic_inverse_of ||= automatic_inverse_of end end end @@ -419,6 +419,8 @@ module ActiveRecord inverse_name end end + + false end # Checks if the inverse reflection that is returned from the -- cgit v1.2.3 From 930d0e129c730292ab987eaaf1cd2ba8094e580b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 11:01:27 -0700 Subject: oops. step away from the keyboard aaron. :cry: --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 496237a2f2..cfae2ffdb5 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -416,7 +416,7 @@ module ActiveRecord end if valid_inverse_reflection?(reflection) - inverse_name + return inverse_name end end -- cgit v1.2.3 From 7663149f72b5dba15a8e850e4072a016341e541c Mon Sep 17 00:00:00 2001 From: Vijay Dev Date: Fri, 14 Jun 2013 00:58:11 +0530 Subject: copy edits [ci skip] --- .../lib/active_record/relation/finder_methods.rb | 34 ++++++++++------------ 1 file changed, 15 insertions(+), 19 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index f373714007..f1c4b5392f 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -11,11 +11,11 @@ module ActiveRecord # Person.find([1]) # returns an array for the object with ID = 1 # Person.where("administrator = 1").order("created_on DESC").find(1) # - # NOTE: An RecordNotFound will be raised if one or more ids are not returned. + # ActiveRecord::RecordNotFound will be raised if one or more ids are not found. # - # NOTE: that returned records may not be in the same order as the ids you - # provide since database rows are unordered. Give an explicit order - # to ensure the results are sorted. + # NOTE: The returned records may not be in the same order as the ids you + # provide since database rows are unordered. You'd need to provide an explicit order + # option if you want the results are sorted. # # ==== Find with lock # @@ -34,34 +34,30 @@ module ActiveRecord # ==== Variations of +find+ # # Person.where(name: 'Spartacus', rating: 4) - # # returns a chainable list (which can be empty) + # # returns a chainable list (which can be empty). # # Person.find_by(name: 'Spartacus', rating: 4) - # # returns the first item or nil + # # returns the first item or nil. # # Person.where(name: 'Spartacus', rating: 4).first_or_initialize - # # returns the first item or returns a new instance (requires you call .save to persist against the database) + # # returns the first item or returns a new instance (requires you call .save to persist against the database). # # Person.where(name: 'Spartacus', rating: 4).first_or_create - # # returns the first item or creates it and returns it, available since rails 3.2.1 + # # returns the first item or creates it and returns it, available since Rails 3.2.1. # - # # ==== Alternatives for +find+ # # Person.where(name: 'Spartacus', rating: 4).exists?(conditions = :none) - # # returns true or false + # # returns a boolean indicating if any record with the given conditions exist. # # Person.where(name: 'Spartacus', rating: 4).select("field1, field2, field3") - # # returns a chainable list of instances with only the mentioned fields + # # returns a chainable list of instances with only the mentioned fields. # # Person.where(name: 'Spartacus', rating: 4).ids - # # returns an Array of ids, available since rails 3.2.1 + # # returns an Array of ids, available since Rails 3.2.1. # # Person.where(name: 'Spartacus', rating: 4).pluck(:field1, :field2) - # # returns an Array of the required fields, available since rails 3.1 - # - # Person.arel_table - # # returns an instance of Arel::Table, which allows a comprehensive variety of filters + # # returns an Array of the required fields, available since Rails 3.1. def find(*args) if block_given? to_a.find { |*block_args| yield(*block_args) } @@ -118,9 +114,9 @@ module ActiveRecord # # Person.first # SELECT "people".* FROM "people" LIMIT 1 # - # NOTE: Rails 3 may not +order+ this query by be the primary key. - # The order will depend on the database implementation. - # In order to ensure that behavior use User.order(:id).first instead. + # NOTE: Rails 3 may not order this query by the primary key and the order + # will depend on the database implementation. In order to ensure that behavior, + # use User.order(:id).first instead. # # ==== Rails 4 # -- cgit v1.2.3 From 8f37ba81abbd13b935ce0f26574ff2b720b552f1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 15:36:25 -0700 Subject: This test does not test anything that happens in the real world. If you recreate the models without mucking with internal caches of the relation objects, then the test fails. For example: class Man < ActiveRecord::Base has_many :interests end class Interest < ActiveRecord::Base belongs_to :man end Then do this test: def test_validate_presence_of_parent_fails_without_inverse_of repair_validations(Interest) do Interest.validates_presence_of(:man) assert_no_difference ['Man.count', 'Interest.count'] do man = Man.create(:name => 'John', :interests_attributes => [{:topic=>'Cars'}, {:topic=>'Sports'}]) assert_not_predicate man.errors[:"interests.man"], :empty? end end end The test will fail. This is a bad test, so I am removing it. --- activerecord/lib/active_record/reflection.rb | 7 ------- 1 file changed, 7 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index cfae2ffdb5..1335af9e39 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -310,13 +310,6 @@ module ActiveRecord @inverse_of ||= klass.reflect_on_association inverse_name end - # Clears the cached value of +@inverse_of+ on this object. This will - # not remove the :inverse_of option however, so future calls on the - # +inverse_of+ will have to recompute the inverse. - def clear_inverse_of_cache! - @inverse_of = nil - end - def polymorphic_inverse_of(associated_class) if has_inverse? if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of]) -- cgit v1.2.3 From cbff6ee1802409cdbe44a730a13c22cce485be46 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 15:46:13 -0700 Subject: these methods are never called, so remove them --- activerecord/lib/active_record/reflection.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 1335af9e39..bfb321f930 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -250,14 +250,6 @@ module ActiveRecord end end - def columns(tbl_name) - @columns ||= klass.connection.columns(tbl_name) - end - - def reset_column_information - @columns = nil - end - def check_validity! check_validity_of_inverse! -- cgit v1.2.3 From bf966ad8eef6e42c0d23b48cd1ccc04809d7f68f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 15:57:48 -0700 Subject: only cache the primary key column in one place --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index bfb321f930..246f0761b3 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -226,7 +226,7 @@ module ActiveRecord end def primary_key_column - @primary_key_column ||= klass.columns.find { |c| c.name == klass.primary_key } + klass.columns_hash[klass.primary_key] end def association_foreign_key -- cgit v1.2.3 From f4767bc8448d3d606149618cb279234dfcf803e7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 16:03:40 -0700 Subject: calculate types on construction --- activerecord/lib/active_record/reflection.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 246f0761b3..82d728e2e9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -189,10 +189,14 @@ module ActiveRecord @klass ||= active_record.send(:compute_type, class_name) end + attr_reader :type, :foreign_type + def initialize(*args) super @collection = [:has_many, :has_and_belongs_to_many].include?(macro) @automatic_inverse_of = nil + @type = options[:as] && "#{options[:as]}_type" + @foreign_type = options[:foreign_type] || "#{name}_type" end # Returns a new, unsaved instance of the associated class. +attributes+ will @@ -217,14 +221,6 @@ module ActiveRecord @foreign_key ||= options[:foreign_key] || derive_foreign_key end - def foreign_type - @foreign_type ||= options[:foreign_type] || "#{name}_type" - end - - def type - @type ||= options[:as] && "#{options[:as]}_type" - end - def primary_key_column klass.columns_hash[klass.primary_key] end -- cgit v1.2.3 From 96925570a14dc909daebeae8241fdf2bd2ab786f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 16:06:41 -0700 Subject: table name is cached on the class, so stop caching twice --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 82d728e2e9..2eefda1ca9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -206,7 +206,7 @@ module ActiveRecord end def table_name - @table_name ||= klass.table_name + klass.table_name end def quoted_table_name -- cgit v1.2.3 From 383842d079e74de49dbe81ea4c2db61f79c665d9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Jun 2013 16:07:11 -0700 Subject: quoted table name is also cached --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2eefda1ca9..8a9488656b 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -210,7 +210,7 @@ module ActiveRecord end def quoted_table_name - @quoted_table_name ||= klass.quoted_table_name + klass.quoted_table_name end def join_table -- cgit v1.2.3 From 6553df05bd4514fe3278c34ec7111aa3f5cab8bd Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Fri, 14 Jun 2013 17:25:55 +0200 Subject: Use DatabaseTasks.env instead of Rails.env in databases.rake --- activerecord/lib/active_record/railties/databases.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index f69e9b2217..72a302e82d 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -324,7 +324,7 @@ db_namespace = namespace :db do ActiveRecord::Schema.verbose = false db_namespace["schema:load"].invoke ensure - ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[Rails.env]) + ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[DatabaseTasks.env]) end end -- cgit v1.2.3 From 8867e16a6041e2139696bcb9cfab4642ce55fbdd Mon Sep 17 00:00:00 2001 From: Steven Yang Date: Fri, 14 Jun 2013 14:39:03 +0800 Subject: correct documentation about active_record behavior --- activerecord/lib/active_record/timestamp.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb index ae99cff35e..9253150c4f 100644 --- a/activerecord/lib/active_record/timestamp.rb +++ b/activerecord/lib/active_record/timestamp.rb @@ -10,9 +10,9 @@ module ActiveRecord # # config.active_record.record_timestamps = false # - # Timestamps are in the local timezone by default but you can use UTC by setting: + # Timestamps are in UTC by default but you can use the local timezone by setting: # - # config.active_record.default_timezone = :utc + # config.active_record.default_timezone = :local # # == Time Zone aware attributes # -- cgit v1.2.3 From 067e1505d4e054df566e065f4faf11ee4b430a3d Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Fri, 14 Jun 2013 18:24:16 +0200 Subject: Properly namespace DatabaseTasks --- activerecord/lib/active_record/railties/databases.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 72a302e82d..0b74553bf8 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -324,7 +324,7 @@ db_namespace = namespace :db do ActiveRecord::Schema.verbose = false db_namespace["schema:load"].invoke ensure - ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[DatabaseTasks.env]) + ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[ActiveRecord::Tasks::DatabaseTasks.env]) end end -- cgit v1.2.3 From 36bc4f5a02f34c68d0e640ab84eb086a54cbd6ab Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Wed, 12 Jun 2013 14:15:15 +0200 Subject: regression test + mysql2 adapter raises correct error if conn is closed. --- .../lib/active_record/connection_adapters/mysql2_adapter.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 530a27d099..edeb338310 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -213,9 +213,11 @@ module ActiveRecord # Executes the SQL statement in the context of this connection. def execute(sql, name = nil) - # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been - # made since we established the connection - @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + if @connection + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been + # made since we established the connection + @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + end super end -- cgit v1.2.3 From 6d10d64cbafe70e343cef0f94e015908b9348ac5 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 10 Jun 2013 13:52:22 +0200 Subject: fixture setup does not rely on `AR::Base.configurations`. As you can also configure your database connection using `ENV["DATABASE_URL"]`, the fixture setup can't reply on the `.configurations` Hash. As the fixtures are only loaded when ActiveRecord is actually used (`rails/test_help.rb`) it should be safe to drop the check for an existing configuration. --- activerecord/lib/active_record/fixtures.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 33e19313a0..70eda332b3 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -841,8 +841,6 @@ module ActiveRecord end def setup_fixtures - return if ActiveRecord::Base.configurations.blank? - if pre_loaded_fixtures && !use_transactional_fixtures raise RuntimeError, 'pre_loaded_fixtures requires use_transactional_fixtures' end @@ -875,8 +873,6 @@ module ActiveRecord end def teardown_fixtures - return if ActiveRecord::Base.configurations.blank? - # Rollback changes if a transaction is active. if run_in_transaction? @fixture_connections.each do |connection| -- cgit v1.2.3