Make eager loading via tactical_eager_loading no longer modify object…

…s who already have a cached value for the association

This could potential cause backwards compatibility issues for
users expecting the old behavior, but I don't see a point of
eager loading an association is an association is already loaded,
unless the :eager_reload option is used to force it.

CHANGELOG

@@ -1,5 +1,7 @@
 === master
 
+* Make eager loading via tactical_eager_loading no longer modify objects who already have a cached value for the association (jeremyevans)
+
 * Make association cloning handle cases where clone association sets different :class option than cloned association (jeremyevans)
 
 * Make column schema entries on MySQL include an :extra entry for the Extra column in DESCRIBE output (bschmeck) (#1791)

lib/sequel/plugins/tactical_eager_loading.rb

@@ -143,9 +143,15 @@ def marshallable!
         def load_associated_objects(opts, dynamic_opts=OPTS, &block)
           dynamic_opts = load_association_objects_options(dynamic_opts, &block)
           name = opts[:name]
-          if (!associations.include?(name) || dynamic_opts[:eager_reload]) && opts[:allow_eager] != false && retrieved_by && !frozen? && !dynamic_opts[:callback] && !dynamic_opts[:reload]
+          eager_reload = dynamic_opts[:eager_reload]
+          if (!associations.include?(name) || eager_reload) && opts[:allow_eager] != false && retrieved_by && !frozen? && !dynamic_opts[:callback] && !dynamic_opts[:reload]
             begin
-              retrieved_by.send(:eager_load, retrieved_with.reject(&:frozen?), name=>dynamic_opts[:eager] || OPTS)
+              objects = if eager_reload
+                retrieved_with.reject(&:frozen?)
+              else
+                retrieved_with.reject{|x| x.frozen? || x.associations.include?(name)}
+              end
+              retrieved_by.send(:eager_load, objects, name=>dynamic_opts[:eager] || OPTS)
             rescue Sequel::UndefinedAssociation
               # This can happen if class table inheritance is used and the association
               # is only defined in a subclass.  This particular instance can use the

spec/extensions/tactical_eager_loading_spec.rb

@@ -93,6 +93,16 @@ class ::TacticalEagerLoadingModel < Sequel::Model(:t)
     ts.map{|x| x.associations.fetch(:parent, 1)}.must_equal [ts[2], ts[3], nil, nil]
   end
 
+  it "association getter methods should not clear associations for objects that already have a cached association" do
+    ts.first.parent(:reload=>true)
+    sql_match('SELECT * FROM t WHERE id = 101')
+    ts.map{|x| x.associations.fetch(:parent, 1)}.must_equal [ts[2], 1, 1, 1]
+    ts[1].associations.delete(:parent)
+    ts[1].parent
+    sql_match(/\ASELECT \* FROM t WHERE \(t\.id IN \(102\)\)\z/)
+    ts.map{|x| x.associations.fetch(:parent, 1)}.must_equal [ts[2], ts[3], nil, nil]
+  end
+
   it "association getter methods should support eagerly loading dependent associations via :eager" do
     parents = ts.map{|x| x.parent(:eager=>:children)}
     sql_match(/\ASELECT \* FROM t WHERE \(t\.id IN \(10[12], 10[12]\)\)\z/, /\ASELECT \* FROM t WHERE \(t\.parent_id IN/)