Skip to content

Commit

Permalink
Remove ActiveRecord::Base.to_a
Browse files Browse the repository at this point in the history
On reflection, it seems like a bit of a weird method to have on
ActiveRecord::Base, and it shouldn't be needed most of the time anyway.
  • Loading branch information
jonleighton committed Aug 3, 2012
1 parent 0a6833b commit 55b2488
Show file tree
Hide file tree
Showing 18 changed files with 55 additions and 64 deletions.
2 changes: 1 addition & 1 deletion activemodel/lib/active_model/observing.rb
Expand Up @@ -214,7 +214,7 @@ def observe(*models)
# end
# end
def observed_classes
[observed_class].compact.flatten
Array(observed_class)
end

# The class observed by default is inferred from the observer's class name:
Expand Down
3 changes: 1 addition & 2 deletions activerecord/CHANGELOG.md
Expand Up @@ -54,8 +54,7 @@
*Jon Leighton*

* `Model.all` now returns an `ActiveRecord::Relation`, rather than an
array of records. Use `Model.to_a` or `Relation#to_a` if you really
want an array.
array of records. Use ``Relation#to_a` if you really want an array.

In some specific cases, this may cause breakage when upgrading.
However in most cases the `ActiveRecord::Relation` will just act as a
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/querying.rb
@@ -1,7 +1,7 @@

module ActiveRecord
module Querying
delegate :find, :take, :take!, :first, :first!, :last, :last!, :to_a, :exists?, :any?, :many?, :to => :all
delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, :to => :all
delegate :first_or_create, :first_or_create!, :first_or_initialize, :to => :all
delegate :find_by, :find_by!, :to => :all
delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, :to => :all
Expand Down
Expand Up @@ -359,7 +359,7 @@ def test_deleting
def test_deleting_array
david = Developer.find(1)
david.projects.reload
david.projects.delete(Project.to_a)
david.projects.delete(Project.all.to_a)
assert_equal 0, david.projects.size
assert_equal 0, david.projects(true).size
end
Expand Down Expand Up @@ -426,7 +426,7 @@ def test_destroying
def test_destroying_many
david = Developer.find(1)
david.projects.reload
projects = Project.to_a
projects = Project.all.to_a

assert_no_difference "Project.count" do
david.projects.destroy(*projects)
Expand Down
Expand Up @@ -1130,7 +1130,7 @@ def test_included_in_collection
end

def test_adding_array_and_collection
assert_nothing_raised { Firm.first.clients + Firm.to_a.last.clients }
assert_nothing_raised { Firm.first.clients + Firm.all.last.clients }
end

def test_replace_with_less
Expand Down
14 changes: 3 additions & 11 deletions activerecord/test/cases/base_test.rb
Expand Up @@ -1635,9 +1635,9 @@ def test_last
end

def test_all
developers = Developer.to_a
assert_kind_of Array, developers
assert_equal Developer.to_a, developers
developers = Developer.all
assert_kind_of ActiveRecord::Relation, developers
assert_equal Developer.all, developers
end

def test_all_with_conditions
Expand Down Expand Up @@ -2032,12 +2032,4 @@ def test_default_values_are_deeply_dupped
klass = Class.new(ActiveRecord::Base)
assert_equal ['foo'], klass.all.merge!(select: 'foo').select_values
end

test "Model.to_a returns an array" do
assert_equal Post.all.to_a, Post.to_a
end

test "Model.all returns a relation" do
assert Post.all.is_a?(ActiveRecord::Relation)
end
end
2 changes: 1 addition & 1 deletion activerecord/test/cases/batches_test.rb
Expand Up @@ -116,7 +116,7 @@ def test_find_in_batches_should_ignore_the_order_default_scope
end

def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then_order
special_posts_ids = SpecialPostWithDefaultScope.to_a.map(&:id).sort
special_posts_ids = SpecialPostWithDefaultScope.all.map(&:id).sort
posts = []
SpecialPostWithDefaultScope.find_in_batches do |batch|
posts.concat(batch)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/calculations_test.rb
Expand Up @@ -542,7 +542,7 @@ def test_pluck_with_selection_clause
end

def test_plucks_with_ids
assert_equal Company.to_a.map(&:id).sort, Company.ids.sort
assert_equal Company.all.map(&:id).sort, Company.ids.sort
end

def test_pluck_not_auto_table_name_prefix_if_column_included
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/explain_test.rb
Expand Up @@ -112,7 +112,7 @@ def test_silence_auto_explain
base.expects(:collecting_sqls_for_explain).never
base.logger.expects(:warn).never
base.silence_auto_explain do
with_threshold(0) { Car.to_a }
with_threshold(0) { Car.all }
end
end

Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/finder_test.rb
Expand Up @@ -120,7 +120,7 @@ def test_find_by_ids_with_limit_and_offset
# Also test an edge case: If you have 11 results, and you set a
# limit of 3 and offset of 9, then you should find that there
# will be only 2 results, regardless of the limit.
devs = Developer.to_a
devs = Developer.all
last_devs = Developer.all.merge!(:limit => 3, :offset => 9).find devs.map(&:id)
assert_equal 2, last_devs.size
end
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/inheritance_test.rb
Expand Up @@ -27,7 +27,7 @@ def read_attribute(name)
end
})
company.save!
company = Company.to_a.find { |x| x.id == company.id }
company = Company.all.to_a.find { |x| x.id == company.id }
assert_equal ' ', company.type
end

Expand Down Expand Up @@ -179,7 +179,7 @@ def test_alt_finding_incorrect_type_data

def test_update_all_within_inheritance
Client.update_all "name = 'I am a client'"
assert_equal "I am a client", Client.to_a.first.name
assert_equal "I am a client", Client.first.name
# Order by added as otherwise Oracle tests were failing because of different order of results
assert_equal "37signals", Firm.all.merge!(:order => "id").to_a.first.name
end
Expand Down
12 changes: 6 additions & 6 deletions activerecord/test/cases/log_subscriber_test.rb
Expand Up @@ -54,7 +54,7 @@ def debug message
end

def test_basic_query_logging
Developer.to_a
Developer.all.load
wait
assert_equal 1, @logger.logged(:debug).size
assert_match(/Developer Load/, @logger.logged(:debug).last)
Expand All @@ -71,8 +71,8 @@ def test_exists_query_logging

def test_cached_queries
ActiveRecord::Base.cache do
Developer.to_a
Developer.to_a
Developer.all.load
Developer.all.load
end
wait
assert_equal 2, @logger.logged(:debug).size
Expand All @@ -82,16 +82,16 @@ def test_cached_queries

def test_basic_query_doesnt_log_when_level_is_not_debug
@logger.level = INFO
Developer.to_a
Developer.all.load
wait
assert_equal 0, @logger.logged(:debug).size
end

def test_cached_queries_doesnt_log_when_level_is_not_debug
@logger.level = INFO
ActiveRecord::Base.cache do
Developer.to_a
Developer.to_a
Developer.all.load
Developer.all.load
end
wait
assert_equal 0, @logger.logged(:debug).size
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/migration/rename_column_test.rb
Expand Up @@ -33,7 +33,7 @@ def test_rename_column_using_symbol_arguments
rename_column :test_models, :first_name, :nick_name
TestModel.reset_column_information
assert TestModel.column_names.include?("nick_name")
assert_equal ['foo'], TestModel.to_a.map(&:nick_name)
assert_equal ['foo'], TestModel.all.map(&:nick_name)
end

# FIXME: another integration test. We should decouple this from the
Expand All @@ -46,7 +46,7 @@ def test_rename_column
rename_column "test_models", "first_name", "nick_name"
TestModel.reset_column_information
assert TestModel.column_names.include?("nick_name")
assert_equal ['foo'], TestModel.to_a.map(&:nick_name)
assert_equal ['foo'], TestModel.all.map(&:nick_name)
end

def test_rename_column_preserves_default_value_not_null
Expand Down
26 changes: 13 additions & 13 deletions activerecord/test/cases/named_scope_test.rb
Expand Up @@ -10,12 +10,12 @@ class NamedScopeTest < ActiveRecord::TestCase
fixtures :posts, :authors, :topics, :comments, :author_addresses

def test_implements_enumerable
assert !Topic.to_a.empty?
assert !Topic.all.empty?

assert_equal Topic.to_a, Topic.base
assert_equal Topic.to_a, Topic.base.to_a
assert_equal Topic.first, Topic.base.first
assert_equal Topic.to_a, Topic.base.map { |i| i }
assert_equal Topic.all.to_a, Topic.base
assert_equal Topic.all.to_a, Topic.base.to_a
assert_equal Topic.first, Topic.base.first
assert_equal Topic.all.to_a, Topic.base.map { |i| i }
end

def test_found_items_are_cached
Expand All @@ -37,11 +37,11 @@ def test_reload_expires_cache_of_found_items
end

def test_delegates_finds_and_calculations_to_the_base_class
assert !Topic.to_a.empty?
assert !Topic.all.empty?

assert_equal Topic.to_a, Topic.base.to_a
assert_equal Topic.first, Topic.base.first
assert_equal Topic.count, Topic.base.count
assert_equal Topic.all.to_a, Topic.base.to_a
assert_equal Topic.first, Topic.base.first
assert_equal Topic.count, Topic.base.count
assert_equal Topic.average(:replies_count), Topic.base.average(:replies_count)
end

Expand Down Expand Up @@ -97,7 +97,7 @@ def test_procedural_scopes
end

def test_procedural_scopes_returning_nil
all_topics = Topic.to_a
all_topics = Topic.all

assert_equal all_topics, Topic.written_before(nil)
end
Expand Down Expand Up @@ -138,9 +138,9 @@ def test_scopes_honor_current_scopes_from_when_defined
end

def test_active_records_have_scope_named__all__
assert !Topic.to_a.empty?
assert !Topic.all.empty?

assert_equal Topic.to_a, Topic.base
assert_equal Topic.all.to_a, Topic.base
end

def test_active_records_have_scope_named__scoped__
Expand Down Expand Up @@ -444,6 +444,6 @@ def test_eager_default_scope_relations_are_deprecated
assert_deprecated do
klass.send(:default_scope, klass.where(:id => posts(:welcome).id))
end
assert_equal [posts(:welcome).title], klass.to_a.map(&:title)
assert_equal [posts(:welcome).title], klass.all.map(&:title)
end
end
2 changes: 1 addition & 1 deletion activerecord/test/cases/readonly_test.rb
Expand Up @@ -28,7 +28,7 @@ def test_cant_save_readonly_record


def test_find_with_readonly_option
Developer.to_a.each { |d| assert !d.readonly? }
Developer.all.each { |d| assert !d.readonly? }
Developer.readonly(false).each { |d| assert !d.readonly? }
Developer.readonly(true).each { |d| assert d.readonly? }
Developer.readonly.each { |d| assert d.readonly? }
Expand Down
32 changes: 16 additions & 16 deletions activerecord/test/cases/relation_scoping_test.rb
Expand Up @@ -70,7 +70,7 @@ def test_scoped_find_combines_and_sanitizes_conditions

def test_scoped_find_all
Developer.where("name = 'David'").scoping do
assert_equal [developers(:david)], Developer.to_a
assert_equal [developers(:david)], Developer.all
end
end

Expand Down Expand Up @@ -179,7 +179,7 @@ def test_merge_options
def test_merge_inner_scope_has_priority
Developer.limit(5).scoping do
Developer.limit(10).scoping do
assert_equal 10, Developer.to_a.size
assert_equal 10, Developer.all.size
end
end
end
Expand Down Expand Up @@ -313,32 +313,32 @@ class DefaultScopingTest < ActiveRecord::TestCase

def test_default_scope
expected = Developer.all.merge!(:order => 'salary DESC').to_a.collect { |dev| dev.salary }
received = DeveloperOrderedBySalary.to_a.collect { |dev| dev.salary }
received = DeveloperOrderedBySalary.all.collect { |dev| dev.salary }
assert_equal expected, received
end

def test_default_scope_as_class_method
assert_equal [developers(:david).becomes(ClassMethodDeveloperCalledDavid)], ClassMethodDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(ClassMethodDeveloperCalledDavid)], ClassMethodDeveloperCalledDavid.all
end

def test_default_scope_as_class_method_referencing_scope
assert_equal [developers(:david).becomes(ClassMethodReferencingScopeDeveloperCalledDavid)], ClassMethodReferencingScopeDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(ClassMethodReferencingScopeDeveloperCalledDavid)], ClassMethodReferencingScopeDeveloperCalledDavid.all
end

def test_default_scope_as_block_referencing_scope
assert_equal [developers(:david).becomes(LazyBlockReferencingScopeDeveloperCalledDavid)], LazyBlockReferencingScopeDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(LazyBlockReferencingScopeDeveloperCalledDavid)], LazyBlockReferencingScopeDeveloperCalledDavid.all
end

def test_default_scope_with_lambda
assert_equal [developers(:david).becomes(LazyLambdaDeveloperCalledDavid)], LazyLambdaDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(LazyLambdaDeveloperCalledDavid)], LazyLambdaDeveloperCalledDavid.all
end

def test_default_scope_with_block
assert_equal [developers(:david).becomes(LazyBlockDeveloperCalledDavid)], LazyBlockDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(LazyBlockDeveloperCalledDavid)], LazyBlockDeveloperCalledDavid.all
end

def test_default_scope_with_callable
assert_equal [developers(:david).becomes(CallableDeveloperCalledDavid)], CallableDeveloperCalledDavid.to_a
assert_equal [developers(:david).becomes(CallableDeveloperCalledDavid)], CallableDeveloperCalledDavid.all
end

def test_default_scope_is_unscoped_on_find
Expand All @@ -351,12 +351,12 @@ def test_default_scope_is_unscoped_on_create
end

def test_default_scope_with_conditions_string
assert_equal Developer.where(name: 'David').map(&:id).sort, DeveloperCalledDavid.to_a.map(&:id).sort
assert_equal Developer.where(name: 'David').map(&:id).sort, DeveloperCalledDavid.all.map(&:id).sort
assert_equal nil, DeveloperCalledDavid.create!.name
end

def test_default_scope_with_conditions_hash
assert_equal Developer.where(name: 'Jamis').map(&:id).sort, DeveloperCalledJamis.to_a.map(&:id).sort
assert_equal Developer.where(name: 'Jamis').map(&:id).sort, DeveloperCalledJamis.all.map(&:id).sort
assert_equal 'Jamis', DeveloperCalledJamis.create!.name
end

Expand Down Expand Up @@ -472,16 +472,16 @@ def test_unscoped_with_named_scope_should_not_have_default_scope
end

def test_default_scope_select_ignored_by_aggregations
assert_equal DeveloperWithSelect.to_a.count, DeveloperWithSelect.count
assert_equal DeveloperWithSelect.all.to_a.count, DeveloperWithSelect.count
end

def test_default_scope_select_ignored_by_grouped_aggregations
assert_equal Hash[Developer.to_a.group_by(&:salary).map { |s, d| [s, d.count] }],
assert_equal Hash[Developer.all.group_by(&:salary).map { |s, d| [s, d.count] }],
DeveloperWithSelect.group(:salary).count
end

def test_default_scope_order_ignored_by_aggregations
assert_equal DeveloperOrderedBySalary.to_a.count, DeveloperOrderedBySalary.count
assert_equal DeveloperOrderedBySalary.all.count, DeveloperOrderedBySalary.count
end

def test_default_scope_find_last
Expand All @@ -508,10 +508,10 @@ def test_default_scope_is_threadsafe

threads << Thread.new do
Thread.current[:long_default_scope] = true
assert_equal 1, ThreadsafeDeveloper.to_a.count
assert_equal 1, ThreadsafeDeveloper.all.to_a.count
end
threads << Thread.new do
assert_equal 1, ThreadsafeDeveloper.to_a.count
assert_equal 1, ThreadsafeDeveloper.all.to_a.count
end
threads.each(&:join)
end
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/relations_test.rb
Expand Up @@ -1032,7 +1032,7 @@ def test_except
assert_equal Post.where(:author_id => 1).to_a, author_posts.to_a

all_posts = relation.except(:where, :order, :limit)
assert_equal Post.to_a, all_posts.to_a
assert_equal Post.all, all_posts
end

def test_only
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/xml_serialization_test.rb
Expand Up @@ -286,7 +286,7 @@ def test_except_option
# getting appended to.

def test_modules
projects = MyApplication::Business::Project.to_a
projects = MyApplication::Business::Project.all
xml = projects.to_xml
root = projects.first.class.to_s.underscore.pluralize.tr('/','_').dasherize
assert_match "<#{root} type=\"array\">", xml
Expand Down

0 comments on commit 55b2488

Please sign in to comment.