Consider all associations with :dataset options as instance-specific …

…associations

I don't think it makes sense to try to support a non-instance
specific association that uses the :dataset option.  Always
considering these assocaitions as instance-specific reduces noise
when using the instance_specific_default plugin, since it will
now only warn/raise for associations with blocks, and not
associations with :dataset options.

CHANGELOG

@@ -1,5 +1,7 @@
 === master
 
+* Consider all associations with :dataset options as instance-specific associations (jeremyevans)
+
 * Make Model.finalize_associations not break with instance-specific associations (jeremyevans)
 
 * Make association placeholder loader consider block if instance_specific: false association option is used (jeremyevans)

doc/association_basics.rdoc

@@ -1676,14 +1676,16 @@ instances.
 ==== :instance_specific
 
 This allows you to override the setting of whether the dataset contains instance
-specific code.  For example, if you are passing a block to the association,
+specific code.  If you are passing a block to the association,
 Sequel sets this to true by default, which disables some optimizations that
 would be invalid if the association is instance specific.  If you know that the
 block does not contain instance specific code, you can set this to false to
 reenable the optimizations.  Instance specific code is mostly commonly calling
-model instance methods inside an association block or :dataset proc, but also
+model instance methods inside an association block, but also
 includes cases where the association block can return different values based
 on the runtime environment, such as calls to <tt>Time.now</tt> in the block.
+Associations that use the :dataset option are always considered instance specific,
+even if explicitly specified otherwise.
 
 ==== :cartesian_product_number 
 

lib/sequel/model/associations.rb

@@ -1802,7 +1802,8 @@ def associate(type, name, opts = OPTS, &block)
           opts.merge!(:type => type, :name => name, :cache=>({} if cache_associations), :model => self)
 
           opts[:block] = block if block
-          if !opts.has_key?(:instance_specific) && (block || orig_opts[:block] || orig_opts[:dataset])
+          opts[:instance_specific] = true if orig_opts[:dataset]
+          if !opts.has_key?(:instance_specific) && (block || orig_opts[:block])
             # It's possible the association is instance specific, in that it depends on
             # values other than the foreign key value.  This needs to be checked for
             # in certain places to disable optimizations.
@@ -1814,7 +1815,7 @@ def associate(type, name, opts = OPTS, &block)
             raise(Error, "cannot clone an association to an association of different type (association #{name} with type #{type} cloning #{opts[:clone]} with type #{cloned_assoc[:type]})")
           end
 
-          opts[:use_placeholder_loader] = !opts[:instance_specific] && !opts[:eager_graph] && !orig_opts[:dataset]
+          opts[:use_placeholder_loader] = !opts[:instance_specific] && !opts[:eager_graph]
           opts[:eager_block] = opts[:block] unless opts.include?(:eager_block)
           opts[:graph_join_type] ||= :left_outer
           opts[:order_eager_graph] = true unless opts.include?(:order_eager_graph)

lib/sequel/plugins/instance_specific_default.rb

@@ -11,9 +11,10 @@ module Plugins
     # :instance_specific to false for associations that are not instance specific
     # can improve performance.
     #
-    # Associations are instance-specific if their block or :dataset option calls
-    # an instance method, where the value of the block or :dataset option varies
-    # based on runtime state.  For example, with the following three associations:
+    # Associations are instance-specific if their block calls
+    # a model instance method, or where the value of the block varies
+    # based on runtime state, and the variance is outside of a delayed evaluation.
+    # For example, with the following three associations:
     #
     #   Album.one_to_one :first_track, class: :Track do |ds|
     #     ds.where(number: 1)
@@ -45,6 +46,14 @@ module Plugins
     #     ds.where{date_updated > Date.today - 10}
     #   end
     #
+    # For the +recent_tracks+ association, instead of marking it instance_specific, you
+    # could also use a delayed evaluation, since it doesn't actually contain
+    # instance-specific code:
+    #
+    #   Album.one_to_many :recent_tracks, class: :Track, instance_specific: false do |ds|
+    #     ds.where{date_updated > Sequel.delay{Date.today - 10}}
+    #   end
+    #
     # Possible arguments to provide when loading the plugin:
     #
     # true :: Set the :instance_specific option to true
@@ -55,8 +64,8 @@ module Plugins
     #           an association could be instance-specific.
     #
     # Note that this plugin only affects associations which could be instance
-    # specific, such as those with blocks or with the :dataset option used, where
-    # the :instance_specific option was not specified when the association was created.
+    # specific (those with blocks), where the :instance_specific option was not
+    # specified when the association was created.
     #
     # Usage:
     #

spec/extensions/instance_specific_default_spec.rb

@@ -53,4 +53,17 @@ def @c.name; 'C' end
     c.many_to_one :c, :class=>@c do |ds| ds end
     c.association_reflection(:c)[:instance_specific].must_equal false
   end
+
+  it "should be ignored for associations with a :dataset option" do
+    @c.plugin :instance_specific_default, false
+    @c.many_to_one :c, :class=>@c, :dataset=>proc{|r| r.associated_class.where(:id=>id)}
+    @c.association_reflection(:c)[:instance_specific].must_equal true
+  end
+
+  it "should be considered for when cloning association with block" do
+    @c.plugin :instance_specific_default, false
+    @c.many_to_one :c, :class=>@c do |ds| ds end
+    @c.many_to_one :c, :clone=>:c
+    @c.association_reflection(:c)[:instance_specific].must_equal false
+  end
 end