From 7ed6e370069f87108f659e5365fe83fd3fbfb210 Mon Sep 17 00:00:00 2001 From: Dan Kubb Date: Fri, 2 Oct 2009 13:27:03 -0700 Subject: [PATCH] Updated Resource to be dirty when associations are dirty [#950 state:resolved] --- lib/dm-core/collection.rb | 20 ++++- lib/dm-core/resource.rb | 90 ++++++++++++++++--- spec/public/associations/many_to_one_spec.rb | 9 ++ spec/public/associations/one_to_one_spec.rb | 18 ++++ spec/public/resource_spec.rb | 9 ++ spec/public/shared/resource_shared_spec.rb | 54 +++++++++++ spec/public/transaction_spec.rb | 9 ++ .../associations/many_to_one_spec.rb | 23 +---- spec/semipublic/resource_spec.rb | 24 ----- .../semipublic/shared/resource_shared_spec.rb | 45 ---------- 10 files changed, 197 insertions(+), 104 deletions(-) diff --git a/lib/dm-core/collection.rb b/lib/dm-core/collection.rb index f02c6bdc..f06f7973 100644 --- a/lib/dm-core/collection.rb +++ b/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 diff --git a/lib/dm-core/resource.rb b/lib/dm-core/resource.rb index 4a563dfa..829e14d2 100644 --- a/lib/dm-core/resource.rb +++ b/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), diff --git a/spec/public/associations/many_to_one_spec.rb b/spec/public/associations/many_to_one_spec.rb index 4145b8bf..67ca39b6 100644 --- a/spec/public/associations/many_to_one_spec.rb +++ b/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 diff --git a/spec/public/associations/one_to_one_spec.rb b/spec/public/associations/one_to_one_spec.rb index 117dc583..7bedfe5d 100644 --- a/spec/public/associations/one_to_one_spec.rb +++ b/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 diff --git a/spec/public/resource_spec.rb b/spec/public/resource_spec.rb index 6c9d7a3e..0eedb816 100644 --- a/spec/public/resource_spec.rb +++ b/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 diff --git a/spec/public/shared/resource_shared_spec.rb b/spec/public/shared/resource_shared_spec.rb index 9cb501cd..bfa703c1 100644 --- a/spec/public/shared/resource_shared_spec.rb +++ b/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 diff --git a/spec/public/transaction_spec.rb b/spec/public/transaction_spec.rb index 9ae46292..2f2470eb 100644 --- a/spec/public/transaction_spec.rb +++ b/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 diff --git a/spec/semipublic/associations/many_to_one_spec.rb b/spec/semipublic/associations/many_to_one_spec.rb index 203baa50..810034bf 100644 --- a/spec/semipublic/associations/many_to_one_spec.rb +++ b/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' diff --git a/spec/semipublic/resource_spec.rb b/spec/semipublic/resource_spec.rb index 13c1fffe..1e94e9cb 100644 --- a/spec/semipublic/resource_spec.rb +++ b/spec/semipublic/resource_spec.rb @@ -10,30 +10,6 @@ class ::User property :description, Text 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 - - belongs_to :user - end - - class ::Default - include DataMapper::Resource - - property :name, String, :key => true, :default => 'a default value' - end - @user_model = User end diff --git a/spec/semipublic/shared/resource_shared_spec.rb b/spec/semipublic/shared/resource_shared_spec.rb index 69c0faa9..4f678759 100644 --- a/spec/semipublic/shared/resource_shared_spec.rb +++ b/spec/semipublic/shared/resource_shared_spec.rb @@ -5,51 +5,6 @@ end end - it { @user.should respond_to(:dirty?) } - - describe '#dirty?' do - - describe 'on a non-dirty record' do - - it { @user.should_not be_dirty } - - end - - describe 'on a dirty record' do - - before { @user.age = 100 } - - it { @user.should be_dirty } - - end - - describe 'on a new record, with no 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 attributes, no default attributes, and an identity field' do - - before { @comment = Comment.new } - - it { @comment.should be_dirty } - - end - - describe 'on a new record, with no attributes, default attributes, and no identity field' do - - before { @default = Default.new } - - it { @default.should be_dirty } - - end - - - end - it { @user.should respond_to(:attribute_dirty?) } describe '#attribute_dirty?' do