mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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?
This commit is contained in:
parent
1280ad6d19
commit
6928950def
14 changed files with 66 additions and 38 deletions
|
@ -2,8 +2,8 @@
|
|||
|
||||
module ActiveRecord
|
||||
class AssociationRelation < Relation
|
||||
def initialize(klass, table, predicate_builder, association)
|
||||
super(klass, table, predicate_builder)
|
||||
def initialize(klass, association)
|
||||
super(klass)
|
||||
@association = association
|
||||
end
|
||||
|
||||
|
|
|
@ -124,7 +124,7 @@ module ActiveRecord
|
|||
# Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
|
||||
# through association's scope)
|
||||
def target_scope
|
||||
AssociationRelation.create(klass, klass.arel_table, klass.predicate_builder, self).merge!(klass.all)
|
||||
AssociationRelation.create(klass, self).merge!(klass.all)
|
||||
end
|
||||
|
||||
def extensions
|
||||
|
|
|
@ -32,7 +32,7 @@ module ActiveRecord
|
|||
class CollectionProxy < Relation
|
||||
def initialize(klass, association) #:nodoc:
|
||||
@association = association
|
||||
super klass, klass.arel_table, klass.predicate_builder
|
||||
super klass
|
||||
|
||||
extensions = association.extensions
|
||||
extend(*extensions) if extensions.any?
|
||||
|
|
|
@ -281,7 +281,7 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def relation
|
||||
relation = Relation.create(self, arel_table, predicate_builder)
|
||||
relation = Relation.create(self)
|
||||
|
||||
if finder_needs_type_condition? && !ignore_default_scope?
|
||||
relation.where!(type_condition)
|
||||
|
|
|
@ -291,7 +291,11 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def build_scope(table, predicate_builder = predicate_builder(table))
|
||||
Relation.create(klass, table, predicate_builder)
|
||||
Relation.create(
|
||||
klass,
|
||||
table: table,
|
||||
predicate_builder: predicate_builder
|
||||
)
|
||||
end
|
||||
|
||||
def join_primary_key(*)
|
||||
|
|
|
@ -22,7 +22,7 @@ module ActiveRecord
|
|||
alias :loaded? :loaded
|
||||
alias :locked? :lock_value
|
||||
|
||||
def initialize(klass, table, predicate_builder, values = {})
|
||||
def initialize(klass, table: klass.arel_table, predicate_builder: klass.predicate_builder, values: {})
|
||||
@klass = klass
|
||||
@table = table
|
||||
@values = values
|
||||
|
|
|
@ -23,7 +23,11 @@ module ActiveRecord
|
|||
# build a relation to merge in rather than directly merging
|
||||
# the values.
|
||||
def other
|
||||
other = Relation.create(relation.klass, relation.table, relation.predicate_builder)
|
||||
other = Relation.create(
|
||||
relation.klass,
|
||||
table: relation.table,
|
||||
predicate_builder: relation.predicate_builder
|
||||
)
|
||||
hash.each { |k, v|
|
||||
if k == :joins
|
||||
if Hash === v
|
||||
|
|
|
@ -69,7 +69,7 @@ module ActiveRecord
|
|||
private
|
||||
|
||||
def relation_with(values)
|
||||
result = Relation.create(klass, table, predicate_builder, values)
|
||||
result = Relation.create(klass, values: values)
|
||||
result.extend(*extending_values) if extending_values.any?
|
||||
result
|
||||
end
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -326,5 +326,13 @@ class FakeKlass
|
|||
def enforce_raw_sql_whitelist(*args)
|
||||
# noop
|
||||
end
|
||||
|
||||
def arel_table
|
||||
Post.arel_table
|
||||
end
|
||||
|
||||
def predicate_builder
|
||||
Post.predicate_builder
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue