kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

try using regexes

Browse source
This commit is contained in:
Ben Toews 2017-10-11 13:16:57 -06:00 committed by Matthew Draper
parent 5180fe2cd8
commit 798557145c
27 changed files with 120 additions and 166 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
activerecord/lib/active_record/attribute_methods.rb
View file

@ -167,12 +167,24 @@ module ActiveRecord
end
end
def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc:
# Regexp whitelist. Matches the following:
# "#{table_name}.#{column_name}"
# "#{column_name}"
COLUMN_NAME_WHITELIST = /\A(?:\w+\.)?\w+\z/i
# Regexp whitelist. Matches the following:
# "#{table_name}.#{column_name}"
# "#{table_name}.#{column_name} #{direction}"
# "#{column_name}"
# "#{column_name} #{direction}"
COLUMN_NAME_ORDER_WHITELIST = /\A(?:\w+\.)?\w+(?:\s+asc|\s+desc)?\z/i
def enforce_raw_sql_whitelist(args, whitelist: COLUMN_NAME_WHITELIST) # :nodoc:
unexpected = args.reject do |arg|
whitelist.include?(arg.to_s) ||
arg.kind_of?(Arel::Node) ||
arg.kind_of?(Arel::Node) ||
arg.is_a?(Arel::Nodes::SqlLiteral) ||
arg.is_a?(Arel::Attributes::Attribute)
arg.is_a?(Arel::Attributes::Attribute) ||
arg.to_s.split(/\s*,\s*/).all? { |part| whitelist.match?(part) }
end
return if unexpected.none?
@ -195,22 +207,6 @@ module ActiveRecord
end
end
# Can the given name be treated as a column name? Returns true if name
# is attribute or attribute alias.
#
# class Person < ActiveRecord::Base
# end
#
# Person.respond_to_attribute?(:name)
# # => true
#
# Person.respond_to_attribute?("foo")
# # => false
def respond_to_attribute?(name) # :nodoc:
name = name.to_s
attribute_names.include?(name) || attribute_aliases.include?(name)
end
# Returns true if the given attribute exists, otherwise false.
#
# class Person < ActiveRecord::Base
@ -242,18 +238,6 @@ module ActiveRecord
ConnectionAdapters::NullColumn.new(name)
end
end
# An Array of String attribute names and aliases for accessing those
# attributes.
#
# class Person < ActiveRecord::Base
# end
#
# Person.attribute_names_and_aliases
# # => ["id", "created_at", "updated_at", "name", "age"]
def attribute_names_and_aliases # :nodoc:
attribute_names + attribute_aliases.keys
end
end
# A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>,

12
activerecord/lib/active_record/relation/calculations.rb
View file

@ -175,7 +175,7 @@ module ActiveRecord
# See also #ids.
#
def pluck(*column_names)
if loaded? && (column_names.map(&:to_s) - @klass.attribute_names_and_aliases).empty?
if loaded? && (column_names.map(&:to_s) - @klass.attribute_names - @klass.attribute_aliases.keys).empty?
return records.pluck(*column_names)
end
@ -183,10 +183,10 @@ module ActiveRecord
relation = apply_join_dependency
relation.pluck(*column_names)
else
enforce_raw_sql_whitelist(column_names, whitelist: allowed_pluck_columns)
enforce_raw_sql_whitelist(column_names)
relation = spawn
relation.select_values = column_names.map { |cn|
@klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn
@klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn
}
result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) }
result.cast_values(klass.attribute_types)
@ -203,12 +203,6 @@ module ActiveRecord
private
def allowed_pluck_columns
@klass.attribute_names_and_aliases.map do |name|
[name, "#{table_name}.#{name}"]
end.flatten
end
def has_include?(column_name)
eager_loading? || (includes_values.present? && column_name && column_name != :all)
end

21
activerecord/lib/active_record/relation/query_methods.rb
View file

@ -299,7 +299,7 @@ module ActiveRecord
def order!(*args) # :nodoc:
@klass.enforce_raw_sql_whitelist(
column_names_from_order_arguments(args),
whitelist: allowed_order_columns
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
)
preprocess_order_args(args)
@ -326,7 +326,7 @@ module ActiveRecord
def reorder!(*args) # :nodoc:
@klass.enforce_raw_sql_whitelist(
column_names_from_order_arguments(args),
whitelist: allowed_order_columns
whitelist: AttributeMethods::ClassMethods::COLUMN_NAME_ORDER_WHITELIST
)
preprocess_order_args(args)
@ -928,20 +928,6 @@ module ActiveRecord
private
def allowed_order_columns
@klass.attribute_names_and_aliases.map do |name|
[name, "#{table_name}.#{name}"].map do |name|
[
name,
"#{name} asc",
"#{name} ASC",
"#{name} desc",
"#{name} DESC"
]
end
end.flatten
end
# Extract column names from arguments passed to #order or #reorder.
def column_names_from_order_arguments(args)
args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg }
@ -1097,9 +1083,6 @@ module ActiveRecord
when Arel::Nodes::Ordering
o.reverse
when String
# ensure we're not dealing with string subclass (Eg. Arel::Nodes::SqlLiteral)
o = String.new(o)
if does_not_support_reverse?(o)
raise IrreversibleOrderError, "Order #{o.inspect} can not be reversed automatically"
end

8
activerecord/test/cases/associations/cascaded_eager_loading_test.rb
View file

@ -121,7 +121,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
silly.parent_id = 1
assert silly.save
topics = Topic.all.merge!(includes: :replies, order: ["topics.id", Arel.sql("replies_topics.id")]).to_a
topics = Topic.all.merge!(includes: :replies, order: ["topics.id", "replies_topics.id"]).to_a
assert_no_queries do
assert_equal 2, topics[0].replies.size
assert_equal 0, topics[1].replies.size
@ -129,14 +129,14 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
end
def test_eager_association_loading_with_belongs_to_sti
replies = Reply.all.merge!(includes: :topic, order: Arel.sql("topics.id")).to_a
replies = Reply.all.merge!(includes: :topic, order: "topics.id").to_a
assert_includes replies, topics(:second)
assert_not_includes replies, topics(:first)
assert_equal topics(:first), assert_no_queries { replies.first.topic }
end
def test_eager_association_loading_with_multiple_stis_and_order
author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: ["authors.name", Arel.sql("comments.body"), Arel.sql("very_special_comments_posts.body")], where: "posts.id = 4").first
author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first
assert_equal authors(:david), author
assert_no_queries do
author.posts.first.special_comments
@ -145,7 +145,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
end
def test_eager_association_loading_of_stis_with_multiple_references
authors = Author.all.merge!(includes: { posts: { special_comments: { post: [ :special_comments, :very_special_comment ] } } }, order: Arel.sql("comments.body, very_special_comments_posts.body"), where: "posts.id = 4").to_a
authors = Author.all.merge!(includes: { posts: { special_comments: { post: [ :special_comments, :very_special_comment ] } } }, order: "comments.body, very_special_comments_posts.body", where: "posts.id = 4").to_a
assert_equal [authors(:david)], authors
assert_no_queries do
authors.first.posts.first.special_comments.first.post.special_comments

2
activerecord/test/cases/associations/eager_load_nested_include_test.rb
View file

@ -120,7 +120,7 @@ class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase
assert_nothing_raised do
# @davey_mcdave doesn't have any author_favorites
includes = { posts: :comments, categorizations: :category, author_favorites: :favorite_author }
Author.all.merge!(includes: includes, where: { authors: { name: @davey_mcdave.name } }, order: Arel.sql("categories.name")).to_a
Author.all.merge!(includes: includes, where: { authors: { name: @davey_mcdave.name } }, order: "categories.name").to_a
end
end
end

20
activerecord/test/cases/associations/eager_test.rb
View file

@ -62,7 +62,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
end
def test_loading_with_one_association_with_non_preload
posts = Post.all.merge!(includes: :last_comment, order: Arel.sql("comments.id DESC")).to_a
posts = Post.all.merge!(includes: :last_comment, order: "comments.id DESC").to_a
post = posts.find { |p| p.id == 1 }
assert_equal Post.find(1).last_comment, post.last_comment
end
@ -317,12 +317,12 @@ class EagerAssociationTest < ActiveRecord::TestCase
end
def test_nested_loading_through_has_one_association_with_order_on_association
aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: Arel.sql("authors.id")).find(author_addresses(:david_address).id)
aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: "authors.id").find(author_addresses(:david_address).id)
assert_equal aa.author.posts.count, aa.author.posts.length
end
def test_nested_loading_through_has_one_association_with_order_on_nested_association
aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: Arel.sql("posts.id")).find(author_addresses(:david_address).id)
aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: "posts.id").find(author_addresses(:david_address).id)
assert_equal aa.author.posts.count, aa.author.posts.length
end
@ -420,7 +420,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
def test_eager_association_loading_with_belongs_to_and_order_string_with_unquoted_table_name
assert_nothing_raised do
Comment.all.merge!(includes: :post, order: Arel.sql("posts.id")).to_a
Comment.all.merge!(includes: :post, order: "posts.id").to_a
end
end
@ -1089,12 +1089,12 @@ class EagerAssociationTest < ActiveRecord::TestCase
end
def test_order_on_join_table_with_include_and_limit
assert_equal 5, Developer.all.merge!(includes: "projects", order: Arel.sql("developers_projects.joined_on DESC"), limit: 5).to_a.size
assert_equal 5, Developer.all.merge!(includes: "projects", order: "developers_projects.joined_on DESC", limit: 5).to_a.size
end
def test_eager_loading_with_order_on_joined_table_preloads
posts = assert_queries(2) do
Post.all.merge!(joins: :comments, includes: :author, order: Arel.sql("comments.id DESC")).to_a
Post.all.merge!(joins: :comments, includes: :author, order: "comments.id DESC").to_a
end
assert_equal posts(:eager_other), posts[1]
assert_equal authors(:mary), assert_no_queries { posts[1].author }
@ -1190,7 +1190,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
if current_adapter?(:OracleAdapter)
firm = Firm.all.merge!(includes: :clients_using_primary_key, order: "clients_using_primary_keys_companies"[0, 30] + ".name").find(1)
else
firm = Firm.all.merge!(includes: :clients_using_primary_key, order: Arel.sql("clients_using_primary_keys_companies.name")).find(1)
firm = Firm.all.merge!(includes: :clients_using_primary_key, order: "clients_using_primary_keys_companies.name").find(1)
end
assert_no_queries do
assert_equal expected, firm.clients_using_primary_key
@ -1207,7 +1207,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
def test_include_has_one_using_primary_key
expected = accounts(:signals37)
firm = Firm.all.merge!(includes: :account_using_primary_key, order: Arel.sql("accounts.id")).to_a.detect { |f| f.id == 1 }
firm = Firm.all.merge!(includes: :account_using_primary_key, order: "accounts.id").to_a.detect { |f| f.id == 1 }
assert_no_queries do
assert_equal expected, firm.account_using_primary_key
end
@ -1278,7 +1278,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
def test_join_eager_with_empty_order_should_generate_valid_sql
assert_nothing_raised do
Post.includes(:comments).order(Arel.sql("")).where(comments: { body: "Thank you for the welcome" }).first
Post.includes(:comments).order("").where(comments: { body: "Thank you for the welcome" }).first
end
end
@ -1382,7 +1382,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
test "preloading associations with string joins and order references" do
author = assert_queries(2) {
Author.includes(:posts).joins("LEFT JOIN posts ON posts.author_id = authors.id").order(Arel.sql("posts.title DESC")).first
Author.includes(:posts).joins("LEFT JOIN posts ON posts.author_id = authors.id").order("posts.title DESC").first
}
assert_no_queries {
assert_equal 5, author.posts.size

2
activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
View file

@ -587,7 +587,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
end
def test_find_should_append_to_association_order
ordered_developers = projects(:active_record).developers.order(Arel.sql("projects.id"))
ordered_developers = projects(:active_record).developers.order("projects.id")
assert_equal ["developers.name desc, developers.id desc", "projects.id"], ordered_developers.order_values
end

2
activerecord/test/cases/associations/has_many_associations_test.rb
View file

@ -550,7 +550,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
end
def test_find_should_append_to_association_order
ordered_clients = companies(:first_firm).clients_sorted_desc.order(Arel.sql("companies.id"))
ordered_clients = companies(:first_firm).clients_sorted_desc.order("companies.id")
assert_equal ["id DESC", "companies.id"], ordered_clients.order_values
end

6
activerecord/test/cases/associations/has_one_through_associations_test.rb
View file

@ -139,7 +139,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
def test_has_one_through_nonpreload_eagerloading
members = assert_queries(1) do
Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: Arel.sql("clubs.name")).to_a #force fallback
Member.all.merge!(includes: :club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback
end
assert_equal 1, members.size
assert_not_nil assert_no_queries { members[0].club }
@ -147,7 +147,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
def test_has_one_through_nonpreload_eager_loading_through_polymorphic
members = assert_queries(1) do
Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: Arel.sql("clubs.name")).to_a #force fallback
Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name").to_a #force fallback
end
assert_equal 1, members.size
assert_not_nil assert_no_queries { members[0].sponsor_club }
@ -156,7 +156,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
def test_has_one_through_nonpreload_eager_loading_through_polymorphic_with_more_than_one_through_record
Sponsor.new(sponsor_club: clubs(:crazy_club), sponsorable: members(:groucho)).save!
members = assert_queries(1) do
Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: Arel.sql("clubs.name DESC")).to_a #force fallback
Member.all.merge!(includes: :sponsor_club, where: ["members.name = ?", "Groucho Marx"], order: "clubs.name DESC").to_a #force fallback
end
assert_equal 1, members.size
assert_not_nil assert_no_queries { members[0].sponsor_club }

8
activerecord/test/cases/associations/inverse_associations_test.rb
View file

@ -223,7 +223,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
f.man.name = "Mungo"
assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"
m = Man.all.merge!(where: { name: "Gordon" }, includes: :face, order: Arel.sql("faces.id")).first
m = Man.all.merge!(where: { name: "Gordon" }, includes: :face, order: "faces.id").first
f = m.face
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
m.name = "Bongo"
@ -329,7 +329,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
end
m = Man.all.merge!(where: { name: "Gordon" }, includes: :interests, order: Arel.sql("interests.id")).first
m = Man.all.merge!(where: { name: "Gordon" }, includes: :interests, order: "interests.id").first
is = m.interests
is.each do |i|
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@ -554,7 +554,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
m.face.description = "pleasing"
assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"
f = Face.all.merge!(includes: :man, order: Arel.sql("men.id"), where: { description: "trusting" }).first
f = Face.all.merge!(includes: :man, order: "men.id", where: { description: "trusting" }).first
m = f.man
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
f.description = "gormless"
@ -637,7 +637,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
m.polymorphic_face.description = "pleasing"
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
f = Face.all.merge!(where: { description: "confused" }, includes: :man, order: Arel.sql("men.id")).first
f = Face.all.merge!(where: { description: "confused" }, includes: :man, order: "men.id").first
m = f.polymorphic_man
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
f.description = "gormless"

2
activerecord/test/cases/associations/join_model_test.rb
View file

@ -420,7 +420,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
end
def test_eager_load_has_many_through_has_many
author = Author.all.merge!(where: ["name = ?", "David"], includes: :comments, order: Arel.sql("comments.id")).first
author = Author.all.merge!(where: ["name = ?", "David"], includes: :comments, order: "comments.id").first
SpecialComment.new; VerySpecialComment.new
assert_no_queries do
assert_equal [1, 2, 3, 5, 6, 7, 8, 9, 10, 12], author.comments.collect(&:id)

6
activerecord/test/cases/associations/nested_through_associations_test.rb
View file

@ -171,7 +171,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
def test_has_many_through_has_one_with_has_many_through_source_reflection_preload_via_joins
assert_includes_and_joins_equal(
Member.where("member_details.id" => member_details(:groucho).id).order(Arel.sql("member_details.id")),
Member.where("member_details.id" => member_details(:groucho).id).order("member_details.id"),
[members(:groucho), members(:some_other_guy)], :organization_member_details
)
@ -203,7 +203,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
def test_has_many_through_has_one_through_with_has_many_source_reflection_preload_via_joins
assert_includes_and_joins_equal(
Member.where("member_details.id" => member_details(:groucho).id).order(Arel.sql("member_details.id")),
Member.where("member_details.id" => member_details(:groucho).id).order("member_details.id"),
[members(:groucho), members(:some_other_guy)], :organization_member_details_2
)
@ -341,7 +341,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase
def test_has_many_through_belongs_to_with_has_many_through_source_reflection_preload_via_joins
assert_includes_and_joins_equal(
Categorization.where("taggings.id" => taggings(:welcome_general).id).order(Arel.sql("taggings.id")),
Categorization.where("taggings.id" => taggings(:welcome_general).id).order("taggings.id"),
[categorizations(:david_welcome_general)], :post_taggings
)
end

6
activerecord/test/cases/base_test.rb
View file

@ -1109,12 +1109,12 @@ class BasicsTest < ActiveRecord::TestCase
end
def test_find_multiple_ordered_last
last = Developer.all.merge!(order: Arel.sql("developers.name, developers.salary DESC")).last
assert_equal last, Developer.all.merge!(order: Arel.sql("developers.name, developers.salary DESC")).to_a.last
last = Developer.all.merge!(order: "developers.name, developers.salary DESC").last
assert_equal last, Developer.all.merge!(order: "developers.name, developers.salary DESC").to_a.last
end
def test_find_keeps_multiple_order_values
combined = Developer.all.merge!(order: Arel.sql("developers.name, developers.salary")).to_a
combined = Developer.all.merge!(order: "developers.name, developers.salary").to_a
assert_equal combined, Developer.all.merge!(order: ["developers.name", "developers.salary"]).to_a
end

14
activerecord/test/cases/calculations_test.rb
View file

@ -144,7 +144,7 @@ class CalculationsTest < ActiveRecord::TestCase
end
def test_should_order_by_calculation
c = Account.group(:firm_id).order(Arel.sql("sum_credit_limit desc, firm_id")).sum(:credit_limit)
c = Account.group(:firm_id).order("sum_credit_limit desc, firm_id").sum(:credit_limit)
assert_equal [105, 60, 53, 50, 50], c.keys.collect { |k| c[k] }
assert_equal [6, 2, 9, 1], c.keys.compact
end
@ -644,7 +644,7 @@ class CalculationsTest < ActiveRecord::TestCase
end
def test_pluck_with_qualified_column_name
assert_equal [1, 2, 3, 4, 5], Topic.order(:id).pluck(Arel.sql("topics.id"))
assert_equal [1, 2, 3, 4, 5], Topic.order(:id).pluck("topics.id")
end
def test_pluck_auto_table_name_prefix
@ -659,7 +659,7 @@ class CalculationsTest < ActiveRecord::TestCase
def test_pluck_not_auto_table_name_prefix_if_column_joined
Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
assert_equal [7], Company.joins(:contracts).pluck(Arel.sql("developer_id"))
assert_equal [7], Company.joins(:contracts).pluck(:developer_id)
end
def test_pluck_with_selection_clause
@ -684,7 +684,7 @@ class CalculationsTest < ActiveRecord::TestCase
def test_pluck_not_auto_table_name_prefix_if_column_included
Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
ids = Company.includes(:contracts).pluck(Arel.sql("developer_id"))
ids = Company.includes(:contracts).pluck(:developer_id)
assert_equal Company.count, ids.length
assert_equal [7], ids.compact
end
@ -704,12 +704,12 @@ class CalculationsTest < ActiveRecord::TestCase
def test_pluck_with_multiple_columns_and_selection_clause
assert_equal [[1, 50], [2, 50], [3, 50], [4, 60], [5, 55], [6, 53]],
Account.pluck(Arel.sql("id, credit_limit"))
Account.pluck("id, credit_limit")
end
def test_pluck_with_multiple_columns_and_includes
Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
companies_and_developers = Company.order(Arel.sql("companies.id")).includes(:contracts).pluck(:name, Arel.sql("developer_id"))
companies_and_developers = Company.order("companies.id").includes(:contracts).pluck(:name, :developer_id)
assert_equal Company.count, companies_and_developers.length
assert_equal ["37signals", nil], companies_and_developers.first
@ -731,7 +731,7 @@ class CalculationsTest < ActiveRecord::TestCase
def test_pluck_columns_with_same_name
expected = [["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]]
actual = Topic.joins(:replies)
.pluck(Arel.sql("topics.title"), Arel.sql("replies_topics.title"))
.pluck("topics.title", "replies_topics.title")
assert_equal expected, actual
end

14
activerecord/test/cases/finder_test.rb
View file

@ -243,15 +243,15 @@ class FinderTest < ActiveRecord::TestCase
end
def test_exists_with_joins
assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order(Arel.sql("replies_topics.created_at DESC")).exists?
assert_equal true, Topic.joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end
def test_exists_with_left_joins
assert_equal true, Topic.left_joins(:replies).where(replies_topics: { approved: true }).order(Arel.sql("replies_topics.created_at DESC")).exists?
assert_equal true, Topic.left_joins(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end
def test_exists_with_eager_load
assert_equal true, Topic.eager_load(:replies).where(replies_topics: { approved: true }).order(Arel.sql("replies_topics.created_at DESC")).exists?
assert_equal true, Topic.eager_load(:replies).where(replies_topics: { approved: true }).order("replies_topics.created_at DESC").exists?
end
def test_exists_with_includes_limit_and_empty_result
@ -267,8 +267,8 @@ class FinderTest < ActiveRecord::TestCase
def test_exists_with_distinct_association_includes_limit_and_order
author = Author.first
assert_equal false, author.unique_categorized_posts.includes(:special_comments).order(Arel.sql("comments.tags_count DESC")).limit(0).exists?
assert_equal true, author.unique_categorized_posts.includes(:special_comments).order(Arel.sql("comments.tags_count DESC")).limit(1).exists?
assert_equal false, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(0).exists?
assert_equal true, author.unique_categorized_posts.includes(:special_comments).order("comments.tags_count DESC").limit(1).exists?
end
def test_exists_should_reference_correct_aliases_while_joining_tables_of_has_many_through_association
@ -1125,11 +1125,11 @@ class FinderTest < ActiveRecord::TestCase
def test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct
assert_equal 2, Post.includes(authors: :author_address).
where.not(author_addresses: { id: nil }).
order(Arel.sql("author_addresses.id DESC")).limit(2).to_a.size
order("author_addresses.id DESC").limit(2).to_a.size
assert_equal 3, Post.includes(author: :author_address, authors: :author_address).
where.not(author_addresses_authors: { id: nil }).
order(Arel.sql("author_addresses_authors.id DESC")).limit(3).to_a.size
order("author_addresses_authors.id DESC").limit(3).to_a.size
end
def test_find_with_nil_inside_set_passed_for_one_attribute

10
activerecord/test/cases/relation/merging_test.rb
View file

@ -47,8 +47,8 @@ class RelationMergingTest < ActiveRecord::TestCase
def test_relation_merging_with_eager_load
relations = []
relations << Post.order(Arel.sql("comments.id DESC")).merge(Post.eager_load(:last_comment)).merge(Post.all)
relations << Post.eager_load(:last_comment).merge(Post.order(Arel.sql("comments.id DESC"))).merge(Post.all)
relations << Post.order("comments.id DESC").merge(Post.eager_load(:last_comment)).merge(Post.all)
relations << Post.eager_load(:last_comment).merge(Post.order("comments.id DESC")).merge(Post.all)
relations.each do |posts|
post = posts.find { |p| p.id == 1 }
@ -126,19 +126,19 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase
test "merging order relations" do
posts_by_author_name = Post.limit(3).joins(:author).
merge(Author.order(:name)).pluck(Arel.sql("authors.name"))
merge(Author.order(:name)).pluck("authors.name")
assert_equal ["Bob", "Bob", "David"], posts_by_author_name
posts_by_author_name = Post.limit(3).joins(:author).
merge(Author.order(Arel.sql("name"))).pluck(Arel.sql("authors.name"))
merge(Author.order("name")).pluck("authors.name")
assert_equal ["Bob", "Bob", "David"], posts_by_author_name
end
test "merging order relations (using a hash argument)" do
posts_by_author_name = Post.limit(4).joins(:author).
merge(Author.order(name: :desc)).pluck(Arel.sql("authors.name"))
merge(Author.order(name: :desc)).pluck("authors.name")
assert_equal ["Mary", "Mary", "Mary", "David"], posts_by_author_name
end

2
activerecord/test/cases/relation/mutation_test.rb
View file

@ -94,7 +94,7 @@ module ActiveRecord
end
test "reverse_order!" do
@relation = Post.order(Arel.sql("title ASC, comments_count DESC"))
@relation = Post.order("title ASC, comments_count DESC")
relation.reverse_order!

49
activerecord/test/cases/relations_test.rb
View file

@ -220,7 +220,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_finding_with_arel_assoc_order
topics = Topic.order("id" => :desc)
topics = Topic.order(Arel.sql("id") => :desc)
assert_equal 5, topics.to_a.size
assert_equal topics(:fifth).title, topics.first.title
end
@ -232,13 +232,13 @@ class RelationTest < ActiveRecord::TestCase
end
def test_finding_with_reversed_arel_assoc_order
topics = Topic.order("id" => :asc).reverse_order
topics = Topic.order(Arel.sql("id") => :asc).reverse_order
assert_equal 5, topics.to_a.size
assert_equal topics(:fifth).title, topics.first.title
end
def test_reverse_order_with_function
topics = Topic.order(Arel.sql("length(title)")).reverse_order
topics = Topic.order("length(title)").reverse_order
assert_equal topics(:second).title, topics.first.title
end
@ -248,9 +248,9 @@ class RelationTest < ActiveRecord::TestCase
end
def test_reverse_order_with_function_other_predicates
topics = Topic.order(Arel.sql("author_name, length(title), id")).reverse_order
topics = Topic.order("author_name, length(title), id").reverse_order
assert_equal topics(:second).title, topics.first.title
topics = Topic.order(Arel.sql("length(author_name), id, length(title)")).reverse_order
topics = Topic.order("length(author_name), id, length(title)").reverse_order
assert_equal topics(:fifth).title, topics.first.title
end
@ -319,7 +319,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_raising_exception_on_invalid_hash_params
e = assert_raise(ArgumentError) { Topic.order(Arel.sql("name"), "id DESC", id: :asfsdf) }
e = assert_raise(ArgumentError) { Topic.order(:name, "id DESC", id: :asfsdf) }
assert_equal 'Direction "asfsdf" is invalid. Valid directions are: [:asc, :desc, :ASC, :DESC, "asc", "desc", "ASC", "DESC"]', e.message
end
@ -373,11 +373,8 @@ class RelationTest < ActiveRecord::TestCase
def test_finding_with_cross_table_order_and_limit
tags = Tag.includes(:taggings).
order(
"tags.name asc",
Arel.sql("taggings.taggable_id asc"),
Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)")
).limit(1).to_a
order("tags.name asc", "taggings.taggable_id asc", Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)")).
limit(1).to_a
assert_equal 1, tags.length
end
@ -507,7 +504,7 @@ class RelationTest < ActiveRecord::TestCase
def test_eager_association_loading_of_stis_with_multiple_references
authors = Author.eager_load(posts: { special_comments: { post: [ :special_comments, :very_special_comment ] } }).
order(Arel.sql("comments.body, very_special_comments_posts.body")).where("posts.id = 4").to_a
order("comments.body, very_special_comments_posts.body").where("posts.id = 4").to_a
assert_equal [authors(:david)], authors
assert_no_queries do
@ -580,7 +577,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_includes_with_select
query = Post.select("comments_count AS ranking").order(Arel.sql("ranking")).includes(:comments)
query = Post.select("comments_count AS ranking").order("ranking").includes(:comments)
.where(comments: { id: 1 })
assert_equal ["comments_count AS ranking"], query.select_values
@ -649,9 +646,9 @@ class RelationTest < ActiveRecord::TestCase
def test_to_sql_on_eager_join
expected = assert_sql {
Post.eager_load(:last_comment).order(Arel.sql("comments.id DESC")).to_a
Post.eager_load(:last_comment).order("comments.id DESC").to_a
}.first
actual = Post.eager_load(:last_comment).order(Arel.sql("comments.id DESC")).to_sql
actual = Post.eager_load(:last_comment).order("comments.id DESC").to_sql
assert_equal expected, actual
end
@ -662,7 +659,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_loading_with_one_association_with_non_preload
posts = Post.eager_load(:last_comment).order(Arel.sql("comments.id DESC"))
posts = Post.eager_load(:last_comment).order("comments.id DESC")
post = posts.find { |p| p.id == 1 }
assert_equal Post.find(1).last_comment, post.last_comment
end
@ -1421,7 +1418,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_ordering_with_extra_spaces
assert_equal authors(:david), Author.order(Arel.sql("id DESC , name DESC")).last
assert_equal authors(:david), Author.order("id DESC , name DESC").last
end
def test_update_all_with_blank_argument
@ -1457,7 +1454,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_update_all_with_joins_and_offset_and_order
all_comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order(Arel.sql("posts.id"), "comments.id")
all_comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order("posts.id", "comments.id")
count = all_comments.count
comments = all_comments.offset(1)
@ -1567,7 +1564,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_automatically_added_order_references
scope = Post.order(Arel.sql("comments.body"))
scope = Post.order("comments.body")
assert_equal ["comments"], scope.references_values
scope = Post.order(Arel.sql("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}"))
@ -1577,14 +1574,14 @@ class RelationTest < ActiveRecord::TestCase
assert_equal ["comments"], scope.references_values
end
scope = Post.order(Arel.sql("comments.body"), Arel.sql("yaks.body"))
scope = Post.order("comments.body", "yaks.body")
assert_equal ["comments", "yaks"], scope.references_values
# Don't infer yaks, let's not go down that road again...
scope = Post.order(Arel.sql("comments.body, yaks.body"))
scope = Post.order("comments.body, yaks.body")
assert_equal ["comments"], scope.references_values
scope = Post.order(Arel.sql("comments.body asc"))
scope = Post.order("comments.body asc")
assert_equal ["comments"], scope.references_values
scope = Post.order(Arel.sql("foo(comments.body)"))
@ -1592,7 +1589,7 @@ class RelationTest < ActiveRecord::TestCase
end
def test_automatically_added_reorder_references
scope = Post.reorder(Arel.sql("comments.body"))
scope = Post.reorder("comments.body")
assert_equal %w(comments), scope.references_values
scope = Post.reorder(Arel.sql("#{Comment.quoted_table_name}.#{Comment.quoted_primary_key}"))
@ -1602,14 +1599,14 @@ class RelationTest < ActiveRecord::TestCase
assert_equal ["comments"], scope.references_values
end
scope = Post.reorder(Arel.sql("comments.body"), Arel.sql("yaks.body"))
scope = Post.reorder("comments.body", "yaks.body")
assert_equal %w(comments yaks), scope.references_values
# Don't infer yaks, let's not go down that road again...
scope = Post.reorder(Arel.sql("comments.body, yaks.body"))
scope = Post.reorder("comments.body, yaks.body")
assert_equal %w(comments), scope.references_values
scope = Post.reorder(Arel.sql("comments.body asc"))
scope = Post.reorder("comments.body asc")
assert_equal %w(comments), scope.references_values
scope = Post.reorder(Arel.sql("foo(comments.body)"))

12
activerecord/test/cases/scoping/default_scoping_test.rb
View file

@ -80,7 +80,7 @@ class DefaultScopingTest < ActiveRecord::TestCase
end
def test_scope_overwrites_default
expected = Developer.all.merge!(order: Arel.sql("salary DESC, name DESC")).to_a.collect(&:name)
expected = Developer.all.merge!(order: "salary DESC, name DESC").to_a.collect(&:name)
received = DeveloperOrderedBySalary.by_name.to_a.collect(&:name)
assert_equal expected, received
end
@ -92,7 +92,7 @@ class DefaultScopingTest < ActiveRecord::TestCase
end
def test_order_after_reorder_combines_orders
expected = Developer.order(Arel.sql("name DESC, id DESC")).collect { |dev| [dev.name, dev.id] }
expected = Developer.order("name DESC, id DESC").collect { |dev| [dev.name, dev.id] }
received = Developer.order("name ASC").reorder("name DESC").order("id DESC").collect { |dev| [dev.name, dev.id] }
assert_equal expected, received
end
@ -104,12 +104,12 @@ class DefaultScopingTest < ActiveRecord::TestCase
end
def test_unscope_after_reordering_and_combining
expected = Developer.order(Arel.sql("id DESC, name DESC")).collect { |dev| [dev.name, dev.id] }
received = DeveloperOrderedBySalary.reorder("name DESC").unscope(:order).order(Arel.sql("id DESC, name DESC")).collect { |dev| [dev.name, dev.id] }
expected = Developer.order("id DESC, name DESC").collect { |dev| [dev.name, dev.id] }
received = DeveloperOrderedBySalary.reorder("name DESC").unscope(:order).order("id DESC, name DESC").collect { |dev| [dev.name, dev.id] }
assert_equal expected, received
expected_2 = Developer.all.collect { |dev| [dev.name, dev.id] }
received_2 = Developer.order(Arel.sql("id DESC, name DESC")).unscope(:order).collect { |dev| [dev.name, dev.id] }
received_2 = Developer.order("id DESC, name DESC").unscope(:order).collect { |dev| [dev.name, dev.id] }
assert_equal expected_2, received_2
expected_3 = Developer.all.collect { |dev| [dev.name, dev.id] }
@ -192,7 +192,7 @@ class DefaultScopingTest < ActiveRecord::TestCase
end
def test_order_to_unscope_reordering
scope = DeveloperOrderedBySalary.order(Arel.sql("salary DESC, name ASC")).reverse_order.unscope(:order)
scope = DeveloperOrderedBySalary.order("salary DESC, name ASC").reverse_order.unscope(:order)
assert !/order/i.match?(scope.to_sql)
end

4
activerecord/test/cases/unsafe_raw_sql_test.rb
View file

@ -110,7 +110,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
test "order: disallows invalid column name" do
with_unsafe_raw_sql_disabled do
assert_raises(ActiveRecord::UnknownAttributeReference) do
Post.order("foo asc").pluck(:id)
Post.order("len(title) asc").pluck(:id)
end
end
end
@ -126,7 +126,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
test "order: disallows invalid column with direction" do
with_unsafe_raw_sql_disabled do
assert_raises(ActiveRecord::UnknownAttributeReference) do
Post.order(foo: :asc).pluck(:id)
Post.order("len(title)" => :asc).pluck(:id)
end
end
end

20
activerecord/test/models/author.rb
View file

@ -7,7 +7,7 @@ class Author < ActiveRecord::Base
has_many :very_special_comments, through: :posts
has_many :posts_with_comments, -> { includes(:comments) }, class_name: "Post"
has_many :popular_grouped_posts, -> { includes(:comments).group("type").having("SUM(comments_count) > 1").select("type") }, class_name: "Post"
has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order(Arel.sql("comments.id")) }, class_name: "Post"
has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order("comments.id") }, class_name: "Post"
has_many :posts_sorted_by_id_limited, -> { order("posts.id").limit(1) }, class_name: "Post"
has_many :posts_with_categories, -> { includes(:categories) }, class_name: "Post"
has_many :posts_with_comments_and_categories, -> { includes(:comments, :categories).order("posts.id") }, class_name: "Post"
@ -20,15 +20,15 @@ class Author < ActiveRecord::Base
end
end
has_many :comments_containing_the_letter_e, through: :posts, source: :comments
has_many :comments_with_order_and_conditions, -> { order(Arel.sql("comments.body")).where("comments.body like 'Thank%'") }, through: :posts, source: :comments
has_many :comments_with_order_and_conditions, -> { order("comments.body").where("comments.body like 'Thank%'") }, through: :posts, source: :comments
has_many :comments_with_include, -> { includes(:post).where(posts: { type: "Post" }) }, through: :posts, source: :comments
has_many :comments_for_first_author, -> { for_first_author }, through: :posts, source: :comments
has_many :first_posts
has_many :comments_on_first_posts, -> { order(Arel.sql("posts.id desc, comments.id asc")) }, through: :first_posts, source: :comments
has_many :comments_on_first_posts, -> { order("posts.id desc, comments.id asc") }, through: :first_posts, source: :comments
has_one :first_post
has_one :comment_on_first_post, -> { order(Arel.sql("posts.id desc, comments.id asc")) }, through: :first_post, source: :comments
has_one :comment_on_first_post, -> { order("posts.id desc, comments.id asc") }, through: :first_post, source: :comments
has_many :thinking_posts, -> { where(title: "So I was thinking") }, dependent: :delete_all, class_name: "Post"
has_many :welcome_posts, -> { where(title: "Welcome to the weblog") }, class_name: "Post"
@ -40,11 +40,11 @@ class Author < ActiveRecord::Base
-> { where(title: "Welcome to the weblog").where(Post.arel_table[:comments_count].gt(0)) },
class_name: "Post"
has_many :comments_desc, -> { order(Arel.sql("comments.id DESC")) }, through: :posts, source: :comments
has_many :comments_desc, -> { order("comments.id DESC") }, through: :posts, source: :comments
has_many :unordered_comments, -> { unscope(:order).distinct }, through: :posts_sorted_by_id_limited, source: :comments
has_many :funky_comments, through: :posts, source: :comments
has_many :ordered_uniq_comments, -> { distinct.order(Arel.sql("comments.id")) }, through: :posts, source: :comments
has_many :ordered_uniq_comments_desc, -> { distinct.order(Arel.sql("comments.id DESC")) }, through: :posts, source: :comments
has_many :ordered_uniq_comments, -> { distinct.order("comments.id") }, through: :posts, source: :comments
has_many :ordered_uniq_comments_desc, -> { distinct.order("comments.id DESC") }, through: :posts, source: :comments
has_many :readonly_comments, -> { readonly }, through: :posts, source: :comments
has_many :special_posts
@ -107,15 +107,15 @@ class Author < ActiveRecord::Base
has_many :similar_posts, -> { distinct }, through: :tags, source: :tagged_posts
has_many :ordered_posts, -> { distinct }, through: :ordered_tags, source: :tagged_posts
has_many :distinct_tags, -> { select("DISTINCT tags.*").order(Arel.sql("tags.name")) }, through: :posts, source: :tags
has_many :distinct_tags, -> { select("DISTINCT tags.*").order("tags.name") }, through: :posts, source: :tags
has_many :tags_with_primary_key, through: :posts
has_many :books
has_many :unpublished_books, -> { where(status: [:proposed, :written]) }, class_name: "Book"
has_many :subscriptions, through: :books
has_many :subscribers, -> { order(Arel.sql("subscribers.nick")) }, through: :subscriptions
has_many :distinct_subscribers, -> { select("DISTINCT subscribers.*").order(Arel.sql("subscribers.nick")) }, through: :subscriptions, source: :subscriber
has_many :subscribers, -> { order("subscribers.nick") }, through: :subscriptions
has_many :distinct_subscribers, -> { select("DISTINCT subscribers.*").order("subscribers.nick") }, through: :subscriptions, source: :subscriber
has_one :essay, primary_key: :name, as: :writer
has_one :essay_category, through: :essay, source: :category

2
activerecord/test/models/category.rb
View file

@ -4,7 +4,7 @@ class Category < ActiveRecord::Base
has_and_belongs_to_many :posts
has_and_belongs_to_many :special_posts, class_name: "Post"
has_and_belongs_to_many :other_posts, class_name: "Post"
has_and_belongs_to_many :posts_with_authors_sorted_by_author_id, -> { includes(:authors).order(Arel.sql("authors.id")) }, class_name: "Post"
has_and_belongs_to_many :posts_with_authors_sorted_by_author_id, -> { includes(:authors).order("authors.id") }, class_name: "Post"
has_and_belongs_to_many :select_testing_posts,
-> { select "posts.*, 1 as correctness_marker" },

2
activerecord/test/models/comment.rb
View file

@ -81,7 +81,7 @@ class CommentThatAutomaticallyAltersPostBody < Comment
end
class CommentWithDefaultScopeReferencesAssociation < Comment
default_scope -> { includes(:developer).order(Arel.sql("developers.name")).references(:developer) }
default_scope -> { includes(:developer).order("developers.name").references(:developer) }
belongs_to :developer
end

2
activerecord/test/models/owner.rb
View file

@ -2,7 +2,7 @@
class Owner < ActiveRecord::Base
self.primary_key = :owner_id
has_many :pets, -> { order Arel.sql("pets.name desc") }
has_many :pets, -> { order "pets.name desc" }
has_many :toys, through: :pets
has_many :persons, through: :pets

2
activerecord/test/models/person.rb
View file

@ -19,7 +19,7 @@ class Person < ActiveRecord::Base
has_many :bad_references
has_many :fixed_bad_references, -> { where favourite: true }, class_name: "BadReference"
has_one :favourite_reference, -> { where "favourite=?", true }, class_name: "Reference"
has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order(Arel.sql("comments.id")) }, through: :readers, source: :post
has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order("comments.id") }, through: :readers, source: :post
has_many :first_posts, -> { where(id: [1, 2]) }, through: :readers
has_many :jobs, through: :references

6
activerecord/test/models/post.rb
View file

@ -85,7 +85,7 @@ class Post < ActiveRecord::Base
has_one :very_special_comment
has_one :very_special_comment_with_post, -> { includes(:post) }, class_name: "VerySpecialComment"
has_one :very_special_comment_with_post_with_joins, -> { joins(:post).order(Arel.sql("posts.id")) }, class_name: "VerySpecialComment"
has_one :very_special_comment_with_post_with_joins, -> { joins(:post).order("posts.id") }, class_name: "VerySpecialComment"
has_many :special_comments
has_many :nonexistent_comments, -> { where "comments.id < 0" }, class_name: "Comment"
@ -323,9 +323,5 @@ class FakeKlass
def enforce_raw_sql_whitelist(*args)
# noop
end
def attribute_names_and_aliases
[]
end
end
end

4
activerecord/test/models/project.rb
View file

@ -2,9 +2,9 @@
class Project < ActiveRecord::Base
belongs_to :mentor
has_and_belongs_to_many :developers, -> { distinct.order Arel.sql("developers.name desc, developers.id desc") }
has_and_belongs_to_many :developers, -> { distinct.order "developers.name desc, developers.id desc" }
has_and_belongs_to_many :readonly_developers, -> { readonly }, class_name: "Developer"
has_and_belongs_to_many :non_unique_developers, -> { order Arel.sql("developers.name desc, developers.id desc") }, class_name: "Developer"
has_and_belongs_to_many :non_unique_developers, -> { order "developers.name desc, developers.id desc" }, class_name: "Developer"
has_and_belongs_to_many :limited_developers, -> { limit 1 }, class_name: "Developer"
has_and_belongs_to_many :developers_named_david, -> { where("name = 'David'").distinct }, class_name: "Developer"
has_and_belongs_to_many :developers_named_david_with_hash_conditions, -> { where(name: "David").distinct }, class_name: "Developer"