Permalink
Browse files

Add interpolation of association conditions back in, in the form of p…

…roc { ... } rather than instance_eval-ing strings
  • Loading branch information...
1 parent fd76058 commit a7e19b30ca71f62af516675023659be061b2b70a @jonleighton jonleighton committed Feb 11, 2011
View
@@ -50,8 +50,27 @@
(for example, add_name_to_users) use the reversible migration's `change`
method instead of the ordinary `up` and `down` methods. [Prem Sichanugrist]
-* Removed support for interpolated SQL conditions. Please use scoping
-along with attribute conditionals as a replacement.
+* Removed support for interpolating string SQL conditions on associations. Instead, you should
+ use a proc, like so:
+
+ Before:
+
+ has_many :things, :conditions => 'foo = #{bar}'
+
+ After:
+
+ has_many :things, :conditions => proc { "foo = #{bar}" }
+
+ Inside the proc, 'self' is the object which is the owner of the association, unless you are
+ eager loading the association, in which case 'self' is the class which the association is within.
+
+ You can have any "normal" conditions inside the proc, so the following will work too:
+
+ has_many :things, :conditions => proc { ["foo = ?", bar] }
+
+ Previously :insert_sql and :delete_sql on has_and_belongs_to_many association allowed you to call
+ 'record' to get the record being inserted or deleted. This is now passed as an argument to
+ the proc.
* Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH]. Example:
@@ -399,10 +399,18 @@ def find_associated_records(ids, reflection, preload_options)
end
end
+ def process_conditions(conditions, klass = self)
+ if conditions.respond_to?(:to_proc)
+ conditions = instance_eval(&conditions)
+ end
+
+ klass.send(:sanitize_sql, conditions)
+ end
+
def append_conditions(reflection, preload_options)
[
- ("(#{reflection.sanitized_conditions})" if reflection.sanitized_conditions),
- ("(#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]),
+ ('(' + process_conditions(reflection.options[:conditions], reflection.klass) + ')' if reflection.options[:conditions]),
+ ('(' + process_conditions(preload_options[:conditions]) + ')' if preload_options[:conditions]),
].compact.map { |x| Arel.sql x }
end
@@ -374,17 +374,15 @@ def uniq_select_value
def custom_counter_sql
if @reflection.options[:counter_sql]
- counter_sql = @reflection.options[:counter_sql]
+ interpolate(@reflection.options[:counter_sql])
else
# replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
- counter_sql = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
+ interpolate(@reflection.options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
end
-
- interpolate_sql(counter_sql)
end
def custom_finder_sql
- interpolate_sql(@reflection.options[:finder_sql])
+ interpolate(@reflection.options[:finder_sql])
end
def find_target
@@ -184,7 +184,8 @@ def construct_scope
def association_scope
scope = target_klass.unscoped
scope = scope.create_with(creation_attributes)
- scope = scope.apply_finder_options(@reflection.options.slice(:conditions, :readonly, :include))
+ scope = scope.apply_finder_options(@reflection.options.slice(:readonly, :include))
+ scope = scope.where(interpolate(@reflection.options[:conditions]))
if select = select_value
scope = scope.select(select)
end
@@ -240,8 +241,12 @@ def find_target?
!loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass
end
- def interpolate_sql(sql, record = nil)
- @owner.send(:interpolate_sql, sql, record)
+ def interpolate(sql, record = nil)
+ if sql.respond_to?(:to_proc)
+ @owner.send(:instance_exec, record, &sql)
+ else
+ sql
+ end
end
def select_value
@@ -108,6 +108,10 @@ def allocate_aliases
end
def process_conditions(conditions, table_name)
+ if conditions.respond_to?(:to_proc)
+ conditions = instance_eval(&conditions)
+ end
+
Arel.sql(sanitize_sql(conditions, table_name))
end
@@ -17,7 +17,7 @@ def insert_record(record, force = true, validate = true)
end
if @reflection.options[:insert_sql]
- @owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record))
+ @owner.connection.insert(interpolate(@reflection.options[:insert_sql], record))
else
stmt = join_table.compile_insert(
join_table[@reflection.foreign_key] => @owner.id,
@@ -42,7 +42,7 @@ def count_records
def delete_records(records, method)
if sql = @reflection.options[:delete_sql]
- records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
+ records.each { |record| @owner.connection.delete(interpolate(sql, record)) }
else
relation = join_table
stmt = relation.where(relation[@reflection.foreign_key].eq(@owner.id).
@@ -119,14 +119,14 @@ def add_conditions(scope)
scope = scope.where(@reflection.through_reflection.klass.send(:type_condition))
end
- scope = scope.where(@reflection.source_reflection.options[:conditions])
+ scope = scope.where(interpolate(@reflection.source_reflection.options[:conditions]))
scope.where(through_conditions)
end
# If there is a hash of conditions then we make sure the keys are scoped to the
# through table name if left ambiguous.
def through_conditions
- conditions = @reflection.through_reflection.options[:conditions]
+ conditions = interpolate(@reflection.through_reflection.options[:conditions])
if conditions.is_a?(Hash)
Hash[conditions.map { |key, value|
@@ -1790,12 +1790,6 @@ def quote_value(value, column = nil)
self.class.connection.quote(value, column)
end
- # Interpolate custom SQL string in instance context.
- # Optional record argument is meant for custom insert_sql.
- def interpolate_sql(sql, record = nil)
- instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__)
- end
-
# Instantiates objects for all attribute classes that needs more than one constructor parameter. This is done
# by calling new on the column type or aggregation type (through composed_of) object with these parameters.
# So having the pairs written_on(1) = "2004", written_on(2) = "6", written_on(3) = "24", will instantiate
@@ -668,6 +668,14 @@ def test_limited_eager_with_numeric_in_association
assert_equal people(:david, :susan), Person.find(:all, :include => [:readers, :primary_contact, :number1_fan], :conditions => "number1_fans_people.first_name like 'M%'", :order => 'people.id', :limit => 2, :offset => 0)
end
+ def test_preload_with_interpolation
+ post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id)
+ assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+
+ post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id)
+ assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
+ end
+
def test_polymorphic_type_condition
post = Post.find(posts(:thinking).id, :include => :taggings)
assert post.taggings.include?(taggings(:thinking_general))
@@ -72,7 +72,7 @@ class DeveloperWithCounterSQL < ActiveRecord::Base
:join_table => "developers_projects",
:association_foreign_key => "project_id",
:foreign_key => "developer_id",
- :counter_sql => 'SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}'
+ :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" }
end
class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
@@ -279,7 +279,6 @@ def test_counting_non_existant_items_using_sql
def test_counting_using_finder_sql
assert_equal 2, Firm.find(4).clients_using_sql.count
- assert_equal 2, Firm.find(4).clients_using_multiline_sql.count
end
def test_belongs_to_sanity
@@ -711,4 +711,11 @@ def test_deleting_from_has_many_through_a_belongs_to_should_not_try_to_update_co
post.author_addresses.delete(address)
assert post[:author_count].nil?
end
+
+ def test_interpolated_conditions
+ post = posts(:welcome)
+ assert !post.tags.empty?
+ assert_equal post.tags, post.interpolated_tags
+ assert_equal post.tags, post.interpolated_tags_2
+ end
end
@@ -243,6 +243,14 @@ def test_dependence_with_missing_association_and_nullify
firm.destroy
end
+ def test_finding_with_interpolated_condition
+ firm = Firm.find(:first)
+ superior = firm.clients.create(:name => 'SuperiorCo')
+ superior.rating = 10
+ superior.save
+ assert_equal 10, firm.clients_with_interpolated_conditions.first.rating
+ end
+
def test_assignment_before_child_saved
firm = Firm.find(1)
firm.account = a = Account.new("credit_limit" => 1000)
@@ -1295,12 +1295,6 @@ def test_count_with_join
assert_equal res6, res7
end
- def test_interpolate_sql
- assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo@bar') }
- assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar) baz') }
- assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar} baz') }
- end
-
def test_scoped_find_conditions
scoped_developers = Developer.send(:with_scope, :find => { :conditions => 'salary > 90000' }) do
Developer.find(:all, :conditions => 'id < 5')
@@ -48,19 +48,16 @@ class Firm < Company
has_many :dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :destroy
has_many :exclusively_dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all
has_many :limited_clients, :class_name => "Client", :limit => 1
+ has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => proc { "rating > #{rating}" }
has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id"
has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id"
- has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}'
- has_many :clients_using_multiline_sql, :class_name => "Client", :finder_sql => '
- SELECT
- companies.*
- FROM companies WHERE companies.client_of = #{id}'
+ has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }
has_many :clients_using_counter_sql, :class_name => "Client",
- :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}',
- :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = #{id}'
+ :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id} " },
+ :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" }
has_many :clients_using_zero_counter_sql, :class_name => "Client",
- :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}',
- :counter_sql => 'SELECT 0 FROM companies WHERE client_of = #{id}'
+ :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" },
+ :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" }
has_many :no_clients_using_counter_sql, :class_name => "Client",
:finder_sql => 'SELECT * FROM companies WHERE client_of = 1000',
:counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000'
@@ -41,6 +41,9 @@ def find_most_recent
has_many :author_categorizations, :through => :author, :source => :categorizations
has_many :author_addresses, :through => :author
+ has_many :comments_with_interpolated_conditions, :class_name => 'Comment',
+ :conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] }
+
has_one :very_special_comment
has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
has_many :special_comments
@@ -57,6 +60,10 @@ def add_joins_and_select
end
end
+ has_many :interpolated_taggings, :class_name => 'Tagging', :as => :taggable, :conditions => proc { "1 = #{1}" }
+ has_many :interpolated_tags, :through => :taggings
+ has_many :interpolated_tags_2, :through => :interpolated_taggings, :source => :tag
+
has_many :taggings_with_delete_all, :class_name => 'Tagging', :as => :taggable, :dependent => :delete_all
has_many :taggings_with_destroy, :class_name => 'Tagging', :as => :taggable, :dependent => :destroy
@@ -7,14 +7,15 @@ class Project < ActiveRecord::Base
has_and_belongs_to_many :developers_named_david, :class_name => "Developer", :conditions => "name = 'David'", :uniq => true
has_and_belongs_to_many :developers_named_david_with_hash_conditions, :class_name => "Developer", :conditions => { :name => 'David' }, :uniq => true
has_and_belongs_to_many :salaried_developers, :class_name => "Developer", :conditions => "salary > 0"
- has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => 'SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id'
- has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => '
- SELECT
- t.*, j.*
- FROM
- developers_projects j,
- developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id'
- has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => "DELETE FROM developers_projects WHERE project_id = \#{id} AND developer_id = \#{record.id}"
+ has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => proc { "SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" }
+ has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => proc {
+ "SELECT
+ t.*, j.*
+ FROM
+ developers_projects j,
+ developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id"
+ }
+ has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => proc { |record| "DELETE FROM developers_projects WHERE project_id = #{id} AND developer_id = #{record.id}" }
has_and_belongs_to_many :developers_with_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || '<new>'}"},
:after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || '<new>'}"},
:before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"},
@@ -6,6 +6,7 @@ class Tagging < ActiveRecord::Base
belongs_to :tag, :include => :tagging
belongs_to :super_tag, :class_name => 'Tag', :foreign_key => 'super_tag_id'
belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id'
+ belongs_to :interpolated_tag, :class_name => 'Tag', :foreign_key => :tag_id, :conditions => proc { "1 = #{1}" }
belongs_to :taggable, :polymorphic => true, :counter_cache => true
has_many :things, :through => :taggable
end

0 comments on commit a7e19b3

Please sign in to comment.