Updated Resource#eql? to be strict match

* Resource#eql? only matches other Resources if they use the exact same
  model.  Resource#== will match other Resources that have a common base
  model.

lib/dm-core/associations/relationship.rb

@@ -186,10 +186,6 @@ def with_repository(object = nil)
           other_model = object.model == child_model ? parent_model : child_model
         end
 
-        if object.kind_of?(DataMapper::Resource)
-          other_model = object == child_model ? parent_model : child_model
-        end
-
         if other_model && other_model.repository == object.repository && object.repository.name != @repository_name
           object.repository.scope { |block_args| yield(*block_args) }
         else

lib/dm-core/resource.rb

@@ -150,31 +150,50 @@ def attribute_set(name, value)
     end
 
     ##
-    # Tests equality of +other+ with receiver
-    # Receiver is equal to +other+ if they are the same object (identity)
-    # or if they are both of the same class and all of their attributes are equal
+    # Tests the equality with another Resource
     #
-    # XXX:
-    # Is the intention to check inheritance (as it is currently):
-    #   return false unless other.kind_of?(model)
-    # or rather is it to check class equality? eg.,
-    #   return false unless other.class == model
+    # Resource is equal to +other+ if they are the same object (identity)
+    # or if they are both of the *same model* and all of their attributes
+    # are equal
     #
-    # @param [Object] other
-    #   Object to compare to
+    # @param [DataMapper::Resource] other
+    #   Resource to compare to
     #
     # @return [TrueClass, FalseClass]
     #   the outcome of the comparison as a boolean
     #
     # @api public
     def eql?(other)
       return true if equal?(other)
-      return false unless other.kind_of?(model)
+      return false unless other.respond_to?(:model) && model.equal?(other.model)
       return true if repository == other.repository && key == other.key && !dirty? && !other.dirty?
+      # TODO: figure out an approach that will only compare loaded
+      # attributes to avoid unecessary lazy loading
       properties.all? { |p| p.get(self) == p.get(other) }
     end
 
-    alias == eql?
+    ##
+    # Tests the equality with another Resource
+    #
+    # Resource is equal to +other+ if they are the same object (identity)
+    # or if they are both of the *same base model* and all of their attributes
+    # are equal
+    #
+    # @param [DataMapper::Resource] other
+    #   Resource to compare to
+    #
+    # @return [TrueClass, FalseClass]
+    #   the outcome of the comparison as a boolean
+    #
+    # @api public
+    def ==(other)
+      return true if equal?(other)
+      return false unless other.respond_to?(:model) && model.base_model.equal?(other.model.base_model)
+      return true if repository == other.repository && key == other.key && !dirty? && !other.dirty?
+      # TODO: figure out an approach that will only compare loaded
+      # attributes to avoid unecessary lazy loading
+      properties.all? { |p| p.get(self) == p.get(other) }
+    end
 
     ##
     # Compares two Resources to allow them to be sorted
@@ -353,8 +372,9 @@ def dirty_attributes
     # @api semipublic
     def dirty?
       return true if dirty_attributes.any?
-      return false unless new_record?
-      model.identity_field || properties.any? { |p| !p.default_for(self).nil? }
+      if new_record?
+        model.identity_field || properties.any? { |p| !p.default_for(self).nil? }
+      end
     end
 
     ##

spec/public/associations/many_to_one_spec.rb

@@ -12,6 +12,8 @@ class User
       has n, :comments
     end
 
+    class Author < User; end
+
     class Article
       include DataMapper::Resource
 
@@ -40,6 +42,11 @@ class Paragraph
     end
   end
 
+  after do
+    # FIXME: should not need to clear STI models explicitly
+    Object.send(:remove_const, :Author) if defined?(Author)
+  end
+
   supported_by :all do
     before do
       @comment     = Comment.create(:body => "Cool spec",

spec/public/resource_spec.rb

@@ -12,6 +12,8 @@ class User
       has n, :comments
     end
 
+    class Author < User; end
+
     class Article
       include DataMapper::Resource
 
@@ -40,6 +42,11 @@ class Paragraph
     end
   end
 
+  after do
+    # FIXME: should not need to clear STI models explicitly
+    Object.send(:remove_const, :Author) if defined?(Author)
+  end
+
   supported_by :all do
     before do
       @model       = User

spec/public/shared/resource_shared_spec.rb

@@ -5,6 +5,92 @@
     end
   end
 
+  [ :==, :=== ].each do |method|
+    it { @user.should respond_to(method) }
+
+    describe "##{method}" do
+      describe 'when comparing to the same object' do
+        before do
+          @other  = @user
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return true' do
+          @return.should be_true
+        end
+      end
+
+      describe 'when comparing to an object that does not respond to model' do
+        before do
+          @other  = Object.new
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return false' do
+          @return.should be_false
+        end
+      end
+
+      describe 'when comparing to a resource with the same properties, but the model is a subclass' do
+        before do
+          @other  = Author.new(@user.attributes)
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return true' do
+          @return.should be_true
+        end
+      end
+
+      describe 'when comparing to a resource with the same repository, key and neither self or the other resource is dirty' do
+        before do
+          @other  = @model.get(@user.key)
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return true' do
+          @return.should be_true
+        end
+      end
+
+      describe 'when comparing to a resource with the same repository, key but either self or the other resource is dirty' do
+        before do
+          @user.age = 20
+          @other  = @model.get(@user.key)
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return false' do
+          @return.should be_false
+        end
+      end
+
+      describe 'when comparing to a resource with the same properties' do
+        before do
+          @other  = @model.new(@user.attributes)
+          @return = @user.send(method, @other)
+        end
+
+        it 'should return true' do
+          @return.should be_true
+        end
+      end
+
+      with_alternate_adapter do
+        describe 'when comparing to a resource with a different repository, but the same properties' do
+          before do
+            @other = repository(:alternate) { @model.create(@user.attributes) }
+            @return = @user.send(method, @other)
+          end
+
+          it 'should return true' do
+            @return.should be_true
+          end
+        end
+      end
+    end
+  end
+
   it { @user.should respond_to(:<=>) }
 
   describe '#<=>' do
@@ -130,38 +216,88 @@
 
   end
 
-  [ :eql?, :==, :=== ].each do |method|
+  it { @user.should respond_to(:eql?) }
 
-    it { @user.should respond_to(method) }
+  describe '#eql?' do
+    describe 'when comparing to the same object' do
+      before do
+        @other  = @user
+        @return = @user.eql?(@other)
+      end
 
-    describe "##{method}" do
+      it 'should return true' do
+        @return.should be_true
+      end
+    end
+
+    describe 'when comparing to an object that does not respond to model' do
+      before do
+        @other  = Object.new
+        @return = @user.eql?(@other)
+      end
+
+      it 'should return false' do
+        @return.should be_false
+      end
+    end
+
+    describe 'when comparing to a resource with the same properties, but the model is a subclass' do
+      before do
+        @other  = Author.new(@user.attributes)
+        @return = @user.eql?(@other)
+      end
 
-      it "should be true when they are the same objects" do
-        @user.send(method, @user).should be_true
+      it 'should return false' do
+        @return.should be_false
       end
+    end
 
-      it "should be true when all the attributes are the same" do
-        @user.send(method, @model.get(@user.key)).should be_true
+    describe 'when comparing to a resource with the same repository, key and neither self or the other resource is dirty' do
+      before do
+        @other  = @model.get(@user.key)
+        @return = @user.eql?(@other)
       end
 
-      it "should be true when all the attributes are the same even if one has not been persisted" do
-        @model.get(@user.key).send(method, @model.new(:name => "dbussink", :age => 25, :description => "Test")).should be_true
+      it 'should return true' do
+        @return.should be_true
       end
+    end
 
-      it "should not be true when the attributes differ even if the keys are the same" do
+    describe 'when comparing to a resource with the same repository, key but either self or the other resource is dirty' do
+      before do
         @user.age = 20
-        @user.send(method, @model.get(@user.key)).should be_false
+        @other  = @model.get(@user.key)
+        @return = @user.eql?(@other)
       end
 
-      with_alternate_adapter do
-        it "should be true when they are instances from different repositories, but the keys and attributes are the same" do
-          @other = repository(:alternate) { @model.create(:name => "dbussink", :age => 25, :description => "Test") }
-          @user.send(method, @other).should be_true
-        end
+      it 'should return false' do
+        @return.should be_false
+      end
+    end
+
+    describe 'when comparing to a resource with the same properties' do
+      before do
+        @other  = @model.new(@user.attributes)
+        @return = @user.eql?(@other)
       end
 
+      it 'should return true' do
+        @return.should be_true
+      end
     end
 
+    with_alternate_adapter do
+      describe 'when comparing to a resource with a different repository, but the same properties' do
+        before do
+          @other = repository(:alternate) { @model.create(@user.attributes) }
+          @return = @user.eql?(@other)
+        end
+
+        it 'should return true' do
+          @return.should be_true
+        end
+      end
+    end
   end
 
   it { @user.should respond_to(:inspect) }