aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md12
-rw-r--r--activerecord/lib/active_record/associations.rb2
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb9
-rw-r--r--activerecord/lib/active_record/attribute_methods/read.rb32
-rw-r--r--activerecord/lib/active_record/attribute_methods/write.rb20
-rw-r--r--activerecord/lib/active_record/core.rb7
-rw-r--r--activerecord/lib/active_record/relation/calculations.rb23
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb6
-rw-r--r--activerecord/test/cases/calculations_test.rb22
-rw-r--r--activerecord/test/cases/migration/create_join_table_test.rb6
-rw-r--r--activerecord/test/cases/migration_test.rb7
11 files changed, 69 insertions, 77 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 2b58276461..1a34ed441f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -5,6 +5,11 @@
*Yves Senn*
+* Deprecate calling `Relation#sum` with a block. To perform a calculation over
+ the array result of the relation, use `to_a.sum(&block)`.
+
+ *Carlos Antonio da Silva*
+
* Fix postgresql adapter to handle BC timestamps correctly
HistoryEvent.create!(:name => "something", :occured_at => Date.new(0) - 5.years)
@@ -741,13 +746,6 @@
*Marc-André Lafortune*
-* Allow blocks for `count` with `ActiveRecord::Relation`, to work similar as
- `Array#count`:
-
- Person.where("age > 26").count { |person| person.gender == 'female' }
-
- *Chris Finne & Carlos Antonio da Silva*
-
* Added support to `CollectionAssociation#delete` for passing `fixnum`
or `string` values as record ids. This finds the records responding
to the `id` and executes delete on them.
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 651b17920c..d8b6d7a86b 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -234,7 +234,7 @@ module ActiveRecord
# others.size | X | X | X
# others.length | X | X | X
# others.count | X | X | X
- # others.sum(args*,&block) | X | X | X
+ # others.sum(*args) | X | X | X
# others.empty? | X | X | X
# others.clear | X | X | X
# others.delete(other,other,...) | X | X | X
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 862ff201de..1548e68cea 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -161,15 +161,6 @@ module ActiveRecord
end
end
- # Calculate sum using SQL, not Enumerable.
- def sum(*args)
- if block_given?
- scope.sum(*args) { |*block_args| yield(*block_args) }
- else
- scope.sum(*args)
- end
- end
-
# Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the
# association, it will be used for the query. Otherwise, construct options and pass them with
# scope to the target class's +count+.
diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb
index 90701938e5..3c03cce838 100644
--- a/activerecord/lib/active_record/attribute_methods/read.rb
+++ b/activerecord/lib/active_record/attribute_methods/read.rb
@@ -32,21 +32,29 @@ module ActiveRecord
protected
- # We want to generate the methods via module_eval rather than define_method,
- # because define_method is slower on dispatch and uses more memory (because it
- # creates a closure).
+ # We want to generate the methods via module_eval rather than
+ # define_method, because define_method is slower on dispatch and
+ # uses more memory (because it creates a closure).
#
- # But sometimes the database might return columns with characters that are not
- # allowed in normal method names (like 'my_column(omg)'. So to work around this
- # we first define with the __temp__ identifier, and then use alias method to
- # rename it to what we want.
- def define_method_attribute(attr_name)
+ # But sometimes the database might return columns with
+ # characters that are not allowed in normal method names (like
+ # 'my_column(omg)'. So to work around this we first define with
+ # the __temp__ identifier, and then use alias method to rename
+ # it to what we want.
+ #
+ # We are also defining a constant to hold the frozen string of
+ # the attribute name. Using a constant means that we do not have
+ # to allocate an object on each call to the attribute method.
+ # Making it frozen means that it doesn't get duped when used to
+ # key the @attributes_cache in read_attribute.
+ def define_method_attribute(name)
+ safe_name = name.unpack('h*').first
generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
- def __temp__
- read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) }
+ def __temp__#{safe_name}
+ read_attribute(AttrNames::ATTR_#{safe_name}) { |n| missing_attribute(n, caller) }
end
- alias_method '#{attr_name}', :__temp__
- undef_method :__temp__
+ alias_method #{name.inspect}, :__temp__#{safe_name}
+ undef_method :__temp__#{safe_name}
STR
end
diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb
index fa9097db1f..cd33494cc3 100644
--- a/activerecord/lib/active_record/attribute_methods/write.rb
+++ b/activerecord/lib/active_record/attribute_methods/write.rb
@@ -9,15 +9,19 @@ module ActiveRecord
module ClassMethods
protected
- def define_method_attribute=(attr_name)
- if attr_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP
- generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__)
- else
- generated_attribute_methods.send(:define_method, "#{attr_name}=") do |new_value|
- write_attribute(attr_name, new_value)
- end
+
+ # See define_method_attribute in read.rb for an explanation of
+ # this code.
+ def define_method_attribute=(name)
+ safe_name = name.unpack('h*').first
+ generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1
+ def __temp__#{safe_name}=(value)
+ write_attribute(AttrNames::ATTR_#{safe_name}, value)
end
- end
+ alias_method #{(name + '=').inspect}, :__temp__#{safe_name}=
+ undef_method :__temp__#{safe_name}=
+ STR
+ end
end
# Updates the attribute identified by <tt>attr_name</tt> with the
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 957027c1ee..94c6684700 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -83,7 +83,12 @@ module ActiveRecord
@attribute_methods_mutex = Mutex.new
# force attribute methods to be higher in inheritance hierarchy than other generated methods
- generated_attribute_methods
+ generated_attribute_methods.const_set(:AttrNames, Module.new {
+ def self.const_missing(name)
+ const_set(name, [name.to_s.sub(/ATTR_/, '')].pack('h*').freeze)
+ end
+ })
+
generated_feature_methods
end
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index 741f94f777..72b035a023 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -1,3 +1,5 @@
+require 'active_support/deprecation'
+
module ActiveRecord
module Calculations
# Count the records.
@@ -13,16 +15,9 @@ module ActiveRecord
#
# Person.count(:age, distinct: true)
# # => counts the number of different age values
- #
- # Person.where("age > 26").count { |person| person.gender == 'female' }
- # # => queries people where "age > 26" then count the loaded results filtering by gender
def count(column_name = nil, options = {})
- if block_given?
- self.to_a.count { |item| yield item }
- else
- column_name, options = nil, column_name if column_name.is_a?(Hash)
- calculate(:count, column_name, options)
- end
+ column_name, options = nil, column_name if column_name.is_a?(Hash)
+ calculate(:count, column_name, options)
end
# Calculates the average value on a given column. Returns +nil+ if there's
@@ -56,13 +51,13 @@ module ActiveRecord
# +calculate+ for examples with options.
#
# Person.sum('age') # => 4562
- # # => returns the total sum of all people's age
- #
- # Person.where('age > 100').sum { |person| person.age - 100 }
- # # queries people where "age > 100" then perform a sum calculation with the block returns
def sum(*args)
if block_given?
- self.to_a.sum(*args) { |item| yield item }
+ ActiveSupport::Deprecation.warn(
+ "Calling #sum with a block is deprecated and will be removed in Rails 4.1. " \
+ "If you want to perform sum calculation over the array of elements, use `to_a.sum(&block)`."
+ )
+ self.to_a.sum(*args) {|*block_args| yield(*block_args)}
else
calculate(:sum, *args)
end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 989d08b3e9..2ded97582d 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1666,6 +1666,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_deprecated { klass.has_many :foo, :counter_sql => 'lol' }
end
+ test "sum calculation with block for array compatibility is deprecated" do
+ assert_deprecated do
+ posts(:welcome).comments.sum { |c| c.id }
+ end
+ end
+
test "has many associations on new records use null relations" do
post = Post.new
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 65d28ea028..5cb7eabf0e 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -383,30 +383,16 @@ class CalculationsTest < ActiveRecord::TestCase
Company.where(:type => "Firm").from('companies').count(:type)
end
- def test_count_with_block_acts_as_array
- accounts = Account.where('id > 0')
- assert_equal Account.count, accounts.count { true }
- assert_equal 0, accounts.count { false }
- assert_equal Account.where('credit_limit > 50').size, accounts.count { |account| account.credit_limit > 50 }
- assert_equal Account.count, Account.count { true }
- assert_equal 0, Account.count { false }
- end
-
- def test_sum_with_block_acts_as_array
- accounts = Account.where('id > 0')
- assert_equal Account.sum(:credit_limit), accounts.sum { |account| account.credit_limit }
- assert_equal Account.sum(:credit_limit) + Account.count, accounts.sum{ |account| account.credit_limit + 1 }
- assert_equal 0, accounts.sum { |account| 0 }
- end
-
def test_sum_with_from_option
assert_equal Account.sum(:credit_limit), Account.from('accounts').sum(:credit_limit)
assert_equal Account.where("credit_limit > 50").sum(:credit_limit),
Account.where("credit_limit > 50").from('accounts').sum(:credit_limit)
end
- def test_sum_array_compatibility
- assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit)
+ def test_sum_array_compatibility_deprecation
+ assert_deprecated do
+ assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit)
+ end
end
def test_average_with_from_option
diff --git a/activerecord/test/cases/migration/create_join_table_test.rb b/activerecord/test/cases/migration/create_join_table_test.rb
index cd1b0e8b47..f262bbaad7 100644
--- a/activerecord/test/cases/migration/create_join_table_test.rb
+++ b/activerecord/test/cases/migration/create_join_table_test.rb
@@ -35,6 +35,12 @@ module ActiveRecord
assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort
end
+ def test_create_join_table_with_symbol_and_string
+ connection.create_join_table :artists, 'musics'
+
+ assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort
+ end
+
def test_create_join_table_with_the_proper_order
connection.create_join_table :videos, :musics
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index b77da39e29..c155f29973 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -755,11 +755,4 @@ class CopyMigrationsTest < ActiveRecord::TestCase
ensure
clear
end
-
- def test_create_join_table_with_symbol_and_string
- connection.create_join_table :artists, 'musics'
-
- assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort
- end
-
end