Fix data cache on model for associations, fixes remi#248

Author: pencil

lib/her/model/associations/association.rb

@@ -47,11 +47,12 @@ def fetch(opts = {})
           attribute_value = @parent.attributes[@name]
           return @opts[:default].try(:dup) if @parent.attributes.include?(@name) && (attribute_value.nil? || !attribute_value.nil? && attribute_value.empty?) && @params.empty?
 
-          if @parent.attributes[@name].blank? || @params.any?
-            path = build_association_path lambda { "#{@parent.request_path(@params)}#{@opts[:path]}" }
-            @klass.get(path, @params)
-          else
-            @parent.attributes[@name]
+          return @cached_result unless @params.any? || @cached_result.nil?
+          return @parent.attributes[@name] unless @params.any? || @parent.attributes[@name].blank?
+
+          path = build_association_path lambda { "#{@parent.request_path(@params)}#{@opts[:path]}" }
+          @klass.get(path, @params).tap do |result|
+            @cached_result = result unless @params.any?
           end
         end
 

lib/her/model/associations/belongs_to_association.rb

@@ -76,12 +76,13 @@ def fetch
           data_key_value = @parent.attributes[@opts[:data_key].to_sym]
           return @opts[:default].try(:dup) if (@parent.attributes.include?(@name) && @parent.attributes[@name].nil? && @params.empty?) || (@parent.persisted? && foreign_key_value.blank? && data_key_value.blank?)
 
-          if @parent.attributes[@name].blank? || @params.any?
-            path_params = @parent.attributes.merge(@params.merge(@klass.primary_key => foreign_key_value))
-            path = build_association_path lambda { @klass.build_request_path(path_params) }
-            @klass.get(path, @params)
-          else
-            @parent.attributes[@name]
+          return @cached_result unless @params.any? || @cached_result.nil?
+          return @parent.attributes[@name] unless @params.any? || @parent.attributes[@name].blank?
+
+          path_params = @parent.attributes.merge(@params.merge(@klass.primary_key => foreign_key_value))
+          path = build_association_path lambda { @klass.build_request_path(path_params) }
+          @klass.get(path, @params).tap do |result|
+            @cached_result = result if @params.blank?
           end
         end
 

spec/model/associations_spec.rb

@@ -137,6 +137,7 @@
     let(:user_with_included_data_after_create) { Foo::User.create }
     let(:user_with_included_data_after_save_existing) { Foo::User.save_existing(5, :name => "Clancy Brown") }
     let(:user_with_included_data_after_destroy) { Foo::User.new(:id => 5).destroy }
+    let(:comment_without_included_parent_data) { Foo::Comment.new(:id => 7, :user_id => 1) }
 
     it "maps an array of included data through has_many" do
       @user_with_included_data.comments.first.should be_a(Foo::Comment)
@@ -149,6 +150,14 @@
       @user_with_included_data.comments.first.user.object_id.should == @user_with_included_data.object_id
     end
 
+    it "does fetch the parent models data only once" do
+      comment_without_included_parent_data.user.object_id.should == comment_without_included_parent_data.user.object_id
+    end
+
+    it "does fetch the parent models data that was cached if called with parameters" do
+      comment_without_included_parent_data.user.object_id.should_not == comment_without_included_parent_data.user.where(:a => 2).object_id
+    end
+
     it "uses the given inverse_of key to set the parent model" do
       @user_with_included_data.posts.first.admin.object_id.should == @user_with_included_data.object_id
     end
@@ -164,6 +173,14 @@
       @user_with_included_data.comments.where(:foo_id => 1).length.should == 1
     end
 
+    it "fetches data that was not included through has_many only once" do
+      @user_without_included_data.comments.first.object_id.should == @user_without_included_data.comments.first.object_id
+    end
+
+    it "fetches data that was cached through has_many if called with parameters" do
+      @user_without_included_data.comments.first.object_id.should_not == @user_without_included_data.comments.where(:foo_id => 1).first.object_id
+    end
+
     it "maps an array of included data through has_one" do
       @user_with_included_data.role.should be_a(Foo::Role)
       @user_with_included_data.role.object_id.should == @user_with_included_data.role.object_id