Updated Resource to be dirty when associations are dirty

[#950 state:resolved]

lib/dm-core/collection.rb

@@ -884,11 +884,11 @@ def clean?
     # Checks if any resources have unsaved changes
     #
     # @return [Boolean]
-    #  true if a resource may be persisted
+    #  true if the resources have unsaved changed
     #
     # @api public
     def dirty?
-      loaded_entries.any? { |resource| resource.dirty? }
+      dirty_object?
     end
 
     # Gets a Human-readable representation of this collection,
@@ -923,6 +923,22 @@ def relationships
       model.relationships(repository.name)
     end
 
+    # Checks if any resources have unsaved changes
+    #
+    # @param [Array] except
+    #   list of object ids to not check for dirtyness
+    #
+    # @return [Boolean]
+    #  true if the resources have unsaved changes
+    #
+    # @api private
+    def dirty_object?(*except)
+      loaded_entries.any? do |resource|
+        next if except.include?(resource.object_id)
+        resource.dirty_object?(*except)
+      end
+    end
+
     private
 
     # Initializes a new Collection identified by the query

lib/dm-core/resource.rb

@@ -145,13 +145,7 @@ def clean?
     #
     # @api public
     def dirty?
-      if original_attributes.any?
-        true
-      elsif new?
-        model.serial || properties.any? { |property| property.default? }
-      else
-        false
-      end
+      dirty_object?
     end
 
     # Returns the value of the attribute.
@@ -294,7 +288,7 @@ def attributes=(attributes)
     def reload
       if saved?
         eager_load(loaded_properties)
-        child_relationships.each { |relationship| relationship.get!(self).reload }
+        child_collections.each { |children| children.reload }
       end
 
       self
@@ -543,6 +537,20 @@ def dirty_attributes
       dirty_attributes
     end
 
+    # Checks if the resource has unsaved changes
+    #
+    # @param [Array] except
+    #   list of object ids to not check for dirtyness
+    #
+    # @return [Boolean]
+    #  true if resource may be persisted
+    #
+    # @api private
+    def dirty_object?(*except)
+      except << object_id
+      dirty_self? || dirty_parents?(*except) || dirty_children?(*except)
+    end
+
     # Saves the resource
     #
     # @return [Boolean]
@@ -563,10 +571,10 @@ def save_self(safe = true)
     #   true if the parents were successfully saved
     #
     # @api private
-    def save_parents(safe = true)
+    def save_parents(safe = true, seen = [])
       parent_relationships.all? do |relationship|
         parent = relationship.get!(self)
-        if parent.dirty? ? parent.save_parents(safe) && parent.save_self(safe) : parent.saved?
+        if seen.include?(parent.object_id) || parent.save_parents(safe, seen << parent.object_id) && parent.save_self(safe)
           relationship.set(self, parent)  # set the FK values
         end
       end
@@ -579,9 +587,8 @@ def save_parents(safe = true)
     #
     # @api private
     def save_children(safe = true)
-      child_relationships.all? do |relationship|
-        association = relationship.get!(self)
-        safe ? association.save : association.save!
+      child_collections.all? do |collection|
+        safe ? collection.save : collection.save!
       end
     end
 
@@ -777,6 +784,63 @@ def child_relationships
       many_to_many + other
     end
 
+    # TODO: document
+    # @api private
+    def parent_resources
+      parent_relationships.map { |relationship| relationship.get!(self) }
+    end
+
+    # TODO: document
+    # @api private
+    def child_collections
+      child_relationships.map { |relationship| relationship.get!(self) }
+    end
+
+    # Checks if the resource has unsaved changes
+    #
+    # @return [Boolean]
+    #  true if the resource has unsaged changes
+    #
+    # @api private
+    def dirty_self?
+      if original_attributes.any?
+        true
+      elsif new?
+        model.serial || properties.any? { |property| property.default? }
+      else
+        false
+      end
+    end
+
+    # Checks if the parents have unsaved changes
+    #
+    # @param [Array] except
+    #   list of object ids to not check for dirtyness
+    #
+    # @return [Boolean]
+    #  true if the parents have unsaved changes
+    #
+    # @api private
+    def dirty_parents?(*except)
+      parent_resources.any? do |parent|
+        next if except.include?(parent.object_id)
+        parent.dirty_object?(*except)
+      end
+    end
+
+    # Checks if the children have unsaved changes
+    #
+    # @param [Array] except
+    #   list of object ids to not check for dirtyness
+    #
+    # @return [Boolean]
+    #  true if the children have unsaved changes
+    #
+    # @api private
+    def dirty_children?(*except)
+      child_collections.any? { |children| children.dirty_object?(*except) }
+    end
+
     # Creates the resource with default values
     #
     # If resource is not dirty or a new (not yet saved),

spec/public/associations/many_to_one_spec.rb

@@ -12,6 +12,9 @@ class User
         property :description, Text
         property :admin,       Boolean, :accessor => :private
 
+        belongs_to :parent, self, :nullable => true
+        has n, :children, self, :inverse => :parent
+
         belongs_to :referrer, self, :nullable => true
         has n, :comments
 
@@ -51,6 +54,12 @@ class Paragraph
       end
     end
 
+    class ::Default
+      include DataMapper::Resource
+
+      property :name, String, :key => true, :default => 'a default value'
+    end
+
     @user_model      = Blog::User
     @author_model    = Blog::Author
     @comment_model   = Blog::Comment

spec/public/associations/one_to_one_spec.rb

@@ -12,6 +12,9 @@ class User
         property :description, Text
         property :admin,       Boolean, :accessor => :private
 
+        belongs_to :parent, self, :nullable => true
+        has n, :children, self, :inverse => :parent
+
         belongs_to :referrer, self, :nullable => true
         belongs_to :comment
 
@@ -50,6 +53,12 @@ class Paragraph
       end
     end
 
+    class ::Default
+      include DataMapper::Resource
+
+      property :name, String, :key => true, :default => 'a default value'
+    end
+
     @user_model      = Blog::User
     @author_model    = Blog::Author
     @comment_model   = Blog::Comment
@@ -93,6 +102,9 @@ class User
         property :description, Text
         property :admin,       Boolean, :accessor => :private
 
+        belongs_to :parent, self, :nullable => true
+        has n, :children, self, :inverse => :parent
+
         has 1, :referral_from, Referral, :child_key => [ :referree_name ]
         has 1, :referral_to,   Referral, :child_key => [ :referrer_name ]
 
@@ -131,6 +143,12 @@ class Paragraph
       end
     end
 
+    class ::Default
+      include DataMapper::Resource
+
+      property :name, String, :key => true, :default => 'a default value'
+    end
+
     @referral_model  = Blog::Referral
     @user_model      = Blog::User
     @author_model    = Blog::Author

spec/public/resource_spec.rb

@@ -12,6 +12,9 @@ class User
         property :description, Text
         property :admin,       Boolean, :accessor => :private
 
+        belongs_to :parent, self, :nullable => true
+        has n, :children, self, :inverse => :parent
+
         belongs_to :referrer, self, :nullable => true
         has n, :comments
 
@@ -51,6 +54,12 @@ class Paragraph
       end
     end
 
+    class ::Default
+      include DataMapper::Resource
+
+      property :name, String, :key => true, :default => 'a default value'
+    end
+
     @user_model      = Blog::User
     @author_model    = Blog::Author
     @comment_model   = Blog::Comment

spec/public/shared/resource_shared_spec.rb

@@ -252,6 +252,60 @@
     end
   end
 
+  it { @user.should respond_to(:dirty?) }
+
+  describe '#dirty?' do
+    describe 'on a record, with dirty attributes' do
+      before { @user.age = 100 }
+
+      it { @user.should be_dirty }
+    end
+
+    describe 'on a record, with no dirty attributes, and dirty parents' do
+      before :all do
+        @user.should_not be_dirty
+
+        parent = @user.parent = @user_model.new(:name => 'Parent')
+        parent.should be_dirty
+      end
+
+      it { @user.should be_dirty }
+    end
+
+    describe 'on a record, with no dirty attributes, and dirty children' do
+      before :all do
+        @user.should_not be_dirty
+
+        child = @user.children.new(:name => 'Child')
+        child.should be_dirty
+      end
+
+      it { @user.should be_dirty }
+    end
+
+    describe 'on a saved record, with no dirty attributes' do
+      it { @user.should_not be_dirty }
+    end
+
+    describe 'on a new record, with no dirty attributes, no default attributes, and no identity field' do
+      before { @user = @user_model.new }
+
+      it { @user.should_not be_dirty }
+    end
+
+    describe 'on a new record, with no dirty attributes, no default attributes, and an identity field' do
+      before { @comment = @comment_model.new }
+
+      it { @comment.should be_dirty }
+    end
+
+    describe 'on a new record, with no dirty attributes, default attributes, and no identity field' do
+      before { @default = Default.new }
+
+      it { @default.should be_dirty }
+    end
+  end
+
   it { @user.should respond_to(:eql?) }
 
   describe '#eql?' do

spec/public/transaction_spec.rb

@@ -12,6 +12,9 @@ class User
         property :description, Text
         property :admin,       Boolean, :accessor => :private
 
+        belongs_to :parent, self, :nullable => true
+        has n, :children, self, :inverse => :parent
+
         belongs_to :referrer, self, :nullable => true
         has n, :comments
 
@@ -51,6 +54,12 @@ class Paragraph
       end
     end
 
+    class ::Default
+      include DataMapper::Resource
+
+      property :name, String, :key => true, :default => 'a default value'
+    end
+
     @user_model      = Blog::User
     @author_model    = Blog::Author
     @comment_model   = Blog::Comment

spec/semipublic/associations/many_to_one_spec.rb

@@ -12,40 +12,23 @@ class ::User
       has n, :comments
     end
 
-    # This is a special class that needs to be an exact copy of User
-    class ::Clone
-      include DataMapper::Resource
-
-      property :name,        String, :key => true
-      property :age,         Integer
-      property :description, Text
-    end
-
     class ::Comment
       include DataMapper::Resource
 
-      property :id,   Serial
-      property :body, Text
+      property :id, Serial
 
       belongs_to :user
     end
 
-    class ::Default
-      include DataMapper::Resource
-
-      property :name, String, :key => true, :default => 'a default value'
-    end
-
     @user_model    = User
     @comment_model = Comment
   end
 
   supported_by :all do
     before :all do
-      comment = @comment_model.create(:body => 'Cool spec', :user => User.create(:name => 'dbussink', :age => 25, :description => 'Test'))
+      comment = @comment_model.create(:user => User.create(:name => 'dbussink', :age => 25, :description => 'Test'))
 
-      comment = @comment_model.get(*comment.key)
-      @user   = comment.user
+      @user = @comment_model.get(*comment.key).user
     end
 
     it_should_behave_like 'A semipublic Resource'