Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Let's be less blasé about method visibility on association proxies

  • Loading branch information...
commit 15601c52e7c7094a6b7b54ef8acfc8299a4d6724 1 parent 63c73dd
@jonleighton jonleighton authored
View
75 activerecord/lib/active_record/associations/association_collection.rb
@@ -333,6 +333,24 @@ def respond_to?(method, include_private = false)
super || @reflection.klass.respond_to?(method, include_private)
end
+ def method_missing(method, *args, &block)
+ match = DynamicFinderMatch.match(method)
+ if match && match.creator?
+ attributes = match.attribute_names
+ return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)])
+ end
+
+ if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
+ super
+ elsif @reflection.klass.scopes[method]
+ @_scopes_cache ||= {}
+ @_scopes_cache[method] ||= {}
+ @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
+ else
+ scoped.readonly(nil).send(method, *args, &block)
+ end
+ end
+
protected
def association_scope
@@ -340,14 +358,6 @@ def association_scope
super.apply_finder_options(options)
end
- def select_value
- super || uniq_select_value
- end
-
- def uniq_select_value
- @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
- end
-
def load_target
if (!@owner.new_record? || foreign_key_present?) && !loaded?
targets = []
@@ -365,22 +375,28 @@ def load_target
target
end
- def method_missing(method, *args, &block)
- match = DynamicFinderMatch.match(method)
- if match && match.creator?
- attributes = match.attribute_names
- return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)])
- end
-
- if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
- super
- elsif @reflection.klass.scopes[method]
- @_scopes_cache ||= {}
- @_scopes_cache[method] ||= {}
- @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
+ def add_record_to_target_with_callbacks(record)
+ callback(:before_add, record)
+ yield(record) if block_given?
+ @target ||= [] unless loaded?
+ if @reflection.options[:uniq] && index = @target.index(record)
+ @target[index] = record
else
- scoped.readonly(nil).send(method, *args, &block)
+ @target << record
end
+ callback(:after_add, record)
+ set_inverse_instance(record)
+ record
+ end
+
+ private
+
+ def select_value
+ super || uniq_select_value
+ end
+
+ def uniq_select_value
+ @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
end
def custom_counter_sql
@@ -419,21 +435,6 @@ def find_target
records
end
- def add_record_to_target_with_callbacks(record)
- callback(:before_add, record)
- yield(record) if block_given?
- @target ||= [] unless loaded?
- if @reflection.options[:uniq] && index = @target.index(record)
- @target[index] = record
- else
- @target << record
- end
- callback(:after_add, record)
- set_inverse_instance(record)
- record
- end
-
- private
def merge_target_lists(loaded, existing)
return loaded if existing.empty?
return existing if loaded.empty?
View
105 activerecord/lib/active_record/associations/association_proxy.rb
@@ -84,6 +84,16 @@ def respond_to?(*args)
super || (load_target && @target.respond_to?(*args))
end
+ # Forwards any missing method call to the \target.
+ def method_missing(method, *args, &block)
+ if load_target
+ return super unless @target.respond_to?(method)
+ @target.send(method, *args, &block)
+ end
+ rescue NoMethodError => e
+ raise e, e.message.sub(/ for #<.*$/, " via proxy for #{@target}")
+ end
+
# Forwards <tt>===</tt> explicitly to the \target because the instance method
# removal above doesn't catch it. Loads the \target if needed.
def ===(other)
@@ -167,14 +177,6 @@ def scoped
end
protected
- def interpolate_sql(sql, record = nil)
- @owner.send(:interpolate_sql, sql, record)
- end
-
- # Forwards the call to the reflection class.
- def sanitize_sql(sql, table_name = @reflection.klass.table_name)
- @reflection.klass.send(:sanitize_sql, sql, table_name)
- end
# Construct the scope for this association.
#
@@ -190,19 +192,12 @@ 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.select(select_value) if select_value = self.select_value
+ if select = select_value
+ scope = scope.select(select)
+ end
scope.where(construct_owner_conditions)
end
- def select_value
- @reflection.options[:select]
- end
-
- # Implemented by (some) subclasses
- def creation_attributes
- { }
- end
-
def aliased_table
target_klass.arel_table
end
@@ -227,6 +222,47 @@ def target_scope
target_klass.scoped
end
+ # Loads the \target if needed and returns it.
+ #
+ # This method is abstract in the sense that it relies on +find_target+,
+ # which is expected to be provided by descendants.
+ #
+ # If the \target is already \loaded it is just returned. Thus, you can call
+ # +load_target+ unconditionally to get the \target.
+ #
+ # ActiveRecord::RecordNotFound is rescued within the method, and it is
+ # not reraised. The proxy is \reset and +nil+ is the return value.
+ def load_target
+ if !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass
+ @target = find_target
+ end
+
+ loaded
+ @target
+ rescue ActiveRecord::RecordNotFound
+ reset
+ end
+
+ private
+
+ def interpolate_sql(sql, record = nil)
+ @owner.send(:interpolate_sql, sql, record)
+ end
+
+ # Forwards the call to the reflection class.
+ def sanitize_sql(sql, table_name = @reflection.klass.table_name)
+ @reflection.klass.send(:sanitize_sql, sql, table_name)
+ end
+
+ def select_value
+ @reflection.options[:select]
+ end
+
+ # Implemented by (some) subclasses
+ def creation_attributes
+ { }
+ end
+
# Returns a hash linking the owner to the association represented by the reflection
def construct_owner_attributes(reflection = @reflection)
attributes = {}
@@ -257,39 +293,6 @@ def set_owner_attributes(record)
end
end
- # Loads the \target if needed and returns it.
- #
- # This method is abstract in the sense that it relies on +find_target+,
- # which is expected to be provided by descendants.
- #
- # If the \target is already \loaded it is just returned. Thus, you can call
- # +load_target+ unconditionally to get the \target.
- #
- # ActiveRecord::RecordNotFound is rescued within the method, and it is
- # not reraised. The proxy is \reset and +nil+ is the return value.
- def load_target
- if !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass
- @target = find_target
- end
-
- loaded
- @target
- rescue ActiveRecord::RecordNotFound
- reset
- end
-
- private
-
- # Forwards any missing method call to the \target.
- def method_missing(method, *args, &block)
- if load_target
- return super unless @target.respond_to?(method)
- @target.send(method, *args, &block)
- end
- rescue NoMethodError => e
- raise e, e.message.sub(/ for #<.*$/, " via proxy for #{@target}")
- end
-
# Should be true if there is a foreign key present on the @owner which
# references the target. This is used to determine whether we can load
# the target if the @owner is currently a new record (and therefore
View
1  activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -19,6 +19,7 @@ def updated?
end
private
+
def update_counters(record)
counter_cache_name = @reflection.counter_cache_column
View
19 activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -12,10 +12,6 @@ def initialize(owner, reflection)
protected
- def count_records
- load_target.size
- end
-
def insert_record(record, force = true, validate = true)
if record.new_record?
return false unless save_record(record, force, validate)
@@ -35,6 +31,16 @@ def insert_record(record, force = true, validate = true)
true
end
+ def association_scope
+ super.joins(construct_joins)
+ end
+
+ private
+
+ def count_records
+ load_target.size
+ end
+
def delete_records(records)
if sql = @reflection.options[:delete_sql]
records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
@@ -61,15 +67,10 @@ def construct_owner_conditions
super(join_table)
end
- def association_scope
- super.joins(construct_joins)
- end
-
def select_value
super || @reflection.klass.arel_table[Arel.star]
end
- private
def invertible_for?(record)
false
end
View
13 activerecord/lib/active_record/associations/has_many_association.rb
@@ -7,6 +7,14 @@ module Associations
# is provided by its child HasManyThroughAssociation.
class HasManyAssociation < AssociationCollection #:nodoc:
protected
+
+ def insert_record(record, force = false, validate = true)
+ set_owner_attributes(record)
+ save_record(record, force, validate)
+ end
+
+ private
+
# Returns the number of records in this collection.
#
# If the association has a counter cache it gets that value. Otherwise
@@ -45,11 +53,6 @@ def cached_counter_attribute_name
"#{@reflection.name}_count"
end
- def insert_record(record, force = false, validate = true)
- set_owner_attributes(record)
- save_record(record, force, validate)
- end
-
# Deletes the records according to the <tt>:dependent</tt> option.
def delete_records(records)
case @reflection.options[:dependent]
View
17 activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -30,13 +30,6 @@ def size
end
protected
- def target_reflection_has_associated_record?
- if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank?
- false
- else
- true
- end
- end
def insert_record(record, force = true, validate = true)
if record.new_record?
@@ -47,6 +40,16 @@ def insert_record(record, force = true, validate = true)
through_association.create!(construct_join_attributes(record))
end
+ private
+
+ def target_reflection_has_associated_record?
+ if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank?
+ false
+ else
+ true
+ end
+ end
+
# TODO - add dependent option support
def delete_records(records)
through_association = @owner.send(@reflection.through_reflection.name)
View
5 activerecord/lib/active_record/associations/has_one_association.rb
@@ -26,11 +26,14 @@ def replace(record, save = true)
self.target = record
end
- private
+ protected
+
def association_scope
super.order(@reflection.options[:order])
end
+ private
+
alias creation_attributes construct_owner_attributes
# The reason that the save param for replace is false, if for create (not just build),
View
1  activerecord/lib/active_record/associations/singular_association.rb
@@ -14,6 +14,7 @@ def build(attributes = {})
end
private
+
def find_target
scoped.first.tap { |record| set_inverse_instance(record) }
end
View
16 activerecord/lib/active_record/associations/through_association.rb
@@ -3,6 +3,13 @@ module ActiveRecord
module Associations
module ThroughAssociation
+ def conditions
+ @conditions = build_conditions unless defined?(@conditions)
+ @conditions
+ end
+
+ alias_method :sql_conditions, :conditions
+
protected
def target_scope
@@ -17,6 +24,8 @@ def association_scope
scope
end
+ private
+
# This scope affects the creation of the associated records (not the join records). At the
# moment we only support creating on a :through association when the source reflection is a
# belongs_to. Thus it's not necessary to set a foreign key on the associated record(s), so
@@ -92,11 +101,6 @@ def construct_join_attributes(associate)
join_attributes
end
- def conditions
- @conditions = build_conditions unless defined?(@conditions)
- @conditions
- end
-
def build_conditions
through_conditions = build_through_conditions
source_conditions = @reflection.source_reflection.options[:conditions]
@@ -127,8 +131,6 @@ def build_sti_condition
@reflection.through_reflection.klass.send(:type_condition).to_sql
end
- alias_method :sql_conditions, :conditions
-
def stale_state
if @reflection.through_reflection.macro == :belongs_to
@owner[@reflection.through_reflection.foreign_key].to_s
Please sign in to comment.
Something went wrong with that request. Please try again.