mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Refactor build_from_hash
to convert dot notation to hash first
This ensures that we're handling all forms of nested tables the same way. We're aware that the `convert_dot_notation_to_hash` method will cause a performance hit, and we intend to come back to it once we've refactored some of the surrounding code. [Melissa Xie & Melanie Gilman]
This commit is contained in:
parent
3317d6958c
commit
fcc3cbc71f
2 changed files with 40 additions and 29 deletions
|
@ -21,35 +21,8 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
def build_from_hash(attributes)
|
def build_from_hash(attributes)
|
||||||
queries = []
|
attributes = convert_dot_notation_to_hash(attributes.stringify_keys)
|
||||||
builder = self
|
expand_from_hash(attributes)
|
||||||
|
|
||||||
attributes.each do |column, value|
|
|
||||||
if value.is_a?(Hash)
|
|
||||||
if value.empty?
|
|
||||||
queries << '1=0'
|
|
||||||
else
|
|
||||||
arel_table = Arel::Table.new(column)
|
|
||||||
association = klass._reflect_on_association(column)
|
|
||||||
builder = self.class.new(association && association.klass, arel_table)
|
|
||||||
|
|
||||||
value.each do |k, v|
|
|
||||||
queries.concat builder.expand(k, v)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
else
|
|
||||||
column = column.to_s
|
|
||||||
|
|
||||||
if column.include?('.')
|
|
||||||
table_name, column = column.split('.', 2)
|
|
||||||
arel_table = Arel::Table.new(table_name)
|
|
||||||
builder = self.class.new(klass, arel_table)
|
|
||||||
end
|
|
||||||
queries.concat builder.expand(column, value)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
queries
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def expand(column, value)
|
def expand(column, value)
|
||||||
|
@ -130,5 +103,37 @@ module ActiveRecord
|
||||||
protected
|
protected
|
||||||
|
|
||||||
attr_reader :klass, :table
|
attr_reader :klass, :table
|
||||||
|
|
||||||
|
def expand_from_hash(attributes)
|
||||||
|
return ["1=0"] if attributes.empty?
|
||||||
|
|
||||||
|
attributes.flat_map do |key, value|
|
||||||
|
if value.is_a?(Hash)
|
||||||
|
arel_table = Arel::Table.new(key)
|
||||||
|
association = klass._reflect_on_association(key)
|
||||||
|
builder = self.class.new(association && association.klass, arel_table)
|
||||||
|
|
||||||
|
builder.expand_from_hash(value)
|
||||||
|
else
|
||||||
|
expand(key, value)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def convert_dot_notation_to_hash(attributes)
|
||||||
|
dot_notation = attributes.keys.select { |s| s.include?(".") }
|
||||||
|
|
||||||
|
dot_notation.each do |key|
|
||||||
|
table_name, column_name = key.split(".")
|
||||||
|
value = attributes.delete(key)
|
||||||
|
attributes[table_name] ||= {}
|
||||||
|
|
||||||
|
attributes[table_name] = attributes[table_name].merge(column_name => value)
|
||||||
|
end
|
||||||
|
|
||||||
|
attributes
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -489,6 +489,12 @@ class FinderTest < ActiveRecord::TestCase
|
||||||
assert_raise(ActiveRecord::RecordNotFound) { Topic.where(topics: { approved: true }).find(1) }
|
assert_raise(ActiveRecord::RecordNotFound) { Topic.where(topics: { approved: true }).find(1) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_find_on_combined_explicit_and_hashed_table_names
|
||||||
|
assert Topic.where('topics.approved' => false, topics: { author_name: "David" }).find(1)
|
||||||
|
assert_raise(ActiveRecord::RecordNotFound) { Topic.where('topics.approved' => true, topics: { author_name: "David" }).find(1) }
|
||||||
|
assert_raise(ActiveRecord::RecordNotFound) { Topic.where('topics.approved' => false, topics: { author_name: "Melanie" }).find(1) }
|
||||||
|
end
|
||||||
|
|
||||||
def test_find_with_hash_conditions_on_joined_table
|
def test_find_with_hash_conditions_on_joined_table
|
||||||
firms = Firm.joins(:account).where(:accounts => { :credit_limit => 50 })
|
firms = Firm.joins(:account).where(:accounts => { :credit_limit => 50 })
|
||||||
assert_equal 1, firms.size
|
assert_equal 1, firms.size
|
||||||
|
|
Loading…
Reference in a new issue