Skip to content
Permalink
Browse files

`WhereClause#predicates` does not need to be public

The only place it was accessed was in tests. Many of them have another
way that they can test their behavior, that doesn't involve reaching
into internals as far as they did. `AssociationScopeTest` is testing a
situation where the where clause would have one bind param per
predicate, so it can just ignore the predicates entirely. The where
chain test was primarly duplicating the logic tested on `WhereClause`
directly, so I instead just make sure it calls the appropriate method
which is fully tested in isolation.
  • Loading branch information...
sgrif committed Jan 27, 2015
1 parent c2c95cd commit d26dd00854c783bcb1249168bb3f4adf9f99be6c
@@ -1,7 +1,7 @@
module ActiveRecord
class Relation
class WhereClause # :nodoc:
attr_reader :predicates, :binds
attr_reader :binds

delegate :any?, :empty?, to: :predicates

@@ -73,6 +73,8 @@ def self.empty

protected

attr_reader :predicates

def referenced_columns
@referenced_columns ||= begin
equality_nodes = predicates.select { |n| equality_node?(n) }
@@ -8,12 +8,7 @@ class AssociationScopeTest < ActiveRecord::TestCase
test 'does not duplicate conditions' do
scope = AssociationScope.scope(Author.new.association(:welcome_posts),
Author.connection)
wheres = scope.where_clause.predicates.map(&:right)
binds = scope.where_clause.binds.map(&:last)
wheres.reject! { |node|
Arel::Nodes::BindParam === node
}
assert_equal wheres.uniq, wheres
assert_equal binds.uniq, binds
end
end
@@ -316,16 +316,16 @@ def test_association_protect_foreign_key
# would be convenient), because this would cause that scope to be applied to any callbacks etc.
def test_build_and_create_should_not_happen_within_scope
car = cars(:honda)
scoped_count = car.foo_bulbs.where_clause.predicates.count
scope = car.foo_bulbs.where_values_hash

bulb = car.foo_bulbs.build
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash

bulb = car.foo_bulbs.create
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash

bulb = car.foo_bulbs.create!
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
end

def test_no_sql_should_be_fired_if_association_already_loaded
@@ -237,16 +237,16 @@ def test_building_the_associated_object_with_an_unrelated_type

def test_build_and_create_should_not_happen_within_scope
pirate = pirates(:blackbeard)
scoped_count = pirate.association(:foo_bulb).scope.where_clause.predicates.count
scope = pirate.association(:foo_bulb).scope.where_values_hash

bulb = pirate.build_foo_bulb
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash

bulb = pirate.create_foo_bulb
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash

bulb = pirate.create_foo_bulb!
assert_not_equal scoped_count, bulb.scope_after_initialize.where_clause.predicates.count
assert_not_equal scope, bulb.scope_after_initialize.where_values_hash
end

def test_create_association
@@ -238,7 +238,7 @@ def test_proxy_association_accessor
end

def test_scoped_allows_conditions
assert developers(:david).projects.merge!(where: 'foo').where_clause.predicates.include?('foo')
assert developers(:david).projects.merge(where: 'foo').to_sql.include?('foo')
end

test "getting a scope from an association" do
@@ -11,22 +11,11 @@ def setup
@name = 'title'
end

def test_not_eq
def test_not_inverts_where_clause
relation = Post.where.not(title: 'hello')
expected_where_clause = Post.where(title: 'hello').where_clause.invert

assert_equal 1, relation.where_clause.predicates.length

value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first

assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
assert_equal 'hello', bind.last
end

def test_not_null
expected = Post.arel_table[@name].not_eq(nil)
relation = Post.where.not(title: nil)
assert_equal([expected], relation.where_clause.predicates)
assert_equal expected_where_clause, relation.where_clause
end

def test_not_with_nil
@@ -35,146 +24,81 @@ def test_not_with_nil
end
end

def test_not_in
expected = Post.arel_table[@name].not_in(%w[hello goodbye])
relation = Post.where.not(title: %w[hello goodbye])
assert_equal([expected], relation.where_clause.predicates)
end

def test_association_not_eq
expected = Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new)
expected = Arel::Nodes::Grouping.new(Comment.arel_table[@name].not_eq(Arel::Nodes::BindParam.new))
relation = Post.joins(:comments).where.not(comments: {title: 'hello'})
assert_equal(expected.to_sql, relation.where_clause.predicates.first.to_sql)
assert_equal(expected.to_sql, relation.where_clause.ast.to_sql)
end

def test_not_eq_with_preceding_where
relation = Post.where(title: 'hello').where.not(title: 'world')
expected_where_clause =
Post.where(title: 'hello').where_clause +
Post.where(title: 'world').where_clause.invert

value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first
assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
assert_equal 'hello', bind.last

value = relation.where_clause.predicates.last
bind = relation.where_clause.binds.last
assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
assert_equal 'world', bind.last
assert_equal expected_where_clause, relation.where_clause
end

def test_not_eq_with_succeeding_where
relation = Post.where.not(title: 'hello').where(title: 'world')
expected_where_clause =
Post.where(title: 'hello').where_clause.invert +
Post.where(title: 'world').where_clause

value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first
assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
assert_equal 'hello', bind.last

value = relation.where_clause.predicates.last
bind = relation.where_clause.binds.last
assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
assert_equal 'world', bind.last
end

def test_not_eq_with_string_parameter
expected = Arel::Nodes::Not.new("title = 'hello'")
relation = Post.where.not("title = 'hello'")
assert_equal([expected], relation.where_clause.predicates)
end

def test_not_eq_with_array_parameter
expected = Arel::Nodes::Not.new("title = 'hello'")
relation = Post.where.not(['title = ?', 'hello'])
assert_equal([expected], relation.where_clause.predicates)
assert_equal expected_where_clause, relation.where_clause
end

def test_chaining_multiple
relation = Post.where.not(author_id: [1, 2]).where.not(title: 'ruby on rails')
expected_where_clause =
Post.where(author_id: [1, 2]).where_clause.invert +
Post.where(title: 'ruby on rails').where_clause.invert

expected = Post.arel_table['author_id'].not_in([1, 2])
assert_equal(expected, relation.where_clause.predicates[0])

value = relation.where_clause.predicates[1]
bind = relation.where_clause.binds.first

assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::NotEqual
assert_equal 'ruby on rails', bind.last
assert_equal expected_where_clause, relation.where_clause
end

def test_rewhere_with_one_condition
relation = Post.where(title: 'hello').where(title: 'world').rewhere(title: 'alone')
expected = Post.where(title: 'alone')

assert_equal 1, relation.where_clause.predicates.size
value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first
assert_bound_ast value, Post.arel_table[@name], Arel::Nodes::Equality
assert_equal 'alone', bind.last
assert_equal expected.where_clause, relation.where_clause
end

def test_rewhere_with_multiple_overwriting_conditions
relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone', body: 'again')
expected = Post.where(title: 'alone', body: 'again')

assert_equal 2, relation.where_clause.predicates.size

value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first
assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality
assert_equal 'alone', bind.last

value = relation.where_clause.predicates[1]
bind = relation.where_clause.binds[1]
assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality
assert_equal 'again', bind.last
end

def assert_bound_ast value, table, type
assert_equal table, value.left
assert_kind_of type, value
assert_kind_of Arel::Nodes::BindParam, value.right
assert_equal expected.where_clause, relation.where_clause
end

def test_rewhere_with_one_overwriting_condition_and_one_unrelated
relation = Post.where(title: 'hello').where(body: 'world').rewhere(title: 'alone')
expected = Post.where(body: 'world', title: 'alone')

assert_equal 2, relation.where_clause.predicates.size

value = relation.where_clause.predicates.first
bind = relation.where_clause.binds.first

assert_bound_ast value, Post.arel_table['body'], Arel::Nodes::Equality
assert_equal 'world', bind.last

value = relation.where_clause.predicates.second
bind = relation.where_clause.binds.second

assert_bound_ast value, Post.arel_table['title'], Arel::Nodes::Equality
assert_equal 'alone', bind.last
assert_equal expected.where_clause, relation.where_clause
end

def test_rewhere_with_range
relation = Post.where(comments_count: 1..3).rewhere(comments_count: 3..5)

assert_equal 1, relation.where_clause.predicates.size
assert_equal Post.where(comments_count: 3..5), relation
end

def test_rewhere_with_infinite_upper_bound_range
relation = Post.where(comments_count: 1..Float::INFINITY).rewhere(comments_count: 3..5)

assert_equal 1, relation.where_clause.predicates.size
assert_equal Post.where(comments_count: 3..5), relation
end

def test_rewhere_with_infinite_lower_bound_range
relation = Post.where(comments_count: -Float::INFINITY..1).rewhere(comments_count: 3..5)

assert_equal 1, relation.where_clause.predicates.size
assert_equal Post.where(comments_count: 3..5), relation
end

def test_rewhere_with_infinite_range
relation = Post.where(comments_count: -Float::INFINITY..Float::INFINITY).rewhere(comments_count: 3..5)

assert_equal 1, relation.where_clause.predicates.size
assert_equal Post.where(comments_count: 3..5), relation
end
end
@@ -112,11 +112,12 @@ class WhereClauseTest < ActiveRecord::TestCase
end

test "ast groups its predicates with AND" do
where_clause = WhereClause.new([
predicates = [
table["id"].in([1, 2, 3]),
table["name"].eq(bind_param),
], [])
expected = Arel::Nodes::And.new(where_clause.predicates)
]
where_clause = WhereClause.new(predicates, [])
expected = Arel::Nodes::And.new(predicates)

assert_equal expected, where_clause.ast
end
@@ -156,7 +156,7 @@ def test_references_values_dont_duplicate
relation = Relation.new(FakeKlass, :b, nil)
relation = relation.merge where: :lol, readonly: true

assert_equal [:lol], relation.where_clause.predicates
assert_equal Relation::WhereClause.new([:lol], []), relation.where_clause
assert_equal true, relation.readonly_value
end

@@ -191,7 +191,7 @@ def self.sanitize_sql(args)

relation = Relation.new(klass, :b, nil)
relation.merge!(where: ['foo = ?', 'bar'])
assert_equal ['foo = bar'], relation.where_clause.predicates
assert_equal Relation::WhereClause.new(['foo = bar'], []), relation.where_clause
end

def test_merging_readonly_false
@@ -426,19 +426,19 @@ def test_default_scope_is_threadsafe

test "additional conditions are ANDed with the default scope" do
scope = DeveloperCalledJamis.where(name: "David")
assert_equal 2, scope.where_clause.predicates.length
assert_equal 2, scope.where_clause.ast.children.length
assert_equal [], scope.to_a
end

test "additional conditions in a scope are ANDed with the default scope" do
scope = DeveloperCalledJamis.david
assert_equal 2, scope.where_clause.predicates.length
assert_equal 2, scope.where_clause.ast.children.length
assert_equal [], scope.to_a
end

test "a scope can remove the condition from the default scope" do
scope = DeveloperCalledJamis.david2
assert_equal 1, scope.where_clause.predicates.length
assert_equal Developer.where(name: "David").map(&:id), scope.map(&:id)
assert_equal 1, scope.where_clause.ast.children.length
assert_equal Developer.where(name: "David"), scope
end
end
@@ -380,8 +380,8 @@ def test_size_should_use_length_when_results_are_loaded
end

def test_should_not_duplicates_where_values
where_values = Topic.where("1=1").scope_with_lambda.where_clause.predicates
assert_equal ["1=1"], where_values
relation = Topic.where("1=1")
assert_equal relation.where_clause, relation.scope_with_lambda.where_clause
end

def test_chaining_with_duplicate_joins
@@ -184,7 +184,7 @@ def test_ensure_that_method_scoping_is_correctly_restored
rescue
end

assert !Developer.all.where_clause.predicates.include?("name = 'Jamis'")
assert_not Developer.all.to_sql.include?("name = 'Jamis'"), "scope was not restored"
end

def test_default_scope_filters_on_joins

0 comments on commit d26dd00

Please sign in to comment.
You can’t perform that action at this time.