From 6928950def1bea9d564778e734822d4f5b8bac61 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Wed, 24 Jan 2018 14:06:30 -0500 Subject: Avoid passing unnecessary arguments to relation Most of the time the table and predicate_builder passed to Relation.new are exactly the arel_table and predicate builder of the given klass. This uses klass.arel_table and klass.predicate_builder as the defaults, so we don't have to pass them in most cases. This does change the signaure of both Relation and AssocationRelation. Are we ok with that? --- activerecord/test/cases/batches_test.rb | 6 ++- .../test/cases/collection_cache_key_test.rb | 6 ++- activerecord/test/cases/relation/mutation_test.rb | 2 +- activerecord/test/cases/relation_test.rb | 50 +++++++++++----------- activerecord/test/cases/relations_test.rb | 6 ++- 5 files changed, 41 insertions(+), 29 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index be8aeed5ac..bbd0b1724a 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -592,7 +592,11 @@ class EachTest < ActiveRecord::TestCase table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias) predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) - posts = ActiveRecord::Relation.create(Post, table_alias, predicate_builder) + posts = ActiveRecord::Relation.create( + Post, + table: table_alias, + predicate_builder: predicate_builder + ) posts.find_each {} end end diff --git a/activerecord/test/cases/collection_cache_key_test.rb b/activerecord/test/cases/collection_cache_key_test.rb index cfe95b2360..a5d908344a 100644 --- a/activerecord/test/cases/collection_cache_key_test.rb +++ b/activerecord/test/cases/collection_cache_key_test.rb @@ -60,7 +60,11 @@ module ActiveRecord table_metadata = ActiveRecord::TableMetadata.new(Developer, table_alias) predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) - developers = ActiveRecord::Relation.create(Developer, table_alias, predicate_builder) + developers = ActiveRecord::Relation.create( + Developer, + table: table_alias, + predicate_builder: predicate_builder + ) developers = developers.where(salary: 100000).order(updated_at: :desc) last_developer_timestamp = developers.first.updated_at diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb index ad3700b73a..932a4eca2c 100644 --- a/activerecord/test/cases/relation/mutation_test.rb +++ b/activerecord/test/cases/relation/mutation_test.rb @@ -139,7 +139,7 @@ module ActiveRecord private def relation - @relation ||= Relation.new(FakeKlass, Post.arel_table, Post.predicate_builder) + @relation ||= Relation.new(FakeKlass) end end end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index dbf3389774..3d64bfb810 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -11,19 +11,19 @@ module ActiveRecord fixtures :posts, :comments, :authors, :author_addresses, :ratings def test_construction - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass, table: :b) assert_equal FakeKlass, relation.klass assert_equal :b, relation.table assert !relation.loaded, "relation is not loaded" end def test_responds_to_model_and_returns_klass - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert_equal FakeKlass, relation.model end def test_initialize_single_values - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end @@ -33,7 +33,7 @@ module ActiveRecord end def test_multi_value_initialize - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) Relation::MULTI_VALUE_METHODS.each do |method| values = relation.send("#{method}_values") assert_equal [], values, method.to_s @@ -42,29 +42,29 @@ module ActiveRecord end def test_extensions - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert_equal [], relation.extensions end def test_empty_where_values_hash - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert_equal({}, relation.where_values_hash) end def test_has_values - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) relation.where!(id: 10) assert_equal({ "id" => 10 }, relation.where_values_hash) end def test_values_wrong_table - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) relation.where! Comment.arel_table[:id].eq(10) assert_equal({}, relation.where_values_hash) end def test_tree_is_not_traversed - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) left = relation.table[:id].eq(10) right = relation.table[:id].eq(10) combine = left.or(right) @@ -73,18 +73,18 @@ module ActiveRecord end def test_scope_for_create - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert_equal({}, relation.scope_for_create) end def test_create_with_value - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) relation.create_with_value = { hello: "world" } assert_equal({ "hello" => "world" }, relation.scope_for_create) end def test_create_with_value_with_wheres - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) assert_equal({}, relation.scope_for_create) relation.where!(id: 10) @@ -95,7 +95,7 @@ module ActiveRecord end def test_empty_scope - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) assert relation.empty_scope? relation.merge!(relation) @@ -109,31 +109,31 @@ module ActiveRecord end def test_empty_eager_loading? - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert !relation.eager_loading? end def test_eager_load_values - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) relation.eager_load! :b assert relation.eager_loading? end def test_references_values - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) assert_equal [], relation.references_values relation = relation.references(:foo).references(:omg, :lol) assert_equal ["foo", "omg", "lol"], relation.references_values end def test_references_values_dont_duplicate - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) relation = relation.references(:foo).references(:foo) assert_equal ["foo"], relation.references_values end test "merging a hash into a relation" do - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) + relation = Relation.new(Post) relation = relation.merge where: { name: :lol }, readonly: true assert_equal({ "name" => :lol }, relation.where_clause.to_h) @@ -141,7 +141,7 @@ module ActiveRecord end test "merging an empty hash into a relation" do - assert_equal Relation::WhereClause.empty, Relation.new(FakeKlass, :b, nil).merge({}).where_clause + assert_equal Relation::WhereClause.empty, Relation.new(FakeKlass).merge({}).where_clause end test "merging a hash with unknown keys raises" do @@ -149,7 +149,7 @@ module ActiveRecord end test "merging nil or false raises" do - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) e = assert_raises(ArgumentError) do relation = relation.merge nil @@ -165,7 +165,7 @@ module ActiveRecord end test "#values returns a dup of the values" do - relation = Relation.new(Post, Post.arel_table, Post.predicate_builder).where!(name: :foo) + relation = Relation.new(Post).where!(name: :foo) values = relation.values values[:where] = nil @@ -173,7 +173,7 @@ module ActiveRecord end test "relations can be created with a values hash" do - relation = Relation.new(FakeKlass, :b, nil, select: [:foo]) + relation = Relation.new(FakeKlass, values: { select: [:foo] }) assert_equal [:foo], relation.select_values end @@ -185,13 +185,13 @@ module ActiveRecord end end - relation = Relation.new(klass, :b, nil) + relation = Relation.new(klass) relation.merge!(where: ["foo = ?", "bar"]) assert_equal Relation::WhereClause.new(["foo = bar"]), relation.where_clause end def test_merging_readonly_false - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) readonly_false_relation = relation.readonly(false) # test merging in both directions assert_equal false, relation.merge(readonly_false_relation).readonly_value @@ -235,7 +235,7 @@ module ActiveRecord def test_merge_raises_with_invalid_argument assert_raises ArgumentError do - relation = Relation.new(FakeKlass, :b, nil) + relation = Relation.new(FakeKlass) relation.merge(true) end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 7785f8c99b..19489f3c71 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1934,6 +1934,10 @@ class RelationTest < ActiveRecord::TestCase table_metadata = ActiveRecord::TableMetadata.new(Post, table_alias) predicate_builder = ActiveRecord::PredicateBuilder.new(table_metadata) - ActiveRecord::Relation.create(Post, table_alias, predicate_builder) + ActiveRecord::Relation.create( + Post, + table: table_alias, + predicate_builder: predicate_builder + ) end end -- cgit v1.2.3